Skip to content

Commit 69f8d4e

Browse files
bwilkersoncommit-bot@chromium.org
authored andcommitted
Fix parsing of super expressions (issue 32393)
Change-Id: I887288254eb1be33c111b6dedd16c2acd3eb3dd1 Reviewed-on: https://dart-review.googlesource.com/45440 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: William Hesse <[email protected]>
1 parent 2ace7c9 commit 69f8d4e

File tree

2 files changed

+87
-66
lines changed

2 files changed

+87
-66
lines changed

pkg/analyzer/lib/src/generated/parser.dart

Lines changed: 62 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -846,12 +846,52 @@ class Parser {
846846
* | identifier
847847
*/
848848
Expression parseAssignableExpression(bool primaryAllowed) {
849-
if (_matchesKeyword(Keyword.SUPER)) {
850-
return parseAssignableSelector(
851-
astFactory.superExpression(getAndAdvance()), false,
852-
allowConditional: false);
849+
//
850+
// A primary expression can start with an identifier. We resolve the
851+
// ambiguity by determining whether the primary consists of anything other
852+
// than an identifier and/or is followed by an assignableSelector.
853+
//
854+
Expression expression = parsePrimaryExpression();
855+
bool isOptional =
856+
primaryAllowed || _isValidAssignableExpression(expression);
857+
while (true) {
858+
while (_isLikelyArgumentList()) {
859+
TypeArgumentList typeArguments = _parseOptionalTypeArguments();
860+
ArgumentList argumentList = parseArgumentList();
861+
Expression currentExpression = expression;
862+
if (currentExpression is SimpleIdentifier) {
863+
expression = astFactory.methodInvocation(
864+
null, null, currentExpression, typeArguments, argumentList);
865+
} else if (currentExpression is PrefixedIdentifier) {
866+
expression = astFactory.methodInvocation(
867+
currentExpression.prefix,
868+
currentExpression.period,
869+
currentExpression.identifier,
870+
typeArguments,
871+
argumentList);
872+
} else if (currentExpression is PropertyAccess) {
873+
expression = astFactory.methodInvocation(
874+
currentExpression.target,
875+
currentExpression.operator,
876+
currentExpression.propertyName,
877+
typeArguments,
878+
argumentList);
879+
} else {
880+
expression = astFactory.functionExpressionInvocation(
881+
expression, typeArguments, argumentList);
882+
}
883+
if (!primaryAllowed) {
884+
isOptional = false;
885+
}
886+
}
887+
Expression selectorExpression = parseAssignableSelector(
888+
expression, isOptional || (expression is PrefixedIdentifier));
889+
if (identical(selectorExpression, expression)) {
890+
return expression;
891+
}
892+
expression = selectorExpression;
893+
isOptional = true;
853894
}
854-
return _parseAssignableExpressionNotStartingWithSuper(primaryAllowed);
855895
}
856896

857897
/**
@@ -4457,7 +4497,7 @@ class Parser {
44574497
*
44584498
* selector ::=
44594499
* assignableSelector
4460-
* | argumentList
4500+
* | argumentPart
44614501
*/
44624502
Expression parsePostfixExpression() {
44634503
Expression operand = parseAssignableExpression(true);
@@ -4583,8 +4623,6 @@ class Parser {
45834623
} else if (keyword == Keyword.THIS) {
45844624
return astFactory.thisExpression(getAndAdvance());
45854625
} else if (keyword == Keyword.SUPER) {
4586-
// TODO(paulberry): verify with Gilad that "super" must be followed by
4587-
// unconditionalAssignableSelector in this case.
45884626
return parseAssignableSelector(
45894627
astFactory.superExpression(getAndAdvance()), false,
45904628
allowConditional: false);
@@ -5416,7 +5454,7 @@ class Parser {
54165454
operator, astFactory.superExpression(getAndAdvance()));
54175455
}
54185456
return astFactory.prefixExpression(
5419-
operator, _parseAssignableExpressionNotStartingWithSuper(false));
5457+
operator, parseAssignableExpression(false));
54205458
} else if (type == TokenType.PLUS) {
54215459
_reportErrorForCurrentToken(ParserErrorCode.MISSING_IDENTIFIER);
54225460
return createSyntheticIdentifier();
@@ -6381,6 +6419,21 @@ class Parser {
63816419
return false;
63826420
}
63836421

6422+
/**
6423+
* Return `true` if the given [expression] is a primary expression that is
6424+
* allowed to be an assignable expression without any assignable selector.
6425+
*/
6426+
bool _isValidAssignableExpression(Expression expression) {
6427+
if (expression is SimpleIdentifier) {
6428+
return true;
6429+
} else if (expression is PropertyAccess) {
6430+
return expression.target is SuperExpression;
6431+
} else if (expression is IndexExpression) {
6432+
return expression.target is SuperExpression;
6433+
}
6434+
return false;
6435+
}
6436+
63846437
/**
63856438
* Increments the error reporting lock level. If level is more than `0`, then
63866439
* [reportError] wont report any error.
@@ -6519,61 +6572,6 @@ class Parser {
65196572
keyword, leftParen, expression, comma, message, rightParen);
65206573
}
65216574

6522-
/**
6523-
* Parse an assignable expression given that the current token is not 'super'.
6524-
* The [primaryAllowed] is `true` if the expression is allowed to be a primary
6525-
* without any assignable selector. Return the assignable expression that was
6526-
* parsed.
6527-
*/
6528-
Expression _parseAssignableExpressionNotStartingWithSuper(
6529-
bool primaryAllowed) {
6530-
//
6531-
// A primary expression can start with an identifier. We resolve the
6532-
// ambiguity by determining whether the primary consists of anything other
6533-
// than an identifier and/or is followed by an assignableSelector.
6534-
//
6535-
Expression expression = parsePrimaryExpression();
6536-
bool isOptional = primaryAllowed || expression is SimpleIdentifier;
6537-
while (true) {
6538-
while (_isLikelyArgumentList()) {
6539-
TypeArgumentList typeArguments = _parseOptionalTypeArguments();
6540-
ArgumentList argumentList = parseArgumentList();
6541-
Expression currentExpression = expression;
6542-
if (currentExpression is SimpleIdentifier) {
6543-
expression = astFactory.methodInvocation(
6544-
null, null, currentExpression, typeArguments, argumentList);
6545-
} else if (currentExpression is PrefixedIdentifier) {
6546-
expression = astFactory.methodInvocation(
6547-
currentExpression.prefix,
6548-
currentExpression.period,
6549-
currentExpression.identifier,
6550-
typeArguments,
6551-
argumentList);
6552-
} else if (currentExpression is PropertyAccess) {
6553-
expression = astFactory.methodInvocation(
6554-
currentExpression.target,
6555-
currentExpression.operator,
6556-
currentExpression.propertyName,
6557-
typeArguments,
6558-
argumentList);
6559-
} else {
6560-
expression = astFactory.functionExpressionInvocation(
6561-
expression, typeArguments, argumentList);
6562-
}
6563-
if (!primaryAllowed) {
6564-
isOptional = false;
6565-
}
6566-
}
6567-
Expression selectorExpression = parseAssignableSelector(
6568-
expression, isOptional || (expression is PrefixedIdentifier));
6569-
if (identical(selectorExpression, expression)) {
6570-
return expression;
6571-
}
6572-
expression = selectorExpression;
6573-
isOptional = true;
6574-
}
6575-
}
6576-
65776575
/**
65786576
* Parse a block when we need to check for an open curly brace and recover
65796577
* when there isn't one. Return the block that was parsed.

pkg/analyzer/test/generated/parser_test.dart

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3717,6 +3717,20 @@ class Wrong<T> {
37173717
// [expectedError(ParserErrorCode.GETTER_WITH_PARAMETERS, 9, 2)]);
37183718
}
37193719

3720+
void test_illegalAssignmentToNonAssignable_assign_int() {
3721+
parseStatement("0 = 1;");
3722+
listener.assertErrors([
3723+
expectedError(ParserErrorCode.ILLEGAL_ASSIGNMENT_TO_NON_ASSIGNABLE, 0, 1)
3724+
]);
3725+
}
3726+
3727+
void test_illegalAssignmentToNonAssignable_assign_this() {
3728+
parseStatement("this = 1;");
3729+
listener.assertErrors([
3730+
expectedError(ParserErrorCode.ILLEGAL_ASSIGNMENT_TO_NON_ASSIGNABLE, 0, 4)
3731+
]);
3732+
}
3733+
37203734
void test_illegalAssignmentToNonAssignable_postfix_minusMinus_literal() {
37213735
parseExpression("0--", errors: [
37223736
expectedError(ParserErrorCode.ILLEGAL_ASSIGNMENT_TO_NON_ASSIGNABLE, 1, 2)
@@ -3742,8 +3756,6 @@ class Wrong<T> {
37423756
}
37433757

37443758
void test_illegalAssignmentToNonAssignable_superAssigned() {
3745-
// TODO(brianwilkerson) When this test starts to pass, remove the test
3746-
// test_illegalAssignmentToNonAssignable_superAssigned.
37473759
parseStatement("super = x;");
37483760
listener.assertErrors(usingFastaParser
37493761
? [
@@ -6587,6 +6599,17 @@ abstract class ExpressionParserTestMixin implements AbstractParserTestCase {
65876599
expect(invocation.argumentList, isNotNull);
65886600
}
65896601

6602+
void test_parseExpression_superMethodInvocation_typeArguments_chained() {
6603+
Expression expression = parseExpression('super.b.c<D>()');
6604+
MethodInvocation invocation = expression as MethodInvocation;
6605+
Expression target = invocation.target;
6606+
expect(target, new isInstanceOf<PropertyAccess>());
6607+
expect(invocation.methodName, isNotNull);
6608+
expect(invocation.methodName.name, 'c');
6609+
expect(invocation.typeArguments, isNotNull);
6610+
expect(invocation.argumentList, isNotNull);
6611+
}
6612+
65906613
void test_parseExpressionList_multiple() {
65916614
List<Expression> result = parseExpressionList('1, 2, 3');
65926615
expect(result, isNotNull);

0 commit comments

Comments
 (0)