Augment. Report recursiveInterfaceInheritance on the clause, if self-reference.
Report direct recursive interface inheritance diagnostics on the inheritance clause that introduces the cycle, rather than on the class or mixin name. This gives a more precise target for self-references in extends, implements, on, and with clauses. Track recursive inheritance reporting per interface element across fragments so that augmentation clauses can produce the specific diagnostic when they introduce the cycle. Defer the generic cycle diagnostic while earlier fragments still have later augmentations to inspect, and use the element target as the fallback location for indirect cycles. Update diagnostic expectations to match the new locations and cover augmentation and part-file cases for recursive extends, implements, on, and with clauses. Change-Id: I480a56e4b766d704c290d67d9ca4f6a73a2f655b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/507300 Reviewed-by: Paul Berry <paulberry@google.com>
This commit is contained in:
@@ -12,7 +12,6 @@ import 'package:analyzer/source/source.dart';
|
||||
import 'package:analyzer/src/dart/ast/ast.dart';
|
||||
import 'package:analyzer/src/dart/ast/extensions.dart';
|
||||
import 'package:analyzer/src/dart/element/element.dart';
|
||||
import 'package:analyzer/src/dart/element/extensions.dart';
|
||||
import 'package:analyzer/src/dart/element/inheritance_manager3.dart';
|
||||
import 'package:analyzer/src/dart/element/type.dart';
|
||||
import 'package:analyzer/src/dart/element/type_system.dart';
|
||||
@@ -79,6 +78,7 @@ class InheritanceOverrideVerifier {
|
||||
withClause: declaration.withClause,
|
||||
interfaceElementState: interfaceElementState(fragment.element),
|
||||
reportInterfaceConflicts: _reportInterfaceConflicts,
|
||||
targetForElement: _targetForElement,
|
||||
);
|
||||
} else if (declaration is ClassTypeAliasImpl) {
|
||||
var fragment = declaration.declaredFragment!;
|
||||
@@ -99,6 +99,7 @@ class InheritanceOverrideVerifier {
|
||||
withClause: declaration.withClause,
|
||||
interfaceElementState: interfaceElementState(fragment.element),
|
||||
reportInterfaceConflicts: _reportInterfaceConflicts,
|
||||
targetForElement: _targetForElement,
|
||||
);
|
||||
} else if (declaration is EnumDeclarationImpl) {
|
||||
var fragment = declaration.declaredFragment!;
|
||||
@@ -119,6 +120,7 @@ class InheritanceOverrideVerifier {
|
||||
withClause: declaration.withClause,
|
||||
interfaceElementState: interfaceElementState(fragment.element),
|
||||
reportInterfaceConflicts: _reportInterfaceConflicts,
|
||||
targetForElement: _targetForElement,
|
||||
);
|
||||
} else if (declaration is MixinDeclarationImpl) {
|
||||
var fragment = declaration.declaredFragment!;
|
||||
@@ -139,6 +141,7 @@ class InheritanceOverrideVerifier {
|
||||
onClause: declaration.onClause,
|
||||
interfaceElementState: interfaceElementState(fragment.element),
|
||||
reportInterfaceConflicts: _reportInterfaceConflicts,
|
||||
targetForElement: _targetForElement,
|
||||
);
|
||||
} else {
|
||||
continue;
|
||||
@@ -265,9 +268,10 @@ class _ClassVerifier {
|
||||
final MixinOnClause? onClause;
|
||||
final NamedType? superclass;
|
||||
final WithClause? withClause;
|
||||
final _InterfaceElementState? interfaceElementState;
|
||||
final _InterfaceElementState interfaceElementState;
|
||||
final void Function(InterfaceElementImpl element, Interface interface)
|
||||
reportInterfaceConflicts;
|
||||
final _DiagnosticTarget? Function(Element element) targetForElement;
|
||||
|
||||
final List<InterfaceType> directSuperInterfaces = [];
|
||||
|
||||
@@ -295,8 +299,9 @@ class _ClassVerifier {
|
||||
this.onClause,
|
||||
this.superclass,
|
||||
this.withClause,
|
||||
this.interfaceElementState,
|
||||
required this.interfaceElementState,
|
||||
required this.reportInterfaceConflicts,
|
||||
required this.targetForElement,
|
||||
}) : libraryUri = library.uri;
|
||||
|
||||
/// Verify inheritance overrides, and return `true` if an error was
|
||||
@@ -349,7 +354,7 @@ class _ClassVerifier {
|
||||
// So, here we skip corresponding nodes to keep the index in sync.
|
||||
if (mixinType is InterfaceTypeImpl &&
|
||||
isInterfaceTypeInterface(mixinType)) {
|
||||
var index = interfaceElementState!.mixinIndex++;
|
||||
var index = interfaceElementState.mixinIndex++;
|
||||
_checkDeclaredMembers(node, mixinType, mixinIndex: index);
|
||||
directSuperInterfaces.add(mixinType);
|
||||
}
|
||||
@@ -672,6 +677,10 @@ class _ClassVerifier {
|
||||
/// [diag.recursiveInterfaceInheritanceOn],
|
||||
/// [diag.recursiveInterfaceInheritanceWith].
|
||||
bool _checkForRecursiveInterfaceInheritance(InterfaceElementImpl element) {
|
||||
if (interfaceElementState.hasReportedRecursiveInterfaceInheritance) {
|
||||
return true;
|
||||
}
|
||||
|
||||
var cycle = element.interfaceCycle;
|
||||
if (cycle == null) {
|
||||
return false;
|
||||
@@ -682,8 +691,9 @@ class _ClassVerifier {
|
||||
reporter.report(
|
||||
diag.recursiveInterfaceInheritanceExtends
|
||||
.withArguments(className: element.displayName)
|
||||
.atSourceRange(element.diagnosticRange(diagnosticSource)),
|
||||
.at(superclass),
|
||||
);
|
||||
interfaceElementState.hasReportedRecursiveInterfaceInheritance = true;
|
||||
return true;
|
||||
}
|
||||
}
|
||||
@@ -694,8 +704,9 @@ class _ClassVerifier {
|
||||
reporter.report(
|
||||
diag.recursiveInterfaceInheritanceOn
|
||||
.withArguments(mixinName: element.displayName)
|
||||
.atSourceRange(element.diagnosticRange(diagnosticSource)),
|
||||
.at(typeAnnotation),
|
||||
);
|
||||
interfaceElementState.hasReportedRecursiveInterfaceInheritance = true;
|
||||
return true;
|
||||
}
|
||||
}
|
||||
@@ -707,8 +718,9 @@ class _ClassVerifier {
|
||||
reporter.report(
|
||||
diag.recursiveInterfaceInheritanceWith
|
||||
.withArguments(className: element.displayName)
|
||||
.atSourceRange(element.diagnosticRange(diagnosticSource)),
|
||||
.at(typeAnnotation),
|
||||
);
|
||||
interfaceElementState.hasReportedRecursiveInterfaceInheritance = true;
|
||||
return true;
|
||||
}
|
||||
}
|
||||
@@ -720,21 +732,27 @@ class _ClassVerifier {
|
||||
reporter.report(
|
||||
diag.recursiveInterfaceInheritanceImplements
|
||||
.withArguments(className: element.displayName)
|
||||
.atSourceRange(element.diagnosticRange(diagnosticSource)),
|
||||
.at(typeAnnotation),
|
||||
);
|
||||
interfaceElementState.hasReportedRecursiveInterfaceInheritance = true;
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
reporter.report(
|
||||
diag.recursiveInterfaceInheritance
|
||||
.withArguments(
|
||||
className: element.displayName,
|
||||
loop: cycle.map((e) => e.displayName).join(', '),
|
||||
)
|
||||
.atSourceRange(classElement.diagnosticRange(diagnosticSource)),
|
||||
// Earlier fragments can see cycles from clauses in later augmentations.
|
||||
// Wait for those clauses before reporting the generic cycle.
|
||||
if (classFragment.nextFragment != null) {
|
||||
return true;
|
||||
}
|
||||
|
||||
targetForElement(element)?.report(
|
||||
diag.recursiveInterfaceInheritance.withArguments(
|
||||
className: element.displayName,
|
||||
loop: cycle.map((e) => e.displayName).join(', '),
|
||||
),
|
||||
);
|
||||
interfaceElementState.hasReportedRecursiveInterfaceInheritance = true;
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -1129,6 +1147,8 @@ class _DiagnosticTarget {
|
||||
|
||||
/// Maintains an [InterfaceElementImpl]'s mixin index across multiple fragments.
|
||||
class _InterfaceElementState {
|
||||
bool hasReportedRecursiveInterfaceInheritance = false;
|
||||
|
||||
int mixinIndex = 0;
|
||||
|
||||
_InterfaceElementState();
|
||||
|
||||
+48
-3
@@ -20,7 +20,7 @@ class RecursiveInterfaceInheritanceExtendsTest
|
||||
test_class() async {
|
||||
await resolveTestCodeWithDiagnostics(r'''
|
||||
class A extends A {}
|
||||
// ^
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritanceExtends] 'A' can't extend itself.
|
||||
''');
|
||||
}
|
||||
@@ -28,7 +28,7 @@ class A extends A {}
|
||||
test_class_abstract() async {
|
||||
await resolveTestCodeWithDiagnostics(r'''
|
||||
class C extends C {
|
||||
// ^
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritanceExtends] 'C' can't extend itself.
|
||||
var foo = 0;
|
||||
bar();
|
||||
@@ -36,11 +36,56 @@ class C extends C {
|
||||
''');
|
||||
}
|
||||
|
||||
@SkippedTest() // TODO(scheglov): implement augmentation
|
||||
test_class_inAugmentation() async {
|
||||
await resolveTestCodeWithDiagnostics(r'''
|
||||
class A {}
|
||||
augment class A extends A {}
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritanceExtends] 'A' can't extend itself.
|
||||
''');
|
||||
}
|
||||
|
||||
test_class_inAugmentation_part() async {
|
||||
var a = getFile('$testPackageLibPath/a.dart');
|
||||
var b = getFile('$testPackageLibPath/b.dart');
|
||||
|
||||
await resolveFilesWithDiagnostics({
|
||||
a: r'''
|
||||
part 'b.dart';
|
||||
|
||||
class A {}
|
||||
''',
|
||||
b: r'''
|
||||
part of 'a.dart';
|
||||
|
||||
augment class A extends A {}
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritanceExtends] 'A' can't extend itself.
|
||||
''',
|
||||
});
|
||||
}
|
||||
|
||||
test_class_inAugmentation_part_indirect() async {
|
||||
var a = getFile('$testPackageLibPath/a.dart');
|
||||
var b = getFile('$testPackageLibPath/b.dart');
|
||||
|
||||
await resolveFilesWithDiagnostics({
|
||||
a: r'''
|
||||
part 'b.dart';
|
||||
|
||||
class A {}
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritance] 'A' can't be a superinterface of itself: B, A.
|
||||
|
||||
class B extends A {}
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritance] 'B' can't be a superinterface of itself: B, A.
|
||||
''',
|
||||
b: r'''
|
||||
part of 'a.dart';
|
||||
|
||||
augment class A extends B {}
|
||||
''',
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
+49
-4
@@ -20,23 +20,68 @@ class RecursiveInterfaceInheritanceImplementsTest
|
||||
test_class() async {
|
||||
await resolveTestCodeWithDiagnostics(r'''
|
||||
class A implements A {}
|
||||
// ^
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritanceImplements] 'A' can't implement itself.
|
||||
''');
|
||||
}
|
||||
|
||||
@SkippedTest() // TODO(scheglov): implement augmentation
|
||||
test_class_inAugmentation() async {
|
||||
await resolveTestCodeWithDiagnostics(r'''
|
||||
class A {}
|
||||
augment class A implements A {}
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritanceImplements] 'A' can't implement itself.
|
||||
''');
|
||||
}
|
||||
|
||||
test_class_inAugmentation_part() async {
|
||||
var a = getFile('$testPackageLibPath/a.dart');
|
||||
var b = getFile('$testPackageLibPath/b.dart');
|
||||
|
||||
await resolveFilesWithDiagnostics({
|
||||
a: r'''
|
||||
part 'b.dart';
|
||||
|
||||
class A {}
|
||||
''',
|
||||
b: r'''
|
||||
part of 'a.dart';
|
||||
|
||||
augment class A implements A {}
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritanceImplements] 'A' can't implement itself.
|
||||
''',
|
||||
});
|
||||
}
|
||||
|
||||
test_class_inAugmentation_part_indirect() async {
|
||||
var a = getFile('$testPackageLibPath/a.dart');
|
||||
var b = getFile('$testPackageLibPath/b.dart');
|
||||
|
||||
await resolveFilesWithDiagnostics({
|
||||
a: r'''
|
||||
part 'b.dart';
|
||||
|
||||
class A {}
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritance] 'A' can't be a superinterface of itself: B, A.
|
||||
|
||||
class B implements A {}
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritance] 'B' can't be a superinterface of itself: B, A.
|
||||
''',
|
||||
b: r'''
|
||||
part of 'a.dart';
|
||||
|
||||
augment class A implements B {}
|
||||
''',
|
||||
});
|
||||
}
|
||||
|
||||
test_class_tail() async {
|
||||
await resolveTestCodeWithDiagnostics(r'''
|
||||
abstract class A implements A {}
|
||||
// ^
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritanceImplements] 'A' can't implement itself.
|
||||
class B implements A {}
|
||||
''');
|
||||
@@ -47,7 +92,7 @@ class B implements A {}
|
||||
class A {}
|
||||
mixin M {}
|
||||
class B = A with M implements B;
|
||||
// ^
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritanceImplements] 'B' can't implement itself.
|
||||
''');
|
||||
}
|
||||
|
||||
@@ -19,19 +19,64 @@ class RecursiveInterfaceInheritanceOnTest extends PubPackageResolutionTest {
|
||||
test_1() async {
|
||||
await resolveTestCodeWithDiagnostics(r'''
|
||||
mixin A on A {}
|
||||
// ^
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritanceOn] 'A' can't use itself as a superclass constraint.
|
||||
''');
|
||||
}
|
||||
|
||||
@SkippedTest() // TODO(scheglov): implement augmentation
|
||||
test_1_inAugmentation() async {
|
||||
await resolveTestCodeWithDiagnostics(r'''
|
||||
mixin A {}
|
||||
augment mixin A on A {}
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritanceOn] 'A' can't use itself as a superclass constraint.
|
||||
''');
|
||||
}
|
||||
|
||||
test_1_inAugmentation_part() async {
|
||||
var a = getFile('$testPackageLibPath/a.dart');
|
||||
var b = getFile('$testPackageLibPath/b.dart');
|
||||
|
||||
await resolveFilesWithDiagnostics({
|
||||
a: r'''
|
||||
part 'b.dart';
|
||||
|
||||
mixin A {}
|
||||
''',
|
||||
b: r'''
|
||||
part of 'a.dart';
|
||||
|
||||
augment mixin A on A {}
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritanceOn] 'A' can't use itself as a superclass constraint.
|
||||
''',
|
||||
});
|
||||
}
|
||||
|
||||
test_1_inAugmentation_part_indirect() async {
|
||||
var a = getFile('$testPackageLibPath/a.dart');
|
||||
var b = getFile('$testPackageLibPath/b.dart');
|
||||
|
||||
await resolveFilesWithDiagnostics({
|
||||
a: r'''
|
||||
part 'b.dart';
|
||||
|
||||
mixin A {}
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritance] 'A' can't be a superinterface of itself: B, A.
|
||||
|
||||
mixin B on A {}
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritance] 'B' can't be a superinterface of itself: B, A.
|
||||
''',
|
||||
b: r'''
|
||||
part of 'a.dart';
|
||||
|
||||
augment mixin A on B {}
|
||||
''',
|
||||
});
|
||||
}
|
||||
|
||||
test_2() async {
|
||||
await resolveTestCodeWithDiagnostics(r'''
|
||||
mixin A on B {}
|
||||
|
||||
@@ -16,18 +16,65 @@ main() {
|
||||
|
||||
@reflectiveTest
|
||||
class RecursiveInterfaceInheritanceWithTest extends PubPackageResolutionTest {
|
||||
@SkippedTest() // TODO(scheglov): implement augmentation
|
||||
test_class_inAugmentation() async {
|
||||
await resolveTestCodeWithDiagnostics(r'''
|
||||
class A extends Object {}
|
||||
augment class A with A {}
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritanceWith] 'A' can't use itself as a mixin.
|
||||
// [diag.classUsedAsMixin] The class 'A' can't be used as a mixin because it's neither a mixin class nor a mixin.
|
||||
''');
|
||||
}
|
||||
|
||||
test_class_inAugmentation_part() async {
|
||||
var a = getFile('$testPackageLibPath/a.dart');
|
||||
var b = getFile('$testPackageLibPath/b.dart');
|
||||
|
||||
await resolveFilesWithDiagnostics({
|
||||
a: r'''
|
||||
part 'b.dart';
|
||||
|
||||
class A extends Object {}
|
||||
''',
|
||||
b: r'''
|
||||
part of 'a.dart';
|
||||
|
||||
augment class A with A {}
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritanceWith] 'A' can't use itself as a mixin.
|
||||
// [diag.classUsedAsMixin] The class 'A' can't be used as a mixin because it's neither a mixin class nor a mixin.
|
||||
''',
|
||||
});
|
||||
}
|
||||
|
||||
test_class_inAugmentation_part_indirect() async {
|
||||
var a = getFile('$testPackageLibPath/a.dart');
|
||||
var b = getFile('$testPackageLibPath/b.dart');
|
||||
|
||||
await resolveFilesWithDiagnostics({
|
||||
a: r'''
|
||||
part 'b.dart';
|
||||
|
||||
class A {}
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritance] 'A' can't be a superinterface of itself: B, A.
|
||||
|
||||
mixin class B implements A {}
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritance] 'B' can't be a superinterface of itself: B, A.
|
||||
''',
|
||||
b: r'''
|
||||
part of 'a.dart';
|
||||
|
||||
augment class A with B {}
|
||||
''',
|
||||
});
|
||||
}
|
||||
|
||||
test_classTypeAlias() async {
|
||||
await resolveTestCodeWithDiagnostics(r'''
|
||||
mixin class M = Object with M;
|
||||
// ^
|
||||
// ^
|
||||
// [diag.recursiveInterfaceInheritanceWith] 'M' can't use itself as a mixin.
|
||||
''');
|
||||
}
|
||||
|
||||
@@ -5,8 +5,9 @@
|
||||
// Test that class with a cyclic hierarchy doesn't cause a loop in dart2js.
|
||||
|
||||
class A extends A {
|
||||
// ^
|
||||
// ^
|
||||
// [analyzer] COMPILE_TIME_ERROR.RECURSIVE_INTERFACE_INHERITANCE
|
||||
// ^
|
||||
// [cfe] 'A' is a supertype of itself.
|
||||
|
||||
// When checking that foo isn't overriding an instance method in the
|
||||
|
||||
@@ -12,15 +12,16 @@ mixin M<T> {}
|
||||
|
||||
class C1 = S with M;
|
||||
class C2 = S with C2;
|
||||
// ^^
|
||||
// [analyzer] COMPILE_TIME_ERROR.RECURSIVE_INTERFACE_INHERITANCE
|
||||
// [cfe] 'C2' is a supertype of itself.
|
||||
// ^^
|
||||
// [analyzer] COMPILE_TIME_ERROR.RECURSIVE_INTERFACE_INHERITANCE
|
||||
// [analyzer] COMPILE_TIME_ERROR.CLASS_USED_AS_MIXIN
|
||||
// ^^
|
||||
// [cfe] 'C2' is a supertype of itself.
|
||||
class C3 = S with M implements A;
|
||||
class C4 = S with M implements C4;
|
||||
// ^^
|
||||
// ^^
|
||||
// [analyzer] COMPILE_TIME_ERROR.RECURSIVE_INTERFACE_INHERITANCE
|
||||
// ^^
|
||||
// [cfe] 'C4' is a supertype of itself.
|
||||
|
||||
void main() {
|
||||
|
||||
@@ -5,16 +5,18 @@
|
||||
class M {}
|
||||
|
||||
mixin class M0 extends Object with M0 {}
|
||||
// ^^
|
||||
// ^^
|
||||
// [analyzer] COMPILE_TIME_ERROR.RECURSIVE_INTERFACE_INHERITANCE
|
||||
// ^^
|
||||
// [cfe] 'M0' is a supertype of itself.
|
||||
// [cfe] 'Object with M0' is a supertype of itself.
|
||||
// ^^^^^^^
|
||||
// [analyzer] COMPILE_TIME_ERROR.MIXIN_CLASS_DECLARATION_WITH_CLAUSE
|
||||
|
||||
mixin class M1 = Object with M1;
|
||||
// ^^
|
||||
// ^^
|
||||
// [analyzer] COMPILE_TIME_ERROR.RECURSIVE_INTERFACE_INHERITANCE
|
||||
// ^^
|
||||
// [cfe] 'M1' is a supertype of itself.
|
||||
|
||||
mixin class M2 = Object with M3;
|
||||
|
||||
@@ -14,14 +14,16 @@ class M<T> {}
|
||||
|
||||
class C1 = S with M;
|
||||
class C2 = S with C2;
|
||||
// ^^
|
||||
// ^^
|
||||
// [analyzer] COMPILE_TIME_ERROR.RECURSIVE_INTERFACE_INHERITANCE
|
||||
// ^^
|
||||
// [cfe] 'C2' is a supertype of itself.
|
||||
|
||||
class C3 = S with M implements A;
|
||||
class C4 = S with M implements C4;
|
||||
// ^^
|
||||
// ^^
|
||||
// [analyzer] COMPILE_TIME_ERROR.RECURSIVE_INTERFACE_INHERITANCE
|
||||
// ^^
|
||||
// [cfe] 'C4' is a supertype of itself.
|
||||
|
||||
void main() {
|
||||
|
||||
@@ -7,14 +7,16 @@
|
||||
class M {}
|
||||
|
||||
class M0 extends Object with M0 {}
|
||||
// ^^
|
||||
// ^^
|
||||
// [analyzer] COMPILE_TIME_ERROR.RECURSIVE_INTERFACE_INHERITANCE
|
||||
// ^^
|
||||
// [cfe] 'M0' is a supertype of itself.
|
||||
// [cfe] 'Object with M0' is a supertype of itself.
|
||||
|
||||
class M1 = Object with M1;
|
||||
// ^^
|
||||
// ^^
|
||||
// [analyzer] COMPILE_TIME_ERROR.RECURSIVE_INTERFACE_INHERITANCE
|
||||
// ^^
|
||||
// [cfe] 'M1' is a supertype of itself.
|
||||
|
||||
class M2 = Object with M3;
|
||||
|
||||
Reference in New Issue
Block a user