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

Parse yield expressions. #1109

Closed
wants to merge 26 commits into from
Closed

Conversation

usrbincc
Copy link
Contributor

Most of this patch consists of code to determine whether or not Mozilla needs parens around yield in a given situation. Mozilla yield parsing has some serious quirks, and I can't be certain I got them all.

Mozilla needing parens for yield seems to be only partially based on precedence. The rest of it seems to be based on some semi-random sense of aesthetics. It makes me curious as to what their parsing code is actually doing. Not quite curious enough to check, but almost.


Fixes #387. The bug there was that jshint was only checking for the end of expression once, and not per-loop. There was some incompleteness in the checking condition, too. Hopefully I got everything.


Fixes #1108. Comma expressions were using the default whitespace checking for infix, which in addition to being wrong, also conflicted with the comma-specific whitespace checking. Fixed by moving all comma whitespace checking over to comma-specific code.

jshint currently has no comprehensive tests of whitespace checking, which is how this bug sneaked in in the first place. I'll do this in a follow-up pull.


Sorry about combining these separate bugfixes into one pull request. The #1108 fix could have conceivably been done separately, but it would have been impossible to parse yield expressions properly without the #387 fix.

- Warn about yield expressions needing parentheses in `moz: true` code.
- The paren warning should be at the `yield` location.
- `yield` should have higher precedence than assign ops.
This also makes sure bitwise assign ops are also tagged with `assign`.
Anything with `nud` should return a value from `expression(10)`.
This is a little tricky:

```js
function f() {
  // okay because it actually parses as
  // yield (a ? b : c);
  yield a ? b : c;
  // needs paren
  yield ? b : c;
  // okay
  a ? yield : yield;
  // needs paren for the second `yield`
  a ? yield : yield ? yield : yield;
}
```
I might be able to use `token.reach` here.
- Don't test line-breaking here. It confuses the issue.
This prevents getting warnings referring to "(number)" instead of "42".
@valueof
Copy link
Member

valueof commented Jun 26, 2013

Thanks!

valueof pushed a commit that referenced this pull request Jun 26, 2013
@Raynos
Copy link
Contributor

Raynos commented Jul 6, 2013

@usrbincc I've started using your commits. It fixes most of the yield complains this is awesome!

One thing it still complains about is function* () { return 42 } a generator without a yield. Do you know where I should look to fix that?

jugglinmike pushed a commit to jugglinmike/jshint that referenced this pull request Oct 21, 2014
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.

Broken white option with several comma-separated expressions Parser is confused about delete statement
3 participants