Check that implicit type arguments satisfy bounds.

This required adding an implementation of super-bounded types to the analyzer. We really could use some more tests to verify that we disallow super-bounded types in all the appropriate locations, but allow it in other cases (it seems that there are no language_2 tests to verify this). To avoid churn, I'll wait until I have confirmation that my reasoning is correct in flutter#34583 before submitting test cases. Fixes flutter#34532. Fixes flutter#34560. Change-Id: I3c5def60bcac0d31b56bead31cb1aab445f18e96 Reviewed-on: https://dart-review.googlesource.com/76280 Commit-Queue: Paul Berry <paulberry@google.com> Reviewed-by: Konstantin Shcheglov <scheglov@google.com> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
a-siva · Sep 26, 2018 · b1a5415 · b1a5415
1 parent 3b511b9
commit b1a5415
Show file tree

Hide file tree

Showing 10 changed files with 210 additions and 26 deletions.
diff --git a/pkg/analyzer/lib/src/dart/element/type.dart b/pkg/analyzer/lib/src/dart/element/type.dart
@@ -11,6 +11,7 @@ import 'package:analyzer/src/dart/element/element.dart';
 import 'package:analyzer/src/dart/element/member.dart';
 import 'package:analyzer/src/generated/engine.dart'
     show AnalysisContext, AnalysisEngine;
+import 'package:analyzer/src/generated/resolver.dart';
 import 'package:analyzer/src/generated/type_system.dart';
 import 'package:analyzer/src/generated/utilities_collection.dart';
 import 'package:analyzer/src/generated/utilities_dart.dart';
