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

test for assignment + logical operators precedence #189

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

heartsentwined
Copy link

This PR adds (failing) test cases demonstrating an assignment operator vs logical operator precedence issue / behavior.

The issue

Compile errors first:

Syntax error on line 1, column 21: unexpected '=' (\u003D)
1 : true if true && foo = 'bar'
^ :~~~~~~~~~~~~~~~~~~~~~^
2 :

(foo should be assigned the value of 'bar'.)

Syntax error on line 1, column 21: unexpected '=' (\u003D)
1 : true if true && foo = 'bar' && true
^ :~~~~~~~~~~~~~~~~~~~~~^
2 :

(foo should be assigned the value of true.)

Similar errors tested for and, || and or.

All compile fine in the original coffeescript compiler.

For compleness, this works fine:

true if foo = 'bar' && true

Existing workaround

Adding parens to group the assignment foo = 'bar' fixes all cases:

true if true && (foo = 'bar')
true if true && (foo = 'bar' && true)

Syntax discussion

I believe that this is an operator precedence issue. While logical operators should, of course, normally take precedence over assignment operators, this should only happen when the logical operator appears at the right hand side of the assignment operator.

Given

true || foo = 'bar' && true

While the && operator taking precedence over the assignment operator makes sense, the case of the || operator would produce this semantically invalid parsing:

(true) = (true)

Since all logical operators will produce a boolean primitive, and that boolean primitives cannot be the receiver of the assignment operator, an exception should be made to change the operator precedence on such cases, producing:

(true) || (foo = true)

I'm sorry but I am not familiar with pegjs, so I can only produce a test case in this PR accompanied by this description.

@michaelficarra
Copy link
Owner

Thanks for the analysis, but this is already known. See #43. We're still unsure about this syntax.

@heartsentwined
Copy link
Author

Thanks for the pointer, I'll join discussions over there.

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.

2 participants