fix #25183, add support to ErrorVerifier for generic methods

[email protected], [email protected]

Review URL: https://codereview.chromium.org/1514683002 .

Author: John Messerly

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

@@ -4868,6 +4868,35 @@ class StaticWarningCode extends ErrorCode {
       const StaticWarningCode('INVALID_METHOD_OVERRIDE_NAMED_PARAM_TYPE',
           "The parameter type '{0}' is not assignable to '{1}' as required by the method it is overriding from '{2}'");
 
+  /**
+   * Generic Method DEP: number of type parameters must match.
+   * <https://github.com/leafpetersen/dep-generic-methods/blob/master/proposal.md#function-subtyping>
+   *
+   * Parameters:
+   * 0: the number of type parameters in the method
+   * 1: the number of type parameters in the overridden method
+   * 2: the name of the class where the overridden method is declared
+   */
+  static const StaticWarningCode INVALID_METHOD_OVERRIDE_TYPE_PARAMETERS =
+      const StaticWarningCode('INVALID_METHOD_OVERRIDE_TYPE_PARAMETERS',
+          "The method has {0} type parameters, but it is overriding a method with {1} type parameters from '{2}'");
+
+  /**
+   * Generic Method DEP: bounds of type parameters must be compatible.
+   * <https://github.com/leafpetersen/dep-generic-methods/blob/master/proposal.md#function-subtyping>
+   *
+   * Parameters:
+   * 0: the type parameter name
+   * 1: the type parameter bound
+   * 2: the overridden type parameter name
+   * 3: the overridden type parameter bound
+   * 4: the name of the class where the overridden method is declared
+   */
+  static const StaticWarningCode INVALID_METHOD_OVERRIDE_TYPE_PARAMETER_BOUND =
+      const StaticWarningCode(
+          'INVALID_METHOD_OVERRIDE_TYPE_PARAMETER_BOUND',
+          "The type parameter '{0}' extends '{1}', but that is stricter than '{2}' extends '{3}' in the overridden method from '{4}'");
+
   /**
    * 7.1 Instance Methods: It is a static warning if an instance method
    * <i>m1</i> overrides an instance method <i>m2</i> and the type of <i>m1</i>

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

@@ -1347,6 +1347,71 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
     if (overridingFT == null || overriddenFT == null) {
       return false;
     }
+
+  // Handle generic function type parameters.
+    // TODO(jmesserly): this duplicates some code in isSubtypeOf and most of
+    // _isGenericFunctionSubtypeOf. Ideally, we'd let TypeSystem produce
+    // an error message once it's ready to "return false".
+    if (!overridingFT.boundTypeParameters.isEmpty) {
+      if (overriddenFT.boundTypeParameters.isEmpty) {
+        overriddenFT = _typeSystem.instantiateToBounds(overriddenFT);
+      } else {
+        List<TypeParameterElement> params1 = overridingFT.boundTypeParameters;
+        List<TypeParameterElement> params2 = overriddenFT.boundTypeParameters;
+        int count = params1.length;
+        if (params2.length != count) {
+          _errorReporter.reportErrorForNode(
+              StaticWarningCode.INVALID_METHOD_OVERRIDE_TYPE_PARAMETERS,
+              errorNameTarget, [
+            count,
+            params2.length,
+            overriddenExecutable.enclosingElement.displayName
+          ]);
+          return true;
+        }
+        // We build up a substitution matching up the type parameters
+        // from the two types, {variablesFresh/variables1} and
+        // {variablesFresh/variables2}
+        List<DartType> variables1 = new List<DartType>();
+        List<DartType> variables2 = new List<DartType>();
+        List<DartType> variablesFresh = new List<DartType>();
+        for (int i = 0; i < count; i++) {
+          TypeParameterElement p1 = params1[i];
+          TypeParameterElement p2 = params2[i];
+          TypeParameterElementImpl pFresh =
+              new TypeParameterElementImpl(p1.name, -1);
+
+          DartType variable1 = p1.type;
+          DartType variable2 = p2.type;
+          DartType variableFresh = new TypeParameterTypeImpl(pFresh);
+
+          variables1.add(variable1);
+          variables2.add(variable2);
+          variablesFresh.add(variableFresh);
+          DartType bound1 = p1.bound ?? DynamicTypeImpl.instance;
+          DartType bound2 = p2.bound ?? DynamicTypeImpl.instance;
+          bound1 = bound1.substitute2(variablesFresh, variables1);
+          bound2 = bound2.substitute2(variablesFresh, variables2);
+          pFresh.bound = bound2;
+          if (!_typeSystem.isSubtypeOf(bound2, bound1)) {
+            _errorReporter.reportErrorForNode(
+                StaticWarningCode.INVALID_METHOD_OVERRIDE_TYPE_PARAMETER_BOUND,
+                errorNameTarget, [
+              p1.displayName,
+              p1.bound,
+              p2.displayName,
+              p2.bound,
+              overriddenExecutable.enclosingElement.displayName
+            ]);
+            return true;
+          }
+        }
+        // Proceed with the rest of the checks, using instantiated types.
+        overridingFT = overridingFT.instantiate(variablesFresh);
+        overriddenFT = overriddenFT.instantiate(variablesFresh);
+      }
+    }
+
     DartType overridingFTReturnType = overridingFT.returnType;
     DartType overriddenFTReturnType = overriddenFT.returnType;
     List<DartType> overridingNormalPT = overridingFT.normalParameterTypes;

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

@@ -565,6 +565,15 @@ abstract class TypeSystem {
   DartType getLeastUpperBound(
       TypeProvider typeProvider, DartType type1, DartType type2);
 
+  /**
+   * Given a [function] type, instantiate it with its bounds.
+   *
+   * The behavior of this method depends on the type system, for example, in
+   * classic Dart `dynamic` will be used for all type arguments, whereas
+   * strong mode prefers the actual bound type if it was specified.
+   */
+  FunctionType instantiateToBounds(FunctionType function);
+
   /**
    * Return `true` if the [leftType] is assignable to the [rightType] (that is,
    * if leftType <==> rightType).
@@ -673,6 +682,18 @@ class TypeSystemImpl implements TypeSystem {
     }
   }
 
+  /**
+   * Instantiate the function type using `dynamic` for all generic parameters.
+   */
+  FunctionType instantiateToBounds(FunctionType function) {
+    int count = function.boundTypeParameters.length;
+    if (count == 0) {
+      return function;
+    }
+    return function.instantiate(
+        new List<DartType>.filled(count, DynamicTypeImpl.instance));
+  }
+
   @override
   bool isAssignableTo(DartType leftType, DartType rightType) {
     return leftType.isAssignableTo(rightType);

pkg/analyzer/test/generated/resolver_test.dart

@@ -12972,6 +12972,98 @@ class C<T> {
     expect(f.staticType.toString(), '<S>(S) → dynamic');
   }
 
+  void test_genericMethod_override() {
+    _resolveTestUnit(r'''
+class C {
+  /*=T*/ f/*<T>*/(/*=T*/ x) => null;
+}
+class D extends C {
+  /*=T*/ f/*<T>*/(/*=T*/ x) => null; // from D
+}
+''');
+    SimpleIdentifier f =
+        _findIdentifier('f/*<T>*/(/*=T*/ x) => null; // from D');
+    MethodElementImpl e = f.staticElement;
+    expect(e.typeParameters.toString(), '[T]');
+    expect(e.type.boundTypeParameters.toString(), '[T]');
+    expect(e.type.toString(), '<T>(T) → T');
+
+    FunctionType ft = e.type.instantiate([typeProvider.stringType]);
+    expect(ft.toString(), '(String) → String');
+  }
+
+  void test_genericMethod_override_bounds() {
+    _resolveTestUnit(r'''
+class A {}
+class B extends A {}
+class C {
+  /*=T*/ f/*<T extends B>*/(/*=T*/ x) => null;
+}
+class D extends C {
+  /*=T*/ f/*<T extends A>*/(/*=T*/ x) => null;
+}
+''');
+  }
+
+  void test_genericMethod_override_invalidReturnType() {
+    Source source = addSource(r'''
+class C {
+  Iterable/*<T>*/ f/*<T>*/(/*=T*/ x) => null;
+}
+class D extends C {
+  String f/*<S>*/(/*=S*/ x) => null;
+}''');
+    // TODO(jmesserly): we can't use assertErrors because STRONG_MODE_* errors
+    // from CodeChecker don't have working equality.
+    List<AnalysisError> errors = analysisContext2.computeErrors(source);
+    expect(errors.map((e) => e.errorCode.name), [
+      'INVALID_METHOD_OVERRIDE_RETURN_TYPE',
+      'STRONG_MODE_INVALID_METHOD_OVERRIDE'
+    ]);
+    expect(errors[0].message, contains('Iterable<S>'),
+        reason: 'errors should be in terms of the type parameters '
+            'at the error location');
+    verify([source]);
+  }
+
+  void test_genericMethod_override_invalidTypeParamBounds() {
+    Source source = addSource(r'''
+class A {}
+class B extends A {}
+class C {
+  /*=T*/ f/*<T extends A>*/(/*=T*/ x) => null;
+}
+class D extends C {
+  /*=T*/ f/*<T extends B>*/(/*=T*/ x) => null;
+}''');
+    // TODO(jmesserly): this is modified code from assertErrors, which we can't
+    // use directly because STRONG_MODE_* errors don't have working equality.
+    List<AnalysisError> errors = analysisContext2.computeErrors(source);
+    expect(errors.map((e) => e.errorCode.name), [
+      'STRONG_MODE_INVALID_METHOD_OVERRIDE',
+      'INVALID_METHOD_OVERRIDE_TYPE_PARAMETER_BOUND'
+    ]);
+    verify([source]);
+  }
+
+  void test_genericMethod_override_invalidTypeParamCount() {
+    Source source = addSource(r'''
+class C {
+  /*=T*/ f/*<T>*/(/*=T*/ x) => null;
+}
+class D extends C {
+  /*=S*/ f/*<T, S>*/(/*=T*/ x) => null;
+}''');
+    // TODO(jmesserly): we can't use assertErrors because STRONG_MODE_* errors
+    // from CodeChecker don't have working equality.
+    List<AnalysisError> errors = analysisContext2.computeErrors(source);
+    expect(errors.map((e) => e.errorCode.name), [
+      'STRONG_MODE_INVALID_METHOD_OVERRIDE',
+      'INVALID_METHOD_OVERRIDE_TYPE_PARAMETERS'
+    ]);
+    verify([source]);
+  }
+
   void test_pseudoGeneric_max_doubleDouble() {
     String code = r'''
 import 'dart:math';