Set ConstructorReference TypeName.type to null; re-arrange bounds checks

A follow up from https://dart-review.googlesource.com/c/sdk/+/211304

Change-Id: Ia68a232befc6371739551eb40ad49a9ef15ee5c0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/211640
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>

pkg/analyzer/lib/src/dart/resolver/constructor_reference_resolver.dart

@@ -53,7 +53,7 @@ class ConstructorReferenceResolver {
       if (enclosingElement is TypeAliasElement) {
         enclosingElement = enclosingElement.aliasedType.element;
       }
-      // TODO(srawlins): Handle `enclosingElement` being a functio typedef:
+      // TODO(srawlins): Handle `enclosingElement` being a function typedef:
       // typedef F<T> = void Function(); var a = F<int>.extensionOnType;`.
       // This is illegal.
       if (enclosingElement is ClassElement) {
@@ -123,16 +123,7 @@ class ConstructorReferenceResolver {
         constructorName.staticElement = constructorElement.declaration;
         constructorName.name?.staticElement = constructorElement.declaration;
         node.staticType = inferred;
-        // TODO(srawlins): Always set the TypeName's type to `null`, here, and
-        // in the "else" case below, at the very end of [_inferArgumentTypes].
-        // This requires refactoring how type arguments are checked against
-        // bounds, as this is currently always done with the [TypeName], in
-        // type_argument_verifier.dart.
-        if (inferred.typeFormals.isNotEmpty) {
-          constructorName.type.type = null;
-        } else {
-          constructorName.type.type = inferredReturnType;
-        }
+        constructorName.type.type = null;
       }
     } else {
       var constructorElement = constructorName.staticElement;
@@ -141,6 +132,7 @@ class ConstructorReferenceResolver {
       } else {
         node.staticType = constructorElement.type;
       }
+      constructorName.type.type = null;
     }
   }
 }

pkg/analyzer/lib/src/error/type_arguments_verifier.dart

