Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@babel/parser error recovery #10363

Merged
merged 29 commits into from Nov 5, 2019

Conversation

nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Aug 22, 2019

Q                       A
Fixed Issues? Fixes #6702, fixes #10316
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature? Yes
Tests Added + Pass? Yes
Documentation PR Link babel/website#2114
Any Dependency Changes?
License MIT

Note for reviewers: #10363 (comment)


OLD MESSAGE: I'm manually reviewing all the tests with a different error. You can find the script I used to generate this list at #10363 (comment).

Review progress

The following tests have more than one error:

  • packages/babel-parser/test/fixtures/core/scope/undecl-export-if/input.js
  • packages/babel-parser/test/fixtures/core/uncategorised/108/input.js @babel/parser error recovery #10363 (comment)
  • packages/babel-parser/test/fixtures/core/uncategorised/366/input.js
  • packages/babel-parser/test/fixtures/core/uncategorised/446/input.js @babel/parser error recovery #10363 (comment)
  • packages/babel-parser/test/fixtures/core/uncategorised/499/input.js @babel/parser error recovery #10363 (comment)
  • packages/babel-parser/test/fixtures/core/uncategorised/500/input.js @babel/parser error recovery #10363 (comment)
  • packages/babel-parser/test/fixtures/core/uncategorised/547/input.js
  • packages/babel-parser/test/fixtures/core/uncategorised/548/input.js
  • packages/babel-parser/test/fixtures/es2015/destructuring/error-operator-for-default/input.js @babel/parser error recovery #10363 (comment)
  • packages/babel-parser/test/fixtures/es2015/identifiers/invalid-escape-seq-const/input.js
  • packages/babel-parser/test/fixtures/es2015/identifiers/invalid-escape-seq-export/input.js
  • packages/babel-parser/test/fixtures/es2015/identifiers/invalid-escape-seq-import/input.js
  • packages/babel-parser/test/fixtures/es2015/modules/duplicate-export-default-and-export-as-default/input.js
  • packages/babel-parser/test/fixtures/es2015/modules/duplicate-named-export-destructuring10/input.js
  • packages/babel-parser/test/fixtures/es2015/modules/duplicate-named-export-destructuring11/input.js
  • packages/babel-parser/test/fixtures/es2015/modules/duplicate-named-export-destructuring12/input.js
  • packages/babel-parser/test/fixtures/es2015/modules/duplicate-named-export-destructuring13/input.js
  • packages/babel-parser/test/fixtures/es2015/modules/duplicate-named-export-destructuring14/input.js
  • packages/babel-parser/test/fixtures/es2015/modules/duplicate-named-export-destructuring15/input.js
  • packages/babel-parser/test/fixtures/es2015/modules/duplicate-named-export-destructuring16/input.js
  • packages/babel-parser/test/fixtures/es2015/modules/duplicate-named-export-destructuring17/input.js
  • packages/babel-parser/test/fixtures/es2015/modules/duplicate-named-export-destructuring18/input.js
  • packages/babel-parser/test/fixtures/es2015/modules/duplicate-named-export-destructuring19/input.js
  • packages/babel-parser/test/fixtures/es2015/modules/duplicate-named-export-destructuring2/input.js
  • packages/babel-parser/test/fixtures/es2015/modules/duplicate-named-export-destructuring3/input.js
  • packages/babel-parser/test/fixtures/es2015/modules/duplicate-named-export-destructuring4/input.js
  • packages/babel-parser/test/fixtures/es2015/modules/duplicate-named-export-destructuring5/input.js
  • packages/babel-parser/test/fixtures/es2015/modules/duplicate-named-export-destructuring6/input.js
  • packages/babel-parser/test/fixtures/es2015/modules/duplicate-named-export-destructuring7/input.js
  • packages/babel-parser/test/fixtures/es2015/modules/duplicate-named-export-destructuring8/input.js
  • packages/babel-parser/test/fixtures/es2015/modules/duplicate-named-export-destructuring9/input.js
  • packages/babel-parser/test/fixtures/es2015/modules/duplicate-named-export/input.js
  • packages/babel-parser/test/fixtures/es2015/shorthand/2/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/233/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/234/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/242/input.js @babel/parser error recovery #10363
  • packages/babel-parser/test/fixtures/es2015/uncategorised/252/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/291/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/333/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/344/input.js
  • packages/babel-parser/test/fixtures/es2015/yield/parameter-default-inside-arrow-inside-generator-5/input.js
  • packages/babel-parser/test/fixtures/es2015/yield/parameter-name-arrow-inside-generator-1/input.js @babel/parser error recovery #10363 (comment)
  • packages/babel-parser/test/fixtures/es2015/yield/parameter-name-arrow-inside-generator-2/input.js
  • packages/babel-parser/test/fixtures/es2015/yield/parameter-name-arrow-inside-generator-3/input.js
  • packages/babel-parser/test/fixtures/es2015/yield/yield-star-parameter-default-inside-generator/input.js
  • packages/babel-parser/test/fixtures/es2018/object-rest-spread/11/input.js
  • packages/babel-parser/test/fixtures/es2018/object-rest-spread/12/input.js
  • packages/babel-parser/test/fixtures/es2018/object-rest-spread/13/input.js
  • packages/babel-parser/test/fixtures/es2018/object-rest-spread/14/input.js
  • packages/babel-parser/test/fixtures/es2018/object-rest-spread/15/input.js
  • packages/babel-parser/test/fixtures/esprima/es2015-export-declaration/invalid-export-named-default/input.js
  • packages/babel-parser/test/fixtures/esprima/es2015-identifier/invalid_escaped_surrogate_pairs/input.js
  • packages/babel-parser/test/fixtures/esprima/es2015-yield/invalid-yield-generator-arrow-parameter/input.js @babel/parser error recovery #10363 (comment)
  • packages/babel-parser/test/fixtures/esprima/es2015-yield/invalid-yield-generator-arrow-parameters/input.js
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0041/input.js
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0042/input.js
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0043/input.js
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0044/input.js
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0087/input.js @babel/parser error recovery #10363 (comment)
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0099/input.js
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0217/input.js @babel/parser error recovery #10363 (comment)
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0218/input.js @babel/parser error recovery #10363 (comment)
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0222/input.js
  • packages/babel-parser/test/fixtures/experimental/dynamic-import/direct-calls-only/input.js @babel/parser error recovery #10363 (comment)
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-102/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-103/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-113/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-114/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-115/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-118/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-120/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-123/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-126/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-127/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-13/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-137/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-138/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-139/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-14/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-142/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-144/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-147/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-15/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-18/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-2/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-20/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-23/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-25/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-3/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-30/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-31/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-41/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-42/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-43/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-46/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-48/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-51/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-54/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-55/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-65/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-66/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-67/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-70/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-72/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-75/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-78/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-79/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-89/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-90/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-91/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-94/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-96/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-99/input.js
  • packages/babel-parser/test/fixtures/experimental/partial-application/in-SuperCall/input.js
  • packages/babel-parser/test/fixtures/experimental/partial-application/in-new/input.js
  • packages/babel-parser/test/fixtures/experimental/pipeline-operator/proposal-smart-error,-topic-style,-unbound-topic,-do-expression,-do-while-loop,-outer-topic-reference-in-loop-body/input.js
  • packages/babel-parser/test/fixtures/experimental/pipeline-operator/proposal-smart-error,-topic-style,-unbound-topic,-do-expression,-for-await-of-loop,-outer-topic-reference-in-loop-body/input.js
  • packages/babel-parser/test/fixtures/experimental/pipeline-operator/proposal-smart-error,-topic-style,-unbound-topic,-do-expression,-for-classic-loop,-outer-topic-reference-in-loop-body/input.js
  • packages/babel-parser/test/fixtures/experimental/pipeline-operator/proposal-smart-error,-topic-style,-unbound-topic,-do-expression,-for-in-loop,-outer-topic-reference-in-loop-body/input.js
  • packages/babel-parser/test/fixtures/experimental/pipeline-operator/proposal-smart-error,-topic-style,-unbound-topic,-do-expression,-for-of-loop,-outer-topic-reference-in-loop-body/input.js
  • packages/babel-parser/test/fixtures/experimental/pipeline-operator/proposal-smart-error,-topic-style,-unbound-topic,-do-expression,-while-loop,-outer-topic-reference-in-loop-body/input.js
  • packages/babel-parser/test/fixtures/experimental/pipeline-operator/proposal-smart-error,-topic-style,-unbound-topic,-do-expression,-with-statement,-outer-topic-reference-in-with-body/input.js
  • packages/babel-parser/test/fixtures/experimental/pipeline-operator/proposal-smart-error,-unbound-topic,-inner-class-in-pipeline-body/input.js
  • packages/babel-parser/test/fixtures/experimental/pipeline-operator/proposal-smart-error,-unbound-topic,-inner-function-in-pipeline-body/input.js
  • packages/babel-parser/test/fixtures/experimental/pipeline-operator/proposal-smart-error,-unbound-topic,-pipeline-head-in-inner-function-in-pipeline-body/input.js
  • packages/babel-parser/test/fixtures/flow/explicit-inexact-object/explicit_inexact_disallowed_in_non_objects3/input.js
  • packages/babel-parser/test/fixtures/flow/explicit-inexact-object/explicit_inexact_disallowed_in_non_objects4/input.js
  • packages/babel-parser/test/fixtures/flow/explicit-inexact-object/explicit_inexact_disallowed_in_non_objects6/input.js
  • packages/babel-parser/test/fixtures/flow/explicit-inexact-object/explicit_inexact_disallowed_in_non_objects7/input.js
  • packages/babel-parser/test/fixtures/flow/typeapp-call/underscore_is_illegal_type_param_name/input.js

