From 55a92d88c06cba88ccbd4fe8cac652599eab83a7 Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Mon, 26 Aug 2024 11:39:11 -0700 Subject: [PATCH] Refactor "could apply" logic for extensions This logic was implemented in a few places between Extension and ElementType (and subtypes). We don't need any logic to live in ElementTypes though, since any calculations that need to be made regarding DartTypes or InterfaceTypes can be made in Extension (`alwaysApplies` and `couldApplyTo`). This fixes https://github.com/dart-lang/dartdoc/issues/3846 and dramatically simplifies the implementation. Also: * Rename `Extension.extendedType` to `Extension.extendedElement`. * Change `PackageGraph._extensions` to a Set, to remove duplication in multiple canonical libraries. * Introduce a classes test. * Delete some redundant end2end testing code. --- lib/src/element_type.dart | 58 ------- .../templates.aot_renderers_for_html.dart | 21 +-- .../templates.runtime_renderers.dart | 31 +--- lib/src/model/extension.dart | 61 ++++--- lib/src/model/inheriting_container.dart | 2 +- lib/src/model/package_graph.dart | 6 +- lib/templates/_extension.html | 2 +- lib/templates/extension.html | 4 +- lib/templates/method.html | 2 +- lib/templates/property.html | 2 +- test/classes_test.dart | 54 +++++++ test/end2end/model_test.dart | 34 +--- test/extensions_test.dart | 150 ++++++++++++++++++ testing/test_package/lib/fake.dart | 18 --- 14 files changed, 264 insertions(+), 181 deletions(-) diff --git a/lib/src/element_type.dart b/lib/src/element_type.dart index 0cf0284145..8e98ee06c5 100644 --- a/lib/src/element_type.dart +++ b/lib/src/element_type.dart @@ -10,7 +10,6 @@ library; import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/nullability_suffix.dart'; import 'package:analyzer/dart/element/type.dart'; -import 'package:analyzer/dart/element/type_system.dart'; import 'package:dartdoc/src/model/comment_referable.dart'; import 'package:dartdoc/src/model/model.dart'; import 'package:dartdoc/src/render/element_type_renderer.dart'; @@ -58,13 +57,8 @@ abstract class ElementType with CommentReferable, Nameable { @override String get breadcrumbName => throw UnimplementedError(); - DartType get instantiatedType; - Iterable get typeArguments; - bool isBoundSupertypeTo(ElementType t); - bool isSubtypeOf(ElementType t); - @override String toString() => '$type'; } @@ -112,16 +106,6 @@ class UndefinedElementType extends ElementType { @override String get nameWithGenerics => '$name$nullabilitySuffix'; - /// Assume that undefined elements don't have useful bounds. - @override - DartType get instantiatedType => type; - - @override - bool isBoundSupertypeTo(ElementType t) => false; - - @override - bool isSubtypeOf(ElementType t) => type.isBottom && !t.type.isBottom; - @override String get linkedName => name; @@ -257,9 +241,6 @@ class TypeParameterElementType extends DefinedElementType { @override String get nameWithGenerics => '$name$nullabilitySuffix'; - - @override - DartType get _bound => type.bound; } /// An [ElementType] associated with an [Element]. @@ -313,45 +294,6 @@ abstract class DefinedElementType extends ElementType { return canonicalClass.isPublic; } - TypeSystem get _typeSystem => library.element.typeSystem; - - DartType get _bound => type; - - /// This type, instantiated to bounds if it isn't already. - @override - late final DartType instantiatedType = () { - final bound = _bound; - if (bound is InterfaceType && - !bound.typeArguments.every((t) => t is InterfaceType)) { - return _typeSystem.instantiateInterfaceToBounds( - element: bound.element, nullabilitySuffix: _bound.nullabilitySuffix); - } else { - return _bound; - } - }(); - - /// Returns whether the instantiated-to-bounds type of this type is a subtype - /// of [type]. - @override - bool isSubtypeOf(ElementType type) => - _typeSystem.isSubtypeOf(instantiatedType, type.instantiatedType); - - /// Whether at least one supertype (including via mixins and interfaces) is - /// equivalent to or a subtype of `this` when instantiated to bounds. - @override - bool isBoundSupertypeTo(ElementType t) { - var type = t.instantiatedType; - if (type is InterfaceType) { - var superTypes = type.allSupertypes; - for (var superType in superTypes) { - if (_typeSystem.isSubtypeOf(superType, instantiatedType)) { - return true; - } - } - } - return false; - } - @override Iterable get typeArguments => []; diff --git a/lib/src/generator/templates.aot_renderers_for_html.dart b/lib/src/generator/templates.aot_renderers_for_html.dart index a3983e40c7..17156a9efe 100644 --- a/lib/src/generator/templates.aot_renderers_for_html.dart +++ b/lib/src/generator/templates.aot_renderers_for_html.dart @@ -690,15 +690,10 @@ String renderExtension(ExtensionTemplateData context0) {
on
-
    '''); - var context3 = context2.extendedType; - buffer.writeln(); - buffer.write(''' -
  • '''); - buffer.write(context3.linkedName); - buffer.write('''
    • '''); - buffer.writeln(); - buffer.write(''' +
      +
    • '''); + buffer.write(context2.extendedElement.linkedName); + buffer.write('''
@@ -720,7 +715,7 @@ String renderExtension(ExtensionTemplateData context0) { buffer.write(_renderExtension_partial_static_methods_10(context2)); buffer.write('\n '); buffer.write(_renderExtension_partial_static_constants_11(context2)); - var context4 = context0.extension; + var context3 = context0.extension; buffer.writeln(); buffer.write(''' @@ -1397,7 +1392,7 @@ String renderMethod(MethodTemplateData context0) { buffer.write(' '); buffer.writeEscaped(context0.parent!.kind.toString()); buffer.write(''' on '''); - buffer.write(context0.parentAsExtension.extendedType.linkedName); + buffer.write(context0.parentAsExtension.extendedElement.linkedName); buffer.write(''''''); } if (!context0.isParentExtension) { @@ -1626,7 +1621,7 @@ String renderProperty(PropertyTemplateData context0) { buffer.write(' '); buffer.writeEscaped(context0.parent!.kind.toString()); buffer.write(''' on '''); - buffer.write(context0.parentAsExtension.extendedType.linkedName); + buffer.write(context0.parentAsExtension.extendedElement.linkedName); buffer.write(''''''); } if (!context0.isParentExtension) { @@ -3645,7 +3640,7 @@ String _deduplicated_lib_templates__extension_html(Extension context0) { buffer.write(context0.linkedName); buffer.write(''' on '''); - buffer.write(context0.extendedType.linkedName); + buffer.write(context0.extendedElement.linkedName); buffer.write('\n '); buffer.write( __deduplicated_lib_templates__extension_html_partial_categorization_0( diff --git a/lib/src/generator/templates.runtime_renderers.dart b/lib/src/generator/templates.runtime_renderers.dart index 5c6dea08e1..f98d72d0c7 100644 --- a/lib/src/generator/templates.runtime_renderers.dart +++ b/lib/src/generator/templates.runtime_renderers.dart @@ -3467,18 +3467,6 @@ class _Renderer_DefinedElementType extends RendererBase { parent: r); }, ), - 'instantiatedType': Property( - getValue: (CT_ c) => c.instantiatedType, - renderVariable: (CT_ c, Property self, - List remainingNames) => - self.renderSimpleVariable(c, remainingNames, 'DartType'), - isNullValue: (CT_ c) => false, - renderValue: (CT_ c, RendererBase r, - List ast, StringSink sink) { - renderSimple(c.instantiatedType, ast, r.template, sink, - parent: r, getters: _invisibleGetters['DartType']!); - }, - ), 'isPublic': Property( getValue: (CT_ c) => c.isPublic, renderVariable: (CT_ c, Property self, @@ -4083,18 +4071,6 @@ class _Renderer_ElementType extends RendererBase { parent: r); }, ), - 'instantiatedType': Property( - getValue: (CT_ c) => c.instantiatedType, - renderVariable: (CT_ c, Property self, - List remainingNames) => - self.renderSimpleVariable(c, remainingNames, 'DartType'), - isNullValue: (CT_ c) => false, - renderValue: (CT_ c, RendererBase r, - List ast, StringSink sink) { - renderSimple(c.instantiatedType, ast, r.template, sink, - parent: r, getters: _invisibleGetters['DartType']!); - }, - ), 'isTypedef': Property( getValue: (CT_ c) => c.isTypedef, renderVariable: (CT_ c, Property self, @@ -4611,8 +4587,8 @@ class _Renderer_Extension extends RendererBase { parent: r); }, ), - 'extendedType': Property( - getValue: (CT_ c) => c.extendedType, + 'extendedElement': Property( + getValue: (CT_ c) => c.extendedElement, renderVariable: (CT_ c, Property self, List remainingNames) { if (remainingNames.isEmpty) { @@ -4629,7 +4605,8 @@ class _Renderer_Extension extends RendererBase { isNullValue: (CT_ c) => false, renderValue: (CT_ c, RendererBase r, List ast, StringSink sink) { - _render_ElementType(c.extendedType, ast, r.template, sink, + _render_ElementType( + c.extendedElement, ast, r.template, sink, parent: r); }, ), diff --git a/lib/src/model/extension.dart b/lib/src/model/extension.dart index 4d501bb1b2..bb6fef4207 100644 --- a/lib/src/model/extension.dart +++ b/lib/src/model/extension.dart @@ -3,11 +3,11 @@ // BSD-style license that can be found in the LICENSE file. import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/dart/element/nullability_suffix.dart'; import 'package:analyzer/dart/element/type.dart'; import 'package:dartdoc/src/element_type.dart'; import 'package:dartdoc/src/model/comment_referable.dart'; import 'package:dartdoc/src/model/model.dart'; -import 'package:dartdoc/src/type_utils.dart'; import 'package:meta/meta.dart'; /// Static extension on a given type, containing methods (including getters, @@ -16,37 +16,50 @@ class Extension extends Container { @override final ExtensionElement element; - late final ElementType extendedType = + late final ElementType extendedElement = getTypeFor(element.extendedType, library); Extension(this.element, super.library, super.packageGraph); - /// Detect if this extension applies to every object. - bool get alwaysApplies => - extendedType.instantiatedType is DynamicType || - extendedType.instantiatedType is VoidType || - extendedType.instantiatedType.isDartCoreObject; - - bool couldApplyTo(T c) => - _couldApplyTo(c.modelType); + /// Whether this extension applies to every static type. + bool get alwaysApplies { + var extendedType = extendedElement.type; + if (extendedType is TypeParameterType) extendedType = extendedType.bound; + return extendedType is DynamicType || + extendedType is VoidType || + extendedType.isDartCoreObject; + } - /// Whether this extension could apply to [type]. - bool _couldApplyTo(DefinedElementType type) { - if (extendedType.instantiatedType is DynamicType || - extendedType.instantiatedType is VoidType) { - return true; + /// Whether this extension could apply to [container]. + /// + /// This makes some assumptions in its calculations. For example, all + /// [InheritingContainer]s represent [InterfaceElement]s, so no care is taken + /// to consider function types or record types. + bool couldApplyTo(InheritingContainer container) { + var extendedType = extendedElement.type; + if (extendedType is TypeParameterType) { + extendedType = extendedType.bound; } - var typeInstantiated = type.instantiatedType; - var extendedInstantiated = extendedType.instantiatedType; - if (typeInstantiated == extendedInstantiated) { + if (extendedType is DynamicType || extendedType is VoidType) { return true; } - if (typeInstantiated.documentableElement == - extendedInstantiated.documentableElement && - extendedType.isSubtypeOf(type)) { - return true; + var otherType = container.modelType.type; + if (otherType is InterfaceType) { + otherType = library.element.typeSystem.instantiateInterfaceToBounds( + element: otherType.element, + nullabilitySuffix: NullabilitySuffix.none, + ); + + for (var superType in [otherType, ...otherType.allSupertypes]) { + var isSameBaseType = superType.element == extendedType.element; + if (isSameBaseType && + library.element.typeSystem.isSubtypeOf(extendedType, superType)) { + return true; + } + } } - return extendedType.isBoundSupertypeTo(type); + + return false; } @override @@ -100,7 +113,7 @@ class Extension extends Container { @override Map get referenceChildren { return _referenceChildren ??= { - ...extendedType.referenceChildren, + ...extendedElement.referenceChildren, // Override extendedType entries with local items. ...super.referenceChildren, }; diff --git a/lib/src/model/inheriting_container.dart b/lib/src/model/inheriting_container.dart index 58b1cf9604..bd30943e16 100644 --- a/lib/src/model/inheriting_container.dart +++ b/lib/src/model/inheriting_container.dart @@ -271,7 +271,7 @@ abstract class InheritingContainer extends Container { /// defined by [element] can exist where this extension applies, not including /// any extension that applies to every type. late final List potentiallyApplicableExtensionsSorted = - packageGraph.extensions.whereDocumented + packageGraph.extensions .where((e) => !e.alwaysApplies) .where((e) => e.couldApplyTo(this)) .toList(growable: false) diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart index 63ce7055d4..93d1897e8c 100644 --- a/lib/src/model/package_graph.dart +++ b/lib/src/model/package_graph.dart @@ -160,7 +160,9 @@ class PackageGraph with CommentReferable, Nameable { _addToImplementers(library.classesAndExceptions); _addToImplementers(library.mixins); _addToImplementers(library.extensionTypes); - _extensions.addAll(library.extensions); + if (library.isCanonical) { + _extensions.addAll(library.extensions.whereDocumented); + } } if (package.isLocal && !package.hasPublicLibraries) { package.warn(PackageWarning.noDocumentableLibrariesInPackage); @@ -383,7 +385,7 @@ class PackageGraph with CommentReferable, Nameable { clazz.definingContainer.hashCode); /// A list of extensions that exist in the package graph. - final List _extensions = []; + final Set _extensions = {}; /// Name of the default package. String get defaultPackageName => packageMeta.name; diff --git a/lib/templates/_extension.html b/lib/templates/_extension.html index f0c8a2877d..8ba73275d4 100644 --- a/lib/templates/_extension.html +++ b/lib/templates/_extension.html @@ -1,6 +1,6 @@
{{{ linkedName }}} - on {{{ extendedType.linkedName }}} + on {{{ extendedElement.linkedName }}} {{ >categorization }}
diff --git a/lib/templates/extension.html b/lib/templates/extension.html index 58925e2a4c..8c53713c11 100644 --- a/lib/templates/extension.html +++ b/lib/templates/extension.html @@ -15,9 +15,7 @@
on
    - {{ #extendedType }} -
  • {{{ linkedName }}}
    • - {{ /extendedType }} +
  • {{{ extendedElement.linkedName }}}
diff --git a/lib/templates/method.html b/lib/templates/method.html index e15ab5bc28..4fd419c494 100644 --- a/lib/templates/method.html +++ b/lib/templates/method.html @@ -24,7 +24,7 @@