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

Re-implement yield parsing improvements #3427

Merged
merged 3 commits into from
Nov 18, 2019

Conversation

jugglinmike
Copy link
Member

@caitp @rwaldron This is the latest in the series of rewrites (the prior patches along these lines were 6759ac9 and gh-3419).

Since the first commit intentionally regresses functionality, we should definitely use a merge commit to land this branch.

Commit messages:

[[FIX]] Improve parsing of `yield`

Re-implement the improvements reverted by the previous commit. Use
distinct logic to differentiate this new implementation, including the
following substantive improvements:

- Correct the reported location of warning W093 ("Did you mean to return
  a conditional instead of an assignment?")
- Permit the use of `yield` as an identifier outside of strict mode and
  generator function bodies
- Remove inaccurate restriction on `yield` within Mozilla generator
  functions
- Forward the context when parsing computed property names

[[CHORE]] Revert `yield` expression improvements

JSHint's parsing of `yield` expressions is derived from code first
contributed by GitHub user "usrbincc." That user could not be reached to
sign JSHint's Contributor License Agreement. Revert the user's
contribution to the application logic (preserving the corresponding
MIT-licensed test code) so that it may be re-written under the terms of
the Contributor License Agreement.

Revert the changes introduced by
88c862df3dba9e2cfa1e44d4be909099d8306c97

@coveralls
Copy link

coveralls commented Sep 15, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 9f55656 on jugglinmike:revert-yield-6 into 43b6354 on jshint:v2.11.0.

@jugglinmike jugglinmike changed the base branch from master to v2.11.0 October 29, 2019 02:15
@jugglinmike
Copy link
Member Author

I've rebased this against the v2.11.0 branch so that we can trial it via a release candidate.

@jugglinmike
Copy link
Member Author

@rwaldron I'm thinking that since no one is actively working on the v2.11.0 feature branch, I should rebase that on master before we merge this. What do you think?

@rwaldron
Copy link
Member

rwaldron commented Nov 7, 2019

@jugglinmike I trust your intuition 👍

JSHint's parsing of `yield` expressions is derived from code first
contributed by GitHub user "usrbincc." That user could not be reached to
sign JSHint's Contributor License Agreement. Revert the user's
contribution to the application logic (preserving the corresponding
MIT-licensed test code) so that it may be re-written under the terms of
the Contributor License Agreement.

Revert the changes introduced by
88c862d
Re-implement the improvements reverted by the previous commit. Use
distinct logic to differentiate this new implementation, including the
following substantive improvements:

- Correct the reported location of warning W093 ("Did you mean to return
  a conditional instead of an assignment?")
- Permit the use of `yield` as an identifier outside of strict mode and
  generator function bodies
- Remove inaccurate restriction on `yield` within Mozilla generator
  functions
- Forward the context when parsing computed property names

This patch introduces a new expected failure for the Test262 test suite.
That case has been corrected in JSHint's `master` branch [1] and will be
resolved when this branch is merged.

[1] 38285cd
@jugglinmike
Copy link
Member Author

That's much appreciated, Rick. This is rebased and ready for review.

Copy link
Member

@rwaldron rwaldron left a comment

Choose a reason for hiding this comment

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

Nice work!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants