Repository: jneen/parsimmon
Pull request #256 (2018-06-14 21:47:52)

Allow multiple expected values to be passed into desc and makeFailure calls
There are certain parsers that will accept multiple strings/characters, but only return one expected value - most parsers created through the use of `Parsimmon.test()` fall into this category.  For example, `oneOf('+-%')` returns `'expected a character/byte matching function [...]'`, whereas in most cases it should probably return `expected one of '+', '-', '%'`.  Especially when you have parsers that include branches, and description arrays are merged, it can become helpful to be able to specify arrays in `.desc()` and `makeFailure()`

I've made small changes to the `.desc()` and `makeFailure()` functions, and added tests to cover both of these changes.  (None of the existing tests needed changing, although I did move the existing `makeFailure` test into a suite given that there are now multiple tests for that function.)  I've also made changes to the API.md file to reflect the addition to the API.

I believe the changes are backwards compatible, and constitute a minor semver change.

It might also be worth at this point changing `oneOf()` to modify the description of the returned parser.  I wanted to make these changes because the 'matching function blah' error message is not very helpful.  However, that arguably constitutes an additional change, and is probably closer to breaking backwards compatibility, at least if people rely on the output of the error message, so I haven't done it in this PR, but I can add it if you want.


Patch:
diff --git a/API.md b/API.md
index 150ebf2..e91a48c 100644
--- a/API.md
+++ b/API.md
@@ -187,7 +187,7 @@ To be used inside of `Parsimmon(fn)`. Generates an object describing how far the
 
 ## Parsimmon.makeFailure(furthest, expectation)
 
-To be used inside of `Parsimmon(fn)`. Generates an object describing how far the unsuccessful parse went (`index`), and what kind of syntax it expected to see (`expectation`). See documentation for `Parsimmon(fn)`.
+To be used inside of `Parsimmon(fn)`. Generates an object describing how far the unsuccessful parse went (`index`), and what kind of syntax it expected to see (`expectation`). The expected value may also be an array of different values.  See documentation for `Parsimmon(fn)`.
 
 ## Parsimmon.isParser(obj)
 
@@ -961,7 +961,9 @@ Identifier.tryParse('hey');
 ## parser.desc(description)
 
 Returns a new parser whose failure message is `description`. For example, `string('x').desc('the letter x')` will indicate that
-`'the letter x'` was expected.
+`'the letter x'` was expected.  Alternatively, an array of failure messages can be passed, if the parser represents multiple
+options.  For example, `oneOf('abc').desc(['a', 'b', 'c'])` will indicate that any of 'a', 'b', or 'c' would be acceptable in
+this case.
 
 It is important to only add descriptions to "low-level" parsers; things like numbers and strings. If you add a description to *every* parser you write then generated error messages will not be very helpful when simple syntax errors occur.
 
