Merge pull request #188 from jneen/topic/test-coverage

Fixes #187 regexp group out of range; 100% tests

Author: Brian Mock

.gitignore

@@ -3,3 +3,4 @@ node_modules
 npm-*.log
 .nyc_output
 coverage
+.DS_Store

API.md

@@ -142,7 +142,7 @@ Parsimmon.test(function(c) {
 });
 ```
 
-## Parsimmon.regexp(regexp, group=0)
+## Parsimmon.regexp(regexp)
 
 Returns a parser that looks for a match to the regexp and yields the entire text matched. The regexp will always match starting at the current parse location. The regexp may only use the following flags: `imu`. Any other flag will result in an error being thrown.
 

CHANGELOG.md

@@ -1,3 +1,8 @@
+## version 1.6.1 (2017-07-01)
+
+* **100% unit test coverage!** This does not mean bugs won't exist, but it keeps us much safer against regressions in the future.
+* **BUGFIX:** `Parsimmon.regexp(regexp, group)` will now correctly fail to parse if the `group` number is out of range for the `regexp` number of groups. This worked correctly in the past, but was broken during a minor code clean up due to missing tests.
+
 ## version 1.6.0 (2017-06-26)
 
 * Adds `Parsimmon.seqObj(...args)`

package.json

@@ -1,6 +1,6 @@
 {
   "name": "parsimmon",
-  "version": "1.6.0",
+  "version": "1.6.1",
   "description": "A monadic LL(infinity) parser combinator library",
   "keywords": [
     "parsing",
@@ -38,7 +38,8 @@
     "precoverage": "npm run test",
     "coverage": "nyc report --reporter=text-lcov | coveralls",
     "pretest": "npm run lint",
-    "test": "nyc node_modules/.bin/mocha -i src --ui tdd --reporter dot test/setup.js test/core/*.test.js test/laws/*.test.js",
+    "test": "nyc --reporter=lcov npm run test:mocha",
+    "test:mocha": "mocha -i src --ui tdd --reporter dot test/setup.js test/core/*.test.js test/laws/*.test.js",
     "watch:test": "mocha --ui tdd --reporter min --watch test/setup.js test/core/*.test.js test/laws/*.test.js"
   }
 }

src/parsimmon.js

@@ -590,11 +590,14 @@ function regexp(re, group) {
   return Parsimmon(function(input, i) {
     var match = anchored.exec(input.slice(i));
     if (match) {
-      var fullMatch = match[0];
-      var groupMatch = match[group];
-      if (groupMatch !== null) {
+      if (0 <= group && group <= match.length) {
+        var fullMatch = match[0];
+        var groupMatch = match[group];
         return makeSuccess(i + fullMatch.length, groupMatch);
       }
+      return makeFailure(
+        'valid match group (0 to ' + match.length + ') in ' + expected
+      );
     }
     return makeFailure(i, expected);
   });

test/.eslintrc

@@ -6,6 +6,8 @@
     "Parsimmon": true,
     "assert": true,
     "suite": true,
+    "setup": true,
+    "teardown": true,
     "test": true
   }
 }

test/core/alt.test.js

@@ -38,4 +38,7 @@ test('Parsimmon.alt', function(){
   assert.throws(function() {
     Parsimmon.alt('not a parser');
   });
+
+
+  assert.strictEqual(Parsimmon.alt().parse('').status, false);
 });

test/core/constructor.test.js

@@ -1,28 +1,48 @@
 'use strict';
 
-test('Parsimmon()', function() {
-  var good = 'just a Q';
-  var bad = 'all I wanted was a Q';
-  var justQ = Parsimmon(function(str, i) {
-    if (str.charAt(i) === 'Q') {
-      return Parsimmon.makeSuccess(i + 1, good);
-    } else {
-      return Parsimmon.makeFailure(i, bad);
-    }
-  });
-  var result1 = justQ.parse('Q');
-  var result2 = justQ.parse('x');
-  assert.deepEqual(result1, {
-    status: true,
-    value: good,
+suite('Parsimmon()', function() {
+
+  test('should work in general', function() {
+    var good = 'just a Q';
+    var bad = 'all I wanted was a Q';
+    var justQ = Parsimmon(function(str, i) {
+      if (str.charAt(i) === 'Q') {
+        return Parsimmon.makeSuccess(i + 1, good);
+      } else {
+        return Parsimmon.makeFailure(i, bad);
+      }
+    });
+    var result1 = justQ.parse('Q');
+    var result2 = justQ.parse('x');
+    assert.deepEqual(result1, {
+      status: true,
+      value: good,
+    });
+    assert.deepEqual(result2, {
+      status: false,
+      index: {
+        offset: 0,
+        line: 1,
+        column: 1
+      },
+      expected: [bad]
+    });
   });
-  assert.deepEqual(result2, {
-    status: false,
-    index: {
-      offset: 0,
-      line: 1,
-      column: 1
-    },
-    expected: [bad]
+
+  test('unsafeUnion works on poorly formatted custom parser', function() {
+    var p1 = Parsimmon.string('a').or(Parsimmon.string('b'));
+    var p2 = Parsimmon(function(str, i) {
+      return {
+        status: false,
+        index: -1,
+        value: null,
+        furthest: i,
+        expected: []
+      };
+    });
+    var p3 = Parsimmon.alt(p2, p1);
+    var result = p3.parse('x');
+    assert.deepStrictEqual(result.expected, ['\'a\'', '\'b\'']);
   });
+
 });

test/core/createLanguage.test.js

@@ -2,6 +2,14 @@
 
 suite('Parsimmon.createLanguage', function() {
 
+  setup(function() {
+    Object.prototype.NASTY = 'dont extend Object.prototype please';
+  });
+
+  teardown(function() {
+    delete Object.prototype.NASTY;
+  });
+
   test('should return an object of parsers', function() {
     var lang = Parsimmon.createLanguage({
       a: function() {
@@ -29,6 +37,16 @@ suite('Parsimmon.createLanguage', function() {
     lang.Parentheses.tryParse('(((())))');
   });
 
+  test('should ignore non-own properties', function() {
+    var obj = Object.create({
+      foo: function() {
+        return Parsimmon.of(1);
+      }
+    });
+    var lang = Parsimmon.createLanguage(obj);
+    assert.strictEqual(lang.foo, undefined);
+  });
+
   test('should allow indirect recursion in parsers', function() {
     var lang = Parsimmon.createLanguage({
       Value: function(r) {

test/core/custom.test.js

@@ -58,4 +58,5 @@ suite('Parsimmon.custom', function(){
 
     assert.deepEqual(parser.parse('acccccb'), {status: true, value: 'acccccb'});
   });
+
 });

test/core/formatError.test.js

@@ -1,14 +1,35 @@
 'use strict';
 
-test('Parsimmon.formatError', function() {
-  var parser =
-    Parsimmon.alt(
-      Parsimmon.fail('a'),
-      Parsimmon.fail('b'),
-      Parsimmon.fail('c')
-    );
-  var expectation = 'expected one of a, b, c, got the end of the input';
-  var input = '';
-  var answer = Parsimmon.formatError(input, parser.parse(input));
-  assert.deepEqual(answer, expectation);
+suite('formatError', function() {
+
+  test('end of input', function() {
+    var parser =
+      Parsimmon.alt(
+        Parsimmon.fail('a'),
+        Parsimmon.fail('b'),
+        Parsimmon.fail('c')
+      );
+    var expectation = 'expected one of a, b, c, got the end of the input';
+    var input = '';
+    var answer = Parsimmon.formatError(input, parser.parse(input));
+    assert.deepEqual(answer, expectation);
+  });
+
+  test('middle of input', function() {
+    var parser =
+      Parsimmon.seq(
+        Parsimmon.string('1'),
+        Parsimmon.alt(
+          Parsimmon.fail('a'),
+          Parsimmon.fail('b'),
+          Parsimmon.fail('c')
+        )
+      );
+    var expectation =
+      'expected one of a, b, c at line 1 column 2, got \'...x11111111111...\'';
+    var input = '1x1111111111111111111111111111';
+    var answer = Parsimmon.formatError(input, parser.parse(input));
+    assert.deepEqual(answer, expectation);
+  });
+
 });

test/core/noneOf.test.js

@@ -0,0 +1,19 @@
+'use strict';
+
+suite('noneOf', function() {
+
+  test('matches EVERYTHING BUT the characters specified', function() {
+    var parser = Parsimmon.noneOf('abc');
+    var a = 'a'.charCodeAt(0);
+    var c = 'c'.charCodeAt(0);
+    for (var i = 0; i < 65535; i++) {
+      var s = String.fromCharCode(i);
+      if (a <= i && i <= c) {
+        assert.strictEqual(parser.parse(s).status, false);
+      } else {
+        assert.strictEqual(parser.parse(s).status, true);
+      }
+    }
+  });
+
+});

test/core/oneOf.test.js

@@ -0,0 +1,19 @@
+'use strict';
+
+suite('oneOf', function() {
+
+  test('matches ONLY the characters specified', function() {
+    var parser = Parsimmon.oneOf('abc');
+    var a = 'a'.charCodeAt(0);
+    var c = 'c'.charCodeAt(0);
+    for (var i = 0; i < 65535; i++) {
+      var s = String.fromCharCode(i);
+      if (a <= i && i <= c) {
+        assert.strictEqual(parser.parse(s).status, true);
+      } else {
+        assert.strictEqual(parser.parse(s).status, false);
+      }
+    }
+  });
+
+});

test/core/parse.test.js

@@ -17,4 +17,13 @@ suite('.parse', function() {
     assert.deepEqual(result.expected, ['a', 'b', 'c']);
   });
 
+  test('throws when given a non-string argument', function() {
+    assert.throws(function() {
+      Parsimmon.of('kaboom').parse(0);
+    });
+    assert.throws(function() {
+      Parsimmon.of('kaboom').parse();
+    });
+  });
+
 });

test/core/regexp.test.js

@@ -43,9 +43,11 @@ suite('Parsimmon.regexp', function() {
     var parser0 = Parsimmon.regexp(/(\w)(\d)/, 0);
     var parser1 = Parsimmon.regexp(/(\w)(\d)/, 1);
     var parser2 = Parsimmon.regexp(/(\w)(\d)/, 2);
-    assert.equal(parser0.parse('a1').value, 'a1');
-    assert.equal(parser1.parse('a1').value, 'a');
-    assert.equal(parser2.parse('a1').value, '1');
+    var parser3 = Parsimmon.regexp(/(\w)(\d)/, 8);
+    assert.strictEqual(parser0.parse('a1').value, 'a1');
+    assert.strictEqual(parser1.parse('a1').value, 'a');
+    assert.strictEqual(parser2.parse('a1').value, '1');
+    assert.strictEqual(parser3.parse('a1').status, false);
   });
 
 });

test/core/seqObj.test.js

@@ -32,6 +32,17 @@ suite('Parsimmon.seqObj', function() {
     });
   });
 
+
+  test('every parser is used', function() {
+    var parser = Parsimmon.seqObj(
+      ['a', Parsimmon.of(1)],
+      ['b', Parsimmon.of(2)],
+      ['c', Parsimmon.fail('oopsy')]
+    );
+    var result = parser.parse('');
+    assert.strictEqual(result.status, false);
+  });
+
   test('every parser is used', function() {
     var parser = Parsimmon.seqObj(
       Parsimmon.string('a'),
@@ -60,6 +71,9 @@ suite('Parsimmon.seqObj', function() {
     assert.throws(function() {
       Parsimmon.seqObj(['cool', 'potato']);
     });
+    assert.throws(function() {
+      Parsimmon.seqObj(0);
+    });
   });
 
 });