fix #25578, implement @covariant parameter overrides

[email protected]

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

Author: John Messerly

pkg/analyzer/lib/dart/element/element.dart

@@ -1612,6 +1612,12 @@ abstract class ParameterElement
    */
   String get defaultValueCode;
 
+  /**
+   * Return `true` if this parameter is covariant, meaning it is allowed to have
+   * a narrower type in an override.
+   */
+  bool get isCovariant;
+
   /**
    * Return `true` if this parameter is an initializing formal parameter.
    */

pkg/analyzer/lib/src/dart/element/element.dart

@@ -2440,6 +2440,12 @@ class DynamicElementImpl extends ElementImpl implements TypeDefiningElement {
  * A concrete implementation of an [ElementAnnotation].
  */
 class ElementAnnotationImpl implements ElementAnnotation {
+  /**
+   * The name of the top-level variable used to mark a method parameter as
+   * covariant.
+   */
+  static String _COVARIANT_VARIABLE_NAME = "checked";
+
   /**
    * The name of the class used to mark an element as being deprecated.
    */
@@ -2543,6 +2549,15 @@ class ElementAnnotationImpl implements ElementAnnotation {
   @override
   AnalysisContext get context => compilationUnit.library.context;
 
+  /**
+   * Return `true` if this annotation marks the associated parameter as being
+   * covariant, meaning it is allowed to have a narrower type in an override.
+   */
+  bool get isCovariant =>
+      element is PropertyAccessorElement &&
+      element.name == _COVARIANT_VARIABLE_NAME &&
+      element.library?.name == _META_LIB_NAME;
+
   @override
   bool get isDeprecated {
     if (element?.library?.isDartCore == true) {
@@ -6752,6 +6767,13 @@ class ParameterElementImpl extends VariableElementImpl
    */
   int _visibleRangeLength = -1;
 
+  /**
+   * True if this parameter inherits from a covariant parameter. This happens
+   * when it overrides a method in a supertype that has a corresponding
+   * covariant parameter.
+   */
+  bool inheritsCovariant = false;
+
   /**
    * Initialize a newly created parameter element to have the given [name] and
    * [nameOffset].
@@ -6906,6 +6928,19 @@ class ParameterElementImpl extends VariableElementImpl
     return super.isConst;
   }
 
+  @override
+  bool get isCovariant {
+    if (inheritsCovariant) {
+      return true;
+    }
+    for (ElementAnnotationImpl annotation in metadata) {
+      if (annotation.isCovariant) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   @override
   bool get isFinal {
     if (_unlinkedParam != null) {

pkg/analyzer/lib/src/dart/element/handle.dart

@@ -894,6 +894,9 @@ class ParameterElementHandle extends VariableElementHandle
   @override
   String get defaultValueCode => actualElement.defaultValueCode;
 
+  @override
+  bool get isCovariant => actualElement.isCovariant;
+
   @override
   bool get isInitializingFormal => actualElement.isInitializingFormal;
 

pkg/analyzer/lib/src/dart/element/member.dart

@@ -232,6 +232,9 @@ class FieldFormalParameterMember extends ParameterMember
       [DartType type])
       : super(baseElement, definingType, type);
 
+  @override
+  bool get isCovariant => baseElement.isCovariant;
+
   @override
   FieldElement get field {
     FieldElement field = (baseElement as FieldFormalParameterElement).field;
@@ -674,6 +677,9 @@ class ParameterMember extends VariableMember
   @override
   int get hashCode => baseElement.hashCode;
 
+  @override
+  bool get isCovariant => baseElement.isCovariant;
+
   @override
   bool get isInitializingFormal => baseElement.isInitializingFormal;
 

pkg/analyzer/lib/src/dart/element/type.dart

@@ -797,14 +797,17 @@ class FunctionTypeImpl extends TypeImpl implements FunctionType {
     return relate(
         this,
         type,
-        (DartType t, DartType s) =>
+        (DartType t, DartType s, _, __) =>
             (t as TypeImpl).isMoreSpecificThan(s, withDynamic),
         new TypeSystemImpl().instantiateToBounds);
   }
 
   @override
   bool isSubtypeOf(DartType type) {
-    return relate(this, type, (DartType t, DartType s) => t.isAssignableTo(s),
+    return relate(
+        this,
+        type,
+        (DartType t, DartType s, _, __) => t.isAssignableTo(s),
         new TypeSystemImpl().instantiateToBounds);
   }
 
@@ -966,17 +969,18 @@ class FunctionTypeImpl extends TypeImpl implements FunctionType {
    *
    * Used for the various relations on function types which have the same
    * structural rules for handling optional parameters and arity, but use their
-   * own relation for comparing corresponding paramaters or return types.
+   * own relation for comparing corresponding parameters or return types.
    *
    * If [returnRelation] is omitted, uses [parameterRelation] for both.
    */
   static bool relate(
       FunctionType t,
       DartType other,
-      bool parameterRelation(DartType t, DartType s),
+      bool parameterRelation(
+          DartType t, DartType s, bool tIsCovariant, bool sIsCovariant),
       DartType instantiateToBounds(DartType t),
       {bool returnRelation(DartType t, DartType s)}) {
-    returnRelation ??= parameterRelation;
+    returnRelation ??= (t, s) => parameterRelation(t, s, false, false);
 
     // Trivial base cases.
     if (other == null) {
@@ -1014,12 +1018,39 @@ class FunctionTypeImpl extends TypeImpl implements FunctionType {
     }
 
     // Test the parameter types.
-    List<DartType> tRequired = t.normalParameterTypes;
-    List<DartType> sRequired = s.normalParameterTypes;
-    List<DartType> tOptional = t.optionalParameterTypes;
-    List<DartType> sOptional = s.optionalParameterTypes;
-    Map<String, DartType> tNamed = t.namedParameterTypes;
-    Map<String, DartType> sNamed = s.namedParameterTypes;
+
+    // TODO(jmesserly): this could be implemented with less allocation if we
+    // wanted, by taking advantage of the fact that positional arguments must
+    // appear before named ones.
+    var tRequired = <ParameterElement>[];
+    var tOptional = <ParameterElement>[];
+    var tNamed = <String, ParameterElement>{};
+    for (var p in t.parameters) {
+      var kind = p.parameterKind;
+      if (kind == ParameterKind.REQUIRED) {
+        tRequired.add(p);
+      } else if (kind == ParameterKind.POSITIONAL) {
+        tOptional.add(p);
+      } else {
+        assert(kind == ParameterKind.NAMED);
+        tNamed[p.name] = p;
+      }
+    }
+
+    var sRequired = <ParameterElement>[];
+    var sOptional = <ParameterElement>[];
+    var sNamed = <String, ParameterElement>{};
+    for (var p in s.parameters) {
+      var kind = p.parameterKind;
+      if (kind == ParameterKind.REQUIRED) {
+        sRequired.add(p);
+      } else if (kind == ParameterKind.POSITIONAL) {
+        sOptional.add(p);
+      } else {
+        assert(kind == ParameterKind.NAMED);
+        sNamed[p.name] = p;
+      }
+    }
 
     // If one function has positional and the other has named parameters,
     // they don't relate.
@@ -1037,19 +1068,21 @@ class FunctionTypeImpl extends TypeImpl implements FunctionType {
     // For each named parameter in s, make sure we have a corresponding one
     // that relates.
     for (String key in sNamed.keys) {
-      var tParamType = tNamed[key];
-      if (tParamType == null) {
+      var tParam = tNamed[key];
+      if (tParam == null) {
         return false;
       }
-      if (!parameterRelation(tParamType, sNamed[key])) {
+      var sParam = sNamed[key];
+      if (!parameterRelation(
+          tParam.type, sParam.type, tParam.isCovariant, sParam.isCovariant)) {
         return false;
       }
     }
 
     // Make sure all of the positional parameters (both required and optional)
     // relate to each other.
-    List<DartType> tPositional = tRequired;
-    List<DartType> sPositional = sRequired;
+    var tPositional = tRequired;
+    var sPositional = sRequired;
 
     if (tOptional.isNotEmpty) {
       tPositional = tPositional.toList()..addAll(tOptional);
@@ -1070,7 +1103,10 @@ class FunctionTypeImpl extends TypeImpl implements FunctionType {
     }
 
     for (int i = 0; i < sPositional.length; i++) {
-      if (!parameterRelation(tPositional[i], sPositional[i])) {
+      var tParam = tPositional[i];
+      var sParam = sPositional[i];
+      if (!parameterRelation(
+          tParam.type, sParam.type, tParam.isCovariant, sParam.isCovariant)) {
         return false;
       }
     }

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

@@ -419,7 +419,7 @@ class ElementResolver extends SimpleAstVisitor<Object> {
 
   @override
   Object visitFieldFormalParameter(FieldFormalParameter node) {
-    _resolveMetadataForParameter(node.element, node);
+    _resolveMetadataForParameter(node);
     return super.visitFieldFormalParameter(node);
   }
 
@@ -463,7 +463,7 @@ class ElementResolver extends SimpleAstVisitor<Object> {
 
   @override
   Object visitFunctionTypedFormalParameter(FunctionTypedFormalParameter node) {
-    _resolveMetadataForParameter(node.element, node);
+    _resolveMetadataForParameter(node);
     return null;
   }
 
@@ -1055,7 +1055,7 @@ class ElementResolver extends SimpleAstVisitor<Object> {
 
   @override
   Object visitSimpleFormalParameter(SimpleFormalParameter node) {
-    _resolveMetadataForParameter(node.element, node);
+    _resolveMetadataForParameter(node);
     return null;
   }
 
@@ -2270,8 +2270,7 @@ class ElementResolver extends SimpleAstVisitor<Object> {
    * Given a [node] that can have annotations associated with it, resolve the
    * annotations in the element model representing annotations to the node.
    */
-  void _resolveMetadataForParameter(
-      Element element, NormalFormalParameter node) {
+  void _resolveMetadataForParameter(NormalFormalParameter node) {
     _resolveAnnotations(node.metadata);
   }
 

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

@@ -2491,7 +2491,6 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
       DartType expectedStaticType,
       ErrorCode errorCode) {
     // TODO(leafp): Move the Downcast functionality here.
-    // TODO(leafp): Support strict downcasts
     if (!_expressionIsAssignableAtType(
         expression, actualStaticType, expectedStaticType)) {
       _errorReporter.reportTypeErrorForNode(
@@ -5038,10 +5037,10 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
           FunctionType requiredMemberFT = _inheritanceManager
               .substituteTypeArgumentsInMemberFromInheritance(
                   requiredMemberType, memberName, enclosingType);
-          foundConcreteFT =
-              _typeSystem.typeToConcreteType(_typeProvider, foundConcreteFT);
-          requiredMemberFT =
-              _typeSystem.typeToConcreteType(_typeProvider, requiredMemberFT);
+          foundConcreteFT = _typeSystem.functionTypeToConcreteType(
+              _typeProvider, foundConcreteFT);
+          requiredMemberFT = _typeSystem.functionTypeToConcreteType(
+              _typeProvider, requiredMemberFT);
           if (_typeSystem.isSubtypeOf(foundConcreteFT, requiredMemberFT)) {
             continue;
           }
@@ -6229,11 +6228,10 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
   bool _expressionIsAssignableAtType(Expression expression,
       DartType actualStaticType, DartType expectedStaticType) {
     bool concrete = _options.strongMode && checker.isKnownFunction(expression);
-    if (concrete) {
-      actualStaticType =
-          _typeSystem.typeToConcreteType(_typeProvider, actualStaticType);
+    if (concrete && actualStaticType is FunctionType) {
+      actualStaticType = _typeSystem.functionTypeToConcreteType(
+          _typeProvider, actualStaticType);
       // TODO(leafp): Move the Downcast functionality here.
-      // TODO(leafp): Support strict downcasts
     }
     return _typeSystem.isAssignableTo(actualStaticType, expectedStaticType);
   }

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

@@ -7334,7 +7334,8 @@ class ResolverVisitor extends ScopedVisitor {
         !FunctionTypeImpl.relate(
             expectedClosureType,
             staticClosureType,
-            (DartType t, DartType s) => (t as TypeImpl).isMoreSpecificThan(s),
+            (DartType t, DartType s, _, __) =>
+                (t as TypeImpl).isMoreSpecificThan(s),
             new TypeSystemImpl().instantiateToBounds,
             returnRelation: (s, t) => true)) {
       return;

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

@@ -811,18 +811,30 @@ class StrongTypeSystemImpl extends TypeSystem {
         provider.dynamicType;
   }
 
-  /**
-   * Check that [f1] is a subtype of [f2].
-   *
-   * This will always assume function types use fuzzy arrows, in other words
-   * that dynamic parameters of f1 and f2 are treated as bottom.
-   */
+  /// Check that [f1] is a subtype of [f2].
+  ///
+  /// This will always assume function types use fuzzy arrows, in other words
+  /// that dynamic parameters of f1 and f2 are treated as bottom.
   bool _isFunctionSubtypeOf(FunctionType f1, FunctionType f2) {
     return FunctionTypeImpl.relate(
         f1,
         f2,
-        (DartType t1, DartType t2) =>
-            _isSubtypeOf(t2, t1, null, dynamicIsBottom: true),
+        (t1, t2, _, __) => _isSubtypeOf(t2, t1, null, dynamicIsBottom: true),
+        instantiateToBounds,
+        returnRelation: isSubtypeOf);
+  }
+
+  /// Check that [f1] is a subtype of [f2] for an override.
+  ///
+  /// This is different from the normal function subtyping in two ways:
+  /// - we know the function types are strict arrows,
+  /// - it allows opt-in covariant parameters.
+  bool isOverrideSubtypeOf(FunctionType f1, FunctionType f2) {
+    return FunctionTypeImpl.relate(
+        f1,
+        f2,
+        (t1, t2, t1Covariant, _) =>
+            isSubtypeOf(t2, t1) || t1Covariant && isSubtypeOf(t1, t2),
         instantiateToBounds,
         returnRelation: isSubtypeOf);
   }

pkg/analyzer/lib/src/summary/link.dart

@@ -3901,6 +3901,9 @@ class ParameterElementForLink implements ParameterElementImpl {
   DartType _inferredType;
   DartType _declaredType;
 
+  @override
+  bool inheritsCovariant = false;
+
   ParameterElementForLink(this.enclosingElement, this._unlinkedParam,
       this._typeParameterContext, this.compilationUnit, this._parameterIndex) {
     if (_unlinkedParam.initializer?.bodyExpr != null) {
@@ -3915,6 +3918,9 @@ class ParameterElementForLink implements ParameterElementImpl {
   bool get hasImplicitType =>
       !_unlinkedParam.isFunctionTyped && _unlinkedParam.type == null;
 
+  @override
+  bool get isCovariant => false;
+
   @override
   String get name => _unlinkedParam.name;