@@ -32,8 +32,67 @@ class TypeArgumentsVerifier {
   TypeSystemImpl get _typeSystem =>
       _libraryElement.typeSystem as TypeSystemImpl;
 
+  void checkConstructorReference(ConstructorReference node) {
+    var classElement = node.constructorName.type.name.staticElement;
+    List<TypeParameterElement> typeParameters;
+    if (classElement is TypeAliasElement) {
+      typeParameters = classElement.typeParameters;
+    } else if (classElement is ClassElement) {
+      typeParameters = classElement.typeParameters;
+    } else {
+      return;
+    }
+
+    if (typeParameters.isEmpty) {
+      return;
+    }
+    var typeArgumentList = node.constructorName.type.typeArguments;
+    if (typeArgumentList == null) {
+      return;
+    }
+    var constructorType = node.staticType;
+    if (constructorType is DynamicType) {
+      // An erroneous constructor reference.
+      return;
+    }
+    if (constructorType is! FunctionType) {
+      return;
+    }
+    var typeArguments = [
+      for (var type in typeArgumentList.arguments) type.type!,
+    ];
+    if (typeArguments.length != typeParameters.length) {
+      // Wrong number of type arguments to be reported elsewhere.
+      return;
+    }
+    var typeArgumentListLength = typeArgumentList.arguments.length;
+    var substitution = Substitution.fromPairs(typeParameters, typeArguments);
+    for (var i = 0; i < typeArguments.length; i++) {
+      var typeParameter = typeParameters[i];
+      var typeArgument = typeArguments[i];
+
+      var bound = typeParameter.bound;
+      if (bound == null) {
+        continue;
+      }
+
+      bound = _libraryElement.toLegacyTypeIfOptOut(bound);
+      bound = substitution.substituteType(bound);
+
+      if (!_typeSystem.isSubtypeOf(typeArgument, bound)) {
+        var errorNode =
+            i < typeArgumentListLength ? typeArgumentList.arguments[i] : node;
+        _errorReporter.reportErrorForNode(
+          CompileTimeErrorCode.TYPE_ARGUMENT_NOT_MATCHING_BOUNDS,
+          errorNode,
+          [typeArgument, typeParameter.name, bound],
+        );
+      }
+    }
+  }
+
   void checkFunctionExpressionInvocation(FunctionExpressionInvocation node) {
-    _checkTypeArguments(
+    _checkInvocationTypeArguments(
       node.typeArguments?.arguments,
       node.function.staticType,
       node.staticInvokeType,
@@ -42,7 +101,7 @@ class TypeArgumentsVerifier {
   }
 
   void checkFunctionReference(FunctionReference node) {
-    _checkTypeArguments(
+    _checkInvocationTypeArguments(
       node.typeArguments?.arguments,
       node.function.staticType,
       node.staticType,
@@ -80,7 +139,7 @@ class TypeArgumentsVerifier {
   }
 
   void checkMethodInvocation(MethodInvocation node) {
-    _checkTypeArguments(
+    _checkInvocationTypeArguments(
       node.typeArguments?.arguments,
       node.function.staticType,
       node.staticInvokeType,
@@ -313,40 +372,9 @@ class TypeArgumentsVerifier {
     }
   }
 
-  /// Checks to ensure that the given list of type [arguments] does not have a
-  /// type parameter as a type argument. The [errorCode] is either
-  /// [CompileTimeErrorCode.INVALID_TYPE_ARGUMENT_IN_CONST_LIST] or
-  /// [CompileTimeErrorCode.INVALID_TYPE_ARGUMENT_IN_CONST_MAP].
-  void _checkTypeArgumentConst(
-      NodeList<TypeAnnotation> arguments, ErrorCode errorCode) {
-    for (TypeAnnotation type in arguments) {
-      if (type is TypeName && type.type is TypeParameterType) {
-        _errorReporter.reportErrorForNode(errorCode, type, [type.name]);
-      }
-    }
-  }
-
-  /// Verify that the given list of [typeArguments] contains exactly the
-  /// [expectedCount] of elements, reporting an error with the [errorCode]
-  /// if not.
-  void _checkTypeArgumentCount(
-    TypeArgumentList typeArguments,
-    int expectedCount,
-    ErrorCode errorCode,
-  ) {
-    int actualCount = typeArguments.arguments.length;
-    if (actualCount != expectedCount) {
-      _errorReporter.reportErrorForNode(
-        errorCode,
-        typeArguments,
-        [actualCount],
-      );
-    }
-  }
-
   /// Verify that each type argument in [typeArgumentList] is within its bounds,
   /// as defined by [genericType].
-  void _checkTypeArguments(
+  void _checkInvocationTypeArguments(
     List<TypeAnnotation>? typeArgumentList,
     DartType? genericType,
     DartType? instantiatedType,
@@ -407,6 +435,37 @@ class TypeArgumentsVerifier {
     }
   }
 
+  /// Checks to ensure that the given list of type [arguments] does not have a
+  /// type parameter as a type argument. The [errorCode] is either
+  /// [CompileTimeErrorCode.INVALID_TYPE_ARGUMENT_IN_CONST_LIST] or
+  /// [CompileTimeErrorCode.INVALID_TYPE_ARGUMENT_IN_CONST_MAP].
+  void _checkTypeArgumentConst(
+      NodeList<TypeAnnotation> arguments, ErrorCode errorCode) {
+    for (TypeAnnotation type in arguments) {
+      if (type is TypeName && type.type is TypeParameterType) {
+        _errorReporter.reportErrorForNode(errorCode, type, [type.name]);
+      }
+    }
+  }
+
+  /// Verify that the given list of [typeArguments] contains exactly the
+  /// [expectedCount] of elements, reporting an error with the [errorCode]
+  /// if not.
+  void _checkTypeArgumentCount(
+    TypeArgumentList typeArguments,
+    int expectedCount,
+    ErrorCode errorCode,
+  ) {
+    int actualCount = typeArguments.arguments.length;
+    if (actualCount != expectedCount) {
+      _errorReporter.reportErrorForNode(
+        errorCode,
+        typeArguments,
+        [actualCount],
+      );
+    }
+  }
+
   /// Given a [node] without type arguments that refers to [element], issues
   /// an error if [type] is a generic type, and the type arguments were not
   /// supplied from inference or a non-dynamic default instantiation.

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

@@ -557,6 +557,11 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
     }
   }
 
+  @override
+  void visitConstructorReference(ConstructorReference node) {
+    _typeArgumentsVerifier.checkConstructorReference(node);
+  }
+
   @override
   void visitContinueStatement(ContinueStatement node) {
     var labelNode = node.label;

pkg/analyzer/test/src/dart/resolution/constructor_reference_test.dart

@@ -37,7 +37,27 @@ const a = TA<int, String>.foo;
           substitution: {'T': 'String', 'U': 'int'}),
       classElement,
       'A<String, int> Function()',
-      expectedTypeNameType: 'A<String, int>',
+      expectedTypeNameElement: findElement.typeAlias('TA'),
+    );
+  }
+
+  test_alias_generic_const_differingNumberOfTypeParamters() async {
+    await assertNoErrorsInCode('''
+class A<T, U> {
+  A.foo() {}
+}
+typedef TA<T> = A<T, String>;
+
+const x = TA<int>.foo;
+''');
+
+    var classElement = findElement.class_('A');
+    assertConstructorReference(
+      findNode.constructorReference('TA<int>.foo;'),
+      elementMatcher(classElement.getNamedConstructor('foo')!,
+          substitution: {'T': 'int', 'U': 'String'}),
+      classElement,
+      'A<int, String> Function()',
       expectedTypeNameElement: findElement.typeAlias('TA'),
     );
   }
@@ -61,7 +81,6 @@ void bar() {
           substitution: {'T': 'String', 'U': 'int'}),
       classElement,
       'A<String, int> Function()',
-      expectedTypeNameType: 'A<String, int>',
       expectedTypeNameElement: findElement.typeAlias('TA'),
     );
   }
@@ -85,7 +104,6 @@ void bar() {
           substitution: {'T': 'int'}),
       classElement,
       'A<int> Function()',
-      expectedTypeNameType: 'A<int>',
       expectedTypeNameElement: findElement.typeAlias('TA'),
     );
   }
@@ -109,7 +127,6 @@ void bar() {
           substitution: {'T': 'int'}),
       classElement,
       'A<int> Function()',
-      expectedTypeNameType: 'A<int>',
       expectedTypeNameElement: findElement.typeAlias('TA'),
     );
   }
