Skip to content

Commit

Permalink
[stable] [parser] Ensure that list and map pattern parsing always mak…
Browse files Browse the repository at this point in the history
…es progress.

In rare circumstances involving syntax errors, the `parsePattern`
method inserts a synthetic token but does not consume any
tokens. Usually when this happens it's not a problem, because whatever
method is calling `parsePattern` consumes some tokens, so the parser
always makes progress. However, when parsing list patterns, after
calling `parsePattern`, the parser would look for a `,`, and if it
didn't find one, it would supply a synthetic `,` and call
`parsePattern` again, resulting in an infinite loop. A similar
situation happened with map patterns, though the situation was more
complex because in between the calls to `parsePattern`, the parser
would also create synthetic key expressions and `:`s.

To fix the problem, when parsing a list or map pattern, after the call
to `parsePattern`, the parser checks whether any tokens were
consumed. If no tokens were consumed, it ignores the next token from
the input stream in order to make progress.

I also investigated whether there were similar issues with
parenthesized/record patterns and switch expressions, since those
constructs also consist of a sequence of patterns separated by tokens
and other things that could in principle be supplied
synthetically. Fortunately, parser recovery doesn't get into an
infinite loop in those cases, so I didn't make any further
changes. But I did include test cases to make sure.

Fixes: #52376

Bug: #52352
Change-Id: Ic9abf1bb82b8d7e1eb18e8a771a2ff9116fcfe21
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/302803
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/303081
Reviewed-by: Jens Johansen <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
  • Loading branch information
