diff --git a/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart b/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart index c564a745d5c..d2325e1e369 100644 --- a/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart +++ b/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart @@ -86,10 +86,6 @@ class LibraryAnalyzer { final Map _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( diff --git a/pkg/analyzer/lib/src/dart/element/inheritance_manager3.dart b/pkg/analyzer/lib/src/dart/element/inheritance_manager3.dart index 0dce28ee9d0..dc3a38450eb 100644 --- a/pkg/analyzer/lib/src/dart/element/inheritance_manager3.dart +++ b/pkg/analyzer/lib/src/dart/element/inheritance_manager3.dart @@ -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, ), ); } diff --git a/pkg/analyzer/lib/src/error/inheritance_override.dart b/pkg/analyzer/lib/src/error/inheritance_override.dart index d07cd43b44f..15b1d224723 100644 --- a/pkg/analyzer/lib/src/error/inheritance_override.dart +++ b/pkg/analyzer/lib/src/error/inheritance_override.dart @@ -39,9 +39,16 @@ class InheritanceOverrideVerifier { final Map _interfaceElementStates = {}; + final Map + _diagnosticReportersByFragment; - InheritanceOverrideVerifier(this._typeSystem, this._inheritance) - : _typeProvider = _typeSystem.typeProvider; + InheritanceOverrideVerifier( + this._typeSystem, + this._inheritance, { + required Map + 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 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 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? 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; diff --git a/pkg/analyzer/test/src/diagnostics/inconsistent_inheritance_getter_and_method_test.dart b/pkg/analyzer/test/src/diagnostics/inconsistent_inheritance_getter_and_method_test.dart index 2773553ab74..d446caffa09 100644 --- a/pkg/analyzer/test/src/diagnostics/inconsistent_inheritance_getter_and_method_test.dart +++ b/pkg/analyzer/test/src/diagnostics/inconsistent_inheritance_getter_and_method_test.dart @@ -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 { diff --git a/pkg/analyzer/test/src/diagnostics/inconsistent_inheritance_test.dart b/pkg/analyzer/test/src/diagnostics/inconsistent_inheritance_test.dart index c02b79f651d..861d33f360a 100644 --- a/pkg/analyzer/test/src/diagnostics/inconsistent_inheritance_test.dart +++ b/pkg/analyzer/test/src/diagnostics/inconsistent_inheritance_test.dart @@ -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; } ''');