Refactor "could apply" logic for extensions (#3847)

srawlins · web-flow · commit dc566301f6fa · 2024-08-27T09:52:49.000-07:00
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 #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.
diff --git 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<ElementType> 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<ElementType> get typeArguments => [];
 
diff --git 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<T extends Extension>(ExtensionTemplateData<T> context0) {
       <dl class="dl-horizontal">
         <dt>on</dt>
         <dd>
-          <ul class="comma-separated clazz-relationships">''');
-  var context3 = context2.extendedType;
-  buffer.writeln();
-  buffer.write('''
-              <li>''');
-  buffer.write(context3.linkedName);
-  buffer.write('''</li>''');
-  buffer.writeln();
-  buffer.write('''
+          <ul class="comma-separated clazz-relationships">
+            <li>''');
+  buffer.write(context2.extendedElement.linkedName);
+  buffer.write('''</li>
           </ul>
         </dd>
       </dl>
@@ -720,7 +715,7 @@ String renderExtension<T extends Extension>(ExtensionTemplateData<T> 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('''</h5>''');
   }
   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('''</h5>''');
   }
   if (!context0.isParentExtension) {
@@ -3645,7 +3640,7 @@ String _deduplicated_lib_templates__extension_html(Extension context0) {
   buffer.write(context0.linkedName);
   buffer.write('''</span>
   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
@@ -3467,18 +3467,6 @@ class _Renderer_DefinedElementType extends RendererBase<DefinedElementType> {
                         parent: r);
                   },
                 ),
-                'instantiatedType': Property(
-                  getValue: (CT_ c) => c.instantiatedType,
-                  renderVariable: (CT_ c, Property<CT_> self,
-                          List<String> remainingNames) =>
-                      self.renderSimpleVariable(c, remainingNames, 'DartType'),
-                  isNullValue: (CT_ c) => false,
-                  renderValue: (CT_ c, RendererBase<CT_> r,
-                      List<MustachioNode> 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<CT_> self,
@@ -4083,18 +4071,6 @@ class _Renderer_ElementType extends RendererBase<ElementType> {
                         parent: r);
                   },
                 ),
-                'instantiatedType': Property(
-                  getValue: (CT_ c) => c.instantiatedType,
-                  renderVariable: (CT_ c, Property<CT_> self,
-                          List<String> remainingNames) =>
-                      self.renderSimpleVariable(c, remainingNames, 'DartType'),
-                  isNullValue: (CT_ c) => false,
-                  renderValue: (CT_ c, RendererBase<CT_> r,
-                      List<MustachioNode> 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<CT_> self,
@@ -4611,8 +4587,8 @@ class _Renderer_Extension extends RendererBase<Extension> {
                         parent: r);
                   },
                 ),
-                'extendedType': Property(
-                  getValue: (CT_ c) => c.extendedType,
+                'extendedElement': Property(
+                  getValue: (CT_ c) => c.extendedElement,
                   renderVariable:
                       (CT_ c, Property<CT_> self, List<String> remainingNames) {
                     if (remainingNames.isEmpty) {
@@ -4629,7 +4605,8 @@ class _Renderer_Extension extends RendererBase<Extension> {
                   isNullValue: (CT_ c) => false,
                   renderValue: (CT_ c, RendererBase<CT_> r,
                       List<MustachioNode> 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
@@ -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 extends InheritingContainer>(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<String, CommentReferable> 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
@@ -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<Extension> 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
@@ -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<Extension> _extensions = [];
+  final Set<Extension> _extensions = {};
 
   /// Name of the default package.
   String get defaultPackageName => packageMeta.name;
diff --git a/lib/templates/_extension.html b/lib/templates/_extension.html
@@ -1,6 +1,6 @@
 <dt id="{{ htmlId }}">
   <span class="name {{ #isDeprecated }}deprecated{{ /isDeprecated }}">{{{ linkedName }}}</span>
-  on {{{ extendedType.linkedName }}}
+  on {{{ extendedElement.linkedName }}}
   {{ >categorization }}
 </dt>
 <dd>
diff --git a/lib/templates/extension.html b/lib/templates/extension.html
@@ -15,9 +15,7 @@
         <dt>on</dt>
         <dd>
           <ul class="comma-separated clazz-relationships">
-            {{ #extendedType }}
-              <li>{{{ linkedName }}}</li>
-            {{ /extendedType }}
+            <li>{{{ extendedElement.linkedName }}}</li>
           </ul>
         </dd>
       </dl>
diff --git a/lib/templates/method.html b/lib/templates/method.html
@@ -24,7 +24,7 @@
   <div id="dartdoc-sidebar-left" class="sidebar sidebar-offcanvas-left">
     {{ >search_sidebar }}
     {{ #isParentExtension }}
-    <h5>{{ parent.name }} {{ parent.kind }} on {{{ parentAsExtension.extendedType.linkedName }}}</h5>
+    <h5>{{ parent.name }} {{ parent.kind }} on {{{ parentAsExtension.extendedElement.linkedName }}}</h5>
     {{ /isParentExtension }}
     {{ ^isParentExtension }}
     <h5>{{ parent.name }} {{ parent.kind }}</h5>
diff --git a/lib/templates/property.html b/lib/templates/property.html
@@ -36,7 +36,7 @@
 <div id="dartdoc-sidebar-left" class="sidebar sidebar-offcanvas-left">
   {{ >search_sidebar }}
   {{ #isParentExtension }}
-  <h5>{{ parent.name }} {{ parent.kind }} on {{{ parentAsExtension.extendedType.linkedName }}}</h5>
+  <h5>{{ parent.name }} {{ parent.kind }} on {{{ parentAsExtension.extendedElement.linkedName }}}</h5>
   {{ /isParentExtension }}
   {{ ^isParentExtension }}
   <h5>{{ parent.name }} {{ parent.kind }}</h5>
diff --git a/test/classes_test.dart b/test/classes_test.dart
diff --git a/test/end2end/model_test.dart b/test/end2end/model_test.dart
diff --git a/test/extensions_test.dart b/test/extensions_test.dart
diff --git a/testing/test_package/lib/fake.dart b/testing/test_package/lib/fake.dart
