-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
Upgrade: espree to 3.2.0, remove tests with SyntaxErrors (fixes #7169) #7170
Conversation
LGTM |
@not-an-aardvark, thanks for your PR! By analyzing the annotation information on this pull request, we identified @nzakas, @pmcelhaney and @mysticatea to be potential reviewers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, as soon as we confirm this is the way we want to fix this issue.
@bterlson - can you confirm that duplicate args in non-simple parameter lists are a syntax error here, and whether that's true in both strict and sloppy mode? |
@ljharb: confirmed, and it's true in both sloppy and strict. See: https://tc39.github.io/ecma262/#sec-function-definitions-static-semantics-early-errors. |
LGTM |
@not-an-aardvark Could you update |
531cff5
to
cde34cf
Compare
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
Though this does not have 2 days yet, I'll merge this PR since this has gotten enough eyes and other PRs are failing tests. |
What is the purpose of this pull request? (put an "X" next to item)
[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[x] Other, please explain:
Please check each item to ensure your pull request is ready:
What changes did you make? (Give an overview)
This removes tests that were failing due to the update to Espree 3.2.0. (See #7169)
These tests contained code that had a SyntaxError according to the ES spec, but previously had been parsed by Acorn rather than throwing an error. The fix is to just remove the tests.
Downstream users of ESLint will not be affected by this update unless their code contained invalid syntax, in which case this update is a bugfix for them. (Code like this would have thrown an early error at runtime anyway.)
edit: For what it's worth, here's the clause in ecma262:
Is there anything you'd like reviewers to focus on?
Also see: #7101