Fasta: Signature change with and without noSuchMethod

The below code is not okay, even if we add a noSuchMethod to C.

```
class B {
  foo(int x) => x;
}

class C extends B {
  foo(int x, {int y});
}
```

Issue #32660.

Change-Id: I1ace4443c3732fdd5faefa50e0d2cb68023e97c4
Reviewed-on: https://dart-review.googlesource.com/72106
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Peter von der Ahé <ahe@google.com>
diff --git a/pkg/front_end/lib/src/fasta/kernel/kernel_class_builder.dart b/pkg/front_end/lib/src/fasta/kernel/kernel_class_builder.dart
index 6ea64a0..c143b6d 100644
--- a/pkg/front_end/lib/src/fasta/kernel/kernel_class_builder.dart
+++ b/pkg/front_end/lib/src/fasta/kernel/kernel_class_builder.dart
@@ -428,19 +428,14 @@
   }
 
   void checkAbstractMembers(CoreTypes coreTypes, ClassHierarchy hierarchy) {
-    if (isAbstract ||
-        hierarchy.getDispatchTarget(cls, noSuchMethodName).enclosingClass !=
-            coreTypes.objectClass) {
+    if (isAbstract) {
       // Unimplemented members allowed
-      // TODO(dmitryas): Call hasUserDefinedNoSuchMethod instead when ready.
       return;
     }
 
     List<LocatedMessage> context = null;
 
     bool mustHaveImplementation(Member member) {
-      // Forwarding stub
-      if (member is Procedure && member.isSyntheticForwarder) return false;
       // Public member
       if (!member.name.isPrivate) return true;
       // Private member in different library
@@ -451,6 +446,63 @@
       return true;
     }
 
+    bool isValidImplementation(Member interfaceMember, Member dispatchTarget,
+        {bool setters}) {
+      // If they're the exact same it's valid.
+      if (interfaceMember == dispatchTarget) return true;
+
+      if (interfaceMember is Procedure && dispatchTarget is Procedure) {
+        // E.g. getter vs method.
+        if (interfaceMember.kind != dispatchTarget.kind) return false;
+
+        if (dispatchTarget.function.positionalParameters.length <
+                interfaceMember.function.requiredParameterCount ||
+            dispatchTarget.function.positionalParameters.length <
+                interfaceMember.function.positionalParameters.length)
+          return false;
+
+        if (interfaceMember.function.requiredParameterCount <
+            dispatchTarget.function.requiredParameterCount) return false;
+
+        if (dispatchTarget.function.namedParameters.length <
+            interfaceMember.function.namedParameters.length) return false;
+
+        // Two Procedures of the same kind with the same number of parameters.
+        return true;
+      }
+
+      if ((interfaceMember is Field || interfaceMember is Procedure) &&
+          (dispatchTarget is Field || dispatchTarget is Procedure)) {
+        if (setters) {
+          bool interfaceMemberHasSetter =
+              (interfaceMember is Field && interfaceMember.hasSetter) ||
+                  interfaceMember is Procedure && interfaceMember.isSetter;
+          bool dispatchTargetHasSetter =
+              (dispatchTarget is Field && dispatchTarget.hasSetter) ||
+                  dispatchTarget is Procedure && dispatchTarget.isSetter;
+          // Combination of (settable) field and/or (procedure) setter is valid.
+          return interfaceMemberHasSetter && dispatchTargetHasSetter;
+        } else {
+          bool interfaceMemberHasGetter = interfaceMember is Field ||
+              interfaceMember is Procedure && interfaceMember.isGetter;
+          bool dispatchTargetHasGetter = dispatchTarget is Field ||
+              dispatchTarget is Procedure && dispatchTarget.isGetter;
+          // Combination of field and/or (procedure) getter is valid.
+          return interfaceMemberHasGetter && dispatchTargetHasGetter;
+        }
+      }
+
+      unhandled(
+          "${interfaceMember.runtimeType} and ${dispatchTarget.runtimeType}",
+          "isValidImplementation",
+          interfaceMember.fileOffset,
+          interfaceMember.fileUri);
+    }
+
+    bool hasNoSuchMethod =
+        hierarchy.getDispatchTarget(cls, noSuchMethodName).enclosingClass !=
+            coreTypes.objectClass;
+
     void findMissingImplementations({bool setters}) {
       List<Member> dispatchTargets =
           hierarchy.getDispatchTargets(cls, setters: setters);
@@ -464,10 +516,17 @@
                   0) {
             targetIndex++;
           }
-          if (targetIndex >= dispatchTargets.length ||
+          bool foundTarget = targetIndex < dispatchTargets.length &&
               ClassHierarchy.compareMembers(
-                      dispatchTargets[targetIndex], interfaceMember) >
-                  0) {
+                      dispatchTargets[targetIndex], interfaceMember) <=
+                  0;
+          bool hasProblem = true;
+          if (foundTarget &&
+              isValidImplementation(
+                  interfaceMember, dispatchTargets[targetIndex],
+                  setters: setters)) hasProblem = false;
+          if (hasNoSuchMethod && !foundTarget) hasProblem = false;
+          if (hasProblem) {
             Name name = interfaceMember.name;
             String displayName = name.name + (setters ? "=" : "");
             context ??= <LocatedMessage>[];
diff --git a/pkg/front_end/messages.yaml b/pkg/front_end/messages.yaml
index 3aa09c8..98522b3 100644
--- a/pkg/front_end/messages.yaml
+++ b/pkg/front_end/messages.yaml
@@ -1719,7 +1719,7 @@
       abstract class B {
         get foo;
       }
-      class C extends A implements B {}
+      abstract class C extends A implements B {}
     - >-
       class A {
         var foo;
@@ -1727,7 +1727,7 @@
       abstract class B {
         foo();
       }
-      class C extends A implements B {}
+      abstract class C extends A implements B {}
     - >-
       class A {
         get foo => 0;
@@ -1735,7 +1735,7 @@
       abstract class B {
         foo();
       }
-      class C extends A implements B {}
+      abstract class C extends A implements B {}
 
 InheritedMembersConflictCause1:
   template: "This is one inherited member."
diff --git a/tests/language_2/language_2_analyzer.status b/tests/language_2/language_2_analyzer.status
index a6b3e47..432a67f 100644
--- a/tests/language_2/language_2_analyzer.status
+++ b/tests/language_2/language_2_analyzer.status
@@ -117,6 +117,8 @@
 regress_29784_test/02: MissingCompileTimeError # Issue 29784
 regress_30121_test: CompileTimeError # Issue 31087
 regress_30339_test: CompileTimeError
+regress_32660_test/01: MissingCompileTimeError # Issue #32660.
+regress_32660_test/04: CompileTimeError
 regress_33235_03_test/01: MissingCompileTimeError
 regress_33235_07_test/03: MissingCompileTimeError
 regress_33235_10_test/01: MissingCompileTimeError
diff --git a/tests/language_2/language_2_dartdevc.status b/tests/language_2/language_2_dartdevc.status
index e86031a..593a7ce 100644
--- a/tests/language_2/language_2_dartdevc.status
+++ b/tests/language_2/language_2_dartdevc.status
@@ -144,6 +144,8 @@
 regress_29784_test/02: MissingCompileTimeError
 regress_30121_test: CompileTimeError # Issue 31087
 regress_30339_test: CompileTimeError # As expected. Should we make this a multi test?
+regress_32660_test/01: MissingCompileTimeError # Issue #32660.
+regress_32660_test/04: CompileTimeError
 regress_33235_03_test/01: MissingCompileTimeError
 regress_33235_07_test/03: MissingCompileTimeError
 regress_33235_10_test/01: MissingCompileTimeError
diff --git a/tests/language_2/language_2_kernel.status b/tests/language_2/language_2_kernel.status
index 9c854d1..3925076 100644
--- a/tests/language_2/language_2_kernel.status
+++ b/tests/language_2/language_2_kernel.status
@@ -202,10 +202,6 @@
 web_int_literals_test/*: SkipByDesign # Test applies only to JavaScript targets
 
 [ $fasta ]
-abstract_override_adds_optional_args_concrete_subclass_test: MissingCompileTimeError # Issue 32014.
-abstract_override_adds_optional_args_concrete_test: MissingCompileTimeError # Issue 32014.
-additional_interface_adds_optional_args_concrete_subclass_test: MissingCompileTimeError # Issue 32014.
-additional_interface_adds_optional_args_concrete_test: MissingCompileTimeError # Issue 32014.
 async_return_types_test/nestedFuture: MissingCompileTimeError # Issue 33068
 const_cast2_test/01: CompileTimeError # Issue 32517
 const_cast2_test/none: CompileTimeError # Issue 32517
diff --git a/tests/language_2/regress_32660_test.dart b/tests/language_2/regress_32660_test.dart
new file mode 100644
index 0000000..eab8d80
--- /dev/null
+++ b/tests/language_2/regress_32660_test.dart
@@ -0,0 +1,78 @@
+// 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.
+
+// noSuchMethod does not overwrite actual implementations, so if an
+// implemetation of a member exists that doesn't fulfill the interface it's
+// an error.
+// On the other hand, if no implementation exists,
+// noSuchMethod will take its place and everything is okay.
+
+class B {
+  foo(int x, // force formatter to not combine these lines.
+          {int y} //# 02: compile-time error
+          ) =>
+      x;
+}
+
+class C extends B {
+  foo(int x, // force formatter to not combine these lines.
+      {int y} //# 01: compile-time error
+      );
+  bar();
+
+  noSuchMethod(i) {
+    print("No such method!");
+    return 42;
+  }
+}
+
+abstract class D {
+  foo(int x, // force formatter to not combine these lines.
+      {int y} //# 03: ok
+      );
+}
+
+abstract class E {
+  foo(int x, // force formatter to not combine these lines.
+      {int y} //# 04: ok
+      );
+}
+
+class F extends D implements E {
+  noSuchMethod(i) {
+    print("No such method!");
+    return 42;
+  }
+}
+
+class G {
+  foo(int x, // force formatter to not combine these lines.
+          {int y} //# 05: ok
+          ) =>
+      x;
+}
+
+class H {
+  foo(int x, // force formatter to not combine these lines.
+          {int y} //# 06: compile-time error
+          ) =>
+      x;
+}
+
+class I extends G implements H {
+  noSuchMethod(i) {
+    print("No such method: $i!");
+    return 42;
+  }
+}
+
+main() {
+  var c = new C();
+  c.foo(123);
+  c.bar();
+  var f = new F();
+  f.foo(42);
+  var i = new I();
+  i.foo(42);
+}