Stop creating synthetic identifiers to use as error nodes.

By changing the type of error nodes to SyntacticEntity, we can use Token objects directly as error nodes, rather than having to wrap them in synthetic identifiers. Change-Id: I29c2fec7007766b8bc6103da835093ea326e939b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366420 Commit-Queue: Paul Berry <paulberry@google.com> Reviewed-by: Konstantin Shcheglov <scheglov@google.com> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
dart-lang · May 14, 2024 · 261e118 · 261e118
1 parent 7c5437b
commit 261e118
Show file tree

Hide file tree

Showing 8 changed files with 83 additions and 58 deletions.
diff --git a/pkg/analyzer/lib/error/listener.dart b/pkg/analyzer/lib/error/listener.dart
@@ -4,6 +4,7 @@
 
 import 'package:analyzer/dart/ast/ast.dart'
     show AstNode, ConstructorDeclaration;
+import 'package:analyzer/dart/ast/syntactic_entity.dart';
 import 'package:analyzer/dart/ast/token.dart';
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/type.dart';
@@ -85,6 +86,25 @@ class ErrorReporter {
     );
   }
 
+  /// Report an error with the given [errorCode] and [arguments].
+  /// The [entity] is used to compute the location of the error.
+  void atEntity({
+    required SyntacticEntity entity,
+    required ErrorCode errorCode,
+    List<Object>? arguments,
+    List<DiagnosticMessage>? contextMessages,
+    Object? data,
+  }) {
+    atOffset(
+      errorCode: errorCode,
+      offset: entity.offset,
+      length: entity.length,
+      arguments: arguments,
+      contextMessages: contextMessages,
+      data: data,
+    );
+  }
+
   /// Report an error with the given [errorCode] and [arguments].
   /// The [node] is used to compute the location of the error.
   void atNode(

diff --git a/pkg/analyzer/lib/src/dart/element/generic_inferrer.dart b/pkg/analyzer/lib/src/dart/element/generic_inferrer.dart
@@ -14,6 +14,7 @@ import 'package:analyzer/dart/ast/ast.dart'
         Expression,
         InvocationExpression,
         SimpleIdentifier;
+import 'package:analyzer/dart/ast/syntactic_entity.dart';
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/type.dart';
 import 'package:analyzer/error/listener.dart' show ErrorReporter;
@@ -74,9 +75,10 @@ class GenericInferrer {
   /// `null` if errors shouldn't be reported.
   final ErrorReporter? errorReporter;
 
-  /// The [AstNode] to which errors should be attached.  May be `null` if errors
-  /// are not being reported (that is, if [errorReporter] is also `null`).
-  final AstNode? errorNode;
+  /// The [SyntacticEntity] to which errors should be attached.  May be `null`
+  /// if errors are not being reported (that is, if [errorReporter] is also
+  /// `null`).
+  final SyntacticEntity? errorEntity;
 
   /// Indicates whether the "generic metadata" feature is enabled.  When it is,
   /// type arguments are allowed to be instantiated with generic function types.
@@ -114,15 +116,15 @@ class GenericInferrer {
 
   GenericInferrer(this._typeSystem, this._typeFormals,
       {this.errorReporter,
-      this.errorNode,
+      this.errorEntity,
       required this.genericMetadataIsEnabled,
       required bool strictInference,
       required TypeSystemOperations typeSystemOperations,
       required this.dataForTesting})
       : _strictInference = strictInference,
         _typeSystemOperations = typeSystemOperations {
     if (errorReporter != null) {
-      assert(errorNode != null);
+      assert(errorEntity != null);
     }
     _typeParameters.addAll(_typeFormals);
     for (var formal in _typeFormals) {
@@ -266,9 +268,9 @@ class GenericInferrer {
       if (!success) {
         if (failAtError) return null;
         hasErrorReported = true;
-        errorReporter?.atNode(
-          errorNode!,
-          CompileTimeErrorCode.COULD_NOT_INFER,
+        errorReporter?.atEntity(
+          entity: errorEntity!,
+          errorCode: CompileTimeErrorCode.COULD_NOT_INFER,
           arguments: [
             parameter.name,
             _formatError(parameter, inferred, constraints)
@@ -289,9 +291,9 @@ class GenericInferrer {
         hasErrorReported = true;
         var typeFormals = inferred.typeFormals;
         var typeFormalsStr = typeFormals.map(_elementStr).join(', ');
-        errorReporter!.atNode(
-          errorNode!,
-          CompileTimeErrorCode.COULD_NOT_INFER,
+        errorReporter!.atEntity(
+          entity: errorEntity!,
+          errorCode: CompileTimeErrorCode.COULD_NOT_INFER,
           arguments: [
             parameter.name,
             ' Inferred candidate type ${_typeStr(inferred)} has type parameters'
@@ -310,7 +312,7 @@ class GenericInferrer {
         // mode.
         _reportInferenceFailure(
           errorReporter: errorReporter,
-          errorNode: errorNode,
+          errorEntity: errorEntity,
           genericMetadataIsEnabled: genericMetadataIsEnabled,
         );
       }
@@ -330,9 +332,9 @@ class GenericInferrer {
         var typeParamBound = Substitution.fromPairs(_typeFormals, inferredTypes)
             .substituteType(typeParam.bound ?? typeProvider.objectType);
         // TODO(jmesserly): improve this error message.
-        errorReporter?.atNode(
-          errorNode!,
-          CompileTimeErrorCode.COULD_NOT_INFER,
+        errorReporter?.atEntity(
+          entity: errorEntity!,
+          errorCode: CompileTimeErrorCode.COULD_NOT_INFER,
           arguments: [
             typeParam.name,
             "\nRecursive bound cannot be instantiated: '$typeParamBound'."
@@ -345,7 +347,7 @@ class GenericInferrer {
 
     if (!hasErrorReported) {
       _checkArgumentsNotMatchingBounds(
-        errorNode: errorNode,
+        errorEntity: errorEntity,
         errorReporter: errorReporter,
         typeArguments: result,
       );
@@ -357,7 +359,7 @@ class GenericInferrer {
 
   /// Check that inferred [typeArguments] satisfy the [typeParameters] bounds.
   void _checkArgumentsNotMatchingBounds({
-    required AstNode? errorNode,
+    required SyntacticEntity? errorEntity,
     required ErrorReporter? errorReporter,
     required List<DartType> typeArguments,
   }) {
@@ -373,9 +375,9 @@ class GenericInferrer {
       var substitution = Substitution.fromPairs(_typeFormals, typeArguments);
       var bound = substitution.substituteType(rawBound);
       if (!_typeSystem.isSubtypeOf(argument, bound)) {
-        errorReporter?.atNode(
-          errorNode!,
-          CompileTimeErrorCode.COULD_NOT_INFER,
+        errorReporter?.atEntity(
+          entity: errorEntity!,
+          errorCode: CompileTimeErrorCode.COULD_NOT_INFER,
           arguments: [
             parameter.name,
             "\n'${_typeStr(argument)}' doesn't conform to "
@@ -639,49 +641,50 @@ class GenericInferrer {
     return t;
   }
 
-  /// Reports an inference failure on [errorNode] according to its type.
+  /// Reports an inference failure on [errorEntity] according to its type.
   void _reportInferenceFailure({
     ErrorReporter? errorReporter,
-    AstNode? errorNode,
+    SyntacticEntity? errorEntity,
     required bool genericMetadataIsEnabled,
   }) {
-    if (errorReporter == null || errorNode == null) {
+    if (errorReporter == null || errorEntity == null) {
       return;
     }
-    if (errorNode.parent is InvocationExpression &&
-        errorNode.parent?.parent is AsExpression) {
+    if (errorEntity is AstNode &&
+        errorEntity.parent is InvocationExpression &&
+        errorEntity.parent?.parent is AsExpression) {
       // Casts via `as` do not play a part in downward inference. We allow an
       // exception when inference has "failed" but the return value is
       // immediately cast with `as`.
       return;
     }
-    if (errorNode is ConstructorName &&
-        !(errorNode.type.type as InterfaceType).element.hasOptionalTypeArgs) {
-      String constructorName = errorNode.name == null
-          ? errorNode.type.qualifiedName
-          : '${errorNode.type}.${errorNode.name}';
+    if (errorEntity is ConstructorName &&
+        !(errorEntity.type.type as InterfaceType).element.hasOptionalTypeArgs) {
+      String constructorName = errorEntity.name == null
+          ? errorEntity.type.qualifiedName
+          : '${errorEntity.type}.${errorEntity.name}';
       errorReporter.atNode(
-        errorNode,
+        errorEntity,
         WarningCode.INFERENCE_FAILURE_ON_INSTANCE_CREATION,
         arguments: [constructorName],
       );
-    } else if (errorNode is Annotation) {
+    } else if (errorEntity is Annotation) {
       if (genericMetadataIsEnabled) {
         // Only report an error if generic metadata is valid syntax.
-        var element = errorNode.name.staticElement;
+        var element = errorEntity.name.staticElement;
         if (element != null && !element.hasOptionalTypeArgs) {
-          String constructorName = errorNode.constructorName == null
-              ? errorNode.name.name
-              : '${errorNode.name.name}.${errorNode.constructorName}';
+          String constructorName = errorEntity.constructorName == null
+              ? errorEntity.name.name
+              : '${errorEntity.name.name}.${errorEntity.constructorName}';
           errorReporter.atNode(
-            errorNode,
+            errorEntity,
             WarningCode.INFERENCE_FAILURE_ON_INSTANCE_CREATION,
             arguments: [constructorName],
           );
         }
       }
-    } else if (errorNode is SimpleIdentifier) {
-      var element = errorNode.staticElement;
+    } else if (errorEntity is SimpleIdentifier) {
+      var element = errorEntity.staticElement;
       if (element != null) {
         if (element is VariableElement) {
           // For variable elements, we check their type and possible alias type.
@@ -698,19 +701,19 @@ class GenericInferrer {
         }
         if (!element.hasOptionalTypeArgs) {
           errorReporter.atNode(
-            errorNode,
+            errorEntity,
             WarningCode.INFERENCE_FAILURE_ON_FUNCTION_INVOCATION,
-            arguments: [errorNode.name],
+            arguments: [errorEntity.name],
           );
           return;
         }
       }
-    } else if (errorNode is Expression) {
-      var type = errorNode.staticType;
+    } else if (errorEntity is Expression) {
+      var type = errorEntity.staticType;
       if (type != null) {
         var typeDisplayString = _typeStr(type);
         errorReporter.atNode(
-          errorNode,
+          errorEntity,
           WarningCode.INFERENCE_FAILURE_ON_GENERIC_INVOCATION,
           arguments: [typeDisplayString],
         );

diff --git a/pkg/analyzer/lib/src/dart/element/type_system.dart b/pkg/analyzer/lib/src/dart/element/type_system.dart
@@ -5,6 +5,7 @@
 import 'dart:collection';
 
 import 'package:analyzer/dart/ast/ast.dart' show AstNode;
+import 'package:analyzer/dart/ast/syntactic_entity.dart';
 import 'package:analyzer/dart/ast/token.dart' show TokenType;
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/type.dart';
@@ -652,7 +653,7 @@ class TypeSystemImpl implements TypeSystem {
     // are implied by this.
     var inferrer = GenericInferrer(this, fnType.typeFormals,
         errorReporter: errorReporter,
-        errorNode: errorNode,
+        errorEntity: errorNode,
         genericMetadataIsEnabled: genericMetadataIsEnabled,
         strictInference: strictInference,
         typeSystemOperations: typeSystemOperations,
@@ -1697,7 +1698,7 @@ class TypeSystemImpl implements TypeSystem {
     required DartType declaredReturnType,
     required DartType contextReturnType,
     ErrorReporter? errorReporter,
-    AstNode? errorNode,
+    SyntacticEntity? errorEntity,
     required bool genericMetadataIsEnabled,
     bool isConst = false,
     required bool strictInference,
@@ -1712,7 +1713,7 @@ class TypeSystemImpl implements TypeSystem {
     // are implied by this.
     var inferrer = GenericInferrer(this, typeParameters,
         errorReporter: errorReporter,
-        errorNode: errorNode,
+        errorEntity: errorEntity,
         genericMetadataIsEnabled: genericMetadataIsEnabled,
         strictInference: strictInference,
         typeSystemOperations: typeSystemOperations,

diff --git a/pkg/analyzer/lib/src/dart/resolver/extension_member_resolver.dart b/pkg/analyzer/lib/src/dart/resolver/extension_member_resolver.dart
@@ -344,7 +344,7 @@ class ExtensionMemberResolver {
         _typeSystem,
         typeParameters,
         errorReporter: _errorReporter,
-        errorNode: SimpleIdentifierImpl(node.name),
+        errorEntity: node.name,
         genericMetadataIsEnabled: _genericMetadataIsEnabled,
         strictInference: _resolver.analysisOptions.strictInference,
         typeSystemOperations: _resolver.flowAnalysis.typeOperations,

diff --git a/pkg/analyzer/lib/src/dart/resolver/invocation_inferrer.dart b/pkg/analyzer/lib/src/dart/resolver/invocation_inferrer.dart
@@ -5,6 +5,7 @@
 import 'package:_fe_analyzer_shared/src/base/errors.dart';
 import 'package:_fe_analyzer_shared/src/deferred_function_literal_heuristic.dart';
 import 'package:_fe_analyzer_shared/src/flow_analysis/flow_analysis.dart';
+import 'package:analyzer/dart/ast/syntactic_entity.dart';
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/type.dart';
 import 'package:analyzer/src/dart/ast/ast.dart';
@@ -111,8 +112,8 @@ class AugmentedInvocationInferrer
   }) : super._();
 
   @override
-  AstNode get _errorNode {
-    return SimpleIdentifierImpl(node.augmentedKeyword);
+  SyntacticEntity get _errorEntity {
+    return node.augmentedKeyword;
   }
 
   @override
@@ -137,7 +138,7 @@ abstract class FullInvocationInferrer<Node extends AstNodeImpl>
       required super.contextType,
       required super.whyNotPromotedList});
 
-  AstNode get _errorNode => node;
+  SyntacticEntity get _errorEntity => node;
 
   bool get _isConst => false;
 
@@ -223,7 +224,7 @@ abstract class FullInvocationInferrer<Node extends AstNodeImpl>
         contextReturnType: contextType,
         isConst: _isConst,
         errorReporter: resolver.errorReporter,
-        errorNode: _errorNode,
+        errorEntity: _errorEntity,
         genericMetadataIsEnabled: resolver.genericMetadataIsEnabled,
         strictInference: resolver.analysisOptions.strictInference,
         strictCasts: resolver.analysisOptions.strictCasts,
@@ -339,7 +340,7 @@ class FunctionExpressionInvocationInferrer
       : super._();
 
   @override
-  ExpressionImpl get _errorNode => node.function;
+  ExpressionImpl get _errorEntity => node.function;
 }
 
 /// Specialization of [InvocationInferrer] for performing type inference on AST
@@ -355,7 +356,7 @@ class InstanceCreationInferrer
       : super._();
 
   @override
-  ConstructorNameImpl get _errorNode => node.constructorName;
+  ConstructorNameImpl get _errorEntity => node.constructorName;
 
   @override
   bool get _isConst => node.isConst;
@@ -407,7 +408,7 @@ abstract class InvocationExpressionInferrer<
       : super._();
 
   @override
-  Expression get _errorNode => node.function;
+  Expression get _errorEntity => node.function;
 
   @override
   TypeArgumentListImpl? get _typeArguments => node.typeArguments;

diff --git a/pkg/analyzer/lib/src/dart/resolver/typed_literal_resolver.dart b/pkg/analyzer/lib/src/dart/resolver/typed_literal_resolver.dart
@@ -420,7 +420,7 @@ class TypedLiteralResolver {
       contextReturnType: contextType,
       isConst: node.isConst,
       errorReporter: _errorReporter,
-      errorNode: node,
+      errorEntity: node,
       genericMetadataIsEnabled: _genericMetadataIsEnabled,
       strictInference: _resolver.analysisOptions.strictInference,
       strictCasts: _resolver.analysisOptions.strictCasts,

diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart
@@ -3805,7 +3805,7 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
     var inferrer = GenericInferrer(
       typeSystem,
       typeParameters,
-      errorNode: errorNode,
+      errorEntity: errorNode,
       genericMetadataIsEnabled: genericMetadataIsEnabled,
       strictInference: analysisOptions.strictInference,
       typeSystemOperations: flowAnalysis.typeOperations,

diff --git a/pkg/analyzer/test/src/dart/element/generic_inferrer_test.dart b/pkg/analyzer/test/src/dart/element/generic_inferrer_test.dart
@@ -621,7 +621,7 @@ class GenericFunctionInferenceTest extends AbstractTypeSystemTest {
       declaredReturnType: ft.returnType,
       contextReturnType: returnType,
       errorReporter: reporter,
-      errorNode: NullLiteralImpl(
+      errorEntity: NullLiteralImpl(
         literal: KeywordToken(Keyword.NULL, 0),
       ),
       genericMetadataIsEnabled: true,