diff --git a/pkg/analysis_server/test/search/type_hierarchy_test.dart b/pkg/analysis_server/test/search/type_hierarchy_test.dart index 774aef669d8..5d5f1269204 100644 --- a/pkg/analysis_server/test/search/type_hierarchy_test.dart +++ b/pkg/analysis_server/test/search/type_hierarchy_test.dart @@ -64,7 +64,18 @@ class B extends A { 'superclass': 1, 'interfaces': [], 'mixins': [], - 'subclasses': [1], + 'subclasses': [2], + }, + { + 'classElement': { + 'kind': 'CLASS', + 'name': 'Object', + 'location': anything, + 'flags': 0, + }, + 'interfaces': [], + 'mixins': [], + 'subclasses': [], }, { 'classElement': { @@ -76,7 +87,7 @@ class B extends A { 'superclass': 0, 'interfaces': [], 'mixins': [], - 'subclasses': [], + 'subclasses': [0], }, ]); } diff --git a/pkg/analyzer/lib/src/dart/analysis/driver.dart b/pkg/analyzer/lib/src/dart/analysis/driver.dart index 689efd80982..51338a1004a 100644 --- a/pkg/analyzer/lib/src/dart/analysis/driver.dart +++ b/pkg/analyzer/lib/src/dart/analysis/driver.dart @@ -109,7 +109,7 @@ testFineAfterLibraryAnalyzerHook; // TODO(scheglov): Clean up the list of implicitly analyzed files. class AnalysisDriver { /// The version of data format, should be incremented on every format change. - static const int DATA_VERSION = 522; + static const int DATA_VERSION = 523; /// The number of exception contexts allowed to write. Once this field is /// zero, we stop writing any new exception contexts in this process. diff --git a/pkg/analyzer/lib/src/dart/element/element.dart b/pkg/analyzer/lib/src/dart/element/element.dart index 066b838d23c..c6e38496def 100644 --- a/pkg/analyzer/lib/src/dart/element/element.dart +++ b/pkg/analyzer/lib/src/dart/element/element.dart @@ -4600,6 +4600,10 @@ abstract class InterfaceElementImpl extends InstanceElementImpl InterfaceTypeImpl? _thisType; + /// If not `null`, this element was part of a supertypes cycle. The cycle + /// is broken by clearing supertypes for all cycle elements. + List? interfaceCycle; + /// The cached result of [allSupertypes]. List? _allSupertypes; diff --git a/pkg/analyzer/lib/src/error/inheritance_override.dart b/pkg/analyzer/lib/src/error/inheritance_override.dart index 111c597efe3..96d9ffa2494 100644 --- a/pkg/analyzer/lib/src/error/inheritance_override.dart +++ b/pkg/analyzer/lib/src/error/inheritance_override.dart @@ -560,82 +560,71 @@ class _ClassVerifier { /// [CompileTimeErrorCode.recursiveInterfaceInheritanceImplements], /// [CompileTimeErrorCode.recursiveInterfaceInheritanceOn], /// [CompileTimeErrorCode.recursiveInterfaceInheritanceWith]. - bool _checkForRecursiveInterfaceInheritance( - InterfaceElementImpl element, [ - List? path, - ]) { - path ??= []; - - // Detect error condition. - int size = path.length; - // If this is not the base case (size > 0), and the enclosing class is the - // given class element then report an error. - if (size > 0 && classElement == element) { - String className = classElement.displayName; - if (size > 1) { - // Construct a string showing the cyclic implements path: - // "A, B, C, D, A" - String separator = ", "; - StringBuffer buffer = StringBuffer(); - for (int i = 0; i < size; i++) { - buffer.write(path[i].displayName); - buffer.write(separator); - } - buffer.write(element.displayName); - reporter.atElement2( - classElement, - CompileTimeErrorCode.recursiveInterfaceInheritance, - arguments: [className, buffer.toString()], - ); - return true; - } else { - // RECURSIVE_INTERFACE_INHERITANCE_BASE_CASE_EXTENDS or - // RECURSIVE_INTERFACE_INHERITANCE_BASE_CASE_IMPLEMENTS or - // RECURSIVE_INTERFACE_INHERITANCE_ON or - // RECURSIVE_INTERFACE_INHERITANCE_BASE_CASE_WITH - reporter.atElement2( - classElement, - _getRecursiveErrorCode(element), - arguments: [className], - ); - return true; - } - } - - if (path.indexOf(element) > 0) { + bool _checkForRecursiveInterfaceInheritance(InterfaceElementImpl element) { + var cycle = element.interfaceCycle; + if (cycle == null) { return false; } - path.add(element); - // n-case - var supertype = element.supertype; - if (supertype != null && - _checkForRecursiveInterfaceInheritance(supertype.element, path)) { - return true; - } - - for (var type in element.mixins) { - if (_checkForRecursiveInterfaceInheritance(type.element, path)) { + if (superclass case var superclass?) { + if (superclass.element == element) { + reporter.atElement2( + element, + CompileTimeErrorCode.recursiveInterfaceInheritanceExtends, + arguments: [element.displayName], + ); return true; } } - if (element is MixinElementImpl) { - for (var type in element.superclassConstraints) { - if (_checkForRecursiveInterfaceInheritance(type.element, path)) { + if (onClause case var onClause?) { + for (var typeAnnotation in onClause.superclassConstraints) { + if (typeAnnotation.element == element) { + reporter.atElement2( + element, + CompileTimeErrorCode.recursiveInterfaceInheritanceOn, + arguments: [element.displayName], + ); return true; } } } - for (var type in element.interfaces) { - if (_checkForRecursiveInterfaceInheritance(type.element, path)) { - return true; + if (withClause case var withClause?) { + for (var typeAnnotation in withClause.mixinTypes) { + if (typeAnnotation.element == element) { + reporter.atElement2( + element, + CompileTimeErrorCode.recursiveInterfaceInheritanceWith, + arguments: [element.displayName], + ); + return true; + } } } - path.removeAt(path.length - 1); - return false; + if (implementsClause case var implementsClause?) { + for (var typeAnnotation in implementsClause.interfaces) { + if (typeAnnotation.element == element) { + reporter.atElement2( + element, + CompileTimeErrorCode.recursiveInterfaceInheritanceImplements, + arguments: [element.displayName], + ); + return true; + } + } + } + + reporter.atElement2( + classElement, + CompileTimeErrorCode.recursiveInterfaceInheritance, + arguments: [ + element.displayName, + cycle.map((e) => e.displayName).join(', '), + ], + ); + return true; } void _checkIllegalConcreteEnumMemberDeclaration(Token name) { @@ -742,30 +731,6 @@ class _ClassVerifier { return true; } - /// Return the error code that should be used when the given class [element] - /// references itself directly. - DiagnosticCode _getRecursiveErrorCode(InterfaceElement element) { - if (element.supertype?.element == classElement) { - return CompileTimeErrorCode.recursiveInterfaceInheritanceExtends; - } - - if (element is MixinElement) { - for (var type in element.superclassConstraints) { - if (type.element == classElement) { - return CompileTimeErrorCode.recursiveInterfaceInheritanceOn; - } - } - } - - for (var type in element.mixins) { - if (type.element == classElement) { - return CompileTimeErrorCode.recursiveInterfaceInheritanceWith; - } - } - - return CompileTimeErrorCode.recursiveInterfaceInheritanceImplements; - } - /// 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) { diff --git a/pkg/analyzer/lib/src/summary2/bundle_reader.dart b/pkg/analyzer/lib/src/summary2/bundle_reader.dart index 6ba8a982c80..9bbc6bedeb0 100644 --- a/pkg/analyzer/lib/src/summary2/bundle_reader.dart +++ b/pkg/analyzer/lib/src/summary2/bundle_reader.dart @@ -293,6 +293,7 @@ class LibraryReader { element.supertype = reader._readOptionalInterfaceType(); element.mixins = reader._readInterfaceTypeList(); element.interfaces = reader._readInterfaceTypeList(); + element.interfaceCycle = reader.readOptionalElementList(); }), ); @@ -493,6 +494,7 @@ class LibraryReader { element.supertype = reader._readOptionalInterfaceType(); element.mixins = reader._readInterfaceTypeList(); element.interfaces = reader._readInterfaceTypeList(); + element.interfaceCycle = reader.readOptionalElementList(); }), ); @@ -632,6 +634,7 @@ class LibraryReader { reader._addTypeParameters2(element.typeParameters); element.typeErasure = reader.readRequiredType(); element.interfaces = reader._readInterfaceTypeList(); + element.interfaceCycle = reader.readOptionalElementList(); }), ); @@ -967,6 +970,7 @@ class LibraryReader { reader._addTypeParameters2(element.typeParameters); element.superclassConstraints = reader._readInterfaceTypeList(); element.interfaces = reader._readInterfaceTypeList(); + element.interfaceCycle = reader.readOptionalElementList(); }), ); @@ -1537,6 +1541,10 @@ class ResolutionReader { return _readNodeList(); } + List? readOptionalElementList() { + return _reader.readOptionalObject(readElementList); + } + ExpressionImpl? readOptionalExpression() { if (_reader.readBool()) { return _readRequiredNode() as ExpressionImpl; diff --git a/pkg/analyzer/lib/src/summary2/bundle_writer.dart b/pkg/analyzer/lib/src/summary2/bundle_writer.dart index fd1bcf4cec1..af4965469f9 100644 --- a/pkg/analyzer/lib/src/summary2/bundle_writer.dart +++ b/pkg/analyzer/lib/src/summary2/bundle_writer.dart @@ -160,6 +160,7 @@ class BundleWriter { _resolutionSink.writeType(element.supertype); _resolutionSink._writeTypeList(element.mixins); _resolutionSink._writeTypeList(element.interfaces); + _resolutionSink.writeOptionalElementList(element.interfaceCycle); }); }); }); @@ -275,6 +276,7 @@ class BundleWriter { _resolutionSink.writeType(element.supertype); _resolutionSink._writeTypeList(element.mixins); _resolutionSink._writeTypeList(element.interfaces); + _resolutionSink.writeOptionalElementList(element.interfaceCycle); }); }); }); @@ -369,6 +371,7 @@ class BundleWriter { _resolutionSink.withTypeParameters(element.typeParameters, () { _resolutionSink.writeType(element.typeErasure); _resolutionSink._writeTypeList(element.interfaces); + _resolutionSink.writeOptionalElementList(element.interfaceCycle); }); }); }); @@ -572,6 +575,7 @@ class BundleWriter { _resolutionSink.withTypeParameters(element.typeParameters, () { _resolutionSink._writeTypeList(element.superclassConstraints); _resolutionSink._writeTypeList(element.interfaces); + _resolutionSink.writeOptionalElementList(element.interfaceCycle); }); }); }); @@ -913,6 +917,10 @@ class ResolutionSink extends _SummaryDataWriter { } } + void writeOptionalElementList(List? elements) { + writeOptionalObject(elements, (it) => _writeElementList(it)); + } + void writeOptionalTypeList(List? types) { if (types != null) { writeBool(true); diff --git a/pkg/analyzer/lib/src/summary2/interface_cycles.dart b/pkg/analyzer/lib/src/summary2/interface_cycles.dart new file mode 100644 index 00000000000..649d8e5fc98 --- /dev/null +++ b/pkg/analyzer/lib/src/summary2/interface_cycles.dart @@ -0,0 +1,107 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:_fe_analyzer_shared/src/util/dependency_walker.dart' as graph; +import 'package:analyzer/src/dart/ast/ast.dart'; +import 'package:analyzer/src/dart/element/element.dart'; +import 'package:analyzer/src/summary2/link.dart'; + +/// Clears interfaces for declarations that have cycles. +void breakInterfaceCycles(Linker linker, List declarations) { + var walker = _ImplementsWalker(); + var elements = []; + for (var declaration in declarations) { + if (declaration is DeclarationImpl) { + var element = declaration.declaredFragment!.element; + + // Handled elsewhere. + if (element is ExtensionTypeElementImpl) { + continue; + } + + if (element is InterfaceElementImpl) { + elements.add(element); + } + } + } + + for (var element in elements) { + var node = walker.getNode(element); + walker.walk(node); + } +} + +class _ImplementsNode extends graph.Node<_ImplementsNode> { + final _ImplementsWalker walker; + final InterfaceElementImpl element; + + @override + bool isEvaluated = false; + + _ImplementsNode(this.walker, this.element); + + @override + List<_ImplementsNode> computeDependencies() { + return [ + element.supertype, + ...element.mixins, + ...element.interfaces, + if (element case MixinElementImpl element) + ...element.superclassConstraints, + ].nonNulls + .map((interface) => interface.element) + .map(walker.getNode) + .toList(); + } + + void _evaluate() { + isEvaluated = true; + } + + void _markCircular(List elements) { + isEvaluated = true; + + element.interfaceCycle = elements; + + switch (element) { + case ClassElementImpl element: + var typeProvider = element.library.typeProvider; + element.supertype = typeProvider.objectType; + element.mixins = []; + element.interfaces = []; + case EnumElementImpl element: + element.mixins = []; + element.interfaces = []; + case ExtensionTypeElementImpl element: + element.interfaces = []; + case MixinElementImpl element: + var typeProvider = element.library.typeProvider; + element.superclassConstraints = [typeProvider.objectType]; + element.interfaces = []; + default: + throw UnimplementedError('${element.runtimeType}'); + } + } +} + +class _ImplementsWalker extends graph.DependencyWalker<_ImplementsNode> { + final Map nodeMap = Map.identity(); + + @override + void evaluate(_ImplementsNode v) { + v._evaluate(); + } + + @override + void evaluateScc(List<_ImplementsNode> scc) { + var elements = scc.map((node) => node.element).toList(); + for (var node in scc) { + node._markCircular(elements); + } + } + + _ImplementsNode getNode(InterfaceElementImpl element) { + return nodeMap[element] ??= _ImplementsNode(this, element); + } +} diff --git a/pkg/analyzer/lib/src/summary2/types_builder.dart b/pkg/analyzer/lib/src/summary2/types_builder.dart index b46859a9aee..fcb49b7bb69 100644 --- a/pkg/analyzer/lib/src/summary2/types_builder.dart +++ b/pkg/analyzer/lib/src/summary2/types_builder.dart @@ -14,6 +14,7 @@ import 'package:analyzer/src/dart/element/type_system.dart'; import 'package:analyzer/src/dart/resolver/flow_analysis_visitor.dart'; import 'package:analyzer/src/summary2/default_types_builder.dart'; import 'package:analyzer/src/summary2/extension_type.dart'; +import 'package:analyzer/src/summary2/interface_cycles.dart'; import 'package:analyzer/src/summary2/link.dart'; import 'package:analyzer/src/summary2/type_builder.dart'; import 'package:analyzer/src/utilities/extensions/collection.dart'; @@ -94,6 +95,7 @@ class TypesBuilder { buildExtensionTypes(_linker, nodes.declarations); _MixinsInference(_toInferMixins).perform(); + breakInterfaceCycles(_linker, nodes.declarations); } void _addFragmentWithClause( diff --git a/pkg/analyzer/test/src/dart/analysis/index_test.dart b/pkg/analyzer/test/src/dart/analysis/index_test.dart index 000f6dc2837..6ea6075ec56 100644 --- a/pkg/analyzer/test/src/dart/analysis/index_test.dart +++ b/pkg/analyzer/test/src/dart/analysis/index_test.dart @@ -1241,7 +1241,7 @@ void f() { test_isReferencedBy_ConstructorElement_classTypeAlias() async { await _indexTestUnit(''' class M {} -class A implements B { +class A { A() {} A.named() {} } @@ -1256,14 +1256,14 @@ void f() { '''); var constructor = findElement2.unnamedConstructor('A'); assertElementIndexText(constructor, r''' -118 9:8 || IS_INVOKED_BY qualified -158 11:8 || IS_INVOKED_BY qualified +105 9:8 || IS_INVOKED_BY qualified +145 11:8 || IS_INVOKED_BY qualified '''); var constructor_named = findElement2.constructor('named', of: 'A'); assertElementIndexText(constructor_named, r''' -135 10:8 |.named| IS_INVOKED_BY qualified -175 12:8 |.named| IS_INVOKED_BY qualified +122 10:8 |.named| IS_INVOKED_BY qualified +162 12:8 |.named| IS_INVOKED_BY qualified '''); } diff --git a/pkg/analyzer/test/src/dart/resolution/class_test.dart b/pkg/analyzer/test/src/dart/resolution/class_test.dart index c000b2864e3..3e017d59608 100644 --- a/pkg/analyzer/test/src/dart/resolution/class_test.dart +++ b/pkg/analyzer/test/src/dart/resolution/class_test.dart @@ -101,7 +101,7 @@ class X extends A {} ], ); - assertElementTypes(findElement2.class_('X').allSupertypes, ['A', 'B', 'C']); + assertElementTypes(findElement2.class_('X').allSupertypes, ['A', 'Object']); } test_element_typeFunction_extends() async { @@ -170,11 +170,6 @@ main() { [ error(CompileTimeErrorCode.recursiveInterfaceInheritance, 6, 1), error(CompileTimeErrorCode.recursiveInterfaceInheritance, 33, 1), - error( - CompileTimeErrorCode.nonAbstractClassInheritsAbstractMemberFivePlus, - 60, - 1, - ), error(WarningCode.unusedLocalVariable, 150, 1), ], ); diff --git a/pkg/analyzer/test/src/diagnostics/recursive_factory_redirect_test.dart b/pkg/analyzer/test/src/diagnostics/recursive_factory_redirect_test.dart index 8fa49ee7b44..2ee51bdf132 100644 --- a/pkg/analyzer/test/src/diagnostics/recursive_factory_redirect_test.dart +++ b/pkg/analyzer/test/src/diagnostics/recursive_factory_redirect_test.dart @@ -131,6 +131,7 @@ class C implements A, B { } ''', [ + error(CompileTimeErrorCode.redirectToInvalidReturnType, 26, 1), error(CompileTimeErrorCode.recursiveInterfaceInheritance, 37, 1), error(CompileTimeErrorCode.recursiveFactoryRedirect, 70, 1), error(CompileTimeErrorCode.recursiveInterfaceInheritance, 81, 1), diff --git a/pkg/analyzer/test/src/summary/elements/class_test.dart b/pkg/analyzer/test/src/summary/elements/class_test.dart index 384872bd607..58e0e1971e5 100644 --- a/pkg/analyzer/test/src/summary/elements/class_test.dart +++ b/pkg/analyzer/test/src/summary/elements/class_test.dart @@ -4460,11 +4460,6 @@ library firstFragment: #F7 #E3 U firstFragment: #F8 - supertype: C - alias: ::@typeAlias::A - typeArguments - U - T constructors named reference: ::@class::C::@constructor::named @@ -6548,6 +6543,132 @@ library '''); } + test_class_cycle_interfaces() async { + var library = await buildLibrary(r''' +class A implements B {} +class B implements A {} +'''); + checkElementText(library, r''' +library + reference: + fragments + #F0 + element: + classes + #F1 class A (nameOffset:6) (firstTokenOffset:0) (offset:6) + element: ::@class::A + constructors + #F2 synthetic new (nameOffset:) (firstTokenOffset:) (offset:6) + element: ::@class::A::@constructor::new + typeName: A + #F3 class B (nameOffset:30) (firstTokenOffset:24) (offset:30) + element: ::@class::B + constructors + #F4 synthetic new (nameOffset:) (firstTokenOffset:) (offset:30) + element: ::@class::B::@constructor::new + typeName: B + classes + class A + reference: ::@class::A + firstFragment: #F1 + constructors + synthetic new + reference: ::@class::A::@constructor::new + firstFragment: #F2 + class B + reference: ::@class::B + firstFragment: #F3 + constructors + synthetic new + reference: ::@class::B::@constructor::new + firstFragment: #F4 +'''); + } + + test_class_cycle_mixins() async { + var library = await buildLibrary(r''' +class A with B {} +class B with A {} +'''); + checkElementText(library, r''' +library + reference: + fragments + #F0 + element: + classes + #F1 class A (nameOffset:6) (firstTokenOffset:0) (offset:6) + element: ::@class::A + constructors + #F2 synthetic new (nameOffset:) (firstTokenOffset:) (offset:6) + element: ::@class::A::@constructor::new + typeName: A + #F3 class B (nameOffset:24) (firstTokenOffset:18) (offset:24) + element: ::@class::B + constructors + #F4 synthetic new (nameOffset:) (firstTokenOffset:) (offset:24) + element: ::@class::B::@constructor::new + typeName: B + classes + class A + reference: ::@class::A + firstFragment: #F1 + constructors + synthetic new + reference: ::@class::A::@constructor::new + firstFragment: #F2 + class B + reference: ::@class::B + firstFragment: #F3 + constructors + synthetic new + reference: ::@class::B::@constructor::new + firstFragment: #F4 +'''); + } + + test_class_cycle_supertype() async { + var library = await buildLibrary(r''' +class A extends B {} +class B extends A {} +'''); + checkElementText(library, r''' +library + reference: + fragments + #F0 + element: + classes + #F1 class A (nameOffset:6) (firstTokenOffset:0) (offset:6) + element: ::@class::A + constructors + #F2 synthetic new (nameOffset:) (firstTokenOffset:) (offset:6) + element: ::@class::A::@constructor::new + typeName: A + #F3 class B (nameOffset:27) (firstTokenOffset:21) (offset:27) + element: ::@class::B + constructors + #F4 synthetic new (nameOffset:) (firstTokenOffset:) (offset:27) + element: ::@class::B::@constructor::new + typeName: B + classes + class A + reference: ::@class::A + firstFragment: #F1 + constructors + synthetic new + reference: ::@class::A::@constructor::new + firstFragment: #F2 + class B + reference: ::@class::B + firstFragment: #F3 + constructors + synthetic new + reference: ::@class::B::@constructor::new + firstFragment: #F4 +'''); + } + test_class_documented() async { var library = await buildLibrary(''' /** diff --git a/pkg/analyzer/test/src/summary/elements/mixin_test.dart b/pkg/analyzer/test/src/summary/elements/mixin_test.dart index 5223987a9a5..f18cad3fcad 100644 --- a/pkg/analyzer/test/src/summary/elements/mixin_test.dart +++ b/pkg/analyzer/test/src/summary/elements/mixin_test.dart @@ -233,6 +233,66 @@ library '''); } + test_mixin_cycle_interfaces() async { + var library = await buildLibrary(r''' +mixin A implements B {} +mixin B implements A {} +'''); + checkElementText(library, r''' +library + reference: + fragments + #F0 + element: + mixins + #F1 mixin A (nameOffset:6) (firstTokenOffset:0) (offset:6) + element: ::@mixin::A + #F2 mixin B (nameOffset:30) (firstTokenOffset:24) (offset:30) + element: ::@mixin::B + mixins + mixin A + reference: ::@mixin::A + firstFragment: #F1 + superclassConstraints + Object + mixin B + reference: ::@mixin::B + firstFragment: #F2 + superclassConstraints + Object +'''); + } + + test_mixin_cycle_superclassConstraints() async { + var library = await buildLibrary(r''' +mixin A on B {} +mixin B on A {} +'''); + checkElementText(library, r''' +library + reference: + fragments + #F0 + element: + mixins + #F1 mixin A (nameOffset:6) (firstTokenOffset:0) (offset:6) + element: ::@mixin::A + #F2 mixin B (nameOffset:22) (firstTokenOffset:16) (offset:22) + element: ::@mixin::B + mixins + mixin A + reference: ::@mixin::A + firstFragment: #F1 + superclassConstraints + Object + mixin B + reference: ::@mixin::B + firstFragment: #F2 + superclassConstraints + Object +'''); + } + test_mixin_field_inferredType_final() async { var library = await buildLibrary(''' mixin M { diff --git a/pkg/linter/test/rules/null_closures_test.dart b/pkg/linter/test/rules/null_closures_test.dart index 04e6b4209c2..0c491732703 100644 --- a/pkg/linter/test/rules/null_closures_test.dart +++ b/pkg/linter/test/rules/null_closures_test.dart @@ -124,7 +124,6 @@ void test_cycle() { // No lint error(CompileTimeErrorCode.recursiveInterfaceInheritance, 6, 1), error(CompileTimeErrorCode.recursiveInterfaceInheritance, 41, 1), - error(CompileTimeErrorCode.noDefaultSuperConstructorImplicit, 41, 1), error(CompileTimeErrorCode.argumentTypeNotAssignable, 81, 4), ], );