Skip to content

Commit

Permalink
[analyzer,parser] Check for named before positional in analyzer
Browse files Browse the repository at this point in the history 
Since the check or the absence of the check for a named argument
appearing before a positional parameter is feature-specific
considering the language feature that allows placing the named
arguments anywhere, it should be performed by the listeners of the
parser and guarded by an experiment flag, in alignment with other
experimental featuers. This CL moves the check and the reporting of
the error from the parser into the Analyzer, so that later this check
can be skipped if the corresponding experiment flag is enabled.

Part of #47451.

Change-Id: Ib795d418af429ee04ecfff6dfa71ec1e836fe798
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/216640
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Chloe Stefantsova <dmitryas@google.com>
  • Loading branch information
Chloe Stefantsova authored and commit-bot@chromium.org committed Oct 14, 2021
1 parent 8516cdc commit a666b5a
Show file tree
Hide file tree
Showing 8 changed files with 13 additions and 718 deletions.
5 changes: 0 additions & 5 deletions pkg/_fe_analyzer_shared/lib/src/parser/parser_impl.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6446,7 +6446,6 @@ class Parser {
assert(optional('(', begin));
listener.beginArguments(begin);
int argumentCount = 0;
bool hasSeenNamedArgument = false;
bool old = mayParseFunctionExpressions;
mayParseFunctionExpressions = true;
while (true) {
Expand All @@ -6461,10 +6460,6 @@ class Parser {
ensureIdentifier(token, IdentifierContext.namedArgumentReference)
.next!;
colon = token;
hasSeenNamedArgument = true;
} else if (hasSeenNamedArgument) {
// Positional argument after named argument.
reportRecoverableError(next, codes.messagePositionalAfterNamedArgument);
}
token = parseExpression(token);
next = token.next!;
Expand Down
13 changes: 13 additions & 0 deletions pkg/analyzer/lib/src/fasta/ast_builder.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import 'package:_fe_analyzer_shared/src/messages/codes.dart'
messageMissingAssignableSelector,
messageNativeClauseShouldBeAnnotation,
messageOperatorWithTypeParameters,
messagePositionalAfterNamedArgument,
templateDuplicateLabelInSwitchStatement,
templateExpectedButGot,
templateExpectedIdentifier,
Expand Down Expand Up @@ -577,6 +578,18 @@ class AstBuilder extends StackListener {
var expressions = popTypedList2<Expression>(count);
ArgumentList arguments =
ast.argumentList(leftParenthesis, expressions, rightParenthesis);

bool hasSeenNamedArgument = false;
for (Expression expression in expressions) {
if (expression is NamedExpression) {
hasSeenNamedArgument = true;
} else if (hasSeenNamedArgument) {
// Positional argument after named argument.
handleRecoverableError(messagePositionalAfterNamedArgument,
expression.beginToken, expression.endToken);
}
}

push(ast.methodInvocation(
null, null, _tmpSimpleIdentifier(), null, arguments));
}
Expand Down
15 changes: 0 additions & 15 deletions pkg/front_end/parser_testcases/error_recovery/bracket_mismatch_01.dart.expect
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,5 @@
Problems reported:

parser/error_recovery/bracket_mismatch_01:25:11: Place positional arguments before named arguments.
D(),
^

parser/error_recovery/bracket_mismatch_01:26:11: Place positional arguments before named arguments.
D(),
^

parser/error_recovery/bracket_mismatch_01:27:9: Place positional arguments before named arguments.
]),
^

