Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Bug 754921: improve messages with invalid syntax

1) When JSON.parse() failed on invalid JSON (e.g. extraneous trailing
comma) in ArgumentsJSON it raised a SyntaxError with no position
information, which was displayed to the user as:

  Syntax error at line , column : Unexpected token

2) Also before this change all invalid parameter syntax (such as
{{macro(1,)}} matched the ArgumentsJSON production, thus hitting the bug
(1) described above.

3) Also improved a helper in test-macros.js to help newbies (like
myself) understand the structure of the testcase fixtures.
  • Loading branch information...
commit 1e944e00ac308af4753c78f8aaa8e98aeb7143b7 1 parent 7300162
Nickolay Ponomarev nickolay authored
17 lib/kumascript/parser.pegjs
View
@@ -41,7 +41,22 @@ Arguments
= "(" __ args:ArgumentList? __ ")" { return args; }
ArgumentsJSON
- = "(" json_args:[^)]+ ")" { return [JSON.parse(json_args.join(''))]; }
+ = "(" __ &"{" json_args:[^)]+ __ ")" {
+ try {
+ return [JSON.parse(json_args.join(''))];
+ } catch (e) {
+ // Try to provide better diagnostics than
+ // "Syntax error at line , column" using PEG's internal APIs.
+ var errorPosition = computeErrorPosition();
+ throw new PEG.parser.SyntaxError(
+ ["valid JSON object as the parameter of the preceding macro"],
+ json_args.join(''),
+ offset,
+ errorPosition.line,
+ errorPosition.column
+ );
+ }
+ }
ArgumentList
= head:Argument tail:(__ "," __ Argument)* {
9 tests/fixtures/macros-syntax-error-argumentlist.txt
View
@@ -0,0 +1,9 @@
+Testing that an invalid ArgumentsList still
+produces a sensible error message.
+
+{{ add(1,) }}
+---
+Testing that an invalid ArgumentsList still
+produces a sensible error message.
+
+{{ add(1,) }}
16 tests/fixtures/macros-syntax-error-argumentsjson.txt
View
@@ -0,0 +1,16 @@
+Testing that an invalid ArgumentsJSON still
+produces a sensible error message.
+
+{{ MacroWithJson({
+ "es":"es/JavaScript/Acerca_de_JavaScript",
+ "en": "en/JavaScript/About_JavaScript",
+}) }}
+---
+Testing that an invalid ArgumentsJSON still
+produces a sensible error message.
+
+{{ MacroWithJson({
+ "es":"es/JavaScript/Acerca_de_JavaScript",
+ "en": "en/JavaScript/About_JavaScript",
+}) }}
+
35 tests/test-macros.js
View
@@ -20,6 +20,10 @@ function processFixture(test, mp, fixture_path, next) {
src = parts[0],
expected = parts[1],
ctx = new ks_api.APIContext({ });
+ if (parts.length < 2) {
+ throw "Please provide an expected result after '---' in " +
+ fixture_path;
+ }
mp.process(src, ctx, function (errors, result) {
test.equal(result.trim(), expected.trim());
return next(errors, result);
@@ -27,6 +31,26 @@ function processFixture(test, mp, fixture_path, next) {
});
}
+function makeErrorHandlingTestcase(fixtureName) {
+ return function(test) {
+ var mp = new ks_macros.MacroProcessor({
+ loader_class: ks_test_utils.JSONifyLoader
+ });
+ processFixture(test, mp, fixtureName,
+ function (errors, result) {
+
+ test.ok(errors, "There should be errors");
+ test.equal(errors.length, 1, "There should be 1 error");
+
+ var e = errors[0];
+ test.equal(e.name, 'DocumentParsingError');
+ test.notEqual(null, e.message.match(
+ /^Syntax error at line \d+, column \d+:[\s\S]*-+\^/));
+ test.done();
+ });
+ };
+}
+
module.exports = nodeunit.testCase({
"Basic macro substitution should work": function (test) {
@@ -41,7 +65,7 @@ module.exports = nodeunit.testCase({
},
"Errors in document parsing should be handled gracefully and reported": function (test) {
- var mp = new ks_macros.MacroProcessor({
+ var mp = new ks_macros.MacroProcessor({
loader_class: ks_test_utils.JSONifyLoader
});
processFixture(test, mp, 'macros-document-syntax-error.txt',
@@ -58,7 +82,7 @@ module.exports = nodeunit.testCase({
// indicator appears at the expected spot in the context lines
// included in the message.
test.equal(265, e.message.indexOf('-----------------------------^'));
-
+
test.done();
});
},
@@ -196,6 +220,11 @@ module.exports = nodeunit.testCase({
done = true;
}
);
- }
+ },
+
+ "Errors in ArgumentsJSON should be reported with line and column numbers":
+ makeErrorHandlingTestcase('macros-syntax-error-argumentsjson.txt'),
+ "Errors in ArgumentList should be reported with line and column numbers":
+ makeErrorHandlingTestcase('macros-syntax-error-argumentlist.txt'),
});
Please sign in to comment.
Something went wrong with that request. Please try again.