From: Alex Lam S.L Date: Sun, 26 Feb 2017 19:40:54 +0000 (+0800) Subject: improve error messages (#1506) X-Git-Url: https://git.ndcode.org/public/gitweb.cgi?a=commitdiff_plain;h=872270b14986b2f24df406425eda5a3bf7a3b56a;p=UglifyJS.git improve error messages (#1506) - better inheritance of `Error` sub-classes - mark parse error against source in CLI closes #235 closes #348 closes #524 closes #1356 closes #1405 --- diff --git a/bin/uglifyjs b/bin/uglifyjs index d0c3abb2..367d66e2 100755 --- a/bin/uglifyjs +++ b/bin/uglifyjs @@ -364,7 +364,21 @@ async.eachLimit(files, 1, function (file, cb) { } catch(ex) { if (ex instanceof UglifyJS.JS_Parse_Error) { print_error("Parse error at " + file + ":" + ex.line + "," + ex.col); - print_error(ex.message); + var col = ex.col; + var line = code.split(/\r?\n/)[ex.line - (col ? 1 : 2)]; + if (line) { + if (col > 40) { + line = line.slice(col - 40); + col = 40; + } + if (col) { + print_error(line.slice(0, 80)); + print_error(line.slice(0, col).replace(/\S/g, " ") + "^"); + } else { + print_error(line.slice(-40)); + print_error(line.slice(-40).replace(/\S/g, " ") + "^"); + } + } print_error(ex.stack); process.exit(1); } @@ -390,7 +404,7 @@ async.eachLimit(files, 1, function (file, cb) { var compressor = COMPRESS && UglifyJS.Compressor(COMPRESS); } catch(ex) { if (ex instanceof UglifyJS.DefaultsError) { - print_error(ex.msg); + print_error(ex.message); print_error("Supported options:"); print_error(sys.inspect(ex.defs)); process.exit(1); diff --git a/lib/parse.js b/lib/parse.js index 37f06df7..9b198ccd 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -195,12 +195,11 @@ function JS_Parse_Error(message, filename, line, col, pos) { this.line = line; this.col = col; this.pos = pos; - this.stack = new Error().stack; -}; - -JS_Parse_Error.prototype.toString = function() { - return this.message + " (line: " + this.line + ", col: " + this.col + ", pos: " + this.pos + ")" + "\n\n" + this.stack; }; +JS_Parse_Error.prototype = Object.create(Error.prototype); +JS_Parse_Error.prototype.constructor = JS_Parse_Error; +JS_Parse_Error.prototype.name = "SyntaxError"; +configure_error_stack(JS_Parse_Error); function js_error(message, filename, line, col, pos) { throw new JS_Parse_Error(message, filename, line, col, pos); @@ -350,13 +349,13 @@ function tokenizer($TEXT, filename, html5_comments, shebang) { }); if (prefix) num = prefix + num; if (RE_OCT_NUMBER.test(num) && next_token.has_directive("use strict")) { - parse_error("SyntaxError: Legacy octal literals are not allowed in strict mode"); + parse_error("Legacy octal literals are not allowed in strict mode"); } var valid = parse_js_number(num); if (!isNaN(valid)) { return token("num", valid); } else { - parse_error("SyntaxError: Invalid syntax: " + num); + parse_error("Invalid syntax: " + num); } }; @@ -395,7 +394,7 @@ function tokenizer($TEXT, filename, html5_comments, shebang) { // Parse if (ch === "0") return "\0"; if (ch.length > 0 && next_token.has_directive("use strict")) - parse_error("SyntaxError: Legacy octal escape sequences are not allowed in strict mode"); + parse_error("Legacy octal escape sequences are not allowed in strict mode"); return String.fromCharCode(parseInt(ch, 8)); } @@ -404,18 +403,18 @@ function tokenizer($TEXT, filename, html5_comments, shebang) { for (; n > 0; --n) { var digit = parseInt(next(true), 16); if (isNaN(digit)) - parse_error("SyntaxError: Invalid hex-character pattern in string"); + parse_error("Invalid hex-character pattern in string"); num = (num << 4) | digit; } return num; }; - var read_string = with_eof_error("SyntaxError: Unterminated string constant", function(quote_char){ + var read_string = with_eof_error("Unterminated string constant", function(quote_char){ var quote = next(), ret = ""; for (;;) { var ch = next(true, true); if (ch == "\\") ch = read_escaped_char(true); - else if (NEWLINE_CHARS(ch)) parse_error("SyntaxError: Unterminated string constant"); + else if (NEWLINE_CHARS(ch)) parse_error("Unterminated string constant"); else if (ch == quote) break; ret += ch; } @@ -440,7 +439,7 @@ function tokenizer($TEXT, filename, html5_comments, shebang) { return next_token; }; - var skip_multiline_comment = with_eof_error("SyntaxError: Unterminated multiline comment", function(){ + var skip_multiline_comment = with_eof_error("Unterminated multiline comment", function(){ var regex_allowed = S.regex_allowed; var i = find("*/", true); var text = S.text.substring(S.pos, i).replace(/\r\n|\r|\u2028|\u2029/g, '\n'); @@ -460,9 +459,9 @@ function tokenizer($TEXT, filename, html5_comments, shebang) { else break; } else { - if (ch != "u") parse_error("SyntaxError: Expecting UnicodeEscapeSequence -- uXXXX"); + if (ch != "u") parse_error("Expecting UnicodeEscapeSequence -- uXXXX"); ch = read_escaped_char(); - if (!is_identifier_char(ch)) parse_error("SyntaxError: Unicode char: " + ch.charCodeAt(0) + " is not valid in identifier"); + if (!is_identifier_char(ch)) parse_error("Unicode char: " + ch.charCodeAt(0) + " is not valid in identifier"); name += ch; backslash = false; } @@ -474,10 +473,10 @@ function tokenizer($TEXT, filename, html5_comments, shebang) { return name; }; - var read_regexp = with_eof_error("SyntaxError: Unterminated regular expression", function(regexp){ + var read_regexp = with_eof_error("Unterminated regular expression", function(regexp){ var prev_backslash = false, ch, in_class = false; while ((ch = next(true))) if (NEWLINE_CHARS(ch)) { - parse_error("SyntaxError: Unexpected line terminator"); + parse_error("Unexpected line terminator"); } else if (prev_backslash) { regexp += "\\" + ch; prev_backslash = false; @@ -498,7 +497,7 @@ function tokenizer($TEXT, filename, html5_comments, shebang) { try { return token("regexp", new RegExp(regexp, mods)); } catch(e) { - parse_error("SyntaxError: " + e.message); + parse_error(e.message); } }); @@ -599,7 +598,7 @@ function tokenizer($TEXT, filename, html5_comments, shebang) { } break; } - parse_error("SyntaxError: Unexpected character '" + ch + "'"); + parse_error("Unexpected character '" + ch + "'"); }; next_token.context = function(nc) { @@ -756,14 +755,14 @@ function parse($TEXT, options) { function unexpected(token) { if (token == null) token = S.token; - token_error(token, "SyntaxError: Unexpected token: " + token.type + " (" + token.value + ")"); + token_error(token, "Unexpected token: " + token.type + " (" + token.value + ")"); }; function expect_token(type, val) { if (is(type, val)) { return next(); } - token_error(S.token, "SyntaxError: Unexpected token " + S.token.type + " «" + S.token.value + "»" + ", expected " + type + " «" + val + "»"); + token_error(S.token, "Unexpected token " + S.token.type + " «" + S.token.value + "»" + ", expected " + type + " «" + val + "»"); }; function expect(punc) { return expect_token("punc", punc); }; @@ -892,7 +891,7 @@ function parse($TEXT, options) { case "return": if (S.in_function == 0 && !options.bare_returns) - croak("SyntaxError: 'return' outside of function"); + croak("'return' outside of function"); return new AST_Return({ value: ( is("punc", ";") ? (next(), null) @@ -909,7 +908,7 @@ function parse($TEXT, options) { case "throw": if (S.token.nlb) - croak("SyntaxError: Illegal newline after 'throw'"); + croak("Illegal newline after 'throw'"); return new AST_Throw({ value: (tmp = expression(true), semicolon(), tmp) }); @@ -925,7 +924,7 @@ function parse($TEXT, options) { case "with": if (S.input.has_directive("use strict")) { - croak("SyntaxError: Strict mode may not include a with statement"); + croak("Strict mode may not include a with statement"); } return new AST_With({ expression : parenthesised(), @@ -945,7 +944,7 @@ function parse($TEXT, options) { // syntactically incorrect if it contains a // LabelledStatement that is enclosed by a // LabelledStatement with the same Identifier as label. - croak("SyntaxError: Label " + label.name + " defined twice"); + croak("Label " + label.name + " defined twice"); } expect(":"); S.labels.push(label); @@ -958,7 +957,7 @@ function parse($TEXT, options) { label.references.forEach(function(ref){ if (ref instanceof AST_Continue) { ref = ref.label.start; - croak("SyntaxError: Continue label `" + label.name + "` refers to non-IterationStatement.", + croak("Continue label `" + label.name + "` refers to non-IterationStatement.", ref.line, ref.col, ref.pos); } }); @@ -978,11 +977,11 @@ function parse($TEXT, options) { if (label != null) { ldef = find_if(function(l){ return l.name == label.name }, S.labels); if (!ldef) - croak("SyntaxError: Undefined label " + label.name); + croak("Undefined label " + label.name); label.thedef = ldef; } else if (S.in_loop == 0) - croak("SyntaxError: " + type.TYPE + " not inside a loop or switch"); + croak(type.TYPE + " not inside a loop or switch"); semicolon(); var stat = new type({ label: label }); if (ldef) ldef.references.push(stat); @@ -998,7 +997,7 @@ function parse($TEXT, options) { : expression(true, true); if (is("operator", "in")) { if (init instanceof AST_Var && init.definitions.length > 1) - croak("SyntaxError: Only one variable declaration allowed in for..in loop"); + croak("Only one variable declaration allowed in for..in loop"); next(); return for_in(init); } @@ -1148,7 +1147,7 @@ function parse($TEXT, options) { }); } if (!bcatch && !bfinally) - croak("SyntaxError: Missing catch/finally blocks"); + croak("Missing catch/finally blocks"); return new AST_Try({ body : body, bcatch : bcatch, @@ -1242,7 +1241,7 @@ function parse($TEXT, options) { break; case "operator": if (!is_identifier_string(tok.value)) { - croak("SyntaxError: Invalid getter/setter name: " + tok.value, + croak("Invalid getter/setter name: " + tok.value, tok.line, tok.col, tok.pos); } ret = _make_symbol(AST_SymbolRef); @@ -1397,7 +1396,7 @@ function parse($TEXT, options) { function as_symbol(type, noerror) { if (!is("name")) { - if (!noerror) croak("SyntaxError: Name expected"); + if (!noerror) croak("Name expected"); return null; } var sym = _make_symbol(type); @@ -1461,7 +1460,7 @@ function parse($TEXT, options) { function make_unary(ctor, op, expr) { if ((op == "++" || op == "--") && !is_assignable(expr)) - croak("SyntaxError: Invalid use of " + op + " operator"); + croak("Invalid use of " + op + " operator"); return new ctor({ operator: op, expression: expr }); }; @@ -1525,7 +1524,7 @@ function parse($TEXT, options) { end : prev() }); } - croak("SyntaxError: Invalid assignment"); + croak("Invalid assignment"); } return left; }; diff --git a/lib/utils.js b/lib/utils.js index a0571d65..46adfd4c 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -78,13 +78,28 @@ function repeat_string(str, i) { return d; }; +function configure_error_stack(fn) { + Object.defineProperty(fn.prototype, "stack", { + get: function() { + var err = new Error(this.message); + err.name = this.name; + try { + throw err; + } catch(e) { + return e.stack; + } + } + }); +} + function DefaultsError(msg, defs) { - Error.call(this, msg); - this.msg = msg; + this.message = msg; this.defs = defs; }; DefaultsError.prototype = Object.create(Error.prototype); DefaultsError.prototype.constructor = DefaultsError; +DefaultsError.prototype.name = "DefaultsError"; +configure_error_stack(DefaultsError); DefaultsError.croak = function(msg, defs) { throw new DefaultsError(msg, defs); diff --git a/test/input/invalid/eof.js b/test/input/invalid/eof.js new file mode 100644 index 00000000..330d5023 --- /dev/null +++ b/test/input/invalid/eof.js @@ -0,0 +1 @@ +foo, bar( diff --git a/test/input/invalid/simple.js b/test/input/invalid/simple.js new file mode 100644 index 00000000..98a07d20 --- /dev/null +++ b/test/input/invalid/simple.js @@ -0,0 +1 @@ +function f(a{} diff --git a/test/input/invalid/tab.js b/test/input/invalid/tab.js new file mode 100644 index 00000000..f209b8ee --- /dev/null +++ b/test/input/invalid/tab.js @@ -0,0 +1 @@ + foo( xyz, 0abc); diff --git a/test/mocha/cli.js b/test/mocha/cli.js index 52c70935..c07eeee7 100644 --- a/test/mocha/cli.js +++ b/test/mocha/cli.js @@ -199,4 +199,43 @@ describe("bin/uglifyjs", function () { done(); }); }); + it("Should fail with invalid syntax", function(done) { + var command = uglifyjscmd + ' test/input/invalid/simple.js'; + + exec(command, function (err, stdout, stderr) { + assert.ok(err); + var lines = stderr.split(/\n/); + assert.strictEqual(lines[0], "Parse error at test/input/invalid/simple.js:1,12"); + assert.strictEqual(lines[1], "function f(a{}"); + assert.strictEqual(lines[2], " ^"); + assert.strictEqual(lines[3], "SyntaxError: Unexpected token punc «{», expected punc «,»"); + done(); + }); + }); + it("Should fail with correct marking of tabs", function(done) { + var command = uglifyjscmd + ' test/input/invalid/tab.js'; + + exec(command, function (err, stdout, stderr) { + assert.ok(err); + var lines = stderr.split(/\n/); + assert.strictEqual(lines[0], "Parse error at test/input/invalid/tab.js:1,12"); + assert.strictEqual(lines[1], "\t\tfoo(\txyz, 0abc);"); + assert.strictEqual(lines[2], "\t\t \t ^"); + assert.strictEqual(lines[3], "SyntaxError: Invalid syntax: 0abc"); + done(); + }); + }); + it("Should fail with correct marking at start of line", function(done) { + var command = uglifyjscmd + ' test/input/invalid/eof.js'; + + exec(command, function (err, stdout, stderr) { + assert.ok(err); + var lines = stderr.split(/\n/); + assert.strictEqual(lines[0], "Parse error at test/input/invalid/eof.js:2,0"); + assert.strictEqual(lines[1], "foo, bar("); + assert.strictEqual(lines[2], " ^"); + assert.strictEqual(lines[3], "SyntaxError: Unexpected token: eof (undefined)"); + done(); + }); + }); }); diff --git a/test/mocha/comment.js b/test/mocha/comment.js index 69cdb3d5..56470e0f 100644 --- a/test/mocha/comment.js +++ b/test/mocha/comment.js @@ -13,7 +13,7 @@ describe("Comment", function() { var fail = function(e) { return e instanceof uglify.JS_Parse_Error && - e.message === "SyntaxError: Unexpected token: operator (>)" && + e.message === "Unexpected token: operator (>)" && e.line === 2 && e.col === 0; } @@ -36,7 +36,7 @@ describe("Comment", function() { var fail = function(e) { return e instanceof uglify.JS_Parse_Error && - e.message === "SyntaxError: Unexpected token: operator (>)" && + e.message === "Unexpected token: operator (>)" && e.line === 5 && e.col === 0; } diff --git a/test/mocha/directives.js b/test/mocha/directives.js index 82594758..bc763ae0 100644 --- a/test/mocha/directives.js +++ b/test/mocha/directives.js @@ -168,7 +168,7 @@ describe("Directives", function() { throw new Error("Expected parser to fail"); } catch (e) { assert.strictEqual(e instanceof uglify.JS_Parse_Error, true); - assert.strictEqual(e.message, "SyntaxError: Unexpected token: punc (])"); + assert.strictEqual(e.message, "Unexpected token: punc (])"); } test_directive(tokenizer, tests[i]); diff --git a/test/mocha/getter-setter.js b/test/mocha/getter-setter.js index a292fa00..641a2026 100644 --- a/test/mocha/getter-setter.js +++ b/test/mocha/getter-setter.js @@ -71,7 +71,7 @@ describe("Getters and setters", function() { var fail = function(data) { return function (e) { return e instanceof UglifyJS.JS_Parse_Error && - e.message === "SyntaxError: Invalid getter/setter name: " + data.operator; + e.message === "Invalid getter/setter name: " + data.operator; }; }; diff --git a/test/mocha/line-endings.js b/test/mocha/line-endings.js index ef46bccd..10e2a1c5 100644 --- a/test/mocha/line-endings.js +++ b/test/mocha/line-endings.js @@ -50,7 +50,7 @@ describe("line-endings", function() { } var fail = function(e) { return e instanceof Uglify.JS_Parse_Error && - e.message === "SyntaxError: Unexpected line terminator"; + e.message === "Unexpected line terminator"; } for (var i = 0; i < inputs.length; i++) { assert.throws(test(inputs[i]), fail); diff --git a/test/mocha/minify.js b/test/mocha/minify.js index 1b830cb5..0cf8c5c1 100644 --- a/test/mocha/minify.js +++ b/test/mocha/minify.js @@ -110,7 +110,7 @@ describe("minify", function() { inSourceMap: "inline", sourceMapInline: true }); - }, "multiple input and inline source map"); + }); }); it("Should fail with SpiderMonkey and inline source map", function() { assert.throws(function() { @@ -119,7 +119,7 @@ describe("minify", function() { sourceMapInline: true, spidermonkey: true }); - }, "SpiderMonkey and inline source map"); + }); }); }); @@ -156,4 +156,19 @@ describe("minify", function() { }); }); + describe("JS_Parse_Error", function() { + it("should throw syntax error", function() { + assert.throws(function() { + Uglify.minify("function f(a{}", { fromString: true }); + }, function(err) { + assert.ok(err instanceof Error); + assert.strictEqual(err.stack.split(/\n/)[0], "SyntaxError: Unexpected token punc «{», expected punc «,»"); + assert.strictEqual(err.filename, 0); + assert.strictEqual(err.line, 1); + assert.strictEqual(err.col, 12); + return true; + }); + }); + }); + }); diff --git a/test/mocha/number-literal.js b/test/mocha/number-literal.js index 8e05574a..e80a5313 100644 --- a/test/mocha/number-literal.js +++ b/test/mocha/number-literal.js @@ -15,7 +15,7 @@ describe("Number literals", function () { } var error = function(e) { return e instanceof uglify.JS_Parse_Error && - e.message === "SyntaxError: Legacy octal literals are not allowed in strict mode"; + e.message === "Legacy octal literals are not allowed in strict mode"; } for (var i = 0; i < inputs.length; i++) { assert.throws(test(inputs[i]), error, inputs[i]); diff --git a/test/mocha/string-literal.js b/test/mocha/string-literal.js index eb9e6f1c..6e337a24 100644 --- a/test/mocha/string-literal.js +++ b/test/mocha/string-literal.js @@ -19,7 +19,7 @@ describe("String literals", function() { var error = function(e) { return e instanceof UglifyJS.JS_Parse_Error && - e.message === "SyntaxError: Unterminated string constant"; + e.message === "Unterminated string constant"; }; for (var input in inputs) { @@ -49,7 +49,7 @@ describe("String literals", function() { var error = function(e) { return e instanceof UglifyJS.JS_Parse_Error && - e.message === "SyntaxError: Legacy octal escape sequences are not allowed in strict mode"; + e.message === "Legacy octal escape sequences are not allowed in strict mode"; } for (var input in inputs) { diff --git a/test/mocha/with.js b/test/mocha/with.js index 734e1e13..a74ef41a 100644 --- a/test/mocha/with.js +++ b/test/mocha/with.js @@ -9,7 +9,7 @@ describe("With", function() { } var error = function(e) { return e instanceof uglify.JS_Parse_Error && - e.message === "SyntaxError: Strict mode may not include a with statement"; + e.message === "Strict mode may not include a with statement"; } assert.throws(test, error); });