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);
+}