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

Disable automatic semicolon insertion at end of object patterns #146

Merged
merged 3 commits into from
Apr 13, 2021

Conversation

nbrahms
Copy link
Contributor

@nbrahms nbrahms commented Apr 9, 2021

"object_type" nodes admit automatic semicolons, whereas "object_pattern"
nodes do not. The current grammar will greedily attempt to consume an
object_type node, and insert an automatic semicolon within this type:

({a}: {a: number}) => number

Since object types can never themselves be typed, I use two-token
lookahead when deciding to insert an automatic semicolon before the
closing braces of a braced sequence of tokens; if a type annotation
follows the closing brace, we will not allow an automatic semicolon.

Fixes #144.

"object_type" nodes admit automatic semicolons, whereas "object_pattern"
nodes do not. The current grammar will greedily attempt to consume an
object_type node, and insert an automatic semicolon within this type:

  ({a}: {a: number}) => number

Since object types can never themselves be typed, I use two-token
lookahead when deciding to insert an automatic semicolon before the
closing braces of a braced sequence of tokens; if a type annotation
follows the closing brace, we will not allow an automatic semicolon.
@maxbrunsfeld
Copy link
Contributor

@dcreager @patrickt Do you know why the GH actions CI isn't running on PRs? I think we had this problem with a different grammar repo.

Also, I am responsible for the windows CI failure. In 892494d, I restructured the Node and Rust bindings' structure so that the repo is installable as a single module. On Windows, my approach seems to have led to a duplicate symbol error. I will try to take a look at this soon (unless you all know anyone else who loves debugging linker errors 🤔 ). But in the meantime, I don't think it needs to block @nbrahms's PR.

I do think we should try to get a green CI run on another platform though.

Copy link
Contributor

@maxbrunsfeld maxbrunsfeld left a comment

Choose a reason for hiding this comment

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

Your solution makes sense to me, and you've added good test coverage. If possible, I'd like to get CI running properly before we merge.

@mjambon
Copy link
Contributor

mjambon commented Apr 9, 2021

@dcreager @patrickt @maxbrunsfeld, @nbrahms was wondering what the intention was with the branch filter **:** in the GHA config. Another thing to check is whether the job would trigger when the PR is from a forked repo.

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.

Support destructured function parameters in function types
3 participants