diff --git a/package-lock.json b/package-lock.json
index c4d7c3b..7e2aa17 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -4329,12 +4329,14 @@
         "balanced-match": {
           "version": "1.0.0",
           "bundled": true,
-          "dev": true
+          "dev": true,
+          "optional": true
         },
         "brace-expansion": {
           "version": "1.1.11",
           "bundled": true,
           "dev": true,
+          "optional": true,
           "requires": {
             "balanced-match": "^1.0.0",
             "concat-map": "0.0.1"
@@ -4349,17 +4351,20 @@
         "code-point-at": {
           "version": "1.1.0",
           "bundled": true,
-          "dev": true
+          "dev": true,
+          "optional": true
         },
         "concat-map": {
           "version": "0.0.1",
           "bundled": true,
-          "dev": true
+          "dev": true,
+          "optional": true
         },
         "console-control-strings": {
           "version": "1.1.0",
           "bundled": true,
-          "dev": true
+          "dev": true,
+          "optional": true
         },
         "core-util-is": {
           "version": "1.0.2",
@@ -4476,7 +4481,8 @@
         "inherits": {
           "version": "2.0.3",
           "bundled": true,
-          "dev": true
+          "dev": true,
+          "optional": true
         },
         "ini": {
           "version": "1.3.5",
@@ -4488,6 +4494,7 @@
           "version": "1.0.0",
           "bundled": true,
           "dev": true,
+          "optional": true,
           "requires": {
             "number-is-nan": "^1.0.0"
           }
@@ -4502,6 +4509,7 @@
           "version": "3.0.4",
           "bundled": true,
           "dev": true,
+          "optional": true,
           "requires": {
             "brace-expansion": "^1.1.7"
           }
@@ -4509,12 +4517,14 @@
         "minimist": {
           "version": "0.0.8",
           "bundled": true,
-          "dev": true
+          "dev": true,
+          "optional": true
         },
         "minipass": {
           "version": "2.2.4",
           "bundled": true,
           "dev": true,
+          "optional": true,
           "requires": {
             "safe-buffer": "^5.1.1",
             "yallist": "^3.0.0"
@@ -4533,6 +4543,7 @@
           "version": "0.5.1",
           "bundled": true,
           "dev": true,
+          "optional": true,
           "requires": {
             "minimist": "0.0.8"
           }
@@ -4613,7 +4624,8 @@
         "number-is-nan": {
           "version": "1.0.1",
           "bundled": true,
-          "dev": true
+          "dev": true,
+          "optional": true
         },
         "object-assign": {
           "version": "4.1.1",
@@ -4625,6 +4637,7 @@
           "version": "1.4.0",
           "bundled": true,
           "dev": true,
+          "optional": true,
           "requires": {
             "wrappy": "1"
           }
@@ -4746,6 +4759,7 @@
           "version": "1.0.2",
           "bundled": true,
           "dev": true,
+          "optional": true,
           "requires": {
             "code-point-at": "^1.0.0",
             "is-fullwidth-code-point": "^1.0.0",
diff --git a/src/parsimmon.js b/src/parsimmon.js
index 155a5c5..bf4c1cc 100644
--- a/src/parsimmon.js
+++ b/src/parsimmon.js
@@ -239,12 +239,15 @@ function makeSuccess(index, value) {
 }
 
 function makeFailure(index, expected) {
+  if (!isArray(expected)) {
+    expected = [expected];
+  }
   return {
     status: false,
     index: -1,
     value: null,
     furthest: index,
-    expected: [expected]
+    expected: expected
   };
 }
 
@@ -775,11 +778,14 @@ _.notFollowedBy = function(x) {
 };
 
 _.desc = function(expected) {
+  if (!isArray(expected)) {
+    expected = [expected];
+  }
   var self = this;
   return Parsimmon(function(input, i) {
     var reply = self._(input, i);
     if (!reply.status) {
-      reply.expected = [expected];
+      reply.expected = expected;
     }
     return reply;
   });
@@ -923,15 +929,19 @@ function test(predicate) {
 }
 
 function oneOf(str) {
+  var expected = str.split("");
+  for (var idx = 0; idx < expected.length; idx++) {
+    expected[idx] = "'" + expected[idx] + "'";
+  }
   return test(function(ch) {
     return str.indexOf(ch) >= 0;
-  });
+  }).desc(expected);
 }
 
 function noneOf(str) {
   return test(function(ch) {
     return str.indexOf(ch) < 0;
-  });
+  }).desc("none of '" + str + "'");
 }
 
 function custom(parsingFunction) {


Test patch:
diff --git a/test/core/desc.test.js b/test/core/desc.test.js
index 20cc2a1..4854fa3 100644
--- a/test/core/desc.test.js
+++ b/test/core/desc.test.js
@@ -56,4 +56,33 @@ suite("desc", function() {
       expected: ["the letter y"]
     });
   });
+
+  test("allows multiple descriptions to be passed as an array", function() {
+    var x = Parsimmon.oneOf("xyz")
+      .desc(["x", "y", "z"])
+      .atLeast(1);
+    var y = Parsimmon.oneOf("abc").desc(["a", "b", "c"]);
+
+    var parser = x.then(y);
+
+    assert.deepEqual(parser.parse("~"), {
+      status: false,
+      index: {
+        offset: 0,
+        line: 1,
+        column: 1
+      },
+      expected: ["x", "y", "z"]
+    });
+
+    assert.deepEqual(parser.parse("x"), {
+      status: false,
+      index: {
+        offset: 1,
+        line: 1,
+        column: 2
+      },
+      expected: ["a", "b", "c", "x", "y", "z"]
+    });
+  });
 });
diff --git a/test/core/makeFailure.test.js b/test/core/makeFailure.test.js
index eb9c95e..57494fc 100644
--- a/test/core/makeFailure.test.js
+++ b/test/core/makeFailure.test.js
@@ -1,14 +1,28 @@
 "use strict";
 
-test("Parsimmon.makeFailure", function() {
-  var furthest = 4444;
-  var expected = "waiting in the clock tower";
-  var result = Parsimmon.makeFailure(furthest, expected);
-  assert.deepEqual(result, {
-    status: false,
-    index: -1,
-    value: null,
-    furthest: furthest,
-    expected: [expected]
+suite("Parsimmon.makeFailure", function() {
+  test("creates a failure result", function() {
+    var furthest = 4444;
+    var expected = "waiting in the clock tower";
+    var result = Parsimmon.makeFailure(furthest, expected);
+    assert.deepEqual(result, {
+      status: false,
+      index: -1,
+      value: null,
+      furthest: furthest,
+      expected: [expected]
+    });
+  });
+  test("creates a result with multiple expected values", function() {
+    var furthest = 4444;
+    var expected = ["once", "twice", "three times a lady"];
+    var result = Parsimmon.makeFailure(furthest, expected);
+    assert.deepEqual(result, {
+      status: false,
+      index: -1,
+      value: null,
+      furthest: furthest,
+      expected: expected
+    });
   });
 });