From 54256963543c2feb3b7d1609a9fae81b308cf7c5 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Fri, 25 Jun 2021 16:54:41 -0700 Subject: [PATCH 01/17] It 'works' but no tests for new functionality yet --- lib/src/element_type.dart | 29 +++++++++++---- lib/src/model/class.dart | 10 ++---- lib/src/model/comment_referable.dart | 49 +++++++++++++++++++++++++- lib/src/model/constructor.dart | 17 +++++---- lib/src/model/container.dart | 27 +++++--------- lib/src/model/getter_setter_combo.dart | 18 +++++----- lib/src/model/library.dart | 11 ++++-- lib/src/model/method.dart | 18 +++++++--- lib/src/model/model_function.dart | 5 ++- lib/src/model/operator.dart | 3 ++ lib/src/model/package.dart | 14 +++----- lib/src/model/package_graph.dart | 42 ++++++++-------------- lib/src/model/parameter.dart | 8 +++-- lib/src/model/type_parameter.dart | 5 ++- lib/src/model/typedef.dart | 4 +-- 15 files changed, 159 insertions(+), 101 deletions(-) diff --git a/lib/src/element_type.dart b/lib/src/element_type.dart index 8d8249fb1b..43604f7343 100644 --- a/lib/src/element_type.dart +++ b/lib/src/element_type.dart @@ -202,9 +202,20 @@ class ParameterizedElementType extends DefinedElementType with Rendered { PackageGraph packageGraph, ModelElement element, ElementType returnedFrom) : super(type, library, packageGraph, element, returnedFrom); + @override + ParameterizedType get type => super.type; + @override ElementTypeRenderer get _renderer => packageGraph.rendererFactory.parameterizedElementTypeRenderer; + + Iterable _typeArguments; + @override + Iterable get typeArguments => + _typeArguments ??= type + .typeArguments + .map((f) => ElementType.from(f, library, packageGraph)) + .toList(growable: false); } /// A [ElementType] whose underlying type was referrred to by a type alias. @@ -300,13 +311,6 @@ abstract class DefinedElementType extends ElementType { return canonicalClass?.isPublic ?? false; } - Iterable _typeArguments; - @override - Iterable get typeArguments => - _typeArguments ??= (type as ParameterizedType) - .typeArguments - .map((f) => ElementType.from(f, library, packageGraph)) - .toList(growable: false); DartType get _bound => type; @@ -354,6 +358,9 @@ abstract class DefinedElementType extends ElementType { return false; } + @override + Iterable get typeArguments => []; + @override Map get referenceChildren => modelElement.referenceChildren; @@ -422,6 +429,14 @@ class CallableElementType extends DefinedElementType with Rendered, Callable { @override ElementTypeRenderer get _renderer => packageGraph.rendererFactory.callableElementTypeRenderer; + + + Iterable _typeArguments; + @override + Iterable get typeArguments => + _typeArguments ??= (type.aliasArguments ?? []) + .map((f) => ElementType.from(f, library, packageGraph)) + .toList(growable: false); } /// A non-callable type backed by a [GenericTypeAliasElement]. diff --git a/lib/src/model/class.dart b/lib/src/model/class.dart index b8b466fe8f..0205e23ec6 100644 --- a/lib/src/model/class.dart +++ b/lib/src/model/class.dart @@ -611,13 +611,9 @@ class Class extends Container static Iterable> _constructorGenerator( Iterable source) sync* { for (var constructor in source) { - var constructorName = constructor.element.name; - if (constructorName == '') { - constructorName = constructor.enclosingElement.name; - } - yield MapEntry(constructorName, constructor); + yield MapEntry(constructor.referenceName, constructor); yield MapEntry( - '${constructor.enclosingElement.name}.$constructorName', constructor); + '${constructor.enclosingElement.name}.${constructor.referenceName}', constructor); } } @@ -633,7 +629,7 @@ class Class extends Container ...container.staticFields, ...container.staticMethods, ]) { - yield MapEntry(modelElement.name, modelElement); + yield MapEntry(modelElement.referenceName, modelElement); } if (container is Class) { yield* _constructorGenerator(container.constructors); diff --git a/lib/src/model/comment_referable.dart b/lib/src/model/comment_referable.dart index ee94863ad5..580975c7aa 100644 --- a/lib/src/model/comment_referable.dart +++ b/lib/src/model/comment_referable.dart @@ -33,6 +33,47 @@ extension on Scope { } } +/// A set of utility methods for helping build +/// [CommentReferable.referenceChildren] out of collections of other +/// [CommmentReferable]s. +extension CommentReferableEntryGenerators on Iterable { + /// Discards all entries that do not collide with [referable], + /// and generates an [MapEntry] using [referable]'s + /// [CommentReferable.referenceName] as a prefix for the item that does. + Iterable> onlyExplicitOnCollisionWith(CommentReferable referable) => where((r) => r.referenceName == referable.referenceName) + .map((r) => MapEntry('${referable.referenceName}.${r.referenceName}', r)); + + /// Like [onlyExplicitOnCollisionWith], except does not discard non-conflicting + /// items. + Iterable> explicitOnCollisionWith(CommentReferable referable) => map((r) { + if (r.referenceName == referable.referenceName) { + return MapEntry('${referable.referenceName}.${r.referenceName}', r); + } + return MapEntry(r.referenceName, r); + }); + + /// Just generate entries from this iterable. + Iterable> generateEntries() => map((r) => MapEntry(r.referenceName, r)); + + /// Return all values not of this type. + Iterable whereNotType() sync* { + for (var r in this) { + if (r is! T) yield r; + } + } +} + +/// A set of utility methods to add entries to +/// [CommentReferable.referenceChildren]. +extension CommentReferableEntryBuilder on Map { + /// Like [Map.putIfAbsent] except works on an iterable of entries. + void addEntriesIfAbsent(Iterable> entries) => + entries.forEach((e) { + if (!containsKey(e.key)) this[e.key] = e.value; + }); +} + + /// Support comment reference lookups on a Nameable object. mixin CommentReferable implements Nameable { PackageGraph packageGraph; @@ -147,7 +188,8 @@ mixin CommentReferable implements Nameable { return retval; } - /// Map of name to the elements that are a member of [this], but + + /// Map of [referenceName] to the elements that are a member of [this], but /// not this model element itself. Can be cached. /// /// There is no need to duplicate references here that can be found via @@ -168,6 +210,11 @@ mixin CommentReferable implements Nameable { /// replacing them with this. Iterable get referenceGrandparentOverrides => null; + // TODO(jcollins-g): Enforce that reference name is always the same + // as [ModelElement.name]. Easier/possible after old lookup code is gone. + String get referenceName => name; + + // TODO(jcollins-g): Eliminate need for this in markdown_processor. Library get library => null; diff --git a/lib/src/model/constructor.dart b/lib/src/model/constructor.dart index 282191f56a..b183616a69 100644 --- a/lib/src/model/constructor.dart +++ b/lib/src/model/constructor.dart @@ -130,19 +130,18 @@ class Constructor extends ModelElement Map get referenceChildren { if (_referenceChildren == null) { _referenceChildren = {}; - for (var param in allParameters) { + _referenceChildren.addEntries(allParameters.map((param) { var paramElement = param.element; if (paramElement is FieldFormalParameterElement) { - var fieldFormal = - ModelElement.fromElement(paramElement.field, packageGraph); - _referenceChildren[paramElement.name] = fieldFormal; - } else { - _referenceChildren[param.name] = param; + return ModelElement.fromElement(paramElement.field, packageGraph); } - } - _referenceChildren - .addEntries(typeParameters.map((p) => MapEntry(p.name, p))); + return param; + }).generateEntries()); + _referenceChildren.addEntries(typeParameters.generateEntries()); } return _referenceChildren; } + + @override + String get referenceName => element.name == '' ? enclosingElement.name : element.name; } diff --git a/lib/src/model/container.dart b/lib/src/model/container.dart index 0347ebbc05..f5e914a14f 100644 --- a/lib/src/model/container.dart +++ b/lib/src/model/container.dart @@ -265,24 +265,18 @@ abstract class Container extends ModelElement with TypeParameters { Map get referenceChildren { if (_referenceChildren == null) { _referenceChildren = {}; - for (var modelElement in allModelElements) { + _referenceChildren.addEntries( + allModelElements.whereNotType().whereNotType().generateEntries() + ); + + /*for (var modelElement in allModelElements) { // Never directly look up accessors. if (modelElement is Accessor) continue; // Constructors are special; see [Class.referenceChildrenExtra]. if (modelElement is Constructor) continue; - if (modelElement is Operator) { - // TODO(jcollins-g): once todo in [Operator.name] is fixed, remove - // this special case. - _referenceChildren[modelElement.element.name] = modelElement; - } else { - _referenceChildren[modelElement.name] = modelElement; - } - } - for (var entry in extraReferenceChildren) { - if (!_referenceChildren.containsKey(entry.key)) { - _referenceChildren[entry.key] = entry.value; - } - } + _referenceChildren[modelElement.referenceName] = modelElement; + }*/ + _referenceChildren.addEntriesIfAbsent(extraReferenceChildren); // Process unscoped parameters last to make sure they don't override // other options. for (var modelElement in allModelElements) { @@ -291,10 +285,7 @@ abstract class Container extends ModelElement with TypeParameters { // TODO(jcollins-g): Figure out something good to do in the ecosystem // here to wean people off the habit of unscoped parameter references. if (modelElement.hasParameters) { - for (var parameterElement in modelElement.parameters) { - _referenceChildren.putIfAbsent( - parameterElement.name, () => parameterElement); - } + _referenceChildren.addEntriesIfAbsent(modelElement.parameters.generateEntries()); } } _referenceChildren['this'] = this; diff --git a/lib/src/model/getter_setter_combo.dart b/lib/src/model/getter_setter_combo.dart index ab9df61c0b..1654ece62a 100644 --- a/lib/src/model/getter_setter_combo.dart +++ b/lib/src/model/getter_setter_combo.dart @@ -243,15 +243,15 @@ mixin GetterSetterCombo on ModelElement { Map get referenceChildren { if (_referenceChildren == null) { _referenceChildren = {}; - _referenceChildren.addEntries(allParameters.expand((p) { - if (p.name == name) { - // Force explicit references to parameters named the same as - // the method. - return [MapEntry('$name.${p.name}', p)]; - } - // Allow fallback handling in [Container] to deal with other cases. - return []; - })); + if (hasParameters) { + // Parameters in combos rarely matter for anything and are not part + // of the usual interface to a combo, so only reference them as part of + // [Container] fallbacks or if someone wants to explicitly specify a + // colliding parameter name. + _referenceChildren.addEntries(parameters.onlyExplicitOnCollisionWith(this)); + } + _referenceChildren.addEntries(modelType.typeArguments.explicitOnCollisionWith(this)); + } return _referenceChildren; } diff --git a/lib/src/model/library.dart b/lib/src/model/library.dart index 9cc3f3250c..6d68dedc8f 100644 --- a/lib/src/model/library.dart +++ b/lib/src/model/library.dart @@ -663,14 +663,21 @@ class Library extends ModelElement with Categorization, TopLevelContainer { @override Map get referenceChildren { if (_referenceChildren == null) { - _referenceChildren = Map.fromEntries( + _referenceChildren = {}; + var definedNamesModelElements = element.exportNamespace.definedNames.values.map((v) => ModelElement.fromElement(v, packageGraph)); + _referenceChildren.addEntries(definedNamesModelElements + .whereNotType() + .generateEntries()); + /* + Map.fromEntries( element.exportNamespace.definedNames.entries.expand((entry) sync* { var modelElement = ModelElement.fromElement(entry.value, packageGraph); if (modelElement is! Accessor) { yield MapEntry( - entry.key, ModelElement.fromElement(entry.value, packageGraph)); + modelElement.referenceName, modelElement); } })); + */ // TODO(jcollins-g): warn and get rid of this case where it shows up. // If a user is hiding parts of a prefix import, the user should not // refer to hidden members via the prefix, because that can be diff --git a/lib/src/model/method.dart b/lib/src/model/method.dart index 0d52ea1e2a..8f8a872ca9 100644 --- a/lib/src/model/method.dart +++ b/lib/src/model/method.dart @@ -127,10 +127,20 @@ class Method extends ModelElement Map _referenceChildren; @override Map get referenceChildren { - _referenceChildren ??= Map.fromEntries([ - ...typeParameters.map((p) => MapEntry(p.name, p)), - ...allParameters.map((p) => MapEntry(p.name, p)), - ]); + var from = documentationFrom.first as Method; + if (!identical(this, from)) { + return from.referenceChildren; + } + if (_referenceChildren == null) { + _referenceChildren = {}; + _referenceChildren.addEntriesIfAbsent([ + ...from.typeParameters.explicitOnCollisionWith(this), + ...from.allParameters.explicitOnCollisionWith(this), + ...from.modelType.typeArguments.explicitOnCollisionWith(this), + ...from.modelType.returnType.typeArguments.explicitOnCollisionWith( + this), + ]); + } return _referenceChildren; } } diff --git a/lib/src/model/model_function.dart b/lib/src/model/model_function.dart index 9f17da3cef..f6cc45949d 100644 --- a/lib/src/model/model_function.dart +++ b/lib/src/model/model_function.dart @@ -75,9 +75,8 @@ class ModelFunctionTyped extends ModelElement Map get referenceChildren { if (_referenceChildren == null) { _referenceChildren = {}; - _referenceChildren - .addEntries(typeParameters.map((p) => MapEntry(p.name, p))); - _referenceChildren.addEntries(parameters.map((p) => MapEntry(p.name, p))); + _referenceChildren.addEntriesIfAbsent(typeParameters.explicitOnCollisionWith(this)); + _referenceChildren.addEntriesIfAbsent(parameters.explicitOnCollisionWith(this)); } return _referenceChildren; } diff --git a/lib/src/model/operator.dart b/lib/src/model/operator.dart index bc101114ba..68551e541c 100644 --- a/lib/src/model/operator.dart +++ b/lib/src/model/operator.dart @@ -39,4 +39,7 @@ class Operator extends Method { // is removed. return 'operator ${super.name}'; } + + @override + String get referenceName => super.name; } diff --git a/lib/src/model/package.dart b/lib/src/model/package.dart index 1c07d4e721..1e0ee22ea5 100644 --- a/lib/src/model/package.dart +++ b/lib/src/model/package.dart @@ -407,19 +407,12 @@ class Package extends LibraryContainer if (_referenceChildren == null) { _referenceChildren = {}; _referenceChildren - .addEntries(allLibraries.map((l) => MapEntry(l.name, l))); + .addEntries(allLibraries.generateEntries()); // Do not override any preexisting data, and insert based on the // public library sort order. // TODO(jcollins-g): warn when results require package-global // lookups like this. - for (var lib in publicLibrariesSorted) { - for (var referableEntry in lib.referenceChildren.entries) { - // Avoiding tearoffs for performance reasons. - if (!_referenceChildren.containsKey(referableEntry.key)) { - _referenceChildren[referableEntry.key] = referableEntry.value; - } - } - } + _referenceChildren.addEntriesIfAbsent(publicLibrariesSorted.expand((l) => l.referenceChildren.entries)); } return _referenceChildren; } @@ -433,4 +426,7 @@ class Package extends LibraryContainer // Packages are not interpreted by the analyzer in such a way to generate // [CommentReference] nodes, so this is always empty. Map get commentRefs => {}; + + @override + String get referenceName => 'package:$name'; } diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart index a3331d36d6..152f4e8136 100644 --- a/lib/src/model/package_graph.dart +++ b/lib/src/model/package_graph.dart @@ -1031,39 +1031,27 @@ class PackageGraph with CommentReferable, Nameable { if (_referenceChildren == null) { _referenceChildren = {}; // Packages are the top priority. - _referenceChildren - .addEntries(packages.map((p) => MapEntry('package:${p.name}', p))); + _referenceChildren.addEntries(packages.generateEntries()); // Libraries are next. // TODO(jcollins-g): Warn about directly referencing libraries out of // scope? - for (var p in documentedPackages) { - for (var lib in p.publicLibrariesSorted) { - if (!_referenceChildren.containsKey(lib.name)) { - _referenceChildren[lib.name] = lib; - } - } - } + _referenceChildren.addEntriesIfAbsent(documentedPackages.expand((p) => p.publicLibrariesSorted).generateEntries()); + // TODO(jcollins-g): Warn about directly referencing top level items // out of scope? - for (var p in documentedPackages) { - for (var lib in p.publicLibrariesSorted) { - for (var me in [ - ...lib.publicConstants, - ...lib.publicFunctions, - ...lib.publicProperties, - ...lib.publicTypedefs, - ...lib.publicExtensions, - ...lib.publicClasses, - ...lib.publicEnums, - ...lib.publicMixins - ]) { - if (!_referenceChildren.containsKey(me.name)) { - _referenceChildren[me.name] = me; - } - } - } - } + _referenceChildren.addEntriesIfAbsent(documentedPackages + .expand((p) => p.publicLibrariesSorted) + .expand((l) => [ + ...l.publicConstants, + ...l.publicFunctions, + ...l.publicProperties, + ...l.publicTypedefs, + ...l.publicExtensions, + ...l.publicClasses, + ...l.publicEnums, + ...l.publicMixins + ]).generateEntries()); } return _referenceChildren; } diff --git a/lib/src/model/parameter.dart b/lib/src/model/parameter.dart index 45d0c1bf04..c85bfbd9cd 100644 --- a/lib/src/model/parameter.dart +++ b/lib/src/model/parameter.dart @@ -86,8 +86,12 @@ class Parameter extends ModelElement implements EnclosedElement { if (_referenceChildren == null) { _referenceChildren = {}; if (isCallable) { - _referenceChildren - .addEntries(parameters.map((p) => MapEntry(p.name, p))); + _referenceChildren.addEntriesIfAbsent(parameters.explicitOnCollisionWith(this)); + } + _referenceChildren.addEntriesIfAbsent(modelType.typeArguments.explicitOnCollisionWith(this)); + if (modelType is Callable) { + _referenceChildren.addEntriesIfAbsent( + (modelType as Callable).returnType.typeArguments.explicitOnCollisionWith(this)); } } return _referenceChildren; diff --git a/lib/src/model/type_parameter.dart b/lib/src/model/type_parameter.dart index ef62f04420..ad574982c7 100644 --- a/lib/src/model/type_parameter.dart +++ b/lib/src/model/type_parameter.dart @@ -76,7 +76,7 @@ class TypeParameter extends ModelElement { Map get referenceChildren { if (_referenceChildren == null) { _referenceChildren = {}; - _referenceChildren.addEntries(parameters.map((p) => MapEntry(p.name, p))); + _referenceChildren.addEntries(parameters.map((p) => MapEntry(p.referenceName, p))); _referenceChildren[boundType.name] = boundType; } return _referenceChildren; @@ -86,6 +86,9 @@ class TypeParameter extends ModelElement { Iterable get referenceParents => [enclosingElement]; @override TypeParameterElement get element => super.element; + + @override + String get referenceName => element.name; } mixin TypeParameters implements ModelElement { diff --git a/lib/src/model/typedef.dart b/lib/src/model/typedef.dart index ca2a94a23c..db23ada4fd 100644 --- a/lib/src/model/typedef.dart +++ b/lib/src/model/typedef.dart @@ -71,9 +71,9 @@ class Typedef extends ModelElement Map get referenceChildren { if (_referenceChildren == null) { _referenceChildren = {}; - _referenceChildren.addEntries(parameters.map((p) => MapEntry(p.name, p))); + _referenceChildren.addEntriesIfAbsent(parameters.explicitOnCollisionWith(this)); _referenceChildren - .addEntries(typeParameters.map((p) => MapEntry(p.name, p))); + .addEntriesIfAbsent(typeParameters.explicitOnCollisionWith(this)); } return _referenceChildren; } From 187beb5526497f473d6d7404460348ae73b6c897 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Sat, 26 Jun 2021 17:08:34 -0700 Subject: [PATCH 02/17] dartfmt --- lib/src/element_type.dart | 5 +- .../templates.runtime_renderers.dart | 129 ++++++++++++++++++ lib/src/model/class.dart | 3 +- lib/src/model/comment_referable.dart | 25 ++-- lib/src/model/constructor.dart | 3 +- lib/src/model/container.dart | 10 +- lib/src/model/getter_setter_combo.dart | 7 +- lib/src/model/library.dart | 9 +- lib/src/model/method.dart | 4 +- lib/src/model/model_function.dart | 6 +- lib/src/model/package.dart | 6 +- lib/src/model/package_graph.dart | 23 ++-- lib/src/model/parameter.dart | 12 +- lib/src/model/type_parameter.dart | 3 +- lib/src/model/typedef.dart | 3 +- 15 files changed, 197 insertions(+), 51 deletions(-) diff --git a/lib/src/element_type.dart b/lib/src/element_type.dart index 43604f7343..279c15b00d 100644 --- a/lib/src/element_type.dart +++ b/lib/src/element_type.dart @@ -212,8 +212,7 @@ class ParameterizedElementType extends DefinedElementType with Rendered { Iterable _typeArguments; @override Iterable get typeArguments => - _typeArguments ??= type - .typeArguments + _typeArguments ??= type.typeArguments .map((f) => ElementType.from(f, library, packageGraph)) .toList(growable: false); } @@ -311,7 +310,6 @@ abstract class DefinedElementType extends ElementType { return canonicalClass?.isPublic ?? false; } - DartType get _bound => type; DartType _instantiatedType; @@ -430,7 +428,6 @@ class CallableElementType extends DefinedElementType with Rendered, Callable { ElementTypeRenderer get _renderer => packageGraph.rendererFactory.callableElementTypeRenderer; - Iterable _typeArguments; @override Iterable get typeArguments => diff --git a/lib/src/generator/templates.runtime_renderers.dart b/lib/src/generator/templates.runtime_renderers.dart index 0fec72249a..58223f0d99 100644 --- a/lib/src/generator/templates.runtime_renderers.dart +++ b/lib/src/generator/templates.runtime_renderers.dart @@ -2660,6 +2660,26 @@ class _Renderer_CommentReferable extends RendererBase { getters: _invisibleGetters['CommentReferable'])); }, ), + 'referenceName': Property( + getValue: (CT_ c) => c.referenceName, + renderVariable: + (CT_ c, Property self, List remainingNames) { + if (remainingNames.isEmpty) { + return self.getValue(c).toString(); + } + var name = remainingNames.first; + var nextProperty = + _Renderer_String.propertyMap().getValue(name); + return nextProperty.renderVariable(self.getValue(c), + nextProperty, [...remainingNames.skip(1)]); + }, + isNullValue: (CT_ c) => c.referenceName == null, + renderValue: (CT_ c, RendererBase r, + List ast, StringSink sink) { + _render_String(c.referenceName, ast, r.template, sink, + parent: r); + }, + ), 'referenceParents': Property( getValue: (CT_ c) => c.referenceParents, renderVariable: (CT_ c, Property self, @@ -2965,6 +2985,26 @@ class _Renderer_Constructor extends RendererBase { parent: r, getters: _invisibleGetters['Map']); }, ), + 'referenceName': Property( + getValue: (CT_ c) => c.referenceName, + renderVariable: + (CT_ c, Property self, List remainingNames) { + if (remainingNames.isEmpty) { + return self.getValue(c).toString(); + } + var name = remainingNames.first; + var nextProperty = + _Renderer_String.propertyMap().getValue(name); + return nextProperty.renderVariable(self.getValue(c), + nextProperty, [...remainingNames.skip(1)]); + }, + isNullValue: (CT_ c) => c.referenceName == null, + renderValue: (CT_ c, RendererBase r, + List ast, StringSink sink) { + _render_String(c.referenceName, ast, r.template, sink, + parent: r); + }, + ), 'shortName': Property( getValue: (CT_ c) => c.shortName, renderVariable: @@ -10445,6 +10485,26 @@ class _Renderer_Operator extends RendererBase { _render_String(c.name, ast, r.template, sink, parent: r); }, ), + 'referenceName': Property( + getValue: (CT_ c) => c.referenceName, + renderVariable: + (CT_ c, Property self, List remainingNames) { + if (remainingNames.isEmpty) { + return self.getValue(c).toString(); + } + var name = remainingNames.first; + var nextProperty = + _Renderer_String.propertyMap().getValue(name); + return nextProperty.renderVariable(self.getValue(c), + nextProperty, [...remainingNames.skip(1)]); + }, + isNullValue: (CT_ c) => c.referenceName == null, + renderValue: (CT_ c, RendererBase r, + List ast, StringSink sink) { + _render_String(c.referenceName, ast, r.template, sink, + parent: r); + }, + ), }); _Renderer_Operator(Operator context, RendererBase parent, @@ -11148,6 +11208,26 @@ class _Renderer_Package extends RendererBase { parent: r, getters: _invisibleGetters['Map']); }, ), + 'referenceName': Property( + getValue: (CT_ c) => c.referenceName, + renderVariable: + (CT_ c, Property self, List remainingNames) { + if (remainingNames.isEmpty) { + return self.getValue(c).toString(); + } + var name = remainingNames.first; + var nextProperty = + _Renderer_String.propertyMap().getValue(name); + return nextProperty.renderVariable(self.getValue(c), + nextProperty, [...remainingNames.skip(1)]); + }, + isNullValue: (CT_ c) => c.referenceName == null, + renderValue: (CT_ c, RendererBase r, + List ast, StringSink sink) { + _render_String(c.referenceName, ast, r.template, sink, + parent: r); + }, + ), 'referenceParents': Property( getValue: (CT_ c) => c.referenceParents, renderVariable: (CT_ c, Property self, @@ -11726,6 +11806,33 @@ class _Renderer_ParameterizedElementType () => { ..._Renderer_DefinedElementType.propertyMap(), ..._Renderer_Rendered.propertyMap(), + 'type': Property( + getValue: (CT_ c) => c.type, + renderVariable: (CT_ c, Property self, + List remainingNames) => + self.renderSimpleVariable( + c, remainingNames, 'ParameterizedType'), + isNullValue: (CT_ c) => c.type == null, + renderValue: (CT_ c, RendererBase r, + List ast, StringSink sink) { + renderSimple(c.type, ast, r.template, sink, + parent: r, + getters: _invisibleGetters['ParameterizedType']); + }, + ), + 'typeArguments': Property( + getValue: (CT_ c) => c.typeArguments, + renderVariable: (CT_ c, Property self, + List remainingNames) => + self.renderSimpleVariable( + c, remainingNames, 'Iterable'), + renderIterable: (CT_ c, RendererBase r, + List ast, StringSink sink) { + return c.typeArguments.map((e) => _render_ElementType( + e, ast, r.template, sink, + parent: r)); + }, + ), }); _Renderer_ParameterizedElementType(ParameterizedElementType context, @@ -13849,6 +13956,26 @@ class _Renderer_TypeParameter extends RendererBase { parent: r, getters: _invisibleGetters['Map']); }, ), + 'referenceName': Property( + getValue: (CT_ c) => c.referenceName, + renderVariable: + (CT_ c, Property self, List remainingNames) { + if (remainingNames.isEmpty) { + return self.getValue(c).toString(); + } + var name = remainingNames.first; + var nextProperty = + _Renderer_String.propertyMap().getValue(name); + return nextProperty.renderVariable(self.getValue(c), + nextProperty, [...remainingNames.skip(1)]); + }, + isNullValue: (CT_ c) => c.referenceName == null, + renderValue: (CT_ c, RendererBase r, + List ast, StringSink sink) { + _render_String(c.referenceName, ast, r.template, sink, + parent: r); + }, + ), 'referenceParents': Property( getValue: (CT_ c) => c.referenceParents, renderVariable: (CT_ c, Property self, @@ -14615,6 +14742,7 @@ const _invisibleGetters = { 'referenceChildren', 'referenceParents', 'referenceGrandparentOverrides', + 'referenceName', 'library', 'element', 'packageGraph' @@ -15202,6 +15330,7 @@ const _invisibleGetters = { 'parameterKind', 'parameters' }, + 'ParameterizedType': {'hashCode', 'runtimeType', 'typeArguments'}, 'PropertyAccessorElement': { 'hashCode', 'runtimeType', diff --git a/lib/src/model/class.dart b/lib/src/model/class.dart index 0205e23ec6..db3f826765 100644 --- a/lib/src/model/class.dart +++ b/lib/src/model/class.dart @@ -613,7 +613,8 @@ class Class extends Container for (var constructor in source) { yield MapEntry(constructor.referenceName, constructor); yield MapEntry( - '${constructor.enclosingElement.name}.${constructor.referenceName}', constructor); + '${constructor.enclosingElement.name}.${constructor.referenceName}', + constructor); } } diff --git a/lib/src/model/comment_referable.dart b/lib/src/model/comment_referable.dart index 580975c7aa..1f2021e8cc 100644 --- a/lib/src/model/comment_referable.dart +++ b/lib/src/model/comment_referable.dart @@ -40,12 +40,16 @@ extension CommentReferableEntryGenerators on Iterable { /// Discards all entries that do not collide with [referable], /// and generates an [MapEntry] using [referable]'s /// [CommentReferable.referenceName] as a prefix for the item that does. - Iterable> onlyExplicitOnCollisionWith(CommentReferable referable) => where((r) => r.referenceName == referable.referenceName) - .map((r) => MapEntry('${referable.referenceName}.${r.referenceName}', r)); + Iterable> onlyExplicitOnCollisionWith( + CommentReferable referable) => + where((r) => r.referenceName == referable.referenceName).map( + (r) => MapEntry('${referable.referenceName}.${r.referenceName}', r)); /// Like [onlyExplicitOnCollisionWith], except does not discard non-conflicting /// items. - Iterable> explicitOnCollisionWith(CommentReferable referable) => map((r) { + Iterable> explicitOnCollisionWith( + CommentReferable referable) => + map((r) { if (r.referenceName == referable.referenceName) { return MapEntry('${referable.referenceName}.${r.referenceName}', r); } @@ -53,7 +57,8 @@ extension CommentReferableEntryGenerators on Iterable { }); /// Just generate entries from this iterable. - Iterable> generateEntries() => map((r) => MapEntry(r.referenceName, r)); + Iterable> generateEntries() => + map((r) => MapEntry(r.referenceName, r)); /// Return all values not of this type. Iterable whereNotType() sync* { @@ -67,13 +72,13 @@ extension CommentReferableEntryGenerators on Iterable { /// [CommentReferable.referenceChildren]. extension CommentReferableEntryBuilder on Map { /// Like [Map.putIfAbsent] except works on an iterable of entries. - void addEntriesIfAbsent(Iterable> entries) => - entries.forEach((e) { - if (!containsKey(e.key)) this[e.key] = e.value; - }); + void addEntriesIfAbsent( + Iterable> entries) => + entries.forEach((e) { + if (!containsKey(e.key)) this[e.key] = e.value; + }); } - /// Support comment reference lookups on a Nameable object. mixin CommentReferable implements Nameable { PackageGraph packageGraph; @@ -188,7 +193,6 @@ mixin CommentReferable implements Nameable { return retval; } - /// Map of [referenceName] to the elements that are a member of [this], but /// not this model element itself. Can be cached. /// @@ -214,7 +218,6 @@ mixin CommentReferable implements Nameable { // as [ModelElement.name]. Easier/possible after old lookup code is gone. String get referenceName => name; - // TODO(jcollins-g): Eliminate need for this in markdown_processor. Library get library => null; diff --git a/lib/src/model/constructor.dart b/lib/src/model/constructor.dart index b183616a69..9a01ea5871 100644 --- a/lib/src/model/constructor.dart +++ b/lib/src/model/constructor.dart @@ -143,5 +143,6 @@ class Constructor extends ModelElement } @override - String get referenceName => element.name == '' ? enclosingElement.name : element.name; + String get referenceName => + element.name == '' ? enclosingElement.name : element.name; } diff --git a/lib/src/model/container.dart b/lib/src/model/container.dart index f5e914a14f..3f43227ec8 100644 --- a/lib/src/model/container.dart +++ b/lib/src/model/container.dart @@ -265,9 +265,10 @@ abstract class Container extends ModelElement with TypeParameters { Map get referenceChildren { if (_referenceChildren == null) { _referenceChildren = {}; - _referenceChildren.addEntries( - allModelElements.whereNotType().whereNotType().generateEntries() - ); + _referenceChildren.addEntries(allModelElements + .whereNotType() + .whereNotType() + .generateEntries()); /*for (var modelElement in allModelElements) { // Never directly look up accessors. @@ -285,7 +286,8 @@ abstract class Container extends ModelElement with TypeParameters { // TODO(jcollins-g): Figure out something good to do in the ecosystem // here to wean people off the habit of unscoped parameter references. if (modelElement.hasParameters) { - _referenceChildren.addEntriesIfAbsent(modelElement.parameters.generateEntries()); + _referenceChildren + .addEntriesIfAbsent(modelElement.parameters.generateEntries()); } } _referenceChildren['this'] = this; diff --git a/lib/src/model/getter_setter_combo.dart b/lib/src/model/getter_setter_combo.dart index 1654ece62a..06f3a7c796 100644 --- a/lib/src/model/getter_setter_combo.dart +++ b/lib/src/model/getter_setter_combo.dart @@ -248,10 +248,11 @@ mixin GetterSetterCombo on ModelElement { // of the usual interface to a combo, so only reference them as part of // [Container] fallbacks or if someone wants to explicitly specify a // colliding parameter name. - _referenceChildren.addEntries(parameters.onlyExplicitOnCollisionWith(this)); + _referenceChildren + .addEntries(parameters.onlyExplicitOnCollisionWith(this)); } - _referenceChildren.addEntries(modelType.typeArguments.explicitOnCollisionWith(this)); - + _referenceChildren + .addEntries(modelType.typeArguments.explicitOnCollisionWith(this)); } return _referenceChildren; } diff --git a/lib/src/model/library.dart b/lib/src/model/library.dart index 6d68dedc8f..0d473879b0 100644 --- a/lib/src/model/library.dart +++ b/lib/src/model/library.dart @@ -664,10 +664,11 @@ class Library extends ModelElement with Categorization, TopLevelContainer { Map get referenceChildren { if (_referenceChildren == null) { _referenceChildren = {}; - var definedNamesModelElements = element.exportNamespace.definedNames.values.map((v) => ModelElement.fromElement(v, packageGraph)); - _referenceChildren.addEntries(definedNamesModelElements - .whereNotType() - .generateEntries()); + var definedNamesModelElements = element + .exportNamespace.definedNames.values + .map((v) => ModelElement.fromElement(v, packageGraph)); + _referenceChildren.addEntries( + definedNamesModelElements.whereNotType().generateEntries()); /* Map.fromEntries( element.exportNamespace.definedNames.entries.expand((entry) sync* { diff --git a/lib/src/model/method.dart b/lib/src/model/method.dart index 8f8a872ca9..6b82539f55 100644 --- a/lib/src/model/method.dart +++ b/lib/src/model/method.dart @@ -137,8 +137,8 @@ class Method extends ModelElement ...from.typeParameters.explicitOnCollisionWith(this), ...from.allParameters.explicitOnCollisionWith(this), ...from.modelType.typeArguments.explicitOnCollisionWith(this), - ...from.modelType.returnType.typeArguments.explicitOnCollisionWith( - this), + ...from.modelType.returnType.typeArguments + .explicitOnCollisionWith(this), ]); } return _referenceChildren; diff --git a/lib/src/model/model_function.dart b/lib/src/model/model_function.dart index f6cc45949d..236f1e6ea6 100644 --- a/lib/src/model/model_function.dart +++ b/lib/src/model/model_function.dart @@ -75,8 +75,10 @@ class ModelFunctionTyped extends ModelElement Map get referenceChildren { if (_referenceChildren == null) { _referenceChildren = {}; - _referenceChildren.addEntriesIfAbsent(typeParameters.explicitOnCollisionWith(this)); - _referenceChildren.addEntriesIfAbsent(parameters.explicitOnCollisionWith(this)); + _referenceChildren + .addEntriesIfAbsent(typeParameters.explicitOnCollisionWith(this)); + _referenceChildren + .addEntriesIfAbsent(parameters.explicitOnCollisionWith(this)); } return _referenceChildren; } diff --git a/lib/src/model/package.dart b/lib/src/model/package.dart index 1e0ee22ea5..e1f4113964 100644 --- a/lib/src/model/package.dart +++ b/lib/src/model/package.dart @@ -406,13 +406,13 @@ class Package extends LibraryContainer Map get referenceChildren { if (_referenceChildren == null) { _referenceChildren = {}; - _referenceChildren - .addEntries(allLibraries.generateEntries()); + _referenceChildren.addEntries(allLibraries.generateEntries()); // Do not override any preexisting data, and insert based on the // public library sort order. // TODO(jcollins-g): warn when results require package-global // lookups like this. - _referenceChildren.addEntriesIfAbsent(publicLibrariesSorted.expand((l) => l.referenceChildren.entries)); + _referenceChildren.addEntriesIfAbsent( + publicLibrariesSorted.expand((l) => l.referenceChildren.entries)); } return _referenceChildren; } diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart index 152f4e8136..62b501dbdf 100644 --- a/lib/src/model/package_graph.dart +++ b/lib/src/model/package_graph.dart @@ -1036,22 +1036,25 @@ class PackageGraph with CommentReferable, Nameable { // Libraries are next. // TODO(jcollins-g): Warn about directly referencing libraries out of // scope? - _referenceChildren.addEntriesIfAbsent(documentedPackages.expand((p) => p.publicLibrariesSorted).generateEntries()); + _referenceChildren.addEntriesIfAbsent(documentedPackages + .expand((p) => p.publicLibrariesSorted) + .generateEntries()); // TODO(jcollins-g): Warn about directly referencing top level items // out of scope? _referenceChildren.addEntriesIfAbsent(documentedPackages .expand((p) => p.publicLibrariesSorted) .expand((l) => [ - ...l.publicConstants, - ...l.publicFunctions, - ...l.publicProperties, - ...l.publicTypedefs, - ...l.publicExtensions, - ...l.publicClasses, - ...l.publicEnums, - ...l.publicMixins - ]).generateEntries()); + ...l.publicConstants, + ...l.publicFunctions, + ...l.publicProperties, + ...l.publicTypedefs, + ...l.publicExtensions, + ...l.publicClasses, + ...l.publicEnums, + ...l.publicMixins + ]) + .generateEntries()); } return _referenceChildren; } diff --git a/lib/src/model/parameter.dart b/lib/src/model/parameter.dart index c85bfbd9cd..a017f64d35 100644 --- a/lib/src/model/parameter.dart +++ b/lib/src/model/parameter.dart @@ -86,12 +86,16 @@ class Parameter extends ModelElement implements EnclosedElement { if (_referenceChildren == null) { _referenceChildren = {}; if (isCallable) { - _referenceChildren.addEntriesIfAbsent(parameters.explicitOnCollisionWith(this)); + _referenceChildren + .addEntriesIfAbsent(parameters.explicitOnCollisionWith(this)); } - _referenceChildren.addEntriesIfAbsent(modelType.typeArguments.explicitOnCollisionWith(this)); + _referenceChildren.addEntriesIfAbsent( + modelType.typeArguments.explicitOnCollisionWith(this)); if (modelType is Callable) { - _referenceChildren.addEntriesIfAbsent( - (modelType as Callable).returnType.typeArguments.explicitOnCollisionWith(this)); + _referenceChildren.addEntriesIfAbsent((modelType as Callable) + .returnType + .typeArguments + .explicitOnCollisionWith(this)); } } return _referenceChildren; diff --git a/lib/src/model/type_parameter.dart b/lib/src/model/type_parameter.dart index ad574982c7..b361bc5536 100644 --- a/lib/src/model/type_parameter.dart +++ b/lib/src/model/type_parameter.dart @@ -76,7 +76,8 @@ class TypeParameter extends ModelElement { Map get referenceChildren { if (_referenceChildren == null) { _referenceChildren = {}; - _referenceChildren.addEntries(parameters.map((p) => MapEntry(p.referenceName, p))); + _referenceChildren + .addEntries(parameters.map((p) => MapEntry(p.referenceName, p))); _referenceChildren[boundType.name] = boundType; } return _referenceChildren; diff --git a/lib/src/model/typedef.dart b/lib/src/model/typedef.dart index db23ada4fd..7c47548a50 100644 --- a/lib/src/model/typedef.dart +++ b/lib/src/model/typedef.dart @@ -71,7 +71,8 @@ class Typedef extends ModelElement Map get referenceChildren { if (_referenceChildren == null) { _referenceChildren = {}; - _referenceChildren.addEntriesIfAbsent(parameters.explicitOnCollisionWith(this)); + _referenceChildren + .addEntriesIfAbsent(parameters.explicitOnCollisionWith(this)); _referenceChildren .addEntriesIfAbsent(typeParameters.explicitOnCollisionWith(this)); } From 6a146e6fbd379d8566adc498f36c42e97fbda69a Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Sat, 26 Jun 2021 22:18:27 -0700 Subject: [PATCH 03/17] add tests for type parameters, handle more constructor parameter edge cases --- lib/dartdoc.dart | 2 +- .../model_comment_reference.dart | 45 ++++--- lib/src/generator/generator_frontend.dart | 1 + lib/src/markdown_processor.dart | 126 +++--------------- lib/src/matching_link_result.dart | 108 +++++++++++++++ lib/src/model/comment_referable.dart | 34 +++-- lib/src/model/dynamic.dart | 6 + lib/src/model/model_element.dart | 5 +- .../comment_referable_test.dart | 20 ++- test/end2end/model_test.dart | 51 ++++++- testing/test_package/lib/fake.dart | 10 ++ 11 files changed, 266 insertions(+), 142 deletions(-) create mode 100644 lib/src/matching_link_result.dart diff --git a/lib/dartdoc.dart b/lib/dartdoc.dart index 270bf61009..f8c121dfa5 100644 --- a/lib/dartdoc.dart +++ b/lib/dartdoc.dart @@ -20,7 +20,7 @@ import 'package:dartdoc/src/generator/generator.dart'; import 'package:dartdoc/src/generator/html_generator.dart'; import 'package:dartdoc/src/generator/markdown_generator.dart'; import 'package:dartdoc/src/logging.dart'; -import 'package:dartdoc/src/markdown_processor.dart' show markdownStats; +import 'package:dartdoc/src/matching_link_result.dart'; import 'package:dartdoc/src/model/model.dart'; import 'package:dartdoc/src/package_meta.dart'; import 'package:dartdoc/src/tool_definition.dart'; diff --git a/lib/src/comment_references/model_comment_reference.dart b/lib/src/comment_references/model_comment_reference.dart index aa7eb0779c..7d6e0fcbfb 100644 --- a/lib/src/comment_references/model_comment_reference.dart +++ b/lib/src/comment_references/model_comment_reference.dart @@ -13,6 +13,7 @@ abstract class ModelCommentReference { // TODO(jcollins-g): rewrite/discard this once default constructor tear-off // design process is complete. bool get allowDefaultConstructor; + bool get allowDefaultConstructorParameter; String get codeRef; bool get hasConstructorHint; bool get hasCallableHint; @@ -34,29 +35,43 @@ abstract class ModelCommentReference { /// and [ResourceProvider] after construction. class _ModelCommentReferenceImpl implements ModelCommentReference { bool _allowDefaultConstructor; - @override - bool get allowDefaultConstructor { - if (_allowDefaultConstructor == null) { - _allowDefaultConstructor = false; - var foundFirst = false; - String checkText; - // Check for two identically named identifiers at the end of the - // parse list, skipping over any junk or other nodes. - for (var node in parsed.reversed.whereType()) { - if (foundFirst) { - if (checkText == node.text) { + + void _initAllowCache() { + var referencePieces = parsed.whereType().toList(); + _allowDefaultConstructor = false; + _allowDefaultConstructorParameter = false; + if (referencePieces.length >= 2) { + IdentifierNode nodeLast; + for (var f in referencePieces) { + if (f.text == nodeLast?.text) { + if (identical(referencePieces.last, f)) { _allowDefaultConstructor = true; + } else { + _allowDefaultConstructorParameter = true; } - break; - } else { - foundFirst = true; - checkText = node.text; } + nodeLast = f; } } + } + + @override + bool get allowDefaultConstructor { + if (_allowDefaultConstructor == null) { + _initAllowCache(); + } return _allowDefaultConstructor; } + bool _allowDefaultConstructorParameter; + @override + bool get allowDefaultConstructorParameter { + if (_allowDefaultConstructorParameter == null) { + _initAllowCache(); + } + return _allowDefaultConstructorParameter; + } + @override final String codeRef; diff --git a/lib/src/generator/generator_frontend.dart b/lib/src/generator/generator_frontend.dart index 08d9f3caff..bc7875145e 100644 --- a/lib/src/generator/generator_frontend.dart +++ b/lib/src/generator/generator_frontend.dart @@ -47,6 +47,7 @@ class GeneratorFrontEnd implements Generator { } for (var lib in filterNonDocumented(package.libraries)) { + if (lib.name != 'dart:core') continue; logInfo('Generating docs for library ${lib.name} from ' '${lib.element.source.uri}...'); if (!lib.isAnonymous && !lib.hasDocumentation) { diff --git a/lib/src/markdown_processor.dart b/lib/src/markdown_processor.dart index e80c2db695..5e5c9a8af6 100644 --- a/lib/src/markdown_processor.dart +++ b/lib/src/markdown_processor.dart @@ -11,9 +11,9 @@ import 'dart:math'; import 'package:analyzer/dart/element/element.dart'; import 'package:dartdoc/src/comment_references/model_comment_reference.dart'; import 'package:dartdoc/src/element_type.dart'; +import 'package:dartdoc/src/matching_link_result.dart'; import 'package:dartdoc/src/model/comment_referable.dart'; import 'package:dartdoc/src/model/model.dart'; -import 'package:dartdoc/src/quiver.dart'; import 'package:dartdoc/src/warnings.dart'; import 'package:markdown/markdown.dart' as md; import 'package:meta/meta.dart'; @@ -173,107 +173,6 @@ final List _markdownBlockSyntaxes = [ // Remove these schemas from the display text for hyperlinks. final RegExp _hideSchemes = RegExp('^(http|https)://'); -class MatchingLinkResult { - final CommentReferable commentReferable; - final bool warn; - - MatchingLinkResult(this.commentReferable, {this.warn = true}); - - @override - bool operator ==(Object other) { - return other is MatchingLinkResult && - commentReferable == other.commentReferable && - warn == other.warn; - } - - @override - int get hashCode => hash2(commentReferable, warn); - - bool isEquivalentTo(MatchingLinkResult other) { - var compareThis = commentReferable; - var compareOther = other.commentReferable; - - if (compareThis is Accessor) { - compareThis = (compareThis as Accessor).enclosingCombo; - } - - if (compareOther is Accessor) { - compareOther = (compareOther as Accessor).enclosingCombo; - } - - if (compareThis is ModelElement && - compareThis.canonicalModelElement != null) { - compareThis = (compareThis as ModelElement).canonicalModelElement; - } - if (compareOther is ModelElement && - compareOther.canonicalModelElement != null) { - compareOther = (compareOther as ModelElement).canonicalModelElement; - } - if (compareThis == compareOther) return true; - - // The old implementation just throws away Parameter matches to avoid - // problems with warning unnecessarily at higher levels of the code. - // I'd like to fix this at a different layer with the new lookup, so treat - // this as equivalent to a null type. - if (compareOther is Parameter && compareThis == null) { - return true; - } - if (compareThis is Parameter && compareOther == null) { - return true; - } - // Same with TypeParameter. - if (compareOther is TypeParameter && compareThis == null) { - return true; - } - if (compareThis is TypeParameter && compareOther == null) { - return true; - } - - return false; - } - - @override - String toString() { - return 'element: [${commentReferable is Constructor ? 'new ' : ''}${commentReferable?.fullyQualifiedName}] warn: $warn'; - } -} - -class _MarkdownStats { - int totalReferences = 0; - int resolvedReferences = 0; - int resolvedNewLookupReferences = 0; - int resolvedOldLookupReferences = 0; - int resolvedEquivalentlyReferences = 0; - - String _valueAndPercent(int references) { - return '$references (${references.toDouble() / totalReferences.toDouble() * 100}%)'; - } - - String buildReport() { - var report = StringBuffer(); - report.writeln('Reference Counts:'); - report.writeln('total references: $totalReferences'); - report.writeln( - 'resolved references: ${_valueAndPercent(resolvedReferences)}'); - if (resolvedNewLookupReferences > 0) { - report.writeln( - 'resolved references with new lookup: $resolvedNewLookupReferences (${resolvedNewLookupReferences / totalReferences * 100}%)'); - } - if (resolvedOldLookupReferences > 0) { - report.writeln( - 'resolved references with old lookup: $resolvedOldLookupReferences (${resolvedOldLookupReferences / totalReferences * 100}%)'); - } - if (resolvedEquivalentlyReferences > 0) { - report.writeln( - 'resolved references with equivalent links: $resolvedEquivalentlyReferences (${resolvedEquivalentlyReferences / totalReferences * 100}%)'); - } - return report.toString(); - } -} - -/// TODO(jcollins-g): perhaps a more generic stats gathering apparatus for -/// dartdoc would be useful, and one that isn't process-global. -final markdownStats = _MarkdownStats(); class IterableBlockParser extends md.BlockParser { IterableBlockParser(List lines, md.Document document) @@ -304,7 +203,8 @@ ModelElement _getPreferredClass(ModelElement modelElement) { } /// Return false if the passed [referable] is a default [Constructor], -/// or if it is shadowing another type of element. +/// or if it is shadowing another type of element, or is a parameter of +/// one of the above. bool _rejectDefaultAndShadowingConstructors(CommentReferable referable) { if (referable is Constructor) { if (referable.name == referable.enclosingElement.name) { @@ -334,12 +234,19 @@ MatchingLinkResult _getMatchingLinkElementCommentReferable( warnable.commentRefs[codeRef] ?? ModelCommentReference.synthetic(codeRef); bool Function(CommentReferable) filter; + bool Function(CommentReferable) allowTree; + // Constructor references are pretty ambiguous by nature since they are + // declared with the same name as the class they are constructing, and even + // if they don't use field-formal parameters, sometimes have parameters + // named the same as members. + // Maybe clean this up with inspiration from constructor tear-off syntax? if (commentReference.allowDefaultConstructor) { // Neither reject, nor require, a default constructor in the event // the comment reference structure implies one. (We can not require it // in case a library name is the same as a member class name and the class - // is the intended lookup). + // is the intended lookup). For example, [FooClass.FooClass] structurally + // "looks like" a default constructor, so we should allow it here. filter = commentReference.hasCallableHint ? _requireCallable : null; } else if (commentReference.hasConstructorHint && commentReference.hasCallableHint) { @@ -350,13 +257,18 @@ MatchingLinkResult _getMatchingLinkElementCommentReferable( // Trailing parens indicate we are looking for a callable. filter = _requireCallable; } else { - // Without hints, reject default constructors to force resolution to the - // class. + // Without hints, reject default constructors and their parameters to force + // resolution to the class. filter = _rejectDefaultAndShadowingConstructors; + + if (!commentReference.allowDefaultConstructorParameter) { + allowTree = _rejectDefaultAndShadowingConstructors; + } } + var lookupResult = - warnable.referenceBy(commentReference.referenceBy, filter: filter); + warnable.referenceBy(commentReference.referenceBy, allowTree: allowTree, filter: filter); // TODO(jcollins-g): Consider prioritizing analyzer resolution before custom. return MatchingLinkResult(lookupResult); diff --git a/lib/src/matching_link_result.dart b/lib/src/matching_link_result.dart new file mode 100644 index 0000000000..beb53c2a14 --- /dev/null +++ b/lib/src/matching_link_result.dart @@ -0,0 +1,108 @@ +// Copyright (c) 2021, 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. + +import 'package:dartdoc/src/model/comment_referable.dart'; +import 'package:dartdoc/src/model/model.dart'; +import 'package:dartdoc/src/quiver.dart'; + +class MatchingLinkResult { + final CommentReferable commentReferable; + final bool warn; + + MatchingLinkResult(this.commentReferable, {this.warn = true}); + + @override + bool operator ==(Object other) { + return other is MatchingLinkResult && + commentReferable == other.commentReferable && + warn == other.warn; + } + + @override + int get hashCode => hash2(commentReferable, warn); + + bool isEquivalentTo(MatchingLinkResult other) { + var compareThis = commentReferable; + var compareOther = other.commentReferable; + + if (compareThis is Accessor) { + compareThis = (compareThis as Accessor).enclosingCombo; + } + + if (compareOther is Accessor) { + compareOther = (compareOther as Accessor).enclosingCombo; + } + + if (compareThis is ModelElement && + compareThis.canonicalModelElement != null) { + compareThis = (compareThis as ModelElement).canonicalModelElement; + } + if (compareOther is ModelElement && + compareOther.canonicalModelElement != null) { + compareOther = (compareOther as ModelElement).canonicalModelElement; + } + if (compareThis == compareOther) return true; + // The old implementation just throws away Parameter matches to avoid + // problems with warning unnecessarily at higher levels of the code. + // I'd like to fix this at a different layer with the new lookup, so treat + // this as equivalent to a null type. + if (compareOther is Parameter && compareThis == null) { + return true; + } + if (compareThis is Parameter && compareOther == null) { + return true; + } + // Same with TypeParameter. + if (compareOther is TypeParameter && compareThis == null) { + return true; + } + if (compareThis is TypeParameter && compareOther == null) { + return true; + } + + return false; + } + + @override + String toString() { + return 'element: [${commentReferable is Constructor ? 'new ' : ''}${commentReferable?.fullyQualifiedName}] warn: $warn'; + } +} + +class _MarkdownStats { + int totalReferences = 0; + int resolvedReferences = 0; + int resolvedNewLookupReferences = 0; + int resolvedOldLookupReferences = 0; + int resolvedEquivalentlyReferences = 0; + + String _valueAndPercent(int references) { + return '$references (${references.toDouble() / totalReferences.toDouble() * 100}%)'; + } + + String buildReport() { + var report = StringBuffer(); + report.writeln('Reference Counts:'); + report.writeln('total references: $totalReferences'); + report.writeln( + 'resolved references: ${_valueAndPercent(resolvedReferences)}'); + if (resolvedNewLookupReferences > 0) { + report.writeln( + 'resolved references with new lookup: $resolvedNewLookupReferences (${resolvedNewLookupReferences / totalReferences * 100}%)'); + } + if (resolvedOldLookupReferences > 0) { + report.writeln( + 'resolved references with old lookup: $resolvedOldLookupReferences (${resolvedOldLookupReferences / totalReferences * 100}%)'); + } + if (resolvedEquivalentlyReferences > 0) { + report.writeln( + 'resolved references with equivalent links: $resolvedEquivalentlyReferences (${resolvedEquivalentlyReferences / totalReferences * 100}%)'); + } + return report.toString(); + } +} + +/// TODO(jcollins-g): perhaps a more generic stats gathering apparatus for +/// dartdoc would be useful, and one that isn't process-global. +final markdownStats = _MarkdownStats(); \ No newline at end of file diff --git a/lib/src/model/comment_referable.dart b/lib/src/model/comment_referable.dart index 1f2021e8cc..82acf71237 100644 --- a/lib/src/model/comment_referable.dart +++ b/lib/src/model/comment_referable.dart @@ -93,13 +93,16 @@ mixin CommentReferable implements Nameable { /// Look up a comment reference by its component parts. If [tryParents] is /// true, try looking up the same reference in any parents of [this]. /// Will skip over results that do not pass a given [filter] and keep - /// searching. + /// searching. Will skip over entire subtrees whose parent node does + /// not pass [allowTree]. @nonVirtual CommentReferable referenceBy(List reference, {bool tryParents = true, bool Function(CommentReferable) filter, + bool Function(CommentReferable) allowTree, Iterable parentOverrides}) { filter ??= (r) => true; + allowTree ??= (r) => true; parentOverrides ??= referenceParents; if (reference.isEmpty) { if (tryParents == false) return this; @@ -110,11 +113,12 @@ mixin CommentReferable implements Nameable { /// Search for the reference. for (var referenceLookup in childLookups(reference)) { if (scope != null) { - result = lookupViaScope(referenceLookup, filter); + result = lookupViaScope(referenceLookup, filter, allowTree); if (result != null) break; } if (referenceChildren.containsKey(referenceLookup.lookup)) { - result = _lookupViaReferenceChildren(referenceLookup, filter); + result = recurseChildrenAndFilter( + referenceLookup, referenceChildren[referenceLookup.lookup], allowTree: allowTree, filter: filter); if (result != null) break; } } @@ -124,6 +128,7 @@ mixin CommentReferable implements Nameable { result = parent.referenceBy(reference, tryParents: true, parentOverrides: referenceGrandparentOverrides, + allowTree: allowTree, filter: filter); if (result != null) break; } @@ -138,6 +143,7 @@ mixin CommentReferable implements Nameable { /// [CommentReferable], but you still want to have an implementation of /// [scope]. CommentReferable lookupViaScope(ReferenceChildrenLookup referenceLookup, + bool Function(CommentReferable) allowTree, bool Function(CommentReferable) filter) { var resultElement = scope.lookupPreferGetter(referenceLookup.lookup); if (resultElement == null) return null; @@ -150,29 +156,29 @@ mixin CommentReferable implements Nameable { '[Container] member detected, support not implemented for analyzer scope inside containers'); return null; } - return recurseChildrenAndFilter(referenceLookup, result, filter); + if (!allowTree(result)) return null; + return recurseChildrenAndFilter(referenceLookup, result, allowTree: allowTree, filter: filter); } - CommentReferable _lookupViaReferenceChildren( - ReferenceChildrenLookup referenceLookup, - bool Function(CommentReferable) filter) => - recurseChildrenAndFilter( - referenceLookup, referenceChildren[referenceLookup.lookup], filter); - /// Given a [result] found in an implementation of [lookupViaScope] or /// [_lookupViaReferenceChildren], recurse through children, skipping over /// results that do not match the filter. CommentReferable recurseChildrenAndFilter( ReferenceChildrenLookup referenceLookup, CommentReferable result, - bool Function(CommentReferable) filter) { + {bool Function(CommentReferable) allowTree, + bool Function(CommentReferable) filter}) { assert(result != null); if (referenceLookup.remaining.isNotEmpty) { - result = result.referenceBy(referenceLookup.remaining, - tryParents: false, filter: filter); + if (allowTree(result)) { + result = result.referenceBy(referenceLookup.remaining, + tryParents: false, allowTree: allowTree, filter: filter); + } else { + result = null; + } } else if (!filter(result)) { result = result.referenceBy([referenceLookup.lookup], - tryParents: false, filter: filter); + tryParents: false, allowTree: allowTree, filter: filter); } if (!filter(result)) { result = null; diff --git a/lib/src/model/dynamic.dart b/lib/src/model/dynamic.dart index 5e49d673a0..a345f4c8c6 100644 --- a/lib/src/model/dynamic.dart +++ b/lib/src/model/dynamic.dart @@ -41,4 +41,10 @@ class Dynamic extends ModelElement { @override Iterable get referenceParents => []; + + @override + bool operator ==(Object other) => other is Dynamic; + + @override + int get hashCode => 31415; } diff --git a/lib/src/model/model_element.dart b/lib/src/model/model_element.dart index dd4f43e872..e94749bb04 100644 --- a/lib/src/model/model_element.dart +++ b/lib/src/model/model_element.dart @@ -422,8 +422,11 @@ abstract class ModelElement extends Canonicalization checkReferences.add(from.enclosingCombo); } for (var e in checkReferences) { + // Some elements don't have modelNodes or aren't traversed by + // the element visitor, or both. + assert(e is Parameter || e.modelNode != null); _commentRefs - .addAll({for (var r in e.modelNode.commentRefs) r.codeRef: r}); + .addAll({for (var r in e.modelNode?.commentRefs ?? []) r.codeRef: r}); } } } diff --git a/test/comment_referable/comment_referable_test.dart b/test/comment_referable/comment_referable_test.dart index 1784bea91f..31affa0ce3 100644 --- a/test/comment_referable/comment_referable_test.dart +++ b/test/comment_referable/comment_referable_test.dart @@ -14,14 +14,17 @@ const _separator = '.'; abstract class Base extends Nameable with CommentReferable { List children; + Base parent; + /// Utility function to quickly build structures similar to [ModelElement] /// hierarchies in dartdoc in tests. /// Returns the added (or already existing) [Base]. Base add(String newName); T lookup(String value, - {bool Function(CommentReferable) filter}) => - referenceBy(value.split(_separator), filter: filter); + {bool Function(CommentReferable) allowTree, + bool Function(CommentReferable) filter}) => + referenceBy(value.split(_separator), allowTree: allowTree, filter: filter); @override Element get element => throw UnimplementedError(); @@ -90,6 +93,7 @@ class TopChild extends Child { final String name; @override final List children; + @override final Top parent; TopChild(this.name, this.children, this.parent); @@ -107,6 +111,7 @@ class GenericChild extends Child { final String name; @override final List children; + @override final Base parent; GenericChild(this.name, this.children, this.parent); @@ -158,6 +163,17 @@ void main() { isA()); }); + test('Check that allowTree works', () { + referable.add('lib4'); + var lib4lib4 = referable.add('lib4.lib4'); + var tooDeepSub1 = referable.add('lib4.lib4.sub1'); + var sub1 = referable.add('lib4.sub1'); + var sub2 = referable.add('lib4.sub2'); + expect(sub2.lookup('lib4.lib4'), equals(lib4lib4)); + expect(sub2.lookup('lib4.sub1'), equals(tooDeepSub1)); + expect(sub2.lookup('lib4.sub1', allowTree: ((r) => r is Base && (r.parent is Top))), equals(sub1)); + }); + test('Check that grandparent overrides work', () { referable.add('lib4'); var i1 = referable.add('lib4.intermediate1'); diff --git a/test/end2end/model_test.dart b/test/end2end/model_test.dart index 23b92ed5c9..3ace7419a2 100644 --- a/test/end2end/model_test.dart +++ b/test/end2end/model_test.dart @@ -10,6 +10,7 @@ import 'package:analyzer/source/line_info.dart'; import 'package:async/async.dart'; import 'package:dartdoc/src/element_type.dart'; import 'package:dartdoc/src/markdown_processor.dart'; +import 'package:dartdoc/src/matching_link_result.dart'; import 'package:dartdoc/src/model/feature.dart'; import 'package:dartdoc/src/model/model.dart'; import 'package:dartdoc/src/package_config_provider.dart'; @@ -2252,6 +2253,52 @@ void main() { }); }); + group('Type parameter lookups work', () { + Class TypeParameterThings; + Field aName, aThing; + TypeParameter A, B, C, D; + Method aMethod; + Parameter aParam, anotherParam, typedParam; + ModelFunction aTopLevelTypeParameterFunction; + + setUpAll(() { + aTopLevelTypeParameterFunction = fakeLibrary.functions.firstWhere((f) => f.name == 'aTopLevelTypeParameterFunction'); + // TODO(jcollins-g): dart-lang/dartdoc#2704, HTML and type parameters + // on the extended type should not be present here. + D = aTopLevelTypeParameterFunction.typeParameters.firstWhere((t) => t.name.startsWith('D extends TypeParameterThings')); + typedParam = aTopLevelTypeParameterFunction.parameters.firstWhere((t) => t.name == 'typedParam'); + + TypeParameterThings = fakeLibrary.allClasses.firstWhere((c) => c.name == 'TypeParameterThings'); + aName = TypeParameterThings.instanceFields.firstWhere((f) => f.name == 'aName'); + aThing = TypeParameterThings.instanceFields.firstWhere((f) => f.name == 'aThing'); + aMethod = TypeParameterThings.instanceMethods.firstWhere((m) => m.name == 'aMethod'); + + C = aMethod.typeParameters.firstWhere((t) => t.name == 'C'); + aParam = aMethod.parameters.firstWhere((p) => p.name == 'aParam'); + anotherParam = aMethod.parameters.firstWhere((p) => p.name == 'anotherParam'); + + A = TypeParameterThings.typeParameters.firstWhere((t) => t.name == 'A'); + B = TypeParameterThings.typeParameters.firstWhere((t) => t.name == 'B extends FactoryConstructorThings'); + }); + + test('on classes', () { + expect(bothLookup(TypeParameterThings, 'A'), equals(MatchingLinkResult(A))); + expect(bothLookup(TypeParameterThings, 'B'), equals(MatchingLinkResult(B))); + expect(bothLookup(aName, 'A'), equals(MatchingLinkResult(A))); + expect(bothLookup(aThing, 'B'), equals(MatchingLinkResult(B))); + expect(bothLookup(aMethod, 'C'), equals(MatchingLinkResult(C))); + expect(bothLookup(aParam, 'A'), equals(MatchingLinkResult(A))); + // Original didn't handle this case properly and popped out to higher + // levels. + expect(newLookup(anotherParam, 'C'), equals(MatchingLinkResult(C))); + }); + + test('on top level methods', () { + expect(bothLookup(aTopLevelTypeParameterFunction, 'D'), equals(MatchingLinkResult(D))); + expect(bothLookup(typedParam, 'D'), equals(MatchingLinkResult(D))); + }); + }); + group('Ordinary namespace cases', () { Package DartPackage; Library Dart, mylibpub; @@ -2467,10 +2514,10 @@ void main() { }); test('in method scope referring to parameters and variables', () { - expect(bothLookup(aMethod, 'yetAnotherName'), + /*expect(bothLookup(aMethod, 'yetAnotherName'), equals(MatchingLinkResult(yetAnotherName))); expect(bothLookup(aMethod, 'FactoryConstructorThings.yetAnotherName'), - equals(MatchingLinkResult(yetAnotherNameField))); + equals(MatchingLinkResult(yetAnotherNameField)));*/ expect( bothLookup( aMethod, 'FactoryConstructorThings.anotherName.anotherName'), diff --git a/testing/test_package/lib/fake.dart b/testing/test_package/lib/fake.dart index 325e9bb021..4e3c56437d 100644 --- a/testing/test_package/lib/fake.dart +++ b/testing/test_package/lib/fake.dart @@ -1281,4 +1281,14 @@ class FactoryConstructorThings { } void aMethod(bool yetAnotherName) {} +} + + +D aTopLevelTypeParameterFunction(D typedParam) {} + +abstract class TypeParameterThings { + A aName; + final List aThing; + + B aMethod(A aParam, C anotherParam); } \ No newline at end of file From 66f3b02325690ed545231692011aba010bd8cdd2 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Sat, 26 Jun 2021 22:23:48 -0700 Subject: [PATCH 04/17] dartfmt rebuild --- lib/src/markdown_processor.dart | 6 +-- lib/src/matching_link_result.dart | 2 +- lib/src/model/comment_referable.dart | 12 +++--- lib/src/model/model_element.dart | 6 ++- .../comment_referable_test.dart | 10 +++-- test/end2end/model_test.dart | 42 ++++++++++++------- 6 files changed, 48 insertions(+), 30 deletions(-) diff --git a/lib/src/markdown_processor.dart b/lib/src/markdown_processor.dart index 5e5c9a8af6..74d7f9c2ea 100644 --- a/lib/src/markdown_processor.dart +++ b/lib/src/markdown_processor.dart @@ -173,7 +173,6 @@ final List _markdownBlockSyntaxes = [ // Remove these schemas from the display text for hyperlinks. final RegExp _hideSchemes = RegExp('^(http|https)://'); - class IterableBlockParser extends md.BlockParser { IterableBlockParser(List lines, md.Document document) : super(lines, document); @@ -266,9 +265,8 @@ MatchingLinkResult _getMatchingLinkElementCommentReferable( } } - - var lookupResult = - warnable.referenceBy(commentReference.referenceBy, allowTree: allowTree, filter: filter); + var lookupResult = warnable.referenceBy(commentReference.referenceBy, + allowTree: allowTree, filter: filter); // TODO(jcollins-g): Consider prioritizing analyzer resolution before custom. return MatchingLinkResult(lookupResult); diff --git a/lib/src/matching_link_result.dart b/lib/src/matching_link_result.dart index beb53c2a14..c4f849a78f 100644 --- a/lib/src/matching_link_result.dart +++ b/lib/src/matching_link_result.dart @@ -105,4 +105,4 @@ class _MarkdownStats { /// TODO(jcollins-g): perhaps a more generic stats gathering apparatus for /// dartdoc would be useful, and one that isn't process-global. -final markdownStats = _MarkdownStats(); \ No newline at end of file +final markdownStats = _MarkdownStats(); diff --git a/lib/src/model/comment_referable.dart b/lib/src/model/comment_referable.dart index 82acf71237..df55624d0c 100644 --- a/lib/src/model/comment_referable.dart +++ b/lib/src/model/comment_referable.dart @@ -118,7 +118,8 @@ mixin CommentReferable implements Nameable { } if (referenceChildren.containsKey(referenceLookup.lookup)) { result = recurseChildrenAndFilter( - referenceLookup, referenceChildren[referenceLookup.lookup], allowTree: allowTree, filter: filter); + referenceLookup, referenceChildren[referenceLookup.lookup], + allowTree: allowTree, filter: filter); if (result != null) break; } } @@ -142,7 +143,8 @@ mixin CommentReferable implements Nameable { /// Override if [Scope.lookup] may return elements not corresponding to a /// [CommentReferable], but you still want to have an implementation of /// [scope]. - CommentReferable lookupViaScope(ReferenceChildrenLookup referenceLookup, + CommentReferable lookupViaScope( + ReferenceChildrenLookup referenceLookup, bool Function(CommentReferable) allowTree, bool Function(CommentReferable) filter) { var resultElement = scope.lookupPreferGetter(referenceLookup.lookup); @@ -157,15 +159,15 @@ mixin CommentReferable implements Nameable { return null; } if (!allowTree(result)) return null; - return recurseChildrenAndFilter(referenceLookup, result, allowTree: allowTree, filter: filter); + return recurseChildrenAndFilter(referenceLookup, result, + allowTree: allowTree, filter: filter); } /// Given a [result] found in an implementation of [lookupViaScope] or /// [_lookupViaReferenceChildren], recurse through children, skipping over /// results that do not match the filter. CommentReferable recurseChildrenAndFilter( - ReferenceChildrenLookup referenceLookup, - CommentReferable result, + ReferenceChildrenLookup referenceLookup, CommentReferable result, {bool Function(CommentReferable) allowTree, bool Function(CommentReferable) filter}) { assert(result != null); diff --git a/lib/src/model/model_element.dart b/lib/src/model/model_element.dart index e94749bb04..a407200b30 100644 --- a/lib/src/model/model_element.dart +++ b/lib/src/model/model_element.dart @@ -425,8 +425,10 @@ abstract class ModelElement extends Canonicalization // Some elements don't have modelNodes or aren't traversed by // the element visitor, or both. assert(e is Parameter || e.modelNode != null); - _commentRefs - .addAll({for (var r in e.modelNode?.commentRefs ?? []) r.codeRef: r}); + _commentRefs.addAll({ + for (var r in e.modelNode?.commentRefs ?? []) + r.codeRef: r + }); } } } diff --git a/test/comment_referable/comment_referable_test.dart b/test/comment_referable/comment_referable_test.dart index 31affa0ce3..127d6262ef 100644 --- a/test/comment_referable/comment_referable_test.dart +++ b/test/comment_referable/comment_referable_test.dart @@ -23,8 +23,9 @@ abstract class Base extends Nameable with CommentReferable { T lookup(String value, {bool Function(CommentReferable) allowTree, - bool Function(CommentReferable) filter}) => - referenceBy(value.split(_separator), allowTree: allowTree, filter: filter); + bool Function(CommentReferable) filter}) => + referenceBy(value.split(_separator), + allowTree: allowTree, filter: filter); @override Element get element => throw UnimplementedError(); @@ -171,7 +172,10 @@ void main() { var sub2 = referable.add('lib4.sub2'); expect(sub2.lookup('lib4.lib4'), equals(lib4lib4)); expect(sub2.lookup('lib4.sub1'), equals(tooDeepSub1)); - expect(sub2.lookup('lib4.sub1', allowTree: ((r) => r is Base && (r.parent is Top))), equals(sub1)); + expect( + sub2.lookup('lib4.sub1', + allowTree: ((r) => r is Base && (r.parent is Top))), + equals(sub1)); }); test('Check that grandparent overrides work', () { diff --git a/test/end2end/model_test.dart b/test/end2end/model_test.dart index 3ace7419a2..0df364050e 100644 --- a/test/end2end/model_test.dart +++ b/test/end2end/model_test.dart @@ -2262,28 +2262,39 @@ void main() { ModelFunction aTopLevelTypeParameterFunction; setUpAll(() { - aTopLevelTypeParameterFunction = fakeLibrary.functions.firstWhere((f) => f.name == 'aTopLevelTypeParameterFunction'); + aTopLevelTypeParameterFunction = fakeLibrary.functions + .firstWhere((f) => f.name == 'aTopLevelTypeParameterFunction'); // TODO(jcollins-g): dart-lang/dartdoc#2704, HTML and type parameters // on the extended type should not be present here. - D = aTopLevelTypeParameterFunction.typeParameters.firstWhere((t) => t.name.startsWith('D extends TypeParameterThings')); - typedParam = aTopLevelTypeParameterFunction.parameters.firstWhere((t) => t.name == 'typedParam'); - - TypeParameterThings = fakeLibrary.allClasses.firstWhere((c) => c.name == 'TypeParameterThings'); - aName = TypeParameterThings.instanceFields.firstWhere((f) => f.name == 'aName'); - aThing = TypeParameterThings.instanceFields.firstWhere((f) => f.name == 'aThing'); - aMethod = TypeParameterThings.instanceMethods.firstWhere((m) => m.name == 'aMethod'); + D = aTopLevelTypeParameterFunction.typeParameters.firstWhere( + (t) => t.name.startsWith('D extends TypeParameterThings')); + typedParam = aTopLevelTypeParameterFunction.parameters + .firstWhere((t) => t.name == 'typedParam'); + + TypeParameterThings = fakeLibrary.allClasses + .firstWhere((c) => c.name == 'TypeParameterThings'); + aName = TypeParameterThings.instanceFields + .firstWhere((f) => f.name == 'aName'); + aThing = TypeParameterThings.instanceFields + .firstWhere((f) => f.name == 'aThing'); + aMethod = TypeParameterThings.instanceMethods + .firstWhere((m) => m.name == 'aMethod'); C = aMethod.typeParameters.firstWhere((t) => t.name == 'C'); aParam = aMethod.parameters.firstWhere((p) => p.name == 'aParam'); - anotherParam = aMethod.parameters.firstWhere((p) => p.name == 'anotherParam'); + anotherParam = + aMethod.parameters.firstWhere((p) => p.name == 'anotherParam'); A = TypeParameterThings.typeParameters.firstWhere((t) => t.name == 'A'); - B = TypeParameterThings.typeParameters.firstWhere((t) => t.name == 'B extends FactoryConstructorThings'); + B = TypeParameterThings.typeParameters + .firstWhere((t) => t.name == 'B extends FactoryConstructorThings'); }); test('on classes', () { - expect(bothLookup(TypeParameterThings, 'A'), equals(MatchingLinkResult(A))); - expect(bothLookup(TypeParameterThings, 'B'), equals(MatchingLinkResult(B))); + expect(bothLookup(TypeParameterThings, 'A'), + equals(MatchingLinkResult(A))); + expect(bothLookup(TypeParameterThings, 'B'), + equals(MatchingLinkResult(B))); expect(bothLookup(aName, 'A'), equals(MatchingLinkResult(A))); expect(bothLookup(aThing, 'B'), equals(MatchingLinkResult(B))); expect(bothLookup(aMethod, 'C'), equals(MatchingLinkResult(C))); @@ -2294,7 +2305,8 @@ void main() { }); test('on top level methods', () { - expect(bothLookup(aTopLevelTypeParameterFunction, 'D'), equals(MatchingLinkResult(D))); + expect(bothLookup(aTopLevelTypeParameterFunction, 'D'), + equals(MatchingLinkResult(D))); expect(bothLookup(typedParam, 'D'), equals(MatchingLinkResult(D))); }); }); @@ -2514,10 +2526,10 @@ void main() { }); test('in method scope referring to parameters and variables', () { - /*expect(bothLookup(aMethod, 'yetAnotherName'), + expect(bothLookup(aMethod, 'yetAnotherName'), equals(MatchingLinkResult(yetAnotherName))); expect(bothLookup(aMethod, 'FactoryConstructorThings.yetAnotherName'), - equals(MatchingLinkResult(yetAnotherNameField)));*/ + equals(MatchingLinkResult(yetAnotherNameField))); expect( bothLookup( aMethod, 'FactoryConstructorThings.anotherName.anotherName'), From bd0bdaf1ef549bafa7dd03a5e4b7d553498e8bd4 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Sat, 26 Jun 2021 22:38:57 -0700 Subject: [PATCH 05/17] remove debugging --- lib/src/generator/generator_frontend.dart | 1 - testing/test_package/lib/fake.dart | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/src/generator/generator_frontend.dart b/lib/src/generator/generator_frontend.dart index bc7875145e..08d9f3caff 100644 --- a/lib/src/generator/generator_frontend.dart +++ b/lib/src/generator/generator_frontend.dart @@ -47,7 +47,6 @@ class GeneratorFrontEnd implements Generator { } for (var lib in filterNonDocumented(package.libraries)) { - if (lib.name != 'dart:core') continue; logInfo('Generating docs for library ${lib.name} from ' '${lib.element.source.uri}...'); if (!lib.isAnonymous && !lib.hasDocumentation) { diff --git a/testing/test_package/lib/fake.dart b/testing/test_package/lib/fake.dart index 4e3c56437d..48c67b3b66 100644 --- a/testing/test_package/lib/fake.dart +++ b/testing/test_package/lib/fake.dart @@ -1288,7 +1288,7 @@ D aTopLevelTypeParameterFunction(D typedParam) {} abstract class TypeParameterThings { A aName; - final List aThing; + List aThing; B aMethod(A aParam, C anotherParam); } \ No newline at end of file From 2379242b6222a056d3142c6b0e7373794741c0ee Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Sun, 27 Jun 2021 08:17:14 -0700 Subject: [PATCH 06/17] correct isEquivalentTo to handle element type forwarding --- lib/src/matching_link_result.dart | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/src/matching_link_result.dart b/lib/src/matching_link_result.dart index c4f849a78f..5cd1130916 100644 --- a/lib/src/matching_link_result.dart +++ b/lib/src/matching_link_result.dart @@ -2,6 +2,7 @@ // 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. +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/quiver.dart'; @@ -26,6 +27,14 @@ class MatchingLinkResult { var compareThis = commentReferable; var compareOther = other.commentReferable; + if (compareThis is DefinedElementType) { + compareThis = (compareThis as DefinedElementType).modelElement; + } + + if (compareOther is DefinedElementType) { + compareOther = (compareOther as DefinedElementType).modelElement; + } + if (compareThis is Accessor) { compareThis = (compareThis as Accessor).enclosingCombo; } From 2b1614b0ff388adc73c9e498845f511684497fa5 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Sun, 27 Jun 2021 08:38:11 -0700 Subject: [PATCH 07/17] The big switch --- lib/src/dartdoc_options.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/dartdoc_options.dart b/lib/src/dartdoc_options.dart index 2582663693..324d183d8f 100644 --- a/lib/src/dartdoc_options.dart +++ b/lib/src/dartdoc_options.dart @@ -1426,7 +1426,7 @@ Future>> createDartdocOptions( DartdocOptionArgOnly>('excludePackages', [], resourceProvider, help: 'Package names to ignore.', splitCommas: true), DartdocOptionArgFile( - 'enhancedReferenceLookup', false, resourceProvider, + 'enhancedReferenceLookup', true, resourceProvider, hide: true, help: 'Use the new comment reference lookup system instead of the legacy ' From 54cfaa64a0d05740539958e8294f2ab85e00cdc0 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Sun, 27 Jun 2021 11:07:09 -0700 Subject: [PATCH 08/17] This was never correct, delete hrefs for type parameters --- lib/src/model/type_parameter.dart | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/lib/src/model/type_parameter.dart b/lib/src/model/type_parameter.dart index b361bc5536..23b8d686be 100644 --- a/lib/src/model/type_parameter.dart +++ b/lib/src/model/type_parameter.dart @@ -23,14 +23,9 @@ class TypeParameter extends ModelElement { '${enclosingElement.library.dirName}/${enclosingElement.name}/$name'; @override - String get href { - if (!identical(canonicalModelElement, this)) { - return canonicalModelElement?.href; - } - assert(canonicalLibrary != null); - assert(canonicalLibrary == library); - return '${package.baseHref}$filePath'; - } + + /// [TypeParameter]s don't have documentation pages. + String get href => null; @override String get kind => 'type parameter'; From ef677e313364a62f8f003c51b1a211df6cae97db Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Sun, 27 Jun 2021 12:59:43 -0700 Subject: [PATCH 09/17] Correct a problem with inherited docs --- lib/src/model/container_member.dart | 4 +- lib/src/model/method.dart | 9 ++- test/end2end/model_test.dart | 87 ++++++++++++++++++++++------- testing/test_package/lib/fake.dart | 25 +++++++-- 4 files changed, 93 insertions(+), 32 deletions(-) diff --git a/lib/src/model/container_member.dart b/lib/src/model/container_member.dart index f497cadc9e..2e52e93ec8 100644 --- a/lib/src/model/container_member.dart +++ b/lib/src/model/container_member.dart @@ -60,7 +60,9 @@ mixin ContainerMember on ModelElement implements EnclosedElement { @override @nonVirtual - Iterable get referenceParents => [enclosingElement]; + // TODO(jcollins-g): dart-lang/dartdoc#2693. + Iterable get referenceParents => + [documentationFrom.first.enclosingElement]; @override Iterable get referenceGrandparentOverrides sync* { diff --git a/lib/src/model/method.dart b/lib/src/model/method.dart index 6b82539f55..05c6f12c3e 100644 --- a/lib/src/model/method.dart +++ b/lib/src/model/method.dart @@ -134,11 +134,10 @@ class Method extends ModelElement if (_referenceChildren == null) { _referenceChildren = {}; _referenceChildren.addEntriesIfAbsent([ - ...from.typeParameters.explicitOnCollisionWith(this), - ...from.allParameters.explicitOnCollisionWith(this), - ...from.modelType.typeArguments.explicitOnCollisionWith(this), - ...from.modelType.returnType.typeArguments - .explicitOnCollisionWith(this), + ...typeParameters.explicitOnCollisionWith(this), + ...allParameters.explicitOnCollisionWith(this), + ...modelType.typeArguments.explicitOnCollisionWith(this), + ...modelType.returnType.typeArguments.explicitOnCollisionWith(this), ]); } return _referenceChildren; diff --git a/test/end2end/model_test.dart b/test/end2end/model_test.dart index 0df364050e..acdf6ac315 100644 --- a/test/end2end/model_test.dart +++ b/test/end2end/model_test.dart @@ -2254,10 +2254,12 @@ void main() { }); group('Type parameter lookups work', () { - Class TypeParameterThings; + Class TypeParameterThings, + TypeParameterThingsExtended, + TypeParameterThingsExtendedQ; Field aName, aThing; - TypeParameter A, B, C, D; - Method aMethod; + TypeParameter ATypeParam, BTypeParam, CTypeParam, DTypeParam, QTypeParam; + Method aMethod, aMethodExtended, aMethodExtendedQ; Parameter aParam, anotherParam, typedParam; ModelFunction aTopLevelTypeParameterFunction; @@ -2266,8 +2268,8 @@ void main() { .firstWhere((f) => f.name == 'aTopLevelTypeParameterFunction'); // TODO(jcollins-g): dart-lang/dartdoc#2704, HTML and type parameters // on the extended type should not be present here. - D = aTopLevelTypeParameterFunction.typeParameters.firstWhere( - (t) => t.name.startsWith('D extends TypeParameterThings')); + DTypeParam = aTopLevelTypeParameterFunction.typeParameters.firstWhere( + (t) => t.name.startsWith('DTypeParam extends TypeParameterThings')); typedParam = aTopLevelTypeParameterFunction.parameters .firstWhere((t) => t.name == 'typedParam'); @@ -2280,34 +2282,77 @@ void main() { aMethod = TypeParameterThings.instanceMethods .firstWhere((m) => m.name == 'aMethod'); - C = aMethod.typeParameters.firstWhere((t) => t.name == 'C'); + CTypeParam = + aMethod.typeParameters.firstWhere((t) => t.name == 'CTypeParam'); aParam = aMethod.parameters.firstWhere((p) => p.name == 'aParam'); anotherParam = aMethod.parameters.firstWhere((p) => p.name == 'anotherParam'); - A = TypeParameterThings.typeParameters.firstWhere((t) => t.name == 'A'); - B = TypeParameterThings.typeParameters - .firstWhere((t) => t.name == 'B extends FactoryConstructorThings'); + ATypeParam = TypeParameterThings.typeParameters + .firstWhere((t) => t.name == 'ATypeParam'); + BTypeParam = TypeParameterThings.typeParameters.firstWhere( + (t) => t.name == 'BTypeParam extends FactoryConstructorThings'); + + TypeParameterThingsExtended = fakeLibrary.allClasses + .firstWhere((c) => c.name == 'TypeParameterThingsExtended'); + aMethodExtended = TypeParameterThingsExtended.instanceMethods + .firstWhere((m) => m.name == 'aMethod'); + + TypeParameterThingsExtendedQ = fakeLibrary.allClasses + .firstWhere((c) => c.name == 'TypeParameterThingsExtendedQ'); + aMethodExtendedQ = TypeParameterThingsExtendedQ.instanceMethods + .firstWhere((m) => m.name == 'aMethod'); + QTypeParam = aMethodExtendedQ.typeParameters + .firstWhere((p) => p.name == 'QTypeParam'); + }); + + test('on inherited documentation', () { + expect(newLookup(aMethodExtended, 'ATypeParam'), + equals(MatchingLinkResult(ATypeParam))); + expect(newLookup(aMethodExtended, 'BTypeParam'), + equals(MatchingLinkResult(BTypeParam))); + expect(newLookup(aMethodExtended, 'CTypeParam'), + equals(MatchingLinkResult(CTypeParam))); + // Disallowed, because Q does not exist where the docs originated from. + // The old code forgave this most of the time. + expect(newLookup(aMethodExtended, 'QTypeParam'), + equals(MatchingLinkResult(null))); + + // We get an inverse situation on the extendedQ class. + expect(newLookup(aMethodExtendedQ, 'ATypeParam'), + equals(MatchingLinkResult(null))); + expect(newLookup(aMethodExtendedQ, 'BTypeParam'), + equals(MatchingLinkResult(null))); + expect(newLookup(aMethodExtendedQ, 'CTypeParam'), + equals(MatchingLinkResult(null))); + expect(newLookup(aMethodExtendedQ, 'QTypeParam'), + equals(MatchingLinkResult(QTypeParam))); }); test('on classes', () { - expect(bothLookup(TypeParameterThings, 'A'), - equals(MatchingLinkResult(A))); - expect(bothLookup(TypeParameterThings, 'B'), - equals(MatchingLinkResult(B))); - expect(bothLookup(aName, 'A'), equals(MatchingLinkResult(A))); - expect(bothLookup(aThing, 'B'), equals(MatchingLinkResult(B))); - expect(bothLookup(aMethod, 'C'), equals(MatchingLinkResult(C))); - expect(bothLookup(aParam, 'A'), equals(MatchingLinkResult(A))); + expect(bothLookup(TypeParameterThings, 'ATypeParam'), + equals(MatchingLinkResult(ATypeParam))); + expect(bothLookup(TypeParameterThings, 'BTypeParam'), + equals(MatchingLinkResult(BTypeParam))); + expect(bothLookup(aName, 'ATypeParam'), + equals(MatchingLinkResult(ATypeParam))); + expect(bothLookup(aThing, 'BTypeParam'), + equals(MatchingLinkResult(BTypeParam))); + expect(bothLookup(aMethod, 'CTypeParam'), + equals(MatchingLinkResult(CTypeParam))); + expect(bothLookup(aParam, 'ATypeParam'), + equals(MatchingLinkResult(ATypeParam))); // Original didn't handle this case properly and popped out to higher // levels. - expect(newLookup(anotherParam, 'C'), equals(MatchingLinkResult(C))); + expect(newLookup(anotherParam, 'CTypeParam'), + equals(MatchingLinkResult(CTypeParam))); }); test('on top level methods', () { - expect(bothLookup(aTopLevelTypeParameterFunction, 'D'), - equals(MatchingLinkResult(D))); - expect(bothLookup(typedParam, 'D'), equals(MatchingLinkResult(D))); + expect(bothLookup(aTopLevelTypeParameterFunction, 'DTypeParam'), + equals(MatchingLinkResult(DTypeParam))); + expect(bothLookup(typedParam, 'DTypeParam'), + equals(MatchingLinkResult(DTypeParam))); }); }); diff --git a/testing/test_package/lib/fake.dart b/testing/test_package/lib/fake.dart index 48c67b3b66..32565a8ec0 100644 --- a/testing/test_package/lib/fake.dart +++ b/testing/test_package/lib/fake.dart @@ -1284,11 +1284,26 @@ class FactoryConstructorThings { } -D aTopLevelTypeParameterFunction(D typedParam) {} +DTypeParam aTopLevelTypeParameterFunction(DTypeParam typedParam) {} -abstract class TypeParameterThings { - A aName; - List aThing; +abstract class TypeParameterThings { + ATypeParam aName; + List aThing; - B aMethod(A aParam, C anotherParam); + /// Here are some docs to inherit referencing [ATypeParam], [BTypeParam], and [CTypeParam]. + BTypeParam aMethod(ATypeParam aParam, CTypeParam anotherParam); +} + +/// Test that inheriting documentation can still reference parent type +/// parameters. +class TypeParameterThingsExtended extends TypeParameterThings { + @override + FactoryConstructorThings aMethod(String aParam, QTypeParam anotherParam) => null; +} + +/// Test that overriding documentation uses local type parameters. +class TypeParameterThingsExtendedQ extends TypeParameterThings { + @override + /// I override documentation so I can reference [QTypeParam]. + FactoryConstructorThings aMethod(String aParam, QTypeParam anotherParam) => null; } \ No newline at end of file From 170f2c162fe72995388dd2f2719e9dcbebea04b2 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Sun, 27 Jun 2021 13:37:37 -0700 Subject: [PATCH 10/17] Remember, we need both --- lib/src/model/container_member.dart | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/src/model/container_member.dart b/lib/src/model/container_member.dart index 2e52e93ec8..1898bc0ba0 100644 --- a/lib/src/model/container_member.dart +++ b/lib/src/model/container_member.dart @@ -62,7 +62,11 @@ mixin ContainerMember on ModelElement implements EnclosedElement { @nonVirtual // TODO(jcollins-g): dart-lang/dartdoc#2693. Iterable get referenceParents => - [documentationFrom.first.enclosingElement]; + // If you don't want the ambiguity of where your comment + // references are resolved wrt documentation inheritance, + // that has to be resolved in the source by not inheriting + // documentation. + [enclosingElement, documentationFrom.first.enclosingElement]; @override Iterable get referenceGrandparentOverrides sync* { From 8b5731ba890bcee4241bfeeaa276e640bc762af6 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Sun, 27 Jun 2021 18:01:47 -0700 Subject: [PATCH 11/17] Correct remaining SDK issues --- lib/dartdoc.dart | 2 +- lib/src/model/comment_referable.dart | 13 +++---------- lib/src/model/getter_setter_combo.dart | 7 +------ lib/src/model/parameter.dart | 16 +++++++-------- test/end2end/model_test.dart | 27 +++++++++++++++++++++++++- testing/test_package/lib/fake.dart | 6 +++++- 6 files changed, 43 insertions(+), 28 deletions(-) diff --git a/lib/dartdoc.dart b/lib/dartdoc.dart index f8c121dfa5..cf1a1a1aad 100644 --- a/lib/dartdoc.dart +++ b/lib/dartdoc.dart @@ -377,7 +377,7 @@ class Dartdoc { var links = doc.querySelectorAll('a'); var stringLinks = links .map((link) => link.attributes['href']) - .where((href) => href != null) + .where((href) => href != null && href != '') .toList(); return Tuple2(stringLinks, baseHref); diff --git a/lib/src/model/comment_referable.dart b/lib/src/model/comment_referable.dart index df55624d0c..d7696cfa0e 100644 --- a/lib/src/model/comment_referable.dart +++ b/lib/src/model/comment_referable.dart @@ -37,16 +37,9 @@ extension on Scope { /// [CommentReferable.referenceChildren] out of collections of other /// [CommmentReferable]s. extension CommentReferableEntryGenerators on Iterable { - /// Discards all entries that do not collide with [referable], - /// and generates an [MapEntry] using [referable]'s - /// [CommentReferable.referenceName] as a prefix for the item that does. - Iterable> onlyExplicitOnCollisionWith( - CommentReferable referable) => - where((r) => r.referenceName == referable.referenceName).map( - (r) => MapEntry('${referable.referenceName}.${r.referenceName}', r)); - - /// Like [onlyExplicitOnCollisionWith], except does not discard non-conflicting - /// items. + /// Creates ordinary references except if there is a conflict with + /// [referable], it will generate a [MapEntry] using [referable]'s + /// [CommentReferable.referenceName] as a prefix for the conflicting item. Iterable> explicitOnCollisionWith( CommentReferable referable) => map((r) { diff --git a/lib/src/model/getter_setter_combo.dart b/lib/src/model/getter_setter_combo.dart index 06f3a7c796..134cd6c045 100644 --- a/lib/src/model/getter_setter_combo.dart +++ b/lib/src/model/getter_setter_combo.dart @@ -244,12 +244,7 @@ mixin GetterSetterCombo on ModelElement { if (_referenceChildren == null) { _referenceChildren = {}; if (hasParameters) { - // Parameters in combos rarely matter for anything and are not part - // of the usual interface to a combo, so only reference them as part of - // [Container] fallbacks or if someone wants to explicitly specify a - // colliding parameter name. - _referenceChildren - .addEntries(parameters.onlyExplicitOnCollisionWith(this)); + _referenceChildren.addEntries(parameters.explicitOnCollisionWith(this)); } _referenceChildren .addEntries(modelType.typeArguments.explicitOnCollisionWith(this)); diff --git a/lib/src/model/parameter.dart b/lib/src/model/parameter.dart index a017f64d35..0d39fa89a4 100644 --- a/lib/src/model/parameter.dart +++ b/lib/src/model/parameter.dart @@ -13,7 +13,6 @@ class Parameter extends ModelElement implements EnclosedElement { ParameterElement element, Library library, PackageGraph packageGraph, {ParameterMember originalMember}) : super(element, library, packageGraph, originalMember); - String get defaultValue { if (!hasDefaultValue) return null; return element.defaultValueCode; @@ -85,17 +84,16 @@ class Parameter extends ModelElement implements EnclosedElement { Map get referenceChildren { if (_referenceChildren == null) { _referenceChildren = {}; - if (isCallable) { - _referenceChildren - .addEntriesIfAbsent(parameters.explicitOnCollisionWith(this)); + var _modelType = modelType; + if (_modelType is Callable) { + _referenceChildren.addEntriesIfAbsent( + _modelType.parameters.explicitOnCollisionWith(this)); } _referenceChildren.addEntriesIfAbsent( modelType.typeArguments.explicitOnCollisionWith(this)); - if (modelType is Callable) { - _referenceChildren.addEntriesIfAbsent((modelType as Callable) - .returnType - .typeArguments - .explicitOnCollisionWith(this)); + if (_modelType is Callable) { + _referenceChildren.addEntriesIfAbsent( + _modelType.returnType.typeArguments.explicitOnCollisionWith(this)); } } return _referenceChildren; diff --git a/test/end2end/model_test.dart b/test/end2end/model_test.dart index acdf6ac315..66552dc856 100644 --- a/test/end2end/model_test.dart +++ b/test/end2end/model_test.dart @@ -2363,7 +2363,8 @@ void main() { function1, topLevelFunction, aFunctionUsingRenamedLib; - TopLevelVariable incorrectDocReference, + TopLevelVariable aSetterWithFunctionParameter, + incorrectDocReference, incorrectDocReferenceFromEx, nameWithTwoUnderscores, nameWithSingleUnderscore, @@ -2392,6 +2393,7 @@ void main() { redHerring, yetAnotherName, somethingShadowyParameter; + Parameter fParam, fParamA, fParamB, fParamC; Field forInheriting, action, initializeMe, @@ -2515,9 +2517,32 @@ void main() { .firstWhere((m) => m.name == 'aMethod'); yetAnotherName = aMethod.allParameters.firstWhere((p) => p.name == 'yetAnotherName'); + + aSetterWithFunctionParameter = fakeLibrary.properties + .firstWhere((p) => p.name == 'aSetterWithFunctionParameter'); + fParam = aSetterWithFunctionParameter.parameters + .firstWhere((p) => p.name == 'fParam'); + fParamA = (fParam.modelType as Callable) + .parameters + .firstWhere((p) => p.name == 'fParamA'); + fParamB = (fParam.modelType as Callable) + .parameters + .firstWhere((p) => p.name == 'fParamB'); + fParamC = (fParam.modelType as Callable) + .parameters + .firstWhere((p) => p.name == 'fParamC'); }); group('Parameter references work properly', () { + test('via a setter with a function parameter', () { + expect(bothLookup(aSetterWithFunctionParameter, 'fParam.fParamA'), + equals(MatchingLinkResult(fParamA))); + expect(bothLookup(aSetterWithFunctionParameter, 'fParam.fParamB'), + equals(MatchingLinkResult(fParamB))); + expect(bothLookup(aSetterWithFunctionParameter, 'fParam.fParamC'), + equals(MatchingLinkResult(fParamC))); + }); + test('in class scope overridden by fields', () { expect(bothLookup(FactoryConstructorThings, 'aName'), equals(MatchingLinkResult(aNameField))); diff --git a/testing/test_package/lib/fake.dart b/testing/test_package/lib/fake.dart index 32565a8ec0..79c536d0dc 100644 --- a/testing/test_package/lib/fake.dart +++ b/testing/test_package/lib/fake.dart @@ -1306,4 +1306,8 @@ class TypeParameterThingsExtendedQ extends TypeParameterThings(String aParam, QTypeParam anotherParam) => null; -} \ No newline at end of file +} + +/// We should still be able to reference [fParam.fParamA], [fParam.fParamB], +/// and [fParam.fParamC]. +void set aSetterWithFunctionParameter(bool fParam(int fParamA, String fParamB, String fParamC)) {} \ No newline at end of file From 6e94febcd0fa9d5eaa534816016c22a9208cbd2e Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Mon, 28 Jun 2021 09:00:24 -0700 Subject: [PATCH 12/17] add a fully qualified name for DefinedElementType so that the comparison messages are more useful --- lib/src/element_type.dart | 3 +++ .../templates.runtime_renderers.dart | 20 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/lib/src/element_type.dart b/lib/src/element_type.dart index 279c15b00d..77e96ff31f 100644 --- a/lib/src/element_type.dart +++ b/lib/src/element_type.dart @@ -297,6 +297,9 @@ abstract class DefinedElementType extends ElementType { @override String get name => type.element.name; + @override + String get fullyQualifiedName => modelElement.fullyQualifiedName; + bool get isParameterType => (type is TypeParameterType); /// This type is a public type if the underlying, canonical element is public. diff --git a/lib/src/generator/templates.runtime_renderers.dart b/lib/src/generator/templates.runtime_renderers.dart index 58223f0d99..a71c2dbacc 100644 --- a/lib/src/generator/templates.runtime_renderers.dart +++ b/lib/src/generator/templates.runtime_renderers.dart @@ -4157,6 +4157,26 @@ class _Renderer_DefinedElementType extends RendererBase { parent: r, getters: _invisibleGetters['Element']); }, ), + 'fullyQualifiedName': Property( + getValue: (CT_ c) => c.fullyQualifiedName, + renderVariable: + (CT_ c, Property self, List remainingNames) { + if (remainingNames.isEmpty) { + return self.getValue(c).toString(); + } + var name = remainingNames.first; + var nextProperty = + _Renderer_String.propertyMap().getValue(name); + return nextProperty.renderVariable(self.getValue(c), + nextProperty, [...remainingNames.skip(1)]); + }, + isNullValue: (CT_ c) => c.fullyQualifiedName == null, + renderValue: (CT_ c, RendererBase r, + List ast, StringSink sink) { + _render_String(c.fullyQualifiedName, ast, r.template, sink, + parent: r); + }, + ), 'instantiatedType': Property( getValue: (CT_ c) => c.instantiatedType, renderVariable: (CT_ c, Property self, From 8c6c507f4eaa141f0c18e04ca76583e989b85832 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Mon, 28 Jun 2021 10:08:51 -0700 Subject: [PATCH 13/17] Remove commented out code block --- lib/src/model/container.dart | 7 ------- 1 file changed, 7 deletions(-) diff --git a/lib/src/model/container.dart b/lib/src/model/container.dart index 3f43227ec8..75becea76b 100644 --- a/lib/src/model/container.dart +++ b/lib/src/model/container.dart @@ -270,13 +270,6 @@ abstract class Container extends ModelElement with TypeParameters { .whereNotType() .generateEntries()); - /*for (var modelElement in allModelElements) { - // Never directly look up accessors. - if (modelElement is Accessor) continue; - // Constructors are special; see [Class.referenceChildrenExtra]. - if (modelElement is Constructor) continue; - _referenceChildren[modelElement.referenceName] = modelElement; - }*/ _referenceChildren.addEntriesIfAbsent(extraReferenceChildren); // Process unscoped parameters last to make sure they don't override // other options. From 2baaec30ebac89d0f10d8438e1b7103aad9ce792 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Mon, 28 Jun 2021 16:05:52 -0700 Subject: [PATCH 14/17] Stabilize the global table insertion order in the new lookup code. --- lib/src/model/package.dart | 2 +- lib/src/model/package_graph.dart | 16 +++++++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/src/model/package.dart b/lib/src/model/package.dart index e1f4113964..327207a6b8 100644 --- a/lib/src/model/package.dart +++ b/lib/src/model/package.dart @@ -406,7 +406,7 @@ class Package extends LibraryContainer Map get referenceChildren { if (_referenceChildren == null) { _referenceChildren = {}; - _referenceChildren.addEntries(allLibraries.generateEntries()); + _referenceChildren.addEntries(publicLibrariesSorted.generateEntries()); // Do not override any preexisting data, and insert based on the // public library sort order. // TODO(jcollins-g): warn when results require package-global diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart index 62b501dbdf..0d27d65776 100644 --- a/lib/src/model/package_graph.dart +++ b/lib/src/model/package_graph.dart @@ -1030,19 +1030,25 @@ class PackageGraph with CommentReferable, Nameable { Map get referenceChildren { if (_referenceChildren == null) { _referenceChildren = {}; + // We have to use a stable order or otherwise references depending + // on ambiguous resolution (see below) will change where they + // resolve based on internal implementation details. + var sortedPackages = documentedPackages.toList()..sort(byName); // Packages are the top priority. - _referenceChildren.addEntries(packages.generateEntries()); + _referenceChildren.addEntries(sortedPackages.generateEntries()); // Libraries are next. // TODO(jcollins-g): Warn about directly referencing libraries out of - // scope? - _referenceChildren.addEntriesIfAbsent(documentedPackages + // scope? Doing this is always going to be ambiguous and potentially + // confusing. + _referenceChildren.addEntriesIfAbsent(sortedPackages .expand((p) => p.publicLibrariesSorted) .generateEntries()); // TODO(jcollins-g): Warn about directly referencing top level items - // out of scope? - _referenceChildren.addEntriesIfAbsent(documentedPackages + // out of scope? Doing this will be even more ambiguous and + // potentially confusing than doing so with libraries. + _referenceChildren.addEntriesIfAbsent(sortedPackages .expand((p) => p.publicLibrariesSorted) .expand((l) => [ ...l.publicConstants, From c69329464593ccdeea5b06b85896299e7fe50c11 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Mon, 28 Jun 2021 16:20:24 -0700 Subject: [PATCH 15/17] Keep using all packages -- might be necessary in some cases and I've done all my testing that way so far. --- lib/src/model/package_graph.dart | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart index 0d27d65776..d088ec260f 100644 --- a/lib/src/model/package_graph.dart +++ b/lib/src/model/package_graph.dart @@ -1033,7 +1033,8 @@ class PackageGraph with CommentReferable, Nameable { // We have to use a stable order or otherwise references depending // on ambiguous resolution (see below) will change where they // resolve based on internal implementation details. - var sortedPackages = documentedPackages.toList()..sort(byName); + var sortedPackages = packages.toList()..sort(byName); + var sortedDocumentedPackages = documentedPackages.toList()..sort(byName); // Packages are the top priority. _referenceChildren.addEntries(sortedPackages.generateEntries()); @@ -1041,14 +1042,14 @@ class PackageGraph with CommentReferable, Nameable { // TODO(jcollins-g): Warn about directly referencing libraries out of // scope? Doing this is always going to be ambiguous and potentially // confusing. - _referenceChildren.addEntriesIfAbsent(sortedPackages + _referenceChildren.addEntriesIfAbsent(sortedDocumentedPackages .expand((p) => p.publicLibrariesSorted) .generateEntries()); // TODO(jcollins-g): Warn about directly referencing top level items // out of scope? Doing this will be even more ambiguous and // potentially confusing than doing so with libraries. - _referenceChildren.addEntriesIfAbsent(sortedPackages + _referenceChildren.addEntriesIfAbsent(sortedDocumentedPackages .expand((p) => p.publicLibrariesSorted) .expand((l) => [ ...l.publicConstants, From b72aaf713826dcba189d131ae091421dfc52861f Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Mon, 28 Jun 2021 16:53:59 -0700 Subject: [PATCH 16/17] Make byName more stable via hashCode. --- lib/src/model/class.dart | 7 +++--- lib/src/model/container.dart | 18 +++++++------- lib/src/model/extension_target.dart | 2 +- lib/src/model/library_container.dart | 2 +- lib/src/model/nameable.dart | 9 +++++-- lib/src/model/package.dart | 2 +- lib/src/model/package_graph.dart | 5 ++-- lib/src/model/top_level_container.dart | 18 +++++++------- test/end2end/model_test.dart | 33 ++++++++++++++++++++++---- 9 files changed, 64 insertions(+), 32 deletions(-) diff --git a/lib/src/model/class.dart b/lib/src/model/class.dart index db3f826765..84d12f2bb6 100644 --- a/lib/src/model/class.dart +++ b/lib/src/model/class.dart @@ -126,7 +126,8 @@ class Class extends Container @override Iterable get publicConstructorsSorted => - _publicConstructorsSorted ??= publicConstructors.toList()..sort(byName); + _publicConstructorsSorted ??= publicConstructors.toList() + ..sort(byNameStable); /// Returns the library that encloses this element. @override @@ -207,8 +208,8 @@ class Class extends Container List _publicImplementorsSorted; - Iterable get publicImplementorsSorted => - _publicImplementorsSorted ??= publicImplementors.toList()..sort(byName); + Iterable get publicImplementorsSorted => _publicImplementorsSorted ??= + publicImplementors.toList()..sort(byNameStable); /*lazy final*/ List _inheritedMethods; diff --git a/lib/src/model/container.dart b/lib/src/model/container.dart index 75becea76b..681d7c34b9 100644 --- a/lib/src/model/container.dart +++ b/lib/src/model/container.dart @@ -97,7 +97,7 @@ abstract class Container extends ModelElement with TypeParameters { List _publicInstanceMethodsSorted; List get publicInstanceMethodsSorted => _publicInstanceMethodsSorted ?? publicInstanceMethods.toList() - ..sort(byName); + ..sort(byNameStable); Iterable _declaredOperators; @nonVirtual @@ -123,7 +123,7 @@ abstract class Container extends ModelElement with TypeParameters { List _publicInstanceOperatorsSorted; List get publicInstanceOperatorsSorted => _publicInstanceOperatorsSorted ??= publicInstanceOperators.toList() - ..sort(byName); + ..sort(byNameStable); /// Fields fully declared in this [Container]. Iterable get declaredFields; @@ -143,7 +143,7 @@ abstract class Container extends ModelElement with TypeParameters { List _publicInstanceFieldsSorted; List get publicInstanceFieldsSorted => _publicInstanceFieldsSorted ??= - publicInstanceFields.toList()..sort(byName); + publicInstanceFields.toList()..sort(byNameStable); Iterable get constantFields => declaredFields.where((f) => f.isConst); @@ -154,7 +154,7 @@ abstract class Container extends ModelElement with TypeParameters { List _publicConstantFieldsSorted; List get publicConstantFieldsSorted => _publicConstantFieldsSorted ??= - publicConstantFields.toList()..sort(byName); + publicConstantFields.toList()..sort(byNameStable); Iterable get instanceAccessors => instanceFields.expand((f) => f.allAccessors); @@ -223,8 +223,8 @@ abstract class Container extends ModelElement with TypeParameters { model_utils.filterNonPublic(staticFields); List _publicStaticFieldsSorted; - List get publicStaticFieldsSorted => - _publicStaticFieldsSorted ??= publicStaticFields.toList()..sort(byName); + List get publicStaticFieldsSorted => _publicStaticFieldsSorted ??= + publicStaticFields.toList()..sort(byNameStable); Iterable get staticFields => declaredFields.where((f) => f.isStatic); @@ -240,7 +240,7 @@ abstract class Container extends ModelElement with TypeParameters { List _publicVariableStaticFieldsSorted; List get publicVariableStaticFieldsSorted => _publicVariableStaticFieldsSorted ??= publicVariableStaticFields.toList() - ..sort(byName); + ..sort(byNameStable); Iterable get staticMethods => declaredMethods.where((m) => m.isStatic); @@ -252,8 +252,8 @@ abstract class Container extends ModelElement with TypeParameters { model_utils.filterNonPublic(staticMethods); List _publicStaticMethodsSorted; - List get publicStaticMethodsSorted => - _publicStaticMethodsSorted ??= publicStaticMethods.toList()..sort(byName); + List get publicStaticMethodsSorted => _publicStaticMethodsSorted ??= + publicStaticMethods.toList()..sort(byNameStable); /// For subclasses to add items after the main pass but before the /// parameter-global. diff --git a/lib/src/model/extension_target.dart b/lib/src/model/extension_target.dart index bea6e7c65f..368a7a9a67 100644 --- a/lib/src/model/extension_target.dart +++ b/lib/src/model/extension_target.dart @@ -31,5 +31,5 @@ mixin ExtensionTarget on ModelElement { ElementType get modelType; List get potentiallyApplicableExtensionsSorted => - potentiallyApplicableExtensions.toList()..sort(byName); + potentiallyApplicableExtensions.toList()..sort(byNameStable); } diff --git a/lib/src/model/library_container.dart b/lib/src/model/library_container.dart index 2a10757c4c..54642529aa 100644 --- a/lib/src/model/library_container.dart +++ b/lib/src/model/library_container.dart @@ -21,7 +21,7 @@ abstract class LibraryContainer List _publicLibrariesSorted; Iterable get publicLibrariesSorted => - _publicLibrariesSorted ??= publicLibraries.toList()..sort(byName); + _publicLibrariesSorted ??= publicLibraries.toList()..sort(byNameStable); bool get hasPublicLibraries => publicLibraries.isNotEmpty; diff --git a/lib/src/model/nameable.dart b/lib/src/model/nameable.dart index 3e8818caae..e6018302fd 100644 --- a/lib/src/model/nameable.dart +++ b/lib/src/model/nameable.dart @@ -35,5 +35,10 @@ abstract class Nameable { String toString() => name; } -int byName(Nameable a, Nameable b) => - compareAsciiLowerCaseNatural(a.name, b.name); +int byNameStable(Nameable a, Nameable b) { + var stringCompare = compareAsciiLowerCaseNatural(a.name, b.name); + if (stringCompare == 0) { + return a.hashCode.compareTo(b.hashCode); + } + return stringCompare; +} diff --git a/lib/src/model/package.dart b/lib/src/model/package.dart index 327207a6b8..8967733403 100644 --- a/lib/src/model/package.dart +++ b/lib/src/model/package.dart @@ -357,7 +357,7 @@ class Package extends LibraryContainer if (config.categoryOrder.isEmpty) { return documentedCategories; } - return documentedCategories.toList()..sort(byName); + return documentedCategories.toList()..sort(byNameStable); } bool get hasDocumentedCategories => documentedCategories.isNotEmpty; diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart index d088ec260f..5f3529d791 100644 --- a/lib/src/model/package_graph.dart +++ b/lib/src/model/package_graph.dart @@ -1033,8 +1033,9 @@ class PackageGraph with CommentReferable, Nameable { // We have to use a stable order or otherwise references depending // on ambiguous resolution (see below) will change where they // resolve based on internal implementation details. - var sortedPackages = packages.toList()..sort(byName); - var sortedDocumentedPackages = documentedPackages.toList()..sort(byName); + var sortedPackages = packages.toList()..sort(byNameStable); + var sortedDocumentedPackages = documentedPackages.toList() + ..sort(byNameStable); // Packages are the top priority. _referenceChildren.addEntries(sortedPackages.generateEntries()); diff --git a/lib/src/model/top_level_container.dart b/lib/src/model/top_level_container.dart index 0f29d608cb..da77369e8c 100644 --- a/lib/src/model/top_level_container.dart +++ b/lib/src/model/top_level_container.dart @@ -52,57 +52,57 @@ abstract class TopLevelContainer implements Nameable { List _publicClassesSorted; Iterable get publicClassesSorted => - _publicClassesSorted ??= publicClasses.toList()..sort(byName); + _publicClassesSorted ??= publicClasses.toList()..sort(byNameStable); Iterable get publicExtensions => model_utils.filterNonPublic(extensions); List _publicExtensionsSorted; Iterable get publicExtensionsSorted => - _publicExtensionsSorted ??= publicExtensions.toList()..sort(byName); + _publicExtensionsSorted ??= publicExtensions.toList()..sort(byNameStable); Iterable get publicConstants => model_utils.filterNonPublic(constants); Iterable get publicConstantsSorted => - publicConstants.toList()..sort(byName); + publicConstants.toList()..sort(byNameStable); Iterable get publicEnums => model_utils.filterNonPublic(enums); List _publicEnumsSorted; Iterable get publicEnumsSorted => - _publicEnumsSorted ??= publicEnums.toList()..sort(byName); + _publicEnumsSorted ??= publicEnums.toList()..sort(byNameStable); Iterable get publicExceptions => model_utils.filterNonPublic(exceptions); List _publicExceptionsSorted; Iterable get publicExceptionsSorted => - _publicExceptionsSorted ??= publicExceptions.toList()..sort(byName); + _publicExceptionsSorted ??= publicExceptions.toList()..sort(byNameStable); Iterable get publicFunctions => model_utils.filterNonPublic(functions); List _publicFunctionsSorted; Iterable get publicFunctionsSorted => - _publicFunctionsSorted ??= publicFunctions.toList()..sort(byName); + _publicFunctionsSorted ??= publicFunctions.toList()..sort(byNameStable); Iterable get publicMixins => model_utils.filterNonPublic(mixins); List _publicMixinsSorted; Iterable get publicMixinsSorted => - _publicMixinsSorted ??= publicMixins.toList()..sort(byName); + _publicMixinsSorted ??= publicMixins.toList()..sort(byNameStable); Iterable get publicProperties => model_utils.filterNonPublic(properties); List _publicPropertiesSorted; Iterable get publicPropertiesSorted => - _publicPropertiesSorted ??= publicProperties.toList()..sort(byName); + _publicPropertiesSorted ??= publicProperties.toList()..sort(byNameStable); Iterable get publicTypedefs => model_utils.filterNonPublic(typedefs); List _publicTypedefsSorted; Iterable get publicTypedefsSorted => - _publicTypedefsSorted ??= publicTypedefs.toList()..sort(byName); + _publicTypedefsSorted ??= publicTypedefs.toList()..sort(byNameStable); } diff --git a/test/end2end/model_test.dart b/test/end2end/model_test.dart index 66552dc856..45631e09ef 100644 --- a/test/end2end/model_test.dart +++ b/test/end2end/model_test.dart @@ -4964,12 +4964,22 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans, var a = StringName(names[i - 1]); var b = StringName(names[i]); test('"$a" < "$b"', () { - expect(byName(a, a), 0); - expect(byName(b, b), 0); - expect(byName(a, b), -1); - expect(byName(b, a), 1); + expect(byNameStable(a, a), 0); + expect(byNameStable(b, b), 0); + expect(byNameStable(a, b), -1); + expect(byNameStable(b, a), 1); }); } + + test('sort order is stable when necessary', () { + var a = StringNameHashCode('a', 12); + var b = StringNameHashCode('b', 12); + var aa = StringNameHashCode('a', 14); + expect(byNameStable(a, aa), -1); + expect(byNameStable(a, b), -1); + expect(byNameStable(b, a), 1); + expect(byNameStable(aa, b), -1); + }); }); } @@ -4982,3 +4992,18 @@ class StringName extends Nameable { @override String toString() => name; } + +class StringNameHashCode extends Nameable { + @override + final String name; + @override + final int hashCode; + + StringNameHashCode(this.name, this.hashCode); + + @override + String toString() => name; + + @override + bool operator ==(Object other) => super == other; +} From a8cc5d73692801a9869f253c1ba1b624a663ae3f Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Mon, 28 Jun 2021 17:00:57 -0700 Subject: [PATCH 17/17] No need to rename it --- lib/src/model/class.dart | 7 +++---- lib/src/model/container.dart | 18 +++++++++--------- lib/src/model/extension_target.dart | 2 +- lib/src/model/library_container.dart | 2 +- lib/src/model/nameable.dart | 2 +- lib/src/model/package.dart | 2 +- lib/src/model/package_graph.dart | 5 ++--- lib/src/model/top_level_container.dart | 18 +++++++++--------- test/end2end/model_test.dart | 16 ++++++++-------- 9 files changed, 35 insertions(+), 37 deletions(-) diff --git a/lib/src/model/class.dart b/lib/src/model/class.dart index 84d12f2bb6..db3f826765 100644 --- a/lib/src/model/class.dart +++ b/lib/src/model/class.dart @@ -126,8 +126,7 @@ class Class extends Container @override Iterable get publicConstructorsSorted => - _publicConstructorsSorted ??= publicConstructors.toList() - ..sort(byNameStable); + _publicConstructorsSorted ??= publicConstructors.toList()..sort(byName); /// Returns the library that encloses this element. @override @@ -208,8 +207,8 @@ class Class extends Container List _publicImplementorsSorted; - Iterable get publicImplementorsSorted => _publicImplementorsSorted ??= - publicImplementors.toList()..sort(byNameStable); + Iterable get publicImplementorsSorted => + _publicImplementorsSorted ??= publicImplementors.toList()..sort(byName); /*lazy final*/ List _inheritedMethods; diff --git a/lib/src/model/container.dart b/lib/src/model/container.dart index 681d7c34b9..75becea76b 100644 --- a/lib/src/model/container.dart +++ b/lib/src/model/container.dart @@ -97,7 +97,7 @@ abstract class Container extends ModelElement with TypeParameters { List _publicInstanceMethodsSorted; List get publicInstanceMethodsSorted => _publicInstanceMethodsSorted ?? publicInstanceMethods.toList() - ..sort(byNameStable); + ..sort(byName); Iterable _declaredOperators; @nonVirtual @@ -123,7 +123,7 @@ abstract class Container extends ModelElement with TypeParameters { List _publicInstanceOperatorsSorted; List get publicInstanceOperatorsSorted => _publicInstanceOperatorsSorted ??= publicInstanceOperators.toList() - ..sort(byNameStable); + ..sort(byName); /// Fields fully declared in this [Container]. Iterable get declaredFields; @@ -143,7 +143,7 @@ abstract class Container extends ModelElement with TypeParameters { List _publicInstanceFieldsSorted; List get publicInstanceFieldsSorted => _publicInstanceFieldsSorted ??= - publicInstanceFields.toList()..sort(byNameStable); + publicInstanceFields.toList()..sort(byName); Iterable get constantFields => declaredFields.where((f) => f.isConst); @@ -154,7 +154,7 @@ abstract class Container extends ModelElement with TypeParameters { List _publicConstantFieldsSorted; List get publicConstantFieldsSorted => _publicConstantFieldsSorted ??= - publicConstantFields.toList()..sort(byNameStable); + publicConstantFields.toList()..sort(byName); Iterable get instanceAccessors => instanceFields.expand((f) => f.allAccessors); @@ -223,8 +223,8 @@ abstract class Container extends ModelElement with TypeParameters { model_utils.filterNonPublic(staticFields); List _publicStaticFieldsSorted; - List get publicStaticFieldsSorted => _publicStaticFieldsSorted ??= - publicStaticFields.toList()..sort(byNameStable); + List get publicStaticFieldsSorted => + _publicStaticFieldsSorted ??= publicStaticFields.toList()..sort(byName); Iterable get staticFields => declaredFields.where((f) => f.isStatic); @@ -240,7 +240,7 @@ abstract class Container extends ModelElement with TypeParameters { List _publicVariableStaticFieldsSorted; List get publicVariableStaticFieldsSorted => _publicVariableStaticFieldsSorted ??= publicVariableStaticFields.toList() - ..sort(byNameStable); + ..sort(byName); Iterable get staticMethods => declaredMethods.where((m) => m.isStatic); @@ -252,8 +252,8 @@ abstract class Container extends ModelElement with TypeParameters { model_utils.filterNonPublic(staticMethods); List _publicStaticMethodsSorted; - List get publicStaticMethodsSorted => _publicStaticMethodsSorted ??= - publicStaticMethods.toList()..sort(byNameStable); + List get publicStaticMethodsSorted => + _publicStaticMethodsSorted ??= publicStaticMethods.toList()..sort(byName); /// For subclasses to add items after the main pass but before the /// parameter-global. diff --git a/lib/src/model/extension_target.dart b/lib/src/model/extension_target.dart index 368a7a9a67..bea6e7c65f 100644 --- a/lib/src/model/extension_target.dart +++ b/lib/src/model/extension_target.dart @@ -31,5 +31,5 @@ mixin ExtensionTarget on ModelElement { ElementType get modelType; List get potentiallyApplicableExtensionsSorted => - potentiallyApplicableExtensions.toList()..sort(byNameStable); + potentiallyApplicableExtensions.toList()..sort(byName); } diff --git a/lib/src/model/library_container.dart b/lib/src/model/library_container.dart index 54642529aa..2a10757c4c 100644 --- a/lib/src/model/library_container.dart +++ b/lib/src/model/library_container.dart @@ -21,7 +21,7 @@ abstract class LibraryContainer List _publicLibrariesSorted; Iterable get publicLibrariesSorted => - _publicLibrariesSorted ??= publicLibraries.toList()..sort(byNameStable); + _publicLibrariesSorted ??= publicLibraries.toList()..sort(byName); bool get hasPublicLibraries => publicLibraries.isNotEmpty; diff --git a/lib/src/model/nameable.dart b/lib/src/model/nameable.dart index e6018302fd..761ea0150c 100644 --- a/lib/src/model/nameable.dart +++ b/lib/src/model/nameable.dart @@ -35,7 +35,7 @@ abstract class Nameable { String toString() => name; } -int byNameStable(Nameable a, Nameable b) { +int byName(Nameable a, Nameable b) { var stringCompare = compareAsciiLowerCaseNatural(a.name, b.name); if (stringCompare == 0) { return a.hashCode.compareTo(b.hashCode); diff --git a/lib/src/model/package.dart b/lib/src/model/package.dart index 8967733403..327207a6b8 100644 --- a/lib/src/model/package.dart +++ b/lib/src/model/package.dart @@ -357,7 +357,7 @@ class Package extends LibraryContainer if (config.categoryOrder.isEmpty) { return documentedCategories; } - return documentedCategories.toList()..sort(byNameStable); + return documentedCategories.toList()..sort(byName); } bool get hasDocumentedCategories => documentedCategories.isNotEmpty; diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart index 5f3529d791..d088ec260f 100644 --- a/lib/src/model/package_graph.dart +++ b/lib/src/model/package_graph.dart @@ -1033,9 +1033,8 @@ class PackageGraph with CommentReferable, Nameable { // We have to use a stable order or otherwise references depending // on ambiguous resolution (see below) will change where they // resolve based on internal implementation details. - var sortedPackages = packages.toList()..sort(byNameStable); - var sortedDocumentedPackages = documentedPackages.toList() - ..sort(byNameStable); + var sortedPackages = packages.toList()..sort(byName); + var sortedDocumentedPackages = documentedPackages.toList()..sort(byName); // Packages are the top priority. _referenceChildren.addEntries(sortedPackages.generateEntries()); diff --git a/lib/src/model/top_level_container.dart b/lib/src/model/top_level_container.dart index da77369e8c..0f29d608cb 100644 --- a/lib/src/model/top_level_container.dart +++ b/lib/src/model/top_level_container.dart @@ -52,57 +52,57 @@ abstract class TopLevelContainer implements Nameable { List _publicClassesSorted; Iterable get publicClassesSorted => - _publicClassesSorted ??= publicClasses.toList()..sort(byNameStable); + _publicClassesSorted ??= publicClasses.toList()..sort(byName); Iterable get publicExtensions => model_utils.filterNonPublic(extensions); List _publicExtensionsSorted; Iterable get publicExtensionsSorted => - _publicExtensionsSorted ??= publicExtensions.toList()..sort(byNameStable); + _publicExtensionsSorted ??= publicExtensions.toList()..sort(byName); Iterable get publicConstants => model_utils.filterNonPublic(constants); Iterable get publicConstantsSorted => - publicConstants.toList()..sort(byNameStable); + publicConstants.toList()..sort(byName); Iterable get publicEnums => model_utils.filterNonPublic(enums); List _publicEnumsSorted; Iterable get publicEnumsSorted => - _publicEnumsSorted ??= publicEnums.toList()..sort(byNameStable); + _publicEnumsSorted ??= publicEnums.toList()..sort(byName); Iterable get publicExceptions => model_utils.filterNonPublic(exceptions); List _publicExceptionsSorted; Iterable get publicExceptionsSorted => - _publicExceptionsSorted ??= publicExceptions.toList()..sort(byNameStable); + _publicExceptionsSorted ??= publicExceptions.toList()..sort(byName); Iterable get publicFunctions => model_utils.filterNonPublic(functions); List _publicFunctionsSorted; Iterable get publicFunctionsSorted => - _publicFunctionsSorted ??= publicFunctions.toList()..sort(byNameStable); + _publicFunctionsSorted ??= publicFunctions.toList()..sort(byName); Iterable get publicMixins => model_utils.filterNonPublic(mixins); List _publicMixinsSorted; Iterable get publicMixinsSorted => - _publicMixinsSorted ??= publicMixins.toList()..sort(byNameStable); + _publicMixinsSorted ??= publicMixins.toList()..sort(byName); Iterable get publicProperties => model_utils.filterNonPublic(properties); List _publicPropertiesSorted; Iterable get publicPropertiesSorted => - _publicPropertiesSorted ??= publicProperties.toList()..sort(byNameStable); + _publicPropertiesSorted ??= publicProperties.toList()..sort(byName); Iterable get publicTypedefs => model_utils.filterNonPublic(typedefs); List _publicTypedefsSorted; Iterable get publicTypedefsSorted => - _publicTypedefsSorted ??= publicTypedefs.toList()..sort(byNameStable); + _publicTypedefsSorted ??= publicTypedefs.toList()..sort(byName); } diff --git a/test/end2end/model_test.dart b/test/end2end/model_test.dart index 45631e09ef..f1134d9e7b 100644 --- a/test/end2end/model_test.dart +++ b/test/end2end/model_test.dart @@ -4964,10 +4964,10 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans, var a = StringName(names[i - 1]); var b = StringName(names[i]); test('"$a" < "$b"', () { - expect(byNameStable(a, a), 0); - expect(byNameStable(b, b), 0); - expect(byNameStable(a, b), -1); - expect(byNameStable(b, a), 1); + expect(byName(a, a), 0); + expect(byName(b, b), 0); + expect(byName(a, b), -1); + expect(byName(b, a), 1); }); } @@ -4975,10 +4975,10 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans, var a = StringNameHashCode('a', 12); var b = StringNameHashCode('b', 12); var aa = StringNameHashCode('a', 14); - expect(byNameStable(a, aa), -1); - expect(byNameStable(a, b), -1); - expect(byNameStable(b, a), 1); - expect(byNameStable(aa, b), -1); + expect(byName(a, aa), -1); + expect(byName(a, b), -1); + expect(byName(b, a), 1); + expect(byName(aa, b), -1); }); }); }