Skip to content

Commit

Permalink
Refactor "could apply" logic for extensions (#3847)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
srawlins authored Aug 27, 2024
1 parent e233b2b commit dc56630
Show file tree
Hide file tree
Showing 14 changed files with 264 additions and 181 deletions.
58 changes: 0 additions & 58 deletions lib/src/element_type.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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';
}
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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].
Expand Down Expand Up @@ -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 => [];

Expand Down
21 changes: 8 additions & 13 deletions lib/src/generator/templates.aot_renderers_for_html.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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('''
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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(
Expand Down
31 changes: 4 additions & 27 deletions lib/src/generator/templates.runtime_renderers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
},
),
Expand Down
61 changes: 37 additions & 24 deletions lib/src/model/extension.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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,
};
Expand Down
2 changes: 1 addition & 1 deletion lib/src/model/inheriting_container.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 4 additions & 2 deletions lib/src/model/package_graph.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion lib/templates/_extension.html
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<dt id="{{ htmlId }}">
<span class="name {{ #isDeprecated }}deprecated{{ /isDeprecated }}">{{{ linkedName }}}</span>
on {{{ extendedType.linkedName }}}
on {{{ extendedElement.linkedName }}}
{{ >categorization }}
</dt>
<dd>
Expand Down
4 changes: 1 addition & 3 deletions lib/templates/extension.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion lib/templates/method.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion lib/templates/property.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Loading

0 comments on commit dc56630

Please sign in to comment.