search

shapesecurity / shift-parser-js

ECMAScript parser that produces a Shift format AST
http://shift-ast.org/parser.html
Apache License 2.0
248 stars 28 forks source link

Validate fn "name" when enforcing strict mode #381

Open jugglinmike opened 6 years ago

jugglinmike commented 6 years ago

Commit message:

TC39 reached consensus in March 2018 to extend the definition of the term "function code" to include the BindingIdentifier informally referred to as the function's "name" in those productions which include it [1]. This has the effect of restricting the set of valid identifiers for functions whose bodies include a "use strict" directive.

Because the project's test suite does not include tests for this case, no change to existing test material is necessary. However, the test262-parser-tests project does require modification, so this patch should not be applied until that project has been updated accordingly [2].

[1] https://github.com/tc39/ecma262/pull/1158 [2] https://github.com/tc39/test262-parser-tests/pull/17

Would you folks like me to also add tests directly into this project?

jugglinmike commented 6 years ago

The project's linting rules disallow re-assignment of function parameters, and this patch doesn't satisfy that restriction. However, there are a number of previously-existing infractions, so I'm not sure if the linting rules are still being enforced.

bakkot commented 6 years ago

The build is failing because you'll need to add yourself to the contributors file here (sorry for the hassle).

I think this will probably need to wait until the 2019 branch, separately, since this was a normative change which didn't occur until after the 2018 spec was cut. (We track yearly releases, rather than trying to match the current spec at any given time.)

jugglinmike commented 6 years ago

The build is failing because you'll need to add yourself to the contributors file here (sorry for the hassle).

No worries. I'm no stranger to frustrating CLA overhead, myself. Signature offered here: https://github.com/shapesecurity/CLA/pull/28

I think this will probably need to wait until the 2019 branch, separately, since this was a normative change which didn't occur until after the 2018 spec was cut. (We track yearly releases, rather than trying to match the current spec at any given time.)

That's fine by me, as long as I know the patch is otherwise acceptable, I'll leave the merging in your hands.