The following tests generated a different error:

  • packages/babel-parser/test/fixtures/es2015/meta-properties/new-invalid-prop/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/201/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/205/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/209/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/210/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/214/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/215/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/221/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/251/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/284/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/37/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/395/input.js
  • packages/babel-parser/test/fixtures/esprima/es2015-arrow-function/object-binding-pattern-invalid-member-expr/input.js
  • packages/babel-parser/test/fixtures/esprima/es2015-arrow-function/object-binding-pattern-invalid-nested-param/input.js
  • packages/babel-parser/test/fixtures/esprima/es2015-super-property/invalid_super_not_inside_function/input.js
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0015/input.js
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0020/input.js
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0021/input.js
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0025/input.js
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0026/input.js
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0028/input.js
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0033/input.js
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0034/input.js
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0098/input.js
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0163/input.js
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0165/input.js
  • packages/babel-parser/test/fixtures/experimental/decorators-2/no-constructor-decorators/input.js
  • packages/babel-parser/test/fixtures/experimental/decorators/no-constructor-decorators/input.js
  • packages/babel-parser/test/fixtures/flow/explicit-inexact-object/explicit_inexact_disallowed_in_non_objects1/input.js
  • packages/babel-parser/test/fixtures/flow/explicit-inexact-object/explicit_inexact_disallowed_in_non_objects2/input.js
  • packages/babel-parser/test/fixtures/flow/explicit-inexact-object/explicit_inexact_disallowed_in_non_objects5/input.js
  • packages/babel-parser/test/fixtures/flow/explicit-inexact-object/explicit_inexact_forbidden_in_exact/input.js
  • packages/babel-parser/test/fixtures/flow/explicit-inexact-object/explicit_inexact_must_appear_last/input.js
  • packages/babel-parser/test/fixtures/flow/explicit-inexact-object/explicit_inexact_object_invalid1/input.js
  • packages/babel-parser/test/fixtures/flow/explicit-inexact-object/explicit_inexact_object_invalid2/input.js
  • packages/babel-parser/test/fixtures/flow/explicit-inexact-object/explicit_inexact_object_invalid3/input.js
  • packages/babel-parser/test/fixtures/flow/predicates/4/input.js
  • packages/babel-parser/test/fixtures/flow/regression/issue-58-failing-1/input.js
  • packages/babel-parser/test/fixtures/flow/regression/issue-58-failing-2/input.js
  • packages/babel-parser/test/fixtures/flow/regression/issue-58-failing-3/input.js
  • packages/babel-parser/test/fixtures/flow/regression/issue-58-failing-4/input.js

