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

Fix parser regression for bad related diagnostic on missing matching brackets #44158

Merged
merged 8 commits into from Mar 16, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/compiler/diagnosticMessages.json
Expand Up @@ -15,7 +15,7 @@
"category": "Error",
"code": 1006
},
"The parser expected to find a '}' to match the '{' token here.": {
"The parser expected to find a '{1}' to match the '{0}' token here.": {
"category": "Error",
"code": 1007
},
Expand Down
91 changes: 47 additions & 44 deletions src/compiler/parser.ts
Expand Up @@ -1341,24 +1341,27 @@ namespace ts {
return inContext(NodeFlags.AwaitContext);
}

function parseErrorAtCurrentToken(message: DiagnosticMessage, arg0?: any): void {
parseErrorAt(scanner.getTokenPos(), scanner.getTextPos(), message, arg0);
function parseErrorAtCurrentToken(message: DiagnosticMessage, arg0?: any): DiagnosticWithDetachedLocation | undefined {
return parseErrorAt(scanner.getTokenPos(), scanner.getTextPos(), message, arg0);
}

function parseErrorAtPosition(start: number, length: number, message: DiagnosticMessage, arg0?: any): void {
function parseErrorAtPosition(start: number, length: number, message: DiagnosticMessage, arg0?: any): DiagnosticWithDetachedLocation | undefined {
// Don't report another error if it would just be at the same position as the last error.
const lastError = lastOrUndefined(parseDiagnostics);
let result: DiagnosticWithDetachedLocation | undefined;
if (!lastError || start !== lastError.start) {
parseDiagnostics.push(createDetachedDiagnostic(fileName, start, length, message, arg0));
result = createDetachedDiagnostic(fileName, start, length, message, arg0);
parseDiagnostics.push(result);
}

// Mark that we've encountered an error. We'll set an appropriate bit on the next
// node we finish so that it can't be reused incrementally.
parseErrorBeforeNextFinishedNode = true;
return result;
}

function parseErrorAt(start: number, end: number, message: DiagnosticMessage, arg0?: any): void {
parseErrorAtPosition(start, end - start, message, arg0);
function parseErrorAt(start: number, end: number, message: DiagnosticMessage, arg0?: any): DiagnosticWithDetachedLocation | undefined {
return parseErrorAtPosition(start, end - start, message, arg0);
}

function parseErrorAtRange(range: TextRange, message: DiagnosticMessage, arg0?: any): void {
Expand Down Expand Up @@ -1552,6 +1555,23 @@ namespace ts {
return false;
}

function parseExpectedMatchingBrackets(openKind: SyntaxKind, closeKind: SyntaxKind, openParsed: boolean, openPosition: number) {
if (token() === closeKind) {
nextToken();
return;
}
const lastError = parseErrorAtCurrentToken(Diagnostics._0_expected, tokenToString(closeKind));
if (!openParsed) {
return;
}
if (lastError) {
addRelatedInfo(
lastError,
createDetachedDiagnostic(fileName, openPosition, 1, Diagnostics.The_parser_expected_to_find_a_1_to_match_the_0_token_here, tokenToString(openKind), tokenToString(closeKind))
);
}
}

function parseOptional(t: SyntaxKind): boolean {
if (token() === t) {
nextToken();
Expand Down Expand Up @@ -5481,10 +5501,10 @@ namespace ts {

function parseArrayLiteralExpression(): ArrayLiteralExpression {
const pos = getNodePos();
parseExpected(SyntaxKind.OpenBracketToken);
const openBracketParsed = parseExpected(SyntaxKind.OpenBracketToken);
const multiLine = scanner.hasPrecedingLineBreak();
const elements = parseDelimitedList(ParsingContext.ArrayLiteralMembers, parseArgumentOrArrayLiteralElement);
parseExpected(SyntaxKind.CloseBracketToken);
parseExpectedMatchingBrackets(SyntaxKind.OpenBracketToken, SyntaxKind.CloseBracketToken, openBracketParsed, pos);
return finishNode(factory.createArrayLiteralExpression(elements, multiLine), pos);
}

Expand Down Expand Up @@ -5549,19 +5569,10 @@ namespace ts {

function parseObjectLiteralExpression(): ObjectLiteralExpression {
const pos = getNodePos();
const openBracePosition = scanner.getTokenPos();
parseExpected(SyntaxKind.OpenBraceToken);
const openBraceParsed = parseExpected(SyntaxKind.OpenBraceToken);
const multiLine = scanner.hasPrecedingLineBreak();
const properties = parseDelimitedList(ParsingContext.ObjectLiteralMembers, parseObjectLiteralElement, /*considerSemicolonAsDelimiter*/ true);
if (!parseExpected(SyntaxKind.CloseBraceToken)) {
const lastError = lastOrUndefined(parseDiagnostics);
if (lastError && lastError.code === Diagnostics._0_expected.code) {
addRelatedInfo(
lastError,
createDetachedDiagnostic(fileName, openBracePosition, 1, Diagnostics.The_parser_expected_to_find_a_to_match_the_token_here)
);
}
}
parseExpectedMatchingBrackets(SyntaxKind.OpenBraceToken, SyntaxKind.CloseBraceToken, openBraceParsed, pos);
return finishNode(factory.createObjectLiteralExpression(properties, multiLine), pos);
}

Expand Down Expand Up @@ -5643,19 +5654,11 @@ namespace ts {
function parseBlock(ignoreMissingOpenBrace: boolean, diagnosticMessage?: DiagnosticMessage): Block {
const pos = getNodePos();
const hasJSDoc = hasPrecedingJSDocComment();
const openBracePosition = scanner.getTokenPos();
if (parseExpected(SyntaxKind.OpenBraceToken, diagnosticMessage) || ignoreMissingOpenBrace) {
const openBraceParsed = parseExpected(SyntaxKind.OpenBraceToken, diagnosticMessage);
if (openBraceParsed || ignoreMissingOpenBrace) {
const multiLine = scanner.hasPrecedingLineBreak();
const statements = parseList(ParsingContext.BlockStatements, parseStatement);
if (!parseExpected(SyntaxKind.CloseBraceToken)) {
const lastError = lastOrUndefined(parseDiagnostics);
if (lastError && lastError.code === Diagnostics._0_expected.code) {
addRelatedInfo(
lastError,
createDetachedDiagnostic(fileName, openBracePosition, 1, Diagnostics.The_parser_expected_to_find_a_to_match_the_token_here)
);
}
}
parseExpectedMatchingBrackets(SyntaxKind.OpenBraceToken, SyntaxKind.CloseBraceToken, openBraceParsed, pos);
const result = withJSDoc(finishNode(factory.createBlock(statements, multiLine), pos), hasJSDoc);
if (token() === SyntaxKind.EqualsToken) {
parseErrorAtCurrentToken(Diagnostics.Declaration_or_statement_expected_This_follows_a_block_of_statements_so_if_you_intended_to_write_a_destructuring_assignment_you_might_need_to_wrap_the_the_whole_assignment_in_parentheses);
Expand Down Expand Up @@ -5711,9 +5714,10 @@ namespace ts {
const pos = getNodePos();
const hasJSDoc = hasPrecedingJSDocComment();
parseExpected(SyntaxKind.IfKeyword);
parseExpected(SyntaxKind.OpenParenToken);
const openParenPosition = getNodePos();
const openParenParsed = parseExpected(SyntaxKind.OpenParenToken);
const expression = allowInAnd(parseExpression);
parseExpected(SyntaxKind.CloseParenToken);
parseExpectedMatchingBrackets(SyntaxKind.OpenParenToken, SyntaxKind.CloseParenToken, openParenParsed, openParenPosition);
const thenStatement = parseStatement();
const elseStatement = parseOptional(SyntaxKind.ElseKeyword) ? parseStatement() : undefined;
return withJSDoc(finishNode(factory.createIfStatement(expression, thenStatement, elseStatement), pos), hasJSDoc);
Expand All @@ -5725,9 +5729,10 @@ namespace ts {
parseExpected(SyntaxKind.DoKeyword);
const statement = parseStatement();
parseExpected(SyntaxKind.WhileKeyword);
parseExpected(SyntaxKind.OpenParenToken);
const openParenPosition = getNodePos();
const openParenParsed = parseExpected(SyntaxKind.OpenParenToken);
const expression = allowInAnd(parseExpression);
parseExpected(SyntaxKind.CloseParenToken);
parseExpectedMatchingBrackets(SyntaxKind.OpenParenToken, SyntaxKind.CloseParenToken, openParenParsed, openParenPosition);

// From: https://mail.mozilla.org/pipermail/es-discuss/2011-August/016188.html
// 157 min --- All allen at wirfs-brock.com CONF --- "do{;}while(false)false" prohibited in
Expand All @@ -5741,9 +5746,10 @@ namespace ts {
const pos = getNodePos();
const hasJSDoc = hasPrecedingJSDocComment();
parseExpected(SyntaxKind.WhileKeyword);
parseExpected(SyntaxKind.OpenParenToken);
const openParenPosition = getNodePos();
const openParenParsed = parseExpected(SyntaxKind.OpenParenToken);
const expression = allowInAnd(parseExpression);
parseExpected(SyntaxKind.CloseParenToken);
parseExpectedMatchingBrackets(SyntaxKind.OpenParenToken, SyntaxKind.CloseParenToken, openParenParsed, openParenPosition);
const statement = parseStatement();
return withJSDoc(finishNode(factory.createWhileStatement(expression, statement), pos), hasJSDoc);
}
Expand Down Expand Up @@ -5819,9 +5825,10 @@ namespace ts {
const pos = getNodePos();
const hasJSDoc = hasPrecedingJSDocComment();
parseExpected(SyntaxKind.WithKeyword);
parseExpected(SyntaxKind.OpenParenToken);
const openParenPosition = getNodePos();
const openParenParsed = parseExpected(SyntaxKind.OpenParenToken);
const expression = allowInAnd(parseExpression);
parseExpected(SyntaxKind.CloseParenToken);
parseExpectedMatchingBrackets(SyntaxKind.OpenParenToken, SyntaxKind.CloseParenToken, openParenParsed, openParenPosition);
const statement = doInsideOfContext(NodeFlags.InWithStatement, parseStatement);
return withJSDoc(finishNode(factory.createWithStatement(expression, statement), pos), hasJSDoc);
}
Expand Down Expand Up @@ -8074,13 +8081,9 @@ namespace ts {
hasChildren = true;
if (child.kind === SyntaxKind.JSDocTypeTag) {
if (childTypeTag) {
parseErrorAtCurrentToken(Diagnostics.A_JSDoc_typedef_comment_may_not_contain_multiple_type_tags);
const lastError = lastOrUndefined(parseDiagnostics);
const lastError = parseErrorAtCurrentToken(Diagnostics.A_JSDoc_typedef_comment_may_not_contain_multiple_type_tags);
if (lastError) {
addRelatedInfo(
lastError,
createDetachedDiagnostic(fileName, 0, 0, Diagnostics.The_tag_was_first_specified_here)
);
addRelatedInfo(lastError, createDetachedDiagnostic(fileName, 0, 0, Diagnostics.The_tag_was_first_specified_here));
}
break;
}
Expand Down
Expand Up @@ -121,6 +121,7 @@ tests/cases/compiler/constructorWithIncompleteTypeAnnotation.ts(261,1): error TS
if (retValue != 0 ^= {
~~
!!! error TS1005: ')' expected.
!!! related TS1007 tests/cases/compiler/constructorWithIncompleteTypeAnnotation.ts:22:19: The parser expected to find a ')' to match the '(' token here.
Copy link
Member

Choose a reason for hiding this comment

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

Uh-oh, I didn't expect any baseline changes. I counted the spaces and 19 is correct counting from 0; 20 is correct counting from 1. I'm not sure which we do for chars, but I see lines counting from 1 in tests/baselines/reference/jsonParserRecovery/TypeScript_code.errors.txt

I think this change is wrong. Sorry for steering you wrong. =(

~


Expand Down
Expand Up @@ -16,5 +16,4 @@ tests/cases/compiler/errorRecoveryWithDotFollowedByNamespaceKeyword.ts(9,2): err
}

!!! error TS1005: '}' expected.
!!! related TS1007 tests/cases/compiler/errorRecoveryWithDotFollowedByNamespaceKeyword.ts:3:19: The parser expected to find a '}' to match the '{' token here.
!!! related TS1007 tests/cases/compiler/errorRecoveryWithDotFollowedByNamespaceKeyword.ts:2:20: The parser expected to find a '}' to match the '{' token here.
!!! related TS1007 tests/cases/compiler/errorRecoveryWithDotFollowedByNamespaceKeyword.ts:3:18: The parser expected to find a '}' to match the '{' token here.
2 changes: 1 addition & 1 deletion tests/baselines/reference/exportInFunction.errors.txt
Expand Up @@ -7,4 +7,4 @@ tests/cases/compiler/exportInFunction.ts(3,1): error TS1005: '}' expected.


!!! error TS1005: '}' expected.
!!! related TS1007 tests/cases/compiler/exportInFunction.ts:1:14: The parser expected to find a '}' to match the '{' token here.
!!! related TS1007 tests/cases/compiler/exportInFunction.ts:1:13: The parser expected to find a '}' to match the '{' token here.
3 changes: 1 addition & 2 deletions tests/baselines/reference/jsonParserRecovery/JSX.errors.txt
Expand Up @@ -33,5 +33,4 @@ JSX(15,10): error TS1005: '}' expected.
</div>
)

!!! error TS1005: '}' expected.
!!! related TS1007 JSX:4:9: The parser expected to find a '}' to match the '{' token here.
!!! error TS1005: '}' expected.
Expand Up @@ -16,5 +16,4 @@ TypeScript code(1,22): error TS1005: '}' expected.
~~~~
!!! error TS1012: Unexpected token.

!!! error TS1005: '}' expected.
!!! related TS1007 TypeScript code:1:18: The parser expected to find a '}' to match the '{' token here.
!!! error TS1005: '}' expected.
Expand Up @@ -7,5 +7,4 @@ trailing identifier(1,8): error TS1005: '}' expected.
~~~~
!!! error TS1012: Unexpected token.

!!! error TS1005: '}' expected.
!!! related TS1007 trailing identifier:1:4: The parser expected to find a '}' to match the '{' token here.
!!! error TS1005: '}' expected.
2 changes: 1 addition & 1 deletion tests/baselines/reference/missingCloseBrace.errors.txt
Expand Up @@ -13,4 +13,4 @@ tests/cases/compiler/missingCloseBrace.ts(9,1): error TS1005: '}' expected.


!!! error TS1005: '}' expected.
!!! related TS1007 tests/cases/compiler/missingCloseBrace.ts:1:22: The parser expected to find a '}' to match the '{' token here.
!!! related TS1007 tests/cases/compiler/missingCloseBrace.ts:1:21: The parser expected to find a '}' to match the '{' token here.
Expand Up @@ -9,4 +9,4 @@ tests/cases/compiler/missingCloseBraceInObjectLiteral.ts(5,1): error TS1005: '}'


!!! error TS1005: '}' expected.
!!! related TS1007 tests/cases/compiler/missingCloseBraceInObjectLiteral.ts:1:11: The parser expected to find a '}' to match the '{' token here.
!!! related TS1007 tests/cases/compiler/missingCloseBraceInObjectLiteral.ts:1:10: The parser expected to find a '}' to match the '{' token here.
@@ -0,0 +1,8 @@
tests/cases/compiler/missingCloseBracketInArray.ts(1,48): error TS1005: ']' expected.


==== tests/cases/compiler/missingCloseBracketInArray.ts (1 errors) ====
var alphas:string[] = alphas = ["1","2","3","4"

!!! error TS1005: ']' expected.
!!! related TS1007 tests/cases/compiler/missingCloseBracketInArray.ts:1:31: The parser expected to find a ']' to match the '[' token here.
5 changes: 5 additions & 0 deletions tests/baselines/reference/missingCloseBracketInArray.js
@@ -0,0 +1,5 @@
//// [missingCloseBracketInArray.ts]
var alphas:string[] = alphas = ["1","2","3","4"

//// [missingCloseBracketInArray.js]
var alphas = alphas = ["1", "2", "3", "4"];
5 changes: 5 additions & 0 deletions tests/baselines/reference/missingCloseBracketInArray.symbols
@@ -0,0 +1,5 @@
=== tests/cases/compiler/missingCloseBracketInArray.ts ===
var alphas:string[] = alphas = ["1","2","3","4"
>alphas : Symbol(alphas, Decl(missingCloseBracketInArray.ts, 0, 3))
>alphas : Symbol(alphas, Decl(missingCloseBracketInArray.ts, 0, 3))

11 changes: 11 additions & 0 deletions tests/baselines/reference/missingCloseBracketInArray.types
@@ -0,0 +1,11 @@
=== tests/cases/compiler/missingCloseBracketInArray.ts ===
var alphas:string[] = alphas = ["1","2","3","4"
>alphas : string[]
>alphas = ["1","2","3","4" : string[]
>alphas : string[]
>["1","2","3","4" : string[]
>"1" : "1"
>"2" : "2"
>"3" : "3"
>"4" : "4"

32 changes: 32 additions & 0 deletions tests/baselines/reference/missingCloseParenStatements.errors.txt
@@ -0,0 +1,32 @@
tests/cases/compiler/missingCloseParenStatements.ts(2,26): error TS1005: ')' expected.
tests/cases/compiler/missingCloseParenStatements.ts(4,5): error TS1005: ')' expected.
tests/cases/compiler/missingCloseParenStatements.ts(8,39): error TS1005: ')' expected.
tests/cases/compiler/missingCloseParenStatements.ts(11,35): error TS1005: ')' expected.


==== tests/cases/compiler/missingCloseParenStatements.ts (4 errors) ====
var a1, a2, a3 = 0;
if ( a1 && (a2 + a3 > 0) {
~
!!! error TS1005: ')' expected.
!!! related TS1007 tests/cases/compiler/missingCloseParenStatements.ts:2:3: The parser expected to find a ')' to match the '(' token here.
while( (a2 > 0) && a1
{
~
!!! error TS1005: ')' expected.
!!! related TS1007 tests/cases/compiler/missingCloseParenStatements.ts:3:10: The parser expected to find a ')' to match the '(' token here.
do {
var i = i + 1;
a1 = a1 + i;
with ((a2 + a3 > 0) && a1 {
~
!!! error TS1005: ')' expected.
!!! related TS1007 tests/cases/compiler/missingCloseParenStatements.ts:8:17: The parser expected to find a ')' to match the '(' token here.
console.log(x);
}
} while (i < 5 && (a1 > 5);
~
!!! error TS1005: ')' expected.
!!! related TS1007 tests/cases/compiler/missingCloseParenStatements.ts:11:16: The parser expected to find a ')' to match the '(' token here.
}
}
28 changes: 28 additions & 0 deletions tests/baselines/reference/missingCloseParenStatements.js
@@ -0,0 +1,28 @@
//// [missingCloseParenStatements.ts]
var a1, a2, a3 = 0;
if ( a1 && (a2 + a3 > 0) {
while( (a2 > 0) && a1
{
do {
var i = i + 1;
a1 = a1 + i;
with ((a2 + a3 > 0) && a1 {
console.log(x);
}
} while (i < 5 && (a1 > 5);
}
}

//// [missingCloseParenStatements.js]
var a1, a2, a3 = 0;
if (a1 && (a2 + a3 > 0)) {
while ((a2 > 0) && a1) {
do {
var i = i + 1;
a1 = a1 + i;
with ((a2 + a3 > 0) && a1) {
console.log(x);
}
} while (i < 5 && (a1 > 5));
}
}