Simplify analyzer logic for `.call` tearoffs.
Previously, the job of determining whether a `.call` tearoff was
necessary for an expression fell to the expression that contained it.
This made things complicated for set, map, and list literals, because
it required the literal to be visited twice (once to resolve the
subexpressions, and another time to insert `.call` tearoffs). With
this change, insertion of a `.call` tearoff is handled by the visitor
for the expression itself.
(Note that there are a few cases where we don't insert a `.call`
tearoff, or we do so based on a special type, based on the parent
expression. The language team is considering removing these special
cases, but for now, we maintain the existing behavior. We handle
these special cases by looking at `expression.parent`).
Change-Id: I85c813f8c105e3b962dd89d5a7fa8467506078ba
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/232660
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
diff --git a/pkg/analyzer/lib/src/dart/resolver/assignment_expression_resolver.dart b/pkg/analyzer/lib/src/dart/resolver/assignment_expression_resolver.dart
index 16e5f20..dfa4a1e 100644
--- a/pkg/analyzer/lib/src/dart/resolver/assignment_expression_resolver.dart
+++ b/pkg/analyzer/lib/src/dart/resolver/assignment_expression_resolver.dart
@@ -225,16 +225,13 @@
void _resolveTypes(AssignmentExpressionImpl node,
{required Map<DartType, NonPromotionReason> Function()? whyNotPromoted}) {
DartType assignedType;
- DartType? implicitCallContext;
var rightHandSide = node.rightHandSide;
var operator = node.operator.type;
if (operator == TokenType.EQ) {
assignedType = rightHandSide.typeOrThrow;
- implicitCallContext = node.writeType;
} else if (operator == TokenType.QUESTION_QUESTION_EQ) {
assignedType = rightHandSide.typeOrThrow;
- implicitCallContext = node.writeType;
} else if (operator == TokenType.AMPERSAND_AMPERSAND_EQ ||
operator == TokenType.BAR_BAR_EQ) {
assignedType = _typeProvider.boolType;
@@ -255,11 +252,6 @@
}
}
- var callInsertion = _resolver.insertImplicitCallReference(rightHandSide,
- context: implicitCallContext);
- if (callInsertion != null) {
- assignedType = callInsertion.staticType;
- }
DartType nodeType;
if (operator == TokenType.QUESTION_QUESTION_EQ) {
var leftType = node.readType!;
diff --git a/pkg/analyzer/lib/src/dart/resolver/typed_literal_resolver.dart b/pkg/analyzer/lib/src/dart/resolver/typed_literal_resolver.dart
index 5c56ff1..dcde026 100644
--- a/pkg/analyzer/lib/src/dart/resolver/typed_literal_resolver.dart
+++ b/pkg/analyzer/lib/src/dart/resolver/typed_literal_resolver.dart
@@ -119,7 +119,6 @@
node.typeArguments?.accept(_resolver);
_resolveElements(node.elements, context);
- _insertImplicitCallReferences(node);
_resolveListLiteral2(node);
}
@@ -179,7 +178,6 @@
node.typeArguments?.accept(_resolver);
_resolveElements(node.elements, context);
- _insertImplicitCallReferences(node);
_resolveSetOrMapLiteral2(node);
}
@@ -623,34 +621,6 @@
);
}
- void _insertImplicitCallReference(CollectionElement? node) {
- if (node is Expression) {
- _resolver.insertImplicitCallReference(node);
- } else if (node is MapLiteralEntry) {
- _insertImplicitCallReference(node.key);
- _insertImplicitCallReference(node.value);
- } else if (node is IfElement) {
- _insertImplicitCallReference(node.thenElement);
- _insertImplicitCallReference(node.elseElement);
- } else if (node is ForElement) {
- _insertImplicitCallReference(node.body);
- }
- // Nothing to do for [SpreadElement] as analyzer does not desugar this
- // element.
- }
-
- void _insertImplicitCallReferences(TypedLiteral node) {
- if (node is ListLiteral) {
- for (var element in node.elements) {
- _insertImplicitCallReference(element);
- }
- } else if (node is SetOrMapLiteral) {
- for (var element in node.elements) {
- _insertImplicitCallReference(element);
- }
- }
- }
-
void _resolveElements(
List<CollectionElement> elements, CollectionLiteralContext? context) {
for (var element in elements) {
diff --git a/pkg/analyzer/lib/src/dart/resolver/variable_declaration_resolver.dart b/pkg/analyzer/lib/src/dart/resolver/variable_declaration_resolver.dart
index 6ae7fe4..17d7f04 100644
--- a/pkg/analyzer/lib/src/dart/resolver/variable_declaration_resolver.dart
+++ b/pkg/analyzer/lib/src/dart/resolver/variable_declaration_resolver.dart
@@ -64,11 +64,6 @@
_resolver.flowAnalysis.flow?.lateInitializer_end();
}
- var callInsertion = _resolver.insertImplicitCallReference(initializer);
- if (callInsertion != null) {
- initializer = callInsertion.expression;
- }
-
// Initializers of top-level variables and fields are already included
// into elements during linking.
if (element is ConstLocalVariableElementImpl) {
diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart
index 4e32f72..97dc545 100644
--- a/pkg/analyzer/lib/src/generated/resolver.dart
+++ b/pkg/analyzer/lib/src/generated/resolver.dart
@@ -8,6 +8,7 @@
import 'package:analyzer/dart/analysis/features.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/syntactic_entity.dart';
+import 'package:analyzer/dart/ast/token.dart';
import 'package:analyzer/dart/ast/visitor.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/nullability_suffix.dart';
@@ -78,18 +79,6 @@
/// promoted.
typedef WhyNotPromotedGetter = Map<DartType, NonPromotionReason> Function();
-/// Data structure describing the result of inserting an implicit call reference
-/// into the AST.
-class ImplicitCallInsertionResult {
- /// The expression that was inserted.
- final ImplicitCallReferenceImpl expression;
-
- /// The type of the implicit call tear-off.
- final FunctionType staticType;
-
- ImplicitCallInsertionResult(this.expression, this.staticType);
-}
-
/// Maintains and manages contextual type information used for
/// inferring types.
class InferenceContext {
@@ -541,13 +530,6 @@
/// See [CompileTimeErrorCode.ARGUMENT_TYPE_NOT_ASSIGNABLE].
void checkForArgumentTypesNotAssignableInList(ArgumentList argumentList,
List<WhyNotPromotedGetter> whyNotPromotedList) {
- for (var argument in argumentList.arguments) {
- if (argument is NamedExpression) {
- insertImplicitCallReference(argument.expression);
- } else {
- insertImplicitCallReference(argument);
- }
- }
var arguments = argumentList.arguments;
for (int i = 0; i < arguments.length; i++) {
checkForArgumentTypeNotAssignableForArgument(arguments[i],
@@ -809,66 +791,6 @@
return genericFunctionInstantiation;
}
- /// If `expression` should be treated as `expression.call`, inserts an
- /// [ImplicitCallReference] node which wraps [expression].
- ///
- /// If an [ImplicitCallReference] is inserted, returns an
- /// [ImplicitCallInsertionResult] describing what was changed; otherwise,
- /// returns `null`.
- ImplicitCallInsertionResult? insertImplicitCallReference(
- Expression expression,
- {DartType? context}) {
- expression as ExpressionImpl;
- var parent = expression.parent;
- if (parent is CascadeExpression && parent.target == expression) {
- // Do not perform an "implicit tear-off conversion" here. It should only
- // be performed on [parent]. See
- // https://github.com/dart-lang/language/issues/1873.
- return null;
- }
- context ??= InferenceContext.getContext(expression);
- var callMethod =
- getImplicitCallMethod(expression.typeOrThrow, context, expression);
- if (callMethod == null || context == null) {
- return null;
- }
-
- // `expression` is to be treated as `expression.call`.
- context = typeSystem.flatten(context);
- var callMethodType = callMethod.type;
- List<DartType> typeArgumentTypes;
- if (isConstructorTearoffsEnabled &&
- callMethodType.typeFormals.isNotEmpty &&
- context is FunctionType) {
- typeArgumentTypes = typeSystem.inferFunctionTypeInstantiation(
- context,
- callMethodType,
- errorReporter: errorReporter,
- errorNode: expression,
- // If the constructor-tearoffs feature is enabled, then so is
- // generic-metadata.
- genericMetadataIsEnabled: true,
- )!;
- if (typeArgumentTypes.isNotEmpty) {
- callMethodType = callMethodType.instantiate(typeArgumentTypes);
- }
- } else {
- typeArgumentTypes = [];
- }
-
- var callReference = astFactory.implicitCallReference(
- expression: expression,
- staticElement: callMethod,
- typeArguments: null,
- typeArgumentTypes: typeArgumentTypes,
- );
- NodeReplacer.replace(expression, callReference, parent: parent);
-
- callReference.staticType = callMethodType;
-
- return ImplicitCallInsertionResult(callReference, callMethodType);
- }
-
/// If we reached a null-shorting termination, and the [node] has null
/// shorting, make the type of the [node] nullable.
void nullShortingTermination(ExpressionImpl node,
@@ -1176,7 +1098,7 @@
node.visitChildren(this);
typeAnalyzer.visitAsExpression(node as AsExpressionImpl);
flowAnalysis.asExpression(node);
- insertGenericFunctionInstantiation(node);
+ _insertImplicitCallReference(insertGenericFunctionInstantiation(node));
}
@override
@@ -1210,7 +1132,7 @@
@override
void visitAssignmentExpression(AssignmentExpression node) {
_assignmentExpressionResolver.resolve(node as AssignmentExpressionImpl);
- insertGenericFunctionInstantiation(node);
+ _insertImplicitCallReference(insertGenericFunctionInstantiation(node));
}
@override
@@ -1223,7 +1145,7 @@
checkUnreachableNode(node);
analyzeExpression(node.expression, futureUnion);
typeAnalyzer.visitAwaitExpression(node as AwaitExpressionImpl);
- insertGenericFunctionInstantiation(node);
+ _insertImplicitCallReference(insertGenericFunctionInstantiation(node));
}
@override
@@ -1234,7 +1156,7 @@
node, InferenceContext.getContext(node));
}
_binaryExpressionResolver.resolve(node as BinaryExpressionImpl);
- insertGenericFunctionInstantiation(node);
+ _insertImplicitCallReference(insertGenericFunctionInstantiation(node));
}
@override
@@ -1291,6 +1213,7 @@
typeAnalyzer.visitCascadeExpression(node);
nullShortingTermination(node);
+ _insertImplicitCallReference(node);
}
@override
@@ -1386,6 +1309,7 @@
elseExpression = node.elseExpression;
typeAnalyzer.visitConditionalExpression(node as ConditionalExpressionImpl);
+ _insertImplicitCallReference(node);
}
@override
@@ -1447,13 +1371,6 @@
elementResolver.visitConstructorFieldInitializer(
node as ConstructorFieldInitializerImpl);
if (fieldElement != null) {
- if (fieldType != null && expression.staticType != null) {
- var callReference = insertImplicitCallReference(expression)?.expression;
- if (callReference != null) {
- checkForInvalidAssignment(node.fieldName, callReference,
- whyNotPromoted: whyNotPromoted);
- }
- }
var enclosingConstructor = enclosingFunction as ConstructorElement;
checkForFieldInitializerNotAssignable(node, fieldElement,
isConstConstructor: enclosingConstructor.isConst,
@@ -1470,6 +1387,7 @@
@override
void visitConstructorReference(covariant ConstructorReferenceImpl node) {
_constructorReferenceResolver.resolve(node);
+ _insertImplicitCallReference(node);
}
@override
@@ -1674,7 +1592,6 @@
node.expression,
inferenceContext.bodyContext!.contextType,
);
- insertImplicitCallReference(node.expression);
flowAnalysis.flow?.handleExit();
@@ -1845,9 +1762,10 @@
_functionExpressionInvocationResolver.resolve(
node as FunctionExpressionInvocationImpl, whyNotPromotedList);
nullShortingTermination(node);
- insertGenericFunctionInstantiation(node);
+ var replacement = insertGenericFunctionInstantiation(node);
checkForArgumentTypesNotAssignableInList(
node.argumentList, whyNotPromotedList);
+ _insertImplicitCallReference(replacement);
}
@override
@@ -1989,15 +1907,17 @@
type = DynamicTypeImpl.instance;
}
inferenceHelper.recordStaticType(node, type);
- insertGenericFunctionInstantiation(node);
+ var replacement = insertGenericFunctionInstantiation(node);
nullShortingTermination(node);
+ _insertImplicitCallReference(replacement);
}
@override
void visitInstanceCreationExpression(
covariant InstanceCreationExpressionImpl node) {
_instanceCreationExpressionResolver.resolve(node);
+ _insertImplicitCallReference(node);
}
@override
@@ -2132,9 +2052,10 @@
} else {
nullShortingTermination(node);
}
- insertGenericFunctionInstantiation(node);
+ var replacement = insertGenericFunctionInstantiation(node);
checkForArgumentTypesNotAssignableInList(
node.argumentList, whyNotPromotedList);
+ _insertImplicitCallReference(replacement);
}
@override
@@ -2229,19 +2150,19 @@
@override
void visitPostfixExpression(PostfixExpression node) {
_postfixExpressionResolver.resolve(node as PostfixExpressionImpl);
- insertGenericFunctionInstantiation(node);
+ _insertImplicitCallReference(insertGenericFunctionInstantiation(node));
}
@override
void visitPrefixedIdentifier(covariant PrefixedIdentifierImpl node) {
_prefixedIdentifierResolver.resolve(node);
- insertGenericFunctionInstantiation(node);
+ _insertImplicitCallReference(insertGenericFunctionInstantiation(node));
}
@override
void visitPrefixExpression(PrefixExpression node) {
_prefixExpressionResolver.resolve(node as PrefixExpressionImpl);
- insertGenericFunctionInstantiation(node);
+ _insertImplicitCallReference(insertGenericFunctionInstantiation(node));
}
@override
@@ -2283,9 +2204,10 @@
inferenceHelper.recordStaticType(propertyName, type);
inferenceHelper.recordStaticType(node, type);
- insertGenericFunctionInstantiation(node);
+ var replacement = insertGenericFunctionInstantiation(node);
nullShortingTermination(node);
+ _insertImplicitCallReference(replacement);
}
@override
@@ -2328,10 +2250,6 @@
inferenceContext.bodyContext?.addReturnExpression(expression);
flowAnalysis.flow?.handleExit();
-
- if (expression != null) {
- insertImplicitCallReference(expression);
- }
}
@override
@@ -2353,7 +2271,7 @@
@override
void visitSimpleIdentifier(covariant SimpleIdentifierImpl node) {
_simpleIdentifierResolver.resolve(node);
- insertGenericFunctionInstantiation(node);
+ _insertImplicitCallReference(insertGenericFunctionInstantiation(node));
}
@override
@@ -2494,6 +2412,7 @@
checkUnreachableNode(node);
node.visitChildren(this);
typeAnalyzer.visitThisExpression(node as ThisExpressionImpl);
+ _insertImplicitCallReference(node);
}
@override
@@ -2658,6 +2577,62 @@
return typeProvider.futureOrType(type);
}
+ /// If `expression` should be treated as `expression.call`, inserts an
+ /// [ImplicitCallReference] node which wraps [expression].
+ void _insertImplicitCallReference(ExpressionImpl expression) {
+ var parent = expression.parent;
+ if (_shouldSkipImplicitCallReferenceDueToForm(expression, parent)) {
+ return;
+ }
+ var staticType = expression.staticType;
+ if (staticType == null) {
+ return;
+ }
+ DartType? context;
+ if (parent is AssignmentExpression) {
+ context = parent.writeType;
+ } else {
+ context = InferenceContext.getContext(expression);
+ }
+ var callMethod = getImplicitCallMethod(staticType, context, expression);
+ if (callMethod == null || context == null) {
+ return;
+ }
+
+ // `expression` is to be treated as `expression.call`.
+ context = typeSystem.flatten(context);
+ var callMethodType = callMethod.type;
+ List<DartType> typeArgumentTypes;
+ if (isConstructorTearoffsEnabled &&
+ callMethodType.typeFormals.isNotEmpty &&
+ context is FunctionType) {
+ typeArgumentTypes = typeSystem.inferFunctionTypeInstantiation(
+ context,
+ callMethodType,
+ errorReporter: errorReporter,
+ errorNode: expression,
+ // If the constructor-tearoffs feature is enabled, then so is
+ // generic-metadata.
+ genericMetadataIsEnabled: true,
+ )!;
+ if (typeArgumentTypes.isNotEmpty) {
+ callMethodType = callMethodType.instantiate(typeArgumentTypes);
+ }
+ } else {
+ typeArgumentTypes = [];
+ }
+
+ var callReference = astFactory.implicitCallReference(
+ expression: expression,
+ staticElement: callMethod,
+ typeArguments: null,
+ typeArgumentTypes: typeArgumentTypes,
+ );
+ NodeReplacer.replace(expression, callReference, parent: parent);
+
+ callReference.staticType = callMethodType;
+ }
+
/// Continues resolution of a [FunctionExpressionInvocation] that was created
/// from a rewritten [MethodInvocation]. The target function is already
/// resolved.
@@ -2702,6 +2677,34 @@
}
}
+ bool _shouldSkipImplicitCallReferenceDueToForm(
+ Expression expression, AstNode? parent) {
+ while (parent is ParenthesizedExpression) {
+ expression = parent;
+ parent = expression.parent;
+ }
+ if (parent is CascadeExpression && parent.target == expression) {
+ // Do not perform an "implicit tear-off conversion" here. It should only
+ // be performed on [parent]. See
+ // https://github.com/dart-lang/language/issues/1873.
+ return true;
+ }
+ if (parent is ConditionalExpression &&
+ (parent.thenExpression == expression ||
+ parent.elseExpression == expression)) {
+ // Do not perform an "implicit tear-off conversion" on the branches of a
+ // conditional expression.
+ return true;
+ }
+ if (parent is BinaryExpression &&
+ parent.operator.type == TokenType.QUESTION_QUESTION) {
+ // Do not perform an "implicit tear-off conversion" on the branches of a
+ // `??` operator.
+ return true;
+ }
+ return false;
+ }
+
/// Given an [argumentList] and the [parameters] related to the element that
/// will be invoked using those arguments, compute the list of parameters that
/// correspond to the list of arguments.
diff --git a/pkg/analyzer/test/src/dart/resolution/ast_rewrite_test.dart b/pkg/analyzer/test/src/dart/resolution/ast_rewrite_test.dart
index 50d0ec5..77954c0 100644
--- a/pkg/analyzer/test/src/dart/resolution/ast_rewrite_test.dart
+++ b/pkg/analyzer/test/src/dart/resolution/ast_rewrite_test.dart
@@ -47,6 +47,38 @@
);
}
+ test_conditional_else() async {
+ await assertNoErrorsInCode('''
+abstract class A {}
+abstract class C extends A {
+ void call();
+}
+void Function() f(A a, bool b, C c, dynamic d) => b ? d : (b ? a : c);
+''');
+ // `c` is in the "else" position of a conditional expression, so implicit
+ // call tearoff logic should not apply to it.
+ var expr = findNode.conditionalExpression('b ? a : c');
+ expect(expr.thenExpression, TypeMatcher<SimpleIdentifier>());
+ // Therefore the type of `b ? a : c` should be `A`.
+ assertType(expr, 'A');
+ }
+
+ test_conditional_then() async {
+ await assertNoErrorsInCode('''
+abstract class A {}
+abstract class C extends A {
+ void call();
+}
+void Function() f(A a, bool b, C c, dynamic d) => b ? d : (b ? c : a);
+''');
+ // `c` is in the "then" position of a conditional expression, so implicit
+ // call tearoff logic should not apply to it.
+ var expr = findNode.conditionalExpression('b ? c : a');
+ expect(expr.thenExpression, TypeMatcher<SimpleIdentifier>());
+ // Therefore the type of `b ? c : a` should be `A`.
+ assertType(expr, 'A');
+ }
+
test_explicitTypeArguments() async {
await assertNoErrorsInCode('''
class C {
@@ -66,7 +98,26 @@
);
}
- test_ifNull() async {
+ test_ifNull_lhs() async {
+ await assertErrorsInCode('''
+abstract class A {}
+abstract class C extends A {
+ void call();
+}
+
+void Function() f(A a, bool b, C c, dynamic d) => b ? d : c ?? a;
+''', [
+ error(StaticWarningCode.DEAD_NULL_AWARE_EXPRESSION, 130, 1),
+ ]);
+ // `c` is on the LHS of an if-null expression, so implicit call tearoff
+ // logic should not apply to it.
+ var expr = findNode.binary('c ?? a');
+ expect(expr.leftOperand, TypeMatcher<SimpleIdentifier>());
+ // Therefore the type of `c ?? a` should be `A`.
+ assertType(expr, 'A');
+ }
+
+ test_ifNull_rhs() async {
await assertNoErrorsInCode('''
abstract class C {
void call(int t) => t;
@@ -163,6 +214,16 @@
);
}
+ test_parenthesized_cascade_target() async {
+ await assertNoErrorsInCode('''
+abstract class C {
+ void call();
+ void m();
+}
+void Function() f(C c) => (c)..m();
+''');
+ }
+
test_prefixedIdentifier() async {
await assertNoErrorsInCode('''
abstract class C {
