From 3d11f797b0d4e736e623faba6c47504c61e662bd Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Mon, 4 Nov 2019 14:46:30 -0800 Subject: [PATCH 1/8] intermediate -- tests pass --- lib/src/element_type.dart | 27 ++++++++++++++++++++-- lib/src/extension_tree.dart | 40 +++++++++++++++++++++++++++++++++ lib/src/markdown_processor.dart | 24 +++++++++++--------- lib/src/model.dart | 25 ++++----------------- 4 files changed, 83 insertions(+), 33 deletions(-) create mode 100644 lib/src/extension_tree.dart diff --git a/lib/src/element_type.dart b/lib/src/element_type.dart index a373630712..0248db9928 100644 --- a/lib/src/element_type.dart +++ b/lib/src/element_type.dart @@ -10,6 +10,8 @@ import 'dart:collection'; 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/src/dart/element/type.dart' show InterfaceTypeImpl; +import 'package:analyzer/src/generated/type_system.dart' show Dart2TypeSystem; import 'package:dartdoc/src/model.dart'; import 'package:dartdoc/src/model_utils.dart'; @@ -311,6 +313,27 @@ abstract class DefinedElementType extends ElementType { } return _typeArguments; } + + InterfaceTypeImpl _instantiatedType; + + /// Return this type, instantiated to upper-bounds. + DartType get instantiatedType { + if (_instantiatedType == null) { + assert(element is Class || element is ModelFunctionTyped); + Dart2TypeSystem typeSystem = packageGraph.typeSystem; + _instantiatedType = typeSystem + .instantiateToBounds((element.element as ClassElement).thisType); + _instantiatedType = (element.element as ClassElement).instantiate( + typeArguments: _instantiatedType.typeArguments.map((a) { + if (a.isDynamic) { + return typeSystem.typeProvider.neverType; + } + return a; + }).toList(), + nullabilitySuffix: _instantiatedType.nullabilitySuffix); + } + return _instantiatedType; + } } /// Any callable ElementType will mix-in this class, whether anonymous or not. @@ -446,8 +469,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; } diff --git a/lib/src/extension_tree.dart b/lib/src/extension_tree.dart new file mode 100644 index 0000000000..23ffba3208 --- /dev/null +++ b/lib/src/extension_tree.dart @@ -0,0 +1,40 @@ +// 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 'dart:collection'; + +import 'package:analyzer/dart/element/type.dart'; +import 'package:dartdoc/src/model.dart'; + +abstract class ExtensionNodeVisitor { + R visit(ExtensionNode e); +} + +class ExtensionNode { + /// The set of extensions that directly apply to [typeOn]. + final Set extensions = {}; + + /// The set of ExtensionNodes containing [extensions] that apply only to + /// more specific types than [typeOn]. + final Set children = {}; + + /// The type all [extensions] are extending. + final DartType typeOn; + + ExtensionNode(this.typeOn); + + /// A node containing a more general type than [typeOn]. + ExtensionNode parent; + + R accept(ExtensionNodeVisitor v) => v.visit(this); + + @override + int get hashCode => typeOn.hashCode; + + @override + bool operator ==(other) => typeOn == other.typeOn; +} diff --git a/lib/src/markdown_processor.dart b/lib/src/markdown_processor.dart index 06890a372b..cfa3014f2e 100644 --- a/lib/src/markdown_processor.dart +++ b/lib/src/markdown_processor.dart @@ -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(); } } @@ -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; } } @@ -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); } @@ -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 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; @@ -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 '${htmlEscape.convert(codeRef)}'; } diff --git a/lib/src/model.dart b/lib/src/model.dart index a02b983d9a..5b97b5db28 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -36,14 +36,12 @@ import 'package:analyzer/src/dart/element/element.dart'; import 'package:analyzer/src/dart/element/inheritance_manager3.dart'; import 'package:analyzer/src/dart/element/member.dart' show ExecutableMember, Member, ParameterMember; -import 'package:analyzer/src/dart/element/type.dart' show InterfaceTypeImpl; import 'package:analyzer/src/dart/sdk/sdk.dart'; import 'package:analyzer/src/generated/engine.dart'; import 'package:analyzer/src/generated/java_io.dart'; import 'package:analyzer/src/generated/sdk.dart'; import 'package:analyzer/src/generated/source.dart'; import 'package:analyzer/src/generated/source_io.dart'; -import 'package:analyzer/src/generated/type_system.dart' show Dart2TypeSystem; import 'package:analyzer/src/source/package_map_resolver.dart'; import 'package:analyzer/src/source/sdk_ext.dart'; import 'package:args/args.dart'; @@ -1347,24 +1345,9 @@ class Extension extends Container /// Returns [true] if there is an instantiation of [c] to which this extension /// could be applied. bool couldApplyTo(Class c) => - _couldApplyTo(extendedType.type, c.element, packageGraph.typeSystem); - - static bool _couldApplyTo( - DartType extendedType, ClassElement element, Dart2TypeSystem typeSystem) { - InterfaceTypeImpl classInstantiated = - typeSystem.instantiateToBounds(element.thisType); - classInstantiated = element.instantiate( - typeArguments: classInstantiated.typeArguments.map((a) { - if (a.isDynamic) { - return typeSystem.typeProvider.neverType; - } - return a; - }).toList(), - nullabilitySuffix: classInstantiated.nullabilitySuffix); - - return (classInstantiated.element == extendedType.element) || - typeSystem.isSubtypeOf(classInstantiated, extendedType); - } + (c.element == extendedType.element.element) || + packageGraph.typeSystem + .isSubtypeOf(c.modelType.instantiatedType, extendedType.type); @override ModelElement get enclosingElement => library; @@ -1418,7 +1401,7 @@ class Extension extends Container } @override - DefinedElementType get modelType => super.modelType; + ParameterizedElementType get modelType => super.modelType; List _allModelElements; From 083150c95e07d56df91fb80911c9e27d024e137e Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Tue, 5 Nov 2019 10:12:44 -0800 Subject: [PATCH 2/8] First version of extension tree complete --- lib/src/extension_tree.dart | 126 ++++++++++++++++++++++++++++++++---- lib/src/model.dart | 4 +- 2 files changed, 115 insertions(+), 15 deletions(-) diff --git a/lib/src/extension_tree.dart b/lib/src/extension_tree.dart index 23ffba3208..84c7cda487 100644 --- a/lib/src/extension_tree.dart +++ b/lib/src/extension_tree.dart @@ -8,33 +8,133 @@ library dartdoc.extension_tree; import 'dart:collection'; 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'; -abstract class ExtensionNodeVisitor { - R visit(ExtensionNode e); -} - +/// This class defines a node in an [Extension] tree. An extension tree's +/// nodes are arranged given the following: +/// +/// - Each node contains the set of extensions declared as being directly on +/// [extendedType]. +/// - All parents, recursively, of a node contain extensions that could +/// apply to any instantiated-to-upper-bounds type of [extendedType.element]. +/// +/// A tree of ExtensionNodes will have a root node associated with the [Object] +/// type and more specific types will be further down the tree. class ExtensionNode { - /// The set of extensions that directly apply to [typeOn]. - final Set extensions = {}; + /// The set of extensions that directly apply to [extendedType]. + final Set extensions; /// The set of ExtensionNodes containing [extensions] that apply only to - /// more specific types than [typeOn]. + /// more specific types than [extendedType]. final Set children = {}; /// The type all [extensions] are extending. - final DartType typeOn; + final DefinedElementType extendedType; + + /// The [PackageGraph] associated with the tree; must be the same for all + /// nodes. + final PackageGraph packageGraph; - ExtensionNode(this.typeOn); + ExtensionNode(this.extendedType, this.packageGraph, this.extensions); - /// A node containing a more general type than [typeOn]. + /// 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; - R accept(ExtensionNodeVisitor v) => v.visit(this); + /// Do not call addExtension while iterating over the return value to + /// this function. + Iterable allCouldApplyTo(Class c) sync* { + if (couldApplyTo(c)) { + yield* extensions; + yield* children.expand((e) => e.allCouldApplyTo(c)); + } + } + + /// 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)); + if (extendedType.element is! Class) { + throw UnimplementedError( + 'Extension tree does not yet know about extensions not on classes'); + } + assert(extendedType.element is Class); + assert(extendedType.element is Documentable); + ExtensionNode newNode = + ExtensionNode(newExtension.extendedType, packageGraph, {newExtension}); + return _addExtensionNode(newNode); + } + + ExtensionNode _addExtensionNode(ExtensionNode newNode) { + if (extendedType.type == newNode.extendedType.type) { + // Extended on the exact same type. Add to this set. + _merge(newNode); + return this; + } + assert(!newNode.couldApplyTo(extendedType.element)); + + Set foundApplicable = {}; + for (ExtensionNode child in children) { + if (child.couldApplyTo(extendedType.element)) { + // This child should be a parent of, or the same type as the new node. + foundApplicable.add(child); + } + } + + assert(foundApplicable.length <= 1, + 'tree structure invalid, multiple possible parents found'); + if (foundApplicable.isNotEmpty) { + return foundApplicable.first._addExtensionNode(newNode); + } + // Some children of this node may need to be reparented under this + // node. + Set reparentThese = {}; + for (ExtensionNode child in children) { + if (newNode.couldApplyTo(child.extendedType.element)) { + reparentThese.add(child); + } + } + for (ExtensionNode reparentMe in reparentThese) { + reparentMe._detach(); + newNode._addChild(reparentMe); + } + _addChild(newNode); + return newNode; + } + + /// Add a child, unconditionally. + void _addChild(ExtensionNode newChild) { + children.add(newChild); + newChild.parent = this; + } + + bool couldApplyTo(Class c) => + (c.element == extendedType.element.element) || + packageGraph.typeSystem + .isSubtypeOf(c.modelType.instantiatedType, extendedType.type); + + /// Remove this subtree from its parent node, unconditionally. + void _detach() { + parent.children.remove(this); + parent == null; + } + + /// Merge this node into [other], unconditionally. + void _merge(ExtensionNode other) { + assert(other.extendedType.type == extendedType.type); + other.extensions.addAll(extensions); + other.children.addAll(children); + } @override - int get hashCode => typeOn.hashCode; + int get hashCode => extendedType.type.hashCode; @override - bool operator ==(other) => typeOn == other.typeOn; + bool operator ==(other) => extendedType.type == other.extendedType.type; } diff --git a/lib/src/model.dart b/lib/src/model.dart index 5b97b5db28..dfe0b54a46 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -1342,8 +1342,8 @@ class Extension extends Container ElementType.from(_extension.extendedType, library, packageGraph); } - /// Returns [true] if there is an instantiation of [c] to which this extension - /// could be applied. + /// Returns [true] if there is an instantiation of [c] to which this + /// extension could be applied. bool couldApplyTo(Class c) => (c.element == extendedType.element.element) || packageGraph.typeSystem From 4bbcc48b741686f707474cc6dbd7d14899870441 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Wed, 6 Nov 2019 13:16:28 -0800 Subject: [PATCH 3/8] it works --- lib/src/element_type.dart | 41 +++++++++++----- lib/src/extension_tree.dart | 94 +++++++++++++++++++++++++++---------- lib/src/model.dart | 35 +++++++++----- test/model_test.dart | 19 +++++++- 4 files changed, 140 insertions(+), 49 deletions(-) diff --git a/lib/src/element_type.dart b/lib/src/element_type.dart index 0248db9928..1fd854aff9 100644 --- a/lib/src/element_type.dart +++ b/lib/src/element_type.dart @@ -10,7 +10,8 @@ import 'dart:collection'; 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/src/dart/element/type.dart' show InterfaceTypeImpl; +import 'package:analyzer/src/dart/element/type.dart' + show InterfaceTypeImpl, TypeParameterTypeImpl; import 'package:analyzer/src/generated/type_system.dart' show Dart2TypeSystem; import 'package:dartdoc/src/model.dart'; import 'package:dartdoc/src/model_utils.dart'; @@ -256,6 +257,11 @@ class TypeParameterElementType extends DefinedElementType { } return _nameWithGenerics; } + + @override + ClassElement get _boundClassElement => interfaceType.element; + @override + InterfaceType get interfaceType => (type as TypeParameterTypeImpl).bound; } /// An [ElementType] associated with an [Element]. @@ -314,23 +320,34 @@ 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; + InterfaceTypeImpl _instantiatedType; /// Return this type, instantiated to upper-bounds. DartType get instantiatedType { if (_instantiatedType == null) { - assert(element is Class || element is ModelFunctionTyped); + if (type.name == 'Pointer') { + print('hi'); + } Dart2TypeSystem typeSystem = packageGraph.typeSystem; - _instantiatedType = typeSystem - .instantiateToBounds((element.element as ClassElement).thisType); - _instantiatedType = (element.element as ClassElement).instantiate( - typeArguments: _instantiatedType.typeArguments.map((a) { - if (a.isDynamic) { - return typeSystem.typeProvider.neverType; - } - return a; - }).toList(), - nullabilitySuffix: _instantiatedType.nullabilitySuffix); + if (!interfaceType.typeArguments.every((t) => t is InterfaceType)) { + _instantiatedType = typeSystem.instantiateToBounds(interfaceType); + _instantiatedType = _boundClassElement.instantiate( + typeArguments: _instantiatedType.typeArguments.map((a) { + if (a.isDynamic) { + return typeSystem.typeProvider.neverType; + } + return a; + }).toList(), + nullabilitySuffix: _instantiatedType.nullabilitySuffix); + } else { + _instantiatedType = interfaceType; + } } return _instantiatedType; } diff --git a/lib/src/extension_tree.dart b/lib/src/extension_tree.dart index 84c7cda487..3f5555a790 100644 --- a/lib/src/extension_tree.dart +++ b/lib/src/extension_tree.dart @@ -5,9 +5,6 @@ /// A structure to keep track of extensions and their applicability efficiently. library dartdoc.extension_tree; -import 'dart:collection'; - -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'; @@ -18,7 +15,7 @@ import 'package:dartdoc/src/special_elements.dart'; /// - Each node contains the set of extensions declared as being directly on /// [extendedType]. /// - All parents, recursively, of a node contain extensions that could -/// apply to any instantiated-to-upper-bounds type of [extendedType.element]. +/// apply to any instantiated type of [extendedType.bound]. /// /// A tree of ExtensionNodes will have a root node associated with the [Object] /// type and more specific types will be further down the tree. @@ -50,7 +47,7 @@ class ExtensionNode { /// Do not call addExtension while iterating over the return value to /// this function. Iterable allCouldApplyTo(Class c) sync* { - if (couldApplyTo(c)) { + if (couldApplyTo(c.modelType)) { yield* extensions; yield* children.expand((e) => e.allCouldApplyTo(c)); } @@ -60,33 +57,55 @@ class ExtensionNode { /// already in the tree, will return the node it was found in. ExtensionNode addExtension(Extension newExtension) { assert(identical(newExtension.packageGraph, packageGraph)); - if (extendedType.element is! Class) { - throw UnimplementedError( - 'Extension tree does not yet know about extensions not on classes'); - } - assert(extendedType.element is Class); - assert(extendedType.element is Documentable); ExtensionNode newNode = ExtensionNode(newExtension.extendedType, packageGraph, {newExtension}); return _addExtensionNode(newNode); } ExtensionNode _addExtensionNode(ExtensionNode newNode) { - if (extendedType.type == newNode.extendedType.type) { + if (newNode.extensions.first.name == 'SymDiff') { + print('hello'); + } + if (extendedType.name == 'String' || + newNode.extendedType.name == 'String') { + print('hello'); + } + if (newNode.extensions.first.name == '') { + print('hmmm'); + } + if (extendedType.instantiatedType == + newNode.extendedType.instantiatedType) { // Extended on the exact same type. Add to this set. _merge(newNode); return this; } - assert(!newNode.couldApplyTo(extendedType.element)); + //assert(!newNode.couldApplyTo(extendedType)); Set foundApplicable = {}; for (ExtensionNode child in children) { - if (child.couldApplyTo(extendedType.element)) { - // This child should be a parent of, or the same type as the new node. + 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 { + newNode._addChild(applicable); + } + } + + for (ExtensionNode child in children) { + if (newNode.couldApplyTo(child.extendedType)) { + return child._addExtensionNode(newNode); + } + } + + /* assert(foundApplicable.length <= 1, 'tree structure invalid, multiple possible parents found'); if (foundApplicable.isNotEmpty) { @@ -94,30 +113,53 @@ class ExtensionNode { } // Some children of this node may need to be reparented under this // node. + Set reparentThese = {}; for (ExtensionNode child in children) { - if (newNode.couldApplyTo(child.extendedType.element)) { + if (newNode.extendedType.type == child.extendedType.type) { + // Any reparenting should already have taken place. + assert(reparentThese.isEmpty); + return child._addExtensionNode(newNode); + } + else if (newNode.isSuperType(child.extendedType)) { reparentThese.add(child); } } for (ExtensionNode reparentMe in reparentThese) { + if (reparentMe.extendedType.name == 'String' && newNode.extendedType.name == 'String') { + print('wtf'); + } reparentMe._detach(); newNode._addChild(reparentMe); } + */ _addChild(newNode); return newNode; } + List childList; + /// Add a child, unconditionally. void _addChild(ExtensionNode newChild) { children.add(newChild); + childList = children.toList(); newChild.parent = this; } - bool couldApplyTo(Class c) => - (c.element == extendedType.element.element) || - packageGraph.typeSystem - .isSubtypeOf(c.modelType.instantiatedType, extendedType.type); + bool isSuperType(DefinedElementType t) => packageGraph.typeSystem + .isSubtypeOf(t.instantiatedType, extendedType.instantiatedType); + + bool isSubtype(DefinedElementType t) => packageGraph.typeSystem + .isSubtypeOf(extendedType.instantiatedType, t.instantiatedType); + + bool couldApplyTo(DefinedElementType t) { + if (t.instantiatedType.element.name == 'Set' && + extendedType.instantiatedType.element.name == 'Set') { + print('hello'); + } + return t.instantiatedType == extendedType.instantiatedType || + isSuperType(t); + } /// Remove this subtree from its parent node, unconditionally. void _detach() { @@ -125,11 +167,11 @@ class ExtensionNode { parent == null; } - /// Merge this node into [other], unconditionally. + /// Merge from [other] node into [this], unconditionally. void _merge(ExtensionNode other) { - assert(other.extendedType.type == extendedType.type); - other.extensions.addAll(extensions); - other.children.addAll(children); + //assert(other.extendedType.type == extendedType.type); + extensions.addAll(other.extensions); + children.addAll(other.children); } @override @@ -137,4 +179,8 @@ class ExtensionNode { @override bool operator ==(other) => extendedType.type == other.extendedType.type; + + @override + String toString() => + '{${extensions.map((e) => e.name).join(', ')}} => ${extendedType.toString()} (${extendedType.instantiatedType.toString()})'; } diff --git a/lib/src/model.dart b/lib/src/model.dart index dfe0b54a46..8e9212d0a5 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -64,6 +64,8 @@ import 'package:path/path.dart' as path; import 'package:pub_semver/pub_semver.dart'; import 'package:quiver/iterables.dart' as quiver; +import 'extension_tree.dart'; + int byName(Nameable a, Nameable b) => compareAsciiLowerCaseNatural(a.name, b.name); @@ -800,10 +802,13 @@ class Class extends Container List _potentiallyApplicableExtensions; Iterable get potentiallyApplicableExtensions { if (_potentiallyApplicableExtensions == null) { + if (name == 'Megatron') { + print('hello'); + } _potentiallyApplicableExtensions = utils - .filterNonDocumented(packageGraph.extensions) - .where((e) => e.couldApplyTo(this)) - .toList(growable: false); + .filterNonDocumented(packageGraph.extensions.allCouldApplyTo(this)) + .toList(growable: false) + ..sort(byName); } return _potentiallyApplicableExtensions; } @@ -5045,17 +5050,26 @@ class PackageGraph { package._libraries.sort((a, b) => compareNatural(a.name, b.name)); package._libraries.forEach((library) { library.allClasses.forEach(_addToImplementors); - // TODO(jcollins-g): Use a better data structure. - _extensions.addAll(library.extensions); }); }); _implementors.values.forEach((l) => l.sort()); allImplementorsAdded = true; - _extensions.sort(byName); - allExtensionsAdded = true; // We should have found all special classes by now. specialClasses.assertSpecials(); + + // Build the extension tracking tree. We have to have found Object + // first, so the traversal is separate from special object discovery. + _extensions = ExtensionNode.fromRoot(this); + documentedPackages.toList().forEach((package) { + package._libraries.forEach((library) { + for (Extension e in library.extensions) { + ExtensionNode returned = _extensions.addExtension(e); + assert(returned != null && returned.extensions.contains(e)); + } + }); + }); + allExtensionsAdded = true; } /// Generate a list of futures for any docs that actually require precaching. @@ -5119,7 +5133,7 @@ class PackageGraph { return _implementors; } - Iterable get extensions { + ExtensionNode get extensions { assert(allExtensionsAdded); return _extensions; } @@ -5161,9 +5175,8 @@ class PackageGraph { /// Map of Class.href to a list of classes implementing that class final Map> _implementors = Map(); - /// A list of extensions that exist in the package graph. - // TODO(jcollins-g): Consider implementing a smarter structure for this. - final List _extensions = List(); + /// A tree of extensions that exist in the package graph. + ExtensionNode _extensions; /// PackageMeta for the default package. final PackageMeta packageMeta; diff --git a/test/model_test.dart b/test/model_test.dart index 43a8876964..434041070c 100644 --- a/test/model_test.dart +++ b/test/model_test.dart @@ -2186,6 +2186,21 @@ void main() { expect(documentOnceReexportTwo.isCanonical, isTrue); }); + test('extension tree structure is built correctly', () { + expect( + packageGraph.extensions.children + .any((e) => e.extensions.any((e) => e.name == 'Arm')), + isTrue); + expect( + packageGraph.extensions.children + .any((e) => e.extensions.any((e) => e.name == 'Leg')), + isTrue); + expect( + packageGraph.extensions.children + .any((e) => e.extensions.any((e) => e.name == 'SymDiff')), + isTrue); + }); + test('classes know about applicableExtensions', () { expect(apple.potentiallyApplicableExtensions, orderedEquals([ext])); expect(string.potentiallyApplicableExtensions, @@ -2193,8 +2208,8 @@ void main() { expect(string.potentiallyApplicableExtensions, contains(documentOnceReexportTwo)); expect(baseTest.potentiallyApplicableExtensions, isEmpty); - expect(anotherExtended.potentiallyApplicableExtensions, - orderedEquals([uphill])); + expect( + anotherExtended.potentiallyApplicableExtensions, orderedEquals([])); expect(bigAnotherExtended.potentiallyApplicableExtensions, orderedEquals([uphill])); }); From 3f5f65927c75b960a8583891a8734e886e6c5d96 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Wed, 6 Nov 2019 20:40:17 -0800 Subject: [PATCH 4/8] It actually works, with tests that are debugged --- lib/src/element_type.dart | 4 +- lib/src/extension_tree.dart | 103 ++++++++++++++++++++++++++++++------ test/model_test.dart | 12 +++-- 3 files changed, 98 insertions(+), 21 deletions(-) diff --git a/lib/src/element_type.dart b/lib/src/element_type.dart index 1fd854aff9..f3764158d2 100644 --- a/lib/src/element_type.dart +++ b/lib/src/element_type.dart @@ -337,14 +337,14 @@ abstract class DefinedElementType extends ElementType { Dart2TypeSystem typeSystem = packageGraph.typeSystem; if (!interfaceType.typeArguments.every((t) => t is InterfaceType)) { _instantiatedType = typeSystem.instantiateToBounds(interfaceType); - _instantiatedType = _boundClassElement.instantiate( + /*_instantiatedType = _boundClassElement.instantiate( typeArguments: _instantiatedType.typeArguments.map((a) { if (a.isDynamic) { return typeSystem.typeProvider.neverType; } return a; }).toList(), - nullabilitySuffix: _instantiatedType.nullabilitySuffix); + nullabilitySuffix: _instantiatedType.nullabilitySuffix);*/ } else { _instantiatedType = interfaceType; } diff --git a/lib/src/extension_tree.dart b/lib/src/extension_tree.dart index 3f5555a790..d1f5396990 100644 --- a/lib/src/extension_tree.dart +++ b/lib/src/extension_tree.dart @@ -5,6 +5,8 @@ /// 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'; @@ -27,6 +29,15 @@ class ExtensionNode { /// more specific types than [extendedType]. final Set children = {}; + /// The set of seen [ModelElement]s backing [extendedType]s in this tree. + /// Only valid at the root. + final Set _seen = {}; + + /// 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]. + final Set _sparesInserted = {}; + /// The type all [extensions] are extending. final DefinedElementType extendedType; @@ -53,25 +64,52 @@ class ExtensionNode { } } + /// Insert a node for the supertype if there are a lot of type-specific + /// extensions for the same class. Makes the heap more efficient for + void _maybeInsertGenericNode(DefinedElementType newExtendedType) { + if (!_seen.contains(newExtendedType.element)) { + _seen.add(newExtendedType.element); + return; + } + if (!_sparesInserted.contains(newExtendedType.element)) { + if (newExtendedType.element.name == 'Megatron') { + print('hi'); + } + ElementType spareType = newExtendedType.element.modelType; + if (spareType is DefinedElementType) { + if (spareType.name == 'Pointer') { + print('hi'); + } + if (spareType.typeArguments.isNotEmpty && + spareType.type != extendedType.type) { + ElementType spareElementType = ElementType.from( + spareType.instantiatedType, + newExtendedType.element.library, + packageGraph); + _addExtensionNode(ExtensionNode(spareElementType, packageGraph, {})); + } + } + _sparesInserted.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) { + if (newExtension.name == 'StructPointer' || + newExtension.name == 'DoublePointer') { + print('hi'); + } 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 (newNode.extensions.first.name == 'SymDiff') { - print('hello'); - } - if (extendedType.name == 'String' || - newNode.extendedType.name == 'String') { - print('hello'); - } - if (newNode.extensions.first.name == '') { - print('hmmm'); + if (newNode.extensions.isEmpty) { + print('hi'); } if (extendedType.instantiatedType == newNode.extendedType.instantiatedType) { @@ -95,7 +133,15 @@ class ExtensionNode { newNode.extendedType.instantiatedType) { newNode._merge(applicable); } else { - newNode._addChild(applicable); + if (applicable.isSubtypeOf(newNode.extendedType)) { + newNode._addChild(applicable); + } else if (newNode.isSubtypeOf(applicable.extendedType) || + newNode.isBoundSuperclassTo(applicable.extendedType)) { + _addChild(applicable); + return applicable._addExtensionNode(newNode); + } else { + print('do nothin'); + } } } @@ -137,28 +183,55 @@ class ExtensionNode { return newNode; } - List childList; + List get childList => children.toList(); /// Add a child, unconditionally. void _addChild(ExtensionNode newChild) { children.add(newChild); - childList = children.toList(); newChild.parent = this; } - bool isSuperType(DefinedElementType t) => packageGraph.typeSystem + /// The instantiated to upper bounds [extendedType] of this node is a supertype of [t]. + bool isSupertypeOf(DefinedElementType t) => packageGraph.typeSystem .isSubtypeOf(t.instantiatedType, extendedType.instantiatedType); - bool isSubtype(DefinedElementType t) => packageGraph.typeSystem + /// The instantiated to upper 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] is a superclass of the class from [t] + /// and any type parameters from [t] result in an instantiated subtype on that class. + 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; + } + bool couldApplyTo(DefinedElementType t) { if (t.instantiatedType.element.name == 'Set' && extendedType.instantiatedType.element.name == 'Set') { print('hello'); } + if (toString().contains('Pointer') && + t.nameWithGenerics.contains('Pointer')) { + print('hello'); + } return t.instantiatedType == extendedType.instantiatedType || - isSuperType(t); + (t.instantiatedType.element == extendedType.instantiatedType.element && + isSubtypeOf(t)) || + isBoundSuperclassTo(t); } /// Remove this subtree from its parent node, unconditionally. diff --git a/test/model_test.dart b/test/model_test.dart index 434041070c..8ff0b0dc0a 100644 --- a/test/model_test.dart +++ b/test/model_test.dart @@ -7,6 +7,7 @@ library dartdoc.model_test; import 'dart:io'; import 'package:dartdoc/dartdoc.dart'; +import 'package:dartdoc/src/extension_tree.dart'; import 'package:dartdoc/src/model.dart'; import 'package:dartdoc/src/model_utils.dart'; import 'package:dartdoc/src/special_elements.dart'; @@ -2187,12 +2188,15 @@ void main() { }); test('extension tree structure is built correctly', () { + ExtensionNode megaTron = packageGraph.extensions.children + .firstWhere((n) => n.extendedType.name.contains('Megatron')); + expect(megaTron.extensions, isEmpty); expect( - packageGraph.extensions.children + megaTron.children .any((e) => e.extensions.any((e) => e.name == 'Arm')), isTrue); expect( - packageGraph.extensions.children + megaTron.children .any((e) => e.extensions.any((e) => e.name == 'Leg')), isTrue); expect( @@ -2208,8 +2212,8 @@ void main() { expect(string.potentiallyApplicableExtensions, contains(documentOnceReexportTwo)); expect(baseTest.potentiallyApplicableExtensions, isEmpty); - expect( - anotherExtended.potentiallyApplicableExtensions, orderedEquals([])); + expect(anotherExtended.potentiallyApplicableExtensions, + orderedEquals([uphill])); expect(bigAnotherExtended.potentiallyApplicableExtensions, orderedEquals([uphill])); }); From 3cb6071e25aeecb449de92a9f2ed28a21651f21b Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Wed, 6 Nov 2019 21:50:17 -0800 Subject: [PATCH 5/8] Cleanups --- lib/src/element_type.dart | 23 ++----- lib/src/extension_tree.dart | 120 ++++++++++++------------------------ lib/src/model.dart | 14 +---- test/model_test.dart | 3 +- 4 files changed, 49 insertions(+), 111 deletions(-) diff --git a/lib/src/element_type.dart b/lib/src/element_type.dart index f3764158d2..19a230e936 100644 --- a/lib/src/element_type.dart +++ b/lib/src/element_type.dart @@ -10,9 +10,6 @@ import 'dart:collection'; 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/src/dart/element/type.dart' - show InterfaceTypeImpl, TypeParameterTypeImpl; -import 'package:analyzer/src/generated/type_system.dart' show Dart2TypeSystem; import 'package:dartdoc/src/model.dart'; import 'package:dartdoc/src/model_utils.dart'; @@ -261,7 +258,7 @@ class TypeParameterElementType extends DefinedElementType { @override ClassElement get _boundClassElement => interfaceType.element; @override - InterfaceType get interfaceType => (type as TypeParameterTypeImpl).bound; + InterfaceType get interfaceType => (type as TypeParameterType).bound; } /// An [ElementType] associated with an [Element]. @@ -326,25 +323,13 @@ abstract class DefinedElementType extends ElementType { ModelElement.fromElement(_boundClassElement, packageGraph); InterfaceType get interfaceType => type; - InterfaceTypeImpl _instantiatedType; + InterfaceType _instantiatedType; - /// Return this type, instantiated to upper-bounds. + /// Return this type, instantiated to bounds if it isn't already. DartType get instantiatedType { if (_instantiatedType == null) { - if (type.name == 'Pointer') { - print('hi'); - } - Dart2TypeSystem typeSystem = packageGraph.typeSystem; if (!interfaceType.typeArguments.every((t) => t is InterfaceType)) { - _instantiatedType = typeSystem.instantiateToBounds(interfaceType); - /*_instantiatedType = _boundClassElement.instantiate( - typeArguments: _instantiatedType.typeArguments.map((a) { - if (a.isDynamic) { - return typeSystem.typeProvider.neverType; - } - return a; - }).toList(), - nullabilitySuffix: _instantiatedType.nullabilitySuffix);*/ + _instantiatedType = packageGraph.typeSystem.instantiateToBounds(interfaceType); } else { _instantiatedType = interfaceType; } diff --git a/lib/src/extension_tree.dart b/lib/src/extension_tree.dart index d1f5396990..b1d616b008 100644 --- a/lib/src/extension_tree.dart +++ b/lib/src/extension_tree.dart @@ -11,16 +11,19 @@ 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. An extension tree's -/// nodes are arranged given the following: +/// 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]. -/// - All parents, recursively, of a node contain extensions that could -/// apply to any instantiated type of [extendedType.bound]. +/// [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]. /// -/// A tree of ExtensionNodes will have a root node associated with the [Object] -/// type and more specific types will be further down the tree. +/// 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 { /// The set of extensions that directly apply to [extendedType]. final Set extensions; @@ -31,12 +34,12 @@ class ExtensionNode { /// The set of seen [ModelElement]s backing [extendedType]s in this tree. /// Only valid at the root. - final Set _seen = {}; + Set _seen; /// 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]. - final Set _sparesInserted = {}; + Set _genericsInserted; /// The type all [extensions] are extending. final DefinedElementType extendedType; @@ -64,42 +67,34 @@ class ExtensionNode { } } - /// Insert a node for the supertype if there are a lot of type-specific - /// extensions for the same class. Makes the heap more efficient for + /// 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)) { + if (!(_seen ??= {}).contains(newExtendedType.element)) { _seen.add(newExtendedType.element); return; } - if (!_sparesInserted.contains(newExtendedType.element)) { - if (newExtendedType.element.name == 'Megatron') { - print('hi'); - } - ElementType spareType = newExtendedType.element.modelType; - if (spareType is DefinedElementType) { - if (spareType.name == 'Pointer') { - print('hi'); - } - if (spareType.typeArguments.isNotEmpty && - spareType.type != extendedType.type) { - ElementType spareElementType = ElementType.from( - spareType.instantiatedType, + 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(spareElementType, packageGraph, {})); + _addExtensionNode(ExtensionNode(genericElementType, packageGraph, {})); } } - _sparesInserted.add(newExtendedType.element); + _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) { - if (newExtension.name == 'StructPointer' || - newExtension.name == 'DoublePointer') { - print('hi'); - } assert(identical(newExtension.packageGraph, packageGraph)); ExtensionNode newNode = ExtensionNode(newExtension.extendedType, packageGraph, {newExtension}); @@ -108,16 +103,12 @@ class ExtensionNode { } ExtensionNode _addExtensionNode(ExtensionNode newNode) { - if (newNode.extensions.isEmpty) { - print('hi'); - } if (extendedType.instantiatedType == newNode.extendedType.instantiatedType) { // Extended on the exact same type. Add to this set. _merge(newNode); return this; } - //assert(!newNode.couldApplyTo(extendedType)); Set foundApplicable = {}; for (ExtensionNode child in children) { @@ -139,8 +130,6 @@ class ExtensionNode { newNode.isBoundSuperclassTo(applicable.extendedType)) { _addChild(applicable); return applicable._addExtensionNode(newNode); - } else { - print('do nothin'); } } } @@ -151,56 +140,31 @@ class ExtensionNode { } } - /* - assert(foundApplicable.length <= 1, - 'tree structure invalid, multiple possible parents found'); - if (foundApplicable.isNotEmpty) { - return foundApplicable.first._addExtensionNode(newNode); - } - // Some children of this node may need to be reparented under this - // node. - - Set reparentThese = {}; - for (ExtensionNode child in children) { - if (newNode.extendedType.type == child.extendedType.type) { - // Any reparenting should already have taken place. - assert(reparentThese.isEmpty); - return child._addExtensionNode(newNode); - } - else if (newNode.isSuperType(child.extendedType)) { - reparentThese.add(child); - } - } - for (ExtensionNode reparentMe in reparentThese) { - if (reparentMe.extendedType.name == 'String' && newNode.extendedType.name == 'String') { - print('wtf'); - } - reparentMe._detach(); - newNode._addChild(reparentMe); - } - */ _addChild(newNode); return newNode; } - List get childList => children.toList(); - /// Add a child, unconditionally. void _addChild(ExtensionNode newChild) { children.add(newChild); newChild.parent = this; } - /// The instantiated to upper bounds [extendedType] of this node is a supertype of [t]. + /// 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 upper bounds [extendedType] of this node is a subtype of [t]. + /// 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] is a superclass of the class from [t] - /// and any type parameters from [t] result in an instantiated subtype on that class. + /// 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 @@ -219,15 +183,11 @@ class ExtensionNode { 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) { - if (t.instantiatedType.element.name == 'Set' && - extendedType.instantiatedType.element.name == 'Set') { - print('hello'); - } - if (toString().contains('Pointer') && - t.nameWithGenerics.contains('Pointer')) { - print('hello'); - } return t.instantiatedType == extendedType.instantiatedType || (t.instantiatedType.element == extendedType.instantiatedType.element && isSubtypeOf(t)) || @@ -254,6 +214,8 @@ class ExtensionNode { 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()})'; } diff --git a/lib/src/model.dart b/lib/src/model.dart index 8e9212d0a5..ba27bc3a77 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -22,7 +22,6 @@ import 'package:analyzer/dart/ast/ast.dart' InstanceCreationExpression; import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/type.dart'; -import 'package:analyzer/dart/element/type_system.dart'; import 'package:analyzer/dart/element/visitor.dart'; import 'package:analyzer/file_system/file_system.dart' as file_system; import 'package:analyzer/file_system/physical_file_system.dart'; @@ -42,6 +41,7 @@ import 'package:analyzer/src/generated/java_io.dart'; import 'package:analyzer/src/generated/sdk.dart'; import 'package:analyzer/src/generated/source.dart'; import 'package:analyzer/src/generated/source_io.dart'; +import 'package:analyzer/src/generated/type_system.dart' show Dart2TypeSystem; import 'package:analyzer/src/source/package_map_resolver.dart'; import 'package:analyzer/src/source/sdk_ext.dart'; import 'package:args/args.dart'; @@ -802,9 +802,6 @@ class Class extends Container List _potentiallyApplicableExtensions; Iterable get potentiallyApplicableExtensions { if (_potentiallyApplicableExtensions == null) { - if (name == 'Megatron') { - print('hello'); - } _potentiallyApplicableExtensions = utils .filterNonDocumented(packageGraph.extensions.allCouldApplyTo(this)) .toList(growable: false) @@ -1347,13 +1344,6 @@ class Extension extends Container ElementType.from(_extension.extendedType, library, packageGraph); } - /// Returns [true] if there is an instantiation of [c] to which this - /// extension could be applied. - bool couldApplyTo(Class c) => - (c.element == extendedType.element.element) || - packageGraph.typeSystem - .isSubtypeOf(c.modelType.instantiatedType, extendedType.type); - @override ModelElement get enclosingElement => library; @@ -5208,7 +5198,7 @@ class PackageGraph { /// TODO(brianwilkerson) Replace the driver with the session. final AnalysisDriver driver; final AnalysisSession session; - final TypeSystem typeSystem; + final Dart2TypeSystem typeSystem; final DartSdk sdk; Map _sdkLibrarySources; diff --git a/test/model_test.dart b/test/model_test.dart index 8ff0b0dc0a..eefd97a4e7 100644 --- a/test/model_test.dart +++ b/test/model_test.dart @@ -2201,7 +2201,8 @@ void main() { isTrue); expect( packageGraph.extensions.children - .any((e) => e.extensions.any((e) => e.name == 'SymDiff')), + .firstWhere((n) => n.extendedType.name == 'Set') + .extensions.any((e) => e.name == 'SymDiff'), isTrue); }); From b77f60221922f67ec2809a8d01b2465f597b765e Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 7 Nov 2019 09:16:23 -0800 Subject: [PATCH 6/8] Add test case for implements --- test/model_test.dart | 20 ++++++++++++++++++++ testing/test_package/lib/fake.dart | 28 ++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/test/model_test.dart b/test/model_test.dart index eefd97a4e7..8f265b8a43 100644 --- a/test/model_test.dart +++ b/test/model_test.dart @@ -2219,6 +2219,26 @@ void main() { orderedEquals([uphill])); }); + test('applicableExtensions include those from implements', () { + Extension extensionCheckLeft, extensionCheckRight, extensionCheckCenter, + extensionCheckImplementor2; + Class implementor, implementor2; + Extension getExtension(String name) => fakeLibrary.extensions.firstWhere((e) => e.name == name); + Class getClass(String name) => fakeLibrary.classes.firstWhere((e) => e.name == name); + extensionCheckLeft = getExtension('ExtensionCheckLeft'); + extensionCheckRight = getExtension('ExtensionCheckRight'); + extensionCheckCenter = getExtension('ExtensionCheckCenter'); + extensionCheckImplementor2 = getExtension('ExtensionCheckImplementor2'); + implementor = getClass('Implementor'); + implementor2 = getClass('Implementor2'); + + expect(implementor2.potentiallyApplicableExtensions, orderedEquals( + [extensionCheckImplementor2, extensionCheckLeft])); + expect(implementor.potentiallyApplicableExtensions, orderedEquals( + [extensionCheckCenter, extensionCheckImplementor2, extensionCheckLeft, + extensionCheckRight])); + }); + test('type parameters and bounds work with applicableExtensions', () { expect( superMegaTron.potentiallyApplicableExtensions, orderedEquals([leg])); diff --git a/testing/test_package/lib/fake.dart b/testing/test_package/lib/fake.dart index 332b86b225..2fb05f536a 100644 --- a/testing/test_package/lib/fake.dart +++ b/testing/test_package/lib/fake.dart @@ -1018,6 +1018,34 @@ class AnotherExtended extends BaseTest {} class BigAnotherExtended extends AnotherExtended {} + +extension ExtensionCheckLeft on Implemented1 { + int get left => 3; +} + +extension ExtensionCheckRight on Implemented2 { + int get right => 4; +} + +extension ExtensionCheckCenter on BaseExtended { + bool get center => true; +} + +class BaseExtended {} + +class Implemented1 {} + +class Implemented2 {} + +class Implementor extends BaseExtended implements Implemented1, Implemented2, Implementor2 {} + +extension ExtensionCheckImplementor2 on Implementor2 { + int get test => 1; +} + +class Implementor2 implements Implemented1 {} + + // // // From d8139567c548aca13163306458a8527d2ae9d9ea Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 7 Nov 2019 12:51:21 -0800 Subject: [PATCH 7/8] Remove extension tree and correct couldApplyTo once more. --- lib/src/extension_tree.dart | 221 ----------------------------- lib/src/model.dart | 71 ++++++--- test/model_test.dart | 59 ++++---- testing/test_package/lib/fake.dart | 19 ++- 4 files changed, 96 insertions(+), 274 deletions(-) delete mode 100644 lib/src/extension_tree.dart diff --git a/lib/src/extension_tree.dart b/lib/src/extension_tree.dart deleted file mode 100644 index b1d616b008..0000000000 --- a/lib/src/extension_tree.dart +++ /dev/null @@ -1,221 +0,0 @@ -// 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 { - /// The set of extensions that directly apply to [extendedType]. - final Set extensions; - - /// The set of ExtensionNodes containing [extensions] that apply only to - /// more specific types than [extendedType]. - final Set children = {}; - - /// The set of seen [ModelElement]s backing [extendedType]s in this tree. - /// Only valid at the root. - Set _seen; - - /// 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 _genericsInserted; - - /// The type all [extensions] are extending. - final DefinedElementType extendedType; - - /// The [PackageGraph] associated with the tree; must be the same for all - /// nodes. - final PackageGraph packageGraph; - - 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 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 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); - 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()})'; -} diff --git a/lib/src/model.dart b/lib/src/model.dart index ba27bc3a77..29e597301a 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -6,7 +6,7 @@ library dartdoc.models; import 'dart:async'; -import 'dart:collection' show UnmodifiableListView; +import 'dart:collection' show HashSet, UnmodifiableListView; import 'dart:convert'; import 'dart:io'; @@ -64,8 +64,6 @@ import 'package:path/path.dart' as path; import 'package:pub_semver/pub_semver.dart'; import 'package:quiver/iterables.dart' as quiver; -import 'extension_tree.dart'; - int byName(Nameable a, Nameable b) => compareAsciiLowerCaseNatural(a.name, b.name); @@ -803,7 +801,8 @@ class Class extends Container Iterable get potentiallyApplicableExtensions { if (_potentiallyApplicableExtensions == null) { _potentiallyApplicableExtensions = utils - .filterNonDocumented(packageGraph.extensions.allCouldApplyTo(this)) + .filterNonDocumented(packageGraph.extensions) + .where((e) => e.couldApplyTo(this)) .toList(growable: false) ..sort(byName); } @@ -1344,6 +1343,49 @@ class Extension extends Container ElementType.from(_extension.extendedType, library, packageGraph); } + bool couldApplyTo(Class c) => _couldApplyTo(c.modelType); + + /// Return true if this extension could apply to [t]. + bool _couldApplyTo(DefinedElementType t) { + if (t.element.name == 'Implementor2' && name == 'ExtensionCheckLeft') { + print('hi'); + } + return t.instantiatedType == extendedType.instantiatedType || + (t.instantiatedType.element == extendedType.instantiatedType.element && + isSubtypeOf(t)) || + isBoundSupertypeTo(t); + } + + /// The instantiated to bounds [extendedType] of this extension is a subtype of + /// [t]. + bool isSubtypeOf(DefinedElementType t) => packageGraph.typeSystem + .isSubtypeOf(extendedType.instantiatedType, t.instantiatedType); + + bool isBoundSupertypeTo(DefinedElementType t) => + _isBoundSupertypeTo(t.type, HashSet()); + + /// Returns true if at least one supertype (including via mixins and + /// interfaces) is equivalent to or a subtype of [extendedType] when + /// instantiated to bounds. + bool _isBoundSupertypeTo( + InterfaceType superType, HashSet visited) { + ClassElement superClass = superType?.element; + if (visited.contains(superType)) return false; + visited.add(superType); + if (superClass == extendedType.type.element && + (superType == extendedType.instantiatedType || + packageGraph.typeSystem + .isSubtypeOf(superType, extendedType.instantiatedType))) { + return true; + } + List supertypes = []; + ClassElementImpl.collectAllSupertypes(supertypes, superType, null); + for (InterfaceType toVisit in supertypes) { + if (_isBoundSupertypeTo(toVisit, visited)) return true; + } + return false; + } + @override ModelElement get enclosingElement => library; @@ -5040,26 +5082,15 @@ class PackageGraph { package._libraries.sort((a, b) => compareNatural(a.name, b.name)); package._libraries.forEach((library) { library.allClasses.forEach(_addToImplementors); + _extensions.addAll(library.extensions); }); }); _implementors.values.forEach((l) => l.sort()); allImplementorsAdded = true; + allExtensionsAdded = true; // We should have found all special classes by now. specialClasses.assertSpecials(); - - // Build the extension tracking tree. We have to have found Object - // first, so the traversal is separate from special object discovery. - _extensions = ExtensionNode.fromRoot(this); - documentedPackages.toList().forEach((package) { - package._libraries.forEach((library) { - for (Extension e in library.extensions) { - ExtensionNode returned = _extensions.addExtension(e); - assert(returned != null && returned.extensions.contains(e)); - } - }); - }); - allExtensionsAdded = true; } /// Generate a list of futures for any docs that actually require precaching. @@ -5123,7 +5154,7 @@ class PackageGraph { return _implementors; } - ExtensionNode get extensions { + Iterable get extensions { assert(allExtensionsAdded); return _extensions; } @@ -5165,8 +5196,8 @@ class PackageGraph { /// Map of Class.href to a list of classes implementing that class final Map> _implementors = Map(); - /// A tree of extensions that exist in the package graph. - ExtensionNode _extensions; + /// A list of extensions that exist in the package graph. + final List _extensions = []; /// PackageMeta for the default package. final PackageMeta packageMeta; diff --git a/test/model_test.dart b/test/model_test.dart index 8f265b8a43..ba49ca7c7d 100644 --- a/test/model_test.dart +++ b/test/model_test.dart @@ -7,7 +7,6 @@ library dartdoc.model_test; import 'dart:io'; import 'package:dartdoc/dartdoc.dart'; -import 'package:dartdoc/src/extension_tree.dart'; import 'package:dartdoc/src/model.dart'; import 'package:dartdoc/src/model_utils.dart'; import 'package:dartdoc/src/special_elements.dart'; @@ -2187,25 +2186,6 @@ void main() { expect(documentOnceReexportTwo.isCanonical, isTrue); }); - test('extension tree structure is built correctly', () { - ExtensionNode megaTron = packageGraph.extensions.children - .firstWhere((n) => n.extendedType.name.contains('Megatron')); - expect(megaTron.extensions, isEmpty); - expect( - megaTron.children - .any((e) => e.extensions.any((e) => e.name == 'Arm')), - isTrue); - expect( - megaTron.children - .any((e) => e.extensions.any((e) => e.name == 'Leg')), - isTrue); - expect( - packageGraph.extensions.children - .firstWhere((n) => n.extendedType.name == 'Set') - .extensions.any((e) => e.name == 'SymDiff'), - isTrue); - }); - test('classes know about applicableExtensions', () { expect(apple.potentiallyApplicableExtensions, orderedEquals([ext])); expect(string.potentiallyApplicableExtensions, @@ -2219,24 +2199,41 @@ void main() { orderedEquals([uphill])); }); - test('applicableExtensions include those from implements', () { - Extension extensionCheckLeft, extensionCheckRight, extensionCheckCenter, - extensionCheckImplementor2; - Class implementor, implementor2; - Extension getExtension(String name) => fakeLibrary.extensions.firstWhere((e) => e.name == name); - Class getClass(String name) => fakeLibrary.classes.firstWhere((e) => e.name == name); + test('applicableExtensions include those from implements & mixins', () { + Extension extensionCheckLeft, + extensionCheckRight, + extensionCheckCenter, + extensionCheckImplementor2, + onNewSchool, + onOldSchool; + Class implementor, implementor2, school; + Extension getExtension(String name) => + fakeLibrary.extensions.firstWhere((e) => e.name == name); + Class getClass(String name) => + fakeLibrary.classes.firstWhere((e) => e.name == name); extensionCheckLeft = getExtension('ExtensionCheckLeft'); extensionCheckRight = getExtension('ExtensionCheckRight'); extensionCheckCenter = getExtension('ExtensionCheckCenter'); extensionCheckImplementor2 = getExtension('ExtensionCheckImplementor2'); + onNewSchool = getExtension('OnNewSchool'); + onOldSchool = getExtension('OnOldSchool'); + implementor = getClass('Implementor'); implementor2 = getClass('Implementor2'); + school = getClass('School'); - expect(implementor2.potentiallyApplicableExtensions, orderedEquals( - [extensionCheckImplementor2, extensionCheckLeft])); - expect(implementor.potentiallyApplicableExtensions, orderedEquals( - [extensionCheckCenter, extensionCheckImplementor2, extensionCheckLeft, - extensionCheckRight])); + expect( + implementor.potentiallyApplicableExtensions, + orderedEquals([ + extensionCheckCenter, + extensionCheckImplementor2, + extensionCheckLeft, + extensionCheckRight + ])); + expect(implementor2.potentiallyApplicableExtensions, + orderedEquals([extensionCheckImplementor2, extensionCheckLeft])); + expect(school.potentiallyApplicableExtensions, + orderedEquals([onNewSchool, onOldSchool])); }); test('type parameters and bounds work with applicableExtensions', () { diff --git a/testing/test_package/lib/fake.dart b/testing/test_package/lib/fake.dart index 2fb05f536a..a195bfb0c1 100644 --- a/testing/test_package/lib/fake.dart +++ b/testing/test_package/lib/fake.dart @@ -1018,7 +1018,6 @@ class AnotherExtended extends BaseTest {} class BigAnotherExtended extends AnotherExtended {} - extension ExtensionCheckLeft on Implemented1 { int get left => 3; } @@ -1037,7 +1036,8 @@ class Implemented1 {} class Implemented2 {} -class Implementor extends BaseExtended implements Implemented1, Implemented2, Implementor2 {} +class Implementor extends BaseExtended + implements Implemented1, Implemented2, Implementor2 {} extension ExtensionCheckImplementor2 on Implementor2 { int get test => 1; @@ -1045,6 +1045,21 @@ extension ExtensionCheckImplementor2 on Implementor2 { class Implementor2 implements Implemented1 {} +class OldSchoolMixin { + aThing() {} +} + +mixin NewSchoolMixin {} + +extension OnNewSchool on NewSchoolMixin { + String get bar => 'hello'; +} + +extension OnOldSchool on OldSchoolMixin { + int get foo => 4; +} + +class School with OldSchoolMixin, NewSchoolMixin {} // // From b332526c2d73fbd27f8c25754a7eefb85a7d05f3 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 7 Nov 2019 14:27:14 -0800 Subject: [PATCH 8/8] Remove debugging code --- lib/src/model.dart | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index 29e597301a..eebbc6e115 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -1347,9 +1347,6 @@ class Extension extends Container /// Return true if this extension could apply to [t]. bool _couldApplyTo(DefinedElementType t) { - if (t.element.name == 'Implementor2' && name == 'ExtensionCheckLeft') { - print('hi'); - } return t.instantiatedType == extendedType.instantiatedType || (t.instantiatedType.element == extendedType.instantiatedType.element && isSubtypeOf(t)) ||