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

Implement optional trailing commas #97

Merged

Conversation

alangpierce
Copy link
Contributor

Trailing commas are allowed in these situations in JS/TS:

They're also allowed in these cases not (yet) supported by AssemblyScript:

  • Object literals
  • Array destructures
  • Object destructures

All of these cases had similar-looking code, which needed to be tweaked to
handle the possibility of a comma before the close-delimiter. Type arguments
were a little different because they take a backtracking approach.

I also fixed a missing case in the AST printer: export function wasn't being
printed right.

Trailing commas are allowed in these situations in JS/TS:
* Array literals
* Argument lists
* Parameter lists
* Enums
* Type parameter lists
* Type argument lists (Technically filed as microsoft/TypeScript#21984 )

They're also allowed in these cases not (yet) supported by AssemblyScript:
* Object literals
* Array destructures
* Object destructures

All of these cases had similar-looking code, which needed to be tweaked to
handle the possibility of a comma before the close-delimiter. Type arguments
were a little different because they take a backtracking approach.

I also fixed a missing case in the AST printer: `export function` wasn't being
printed right.
src/parser.ts Outdated
@@ -900,17 +902,21 @@ export class Parser extends DiagnosticEmitter {

var typeParameters = new Array<TypeParameterNode>();
if (!tn.skip(Token.GREATERTHAN)) {
do {
while (!tn.skip(Token.GREATERTHAN)) {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe you have an idea how the initial double-check for GREATERTHAN could be avoided? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess do/while handles that pretty nicely here. Will update.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I'll just do a length check for the "no type parameters" case so this looks like the other cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, looking good :)

@dcodeIO dcodeIO merged commit 00fee73 into AssemblyScript:master May 7, 2018
@dcodeIO
Copy link
Member

dcodeIO commented May 7, 2018

Thanks!

alangpierce added a commit to alangpierce/assemblyscript that referenced this pull request May 20, 2018
This uses the same strategy as AssemblyScript#97 for two more cases in the parser.
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