The following tests threw a different error:

  • packages/babel-parser/test/fixtures/core/uncategorised/347/input.js
  • packages/babel-parser/test/fixtures/core/uncategorised/453/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/216/input.js
  • packages/babel-parser/test/fixtures/es2015/yield/in-class-heritage/input.js
  • packages/babel-parser/test/fixtures/es2015/yield/in-iterator-stmt/input.js
  • packages/babel-parser/test/fixtures/es2017/async-functions/invalid-escape-await/input.js
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0002/input.js
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0048/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-27/input.js
  • packages/babel-parser/test/fixtures/flow/regression/issue-58-ambiguous/input.js
  • packages/babel-parser/test/fixtures/typescript/variable-declarator/definite-assignment-not-allowed/input.ts

The following input files had to be changed:

  • packages/babel-parser/test/fixtures/es2015/destructuring/binding-this/input.js
  • packages/babel-parser/test/fixtures/es2015/generators/invalid-escape-yield/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/125/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/222/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/223/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/232/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/233/input.js
  • packages/babel-parser/test/fixtures/es2015/uncategorised/234/input.js
  • packages/babel-parser/test/fixtures/esprima/es2015-destructuring-assignment-object-pattern/invalid-pattern-with-method/input.js
  • packages/babel-parser/test/fixtures/esprima/expression-primary-array/migrated_0012/input.js
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0162/input.js
  • packages/babel-parser/test/fixtures/esprima/invalid-syntax/migrated_0169/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-52/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-53/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-56/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-57/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-58/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-59/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-60/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-61/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-62/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-63/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-64/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-68/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-69/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-71/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-73/input.js
  • packages/babel-parser/test/fixtures/experimental/numeric-separator/invalid-74/input.js

