Skip to content

Calculate what extensions could apply with an extension tree #2061

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

Calculate what extensions could apply with an extension tree #2061

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3d11f79
intermediate -- tests pass
jcollins-g Nov 4, 2019
083150c
First version of extension tree complete
jcollins-g Nov 5, 2019
4bbcc48
it works
jcollins-g Nov 6, 2019
3f5f659
It actually works, with tests that are debugged
jcollins-g Nov 7, 2019
3cb6071
Cleanups
jcollins-g Nov 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions lib/src/element_type.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,11 @@ class TypeParameterElementType extends DefinedElementType {
}
return _nameWithGenerics;
}

@override
ClassElement get _boundClassElement => interfaceType.element;
@override
InterfaceType get interfaceType => (type as TypeParameterType).bound;
}

/// An [ElementType] associated with an [Element].
Expand Down Expand Up @@ -311,6 +316,26 @@ abstract class DefinedElementType extends ElementType {
}
return _typeArguments;
}

/// By default, the bound is the type of the declared class.
ClassElement get _boundClassElement => (element.element as ClassElement);
Class get boundClass =>
ModelElement.fromElement(_boundClassElement, packageGraph);
InterfaceType get interfaceType => type;

InterfaceType _instantiatedType;

/// Return this type, instantiated to bounds if it isn't already.
DartType get instantiatedType {
if (_instantiatedType == null) {
if (!interfaceType.typeArguments.every((t) => t is InterfaceType)) {
_instantiatedType = packageGraph.typeSystem.instantiateToBounds(interfaceType);
} else {
_instantiatedType = interfaceType;
}
}
return _instantiatedType;
}
}

/// Any callable ElementType will mix-in this class, whether anonymous or not.
Expand Down Expand Up @@ -446,8 +471,8 @@ class CallableGenericTypeAliasElementType extends ParameterizedElementType
@override
ElementType get returnType {
if (_returnType == null) {
_returnType = ElementType.from(
type.returnType, library, packageGraph, this);
_returnType =
ElementType.from(type.returnType, library, packageGraph, this);
}
return _returnType;
}
Expand Down
221 changes: 221 additions & 0 deletions lib/src/extension_tree.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
// Copyright (c) 2019, 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.

/// A structure to keep track of extensions and their applicability efficiently.
library dartdoc.extension_tree;

import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:dartdoc/src/element_type.dart';
import 'package:dartdoc/src/model.dart';
import 'package:dartdoc/src/special_elements.dart';