@@ -135,7 +152,6 @@ void bar() {
           substitution: {'T': 'String'}),
       classElement,
       'A<String> Function()',
-      expectedTypeNameType: 'A<String>',
       expectedTypeNameElement: findElement.typeAlias('TA'),
     );
   }
@@ -156,7 +172,6 @@ const a = A<int>.new;
           substitution: {'T': 'int'}),
       classElement,
       'A<int> Function()',
-      expectedTypeNameType: 'A<int>',
     );
   }
 
@@ -178,7 +193,6 @@ void bar() {
           substitution: {'T': 'int'}),
       classElement,
       'A<int> Function()',
-      expectedTypeNameType: 'A<int>',
     );
   }
 
@@ -241,7 +255,6 @@ void bar() {
           substitution: {'T': 'int'}),
       classElement,
       'A<int> Function()',
-      expectedTypeNameType: 'A<int>',
     );
   }
 
@@ -265,7 +278,6 @@ void bar() {
       null,
       classElement,
       'dynamic',
-      expectedTypeNameType: 'A<int>',
     );
   }
 
@@ -287,7 +299,6 @@ void bar() {
           substitution: {'T': 'int'}),
       classElement,
       'A<int> Function()',
-      expectedTypeNameType: 'A<int>',
     );
   }
 
@@ -309,7 +320,6 @@ void bar() {
           substitution: {'T': 'int'}),
       classElement,
       'A<int> Function()',
-      expectedTypeNameType: 'A<int>',
     );
   }
 
@@ -331,7 +341,6 @@ void bar() {
           substitution: {'T': 'int'}),
       classElement,
       'A<int> Function()',
-      expectedTypeNameType: 'A<int>',
     );
   }
 
@@ -355,7 +364,6 @@ void bar() {
           substitution: {'T': 'String'}),
       classElement,
       'A<String> Function()',
-      expectedTypeNameType: 'A<String>',
     );
   }
 
@@ -381,7 +389,6 @@ void bar() {
           substitution: {'T': 'int'}),
       classElement,
       'A<int> Function()',
-      expectedTypeNameType: 'A<int>',
       expectedPrefix: findElement.import('package:test/a.dart').prefix,
       expectedTypeNameElement:
           findElement.importFind('package:test/a.dart').typeAlias('TA'),
@@ -409,7 +416,6 @@ void bar() {
           substitution: {'T': 'int'}),
       classElement,
       'A<int> Function()',
-      expectedTypeNameType: 'A<int>',
       expectedPrefix: findElement.import('package:test/a.dart').prefix,
     );
   }
@@ -438,7 +444,6 @@ void bar() {
           substitution: {'T': 'int'}),
       classElement,
       'A<int> Function()',
-      expectedTypeNameType: 'A<int>',
       expectedPrefix: findElement.import('package:test/a.dart').prefix,
     );
   }
@@ -464,7 +469,6 @@ void bar() {
           substitution: {'T': 'int'}),
       classElement,
       'A<int> Function()',
-      expectedTypeNameType: 'A<int>',
       expectedPrefix: findElement.import('package:test/a.dart').prefix,
     );
   }