@@ -87,6 +88,24 @@ class BottomTypeImpl extends TypeImpl {
   @override
   TypeImpl pruned(List<FunctionTypeAliasElement> prune) => this;
 
+  @override
+  DartType replaceTopAndBottom(TypeProvider typeProvider,
+      {bool isCovariant = true}) {
+    if (isCovariant) {
+      return this;
+    } else {
+      // In theory this should never happen, since we only need to do this
+      // replacement when checking super-boundedness of explicitly-specified
+      // types, or types produced by mixin inference or instantiate-to-bounds,
+      // and bottom can't occur in any of those cases.
+      assert(false,
+          'Attempted to check super-boundedness of a type including "bottom"');
+      // But just in case it does, return `dynamic` since that's similar to what
+      // we do with Null.
+      return typeProvider.objectType;
+    }
+  }
+
   @override
   BottomTypeImpl substitute2(
           List<DartType> argumentTypes, List<DartType> parameterTypes,
@@ -337,6 +356,16 @@ class DynamicTypeImpl extends TypeImpl {
   @override
   TypeImpl pruned(List<FunctionTypeAliasElement> prune) => this;
 
+  @override
+  DartType replaceTopAndBottom(TypeProvider typeProvider,
+      {bool isCovariant = true}) {
+    if (isCovariant) {
+      return typeProvider.nullType;
+    } else {
+      return this;
+    }
+  }
+
   @override
   DartType substitute2(
       List<DartType> argumentTypes, List<DartType> parameterTypes,
@@ -775,6 +804,32 @@ abstract class FunctionTypeImpl extends TypeImpl implements FunctionType {
         typeSystem.instantiateToBounds);
   }
 
+  @override
+  DartType replaceTopAndBottom(TypeProvider typeProvider,
+      {bool isCovariant: true}) {
+    var returnType = (this.returnType as TypeImpl)
+        .replaceTopAndBottom(typeProvider, isCovariant: isCovariant);
+    ParameterElement transformParameter(ParameterElement p) {
+      TypeImpl type = p.type;
+      var newType =
+          type.replaceTopAndBottom(typeProvider, isCovariant: !isCovariant);
+      if (identical(newType, type)) return p;
+      return new ParameterElementImpl.synthetic(
+          p.name,
+          newType,
+          // ignore: deprecated_member_use
+          p.parameterKind);
+    }
+
+    var parameters = _transformOrShare(this.parameters, transformParameter);
+    if (identical(returnType, this.returnType) &&
+        identical(parameters, this.parameters)) {
+      return this;
+    } else {
+      return new _FunctionTypeImplStrict._(returnType, typeFormals, parameters);
+    }
+  }
+
   @override
   FunctionType substitute2(
       List<DartType> argumentTypes, List<DartType> parameterTypes,
@@ -1991,6 +2046,21 @@ class InterfaceTypeImpl extends TypeImpl implements InterfaceType {
     }
   }
 
+  @override
+  DartType replaceTopAndBottom(TypeProvider typeProvider,
+      {bool isCovariant: true}) {
+    var typeArguments = _transformOrShare(
+        this.typeArguments,
+        (t) => (t as TypeImpl)
+            .replaceTopAndBottom(typeProvider, isCovariant: isCovariant));
+    if (identical(typeArguments, this.typeArguments)) {
+      return this;
+    } else {
+      return new InterfaceTypeImpl._(element, name, prunedTypedefs)
+        ..typeArguments = typeArguments;
+    }
+  }
+
   @override
   InterfaceTypeImpl substitute2(
       List<DartType> argumentTypes, List<DartType> parameterTypes,
@@ -2622,6 +2692,14 @@ abstract class TypeImpl implements DartType {
    */
   TypeImpl pruned(List<FunctionTypeAliasElement> prune);
 
+  /// Replaces all covariant occurrences of `dynamic`, `Object`, and `void` with
+  /// `Null` and all contravariant occurrences of `Null` with `Object`.
+  ///
+  /// The boolean `isCovariant` indicates whether this type is in covariant or
+  /// contravariant position.
+  DartType replaceTopAndBottom(TypeProvider typeProvider,
+      {bool isCovariant = true});
+
   @override
   DartType resolveToBound(DartType objectType) => this;
 
@@ -2864,6 +2942,12 @@ class TypeParameterTypeImpl extends TypeImpl implements TypeParameterType {
   @override
   TypeImpl pruned(List<FunctionTypeAliasElement> prune) => this;
 
+  @override
+  DartType replaceTopAndBottom(TypeProvider typeProvider,
+      {bool isCovariant = true}) {
+    return this;
+  }
+
   @override
   DartType resolveToBound(DartType objectType) {
     if (element.bound == null) {
@@ -2955,6 +3039,16 @@ class UndefinedTypeImpl extends TypeImpl {
   @override
   TypeImpl pruned(List<FunctionTypeAliasElement> prune) => this;
 
+  @override
+  DartType replaceTopAndBottom(TypeProvider typeProvider,
+      {bool isCovariant = true}) {
+    if (isCovariant) {
+      return typeProvider.nullType;
+    } else {
+      return this;
+    }
+  }
+
   @override
   DartType substitute2(
       List<DartType> argumentTypes, List<DartType> parameterTypes,
@@ -3018,6 +3112,16 @@ class VoidTypeImpl extends TypeImpl implements VoidType {
   @override
   TypeImpl pruned(List<FunctionTypeAliasElement> prune) => this;
 
+  @override
+  DartType replaceTopAndBottom(TypeProvider typeProvider,
+      {bool isCovariant = true}) {
+    if (isCovariant) {
+      return typeProvider.nullType;
+    } else {
+      return this;
+    }
+  }
+
   @override
   VoidTypeImpl substitute2(
           List<DartType> argumentTypes, List<DartType> parameterTypes,

diff --git a/pkg/analyzer/lib/src/generated/error_verifier.dart b/pkg/analyzer/lib/src/generated/error_verifier.dart
@@ -5355,29 +5355,32 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
    * [CompileTimeErrorCode.GENERIC_FUNCTION_CANNOT_BE_BOUND].
    */
   void _checkForTypeArgumentNotMatchingBounds(TypeName typeName) {
-    if (typeName.typeArguments == null) {
-      return;
-    }
     // prepare Type
     DartType type = typeName.type;
     if (type == null) {
       return;
     }
-    Element element = type.element;
-    if (element is ClassElement) {
+    // TODO(paulberry): handle the case where the type name is a typedef.
+    if (type is InterfaceType) {
+      var element = type.element;
       // prepare type parameters
       List<TypeParameterElement> parameterElements = element.typeParameters;
       List<DartType> parameterTypes = element.type.typeArguments;
-      List<DartType> arguments = (type as ParameterizedType).typeArguments;
+      List<DartType> arguments = type.typeArguments;
       // iterate over each bounded type parameter and corresponding argument
-      NodeList<TypeAnnotation> argumentNodes = typeName.typeArguments.arguments;
+      NodeList<TypeAnnotation> argumentNodes =
+          typeName.typeArguments?.arguments;
+      var typeArguments = type.typeArguments;
       int loopThroughIndex =
-          math.min(argumentNodes.length, parameterElements.length);
+          math.min(typeArguments.length, parameterElements.length);
       bool shouldSubstitute =
           arguments.length != 0 && arguments.length == parameterTypes.length;
       for (int i = 0; i < loopThroughIndex; i++) {
-        TypeAnnotation argumentNode = argumentNodes[i];
-        DartType argType = argumentNode.type;
+        DartType argType = typeArguments[i];
+        TypeAnnotation argumentNode =
+            argumentNodes != null && i < argumentNodes.length
+                ? argumentNodes[i]
+                : typeName;
         if (argType is FunctionType && argType.typeFormals.isNotEmpty) {
           _errorReporter.reportTypeErrorForNode(
               CompileTimeErrorCode.GENERIC_FUNCTION_CANNOT_BE_TYPE_ARGUMENT,
@@ -5400,6 +5403,15 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
               errorCode =
                   StaticTypeWarningCode.TYPE_ARGUMENT_NOT_MATCHING_BOUNDS;
             }
+            if (_shouldAllowSuperBoundedTypes(typeName)) {
+              var replacedType =
+                  (argType as TypeImpl).replaceTopAndBottom(_typeProvider);
+              if (!identical(replacedType, argType) &&
+                  _typeSystem.isSubtypeOf(replacedType, boundType)) {
+                // Bound is satisfied under super-bounded rules, so we're ok.
+                continue;
+              }
+            }
             _errorReporter.reportTypeErrorForNode(
                 errorCode, argumentNode, [argType, boundType]);
           }
@@ -6298,6 +6310,19 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
     return false;
   }
 
+  /// Determines if the given [typeName] occurs in a context where super-bounded
+  /// types are allowed.
+  bool _shouldAllowSuperBoundedTypes(TypeName typeName) {
+    var parent = typeName.parent;
+    if (parent is ExtendsClause) return false;
+    if (parent is OnClause) return false;
+    if (parent is ClassTypeAlias) return false;
+    if (parent is WithClause) return false;
+    if (parent is ConstructorName) return false;
+    if (parent is ImplementsClause) return false;
+    return true;
+  }
+
   /**
    * Returns [ExecutableElement]s that are declared in interfaces implemented
    * by the [classElement], but not implemented by the [classElement] or its

diff --git a/pkg/analyzer/lib/src/generated/type_system.dart b/pkg/analyzer/lib/src/generated/type_system.dart
@@ -2277,6 +2277,23 @@ class UnknownInferredType extends TypeImpl {
   @override
   TypeImpl pruned(List<FunctionTypeAliasElement> prune) => this;
 
+  @override
+  DartType replaceTopAndBottom(TypeProvider typeProvider,
+      {bool isCovariant = true}) {
+    // In theory this should never happen, since we only need to do this
+    // replacement when checking super-boundedness of explicitly-specified
+    // types, or types produced by mixin inference or instantiate-to-bounds, and
+    // the unknown type can't occur in any of those cases.
+    assert(
+        false, 'Attempted to check super-boundedness of a type including "?"');
+    // But just in case it does, behave similar to `dynamic`.
+    if (isCovariant) {
+      return typeProvider.nullType;
+    } else {
+      return this;
+    }
+  }
+
   @override
   DartType substitute2(
       List<DartType> argumentTypes, List<DartType> parameterTypes,

diff --git a/pkg/analyzer/test/generated/compile_time_error_code_test.dart b/pkg/analyzer/test/generated/compile_time_error_code_test.dart
@@ -3159,6 +3159,23 @@ class A {
     verify([source]);
   }
 
+  test_instantiate_to_bounds_not_matching_bounds() async {
+    Source source = addSource('''
+class Foo<T> {}
+class Bar<T extends Foo<T>> {}
+class Baz extends Bar {}
+void main() {}
+''');
+    var result = await computeAnalysisResult(source);
+    // Instantiate-to-bounds should have instantiated "Bar" to "Bar<Foo>"
+    expect(result.unit.declaredElement.getType('Baz').supertype.toString(),
+        'Bar<Foo<dynamic>>');
+    // Therefore there should be an error, since Bar's type argument T is Foo,
+    // which doesn't extends Foo<T>.
+    assertErrors(
+        source, [StaticTypeWarningCode.TYPE_ARGUMENT_NOT_MATCHING_BOUNDS]);
+  }
+
   test_instantiateEnum_const() async {
     Source source = addSource(r'''
 enum E { ONE }

diff --git a/pkg/analyzer/test/src/task/strong/inferred_type_test.dart b/pkg/analyzer/test/src/task/strong/inferred_type_test.dart
@@ -486,7 +486,8 @@ void main() {
   }
 
   test_constructors_inferenceFBounded() async {
-    var errors = 'error:COULD_NOT_INFER,error:COULD_NOT_INFER';
+    var errors = 'error:COULD_NOT_INFER,error:COULD_NOT_INFER,'
+        'error:TYPE_ARGUMENT_NOT_MATCHING_BOUNDS';
 //    if (hasExtraTaskModelPass) errors = '$errors,$errors';
     var unit = await checkFile('''
 class Clonable<T> {}
@@ -550,7 +551,8 @@ class NotA {}
 NotA myF() => null;
 
 main() {
-  var x = /*info:INFERRED_TYPE_ALLOCATION*/new /*error:COULD_NOT_INFER*/C(myF);
+  var x = /*info:INFERRED_TYPE_ALLOCATION*/new
+      /*error:COULD_NOT_INFER,error:TYPE_ARGUMENT_NOT_MATCHING_BOUNDS*/C(myF);
 }
 ''');
     var x = findLocalVariable(unit, 'x');

diff --git a/tests/co19_2/co19_2-analyzer.status b/tests/co19_2/co19_2-analyzer.status
@@ -160,6 +160,14 @@ LanguageFeatures/Instantiate-to-bound/Callable/Callable_param_extends_neg_assign
 LanguageFeatures/Instantiate-to-bound/Callable/Callable_param_extends_neg_assign_l1_t10: MissingCompileTimeError # Issue 34087
 LanguageFeatures/Instantiate-to-bound/Callable/Callable_param_extends_neg_assign_l1_t11: MissingCompileTimeError # Issue 34087
 LanguageFeatures/Instantiate-to-bound/Callable/Callable_param_extends_neg_assign_l1_t12: MissingCompileTimeError # Issue 34087
+LanguageFeatures/Instantiate-to-bound/Callable/Callable_ret_extends_neg_assign_l1_t03: MissingCompileTimeError
+LanguageFeatures/Instantiate-to-bound/Callable/Callable_ret_extends_neg_assign_l1_t04: MissingCompileTimeError
+LanguageFeatures/Instantiate-to-bound/Callable/Callable_ret_extends_neg_assign_l1_t05: MissingCompileTimeError
+LanguageFeatures/Instantiate-to-bound/Callable/Callable_ret_extends_neg_assign_l1_t06: MissingCompileTimeError
+LanguageFeatures/Instantiate-to-bound/Callable/Callable_ret_extends_neg_assign_l1_t07: MissingCompileTimeError
+LanguageFeatures/Instantiate-to-bound/Callable/Callable_ret_extends_neg_assign_l1_t08: MissingCompileTimeError
+LanguageFeatures/Instantiate-to-bound/Callable/Callable_ret_extends_neg_assign_l1_t09: MissingCompileTimeError
+LanguageFeatures/Instantiate-to-bound/Callable/Callable_ret_extends_neg_assign_l1_t10: MissingCompileTimeError
 LanguageFeatures/Instantiate-to-bound/Callable/Callable_ret_extends_neg_assign_l1_t12: MissingCompileTimeError # Issue 34087
 LanguageFeatures/Instantiate-to-bound/Callable/Callable_ret_extends_neg_assign_l1_t13: MissingCompileTimeError # Issue 34087
 LanguageFeatures/Instantiate-to-bound/Callable/Callable_ret_extends_neg_assign_l1_t14: MissingCompileTimeError # Issue 34087
@@ -179,18 +187,23 @@ LanguageFeatures/Instantiate-to-bound/class/custom_extends_neg_assign_l4_t05: Mi
 LanguageFeatures/Instantiate-to-bound/class/custom_extends_neg_assign_l4_t06: MissingCompileTimeError # Issue 33152, 33477
 LanguageFeatures/Instantiate-to-bound/class/custom_extends_neg_assign_l4_t07: MissingCompileTimeError # Issue 33152, 33477
 LanguageFeatures/Instantiate-to-bound/function/function_named_extends_neg_assign_l1_t10: CompileTimeError # Issue 33995
-LanguageFeatures/Instantiate-to-bound/function/function_named_extends_pos_l1_t04: CompileTimeError # Issue 33995
 LanguageFeatures/Instantiate-to-bound/function/function_named_extends_pos_l1_t05: CompileTimeError # Issue 33995
 LanguageFeatures/Instantiate-to-bound/function/function_named_extends_pos_l1_t06: CompileTimeError # Issue 33995
 LanguageFeatures/Instantiate-to-bound/function/function_optparam_extends_neg_assign_l1_t10: CompileTimeError # Issue 33995
-LanguageFeatures/Instantiate-to-bound/function/function_optparam_extends_pos_l1_t04: CompileTimeError # Issue 33995
 LanguageFeatures/Instantiate-to-bound/function/function_optparam_extends_pos_l1_t05: CompileTimeError # Issue 33995
 LanguageFeatures/Instantiate-to-bound/function/function_optparam_extends_pos_l1_t06: CompileTimeError # Issue 33995
 LanguageFeatures/Instantiate-to-bound/function/function_param_extends_neg_assign_l1_t10: CompileTimeError # Issue 33995
-LanguageFeatures/Instantiate-to-bound/function/function_param_extends_pos_l1_t04: CompileTimeError # Issue 33995
 LanguageFeatures/Instantiate-to-bound/function/function_param_extends_pos_l1_t05: CompileTimeError # Issue 33995
 LanguageFeatures/Instantiate-to-bound/function/function_pos_l1_t01: CompileTimeError # Issue 33995
 LanguageFeatures/Instantiate-to-bound/function/function_ret_extends_neg_assign_l1_t01: MissingCompileTimeError # Issue 33995
+LanguageFeatures/Instantiate-to-bound/function/function_ret_extends_neg_assign_l1_t02: MissingCompileTimeError
+LanguageFeatures/Instantiate-to-bound/function/function_ret_extends_neg_assign_l1_t03: MissingCompileTimeError
+LanguageFeatures/Instantiate-to-bound/function/function_ret_extends_neg_assign_l1_t04: MissingCompileTimeError
+LanguageFeatures/Instantiate-to-bound/function/function_ret_extends_neg_assign_l1_t05: MissingCompileTimeError
+LanguageFeatures/Instantiate-to-bound/function/function_ret_extends_neg_assign_l1_t06: MissingCompileTimeError
+LanguageFeatures/Instantiate-to-bound/function/function_ret_extends_neg_assign_l1_t07: MissingCompileTimeError
+LanguageFeatures/Instantiate-to-bound/function/function_ret_extends_neg_assign_l1_t08: MissingCompileTimeError
+LanguageFeatures/Instantiate-to-bound/function/function_ret_extends_neg_assign_l1_t09: MissingCompileTimeError
 LanguageFeatures/Instantiate-to-bound/function/function_ret_extends_neg_assign_l1_t19: CompileTimeError # Issue 33995
 LanguageFeatures/Instantiate-to-bound/interface/interface_neg_assign_l2_t01: MissingCompileTimeError # Issue 33995
 LanguageFeatures/Instantiate-to-bound/interface/interface_neg_assign_l2_t02: MissingCompileTimeError # Issue 33308
@@ -207,17 +220,8 @@ LanguageFeatures/Instantiate-to-bound/typedef/typedef_param_extends_neg_l2_t01:
 LanguageFeatures/Instantiate-to-bound/typedef/typedef_ret_extends_neg_l1_t02: Crash # Issue 33599
 LanguageFeatures/Instantiate-to-bound/typedef/typedef_ret_extends_neg_l1_t04: MissingCompileTimeError # Issue 33597
 LanguageFeatures/Instantiate-to-bound/typedef/typedef_ret_extends_neg_l2_t01: Crash # Issue 33599
-LanguageFeatures/Super-bounded-types/motivation_example_A01_t01: CompileTimeError # Issue 32903
-LanguageFeatures/Super-bounded-types/motivation_example_A01_t03: CompileTimeError # Issue 32903
-LanguageFeatures/Super-bounded-types/motivation_example_A01_t05: CompileTimeError # Issue 32903
-LanguageFeatures/Super-bounded-types/static_analysis_A01_t01: CompileTimeError # Issue 32903
 LanguageFeatures/Super-bounded-types/static_analysis_A01_t02: CompileTimeError # Issue 32903
-LanguageFeatures/Super-bounded-types/static_analysis_A01_t03: CompileTimeError # Issue 32903
-LanguageFeatures/Super-bounded-types/static_analysis_A01_t04: CompileTimeError # Issue 32903
 LanguageFeatures/Super-bounded-types/static_analysis_A01_t05: CompileTimeError # Issue 32903
-LanguageFeatures/Super-bounded-types/static_analysis_A01_t06: CompileTimeError # Issue 32903
-LanguageFeatures/Super-bounded-types/static_analysis_A01_t07: CompileTimeError # Issue 32903
 LanguageFeatures/Super-bounded-types/static_analysis_A01_t08: CompileTimeError # Issue 32903
-LanguageFeatures/Super-bounded-types/static_analysis_A01_t09: CompileTimeError # Issue 32903
 LanguageFeatures/int-to-double/min_acceptable_values_t01: CompileTimeError # https://github.com/dart-lang/co19/issues/161
 LanguageFeatures/int-to-double/representation_t01: CompileTimeError # https://github.com/dart-lang/co19/issues/160
diff --git a/tests/language_2/issue34532_test.dart b/tests/language_2/issue34532_test.dart
@@ -0,0 +1,12 @@
+// Copyright (c) 2018, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+class Foo<T> {}
+
+class Bar<T extends Foo<T>> {}
+
+// Should be error here, because Bar completes to Bar<Foo>
+class Baz extends /*@compile-error=unspecified*/ Bar {}
+
+void main() {}