rework parsing local variable declaration with nullable type
... and address comment in https://dart-review.googlesource.com/c/sdk/+/88301
Change-Id: I96c9bcf995fad58403ab455e907843a40339ef84
Reviewed-on: https://dart-review.googlesource.com/c/88303
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Dan Rubel <danrubel@google.com>
Commit-Queue: Dan Rubel <danrubel@google.com>
diff --git a/pkg/analyzer/test/generated/parser_fasta_test.dart b/pkg/analyzer/test/generated/parser_fasta_test.dart
index 9e2ad74..827110a 100644
--- a/pkg/analyzer/test/generated/parser_fasta_test.dart
+++ b/pkg/analyzer/test/generated/parser_fasta_test.dart
@@ -1443,6 +1443,47 @@
return unit;
}
+ void test_assignment_complex() {
+ parseNNBDCompilationUnit('D? foo(X? x) { X? x1; X? x2 = x + bar(7); }');
+ }
+
+ void test_assignment_simple() {
+ parseNNBDCompilationUnit('D? foo(X? x) { X? x1; X? x2 = x; }');
+ }
+
+ void test_conditional() {
+ parseNNBDCompilationUnit('D? foo(X? x) { X ? 7 : y; }');
+ }
+
+ void test_conditional_complex() {
+ parseNNBDCompilationUnit('D? foo(X? x) { X ? x2 = x + bar(7) : y; }');
+ }
+
+ void test_conditional_simple() {
+ parseNNBDCompilationUnit('D? foo(X? x) { X ? x2 = x : y; }');
+ }
+
+ void test_for() {
+ parseNNBDCompilationUnit('main() { for(int x = 0; x < 7; ++x) { } }');
+ }
+
+ void test_for_conditional() {
+ parseNNBDCompilationUnit(
+ 'main() { for(x ? y = 7 : y = 8; y < 10; ++y) { } }');
+ }
+
+ void test_for_nullable() {
+ parseNNBDCompilationUnit('main() { for(int? x = 0; x < 7; ++x) { } }');
+ }
+
+ void test_foreach() {
+ parseNNBDCompilationUnit('main() { for(int x in [7]) { } }');
+ }
+
+ void test_foreach_nullable() {
+ parseNNBDCompilationUnit('main() { for(int? x in [7, null]) { } }');
+ }
+
void test_is_nullable() {
CompilationUnit unit =
parseNNBDCompilationUnit('main() { x is String? ? (x + y) : z; }');
@@ -1476,10 +1517,6 @@
expect(elseExpression, isSimpleIdentifier);
}
- void test_simple_assignment() {
- parseNNBDCompilationUnit('D? foo(X? x) { X? x1; X? x2 = x; }');
- }
-
void test_pragma_missing() {
createParser("library foo;");
_parserProxy.astBuilder.enableNonNullable = true;
diff --git a/pkg/front_end/lib/src/fasta/parser/forwarding_listener.dart b/pkg/front_end/lib/src/fasta/parser/forwarding_listener.dart
index ef520f0..51e3b9e 100644
--- a/pkg/front_end/lib/src/fasta/parser/forwarding_listener.dart
+++ b/pkg/front_end/lib/src/fasta/parser/forwarding_listener.dart
@@ -541,11 +541,6 @@
}
@override
- void handleExpressionStatement(Token token) {
- listener?.handleExpressionStatement(token);
- }
-
- @override
void endFactoryMethod(
Token beginToken, Token factoryKeyword, Token endToken) {
listener?.endFactoryMethod(beginToken, factoryKeyword, endToken);
@@ -997,6 +992,11 @@
}
@override
+ void handleExpressionStatement(Token token) {
+ listener?.handleExpressionStatement(token);
+ }
+
+ @override
void handleExtraneousExpression(Token token, Message message) {
listener?.handleExtraneousExpression(token, message);
}
diff --git a/pkg/front_end/lib/src/fasta/parser/parser.dart b/pkg/front_end/lib/src/fasta/parser/parser.dart
index 1938d8c..a3a59c2 100644
--- a/pkg/front_end/lib/src/fasta/parser/parser.dart
+++ b/pkg/front_end/lib/src/fasta/parser/parser.dart
@@ -5051,9 +5051,6 @@
TypeInfo typeInfo,
bool onlyParseVariableDeclarationStart = false]) {
typeInfo ??= computeType(beforeType, false);
- if (typeInfo.isConditionalExpressionStart(beforeType, this)) {
- typeInfo = noType;
- }
Token token = typeInfo.skipType(beforeType);
Token next = token.next;
@@ -5073,11 +5070,49 @@
computeTypeParamOrArg(next).parseVariables(next, this);
listener.beginLocalFunctionDeclaration(start.next);
token = typeInfo.parseType(beforeType, this);
- next = token.next;
return parseNamedFunctionRest(token, start.next, beforeFormals, false);
}
}
+ if (beforeType == start &&
+ typeInfo.isNullable &&
+ typeInfo.couldBeExpression) {
+ assert(optional('?', token));
+ assert(next.isIdentifier);
+ Token afterIdentifier = next.next;
+ //
+ // found <typeref> `?` <identifier>
+ // with no annotations or modifiers preceeding it
+ //
+ if (optional('=', afterIdentifier)) {
+ //
+ // look past the next expression
+ // to determine if this is part of a conditional expression
+ //
+ final originalListener = listener;
+ listener = new ForwardingListener();
+ // TODO(danrubel): consider using TokenStreamGhostWriter here
+ Token afterExpression =
+ parseExpressionWithoutCascade(afterIdentifier).next;
+ listener = originalListener;
+
+ if (optional(':', afterExpression)) {
+ // Looks like part of a conditional expression.
+ // Drop the type information and reset the last consumed token.
+ typeInfo = noType;
+ token = start;
+ next = token.next;
+ }
+ } else if (!afterIdentifier.isKeyword &&
+ !isOneOfOrEof(afterIdentifier, const [';', ',', ')'])) {
+ // Looks like part of a conditional expression.
+ // Drop the type information and reset the last consumed token.
+ typeInfo = noType;
+ token = start;
+ next = token.next;
+ }
+ }
+
if (token == start) {
// If no annotation, modifier, or type, and this is not a local function
// then this must be an expression statement.
@@ -5087,6 +5122,7 @@
return parseExpressionStatement(start);
}
}
+
if (next.type.isBuiltIn &&
beforeType == start &&
typeInfo.couldBeExpression) {
diff --git a/pkg/front_end/lib/src/fasta/parser/type_info.dart b/pkg/front_end/lib/src/fasta/parser/type_info.dart
index b6e38f5..0871621 100644
--- a/pkg/front_end/lib/src/fasta/parser/type_info.dart
+++ b/pkg/front_end/lib/src/fasta/parser/type_info.dart
@@ -44,13 +44,6 @@
/// in valid code or during recovery.
Token ensureTypeOrVoid(Token token, Parser parser);
- /// Return `true` if the tokens comprising the type represented by the
- /// receiver are the start of a conditional expression.
- /// For example, `A?` or `A.b?` could be the start of a conditional expression
- /// and require arbitrary look ahead to determine if it is,
- /// while `A<T>?` cannot be the start of a conditional expression.
- bool isConditionalExpressionStart(Token token, Parser parser);
-
/// Call this function to parse an optional type (not void) after [token].
/// This function will call the appropriate event methods on the [Parser]'s
/// listener to handle the type. This may modify the token stream
diff --git a/pkg/front_end/lib/src/fasta/parser/type_info_impl.dart b/pkg/front_end/lib/src/fasta/parser/type_info_impl.dart
index c500e04..de844a0 100644
--- a/pkg/front_end/lib/src/fasta/parser/type_info_impl.dart
+++ b/pkg/front_end/lib/src/fasta/parser/type_info_impl.dart
@@ -26,7 +26,6 @@
import 'util.dart'
show
- isOneOfOrEof,
optional,
skipMetadata,
splitGtEq,
@@ -114,9 +113,6 @@
ensureTypeNotVoid(token, parser);
@override
- bool isConditionalExpressionStart(Token token, Parser parser) => false;
-
- @override
Token parseTypeNotVoid(Token token, Parser parser) =>
parseType(token, parser);
@@ -154,9 +150,6 @@
parseType(token, parser);
@override
- bool isConditionalExpressionStart(Token token, Parser parser) => false;
-
- @override
Token parseTypeNotVoid(Token token, Parser parser) =>
parseType(token, parser);
@@ -198,10 +191,6 @@
bool get isNullable => true;
@override
- bool isConditionalExpressionStart(Token token, Parser parser) =>
- isConditionalThenExpression(skipType(token), parser);
-
- @override
Token parseTypeRest(Token start, Token token, Parser parser) {
token = token.next;
assert(optional('?', token));
@@ -241,9 +230,6 @@
parseType(token, parser);
@override
- bool isConditionalExpressionStart(Token token, Parser parser) => false;
-
- @override
Token parseTypeNotVoid(Token token, Parser parser) =>
parseType(token, parser);
@@ -280,10 +266,6 @@
bool get isNullable => true;
@override
- bool isConditionalExpressionStart(Token token, Parser parser) =>
- isConditionalThenExpression(skipType(token), parser);
-
- @override
Token parseTypeRest(Token start, Parser parser) {
Token token = start.next;
assert(optional('?', token));
@@ -319,9 +301,6 @@
parseType(token, parser);
@override
- bool isConditionalExpressionStart(Token token, Parser parser) => false;
-
- @override
Token parseTypeNotVoid(Token token, Parser parser) =>
parseType(token, parser);
@@ -370,9 +349,6 @@
parseType(token, parser);
@override
- bool isConditionalExpressionStart(Token token, Parser parser) => false;
-
- @override
Token parseTypeNotVoid(Token token, Parser parser) =>
ensureTypeNotVoid(token, parser);
@@ -455,12 +431,6 @@
parseType(token, parser);
@override
- bool isConditionalExpressionStart(Token token, Parser parser) {
- //return isConditionalThenExpression(token.next.next, parser);
- return false;
- }
-
- @override
Token parseTypeNotVoid(Token token, Parser parser) =>
parseType(token, parser);
@@ -1169,40 +1139,3 @@
}
return null;
}
-
-/// Return `true` if the tokens after [token]
-/// represent a valid expression followed by a `:`.
-bool isConditionalThenExpression(Token token, Parser parser) {
- // Quick checks for simple situations
- Token next = token.next;
- if (isOneOfOrEof(next, const ['?', ')', ';'])) {
- return false;
- } else if (next.isIdentifier) {
- next = next.next;
- if (optional(';', next)) {
- // <identifier> `;`
- return false;
- } else if (isOneOfOrEof(next, const [
- // Assignment operators
- // TODO(danrubel): Consider replacing this with Token.isAssignmentOperator
- '=', '&=', '&&=', '|=', '||=', '^=', '>>=', '<<=', '-=', '%=',
- '+=', '??=', '/=', '*=', '~/='
- ])) {
- if (optional(';', next.next.next)) {
- // <identifier> = <token> `;`
- return false;
- }
- }
- }
-
- // TODO(danrubel): Enable more complex heavy weight lookahead
- // once the parser can do it without modifying the token stream.
- /*
- final originalListener = parser.listener;
- parser.listener = new ForwardingListener();
- token = parser.parseExpression(token);
- parser.listener = originalListener;
- return optional(':', token.next);
- */
- return true;
-}
