[fasta] Correct override check for methods with covariant parameters

Bug: #32613

Change-Id: Id69e793afd2c933294299d796a28b66db6b5de17
Reviewed-on: https://dart-review.googlesource.com/64840
Commit-Queue: Dmitry Stefantsov <dmitryas@google.com>
Reviewed-by: Erik Ernst <eernst@google.com>
Reviewed-by: Dmitry Stefantsov <dmitryas@google.com>
Reviewed-by: Peter von der Ahé <ahe@google.com>

pkg/front_end/lib/src/fasta/kernel/kernel_class_builder.dart

@@ -289,17 +289,39 @@ abstract class KernelClassBuilder
 
   void checkOverrides(
       ClassHierarchy hierarchy, TypeEnvironment typeEnvironment) {
-    hierarchy.forEachOverridePair(cls,
-        (Member declaredMember, Member interfaceMember, bool isSetter) {
+    handleSeenCovariant(
+        Member declaredMember,
+        Member interfaceMember,
+        bool isSetter,
+        callback(
+            Member declaredMember, Member interfaceMember, bool isSetter)) {
+      // When a parameter is covariant we have to check that we also
+      // override the same member in all parents.
+      for (Supertype supertype in interfaceMember.enclosingClass.supers) {
+        Member m = hierarchy.getInterfaceMember(
+            supertype.classNode, interfaceMember.name,
+            setter: isSetter);
+        if (m != null) {
+          callback(declaredMember, m, isSetter);
+        }
+      }
+    }
+
+    overridePairCallback(
+        Member declaredMember, Member interfaceMember, bool isSetter) {
       if (declaredMember is Constructor || interfaceMember is Constructor) {
         unimplemented("Constructor in override check.",
             declaredMember.fileOffset, fileUri);
       }
       if (declaredMember is Procedure && interfaceMember is Procedure) {
         if (declaredMember.kind == ProcedureKind.Method &&
             interfaceMember.kind == ProcedureKind.Method) {
-          checkMethodOverride(
+          bool seenCovariant = checkMethodOverride(
               hierarchy, typeEnvironment, declaredMember, interfaceMember);
+          if (seenCovariant) {
+            handleSeenCovariant(declaredMember, interfaceMember, isSetter,
+                overridePairCallback);
+          }
         }
         if (declaredMember.kind == ProcedureKind.Getter &&
             interfaceMember.kind == ProcedureKind.Getter) {
@@ -308,8 +330,12 @@ abstract class KernelClassBuilder
         }
         if (declaredMember.kind == ProcedureKind.Setter &&
             interfaceMember.kind == ProcedureKind.Setter) {
-          checkSetterOverride(
+          bool seenCovariant = checkSetterOverride(
               hierarchy, typeEnvironment, declaredMember, interfaceMember);
+          if (seenCovariant) {
+            handleSeenCovariant(declaredMember, interfaceMember, isSetter,
+                overridePairCallback);
+          }
         }
       } else {
         bool declaredMemberHasGetter = declaredMember is Field ||
@@ -324,12 +350,18 @@ abstract class KernelClassBuilder
           checkGetterOverride(
               hierarchy, typeEnvironment, declaredMember, interfaceMember);
         } else if (declaredMemberHasSetter && interfaceMemberHasSetter) {
-          checkSetterOverride(
+          bool seenCovariant = checkSetterOverride(
               hierarchy, typeEnvironment, declaredMember, interfaceMember);
+          if (seenCovariant) {
+            handleSeenCovariant(declaredMember, interfaceMember, isSetter,
+                overridePairCallback);
+          }
         }
       }
       // TODO(ahe): Handle other cases: accessors, operators, and fields.
-    });
+    }
+
+    hierarchy.forEachOverridePair(cls, overridePairCallback);
   }
 
   void checkAbstractMembers(CoreTypes coreTypes, ClassHierarchy hierarchy) {
@@ -728,18 +760,21 @@ abstract class KernelClassBuilder
     return false;
   }
 
-  void checkMethodOverride(
+  /// Returns whether a covariant parameter was seen and more methods thus have
+  /// to be checked.
+  bool checkMethodOverride(
       ClassHierarchy hierarchy,
       TypeEnvironment typeEnvironment,
       Procedure declaredMember,
       Procedure interfaceMember) {
     if (declaredMember.enclosingClass != cls) {
       // TODO(ahe): Include these checks as well, but the message needs to
       // explain that [declaredMember] is inherited.
-      return;
+      return false;
     }
     assert(declaredMember.kind == ProcedureKind.Method);
     assert(interfaceMember.kind == ProcedureKind.Method);
+    bool seenCovariant = false;
     FunctionNode declaredFunction = declaredMember.function;
     FunctionNode interfaceFunction = interfaceMember.function;
 
@@ -807,10 +842,11 @@ abstract class KernelClassBuilder
           interfaceFunction.positionalParameters[i].type,
           declaredParameter.isCovariant,
           declaredParameter);
+      if (declaredParameter.isCovariant) seenCovariant = true;
     }
     if (declaredFunction.namedParameters.isEmpty &&
         interfaceFunction.namedParameters.isEmpty) {
-      return;
+      return seenCovariant;
     }
     if (declaredFunction.namedParameters.length <
         interfaceFunction.namedParameters.length) {
@@ -875,7 +911,9 @@ abstract class KernelClassBuilder
           interfaceNamedParameters.current.type,
           declaredParameter.isCovariant,
           declaredParameter);
+      if (declaredParameter.isCovariant) seenCovariant = true;
     }
+    return seenCovariant;
   }
 
   void checkGetterOverride(
@@ -896,15 +934,17 @@ abstract class KernelClassBuilder
         interfaceMember, declaredType, interfaceType, false, null);
   }
 
