Skip to content

Improve parse error for double comma somewhere inside a call expression#20399

Merged
1 commit merged into
masterfrom
parseErrorDoubleCommaInCall
Jan 8, 2018
Merged

Improve parse error for double comma somewhere inside a call expression#20399
1 commit merged into
masterfrom
parseErrorDoubleCommaInCall

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Dec 1, 2017

Fixes #20376

Simplifies the parse error for this case:

Boolean({
    x: 0,,
});

Currently this results in parse errors all over after the ,, because it causes us to abort parsing the object literal and expect another argument expression. (Then we see the } and it gets worse from there.)

In isInSomeParsingContext we iterate through all parsing contexts. That means that so long as we're somewhere in a call expression, any extra comma will abort parsing the current list.
In this example that's undesirable -- the comma shouldn't break us out of the object literal.
The problem was that isListElement returned true for a comma, but a comma isn't an arguments list element; that's only needed for parsing an OmittedExpression in an array literal.


var p1 = import(...a);
const p2 = import();
const p3 = import(,);
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Had to remove this line as it's now a parse error and not a grammar error, and a parse error would make all of the grammar errors disappear (see grammarErrorAtPos in checker.ts).

@ghost ghost requested a review from rbuckton December 1, 2017 22:26
@ghost ghost force-pushed the parseErrorDoubleCommaInCall branch from 070f3c5 to 3876b9a Compare December 1, 2017 22:26
@ghost
Copy link
Copy Markdown
Author

ghost commented Dec 18, 2017

@rbuckton Could you review?

@ghost ghost merged commit f83283c into master Jan 8, 2018
@ghost ghost deleted the parseErrorDoubleCommaInCall branch January 8, 2018 18:38
@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant