Skip to content

Fix #3441: parentheses wrapping expression throw invalid error #4849

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

Merged
merged 3 commits into from
Jan 16, 2018

Conversation

zdenko
Copy link
Collaborator

@zdenko zdenko commented Jan 9, 2018

Fixes #3441.

Cases where statement is used inside parentheses don't compile:

(a(); break) for a in obj
# error: cannot use a pure statement in an expression
# (a(); break) for a in obj
#       ^^^^^

(a(); break) while a
# error: cannot use a pure statement in an expression
# (a(); break) while a
#       ^^^^^

for a in obj then (a(); break)
# error: cannot use a pure statement in an expression
# for a in obj then (a(); break)
#                         ^^^^^

while a then (a(); break)
# error: cannot use a pure statement in an expression
# while a then (a(); break)
#                    ^^^^^

With this PR:

# (a(); break) for a in obj
# for a in obj then (a(); break)
for (i = 0, len = obj.length; i < len; i++) {
  a = obj[i];
  a();
  break;
}

# (a(); break) while a
# while a then (a(); break)
while (a) {
  a();
  break;
}

When Parens block includes a StatementLiteral (e.g. (b; break) for a in arr), it won't compile since Parens ((b; break)) is compiled as Value and pure statement (break) can't be used in an expression.

This PR checks Parenthetical rule in the grammar and returns Block instead of Parens if StatementLiteral is detected. The Block node is passed further to the Expression.

# it won't compile since `Parens` (`(b; break)`) is compiled as `Value` and
# pure statement (`break`) can't be used in an expression.
# For this reasons, we return `Block`, which is passed further to `Value` and `Expression`.
stm = $2.contains (n) -> n.constructor.name is 'StatementLiteral'
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd rather we don't add such code to the grammar. We can have a node function for that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also we try not to use constructor.name for obfuscation reasons IIRC.

@@ -334,7 +334,7 @@ grammar =
o 'Assignable'
o 'Literal', -> new Value $1
o 'Parenthetical', ->
if $1.constructor.name is 'Parens'
if $1.hasParentheses()
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can use unwrap

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nice.

@GeoffreyBooth GeoffreyBooth merged commit e53307b into jashkenas:master Jan 16, 2018
zdenko added a commit to zdenko/coffeescript that referenced this pull request Jan 17, 2018
GeoffreyBooth pushed a commit that referenced this pull request Feb 1, 2018
…ariables (#4853)

* fix #2047

* Additional check for 'step'; tests

* Fix #4105 (#4855)

* Update output

* Throw warning for unsupported runtimes, e.g. Node < 6 (#4839)

* fix #1403 (#4854)

* Update output

* [Change]: Destructuring with non-final spread should still use rest syntax (#4517) (#4825)

* destructuring optimization

* refactor

* minor improvement, fix errors

* minor refactoring

* improvements

* Update output

* Update output

* Fix #4843: bad output when assigning to @prop in destructuring assignment with defaults (#4848)

* fix #4843

* improvements

* typo

* small fix

* Fix #3441: parentheses wrapping expression throw invalid error  (#4849)

* fix #3441

* improvements

* refactor

* Fix #1726: expression in property access causes unexpected results (#4851)

* fix #1726

* Explain what's happening, rather than just linking to an issue

* Updated output

* Optimization

* Update output

* remove casting to number

* cleanup tests
@GeoffreyBooth GeoffreyBooth mentioned this pull request Feb 1, 2018
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