stereotype441 authored and Commit Queue committed May 19, 2023
1 parent 1618a77 commit 45799c3
Show file tree
Hide file tree
Showing 27 changed files with 1,295 additions and 0 deletions.
12 changes: 12 additions & 0 deletions pkg/_fe_analyzer_shared/lib/src/parser/parser_impl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10028,6 +10028,12 @@ class Parser {
listener.handleRestPattern(dots, hasSubPattern: hasSubPattern);
} else {
token = parsePattern(token, patternContext);
if (identical(next, token.next)) {
// No tokens were consumed (though it's possible that a synthetic
// token was inserted). If this happens, go ahead and skip the next
// token to ensure that progress is made.
token = token.next!;
}
}
next = token.next!;
++count;
Expand Down Expand Up @@ -10103,6 +10109,12 @@ class Parser {
new SyntheticToken(TokenType.COLON, next.charOffset));
}
token = parsePattern(colon, patternContext);
if (identical(next, token.next)) {
// No tokens were consumed (though it's possible that a synthetic
// token was inserted). If this happens, go ahead and skip the next
// token to ensure that progress is made.
token = token.next!;
}
listener.handleMapPatternEntry(colon, token.next!);
}
++count;
Expand Down
152 changes: 152 additions & 0 deletions pkg/analyzer/test/generated/patterns_parser_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9929,6 +9929,158 @@ SwitchExpression
''');
}

test_syntheticIdentifier_insideListPattern() {
_parse('''
void f(Object? x) {
switch (x) {
case [if]:
};
}
''', errors: [
error(ParserErrorCode.MISSING_IDENTIFIER, 45, 2),
]);
var node = findNode.switchPatternCase('case');
assertParsedNodeText(node, r'''
SwitchPatternCase
keyword: case
guardedPattern: GuardedPattern
pattern: ListPattern
leftBracket: [
elements
ConstantPattern
expression: SimpleIdentifier
token: <empty> <synthetic>
rightBracket: ]
colon: :
''');
}

test_syntheticIdentifier_insideMapPattern() {
_parse('''
void f(Object? x) {
switch (x) {
case {0: if}:
};
}
''', errors: [
error(ParserErrorCode.MISSING_IDENTIFIER, 48, 2),
error(ParserErrorCode.EXPECTED_TOKEN, 48, 2),
error(ParserErrorCode.EXPECTED_TOKEN, 48, 2),
]);
var node = findNode.switchPatternCase('case');
assertParsedNodeText(node, r'''
SwitchPatternCase
keyword: case
guardedPattern: GuardedPattern
pattern: MapPattern
leftBracket: {
elements
MapPatternEntry
key: IntegerLiteral
literal: 0
separator: :
value: ConstantPattern
expression: SimpleIdentifier
token: <empty> <synthetic>
MapPatternEntry
key: SimpleIdentifier
token: <empty> <synthetic>
separator: : <synthetic>
value: ConstantPattern
expression: SimpleIdentifier
token: <empty> <synthetic>
rightBracket: }
colon: :
''');
}

test_syntheticIdentifier_insideParenthesizedPattern() {
_parse('''
void f(Object? x) {
switch (x) {
case (if):
};
}
''', errors: [
error(ParserErrorCode.MISSING_IDENTIFIER, 45, 2),
error(ParserErrorCode.EXPECTED_TOKEN, 45, 2),
]);
var node = findNode.switchPatternCase('case');
assertParsedNodeText(node, r'''
SwitchPatternCase
keyword: case
guardedPattern: GuardedPattern
pattern: ParenthesizedPattern
leftParenthesis: (
pattern: ConstantPattern
expression: SimpleIdentifier
token: <empty> <synthetic>
rightParenthesis: )
colon: :
''');
}

test_syntheticIdentifier_insideRecordPattern() {
_parse('''
void f(Object? x) {
switch (x) {
case (_, if):
};
}
''', errors: [
error(ParserErrorCode.MISSING_IDENTIFIER, 48, 2),
error(ParserErrorCode.EXPECTED_TOKEN, 48, 2),
]);
var node = findNode.switchPatternCase('case');
assertParsedNodeText(node, r'''
SwitchPatternCase
keyword: case
guardedPattern: GuardedPattern
pattern: RecordPattern
leftParenthesis: (
fields
PatternField
pattern: WildcardPattern
name: _
PatternField
pattern: ConstantPattern
expression: SimpleIdentifier
token: <empty> <synthetic>
rightParenthesis: )
colon: :
''');
}

test_syntheticIdentifier_insideSwitchExpression() {
_parse('''
void f(Object? x) => switch (x) {if};
''', errors: [
error(ParserErrorCode.MISSING_IDENTIFIER, 33, 2),
error(ParserErrorCode.EXPECTED_TOKEN, 33, 2),
error(ParserErrorCode.EXPECTED_TOKEN, 33, 2),
]);
var node = findNode.switchExpression('if');
assertParsedNodeText(node, r'''
SwitchExpression
switchKeyword: switch
leftParenthesis: (
expression: SimpleIdentifier
token: x
rightParenthesis: )
leftBracket: {
cases
SwitchExpressionCase
guardedPattern: GuardedPattern
pattern: ConstantPattern
expression: SimpleIdentifier
token: <empty> <synthetic>
arrow: => <synthetic>
expression: SimpleIdentifier
token: <empty> <synthetic>
rightBracket: }
''');
}

test_typeQuestionBeforeWhen_conditional() {
// The logic for parsing types has special disambiguation rules for deciding
// whether a trailing `?` should be included in the type; these rules are
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
void f(Object? x) {
switch (x) {
case [if]:
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
Problems reported:

parser/patterns/syntheticIdentifier_insideListPattern:3:11: Expected an identifier, but got 'if'.
case [if]:
^^

beginCompilationUnit(void)
beginMetadataStar(void)
endMetadataStar(0)
beginTopLevelMember(void)
beginTopLevelMethod(, null, null)
handleVoidKeyword(void)
handleIdentifier(f, topLevelFunctionDeclaration)
handleNoTypeVariables(()
beginFormalParameters((, MemberKind.TopLevelMethod)
beginMetadataStar(Object)
endMetadataStar(0)
beginFormalParameter(Object, MemberKind.TopLevelMethod, null, null, null)
handleIdentifier(Object, typeReference)
handleNoTypeArguments(?)
handleType(Object, ?)
handleIdentifier(x, formalParameterDeclaration)
handleFormalParameterWithoutValue())
endFormalParameter(null, null, null, x, null, null, FormalParameterKind.requiredPositional, MemberKind.TopLevelMethod)
endFormalParameters(1, (, ), MemberKind.TopLevelMethod)
handleAsyncModifier(null, null)
beginBlockFunctionBody({)
beginSwitchStatement(switch)
handleIdentifier(x, expression)
handleNoTypeArguments())
handleNoArguments())
handleSend(x, ))
handleParenthesizedCondition((, null, null)
beginSwitchBlock({)
beginCaseExpression(case)
handleNoTypeArguments([)
beginConstantPattern(null)
handleRecoverableError(Message[ExpectedIdentifier, Expected an identifier, but got 'if'., Try inserting an identifier before 'if'., {lexeme: if}], if, if)
handleIdentifier(, expression)
handleNoTypeArguments(if)
handleNoArguments(if)
handleSend(, if)
endConstantPattern(null)
handleListPattern(1, [, ])
handleSwitchCaseNoWhenClause(])
endCaseExpression(case, null, :)
beginSwitchCase(0, 1, case)
endSwitchCase(0, 1, null, null, 0, case, })
endSwitchBlock(1, {, })
endSwitchStatement(switch, })
handleEmptyStatement(;)
endBlockFunctionBody(2, {, })
endTopLevelMethod(void, null, })
endTopLevelDeclaration()
endCompilationUnit(1, )
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
parseUnit(void)
skipErrorTokens(void)
listener: beginCompilationUnit(void)
syntheticPreviousToken(void)
parseTopLevelDeclarationImpl(, Instance of 'DirectiveContext')
parseMetadataStar()
listener: beginMetadataStar(void)
listener: endMetadataStar(0)
parseTopLevelMemberImpl()
listener: beginTopLevelMember(void)
parseTopLevelMethod(, null, null, , Instance of 'VoidType', null, f, false)
listener: beginTopLevelMethod(, null, null)
listener: handleVoidKeyword(void)
ensureIdentifierPotentiallyRecovered(void, topLevelFunctionDeclaration, false)
listener: handleIdentifier(f, topLevelFunctionDeclaration)
parseMethodTypeVar(f)
listener: handleNoTypeVariables(()
parseGetterOrFormalParameters(f, f, false, MemberKind.TopLevelMethod)
parseFormalParameters(f, MemberKind.TopLevelMethod)
parseFormalParametersRest((, MemberKind.TopLevelMethod)
listener: beginFormalParameters((, MemberKind.TopLevelMethod)
parseFormalParameter((, FormalParameterKind.requiredPositional, MemberKind.TopLevelMethod)
parseMetadataStar(()
listener: beginMetadataStar(Object)
listener: endMetadataStar(0)
listener: beginFormalParameter(Object, MemberKind.TopLevelMethod, null, null, null)
listener: handleIdentifier(Object, typeReference)
listener: handleNoTypeArguments(?)
listener: handleType(Object, ?)
ensureIdentifier(?, formalParameterDeclaration)
listener: handleIdentifier(x, formalParameterDeclaration)
listener: handleFormalParameterWithoutValue())
listener: endFormalParameter(null, null, null, x, null, null, FormalParameterKind.requiredPositional, MemberKind.TopLevelMethod)
listener: endFormalParameters(1, (, ), MemberKind.TopLevelMethod)
parseAsyncModifierOpt())
listener: handleAsyncModifier(null, null)
inPlainSync()
parseFunctionBody(), false, false)
listener: beginBlockFunctionBody({)
notEofOrValue(}, switch)
parseStatement({)
parseStatementX({)
parseSwitchStatement({)
listener: beginSwitchStatement(switch)
ensureParenthesizedCondition(switch, allowCase: false)
parseExpressionInParenthesisRest((, allowCase: false)
parseExpression(()
looksLikeOuterPatternEquals(()
skipOuterPattern(()
skipObjectPatternRest(x)
parsePrecedenceExpression((, 1, true, ConstantPatternContext.none)
parseUnaryExpression((, true, ConstantPatternContext.none)
parsePrimary((, expression, ConstantPatternContext.none)
parseSendOrFunctionLiteral((, expression, ConstantPatternContext.none)
parseSend((, expression, ConstantPatternContext.none)
isNextIdentifier(()
ensureIdentifier((, expression)
listener: handleIdentifier(x, expression)
listener: handleNoTypeArguments())
parseArgumentsOpt(x)
listener: handleNoArguments())
listener: handleSend(x, ))
ensureCloseParen(x, ()
listener: handleParenthesizedCondition((, null, null)
parseSwitchBlock())
ensureBlock(), null, switch statement)
listener: beginSwitchBlock({)
notEofOrValue(}, case)
peekPastLabels(case)
listener: beginCaseExpression(case)
parsePattern(case, PatternContext.matching, precedence: 1)
parsePrimaryPattern(case, PatternContext.matching)
listener: handleNoTypeArguments([)
parseListPatternSuffix(case, PatternContext.matching)
parsePattern([, PatternContext.matching, precedence: 1)
parsePrimaryPattern([, PatternContext.matching)
listener: beginConstantPattern(null)
parsePrecedenceExpression([, 7, false, ConstantPatternContext.implicit)
parseUnaryExpression([, false, ConstantPatternContext.implicit)
parsePrimary([, expression, ConstantPatternContext.implicit)
inPlainSync()
parseSend([, expression, ConstantPatternContext.implicit)
isNextIdentifier([)
ensureIdentifier([, expression)
reportRecoverableErrorWithToken(if, Instance of 'Template<(Token) => Message>')
listener: handleRecoverableError(Message[ExpectedIdentifier, Expected an identifier, but got 'if'., Try inserting an identifier before 'if'., {lexeme: if}], if, if)
rewriter()
listener: handleIdentifier(, expression)
listener: handleNoTypeArguments(if)
parseArgumentsOpt()
listener: handleNoArguments(if)
listener: handleSend(, if)
listener: endConstantPattern(null)
listener: handleListPattern(1, [, ])
listener: handleSwitchCaseNoWhenClause(])
ensureColon(])
listener: endCaseExpression(case, null, :)
peekPastLabels(})
parseStatementsInSwitchCase(:, }, case, 0, 1, null, null)
listener: beginSwitchCase(0, 1, case)
listener: endSwitchCase(0, 1, null, null, 0, case, })
notEofOrValue(}, })
listener: endSwitchBlock(1, {, })
listener: endSwitchStatement(switch, })
notEofOrValue(}, ;)
parseStatement(})
parseStatementX(})
parseEmptyStatement(})
listener: handleEmptyStatement(;)
notEofOrValue(}, })
listener: endBlockFunctionBody(2, {, })
listener: endTopLevelMethod(void, null, })
listener: endTopLevelDeclaration()
reportAllErrorTokens(void)
listener: endCompilationUnit(1, )
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
NOTICE: Stream was rewritten by parser!

void f(Object? x) {
switch (x) {
case [*synthetic*if]:
};
}


void[KeywordToken] f[StringToken]([BeginToken]Object[StringToken]?[SimpleToken] x[StringToken])[SimpleToken] {[BeginToken]
switch[KeywordToken] ([BeginToken]x[StringToken])[SimpleToken] {[BeginToken]
case[KeywordToken] [[BeginToken][SyntheticStringToken]if[KeywordToken]][SimpleToken]:[SimpleToken]
}[SimpleToken];[SimpleToken]
}[SimpleToken]
[SimpleToken]
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
void f(Object? x) {
switch (x) {
case [if]:
};
}


void[KeywordToken] f[StringToken]([BeginToken]Object[StringToken]?[SimpleToken] x[StringToken])[SimpleToken] {[BeginToken]
switch[KeywordToken] ([BeginToken]x[StringToken])[SimpleToken] {[BeginToken]
case[KeywordToken] [[BeginToken]if[KeywordToken]][SimpleToken]:[SimpleToken]
}[SimpleToken];[SimpleToken]
}[SimpleToken]
[SimpleToken]
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
void f(Object? x) {
switch (x) {
case {0: if}:
};
}
Loading

0 comments on commit 45799c3

Please sign in to comment.