@@ -491,7 +495,6 @@ foo() {
       classElement.unnamedConstructor,
       classElement,
       'A Function()',
-      expectedTypeNameType: 'A',
     );
   }
 
@@ -518,7 +521,6 @@ foo() {
       classElement.unnamedConstructor,
       classElement,
       'A Function()',
-      expectedTypeNameType: 'A',
     );
   }
 
@@ -547,7 +549,6 @@ foo() {
       classElement.unnamedConstructor,
       classElement,
       'A Function()',
-      expectedTypeNameType: 'A',
     );
   }
 
@@ -571,7 +572,6 @@ A<String> Function() bar() {
       constructorElement,
       classElement,
       'A<Never> Function()',
-      expectedTypeNameType: 'A<Never>',
     );
   }
 
@@ -593,7 +593,6 @@ A<int> Function() bar() {
       constructorElement,
       classElement,
       'A<int> Function()',
-      expectedTypeNameType: 'A<int>',
     );
   }
 
@@ -615,7 +614,6 @@ void bar() {
       constructorElement,
       classElement,
       'A<T> Function<T>()',
-      expectedTypeNameType: null,
     );
   }
 
@@ -637,7 +635,6 @@ void bar() {
       constructorElement,
       classElement,
       'A<T> Function<T extends num>()',
-      expectedTypeNameType: null,
     );
   }
 
@@ -656,7 +653,6 @@ const a1 = A.new;
       classElement.unnamedConstructor,
       classElement,
       'A Function()',
-      expectedTypeNameType: 'A',
     );
   }
 
@@ -677,7 +673,6 @@ void bar() {
       classElement.getNamedConstructor('foo')!,
       classElement,
       'A Function()',
-      expectedTypeNameType: 'A',
     );
   }
 
@@ -698,7 +693,6 @@ bar() {
       classElement.unnamedConstructor,
       classElement,
       'A Function()',
-      expectedTypeNameType: 'A',
     );
   }
 
@@ -719,7 +713,6 @@ const a = TA.new;
       constructorElement,
       classElement,
       'A<T> Function<T>()',
-      expectedTypeNameType: null,
       expectedTypeNameElement: findElement.typeAlias('TA'),
     );
   }
@@ -743,7 +736,6 @@ bar() {
       constructorElement,
       findElement.class_('A'),
       'A<String, U> Function<U>()',
-      expectedTypeNameType: null,
       expectedTypeNameElement: findElement.typeAlias('TA'),
     );
   }
@@ -765,7 +757,6 @@ const a = TA.new;
       elementMatcher(constructorElement, substitution: {'T': 'int'}),
       classElement,
       'A<int> Function()',
-      expectedTypeNameType: 'A<int>',
       expectedTypeNameElement: findElement.typeAlias('TA'),
     );
   }
@@ -789,7 +780,6 @@ bar() {
       elementMatcher(constructorElement, substitution: {'T': 'int'}),
       classElement,
       'A<int> Function()',
-      expectedTypeNameType: 'A<int>',
       expectedTypeNameElement: findElement.typeAlias('TA'),
     );
   }
@@ -817,7 +807,6 @@ void bar() {
       null,
       classElement,
       'dynamic',
-      expectedTypeNameType: 'A<int>',
     );
   }
 
@@ -840,7 +829,6 @@ void bar() {
       classElement.getNamedConstructor('foo')!,
       classElement,
       'A Function()',
-      expectedTypeNameType: 'A',
     );
   }
 }

pkg/analyzer/test/src/dart/resolution/resolution.dart

@@ -166,7 +166,6 @@ mixin ResolutionTest implements ResourceProviderMixin {
     Object? expectedConstructorElement,
     ClassElement expectedClassElement,
     String expectedType, {
-    required String? expectedTypeNameType,
     PrefixElement? expectedPrefix,
     Element? expectedTypeNameElement,
   }) {
@@ -184,7 +183,7 @@ mixin ResolutionTest implements ResourceProviderMixin {
 
     var typeName = node.constructorName.type;
     expectedTypeNameElement ??= expectedClassElement;
-    assertTypeName(typeName, expectedTypeNameElement, expectedTypeNameType,
+    assertTypeName(typeName, expectedTypeNameElement, null,
         expectedPrefix: expectedPrefix);
   }
 

pkg/analyzer/test/src/summary/resynthesize_common.dart

@@ -6332,7 +6332,7 @@ library
                   staticElement: self::@class::A
                   staticType: null
                   token: A @35
-                type: A
+                type: null
             staticType: A Function()
     accessors
       synthetic static get v @-1