dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Augment. Report inconsistentInheritanceGetterAndMethod and inconsistentInheritance only on the introductory declaration.

Browse Source 
Change-Id: I6f03031e2540b7d65995eba9608cdaf9651204e4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/507263
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This commit is contained in:
Konstantin Shcheglov
2026-05-29 09:48:16 -07:00
parent 67cada59b7
commit 53f3ecc74d
5 changed files with 273 additions and 106 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart
+15 -11
View File
@@ -86,10 +86,6 @@ class LibraryAnalyzer {
final Map<FileState, FileAnalysis> _libraryFiles = {};
late final LibraryVerificationContext _libraryVerificationContext;
/// One verifier per library so that state of elements can be shared across
/// fragments in all files of this library.
late final InheritanceOverrideVerifier _inheritanceOverrideVerifier;
final TestingData? _testingData;
final TypeSystemOperations _typeSystemOperations;
@@ -115,10 +111,6 @@ class LibraryAnalyzer {
typeSystem: _typeSystem,
),
);
_inheritanceOverrideVerifier = InheritanceOverrideVerifier(
_typeSystem,
_inheritance,
);
}
TypeProviderImpl get _typeProvider => _libraryElement.typeProvider;
@@ -341,8 +333,17 @@ class LibraryAnalyzer {
/// Compute diagnostics in [_libraryFiles], including errors and warnings,
/// lints, and a few other cases.
void _computeDiagnostics() {
var inheritanceOverrideVerifier = InheritanceOverrideVerifier(
_typeSystem,
_inheritance,
diagnosticReportersByFragment: {
for (var fileAnalysis in _libraryFiles.values)
fileAnalysis.fragment: fileAnalysis.diagnosticReporter,
},
);
for (var fileAnalysis in _libraryFiles.values) {
_computeVerifyErrors(fileAnalysis);
_computeVerifyErrors(fileAnalysis, inheritanceOverrideVerifier);
}
MemberDuplicateDefinitionVerifier.checkLibrary(
@@ -469,14 +470,17 @@ class LibraryAnalyzer {
).afterLibrary();
}
void _computeVerifyErrors(FileAnalysis fileAnalysis) {
void _computeVerifyErrors(
FileAnalysis fileAnalysis,
InheritanceOverrideVerifier inheritanceOverrideVerifier,
) {
var diagnosticReporter = fileAnalysis.diagnosticReporter;
var unit = fileAnalysis.unit;
_computeConstantErrors(fileAnalysis);
// Compute inheritance and override errors.
_inheritanceOverrideVerifier.verifyUnit(unit, diagnosticReporter);
inheritanceOverrideVerifier.verifyUnit(unit, diagnosticReporter);
// Use the ErrorVerifier to compute errors.
ErrorVerifier errorVerifier = ErrorVerifier(
pkg/analyzer/lib/src/dart/element/inheritance_manager3.dart
+16 -13
View File
@@ -59,8 +59,8 @@ class ExtensionTypeConflictingStaticAndInstanceConflict extends Conflict {
/// Failure because of a getter and a method from direct superinterfaces.
class GetterMethodConflict extends Conflict {
final InternalExecutableElement getter;
final InternalExecutableElement method;
final InternalGetterElement getter;
final InternalMethodElement method;
GetterMethodConflict({
required super.name,
@@ -381,15 +381,14 @@ class InheritanceManager3 {
) {
assert(candidates.length > 1);
InternalExecutableElement? getter;
InternalExecutableElement? method;
InternalGetterElement? getter;
InternalMethodElement? method;
for (var candidate in candidates) {
var kind = candidate.kind;
if (kind == ElementKind.GETTER) {
getter ??= candidate;
}
if (kind == ElementKind.METHOD) {
method ??= candidate;
switch (candidate) {
case InternalGetterElement():
getter ??= candidate;
case InternalMethodElement():
method ??= candidate;
}
}
@@ -618,12 +617,16 @@ class InheritanceManager3 {
if (candidate.enclosingElement == mixinElement) {
namedCandidates[name] = [candidate];
if (current.kind != candidate.kind) {
var currentIsGetter = current.kind == ElementKind.GETTER;
var currentIsGetter = current is InternalGetterElement;
mixinConflicts.add(
GetterMethodConflict(
name: name,
getter: currentIsGetter ? current : candidate,
method: currentIsGetter ? candidate : current,
getter: currentIsGetter
? current
: candidate as InternalGetterElement,
method: currentIsGetter
? candidate as InternalMethodElement
: current as InternalMethodElement,
),
);
}
pkg/analyzer/lib/src/error/inheritance_override.dart
+114 -70
View File
@@ -39,9 +39,16 @@ class InheritanceOverrideVerifier {
final Map<InterfaceElementImpl, _InterfaceElementState>
_interfaceElementStates = {};
final Map<LibraryFragmentImpl, DiagnosticReporter>
_diagnosticReportersByFragment;
InheritanceOverrideVerifier(this._typeSystem, this._inheritance)
: _typeProvider = _typeSystem.typeProvider;
InheritanceOverrideVerifier(
this._typeSystem,
this._inheritance, {
required Map<LibraryFragmentImpl, DiagnosticReporter>
diagnosticReportersByFragment,
}) : _typeProvider = _typeSystem.typeProvider,
_diagnosticReportersByFragment = diagnosticReportersByFragment;
void verifyUnit(CompilationUnitImpl unit, DiagnosticReporter reporter) {
var library = unit.declaredFragment!.element;
@@ -71,6 +78,7 @@ class InheritanceOverrideVerifier {
superclass: declaration.extendsClause?.superclass,
withClause: declaration.withClause,
interfaceElementState: interfaceElementState(fragment.element),
reportInterfaceConflicts: _reportInterfaceConflicts,
);
} else if (declaration is ClassTypeAliasImpl) {
var fragment = declaration.declaredFragment!;
@@ -90,6 +98,7 @@ class InheritanceOverrideVerifier {
superclass: declaration.superclass,
withClause: declaration.withClause,
interfaceElementState: interfaceElementState(fragment.element),
reportInterfaceConflicts: _reportInterfaceConflicts,
);
} else if (declaration is EnumDeclarationImpl) {
var fragment = declaration.declaredFragment!;
@@ -109,6 +118,7 @@ class InheritanceOverrideVerifier {
members: declaration.body.members,
withClause: declaration.withClause,
interfaceElementState: interfaceElementState(fragment.element),
reportInterfaceConflicts: _reportInterfaceConflicts,
);
} else if (declaration is MixinDeclarationImpl) {
var fragment = declaration.declaredFragment!;
@@ -127,6 +137,8 @@ class InheritanceOverrideVerifier {
implementsClause: declaration.implementsClause,
members: declaration.body.members,
onClause: declaration.onClause,
interfaceElementState: interfaceElementState(fragment.element),
reportInterfaceConflicts: _reportInterfaceConflicts,
);
} else {
continue;
@@ -140,6 +152,85 @@ class InheritanceOverrideVerifier {
}
}
void _reportInterfaceConflicts(
InterfaceElementImpl element,
Interface interface,
) {
for (var conflict in interface.conflicts) {
var interfaceTarget = _targetForElement(element);
if (interfaceTarget == null) {
continue;
}
var memberName = conflict.name.name;
switch (conflict) {
case GetterMethodConflict():
var target = interfaceTarget;
// Try to use a local declaration related to the conflict.
if (interface.declared[conflict.name] case var declared?) {
target = _targetForElement(declared) ?? target;
}
target.report(
diag.inconsistentInheritanceGetterAndMethod.withArguments(
memberName: memberName,
getterInterface: conflict.getter.enclosingElement.name!,
methodInterface: conflict.method.enclosingElement!.name!,
),
);
case CandidatesConflict():
var inheritedSignatures = conflict.candidates
.map((candidate) {
var className = candidate.enclosingElement!.name;
var typeStr = candidate.type.getDisplayString();
return '$className.$memberName ($typeStr)';
})
.join(', ');
interfaceTarget.report(
diag.inconsistentInheritance.withArguments(
name: memberName,
inheritedSignatures: inheritedSignatures,
),
);
default:
throw StateError('${conflict.runtimeType}');
}
}
}
_DiagnosticTarget? _targetForElement(Element element) {
var nonSynthetic = element.nonSynthetic;
if (nonSynthetic is! ElementImpl) {
return null;
}
return _targetForFragment(nonSynthetic.firstFragment);
}
_DiagnosticTarget? _targetForFragment(FragmentImpl fragment) {
var libraryFragment = fragment.libraryFragment;
if (libraryFragment == null) {
return null;
}
var reporter = _diagnosticReportersByFragment[libraryFragment];
if (reporter == null) {
return null;
}
var offset = fragment.nameOffset;
var length = fragment.name?.length;
if (offset == null || length == null) {
return null;
}
return _DiagnosticTarget(
reporter: reporter,
offset: offset,
length: length,
);
}
/// Returns [ExecutableElement] members that are in the interface of the
/// given class with `@mustBeOverridden`, but don't have implementations.
static List<ExecutableElement> missingMustBeOverridden(
@@ -175,6 +266,8 @@ class _ClassVerifier {
final NamedType? superclass;
final WithClause? withClause;
final _InterfaceElementState? interfaceElementState;
final void Function(InterfaceElementImpl element, Interface interface)
reportInterfaceConflicts;
final List<InterfaceType> directSuperInterfaces = [];
@@ -203,6 +296,7 @@ class _ClassVerifier {
this.superclass,
this.withClause,
this.interfaceElementState,
required this.reportInterfaceConflicts,
}) : libraryUri = library.uri;
/// Verify inheritance overrides, and return `true` if an error was
@@ -230,14 +324,8 @@ class _ClassVerifier {
// Compute the interface of the class.
var interface = inheritance.getInterface(element);
// Report conflicts between direct superinterfaces of the class.
for (var conflict in interface.conflicts) {
var errorToken = switch (conflict) {
GetterMethodConflict() =>
_declaredMemberName(conflict.name) ?? classNameToken,
_ => classNameToken,
};
_reportInconsistentInheritance(errorToken, conflict);
if (identical(classFragment, element.firstFragment)) {
reportInterfaceConflicts(element, interface);
}
if (element.supertype != null) {
@@ -756,30 +844,6 @@ class _ClassVerifier {
return true;
}
/// Returns the name token for a member declared in this class or mixin that
/// matches [name], so getter/method inheritance conflicts can be reported at
/// the overriding declaration instead of the class or mixin name.
Token? _declaredMemberName(Name name) {
for (var member in members) {
if (member is FieldDeclarationImpl) {
for (var field in member.fields.variables) {
var fieldFragment = field.declaredFragment as FieldFragmentImpl;
var fieldElement = fieldFragment.element;
if (fieldElement.getter?.lookupName == name.name) {
return field.name;
}
}
} else if (member is MethodDeclarationImpl) {
var methodFragment = member.declaredFragment!;
var methodElement = methodFragment.element;
if (methodElement.lookupName == name.name) {
return member.name;
}
}
}
return null;
}
/// If [name] is not implemented in the extended concrete class, the
/// issue should be fixed there, and then [classElement] will not have it too.
bool _isNotImplementedInConcreteSuperClass(Name name) {
@@ -839,42 +903,6 @@ class _ClassVerifier {
return false;
}
void _reportInconsistentInheritance(Token errorToken, Conflict conflict) {
var name = conflict.name;
if (conflict is GetterMethodConflict) {
// Members that participate in inheritance are always enclosed in named
// elements so it is safe to assume that
// `conflict.getter.enclosingElement.name` and
// `conflict.method.enclosingElement.name` are both non-`null`.
reporter.report(
diag.inconsistentInheritanceGetterAndMethod
.withArguments(
memberName: name.name,
getterInterface: conflict.getter.enclosingElement!.name!,
methodInterface: conflict.method.enclosingElement!.name!,
)
.at(errorToken),
);
} else if (conflict is CandidatesConflict) {
var candidatesStr = conflict.candidates
.map((candidate) {
var className = candidate.enclosingElement!.name;
var typeStr = candidate.type.getDisplayString();
return '$className.${name.name} ($typeStr)';
})
.join(', ');
reporter.report(
diag.inconsistentInheritance
.withArguments(name: name.name, inheritedSignatures: candidatesStr)
.at(errorToken),
);
} else {
throw StateError('${conflict.runtimeType}');
}
}
void _reportInheritedAbstractMembers(
List<InternalExecutableElement>? elements,
) {
@@ -1083,6 +1111,22 @@ class _ClassVerifier {
}
}
class _DiagnosticTarget {
final DiagnosticReporter reporter;
final int offset;
final int length;
_DiagnosticTarget({
required this.reporter,
required this.offset,
required this.length,
});
void report(LocatableDiagnostic diagnostic) {
reporter.report(diagnostic.atOffset(offset: offset, length: length));
}
}
/// Maintains an [InterfaceElementImpl]'s mixin index across multiple fragments.
class _InterfaceElementState {
int mixinIndex = 0;
pkg/analyzer/test/src/diagnostics/inconsistent_inheritance_getter_and_method_test.dart
+128
View File
@@ -17,6 +17,134 @@ main() {
@reflectiveTest
class InconsistentInheritanceGetterAndMethodTest
extends PubPackageResolutionTest {
test_class_augmentationChain_declaresFieldInAugmentation() async {
await resolveTestCodeWithDiagnostics(r'''
class A {
void foo(String _) {}
}
abstract interface class I {
int get foo => 1;
}
class C extends A implements I {}
augment class C {
int foo = 2;
// ^^^
// [diag.inconsistentInheritanceGetterAndMethod] 'foo' is inherited as a getter (from 'I') and also a method (from 'A').
}
''');
}
test_class_augmentationChain_declaresGetterInAugmentation() async {
await resolveTestCodeWithDiagnostics(r'''
class A {
void foo(String _) {}
}
abstract interface class I {
int get foo => 1;
}
class C extends A implements I {}
augment class C {
int get foo => 2;
// ^^^
// [diag.inconsistentInheritanceGetterAndMethod] 'foo' is inherited as a getter (from 'I') and also a method (from 'A').
}
''');
}
test_class_augmentationChain_declaresGetterInAugmentation_part() async {
var a = getFile('$testPackageLibPath/a.dart');
var b = getFile('$testPackageLibPath/b.dart');
await resolveFilesWithDiagnostics({
a: r'''
part 'b.dart';
class A {
void foo(String _) {}
}
abstract interface class I {
int get foo => 1;
}
class C extends A implements I {}
''',
b: r'''
part of 'a.dart';
augment class C {
int get foo => 2;
// ^^^
// [diag.inconsistentInheritanceGetterAndMethod] 'foo' is inherited as a getter (from 'I') and also a method (from 'A').
}
''',
});
}
test_class_augmentationChain_declaresGetterInIntroduction() async {
await resolveTestCodeWithDiagnostics(r'''
class A {
void foo(String _) {}
}
abstract interface class I {
int get foo => 1;
}
class C extends A implements I {
int get foo => 2;
// ^^^
// [diag.inconsistentInheritanceGetterAndMethod] 'foo' is inherited as a getter (from 'I') and also a method (from 'A').
}
augment class C {}
''');
}
test_class_augmentationChain_declaresMethodInAugmentation() async {
await resolveTestCodeWithDiagnostics(r'''
class A {
void foo(String _) {}
}
abstract interface class I {
int get foo => 1;
}
class C extends A implements I {}
augment class C {
void foo(String _) {}
// ^^^
// [diag.inconsistentInheritanceGetterAndMethod] 'foo' is inherited as a getter (from 'I') and also a method (from 'A').
}
''');
}
test_class_augmentationChain_declaresNoMember() async {
await resolveTestCodeWithDiagnostics(r'''
class A {
void foo(String _) {}
}
abstract interface class I {
int get foo => 1;
}
abstract class C extends A implements I {}
// ^
// [diag.inconsistentInheritanceGetterAndMethod] 'foo' is inherited as a getter (from 'I') and also a method (from 'A').
augment abstract class C {}
''');
}
test_class_implements_getter_implements_method_declaresField() async {
await resolveTestCodeWithDiagnostics(r'''
abstract class A {
pkg/analyzer/test/src/diagnostics/inconsistent_inheritance_test.dart
-12
View File
@@ -42,15 +42,11 @@ abstract class C extends Object {}
part of 'a.dart';
augment abstract class C implements B {}
// ^
// [diag.inconsistentInheritance] Superinterfaces don't have a valid override for 'foo': A.foo (void Function(int)), B.foo (void Function(String)).
''',
c: r'''
part of 'a.dart';
augment abstract class C with A {}
// ^
// [diag.inconsistentInheritance] Superinterfaces don't have a valid override for 'foo': A.foo (void Function(int)), B.foo (void Function(String)).
''',
});
}
@@ -81,15 +77,11 @@ abstract class C extends Object {}
part of 'a.dart';
augment abstract class C with A {}
// ^
// [diag.inconsistentInheritance] Superinterfaces don't have a valid override for 'foo': A.foo (void Function(int)), B.foo (void Function(String)).
''',
c: r'''
part of 'a.dart';
augment abstract class C implements B {}
// ^
// [diag.inconsistentInheritance] Superinterfaces don't have a valid override for 'foo': A.foo (void Function(int)), B.foo (void Function(String)).
''',
});
}
@@ -109,8 +101,6 @@ class A implements I {}
// [diag.inconsistentInheritance] Superinterfaces don't have a valid override for 'foo': M.foo (int Function()), I.foo (String Function()).
augment class A with M {}
// ^
// [diag.inconsistentInheritance] Superinterfaces don't have a valid override for 'foo': M.foo (int Function()), I.foo (String Function()).
''');
}
@@ -309,8 +299,6 @@ enum E implements A {v}
// [diag.inconsistentInheritance] Superinterfaces don't have a valid override for 'foo': A.foo (int Function()), B.foo (String Function()).
augment enum E implements B {
// ^
// [diag.inconsistentInheritance] Superinterfaces don't have a valid override for 'foo': A.foo (int Function()), B.foo (String Function()).
augment v;
}
''');