Collect abstract members immediately instead of counting.
This simplifies diagnosing missing implementations, especially, the
ones inherited from interfaces.
Change-Id: I13705eee07960974a111d78f1eb986e77d27a468
Reviewed-on: https://dart-review.googlesource.com/c/88719
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 a6074c2..a6c5dc6 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
@@ -71,7 +71,7 @@
bool hasNoSuchMethod = false;
- int abstractMemberCount = 0;
+ List<Declaration> abstractMembers = null;
ClassHierarchyBuilder(this.objectClass);
@@ -131,7 +131,7 @@
// superclass.
result = b;
} else {
- abstractMemberCount++;
+ (abstractMembers ??= <Declaration>[]).add(a);
}
}
return result;
@@ -146,9 +146,8 @@
/// implementing/overriding anything.
void handleOnlyA(Declaration member, MergeKind mergeKind) {
Member target = member.target;
- if (mergeKind == MergeKind.supertypes ||
- (mergeKind == MergeKind.superclass && target.isAbstract)) {
- abstractMemberCount++;
+ if (mergeKind == MergeKind.superclass && target.isAbstract) {
+ (abstractMembers ??= <Declaration>[]).add(member);
}
}
@@ -165,7 +164,7 @@
if (mergeKind == MergeKind.supertypes ||
(mergeKind == MergeKind.superclass && target.isAbstract)) {
if (isNameVisibleIn(target.name, cls.library)) {
- abstractMemberCount++;
+ (abstractMembers ??= <Declaration>[]).add(member);
}
}
if (member.parent != objectClass &&
@@ -177,7 +176,7 @@
void add(KernelClassBuilder cls) {
assert(!hasNoSuchMethod);
- assert(abstractMemberCount == 0);
+ assert(abstractMembers == null);
if (cls.isPatch) {
// TODO(ahe): What about patch classes. Have we injected patched members
// into the class-builder's scope?
@@ -203,6 +202,10 @@
scope = mixin.scope;
}
}
+ // TODO(ahe): Consider if removing static members from [localMembers] and
+ // [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.
List<Declaration> localMembers =
new List<Declaration>.from(scope.local.values)
..sort(compareDeclarations);
@@ -244,14 +247,15 @@
nodes[cls] = new ClassHierarchyNode(cls, scope, classMembers, classSetters,
interfaceMembers, interfaceSetters);
- if (abstractMemberCount != 0 && !cls.isAbstract) {
+ if (abstractMembers != null && !cls.isAbstract) {
if (!hasNoSuchMethod) {
- reportMissingMembers(cls, classMembers, classSetters);
+ reportMissingMembers(cls);
+ } else {
+ installNsmHandlers(cls);
}
- installNsmHandlers(cls);
}
hasNoSuchMethod = false;
- abstractMemberCount = 0;
+ abstractMembers = null;
}
MergeResult mergeInterfaces(KernelClassBuilder cls,
@@ -367,32 +371,31 @@
}
}
- void reportMissingMembers(KernelClassBuilder cls, List<Declaration> members,
- List<Declaration> setters) {
- List<LocatedMessage> context = <LocatedMessage>[];
- List<String> missingNames = <String>[];
- for (int j = 0; j < 2; j++) {
- List<Declaration> declarations = j == 0 ? members : setters;
- for (int i = 0; i < declarations.length; i++) {
- Declaration declaration = declarations[i];
- Member target = declaration.target;
- if (target.isAbstract && isNameVisibleIn(target.name, cls.library)) {
- String name = declaration.fullNameForErrors;
- String parentName = declaration.parent.fullNameForErrors;
- String displayName =
- declaration.isSetter ? "$parentName.$name=" : "$parentName.$name";
- missingNames.add(displayName);
- context.add(templateMissingImplementationCause
- .withArguments(displayName)
- .withLocation(
- declaration.fileUri, declaration.charOffset, name.length));
- }
+ void reportMissingMembers(KernelClassBuilder cls) {
+ Map<String, LocatedMessage> contextMap = <String, LocatedMessage>{};
+ for (int i = 0; i < abstractMembers.length; i++) {
+ Declaration declaration = abstractMembers[i];
+ Member target = declaration.target;
+ if (isNameVisibleIn(target.name, cls.library)) {
+ String name = declaration.fullNameForErrors;
+ String parentName = declaration.parent.fullNameForErrors;
+ String displayName =
+ declaration.isSetter ? "$parentName.$name=" : "$parentName.$name";
+ contextMap[displayName] = templateMissingImplementationCause
+ .withArguments(displayName)
+ .withLocation(
+ declaration.fileUri, declaration.charOffset, name.length);
}
}
- if (missingNames.isEmpty) return;
+ if (contextMap.isEmpty) return;
+ List<String> names = new List<String>.from(contextMap.keys)..sort();
+ List<LocatedMessage> context = <LocatedMessage>[];
+ for (int i = 0; i < names.length; i++) {
+ context.add(contextMap[names[i]]);
+ }
cls.addProblem(
templateMissingImplementationNotAbstract.withArguments(
- cls.fullNameForErrors, missingNames),
+ cls.fullNameForErrors, names),
cls.charOffset,
cls.fullNameForErrors.length,
context: context);
@@ -409,11 +412,11 @@
if (node == null) {
bool savedHasNoSuchMethod = hasNoSuchMethod;
hasNoSuchMethod = false;
- int savedAbstractMemberCount = abstractMemberCount;
- abstractMemberCount = 0;
+ List<Declaration> savedAbstractMembers = abstractMembers;
+ abstractMembers = null;
add(declaration);
hasNoSuchMethod = savedHasNoSuchMethod;
- abstractMemberCount = savedAbstractMemberCount;
+ abstractMembers = savedAbstractMembers;
node = nodes[declaration];
}
return node;
@@ -430,14 +433,20 @@
final List<Declaration> result = new List<Declaration>.filled(
aList.length + bList.length, null,
growable: true);
-
int storeIndex = 0;
-
int i = 0;
int j = 0;
while (i < aList.length && j < bList.length) {
final Declaration a = aList[i];
final Declaration b = bList[j];
+ if (a.isStatic) {
+ i++;
+ continue;
+ }
+ if (b.isStatic) {
+ j++;
+ continue;
+ }
final int compare = compareDeclarations(a, b);
if (compare == 0) {
result[storeIndex++] = handleMergeConflict(cls, a, b, mergeKind);
@@ -455,14 +464,18 @@
}
while (i < aList.length) {
final Declaration a = aList[i];
- handleOnlyA(a, mergeKind);
- result[storeIndex++] = a;
+ if (!a.isStatic) {
+ handleOnlyA(a, mergeKind);
+ result[storeIndex++] = a;
+ }
i++;
}
while (j < bList.length) {
final Declaration b = bList[j];
- handleOnlyB(cls, b, mergeKind);
- result[storeIndex++] = b;
+ if (!b.isStatic) {
+ handleOnlyB(cls, b, mergeKind);
+ result[storeIndex++] = b;
+ }
j++;
}
return result..length = storeIndex;
diff --git a/pkg/front_end/testcases/abstract_members.dart.legacy.expect b/pkg/front_end/testcases/abstract_members.dart.legacy.expect
index 3ac995d..ae7e4d9 100644
--- a/pkg/front_end/testcases/abstract_members.dart.legacy.expect
+++ b/pkg/front_end/testcases/abstract_members.dart.legacy.expect
@@ -12,8 +12,12 @@
//
// pkg/front_end/testcases/abstract_members.dart:33:7: Error: The non-abstract class 'MyClass' is missing implementations for these members:
// - A.abstractMethod
-// - A.property3=
// - A.property1=
+// - A.property3=
+// - Interface1.interfaceMethod1
+// - Interface2.interfaceMethod1
+// - Interface2.interfaceMethod2
+// - Interface3.interfaceMethod3
// Try to either
// - provide an implementation,
// - inherit an implementation from a superclass or mixin,
@@ -25,12 +29,24 @@
// pkg/front_end/testcases/abstract_members.dart:21:3: Context: 'A.abstractMethod' is defined here.
// abstractMethod();
// ^^^^^^^^^^^^^^
-// pkg/front_end/testcases/abstract_members.dart:24:12: Context: 'A.property3=' is defined here.
-// void set property3(_);
-// ^^^^^^^^^
// pkg/front_end/testcases/abstract_members.dart:22:12: Context: 'A.property1=' is defined here.
// void set property1(_);
// ^^^^^^^^^
+// pkg/front_end/testcases/abstract_members.dart:24:12: Context: 'A.property3=' is defined here.
+// void set property3(_);
+// ^^^^^^^^^
+// pkg/front_end/testcases/abstract_members.dart:6:8: Context: 'Interface1.interfaceMethod1' is defined here.
+// void interfaceMethod1() {}
+// ^^^^^^^^^^^^^^^^
+// pkg/front_end/testcases/abstract_members.dart:12:7: Context: 'Interface2.interfaceMethod1' is defined here.
+// var interfaceMethod1;
+// ^^^^^^^^^^^^^^^^
+// pkg/front_end/testcases/abstract_members.dart:10:8: Context: 'Interface2.interfaceMethod2' is defined here.
+// void interfaceMethod2() {}
+// ^^^^^^^^^^^^^^^^
+// pkg/front_end/testcases/abstract_members.dart:16:8: Context: 'Interface3.interfaceMethod3' is defined here.
+// void interfaceMethod3() {}
+// ^^^^^^^^^^^^^^^^
// Unhandled errors:
//
@@ -40,8 +56,12 @@
//
// pkg/front_end/testcases/abstract_members.dart:33:7: Error: The non-abstract class 'MyClass' is missing implementations for these members:
// - A.abstractMethod
-// - A.property3=
// - A.property1=
+// - A.property3=
+// - Interface1.interfaceMethod1
+// - Interface2.interfaceMethod1
+// - Interface2.interfaceMethod2
+// - Interface3.interfaceMethod3
// Try to either
// - provide an implementation,
// - inherit an implementation from a superclass or mixin,
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 1e1ebac..932cfc2 100644
--- a/pkg/front_end/testcases/abstract_members.dart.legacy.transformed.expect
+++ b/pkg/front_end/testcases/abstract_members.dart.legacy.transformed.expect
@@ -6,8 +6,12 @@
//
// pkg/front_end/testcases/abstract_members.dart:33:7: Error: The non-abstract class 'MyClass' is missing implementations for these members:
// - A.abstractMethod
-// - A.property3=
// - A.property1=
+// - A.property3=
+// - Interface1.interfaceMethod1
+// - Interface2.interfaceMethod1
+// - Interface2.interfaceMethod2
+// - Interface3.interfaceMethod3
// Try to either
// - provide an implementation,
// - inherit an implementation from a superclass or mixin,
diff --git a/pkg/front_end/testcases/abstract_members.dart.outline.expect b/pkg/front_end/testcases/abstract_members.dart.outline.expect
index 1c95548..5b799a3 100644
--- a/pkg/front_end/testcases/abstract_members.dart.outline.expect
+++ b/pkg/front_end/testcases/abstract_members.dart.outline.expect
@@ -12,8 +12,12 @@
//
// pkg/front_end/testcases/abstract_members.dart:33:7: Error: The non-abstract class 'MyClass' is missing implementations for these members:
// - A.abstractMethod
-// - A.property3=
// - A.property1=
+// - A.property3=
+// - Interface1.interfaceMethod1
+// - Interface2.interfaceMethod1
+// - Interface2.interfaceMethod2
+// - Interface3.interfaceMethod3
// Try to either
// - provide an implementation,
// - inherit an implementation from a superclass or mixin,
@@ -25,12 +29,24 @@
// pkg/front_end/testcases/abstract_members.dart:21:3: Context: 'A.abstractMethod' is defined here.
// abstractMethod();
// ^^^^^^^^^^^^^^
-// pkg/front_end/testcases/abstract_members.dart:24:12: Context: 'A.property3=' is defined here.
-// void set property3(_);
-// ^^^^^^^^^
// pkg/front_end/testcases/abstract_members.dart:22:12: Context: 'A.property1=' is defined here.
// void set property1(_);
// ^^^^^^^^^
+// pkg/front_end/testcases/abstract_members.dart:24:12: Context: 'A.property3=' is defined here.
+// void set property3(_);
+// ^^^^^^^^^
+// pkg/front_end/testcases/abstract_members.dart:6:8: Context: 'Interface1.interfaceMethod1' is defined here.
+// void interfaceMethod1() {}
+// ^^^^^^^^^^^^^^^^
+// pkg/front_end/testcases/abstract_members.dart:12:7: Context: 'Interface2.interfaceMethod1' is defined here.
+// var interfaceMethod1;
+// ^^^^^^^^^^^^^^^^
+// pkg/front_end/testcases/abstract_members.dart:10:8: Context: 'Interface2.interfaceMethod2' is defined here.
+// void interfaceMethod2() {}
+// ^^^^^^^^^^^^^^^^
+// pkg/front_end/testcases/abstract_members.dart:16:8: Context: 'Interface3.interfaceMethod3' is defined here.
+// void interfaceMethod3() {}
+// ^^^^^^^^^^^^^^^^
library;
import self as self;
