diff --git a/pkg/linter/lib/src/rules/overridden_fields.dart b/pkg/linter/lib/src/rules/overridden_fields.dart index ad5974ec6f83..d53dacba4055 100644 --- a/pkg/linter/lib/src/rules/overridden_fields.dart +++ b/pkg/linter/lib/src/rules/overridden_fields.dart @@ -4,45 +4,13 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; -import 'package:analyzer/dart/element/element.dart'; -import 'package:analyzer/dart/element/type.dart'; -import 'package:collection/collection.dart' show IterableExtension; +import 'package:analyzer/dart/element/element2.dart'; import '../analyzer.dart'; import '../extensions.dart'; const _desc = r"Don't override fields."; -Iterable _findAllSupertypesAndMixins( - InterfaceType? interface, List accumulator) { - if (interface == null || - interface.isDartCoreObject || - accumulator.contains(interface)) { - return accumulator; - } - - accumulator.add(interface); - var superclass = interface.superclass; - var interfaces = []; - if (superclass != null) { - interfaces.add(superclass); - } - interfaces - ..addAll(interface.element.mixins) - ..addAll(_findAllSupertypesAndMixins(superclass, accumulator)); - return interfaces.where((i) => i != interface); -} - -Iterable _findAllSupertypesInMixin(MixinElement mixinElement) { - var supertypes = []; - var accumulator = []; - for (var type in mixinElement.superclassConstraints) { - supertypes.add(type); - supertypes.addAll(_findAllSupertypesAndMixins(type, accumulator)); - } - return supertypes; -} - class OverriddenFields extends LintRule { OverriddenFields() : super( @@ -56,15 +24,16 @@ class OverriddenFields extends LintRule { @override void registerNodeProcessors( NodeLintRegistry registry, LinterContext context) { - var visitor = _Visitor(this); + var visitor = _Visitor(this, context.inheritanceManager); registry.addFieldDeclaration(this, visitor); } } class _Visitor extends SimpleAstVisitor { final LintRule rule; + final InheritanceManager3 inheritanceManager; - _Visitor(this.rule); + _Visitor(this.rule, this.inheritanceManager); @override void visitFieldDeclaration(FieldDeclaration node) { @@ -72,50 +41,19 @@ class _Visitor extends SimpleAstVisitor { if (node.isStatic) return; for (var variable in node.fields.variables) { - var declaredField = variable.declaredElement; - if (declaredField != null) { - var overriddenField = _getOverriddenMember(declaredField); - if (overriddenField != null && !overriddenField.isAbstract) { - rule.reportLintForToken(variable.name, - arguments: [overriddenField.enclosingElement3.displayName]); - } - } - } - } - - PropertyAccessorElement? _getOverriddenMember(Element member) { - var memberName = member.name; - var library = member.library; - bool isOverriddenMember(PropertyAccessorElement a) { - if (memberName == null || a.isStatic) { - return false; - } - if (a.isSynthetic && a.name == memberName) { - // Ensure that private members are overriding a member of the same library. - if (Identifier.isPrivateName(memberName)) { - return library == a.library; + var parent = variable.declaredFragment?.element.enclosingElement2; + if (parent is InterfaceElement2) { + var overriddenMember = inheritanceManager.getMember4( + parent, Name(parent.library2.uri, variable.name.lexeme), + forSuper: true); + if (overriddenMember is GetterElement && overriddenMember.isSynthetic) { + var definingInterface = overriddenMember.enclosingElement2; + if (definingInterface != null) { + rule.reportLintForToken(variable.name, + arguments: [definingInterface.displayName]); + } } - return true; } - return false; - } - - bool containsOverriddenMember(InterfaceType i) => - i.accessors.any(isOverriddenMember); - var enclosingElement = member.enclosingElement3; - if (enclosingElement is! InterfaceElement) { - return null; - } - var classElement = enclosingElement; - - Iterable interfaces; - if (classElement is MixinElement) { - interfaces = _findAllSupertypesInMixin(classElement); - } else { - interfaces = - _findAllSupertypesAndMixins(classElement.thisType, []); } - var interface = interfaces.firstWhereOrNull(containsOverriddenMember); - return interface?.accessors.firstWhere(isOverriddenMember); } } diff --git a/pkg/linter/test/rules/overridden_fields_test.dart b/pkg/linter/test/rules/overridden_fields_test.dart index f451c5ee9519..4ae4a4e25b32 100644 --- a/pkg/linter/test/rules/overridden_fields_test.dart +++ b/pkg/linter/test/rules/overridden_fields_test.dart @@ -70,6 +70,20 @@ augment class A { await assertNoDiagnosticsInFile(b.path); } + test_conflictingFieldAndMethod() async { + await assertDiagnostics(r''' +class A { + int x() => 0; +} + +class B extends A { + int x = 9; +} +''', [ + error(CompileTimeErrorCode.CONFLICTING_FIELD_AND_METHOD, 55, 1), + ]); + } + /// https://github.com/dart-lang/linter/issues/2874 test_conflictingStaticAndInstance() async { await assertNoDiagnostics(r''' @@ -115,6 +129,18 @@ class B extends A { ]); } + test_fieldOverridesGetter() async { + await assertNoDiagnostics(r''' +class A { + int get a => 0; +} +class B extends A { + @override + int a = 1; +} +'''); + } + test_mixinInheritsFromNotObject() async { // See: https://github.com/dart-lang/linter/issues/2969 // Preserves existing testing logic but has so many misuses of mixins that @@ -189,6 +215,7 @@ class GC34 extends GC33 { lint(127, 5), lint(194, 9), error(CompileTimeErrorCode.MIXIN_INHERITS_FROM_NOT_OBJECT, 273, 4), + lint(301, 9), lint(343, 5), lint(418, 9), error(CompileTimeErrorCode.MIXIN_INHERITS_FROM_NOT_OBJECT, 472, 4), @@ -203,6 +230,49 @@ class GC34 extends GC33 { ]); } + test_overridingAbstractField() async { + await assertNoDiagnostics(r''' +abstract class A { + abstract int x; +} + +class B extends A { + @override + int x = 1; +} +'''); + } + + test_privateFieldInSameLibrary() async { + await assertDiagnostics(r''' +class A { + int _x = 0; +} + +class B extends A { + int _x = 9; +} +''', [ + error(WarningCode.UNUSED_FIELD, 16, 2), + lint(53, 2), + error(WarningCode.UNUSED_FIELD, 53, 2), + ]); + } + + test_publicFieldInSameLibrary() async { + await assertDiagnostics(r''' +class A { + int x = 0; +} + +class B extends A { + int x = 9; +} +''', [ + lint(52, 1), + ]); + } + test_recursiveInterfaceInheritance() async { // Produces a recursive_interface_inheritance diagnostic. await assertDiagnostics(r'''