Improve detection of conflicts with inherited members and setters
Change-Id: Ifb2fc2ca2e3d812a8e2a56e4fc43c8cfd02f15d5
Reviewed-on: https://dart-review.googlesource.com/c/88942
Reviewed-by: Dmitry Stefantsov <dmitryas@google.com>
diff --git a/pkg/front_end/lib/src/fasta/kernel/class_hierarchy_builder.dart b/pkg/front_end/lib/src/fasta/kernel/class_hierarchy_builder.dart
index 5ee5ae1..89f5f51 100644
--- a/pkg/front_end/lib/src/fasta/kernel/class_hierarchy_builder.dart
+++ b/pkg/front_end/lib/src/fasta/kernel/class_hierarchy_builder.dart
@@ -54,9 +54,12 @@
/// Language Specification](
/// ../../../../../../docs/language/dartLangSpec.tex#classMemberConflicts).
bool isInheritanceConflict(Declaration a, Declaration b) {
+ if (memberKind(a.target) == memberKind(b.target)) return false;
if (a.isField) return !(b.isField || b.isGetter || b.isSetter);
if (b.isField) return !(a.isField || a.isGetter || a.isSetter);
- return memberKind(a.target) != memberKind(b.target);
+ if (a.isSetter) return !(b.isGetter || b.isSetter);
+ if (b.isSetter) return !(a.isGetter || a.isSetter);
+ return true;
}
bool impliesSetter(Declaration declaration) {
@@ -96,7 +99,8 @@
reportInheritanceConflict(cls, a, b);
}
Declaration result = a;
- if (mergeKind == MergeKind.interfaces) {
+ if (mergeKind == MergeKind.accessors) {
+ } else if (mergeKind == MergeKind.interfaces) {
// TODO(ahe): Combine the signatures of a and b. See the section named
// "Combined Member Signatures" in [Dart Programming Language
// Specification](
@@ -211,17 +215,38 @@
// [localSetters] makes sense. It depends on what semantic checks we need
// to perform with respect to static members and inherited members with the
// same name.
+
+ /// Members (excluding setters) declared in [cls].
List<Declaration> localMembers =
new List<Declaration>.from(scope.local.values)
..sort(compareDeclarations);
+
+ /// Setters declared in [cls].
List<Declaration> localSetters =
new List<Declaration>.from(scope.setters.values)
..sort(compareDeclarations);
+
+ // Add implied setters from fields in [localMembers].
localSetters = mergeAccessors(cls, localMembers, localSetters);
+
+ /// Members (excluding setters) declared in [cls] or its superclasses. This
+ /// includes static methods of [cls], but not its superclasses.
List<Declaration> classMembers;
+
+ /// Setters declared in [cls] or its superclasses. This includes static
+ /// setters of [cls], but not its superclasses.
List<Declaration> classSetters;
+
+ /// Members (excluding setters) inherited from interfaces. This contains no static
+ /// members. Is null if no interfaces are implemented by this class or its
+ /// superclasses.
List<Declaration> interfaceMembers;
+
+ /// Setters inherited from interfaces. This contains no static setters. Is
+ /// null if no interfaces are implemented by this class or its
+ /// superclasses.
List<Declaration> interfaceSetters;
+
if (supernode == null) {
// This should be Object.
classMembers = localMembers;
@@ -231,6 +256,16 @@
cls, localMembers, supernode.classMembers, MergeKind.superclass);
classSetters = merge(
cls, localSetters, supernode.classSetters, MergeKind.superclass);
+
+ // Check if local members conflict with inherited setters. This check has
+ // already been performed in the superclass, so we only need to check the
+ // local members.
+ merge(cls, localMembers, classSetters, MergeKind.accessors);
+
+ // Check if local setters conflict with inherited members. As above, we
+ // only need to check the local setters.
+ merge(cls, localSetters, classMembers, MergeKind.accessors);
+
List<KernelTypeBuilder> interfaces = cls.interfaces;
if (interfaces != null) {
MergeResult result = mergeInterfaces(cls, supernode, interfaces);
@@ -243,10 +278,18 @@
if (interfaceMembers != null) {
interfaceMembers =
merge(cls, classMembers, interfaceMembers, MergeKind.supertypes);
+
+ // Check if class setters conflict with members inherited from
+ // interfaces.
+ merge(cls, classSetters, interfaceMembers, MergeKind.accessors);
}
- if (interfaceMembers != null) {
+ if (interfaceSetters != null) {
interfaceSetters =
merge(cls, classSetters, interfaceSetters, MergeKind.supertypes);
+
+ // Check if class members conflict with setters inherited from
+ // interfaces.
+ merge(cls, classMembers, interfaceSetters, MergeKind.accessors);
}
}
nodes[cls] = new ClassHierarchyNode(cls, scope, classMembers, classSetters,
@@ -483,6 +526,8 @@
}
j++;
}
+ if (aList.isEmpty && storeIndex == bList.length) return bList;
+ if (bList.isEmpty && storeIndex == aList.length) return aList;
return result..length = storeIndex;
}
}
@@ -536,4 +581,8 @@
/// Merging class members with interface members.
supertypes,
+
+ /// Merging members with inherited setters, or setters with inherited
+ /// members.
+ accessors,
}
diff --git a/pkg/front_end/testcases/abstract_members.dart.legacy.expect b/pkg/front_end/testcases/abstract_members.dart.legacy.expect
index dd5cc31..867d44f 100644
--- a/pkg/front_end/testcases/abstract_members.dart.legacy.expect
+++ b/pkg/front_end/testcases/abstract_members.dart.legacy.expect
@@ -100,6 +100,26 @@
// var interfaceMethod1;
// ^^^^^^^^^^^^^^^^
//
+// pkg/front_end/testcases/abstract_members.dart:72:16: Error: Can't inherit members that conflict with each other.
+// abstract class F extends E implements Interface1 {}
+// ^
+// pkg/front_end/testcases/abstract_members.dart:67:12: Context: This is one inherited member.
+// void set interfaceMethod1(_) {}
+// ^^^^^^^^^^^^^^^^
+// pkg/front_end/testcases/abstract_members.dart:6:8: Context: This is the other inherited member.
+// void interfaceMethod1() {}
+// ^^^^^^^^^^^^^^^^
+//
+// pkg/front_end/testcases/abstract_members.dart:84:16: Error: Can't inherit members that conflict with each other.
+// abstract class H extends G implements Foo {}
+// ^
+// pkg/front_end/testcases/abstract_members.dart:79:12: Context: This is one inherited member.
+// void set foo(_) {}
+// ^^^
+// pkg/front_end/testcases/abstract_members.dart:75:8: Context: This is the other inherited member.
+// void foo() {}
+// ^^^
+//
// pkg/front_end/testcases/abstract_members.dart:96:16: Error: Can't inherit members that conflict with each other.
// abstract class J extends I implements Bar {}
// ^
@@ -155,6 +175,14 @@
// abstract class D extends C implements Interface2 {}
// ^
//
+// pkg/front_end/testcases/abstract_members.dart:72:16: Error: Can't inherit members that conflict with each other.
+// abstract class F extends E implements Interface1 {}
+// ^
+//
+// pkg/front_end/testcases/abstract_members.dart:84:16: Error: Can't inherit members that conflict with each other.
+// abstract class H extends G implements Foo {}
+// ^
+//
// pkg/front_end/testcases/abstract_members.dart:96:16: Error: Can't inherit members that conflict with each other.
// abstract class J extends I implements Bar {}
// ^
diff --git a/pkg/front_end/testcases/abstract_members.dart.legacy.transformed.expect b/pkg/front_end/testcases/abstract_members.dart.legacy.transformed.expect
index 22ee2e5..9c3e181 100644
--- a/pkg/front_end/testcases/abstract_members.dart.legacy.transformed.expect
+++ b/pkg/front_end/testcases/abstract_members.dart.legacy.transformed.expect
@@ -43,6 +43,14 @@
// abstract class D extends C implements Interface2 {}
// ^
//
+// pkg/front_end/testcases/abstract_members.dart:72:16: Error: Can't inherit members that conflict with each other.
+// abstract class F extends E implements Interface1 {}
+// ^
+//
+// pkg/front_end/testcases/abstract_members.dart:84:16: Error: Can't inherit members that conflict with each other.
+// abstract class H extends G implements Foo {}
+// ^
+//
// pkg/front_end/testcases/abstract_members.dart:96:16: Error: Can't inherit members that conflict with each other.
// abstract class J extends I implements Bar {}
// ^
diff --git a/pkg/front_end/testcases/abstract_members.dart.outline.expect b/pkg/front_end/testcases/abstract_members.dart.outline.expect
index a352fbd..9ef4ea2 100644
--- a/pkg/front_end/testcases/abstract_members.dart.outline.expect
+++ b/pkg/front_end/testcases/abstract_members.dart.outline.expect
@@ -100,6 +100,26 @@
// var interfaceMethod1;
// ^^^^^^^^^^^^^^^^
//
+// pkg/front_end/testcases/abstract_members.dart:72:16: Error: Can't inherit members that conflict with each other.
+// abstract class F extends E implements Interface1 {}
+// ^
+// pkg/front_end/testcases/abstract_members.dart:67:12: Context: This is one inherited member.
+// void set interfaceMethod1(_) {}
+// ^^^^^^^^^^^^^^^^
+// pkg/front_end/testcases/abstract_members.dart:6:8: Context: This is the other inherited member.
+// void interfaceMethod1() {}
+// ^^^^^^^^^^^^^^^^
+//
+// pkg/front_end/testcases/abstract_members.dart:84:16: Error: Can't inherit members that conflict with each other.
+// abstract class H extends G implements Foo {}
+// ^
+// pkg/front_end/testcases/abstract_members.dart:79:12: Context: This is one inherited member.
+// void set foo(_) {}
+// ^^^
+// pkg/front_end/testcases/abstract_members.dart:75:8: Context: This is the other inherited member.
+// void foo() {}
+// ^^^
+//
// pkg/front_end/testcases/abstract_members.dart:96:16: Error: Can't inherit members that conflict with each other.
// abstract class J extends I implements Bar {}
// ^
diff --git a/pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart.legacy.expect b/pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart.legacy.expect
index 6742c9b..1edaa65 100644
--- a/pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart.legacy.expect
+++ b/pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart.legacy.expect
@@ -6,12 +6,34 @@
// pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart:10:8: Context: Previous declaration of 'foo'.
// void foo(int x) {}
// ^^^
+//
+// pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart:10:8: Error: Can't declare a member that conflicts with an inherited one.
+// void foo(int x) {}
+// ^^^
+// pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart:12:12: Context: This is the inherited member.
+// void set foo(int x);
+// ^^^
+//
+// pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart:12:12: Error: Can't declare a member that conflicts with an inherited one.
+// void set foo(int x);
+// ^^^
+// pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart:10:8: Context: This is the inherited member.
+// void foo(int x) {}
+// ^^^
// Unhandled errors:
//
// pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart:12:12: Error: 'foo' is already declared in this scope.
// void set foo(int x);
// ^^^
+//
+// pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart:10:8: Error: Can't declare a member that conflicts with an inherited one.
+// void foo(int x) {}
+// ^^^
+//
+// pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart:12:12: Error: Can't declare a member that conflicts with an inherited one.
+// void set foo(int x);
+// ^^^
library;
import self as self;
diff --git a/pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart.legacy.transformed.expect b/pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart.legacy.transformed.expect
index e9e7677..882dc86 100644
--- a/pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart.legacy.transformed.expect
+++ b/pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart.legacy.transformed.expect
@@ -3,6 +3,14 @@
// pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart:12:12: Error: 'foo' is already declared in this scope.
// void set foo(int x);
// ^^^
+//
+// pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart:10:8: Error: Can't declare a member that conflicts with an inherited one.
+// void foo(int x) {}
+// ^^^
+//
+// pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart:12:12: Error: Can't declare a member that conflicts with an inherited one.
+// void set foo(int x);
+// ^^^
library;
import self as self;
diff --git a/pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart.outline.expect b/pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart.outline.expect
index d8c8b38..fbde11f 100644
--- a/pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart.outline.expect
+++ b/pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart.outline.expect
@@ -6,6 +6,20 @@
// pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart:10:8: Context: Previous declaration of 'foo'.
// void foo(int x) {}
// ^^^
+//
+// pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart:10:8: Error: Can't declare a member that conflicts with an inherited one.
+// void foo(int x) {}
+// ^^^
+// pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart:12:12: Context: This is the inherited member.
+// void set foo(int x);
+// ^^^
+//
+// pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart:12:12: Error: Can't declare a member that conflicts with an inherited one.
+// void set foo(int x);
+// ^^^
+// pkg/front_end/testcases/no_such_method_forwarders/setter_not_shadowed_by_method.dart:10:8: Context: This is the inherited member.
+// void foo(int x) {}
+// ^^^
library;
import self as self;
diff --git a/pkg/front_end/testcases/rasta/super.dart.legacy.expect b/pkg/front_end/testcases/rasta/super.dart.legacy.expect
index 473cece..f359ff5 100644
--- a/pkg/front_end/testcases/rasta/super.dart.legacy.expect
+++ b/pkg/front_end/testcases/rasta/super.dart.legacy.expect
@@ -7,6 +7,20 @@
// void n() {}
// ^
//
+// pkg/front_end/testcases/rasta/super.dart:26:8: Error: Can't declare a member that conflicts with an inherited one.
+// void n() {}
+// ^
+// pkg/front_end/testcases/rasta/super.dart:27:7: Context: This is the inherited member.
+// set n(_) {}
+// ^
+//
+// pkg/front_end/testcases/rasta/super.dart:27:7: Error: Can't declare a member that conflicts with an inherited one.
+// set n(_) {}
+// ^
+// pkg/front_end/testcases/rasta/super.dart:26:8: Context: This is the inherited member.
+// void n() {}
+// ^
+//
// pkg/front_end/testcases/rasta/super.dart:43:5: Error: '+' is not a prefix operator.
// Try removing '+'.
// +super;
@@ -243,6 +257,14 @@
// set n(_) {}
// ^
//
+// pkg/front_end/testcases/rasta/super.dart:26:8: Error: Can't declare a member that conflicts with an inherited one.
+// void n() {}
+// ^
+//
+// pkg/front_end/testcases/rasta/super.dart:27:7: Error: Can't declare a member that conflicts with an inherited one.
+// set n(_) {}
+// ^
+//
// pkg/front_end/testcases/rasta/super.dart:43:5: Error: '+' is not a prefix operator.
// Try removing '+'.
// +super;
diff --git a/pkg/front_end/testcases/rasta/super.dart.legacy.transformed.expect b/pkg/front_end/testcases/rasta/super.dart.legacy.transformed.expect
index f92047d..3228f16 100644
--- a/pkg/front_end/testcases/rasta/super.dart.legacy.transformed.expect
+++ b/pkg/front_end/testcases/rasta/super.dart.legacy.transformed.expect
@@ -4,6 +4,14 @@
// set n(_) {}
// ^
//
+// pkg/front_end/testcases/rasta/super.dart:26:8: Error: Can't declare a member that conflicts with an inherited one.
+// void n() {}
+// ^
+//
+// pkg/front_end/testcases/rasta/super.dart:27:7: Error: Can't declare a member that conflicts with an inherited one.
+// set n(_) {}
+// ^
+//
// pkg/front_end/testcases/rasta/super.dart:43:5: Error: '+' is not a prefix operator.
// Try removing '+'.
// +super;
diff --git a/pkg/front_end/testcases/rasta/super.dart.outline.expect b/pkg/front_end/testcases/rasta/super.dart.outline.expect
index c3496aa..0203b77 100644
--- a/pkg/front_end/testcases/rasta/super.dart.outline.expect
+++ b/pkg/front_end/testcases/rasta/super.dart.outline.expect
@@ -6,6 +6,20 @@
// pkg/front_end/testcases/rasta/super.dart:26:8: Context: Previous declaration of 'n'.
// void n() {}
// ^
+//
+// pkg/front_end/testcases/rasta/super.dart:26:8: Error: Can't declare a member that conflicts with an inherited one.
+// void n() {}
+// ^
+// pkg/front_end/testcases/rasta/super.dart:27:7: Context: This is the inherited member.
+// set n(_) {}
+// ^
+//
+// pkg/front_end/testcases/rasta/super.dart:27:7: Error: Can't declare a member that conflicts with an inherited one.
+// set n(_) {}
+// ^
+// pkg/front_end/testcases/rasta/super.dart:26:8: Context: This is the inherited member.
+// void n() {}
+// ^
library;
import self as self;