/// This class defines a node in an [Extension] tree. A hybrid of a heap and
/// a type tree, an extension tree's nodes are arranged to the following
/// invariants on public methods:
///
/// - Each node contains the set of extensions declared as being directly on
/// [extendedType], or an equivalent type.
/// - All parents, recursively, of a node and the node itself contain extensions
/// that could apply to any instantiated type of [extendedType].
///
/// An extension "could apply" to a type if there exists some valid
/// instantiation of that type where the extension would be applicable if
/// made accessible in the namespace per:
/// https://github.com/dart-lang/language/blob/master/accepted/2.6/static-extension-members/feature-specification.md#implicit-extension-member-invocation
class ExtensionNode {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced that a tree structure is adequate, given that it appears to follow the type hierarchy but the type hierarchy is a graph structure. If the declarations look like

class A {}
class B extends A {}

then, as I understand it, there would be a node for A and a node for B that is a child of the node for A. But if the declarations look like

class A {}
class B {}
class C implements A, B {}

then there would be nodes for A, B, and C, and C would need to be a child of both the node for A and the node for B, which a tree doesn't allow.

In analyzer we just compute a list of the extensions visible within a single library, then recompute the set of applicable extensions based on a given type without trying to build a graph of extensions. I think that's a cleaner approach, but ymmv.

/// The set of extensions that directly apply to [extendedType].
final Set<Extension> extensions;

/// The set of ExtensionNodes containing [extensions] that apply only to
/// more specific types than [extendedType].
final Set<ExtensionNode> children = {};

/// The set of seen [ModelElement]s backing [extendedType]s in this tree.
/// Only valid at the root.
Set<ModelElement> _seen;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean by "Only valid at the root.". If you mean that only the node at the root of the tree will have a value for this field and other nodes will have null values, then consider either creating a subclass such as ExtensionRootNode that's only used for the root node that defines this field, or create an ExtensionTree that holds both this field and the root node. The space savings will probably be minimal, but it will allow you to make this a non-nullable field in the future. Ditto for _genericsInserted.


/// The set of seen [ModelElement]s that have had empty nodes inserted in the
/// tree if appropriate, or simply seen more than once if not. Subset of
/// [_seen].
Set<ModelElement> _genericsInserted;

/// The type all [extensions] are extending.
final DefinedElementType extendedType;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the purposes of dartdoc, could this just be the element associated with the extended type?


/// The [PackageGraph] associated with the tree; must be the same for all
/// nodes.
final PackageGraph packageGraph;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you do have a tree / root node, then this field can be moved there as well and a getter added to walk up the parent chain to find it. That would ensure that it's the same for all nodes.


ExtensionNode(this.extendedType, this.packageGraph, this.extensions);

/// Creates an initially empty root node for [Object].
factory ExtensionNode.fromRoot(PackageGraph packageGraph) => ExtensionNode(
packageGraph.specialClasses[SpecialClass.object].modelType,
packageGraph, {});

/// A node containing a more general type than [extendedType].
ExtensionNode parent;

/// Do not call addExtension while iterating over the return value to
/// this function.
Iterable<Extension> allCouldApplyTo(Class c) sync* {
if (couldApplyTo(c.modelType)) {
yield* extensions;
yield* children.expand((e) => e.allCouldApplyTo(c));
}
}

/// Insert a node for the instantiated-to-bound type of [newExtendedType]s
/// class if there are a lot of type-specific extensions for the same class.
/// Makes the structure more efficient in the C++ template-emulation use case
/// for extension methods.
void _maybeInsertGenericNode(DefinedElementType newExtendedType) {
if (!(_seen ??= {}).contains(newExtendedType.element)) {
_seen.add(newExtendedType.element);
return;
}
if (!(_genericsInserted ??= {}).contains(newExtendedType.element)) {
ElementType genericType = newExtendedType.element.modelType;
if (genericType is DefinedElementType) {
if (genericType.typeArguments.isNotEmpty &&
genericType.type != extendedType.type) {
ElementType genericElementType = ElementType.from(
genericType.instantiatedType,
newExtendedType.element.library,
packageGraph);
_addExtensionNode(ExtensionNode(genericElementType, packageGraph, {}));
}
}
_genericsInserted.add(newExtendedType.element);
}
}

/// Returns the added or modified extension node. If the extension is
/// already in the tree, will return the node it was found in.
ExtensionNode addExtension(Extension newExtension) {
assert(identical(newExtension.packageGraph, packageGraph));
ExtensionNode newNode =
ExtensionNode(newExtension.extendedType, packageGraph, {newExtension});
if (parent == null) _maybeInsertGenericNode(newExtension.extendedType);
return _addExtensionNode(newNode);
}

ExtensionNode _addExtensionNode(ExtensionNode newNode) {
if (extendedType.instantiatedType ==
newNode.extendedType.instantiatedType) {
// Extended on the exact same type. Add to this set.
_merge(newNode);
return this;
}

Set<ExtensionNode> foundApplicable = {};
for (ExtensionNode child in children) {
if (child.couldApplyTo(newNode.extendedType)) {
// This child should be a child of, or the same type as the new node.
foundApplicable.add(child);
}
}

for (ExtensionNode applicable in foundApplicable) {
applicable._detach();
if (applicable.extendedType.instantiatedType ==
newNode.extendedType.instantiatedType) {
newNode._merge(applicable);
} else {
if (applicable.isSubtypeOf(newNode.extendedType)) {
newNode._addChild(applicable);
} else if (newNode.isSubtypeOf(applicable.extendedType) ||
newNode.isBoundSuperclassTo(applicable.extendedType)) {
_addChild(applicable);
return applicable._addExtensionNode(newNode);
}
}
}

for (ExtensionNode child in children) {
if (newNode.couldApplyTo(child.extendedType)) {
return child._addExtensionNode(newNode);
}
}

_addChild(newNode);
return newNode;
}

/// Add a child, unconditionally.
void _addChild(ExtensionNode newChild) {
children.add(newChild);
newChild.parent = this;
}

/// The instantiated to bounds [extendedType] of this node is a supertype of
/// [t].
bool isSupertypeOf(DefinedElementType t) => packageGraph.typeSystem
.isSubtypeOf(t.instantiatedType, extendedType.instantiatedType);

/// The instantiated to bounds [extendedType] of this node is a subtype of
/// [t].
bool isSubtypeOf(DefinedElementType t) => packageGraph.typeSystem
.isSubtypeOf(extendedType.instantiatedType, t.instantiatedType);

/// The class from this [extendedType]:
///
/// 1) is a superclass of the class associated with [t] and
/// 2) any type parameters from [t] instantiated with that superclass type
/// result in a subtype of [t].
bool isBoundSuperclassTo(DefinedElementType t) {
InterfaceType supertype = (t.type.element is ClassElement
? (t.type.element as ClassElement)?.supertype
: null);
ClassElement superclass = supertype?.element;
while (superclass != null) {
if (superclass == extendedType.type.element &&
(supertype == extendedType.instantiatedType ||
packageGraph.typeSystem
.isSubtypeOf(supertype, extendedType.instantiatedType))) {
return true;
}
supertype = superclass.supertype;
superclass = supertype?.element;
}
return false;
}

/// Return true if the [extensions] in this node could apply to [t].
/// If true, the children of this node may also apply but must be checked
/// individually with their [couldApplyTo] methods. If false, neither this
/// node's extensions nor its children could apply.
bool couldApplyTo(DefinedElementType t) {
return t.instantiatedType == extendedType.instantiatedType ||
(t.instantiatedType.element == extendedType.instantiatedType.element &&
isSubtypeOf(t)) ||
isBoundSuperclassTo(t);
}

/// Remove this subtree from its parent node, unconditionally.
void _detach() {
parent.children.remove(this);
parent == null;
}

/// Merge from [other] node into [this], unconditionally.
void _merge(ExtensionNode other) {
//assert(other.extendedType.type == extendedType.type);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uncomment or remove?

extensions.addAll(other.extensions);
children.addAll(other.children);
}

@override
int get hashCode => extendedType.type.hashCode;

@override
bool operator ==(other) => extendedType.type == other.extendedType.type;

@override
/// Intended for debugging only.
/// Format : {ExtensionName1, ExtensionName2, ...} => extendedType (extendedType.instantiatedType)
String toString() =>
'{${extensions.map((e) => e.name).join(', ')}} => ${extendedType.toString()} (${extendedType.instantiatedType.toString()})';
}
24 changes: 14 additions & 10 deletions lib/src/markdown_processor.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,11 +206,13 @@ MatchingLinkResult _getMatchingLinkElement(
if (refModelElement == null && element is ModelElement) {
Container preferredClass = _getPreferredClass(element);
if (preferredClass is Extension) {
element.warn(PackageWarning.notImplemented, message: 'Comment reference resolution inside extension methods is not yet implemented');
element.warn(PackageWarning.notImplemented,
message:
'Comment reference resolution inside extension methods is not yet implemented');
} else {
refModelElement =
_MarkdownCommentReference(codeRef, element, commentRefs, preferredClass)
.computeReferredElement();
refModelElement = _MarkdownCommentReference(
codeRef, element, commentRefs, preferredClass)
.computeReferredElement();
}
}

Expand Down Expand Up @@ -649,8 +651,8 @@ class _MarkdownCommentReference {
Inheritable overriddenElement =
(element as Inheritable).overriddenElement;
while (overriddenElement != null) {
tryClasses.add(
(element as Inheritable).overriddenElement.enclosingElement);
tryClasses
.add((element as Inheritable).overriddenElement.enclosingElement);
overriddenElement = overriddenElement.overriddenElement;
}
}
Expand All @@ -669,7 +671,7 @@ class _MarkdownCommentReference {

if (results.isEmpty && realClass != null) {
for (Class superClass
in realClass.publicSuperChain.map((et) => et.element as Class)) {
in realClass.publicSuperChain.map((et) => et.element)) {
if (!tryClasses.contains(superClass)) {
_getResultsForClass(superClass);
}
Expand Down Expand Up @@ -719,12 +721,12 @@ class _MarkdownCommentReference {
// TODO(jcollins-g): get rid of reimplementation of identifier resolution
// or integrate into ModelElement in a simpler way.
List<Class> superChain = [tryClass];
superChain.addAll(tryClass.interfaces.map((t) => t.element as Class));
superChain.addAll(tryClass.interfaces.map((t) => t.element));
// This seems duplicitous with our caller, but the preferredClass
// hint matters with findCanonicalModelElementFor.
// TODO(jcollins-g): This makes our caller ~O(n^2) vs length of superChain.
// Fortunately superChains are short, but optimize this if it matters.
superChain.addAll(tryClass.superChain.map((t) => t.element as Class));
superChain.addAll(tryClass.superChain.map((t) => t.element));
for (final c in superChain) {
_getResultsForSuperChainElement(c, tryClass);
if (results.isNotEmpty) break;
Expand Down Expand Up @@ -778,7 +780,9 @@ String _linkDocReference(String codeRef, Warnable warnable,
// current element.
warnable.warn(PackageWarning.unresolvedDocReference,
message: codeRef,
referredFrom: warnable.documentationIsLocal ? null : warnable.documentationFrom);
referredFrom: warnable.documentationIsLocal
? null
: warnable.documentationFrom);
}
return '<code>${htmlEscape.convert(codeRef)}</code>';
}
Expand Down
Loading