From 25ab98027d5181c30871394066b1fedc29cc90d3 Mon Sep 17 00:00:00 2001 From: David Majda Date: Sat, 12 Sep 2015 21:17:28 +0200 Subject: [PATCH] Remove info about found string from syntax errors The |found| property wasn't very useful as it mostly contained just one character or |null| (the exception being syntax errors triggered by |error| or |expected|). Similarly, the "but XXX found" part of the error message (based on the |found| property) wasn't much useful and was redundant in presence of location info. For these reasons, this commit removes the |found| property and corresponding part of the error message from syntax errors. It also modifies error location info slightly to cover a range of 0 characters, not 1 character (except when the error is triggered by |error| or |expected|). This corresponds more precisely to the actual situation. Fixes #372. --- README.md | 4 +- lib/compiler/passes/generate-js.js | 50 ++------------ lib/parser.js | 40 ++--------- .../generated-parser-behavior.spec.js | 69 ++++--------------- 4 files changed, 29 insertions(+), 134 deletions(-) diff --git a/README.md b/README.md index 9c15bac..c7f0043 100644 --- a/README.md +++ b/README.md @@ -127,8 +127,8 @@ Using the Parser Using the generated parser is simple — just call its `parse` method and pass an input string as a parameter. The method will return a parse result (the exact value depends on the grammar used to build the parser) or throw an exception if -the input is invalid. The exception will contain `location`, `expected`, `found` -and `message` properties with more details about the error. +the input is invalid. The exception will contain `location`, `expected` and +`message` properties with more details about the error. parser.parse("abba"); // returns ["a", "b", "b", "a"] diff --git a/lib/compiler/passes/generate-js.js b/lib/compiler/passes/generate-js.js index 7ff884b..090c856 100644 --- a/lib/compiler/passes/generate-js.js +++ b/lib/compiler/passes/generate-js.js @@ -777,10 +777,9 @@ function generateJS(ast, options) { ' child.prototype = new ctor();', ' }', '', - ' function peg$SyntaxError(message, expected, found, location) {', + ' function peg$SyntaxError(message, expected, location) {', ' this.message = message;', ' this.expected = expected;', - ' this.found = found;', ' this.location = location;', ' this.name = "SyntaxError";', '', @@ -972,7 +971,6 @@ function generateJS(ast, options) { ' throw peg$buildException(', ' null,', ' [{ type: "other", description: description }],', - ' input.substring(peg$savedPos, peg$currPos),', ' peg$computeLocation(peg$savedPos, peg$currPos)', ' );', ' }', @@ -981,7 +979,6 @@ function generateJS(ast, options) { ' throw peg$buildException(', ' message,', ' null,', - ' input.substring(peg$savedPos, peg$currPos),', ' peg$computeLocation(peg$savedPos, peg$currPos)', ' );', ' }', @@ -1057,7 +1054,7 @@ function generateJS(ast, options) { ' peg$maxFailExpected.push(expected);', ' }', '', - ' function peg$buildException(message, expected, found, location) {', + ' function peg$buildException(message, expected, location) {', ' function cleanupExpected(expected) {', ' var i = 1;', '', @@ -1085,36 +1082,9 @@ function generateJS(ast, options) { ' }', ' }', '', - ' function buildMessage(expected, found) {', - ' function stringEscape(s) {', - ' function hex(ch) { return ch.charCodeAt(0).toString(16).toUpperCase(); }', - '', - /* - * ECMA-262, 5th ed., 7.8.4: All characters may appear literally in a string - * literal except for the closing quote character, backslash, carriage - * return, line separator, paragraph separator, and line feed. Any character - * may appear in the form of an escape sequence. - * - * For portability, we also escape all control and non-ASCII characters. - * Note that "\0" and "\v" escape sequences are not used because JSHint does - * not like the first and IE the second. - */ - ' return s', - ' .replace(/\\\\/g, \'\\\\\\\\\')', // backslash - ' .replace(/"/g, \'\\\\"\')', // closing double quote - ' .replace(/\\x08/g, \'\\\\b\')', // backspace - ' .replace(/\\t/g, \'\\\\t\')', // horizontal tab - ' .replace(/\\n/g, \'\\\\n\')', // line feed - ' .replace(/\\f/g, \'\\\\f\')', // form feed - ' .replace(/\\r/g, \'\\\\r\')', // carriage return - ' .replace(/[\\x00-\\x07\\x0B\\x0E\\x0F]/g, function(ch) { return \'\\\\x0\' + hex(ch); })', - ' .replace(/[\\x10-\\x1F\\x80-\\xFF]/g, function(ch) { return \'\\\\x\' + hex(ch); })', - ' .replace(/[\\u0100-\\u0FFF]/g, function(ch) { return \'\\\\u0\' + hex(ch); })', - ' .replace(/[\\u1000-\\uFFFF]/g, function(ch) { return \'\\\\u\' + hex(ch); });', - ' }', - '', + ' function buildMessage(expected) {', ' var expectedDescs = new Array(expected.length),', - ' expectedDesc, foundDesc, i;', + ' expectedDesc, i;', '', ' for (i = 0; i < expected.length; i++) {', ' expectedDescs[i] = expected[i].description;', @@ -1126,9 +1096,7 @@ function generateJS(ast, options) { ' + expectedDescs[expected.length - 1]', ' : expectedDescs[0];', '', - ' foundDesc = found ? "\\"" + stringEscape(found) + "\\"" : "end of input";', - '', - ' return "Expected " + expectedDesc + " but " + foundDesc + " found.";', + ' return "Expected " + expectedDesc + ".";', ' }', '', ' if (expected !== null) {', @@ -1136,9 +1104,8 @@ function generateJS(ast, options) { ' }', '', ' return new peg$SyntaxError(', - ' message !== null ? message : buildMessage(expected, found),', + ' message !== null ? message : buildMessage(expected),', ' expected,', - ' found,', ' location', ' );', ' }', @@ -1178,10 +1145,7 @@ function generateJS(ast, options) { ' throw peg$buildException(', ' null,', ' peg$maxFailExpected,', - ' peg$maxFailPos < input.length ? input.charAt(peg$maxFailPos) : null,', - ' peg$maxFailPos < input.length', - ' ? peg$computeLocation(peg$maxFailPos, peg$maxFailPos + 1)', - ' : peg$computeLocation(peg$maxFailPos, peg$maxFailPos)', + ' peg$computeLocation(peg$maxFailPos, peg$maxFailPos)', ' );', ' }', ' }', diff --git a/lib/parser.js b/lib/parser.js index cda8fad..70b6894 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -13,10 +13,9 @@ module.exports = (function() { child.prototype = new ctor(); } - function peg$SyntaxError(message, expected, found, location) { + function peg$SyntaxError(message, expected, location) { this.message = message; this.expected = expected; - this.found = found; this.location = location; this.name = "SyntaxError"; @@ -407,7 +406,6 @@ module.exports = (function() { throw peg$buildException( null, [{ type: "other", description: description }], - input.substring(peg$savedPos, peg$currPos), peg$computeLocation(peg$savedPos, peg$currPos) ); } @@ -416,7 +414,6 @@ module.exports = (function() { throw peg$buildException( message, null, - input.substring(peg$savedPos, peg$currPos), peg$computeLocation(peg$savedPos, peg$currPos) ); } @@ -492,7 +489,7 @@ module.exports = (function() { peg$maxFailExpected.push(expected); } - function peg$buildException(message, expected, found, location) { + function peg$buildException(message, expected, location) { function cleanupExpected(expected) { var i = 1; @@ -515,26 +512,9 @@ module.exports = (function() { } } - function buildMessage(expected, found) { - function stringEscape(s) { - function hex(ch) { return ch.charCodeAt(0).toString(16).toUpperCase(); } - - return s - .replace(/\\/g, '\\\\') - .replace(/"/g, '\\"') - .replace(/\x08/g, '\\b') - .replace(/\t/g, '\\t') - .replace(/\n/g, '\\n') - .replace(/\f/g, '\\f') - .replace(/\r/g, '\\r') - .replace(/[\x00-\x07\x0B\x0E\x0F]/g, function(ch) { return '\\x0' + hex(ch); }) - .replace(/[\x10-\x1F\x80-\xFF]/g, function(ch) { return '\\x' + hex(ch); }) - .replace(/[\u0100-\u0FFF]/g, function(ch) { return '\\u0' + hex(ch); }) - .replace(/[\u1000-\uFFFF]/g, function(ch) { return '\\u' + hex(ch); }); - } - + function buildMessage(expected) { var expectedDescs = new Array(expected.length), - expectedDesc, foundDesc, i; + expectedDesc, i; for (i = 0; i < expected.length; i++) { expectedDescs[i] = expected[i].description; @@ -546,9 +526,7 @@ module.exports = (function() { + expectedDescs[expected.length - 1] : expectedDescs[0]; - foundDesc = found ? "\"" + stringEscape(found) + "\"" : "end of input"; - - return "Expected " + expectedDesc + " but " + foundDesc + " found."; + return "Expected " + expectedDesc + "."; } if (expected !== null) { @@ -556,9 +534,8 @@ module.exports = (function() { } return new peg$SyntaxError( - message !== null ? message : buildMessage(expected, found), + message !== null ? message : buildMessage(expected), expected, - found, location ); } @@ -4959,10 +4936,7 @@ module.exports = (function() { throw peg$buildException( null, peg$maxFailExpected, - peg$maxFailPos < input.length ? input.charAt(peg$maxFailPos) : null, - peg$maxFailPos < input.length - ? peg$computeLocation(peg$maxFailPos, peg$maxFailPos + 1) - : peg$computeLocation(peg$maxFailPos, peg$maxFailPos) + peg$computeLocation(peg$maxFailPos, peg$maxFailPos) ); } } diff --git a/spec/behavior/generated-parser-behavior.spec.js b/spec/behavior/generated-parser-behavior.spec.js index af73e1a..acdd064 100644 --- a/spec/behavior/generated-parser-behavior.spec.js +++ b/spec/behavior/generated-parser-behavior.spec.js @@ -1268,9 +1268,8 @@ describe("generated parser behavior", function() { ); expect(parser).toFailToParse("a", { - message: 'Expected a but "a" found.', + message: 'Expected a.', expected: [{ type: "other", description: "a" }], - found: "a", location: { start: { offset: 0, line: 1, column: 1 }, end: { offset: 1, line: 1, column: 2 } @@ -1287,7 +1286,6 @@ describe("generated parser behavior", function() { expect(parser).toFailToParse("a", { message: "a", expected: null, - found: "a", location: { start: { offset: 0, line: 1, column: 1 }, end: { offset: 1, line: 1, column: 2 } @@ -1403,26 +1401,12 @@ describe("generated parser behavior", function() { }); }); - describe("found string reporting", function() { - it("reports found string correctly at the end of input", function() { - var parser = PEG.buildParser('start = "a"', options); - - expect(parser).toFailToParse("", { found: null }); - }); - - it("reports found string correctly in the middle of input", function() { - var parser = PEG.buildParser('start = "a"', options); - - expect(parser).toFailToParse("b", { found: "b" }); - }); - }); - describe("message building", function() { it("builds message correctly with no alternative", function() { var parser = PEG.buildParser('start = "a"', options); expect(parser).toFailToParse("ab", { - message: 'Expected end of input but "b" found.' + message: 'Expected end of input.' }); }); @@ -1430,7 +1414,7 @@ describe("generated parser behavior", function() { var parser = PEG.buildParser('start = "a"', options); expect(parser).toFailToParse("b", { - message: 'Expected "a" but "b" found.' + message: 'Expected "a".' }); }); @@ -1438,46 +1422,19 @@ describe("generated parser behavior", function() { var parser = PEG.buildParser('start = "a" / "b" / "c"', options); expect(parser).toFailToParse("d", { - message: 'Expected "a", "b" or "c" but "d" found.' - }); - }); - - it("builds message correctly at the end of input", function() { - var parser = PEG.buildParser('start = "a"', options); - - expect(parser).toFailToParse("", { - message: 'Expected "a" but end of input found.' - }); - }); - - it("builds message correctly in the middle of input", function() { - var parser = PEG.buildParser('start = "a"', options); - - expect(parser).toFailToParse("b", { - message: 'Expected "a" but "b" found.' + message: 'Expected "a", "b" or "c".' }); }); }); describe("position reporting", function() { - it("reports position correctly at the end of input", function() { - var parser = PEG.buildParser('start = "a"', options); - - expect(parser).toFailToParse("", { - location: { - start: { offset: 0, line: 1, column: 1 }, - end: { offset: 0, line: 1, column: 1 } - } - }); - }); - - it("reports position correctly in the middle of input", function() { + it("reports position correctly with no trailing input", function() { var parser = PEG.buildParser('start = "a"', options); expect(parser).toFailToParse("b", { location: { start: { offset: 0, line: 1, column: 1 }, - end: { offset: 1, line: 1, column: 2 } + end: { offset: 0, line: 1, column: 1 } } }); }); @@ -1488,7 +1445,7 @@ describe("generated parser behavior", function() { expect(parser).toFailToParse("aa", { location: { start: { offset: 1, line: 1, column: 2 }, - end: { offset: 2, line: 1, column: 3 } + end: { offset: 1, line: 1, column: 2 } } }); }); @@ -1504,7 +1461,7 @@ describe("generated parser behavior", function() { expect(parser).toFailToParse("1\n2\n\n3\n\n\n4 5 x", { location: { start: { offset: 13, line: 7, column: 5 }, - end: { offset: 14, line: 7, column: 6 } + end: { offset: 13, line: 7, column: 5 } } }); @@ -1512,19 +1469,19 @@ describe("generated parser behavior", function() { expect(parser).toFailToParse("1\rx", { // Old Mac location: { start: { offset: 2, line: 2, column: 1 }, - end: { offset: 3, line: 2, column: 2 } + end: { offset: 2, line: 2, column: 1 } } }); expect(parser).toFailToParse("1\r\nx", { // Windows location: { start: { offset: 3, line: 2, column: 1 }, - end: { offset: 4, line: 2, column: 2 } + end: { offset: 3, line: 2, column: 1 } } }); expect(parser).toFailToParse("1\n\rx", { // mismatched location: { start: { offset: 3, line: 3, column: 1 }, - end: { offset: 4, line: 3, column: 2 } + end: { offset: 3, line: 3, column: 1 } } }); @@ -1532,13 +1489,13 @@ describe("generated parser behavior", function() { expect(parser).toFailToParse("1\u2028x", { // line separator location: { start: { offset: 2, line: 2, column: 1 }, - end: { offset: 3, line: 2, column: 2 } + end: { offset: 2, line: 2, column: 1 } } }); expect(parser).toFailToParse("1\u2029x", { // paragraph separator location: { start: { offset: 2, line: 2, column: 1 }, - end: { offset: 3, line: 2, column: 2 } + end: { offset: 2, line: 2, column: 1 } } }); });