-  void checkSetterOverride(
+  /// Returns whether a covariant parameter was seen and more methods thus have
+  /// to be checked.
+  bool checkSetterOverride(
       ClassHierarchy hierarchy,
       TypeEnvironment typeEnvironment,
       Member declaredMember,
       Member interfaceMember) {
     if (declaredMember.enclosingClass != cls) {
       // TODO(paulberry): Include these checks as well, but the message needs to
       // explain that [declaredMember] is inherited.
-      return;
+      return false;
     }
     Substitution interfaceSubstitution = _computeInterfaceSubstitution(
         hierarchy, declaredMember, interfaceMember, null, null);
@@ -924,6 +964,7 @@ abstract class KernelClassBuilder
         isCovariant,
         declaredParameter,
         asIfDeclaredParameter: true);
+    return isCovariant;
   }
 
   String get fullNameForErrors {

tests/language_2/language_2_dart2js.status

@@ -598,8 +598,6 @@ identical_const_test/03: MissingCompileTimeError
 identical_const_test/04: MissingCompileTimeError
 issue18628_2_test/01: MissingCompileTimeError
 issue23244_test: Crash # 'file:*/pkg/compiler/lib/src/js_emitter/runtime_type_generator.dart': Failed assertion: line 208 pos 18: '!(_useKernel && _strongMode && !_disableRtiOptimization) ||
-issue31596_override_test/07: MissingCompileTimeError
-issue31596_override_test/08: MissingCompileTimeError
 library_env_test/has_no_html_support: RuntimeError
 library_env_test/has_no_io_support: RuntimeError
 malbounded_instantiation_test/01: MissingCompileTimeError
@@ -791,8 +789,6 @@ invocation_mirror2_test: RuntimeError # mirrors not supported
 invocation_mirror_test: RuntimeError
 issue18628_2_test/01: MissingCompileTimeError
 issue23244_test: Crash # Interpolated value #1 is not an Expression or List of Expressions: [VariableUse(f), Instance of 'LiteralNull', null]
-issue31596_override_test/07: MissingCompileTimeError
-issue31596_override_test/08: MissingCompileTimeError
 library_env_test/has_no_html_support: RuntimeError
 library_env_test/has_no_io_support: RuntimeError
 malbounded_instantiation_test/01: MissingCompileTimeError

tests/language_2/language_2_dartdevc.status

@@ -397,8 +397,6 @@ implicit_creation/implicit_const_not_default_values_test/e6: Pass
 implicit_creation/implicit_const_not_default_values_test/e9: Pass
 instantiate_tearoff_of_call_test: CompileTimeError
 issue18628_2_test/01: MissingCompileTimeError
-issue31596_override_test/07: MissingCompileTimeError
-issue31596_override_test/08: MissingCompileTimeError
 issue31596_super_test/01: CompileTimeError
 issue31596_super_test/02: MissingCompileTimeError
 issue31596_super_test/03: CompileTimeError

tests/language_2/language_2_kernel.status

@@ -255,8 +255,6 @@ field3_test/01: MissingCompileTimeError # Issue 33022
 field3_test/02: MissingCompileTimeError # Issue 33022
 generic_methods_bounds_test/01: MissingCompileTimeError # Issue 33018
 generic_methods_recursive_bound_test/02: MissingCompileTimeError # Issue 33018
-issue31596_override_test/07: MissingCompileTimeError
-issue31596_override_test/08: MissingCompileTimeError
 issue31596_super_test/02: MissingCompileTimeError
 issue31596_super_test/04: MissingCompileTimeError
 malbounded_instantiation_test/01: MissingCompileTimeError # Issue 33018