From e5a16ae775cd2e5742bd9dac659b1d6768e604a9 Mon Sep 17 00:00:00 2001 From: Konstantin Shcheglov Date: Wed, 9 Aug 2023 20:56:25 +0000 Subject: [PATCH] Extension types. Report self reference in implements clause. Change-Id: Icd26dcd6cf7521e075d6a8f13a985f3185c02369 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/319584 Reviewed-by: Phil Quitslund Commit-Queue: Konstantin Shcheglov Reviewed-by: Brian Wilkerson --- .../services/correction/error_fix_status.yaml | 2 + .../lib/src/dart/analysis/driver.dart | 2 +- .../lib/src/dart/element/element.dart | 9 ++- pkg/analyzer/lib/src/error/codes.g.dart | 9 +++ .../lib/src/error/error_code_values.g.dart | 1 + .../lib/src/generated/error_verifier.dart | 15 +++- .../lib/src/summary2/element_flags.dart | 14 +++- .../lib/src/summary2/extension_type.dart | 74 ++++++++++++++++++- pkg/analyzer/messages.yaml | 4 + ...extension_type_implements_itself_test.dart | 35 +++++++++ .../test/src/diagnostics/test_all.dart | 3 + .../test/src/summary/element_text.dart | 9 ++- .../test/src/summary/elements_test.dart | 68 ++++++++++++++++- 13 files changed, 230 insertions(+), 15 deletions(-) create mode 100644 pkg/analyzer/test/src/diagnostics/extension_type_implements_itself_test.dart diff --git a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml index e06a1458a9b..82d6dc4d38c 100644 --- a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml +++ b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml @@ -652,6 +652,8 @@ CompileTimeErrorCode.EXTENSION_TYPE_DECLARES_MEMBER_OF_OBJECT: Offer to delete the invalid member. CompileTimeErrorCode.EXTENSION_TYPE_IMPLEMENTS_DISALLOWED_TYPE: status: noFix +CompileTimeErrorCode.EXTENSION_TYPE_IMPLEMENTS_ITSELF: + status: noFix CompileTimeErrorCode.EXTENSION_TYPE_INHERITED_MEMBER_CONFLICT: status: noFix CompileTimeErrorCode.EXTENSION_TYPE_REPRESENTATION_DEPENDS_ON_ITSELF: diff --git a/pkg/analyzer/lib/src/dart/analysis/driver.dart b/pkg/analyzer/lib/src/dart/analysis/driver.dart index 38908ad4f6a..b6ef6cce13d 100644 --- a/pkg/analyzer/lib/src/dart/analysis/driver.dart +++ b/pkg/analyzer/lib/src/dart/analysis/driver.dart @@ -87,7 +87,7 @@ import 'package:analyzer/src/utilities/uri_cache.dart'; /// TODO(scheglov) Clean up the list of implicitly analyzed files. class AnalysisDriver implements AnalysisDriverGeneric { /// The version of data format, should be incremented on every format change. - static const int DATA_VERSION = 296; + static const int DATA_VERSION = 297; /// 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 4b74b2d84e9..41e9bcc215c 100644 --- a/pkg/analyzer/lib/src/dart/element/element.dart +++ b/pkg/analyzer/lib/src/dart/element/element.dart @@ -2967,8 +2967,13 @@ class ExtensionTypeElementImpl extends InterfaceElementImpl @override late final DartType typeErasure; - /// Whether the element has direct or indirect reference to itself. - bool hasSelfReference = false; + /// Whether the element has direct or indirect reference to itself, + /// in representation. + bool hasRepresentationSelfReference = false; + + /// Whether the element has direct or indirect reference to itself, + /// in implemented superinterfaces. + bool hasImplementsSelfReference = false; ExtensionTypeElementImpl(super.name, super.nameOffset); diff --git a/pkg/analyzer/lib/src/error/codes.g.dart b/pkg/analyzer/lib/src/error/codes.g.dart index 7515bea6813..e1ad281bd31 100644 --- a/pkg/analyzer/lib/src/error/codes.g.dart +++ b/pkg/analyzer/lib/src/error/codes.g.dart @@ -1642,6 +1642,15 @@ class CompileTimeErrorCode extends AnalyzerErrorCode { "Try specifying a different type, or remove the type from the list.", ); + /// No parameters. + static const CompileTimeErrorCode EXTENSION_TYPE_IMPLEMENTS_ITSELF = + CompileTimeErrorCode( + 'EXTENSION_TYPE_IMPLEMENTS_ITSELF', + "The extension type can't implement itself.", + correctionMessage: + "Try removing the superinterface that references this extension type.", + ); + /// Parameters: /// 0: the name of the extension type /// 1: the name of the conflicting member diff --git a/pkg/analyzer/lib/src/error/error_code_values.g.dart b/pkg/analyzer/lib/src/error/error_code_values.g.dart index 6b2f1df8d74..a6719d3b0fa 100644 --- a/pkg/analyzer/lib/src/error/error_code_values.g.dart +++ b/pkg/analyzer/lib/src/error/error_code_values.g.dart @@ -196,6 +196,7 @@ const List errorCodeValues = [ CompileTimeErrorCode.EXTENSION_TYPE_DECLARES_INSTANCE_FIELD, CompileTimeErrorCode.EXTENSION_TYPE_DECLARES_MEMBER_OF_OBJECT, CompileTimeErrorCode.EXTENSION_TYPE_IMPLEMENTS_DISALLOWED_TYPE, + CompileTimeErrorCode.EXTENSION_TYPE_IMPLEMENTS_ITSELF, CompileTimeErrorCode.EXTENSION_TYPE_INHERITED_MEMBER_CONFLICT, CompileTimeErrorCode.EXTENSION_TYPE_REPRESENTATION_DEPENDS_ON_ITSELF, CompileTimeErrorCode.EXTERNAL_FIELD_CONSTRUCTOR_INITIALIZER, diff --git a/pkg/analyzer/lib/src/generated/error_verifier.dart b/pkg/analyzer/lib/src/generated/error_verifier.dart index 104a89bbf6d..4b0bc8a9127 100644 --- a/pkg/analyzer/lib/src/generated/error_verifier.dart +++ b/pkg/analyzer/lib/src/generated/error_verifier.dart @@ -711,6 +711,7 @@ class ErrorVerifier extends RecursiveAstVisitor _checkForNonCovariantTypeParameterPositionInRepresentationType( node, element); _checkForExtensionTypeRepresentationDependsOnItself(node, element); + _checkForExtensionTypeImplementsItself(node, element); _checkForExtensionTypeMemberConflicts( node: node, element: element, @@ -2915,6 +2916,18 @@ class ErrorVerifier extends RecursiveAstVisitor } } + void _checkForExtensionTypeImplementsItself( + ExtensionTypeDeclarationImpl node, + ExtensionTypeElementImpl element, + ) { + if (element.hasImplementsSelfReference) { + errorReporter.reportErrorForToken( + CompileTimeErrorCode.EXTENSION_TYPE_IMPLEMENTS_ITSELF, + node.name, + ); + } + } + void _checkForExtensionTypeMemberConflicts({ required ExtensionTypeDeclaration node, required ExtensionTypeElement element, @@ -2958,7 +2971,7 @@ class ErrorVerifier extends RecursiveAstVisitor ExtensionTypeDeclarationImpl node, ExtensionTypeElementImpl element, ) { - if (element.hasSelfReference) { + if (element.hasRepresentationSelfReference) { errorReporter.reportErrorForToken( CompileTimeErrorCode.EXTENSION_TYPE_REPRESENTATION_DEPENDS_ON_ITSELF, node.name, diff --git a/pkg/analyzer/lib/src/summary2/element_flags.dart b/pkg/analyzer/lib/src/summary2/element_flags.dart index 52554186129..2e174b48d21 100644 --- a/pkg/analyzer/lib/src/summary2/element_flags.dart +++ b/pkg/analyzer/lib/src/summary2/element_flags.dart @@ -97,16 +97,24 @@ class EnumElementFlags { } class ExtensionTypeElementFlags { - static const int _hasSelfReference = 1 << 0; + static const int _hasRepresentationSelfReference = 1 << 0; + static const int _hasImplementsSelfReference = 1 << 1; static void read(SummaryDataReader reader, ExtensionTypeElementImpl element) { var byte = reader.readByte(); - element.hasSelfReference = (byte & _hasSelfReference) != 0; + element.hasRepresentationSelfReference = + (byte & _hasRepresentationSelfReference) != 0; + element.hasImplementsSelfReference = + (byte & _hasImplementsSelfReference) != 0; } static void write(BufferedSink sink, ExtensionTypeElementImpl element) { var result = 0; - result |= element.hasSelfReference ? _hasSelfReference : 0; + result |= element.hasRepresentationSelfReference + ? _hasRepresentationSelfReference + : 0; + result |= + element.hasImplementsSelfReference ? _hasImplementsSelfReference : 0; sink.writeByte(result); } } diff --git a/pkg/analyzer/lib/src/summary2/extension_type.dart b/pkg/analyzer/lib/src/summary2/extension_type.dart index 82ace83deb8..4ca29a20a72 100644 --- a/pkg/analyzer/lib/src/summary2/extension_type.dart +++ b/pkg/analyzer/lib/src/summary2/extension_type.dart @@ -3,7 +3,6 @@ // 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/dart/element/element.dart'; import 'package:analyzer/dart/element/type.dart'; import 'package:analyzer/src/dart/ast/ast.dart'; import 'package:analyzer/src/dart/ast/extensions.dart'; @@ -19,16 +18,29 @@ import 'package:analyzer/src/utilities/extensions/collection.dart'; void buildExtensionTypes(Linker linker, List declarations) { final walker = _Walker(linker); final nodes = <_Node>[]; + final elements = []; for (final declaration in declarations) { if (declaration is ExtensionTypeDeclarationImpl) { final node = walker.getNode(declaration); nodes.add(node); + elements.add(node.element); } } for (final node in nodes) { walker.walk(node); } + + _breakImplementsCycles(elements); +} + +/// Clears interfaces for extension types that have cycles. +void _breakImplementsCycles(List elements) { + final walker = _ImplementsWalker(); + for (final element in elements) { + final node = walker.getNode(element); + walker.walk(node); + } } /// Collector of referenced extension types in a type. @@ -62,6 +74,62 @@ class _ExtensionTypeErasure extends ReplacementVisitor { } } +class _ImplementsNode extends graph.Node<_ImplementsNode> { + final _ImplementsWalker walker; + final ExtensionTypeElementImpl element; + + @override + bool isEvaluated = false; + + _ImplementsNode(this.walker, this.element); + + @override + List<_ImplementsNode> computeDependencies() { + return element.interfaces + .map((interface) => interface.element) + .whereType() + .map(walker.getNode) + .toList(); + } + + void _evaluate() { + isEvaluated = true; + } + + void _markCircular() { + isEvaluated = true; + element.hasImplementsSelfReference = true; + + final representationType = element.representation.type; + final typeSystem = element.library.typeSystem; + + final superInterface = typeSystem.isNonNullable(representationType) + ? typeSystem.objectNone + : typeSystem.objectQuestion; + element.interfaces = [superInterface]; + } +} + +class _ImplementsWalker extends graph.DependencyWalker<_ImplementsNode> { + final Map nodeMap = Map.identity(); + + @override + void evaluate(_ImplementsNode v) { + v._evaluate(); + } + + @override + void evaluateScc(List<_ImplementsNode> scc) { + for (final node in scc) { + node._markCircular(); + } + } + + _ImplementsNode getNode(ExtensionTypeElementImpl element) { + return nodeMap[element] ??= _ImplementsNode(this, element); + } +} + class _Node extends graph.Node<_Node> { final _Walker walker; final ExtensionTypeDeclarationImpl node; @@ -122,14 +190,14 @@ class _Node extends graph.Node<_Node> { } void _markCircular() { - element.hasSelfReference = true; + element.hasRepresentationSelfReference = true; _evaluateWithType(InvalidTypeImpl.instance); } } class _Walker extends graph.DependencyWalker<_Node> { final Linker linker; - final Map nodeMap = Map.identity(); + final Map nodeMap = Map.identity(); _Walker(this.linker); diff --git a/pkg/analyzer/messages.yaml b/pkg/analyzer/messages.yaml index 6ea40772c7f..0e596f9bd41 100644 --- a/pkg/analyzer/messages.yaml +++ b/pkg/analyzer/messages.yaml @@ -5078,6 +5078,10 @@ CompileTimeErrorCode: comment: |- Parameters: 0: the display string of the disallowed type + EXTENSION_TYPE_IMPLEMENTS_ITSELF: + problemMessage: "The extension type can't implement itself." + correctionMessage: Try removing the superinterface that references this extension type. + comment: No parameters. EXTENSION_TYPE_INHERITED_MEMBER_CONFLICT: problemMessage: "The extension type '{0}' has more than one distinct member named '{1}' from implemented types." correctionMessage: Try redeclaring the corresponding member in this extension type. diff --git a/pkg/analyzer/test/src/diagnostics/extension_type_implements_itself_test.dart b/pkg/analyzer/test/src/diagnostics/extension_type_implements_itself_test.dart new file mode 100644 index 00000000000..08ffc0dc8d2 --- /dev/null +++ b/pkg/analyzer/test/src/diagnostics/extension_type_implements_itself_test.dart @@ -0,0 +1,35 @@ +// Copyright (c) 2023, 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:analyzer/src/error/codes.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import '../dart/resolution/context_collection_resolution.dart'; + +main() { + defineReflectiveSuite(() { + defineReflectiveTests(ExtensionTypeImplementsItselfTest); + }); +} + +@reflectiveTest +class ExtensionTypeImplementsItselfTest extends PubPackageResolutionTest { + test_hasCycle2() async { + await assertErrorsInCode(''' +extension type A(int it) implements B {} +extension type B(int it) implements A {} +''', [ + error(CompileTimeErrorCode.EXTENSION_TYPE_IMPLEMENTS_ITSELF, 15, 1), + error(CompileTimeErrorCode.EXTENSION_TYPE_IMPLEMENTS_ITSELF, 56, 1), + ]); + } + + test_hasCycle_self() async { + await assertErrorsInCode(''' +extension type A(int it) implements A {} +''', [ + error(CompileTimeErrorCode.EXTENSION_TYPE_IMPLEMENTS_ITSELF, 15, 1), + ]); + } +} diff --git a/pkg/analyzer/test/src/diagnostics/test_all.dart b/pkg/analyzer/test/src/diagnostics/test_all.dart index 7facc11368e..c42a882ac9a 100644 --- a/pkg/analyzer/test/src/diagnostics/test_all.dart +++ b/pkg/analyzer/test/src/diagnostics/test_all.dart @@ -248,6 +248,8 @@ import 'extension_type_declares_member_of_object_test.dart' as extension_type_declares_member_of_object; import 'extension_type_implements_disallowed_type_test.dart' as extension_type_implements_disallowed_type; +import 'extension_type_implements_itself_test.dart' + as extension_type_implements_itself; import 'extension_type_inherited_member_conflict_test.dart' as extension_type_inherited_member_conflict; import 'extension_type_representation_depends_on_itself_test.dart' @@ -1055,6 +1057,7 @@ main() { extension_type_declares_instance_field.main(); extension_type_declares_member_of_object.main(); extension_type_implements_disallowed_type.main(); + extension_type_implements_itself.main(); extension_type_inherited_member_conflict.main(); extension_type_representation_depends_on_itself.main(); external_field_constructor_initializer.main(); diff --git a/pkg/analyzer/test/src/summary/element_text.dart b/pkg/analyzer/test/src/summary/element_text.dart index f2a201a9a61..c71c8f904f4 100644 --- a/pkg/analyzer/test/src/summary/element_text.dart +++ b/pkg/analyzer/test/src/summary/element_text.dart @@ -500,7 +500,14 @@ class _ElementWriter { void _writeExtensionTypeElement(ExtensionTypeElementImpl e) { _sink.writeIndentedLine(() { - _sink.writeIf(e.hasSelfReference, 'hasSelfReference '); + _sink.writeIf( + e.hasRepresentationSelfReference, + 'hasRepresentationSelfReference ', + ); + _sink.writeIf( + e.hasImplementsSelfReference, + 'hasImplementsSelfReference ', + ); _writeName(e); }); diff --git a/pkg/analyzer/test/src/summary/elements_test.dart b/pkg/analyzer/test/src/summary/elements_test.dart index 83a26381780..ec8046bdf90 100644 --- a/pkg/analyzer/test/src/summary/elements_test.dart +++ b/pkg/analyzer/test/src/summary/elements_test.dart @@ -47202,6 +47202,66 @@ library '''); } + test_interfaces_cycle2() async { + var library = await buildLibrary(r''' +extension type A(int it) implements B {} +extension type B(int it) implements A {} +'''); + + configuration.withConstructors = false; + checkElementText(library, r''' +library + definingUnit + extensionTypes + hasImplementsSelfReference A @15 + representation: self::@extensionType::A::@field::it + typeErasure: int + interfaces + Object + fields + final it @21 + type: int + accessors + synthetic get it @-1 + returnType: int + hasImplementsSelfReference B @56 + representation: self::@extensionType::B::@field::it + typeErasure: int + interfaces + Object + fields + final it @62 + type: int + accessors + synthetic get it @-1 + returnType: int +'''); + } + + test_interfaces_cycle_self() async { + var library = await buildLibrary(r''' +extension type A(int it) implements A {} +'''); + + configuration.withConstructors = false; + checkElementText(library, r''' +library + definingUnit + extensionTypes + hasImplementsSelfReference A @15 + representation: self::@extensionType::A::@field::it + typeErasure: int + interfaces + Object + fields + final it @21 + type: int + accessors + synthetic get it @-1 + returnType: int +'''); + } + test_interfaces_extensionType() async { var library = await buildLibrary(r''' extension type A(num it) {} @@ -47510,7 +47570,7 @@ extension type B(A it) {} library definingUnit extensionTypes - hasSelfReference A @15 + hasRepresentationSelfReference A @15 representation: self::@extensionType::A::@field::it typeErasure: InvalidType interfaces @@ -47521,7 +47581,7 @@ library accessors synthetic get it @-1 returnType: InvalidType - hasSelfReference B @42 + hasRepresentationSelfReference B @42 representation: self::@extensionType::B::@field::it typeErasure: InvalidType interfaces @@ -47558,7 +47618,7 @@ library accessors synthetic get it @-1 returnType: B - hasSelfReference B @42 + hasRepresentationSelfReference B @42 representation: self::@extensionType::B::@field::it typeErasure: InvalidType interfaces @@ -47582,7 +47642,7 @@ extension type A(A it) {} library definingUnit extensionTypes - hasSelfReference A @15 + hasRepresentationSelfReference A @15 representation: self::@extensionType::A::@field::it typeErasure: InvalidType interfaces