Skip to content

Conversation

@GeoffreyBooth
Copy link
Collaborator

Fixes #3098. Now an error is thrown if a semicolon follows an = or anything in the UNFINISHED group of tokens. No tests fail as a result of this PR, so presumably this isn’t a breaking change (aside from the case of a = ; 5 which we want to throw an error).

> ./bin/coffee -bpe 'a = ; 5'
[stdin]:1:5: error: unexpected ;
a = ; 5
    ^

@exportSpecifierList = no

if value is ';'
@error 'unexpected ;' if prev?[0] in ['=', UNFINISHED...]
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we not want = in UNFINISHED?

Copy link
Collaborator

Choose a reason for hiding this comment

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

or maybe the other way around: what does adding in UNFINISHED fixes, given that prev?[0] is '=' is the relevant fix here? Same in suppressSemicolons.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See #3098 (comment)

UNFINISHED covers all the similar tokens like += etc. I think @jashkenas in his comment was worried about the special cases around =, like =(newline)(object literal).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also, putting = into UNFINISHED causes tests to fail.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok. LGTM then.

# Conflicts:
#	lib/coffeescript/lexer.js
@GeoffreyBooth GeoffreyBooth merged commit 4a4f752 into jashkenas:2 Sep 1, 2017
@GeoffreyBooth GeoffreyBooth deleted the semicolon-unsuppression branch September 1, 2017 14:09
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