@JLHwung JLHwung self-requested a review August 22, 2019 20:28
"SyntaxError: Invalid regular expression flag (1:19)",
"SyntaxError: Invalid regular expression flag (1:20)",
"SyntaxError: Invalid regular expression flag (1:21)",
"SyntaxError: Invalid regular expression flag (1:22)"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The input file is var x = /[P QR]/\u0067. It throws one error for each character of \u0067, but I think that it is ok?

@nicolo-ribaudo
Copy link
Member Author

Before reviewing this PR: I'm slowly going through all the 2000 test files changed, to check if all of them are correct. I will leave comments for significant tests.

Copy link
Member Author

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Progress: 280/2054

},
"errors": [
"SyntaxError: Invalid left-hand side in assignment expression (1:0)",
"SyntaxError: Invalid left-hand side in assignment expression (1:0)"

This comment was marked as resolved.

},
"errors": [
"SyntaxError: Invalid left-hand side in for-in statement (1:5)",
"SyntaxError: Invalid left-hand side in for-in statement (1:5)"

This comment was marked as resolved.

},
"errors": [
"SyntaxError: Expecting Unicode escape sequence \\uXXXX (1:1)",
"SyntaxError: Expecting Unicode escape sequence \\uXXXX (1:2)"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is reported twice correctly, because the input file is

\\

@@ -1,3 +1,3 @@
{
"throws": "Bad character escape sequence (1:3)"
}
"throws": "Unterminated string constant (1:0)"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is ok, because the input file is

"\u

TODO: For tests which throw, also test the error that is generated without the errorRecovery option. For tests which don't throw this isn't needed, since the thrown error will always be the first one in the errors array.

},
"errors": [
"SyntaxError: Binding 'eval' in strict mode (1:41)",
"SyntaxError: Binding 'eval' in strict mode (1:41)"

This comment was marked as resolved.

},
"errors": [
"SyntaxError: Octal literal in strict mode (1:35)",
"SyntaxError: Octal literal in strict mode (1:35)"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is duplicated. The input code is

function hello() { 'use strict'; "\1"; }

}
},
"errors": [
"SyntaxError: Legacy octal literals are not allowed in strict mode (1:33)",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicated.

function hello() { 'use strict'; 021; }

},
"errors": [
"SyntaxError: Unexpected reserved word 'interface' (1:37)",
"SyntaxError: Binding 'interface' in strict mode (1:37)"

This comment was marked as resolved.

"errors": [
"SyntaxError: Unexpected reserved word 'static' (1:23)",
"SyntaxError: Binding 'static' in strict mode (1:23)",
"SyntaxError: Binding 'static' in strict mode (1:23)"

This comment was marked as resolved.

Copy link
Member Author

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

347

},
"errors": [
"SyntaxError: Only '=' operator can be used for specifying default value. (1:3)",
"SyntaxError: Invalid left-hand side in array destructuring pattern (1:2)"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This second error should be removed

@JLHwung
Copy link
Contributor

JLHwung commented Aug 24, 2019

Here is a helper script (executed at project root) to output those fixtures whose errors.length > 1, I wish it could help

const glob = require("glob");
const fs = require("fs");
glob("packages/babel-parser/test/fixtures/**/output.json", (er, files) => {
  if (er) {
    console.error(er);
    return;
  }
  files.forEach((file) => {
    fs.readFile(file, (err, data) => {
      const json = JSON.parse(data);
      if (json.errors && json.errors.length > 1) {
        console.log(file);
      }
    })
  })
})

},
"errors": [
"SyntaxError: Escape sequence in keyword export (1:15)",
"SyntaxError: Unexpected keyword 'export' (1:4)"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error should be removed.

Consider change test case to

expor\u{74} default 123;

options.json

{
  "sourceType": "module"
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that it isn't what this test is for. Since it is in the identifiers folder, I think that here we are testing that export can't be used as an identifier, even if it contains escape sequences.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The identifiers/invalid-escape-seq-if is actually testing invalid escape only:

\u0069\u{66} (true) {}

We may create new test cases invalid-escape-seq alongside identifiers so we can test different error types.

},
"errors": [
"SyntaxError: Escape sequence in keyword import (1:40)",
"SyntaxError: Unexpected keyword 'import' (1:4)"
Copy link
Contributor

@JLHwung JLHwung Aug 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error should be removed.
Consider change test case to

\u{69}\u{6d}\u{70}\u{6f}\u{72}\u{74} foo from "foo";

options.json

{
  "sourceType": "module"
}

},
"errors": [
"SyntaxError: The only valid meta property for new is new.target (1:4)",
"SyntaxError: new.target can only be used in functions (1:0)"

This comment was marked as resolved.

@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Aug 24, 2019

Thanks for your script! I have modified it so that it reports

  1. output.json files with more than one error
  2. output.json files with one error different from the one which was previously specified in options.json
  3. options.json files whose throws has changed.
Code
const glob = require("glob");
const pfy = require("util").promisify;

const preq = (pkg, method) => pfy(require(pkg)[method]);

const readFile = preq("fs", "readFile");
const exists = preq("fs", "exists");
const exec = preq("child_process", "exec");

const messageUpdates = [
  {
    old: "Invalid or unexpected token",
    cur: "A numeric separator is only allowed between two digits",
  },
];

glob("packages/babel-parser/test/fixtures/**/input.*", async (er, files) => {
  if (er) {
    console.error(er);
    return;
  }

  const result = {
    multiple: [],
    different: [],
    throws: [],
    input: [],
  };

  await Promise.all(
    files.map(async file => {
      const output = file.replace(/input\.[a-z]+$/, "output.json");
      const options = file.replace(/input\.[a-z]+$/, "options.json");
      try {
        if (await exists(output)) {
          const { errors } = JSON.parse(await readFile(output));
          if (!errors || errors.length === 0) return;

          if (errors.length > 1) {
            result.multiple.push(file);
            return;
          }

          const { throws } = JSON.parse(
            (await exec(`git show master:${options}`)).stdout
          );

          const error = errors[0].replace("SyntaxError: ", "");
          const updated = messageUpdates.some(
            ({ old, cur }) => throws.includes(old) && error.includes(cur)
          );

          if (error !== throws && !updated) {
            result.different.push(file);
            return;
          }
        } else {
          const data = await Promise.all([
            readFile(options),
            exec(`git show master:${options}`),
          ]);

          const { throws } = JSON.parse(data[0]);
          const { throws: oldThrows } = JSON.parse(data[1].stdout);

          if (throws !== oldThrows) {
            result.throws.push(file);
            return;
          }
        }

        const data = await Promise.all([
          readFile(file),
          exec(`git show master:${file}`),
        ]);

        if (data[0].toString() !== data[1].stdout.toString()) {
          result.input.push(file);
        }
      } catch (e) {
        console.error(file, e);
        process.exit();
      }
    })
  );

  console.log("**The following tests have more than one error:**");
  console.log(list(result.multiple));
  console.log("\n");
  console.log("**The following tests generated a different error:**");
  console.log(list(result.different));
  console.log("\n");
  console.log("**The following tests threw a different error:**");
  console.log(list(result.throws));
  console.log("\n");
  console.log("**The following input files had to be changed:**");
  console.log(list(result.input));
});

function list(arr) {
  return arr
    .sort()
    .map(x => ` - [ ] \`${x}\``)
    .join("\n");
}

I will track review progress, based on the output of this script, in the PR description.

},
"errors": [
"SyntaxError: Invalid left-hand side in assignment expression (1:1)",
"SyntaxError: Invalid left-hand side in array destructuring pattern (1:1)"

This comment was marked as resolved.

},
"errors": [
"SyntaxError: Assigning to 'eval' in strict mode (1:15)",
"SyntaxError: Binding 'eval' in strict mode (1:15)"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error should be deduplicated.

Input code:

"use strict"; (eval = 10) => 42

It might be hard to do, because we are first parsing eval = 10 as an assignment expression, and then converting it to a paremeter later when we find =>

@nicolo-ribaudo nicolo-ribaudo force-pushed the parser-error-recovery branch 2 times, most recently from 6664b40 to fd9ca7c Compare August 24, 2019 13:36
},
"errors": [
"SyntaxError: Yield cannot be used as name inside a generator function (2:3)",
"SyntaxError: Binding invalid left-hand side in function paramter list (2:3)"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be deduplicated.

Input code:

function* fn() {
  (yield fn) => {};
}

},
"errors": [
"SyntaxError: Dynamic imports require a parameter: import('a.js') (2:9)",
"SyntaxError: The only valid meta property for import is import.meta (2:16)"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this double error, since it hints about the two possible interpretations.

Input code:

function failsParse() {
  return import.then();
}

@nicolo-ribaudo nicolo-ribaudo added pkg: parser PR: New Feature 🚀 A type of pull request used for our changelog categories labels Aug 24, 2019
@KFlash
Copy link

KFlash commented Aug 25, 2019

@nicolo-ribaudo Interesting feature!! 👍 What about the lexer? Code like this
throw this.raise(this.state.pos - 2, "Unterminated comment"); will still throw. It can easily be prevented if you return e.g. a Token.Invalid and save the errors somewhere.

BTW. it should be possible to recover from all errors if you either 1) return to statement level if invalid expression or 2) Insert a dummy AST node.

I started to experiment with this myself and just done with a lexer that recover just fine, so I can give you ideas if you need them :)