parser/error_recovery/bracket_mismatch_01:27:9: Expected an identifier, but got ']'.
]),
^
Expand Down Expand Up @@ -186,19 +174,16 @@ beginCompilationUnit(class)
endArguments(0, (, ))
handleSend(D, ,)
handleNamedArgument(:)
handleRecoverableError(PositionalAfterNamedArgument, D, D)
handleIdentifier(D, expression)
handleNoTypeArguments(()
beginArguments(()
endArguments(0, (, ))
handleSend(D, ,)
handleRecoverableError(PositionalAfterNamedArgument, D, D)
handleIdentifier(D, expression)
handleNoTypeArguments(()
beginArguments(()
endArguments(0, (, ))
handleSend(D, ,)
handleRecoverableError(PositionalAfterNamedArgument, ], ])
handleRecoverableError(Message[ExpectedIdentifier, Expected an identifier, but got ']'., Try inserting an identifier before ']'., {lexeme: ]}], ], ])
handleIdentifier(, expression)
handleNoTypeArguments(])
Expand Down
6 changes: 0 additions & 6 deletions pkg/front_end/parser_testcases/error_recovery/bracket_mismatch_01.dart.intertwined.expect
View file
Original file line number Diff line number Diff line change
Expand Up @@ -414,8 +414,6 @@ parseUnit(class)
listener: endArguments(0, (, ))
listener: handleSend(D, ,)
listener: handleNamedArgument(:)
reportRecoverableError(D, PositionalAfterNamedArgument)
listener: handleRecoverableError(PositionalAfterNamedArgument, D, D)
parseExpression(,)
parsePrecedenceExpression(,, 1, true)
parseUnaryExpression(,, true)
Expand All @@ -433,8 +431,6 @@ parseUnit(class)
listener: beginArguments(()
listener: endArguments(0, (, ))
listener: handleSend(D, ,)
reportRecoverableError(D, PositionalAfterNamedArgument)
listener: handleRecoverableError(PositionalAfterNamedArgument, D, D)
parseExpression(,)
parsePrecedenceExpression(,, 1, true)
parseUnaryExpression(,, true)
Expand All @@ -452,8 +448,6 @@ parseUnit(class)
listener: beginArguments(()
listener: endArguments(0, (, ))
listener: handleSend(D, ,)
reportRecoverableError(], PositionalAfterNamedArgument)
listener: handleRecoverableError(PositionalAfterNamedArgument, ], ])
parseExpression(,)
parsePrecedenceExpression(,, 1, true)
parseUnaryExpression(,, true)
Expand Down
173 changes: 0 additions & 173 deletions ...d/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart.strong.expect
View file
Original file line number Diff line number Diff line change
@@ -1,177 +1,4 @@
library /*isNonNullableByDefault*/;
//
// Problems in library:
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:17:21: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// method2(foo: 1, 2); // This call.
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:27:16: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// foo(1, z: 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:28:13: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// foo(z: 1, 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:28:16: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// foo(z: 1, 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:32:22: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// new A.foo(1, z: 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:33:19: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// new A.foo(z: 1, 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:33:22: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// new A.foo(z: 1, 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:35:22: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// new B.foo(1, z: 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:36:19: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// new B.foo(z: 1, 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:36:22: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// new B.foo(z: 1, 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:40:18: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// new A(1, z: 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:41:15: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// new A(z: 1, 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:41:18: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// new A(z: 1, 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:43:18: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// new B(1, z: 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:44:15: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// new B(z: 1, 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:44:18: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// new B(z: 1, 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:48:14: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// d(1, z: 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:49:11: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// d(z: 1, 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:49:14: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// d(z: 1, 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:53:14: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// f(1, z: 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:54:11: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// f(z: 1, 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:54:14: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// f(z: 1, 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:58:23: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// a.property(1, z: 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:59:20: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// a.property(z: 1, 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:59:23: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// a.property(z: 1, 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:63:18: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// a.bar(1, z: 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:64:15: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// a.bar(z: 1, 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:64:18: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// a.bar(z: 1, 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:68:18: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// local(1, z: 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:69:15: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// local(z: 1, 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:69:18: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// local(z: 1, 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:77:24: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// super.bar(1, z: 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:78:21: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// super.bar(z: 1, 2, 3);
// ^
//
// pkg/front_end/testcases/unscheduled_experiments/named_arguments_anywhere/all_kinds.dart:78:24: Error: Place positional arguments before named arguments.
// Try moving the positional argument before the named arguments, or add a name to the argument.
// super.bar(z: 1, 2, 3);
// ^
//
import self as self;
import "dart:core" as core;

Expand Down
Loading

0 comments on commit a666b5a

Please sign in to comment.