Fix #29014, make function type parameters invariant.
With the restricted behavior, some tests have been removed, modified,
and added based on the new invariance.
Bug: 29014
Change-Id: I086749bbe40aaa723009f9ae06f58d85554cae53
Reviewed-on: https://dart-review.googlesource.com/56029
Commit-Queue: Mike Fairhurst <mfairhurst@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
diff --git a/pkg/analyzer/lib/src/dart/element/type.dart b/pkg/analyzer/lib/src/dart/element/type.dart
index 5d91528..3ccdac9 100644
--- a/pkg/analyzer/lib/src/dart/element/type.dart
+++ b/pkg/analyzer/lib/src/dart/element/type.dart
@@ -1175,8 +1175,8 @@
/**
* Compares two function types [t] and [s] to see if their corresponding
- * parameter types match [parameterRelation] and their return types match
- * [returnRelation].
+ * parameter types match [parameterRelation], return types match
+ * [returnRelation], and type parameter bounds match [boundsRelation].
*
* Used for the various relations on function types which have the same
* structural rules for handling optional parameters and arity, but use their
@@ -1184,14 +1184,20 @@
*
* If [parameterRelation] is omitted, uses [returnRelation] for both. This
* is convenient for Dart 1 type system methods.
+ *
+ * If [boundsRelation] is omitted, uses [returnRelation]. This is for
+ * backwards compatibility, and convenience for Dart 1 type system methods.
*/
static bool relate(
FunctionType t,
DartType other,
bool returnRelation(DartType t, DartType s),
DartType instantiateToBounds(DartType t),
- {bool parameterRelation(ParameterElement t, ParameterElement s)}) {
+ {bool parameterRelation(ParameterElement t, ParameterElement s),
+ bool boundsRelation(DartType bound2, DartType bound1,
+ TypeParameterElement formal2, TypeParameterElement formal1)}) {
parameterRelation ??= (t, s) => returnRelation(t.type, s.type);
+ boundsRelation ??= (t, s, _, __) => returnRelation(t, s);
// Trivial base cases.
if (other == null) {
@@ -1208,8 +1214,7 @@
// This type cast is safe, because we checked it above.
FunctionType s = other as FunctionType;
if (t.typeFormals.isNotEmpty) {
- List<DartType> freshVariables =
- relateTypeFormals(t, s, (s, t, _, __) => returnRelation(s, t));
+ List<DartType> freshVariables = relateTypeFormals(t, s, boundsRelation);
if (freshVariables == null) {
return false;
}
diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart
index 185d6d3..91509f1 100644
--- a/pkg/analyzer/lib/src/generated/resolver.dart
+++ b/pkg/analyzer/lib/src/generated/resolver.dart
@@ -6748,6 +6748,9 @@
*/
void _inferFunctionExpressionParametersTypes(
Expression mayBeClosure, DartType mayByFunctionType) {
+ // TODO(mfairhurst): remove this code and callers. It's doing
+ // "propagated type" inference for the Dart 1 type system.
+ assert(!strongMode);
// prepare closure
if (mayBeClosure is! FunctionExpression) {
return;
diff --git a/pkg/analyzer/lib/src/generated/type_system.dart b/pkg/analyzer/lib/src/generated/type_system.dart
index d178f25..ab376a1 100644
--- a/pkg/analyzer/lib/src/generated/type_system.dart
+++ b/pkg/analyzer/lib/src/generated/type_system.dart
@@ -646,18 +646,14 @@
if (t1 is FunctionType && t2 is FunctionType) {
return FunctionTypeImpl.relate(
- t1,
- t2,
- (t1, t2) {
- // TODO(jmesserly): should we flip covariance when we're relating
- // type formal bounds? They're more like parameters.
- return matchSubtype(t1, t2);
- },
- _typeSystem.instantiateToBounds,
+ t1, t2, matchSubtype, _typeSystem.instantiateToBounds,
parameterRelation: (p1, p2) {
return _matchSubtypeOf(p2.type, p1.type, null, origin,
covariant: !covariant);
- });
+ },
+ // Type parameter bounds are invariant.
+ boundsRelation: (t1, t2, p1, p2) =>
+ matchSubtype(t1, t2) && matchSubtype(t2, t1));
}
if (t1 is FunctionType && t2 == typeProvider.functionType) {
@@ -1181,7 +1177,10 @@
/// - it allows opt-in covariant parameters.
bool isOverrideSubtypeOf(FunctionType f1, FunctionType f2) {
return FunctionTypeImpl.relate(f1, f2, isSubtypeOf, instantiateToBounds,
- parameterRelation: isOverrideSubtypeOfParameter);
+ parameterRelation: isOverrideSubtypeOfParameter,
+ // Type parameter bounds are invariant.
+ boundsRelation: (t1, t2, p1, p2) =>
+ isSubtypeOf(t1, t2) && isSubtypeOf(t2, t1));
}
/// Check that parameter [p2] is a subtype of [p1], given that we are
@@ -1522,7 +1521,10 @@
/// Check that [f1] is a subtype of [f2].
bool _isFunctionSubtypeOf(FunctionType f1, FunctionType f2) {
return FunctionTypeImpl.relate(f1, f2, isSubtypeOf, instantiateToBounds,
- parameterRelation: (p1, p2) => isSubtypeOf(p2.type, p1.type));
+ parameterRelation: (p1, p2) => isSubtypeOf(p2.type, p1.type),
+ // Type parameter bounds are invariant.
+ boundsRelation: (t1, t2, p1, p2) =>
+ isSubtypeOf(t1, t2) && isSubtypeOf(t2, t1));
}
bool _isInterfaceSubtypeOf(
diff --git a/pkg/analyzer/test/generated/strong_mode_kernel_test.dart b/pkg/analyzer/test/generated/strong_mode_kernel_test.dart
index fdd3388..763d120 100644
--- a/pkg/analyzer/test/generated/strong_mode_kernel_test.dart
+++ b/pkg/analyzer/test/generated/strong_mode_kernel_test.dart
@@ -357,9 +357,18 @@
@override
@failingTest
- test_genericMethod_override_invalidTypeParamBounds() async {
+ test_genericMethod_override_invalidCovariantTypeParamBounds() async {
// Expected 1 errors of type StrongModeCode.STRONG_MODE_INVALID_METHOD_OVERRIDE, found 0
- await super.test_genericMethod_override_invalidTypeParamBounds();
+ await super
+ .test_genericMethod_override_invalidContravariantTypeParamBounds();
+ }
+
+ @override
+ @failingTest
+ test_genericMethod_override_invalidContravariantTypeParamBounds() async {
+ // Expected 1 errors of type StrongModeCode.STRONG_MODE_INVALID_METHOD_OVERRIDE, found 0
+ await super
+ .test_genericMethod_override_invalidContravariantTypeParamBounds();
}
@override
diff --git a/pkg/analyzer/test/generated/strong_mode_test.dart b/pkg/analyzer/test/generated/strong_mode_test.dart
index 6a0a0b3..ab24845 100644
--- a/pkg/analyzer/test/generated/strong_mode_test.dart
+++ b/pkg/analyzer/test/generated/strong_mode_test.dart
@@ -3407,13 +3407,17 @@
test_genericMethod_override_bounds() async {
await resolveTestUnit(r'''
class A {}
-class B extends A {}
-class C {
- T f<T extends B>(T x) => null;
-}
-class D extends C {
+class B {
T f<T extends A>(T x) => null;
}
+// override with the same bound is OK
+class C extends B {
+ T f<T extends A>(T x) => null;
+}
+// override with new name and the same bound is OK
+class D extends B {
+ Q f<Q extends A>(Q x) => null;
+}
''');
}
@@ -3433,20 +3437,27 @@
verify([source]);
}
- test_genericMethod_override_invalidReturnType() async {
+ test_genericMethod_override_differentContextsSameBounds() async {
Source source = addSource(r'''
-class C {
- Iterable<T> f<T>(T x) => null;
+ class GenericMethodBounds<T> {
+ Type get t => T;
+ GenericMethodBounds<E> foo<E extends T>() => new GenericMethodBounds<E>();
+ GenericMethodBounds<E> bar<E extends void Function(T)>() =>
+ new GenericMethodBounds<E>();
}
-class D extends C {
- String f<S>(S x) => null;
-}''');
+
+class GenericMethodBoundsDerived extends GenericMethodBounds<num> {
+ GenericMethodBounds<E> foo<E extends num>() => new GenericMethodBounds<E>();
+ GenericMethodBounds<E> bar<E extends void Function(num)>() =>
+ new GenericMethodBounds<E>();
+}
+''');
await computeAnalysisResult(source);
- assertErrors(source, [StrongModeCode.INVALID_METHOD_OVERRIDE]);
+ assertNoErrors(source);
verify([source]);
}
- test_genericMethod_override_invalidTypeParamBounds() async {
+ test_genericMethod_override_invalidContravariantTypeParamBounds() async {
Source source = addSource(r'''
class A {}
class B extends A {}
@@ -3461,6 +3472,34 @@
verify([source]);
}
+ test_genericMethod_override_invalidCovariantTypeParamBounds() async {
+ Source source = addSource(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;
+}''');
+ await computeAnalysisResult(source);
+ assertErrors(source, [StrongModeCode.INVALID_METHOD_OVERRIDE]);
+ verify([source]);
+ }
+
+ test_genericMethod_override_invalidReturnType() async {
+ Source source = addSource(r'''
+class C {
+ Iterable<T> f<T>(T x) => null;
+}
+class D extends C {
+ String f<S>(S x) => null;
+}''');
+ await computeAnalysisResult(source);
+ assertErrors(source, [StrongModeCode.INVALID_METHOD_OVERRIDE]);
+ verify([source]);
+ }
+
test_genericMethod_override_invalidTypeParamCount() async {
Source source = addSource(r'''
class C {
@@ -3910,6 +3949,48 @@
''');
}
+ @failingTest
+ test_notInstantiatedBound_class_error_recursion() async {
+ String code = r'''
+class A<T extends B> {} // points to a
+class B<T extends A> {} // points to b
+class C<T extends A> {} // points to a cyclical type
+''';
+ await resolveTestUnit(code, noErrors: false);
+ assertErrors(testSource, [
+ StrongModeCode.NOT_INSTANTIATED_BOUND,
+ StrongModeCode.NOT_INSTANTIATED_BOUND,
+ StrongModeCode.NOT_INSTANTIATED_BOUND,
+ ]);
+ }
+
+ @failingTest
+ test_notInstantiatedBound_class_error_recursion_less_direct() async {
+ String code = r'''
+class A<T extends B<A>> {}
+class B<T extends A<B>> {}
+''';
+ await resolveTestUnit(code, noErrors: false);
+ assertErrors(testSource, [
+ StrongModeCode.NOT_INSTANTIATED_BOUND,
+ StrongModeCode.NOT_INSTANTIATED_BOUND,
+ ]);
+ }
+
+ test_notInstantiatedBound_class_error_recursion_typedef() async {
+ String code = r'''
+typedef F(C value);
+class C<T extends F> {}
+class D<T extends C> {}
+''';
+ await resolveTestUnit(code, noErrors: false);
+ assertErrors(testSource, [
+ StrongModeCode.NOT_INSTANTIATED_BOUND,
+ StrongModeCode.NOT_INSTANTIATED_BOUND,
+ CompileTimeErrorCode.TYPE_ALIAS_CANNOT_REFERENCE_ITSELF,
+ ]);
+ }
+
test_notInstantiatedBound_error_class_argument() async {
String code = r'''
class A<K, V extends List<K>> {}
@@ -3959,34 +4040,6 @@
]);
}
- @failingTest
- test_notInstantiatedBound_class_error_recursion() async {
- String code = r'''
-class A<T extends B> {} // points to a
-class B<T extends A> {} // points to b
-class C<T extends A> {} // points to a cyclical type
-''';
- await resolveTestUnit(code, noErrors: false);
- assertErrors(testSource, [
- StrongModeCode.NOT_INSTANTIATED_BOUND,
- StrongModeCode.NOT_INSTANTIATED_BOUND,
- StrongModeCode.NOT_INSTANTIATED_BOUND,
- ]);
- }
-
- @failingTest
- test_notInstantiatedBound_class_error_recursion_less_direct() async {
- String code = r'''
-class A<T extends B<A>> {}
-class B<T extends A<B>> {}
-''';
- await resolveTestUnit(code, noErrors: false);
- assertErrors(testSource, [
- StrongModeCode.NOT_INSTANTIATED_BOUND,
- StrongModeCode.NOT_INSTANTIATED_BOUND,
- ]);
- }
-
test_notInstantiatedBound_error_typedef_argument() async {
String code = r'''
class A<K, V extends List<K>> {}
@@ -4014,20 +4067,6 @@
assertErrors(testSource, [StrongModeCode.NOT_INSTANTIATED_BOUND]);
}
- test_notInstantiatedBound_class_error_recursion_typedef() async {
- String code = r'''
-typedef F(C value);
-class C<T extends F> {}
-class D<T extends C> {}
-''';
- await resolveTestUnit(code, noErrors: false);
- assertErrors(testSource, [
- StrongModeCode.NOT_INSTANTIATED_BOUND,
- StrongModeCode.NOT_INSTANTIATED_BOUND,
- CompileTimeErrorCode.TYPE_ALIAS_CANNOT_REFERENCE_ITSELF,
- ]);
- }
-
test_notInstantiatedBound_ok_class() async {
String code = r'''
class A<T extends int> {}
@@ -4068,6 +4107,15 @@
assertNoErrors(testSource);
}
+ test_notInstantiatedBound_ok_class_function() async {
+ String code = r'''
+class A<T extends void Function<Z>()> {}
+class B<T extends A> {}
+''';
+ await resolveTestUnit(code);
+ assertNoErrors(testSource);
+ }
+
test_notInstantiatedBound_ok_class_typedef() async {
String code = r'''
typedef void F<T extends int>();
@@ -4086,15 +4134,6 @@
assertNoErrors(testSource);
}
- test_notInstantiatedBound_ok_class_function() async {
- String code = r'''
-class A<T extends void Function<Z>()> {}
-class B<T extends A> {}
-''';
- await resolveTestUnit(code);
- assertNoErrors(testSource);
- }
-
test_objectMethodOnFunctions_Anonymous() async {
String code = r'''
void main() {
@@ -4403,7 +4442,7 @@
}
abstract class Override implements Base, BaseCopy {
- f<E>(x) => null;
+ f<E extends int>(x) => null;
}
class C extends Override implements Base {}
diff --git a/pkg/analyzer/test/generated/type_system_test.dart b/pkg/analyzer/test/generated/type_system_test.dart
index c371ec8..a7ec816 100644
--- a/pkg/analyzer/test/generated/type_system_test.dart
+++ b/pkg/analyzer/test/generated/type_system_test.dart
@@ -2145,28 +2145,6 @@
}
}
- void test_genericFunction_simple_fBounded() {
- ClassElementImpl AClass = ElementFactory.classElement2('A', ["Q"]);
- InterfaceType AType = AClass.type;
- ClassElementImpl BClass = ElementFactory.classElement2('B', ["R"]);
- BClass.supertype = AType.instantiate([BClass.typeParameters[0].type]);
- InterfaceType BType = BClass.type;
-
- DartType s = TypeBuilder.variable("S");
- (s.element as TypeParameterElementImpl).bound = AType.instantiate([s]);
- DartType t = TypeBuilder.variable("T", bound: s);
- DartType u = TypeBuilder.variable("U");
- (u.element as TypeParameterElementImpl).bound = BType.instantiate([u]);
- DartType v = TypeBuilder.variable("V", bound: u);
-
- _checkIsStrictSubtypeOf(
- TypeBuilder.function(types: [s]), TypeBuilder.function(types: [u]));
-
- _checkIsStrictSubtypeOf(
- TypeBuilder.function(types: [s, t], required: [s], result: t),
- TypeBuilder.function(types: [u, v], required: [u], result: v));
- }
-
void test_generics() {
ClassElementImpl LClass = ElementFactory.classElement2('L', ["T"]);
InterfaceType LType = LClass.type;
diff --git a/tests/language_2/language_2_analyzer.status b/tests/language_2/language_2_analyzer.status
index 74be6be..402c681 100644
--- a/tests/language_2/language_2_analyzer.status
+++ b/tests/language_2/language_2_analyzer.status
@@ -399,7 +399,6 @@
generic_methods_bounds_test/01: MissingCompileTimeError
generic_methods_dynamic_test/01: MissingCompileTimeError
generic_methods_dynamic_test/03: MissingCompileTimeError
-generic_methods_overriding_test/01: MissingCompileTimeError
generic_methods_overriding_test/03: MissingCompileTimeError
generic_methods_overriding_test/06: StaticWarning
generic_methods_recursive_bound_test/02: MissingCompileTimeError
@@ -1107,8 +1106,6 @@
field3a_negative_test: StaticWarning # Issue 28823
forwarding_stub_tearoff_test: CompileTimeError
generic_methods_generic_function_result_test/none: CompileTimeError # Issue #30207
-generic_methods_overriding_test/01: MissingCompileTimeError # Issue 29070
-generic_methods_overriding_test/03: MissingCompileTimeError # Issue 29070
generic_no_such_method_dispatcher_test: CompileTimeError
generic_tearoff_test: CompileTimeError
import_core_prefix_test: CompileTimeError # "dynamic" should be defined in core.
diff --git a/tests/language_2/language_2_dartdevc.status b/tests/language_2/language_2_dartdevc.status
index 2bfb404..11456f2 100644
--- a/tests/language_2/language_2_dartdevc.status
+++ b/tests/language_2/language_2_dartdevc.status
@@ -63,7 +63,6 @@
generic_local_functions_test: CompileTimeError
generic_methods_generic_function_parameter_test: CompileTimeError
generic_methods_generic_function_result_test/none: CompileTimeError # Issue #30208
-generic_methods_overriding_test/03: MissingCompileTimeError # Issue 29920
generic_no_such_method_dispatcher_simple_test: Skip # This test is just for kernel.
generic_no_such_method_dispatcher_test: CompileTimeError
getter_closure_execution_order_test: RuntimeError # Issue 29920
@@ -391,6 +390,7 @@
function_type_parameter_negative_test: Fail
generic_function_bounds_test: RuntimeError
generic_methods_generic_function_result_test/01: MissingCompileTimeError
+generic_methods_overriding_test/01: MissingCompileTimeError # Issue 29920
generic_methods_recursive_bound_test/02: MissingCompileTimeError
generic_no_such_method_dispatcher_simple_test: CompileTimeError # Warning: Superclass has no method named 'foo'.
generic_no_such_method_dispatcher_test: CompileTimeError # Issue 31533
@@ -685,7 +685,6 @@
generic_function_type_as_type_argument_test/02: MissingCompileTimeError # Issue 29920
generic_instanceof2_test: RuntimeError # Issue 29920; ReferenceError: FooOfK$String is not defined
generic_is_check_test: RuntimeError # Issue 29920; Expect.isTrue(false) fails.
-generic_methods_overriding_test/01: MissingCompileTimeError # Issue 29920
generic_tearoff_test: CompileTimeError
identical_closure2_test: RuntimeError # Issue 29920; Expect.isFalse(true) fails.
infinite_switch_label_test: RuntimeError # Issue 29920; NoSuchMethodError: method not found: '<Unexpected Null Value>'