@nicolo-ribaudo
Copy link
Member Author

Thanks for these insights!

Since this PR is already really big and will be a pain to review, I wanted to only avoid "easy" parser errors for now. I'll make more and more errors recoverable in future iterations.

I started to experiment with this myself and just done with a lexer that recover just fine, so I can give you ideas if you need them :)

If it's open source I'd love to see it!

@KFlash
Copy link

KFlash commented Aug 26, 2019

@nicolo-ribaudo My lexer recovery code can be seen here. Just a WIP for now :)

@nicolo-ribaudo
Copy link
Member Author

Thanks! I won't have time to update this PR in the following, but I'll try to take a look at yours.

@KFlash
Copy link

KFlash commented Aug 31, 2019

@nicolo-ribaudo I'm more or less done with my lexer now in case you plan to take a look. It's located here. It recover from all errors as seen in this tests.

@nicolo-ribaudo nicolo-ribaudo force-pushed the parser-error-recovery branch 2 times, most recently from 11f1fc9 to d0c330a Compare August 31, 2019 23:58
@babel-bot
Copy link
Collaborator

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/11425/

@babel-bot
Copy link
Collaborator

babel-bot commented Sep 1, 2019

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/11563/

@nicolo-ribaudo
Copy link
Member Author

✔️ I added the last 5 commits during the review of this PR

@nicolo-ribaudo nicolo-ribaudo removed the PR: Needs Review A pull request awaiting more approvals label Nov 5, 2019
@nicolo-ribaudo nicolo-ribaudo merged commit 87feda7 into babel:master Nov 5, 2019
@nicolo-ribaudo nicolo-ribaudo deleted the parser-error-recovery branch November 5, 2019 09:15
@thorn0 thorn0 mentioned this pull request Nov 5, 2019
12 tasks
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Feb 4, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Feb 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser PR: New Feature 🚀 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fault-tolerant @bable/parser parsing Error Recovery
4 participants