diff --git a/lib/src/markdown_processor.dart b/lib/src/markdown_processor.dart index 12a66dec2c..ca288d9fd3 100644 --- a/lib/src/markdown_processor.dart +++ b/lib/src/markdown_processor.dart @@ -459,6 +459,10 @@ class _MarkdownCommentReference { // conflicts are allowed, but an instance field is allowed to have the // same name as a named constructor. _reducePreferConstructorViaIndicators, + // Prefer Fields/TopLevelVariables to accessors. + // TODO(jcollins-g): Remove after fixing dart-lang/dartdoc#2396 or + // exclude Accessors from all lookup tables. + _reducePreferCombos, // Prefer the Dart analyzer's resolution of comment references. We // can't start from this because of the differences in Dartdoc // canonicalization. @@ -547,6 +551,14 @@ class _MarkdownCommentReference { } } + void _reducePreferCombos() { + var accessors = results.whereType().toList(); + accessors.forEach((a) { + results.remove(a); + results.add(a.enclosingCombo); + }); + } + void _findTypeParameters() { if (element is TypeParameters) { results.addAll((element as TypeParameters).typeParameters.where((p) => @@ -648,7 +660,7 @@ class _MarkdownCommentReference { prefixToLibrary[codeRefChompedParts.first]?.forEach((l) => l .modelElementsNameMap[lookup] ?.map(_convertConstructors) - ?.forEach((m) => _addCanonicalResult(m, _getPreferredClass(m)))); + ?.forEach((m) => _addCanonicalResult(m))); } } } @@ -660,8 +672,7 @@ class _MarkdownCommentReference { if (codeRefChomped == modelElement.fullyQualifiedNameWithoutLibrary || (modelElement is Library && codeRefChomped == modelElement.fullyQualifiedName)) { - _addCanonicalResult( - _convertConstructors(modelElement), preferredClass); + _addCanonicalResult(_convertConstructors(modelElement)); } } } @@ -671,7 +682,7 @@ class _MarkdownCommentReference { // Only look for partially qualified matches if we didn't find a fully qualified one. if (library.modelElementsNameMap.containsKey(codeRefChomped)) { for (final modelElement in library.modelElementsNameMap[codeRefChomped]) { - _addCanonicalResult(_convertConstructors(modelElement), preferredClass); + _addCanonicalResult(_convertConstructors(modelElement)); } } } @@ -684,15 +695,7 @@ class _MarkdownCommentReference { packageGraph.findRefElementCache.containsKey(codeRefChomped)) { for (var modelElement in packageGraph.findRefElementCache[codeRefChomped]) { - // For fully qualified matches, the original preferredClass passed - // might make no sense. Instead, use the enclosing class from the - // element in [packageGraph.findRefElementCache], because that element's - // enclosing class will be preferred from [codeRefChomped]'s perspective. - _addCanonicalResult( - _convertConstructors(modelElement), - modelElement.enclosingElement is Class - ? modelElement.enclosingElement - : null); + _addCanonicalResult(_convertConstructors(modelElement)); } } } @@ -785,9 +788,8 @@ class _MarkdownCommentReference { } // Add a result, but make it canonical. - void _addCanonicalResult(ModelElement modelElement, Container tryClass) { - results.add(packageGraph.findCanonicalModelElementFor(modelElement.element, - preferredClass: tryClass)); + void _addCanonicalResult(ModelElement modelElement) { + results.add(modelElement.canonicalModelElement); } /// _getResultsForClass assumes codeRefChomped might be a member of tryClass (inherited or not) @@ -804,7 +806,7 @@ class _MarkdownCommentReference { } else { // People like to use 'this' in docrefs too. if (codeRef == 'this') { - _addCanonicalResult(tryClass, null); + _addCanonicalResult(tryClass); } else { // TODO(jcollins-g): get rid of reimplementation of identifier resolution // or integrate into ModelElement in a simpler way. @@ -816,7 +818,7 @@ class _MarkdownCommentReference { // Fortunately superChains are short, but optimize this if it matters. superChain.addAll(tryClass.superChain.map((t) => t.element)); for (final c in superChain) { - _getResultsForSuperChainElement(c, tryClass); + _getResultsForSuperChainElement(c); if (results.isNotEmpty) break; } } @@ -825,12 +827,12 @@ class _MarkdownCommentReference { /// Get any possible results for this class in the superChain. Returns /// true if we found something. - void _getResultsForSuperChainElement(Class c, Class tryClass) { + void _getResultsForSuperChainElement(Class c) { var membersToCheck = (c.allModelElementsByNamePart[codeRefChomped] ?? []) .map(_convertConstructors); for (var modelElement in membersToCheck) { // [thing], a member of this class - _addCanonicalResult(modelElement, tryClass); + _addCanonicalResult(modelElement); } if (codeRefChompedParts.length < 2 || codeRefChompedParts[codeRefChompedParts.length - 2] == c.name) { @@ -838,7 +840,7 @@ class _MarkdownCommentReference { (c.allModelElementsByNamePart[codeRefChompedParts.last] ?? []) .map(_convertConstructors); - membersToCheck.forEach((m) => _addCanonicalResult(m, tryClass)); + membersToCheck.forEach((m) => _addCanonicalResult(m)); } results.remove(null); if (results.isNotEmpty) return; diff --git a/lib/src/model/class.dart b/lib/src/model/class.dart index f320658dfb..1d6849b2e2 100644 --- a/lib/src/model/class.dart +++ b/lib/src/model/class.dart @@ -401,8 +401,7 @@ class Class extends Container var accessorMap = >{}; for (var accessorElement in inheritedAccessorElements) { var name = accessorElement.name.replaceFirst('=', ''); - accessorMap.putIfAbsent(name, () => []); - accessorMap[name].add(accessorElement); + accessorMap.putIfAbsent(name, () => []).add(accessorElement); } // For half-inherited fields, the analyzer only links the non-inherited diff --git a/lib/src/model/library.dart b/lib/src/model/library.dart index 2b7a0c1640..2a7b9622e7 100644 --- a/lib/src/model/library.dart +++ b/lib/src/model/library.dart @@ -2,6 +2,8 @@ // 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 'dart:collection'; + import 'package:analyzer/dart/analysis/results.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/element/element.dart'; @@ -283,8 +285,8 @@ class Library extends ModelElement with Categorization, TopLevelContainer { for (var i in element.imports) { // Ignore invalid imports. if (i.prefix?.name != null && i.importedLibrary != null) { - _prefixToLibrary.putIfAbsent(i.prefix?.name, () => {}); - _prefixToLibrary[i.prefix?.name] + _prefixToLibrary + .putIfAbsent(i.prefix?.name, () => {}) .add(ModelElement.from(i.importedLibrary, library, packageGraph)); } } @@ -585,29 +587,29 @@ class Library extends ModelElement with Categorization, TopLevelContainer { static String getLibraryName(LibraryElement element) => _getLibraryName(element); - /*late final*/ Map> _modelElementsNameMap; + /*late final*/ HashMap> _modelElementsNameMap; /// Map of [fullyQualifiedNameWithoutLibrary] to all matching [ModelElement]s /// in this library. Used for code reference lookups. - Map> get modelElementsNameMap { + HashMap> get modelElementsNameMap { if (_modelElementsNameMap == null) { - _modelElementsNameMap = >{}; + _modelElementsNameMap = HashMap>(); allModelElements.forEach((ModelElement modelElement) { // [definingLibrary] may be null if [element] has been imported or // exported with a non-normalized URI, like "src//a.dart". if (modelElement.definingLibrary == null) return; - _modelElementsNameMap.putIfAbsent( - modelElement.fullyQualifiedNameWithoutLibrary, () => {}); - _modelElementsNameMap[modelElement.fullyQualifiedNameWithoutLibrary] + _modelElementsNameMap + .putIfAbsent( + modelElement.fullyQualifiedNameWithoutLibrary, () => {}) .add(modelElement); }); } return _modelElementsNameMap; } - /*late final*/ Map> _modelElementsMap; + /*late final*/ HashMap> _modelElementsMap; - Map> get modelElementsMap { + HashMap> get modelElementsMap { if (_modelElementsMap == null) { var results = quiver.concat(>[ library.constants, @@ -639,13 +641,13 @@ class Library extends ModelElement with Categorization, TopLevelContainer { ]); }), ]); - _modelElementsMap = >{}; + _modelElementsMap = HashMap>(); results.forEach((modelElement) { - _modelElementsMap.putIfAbsent(modelElement.element, () => {}); - _modelElementsMap[modelElement.element].add(modelElement); + _modelElementsMap + .putIfAbsent(modelElement.element, () => {}) + .add(modelElement); }); - _modelElementsMap.putIfAbsent(element, () => {}); - _modelElementsMap[element].add(this); + _modelElementsMap.putIfAbsent(element, () => {}).add(this); } return _modelElementsMap; } diff --git a/lib/src/model/model_element.dart b/lib/src/model/model_element.dart index 6d54836906..4164262a41 100644 --- a/lib/src/model/model_element.dart +++ b/lib/src/model/model_element.dart @@ -299,8 +299,9 @@ abstract class ModelElement extends Canonicalization library.packageGraph.allConstructedModelElements[key] = newModelElement; if (newModelElement is Inheritable) { var iKey = Tuple2(e, library); - library.packageGraph.allInheritableElements.putIfAbsent(iKey, () => {}); - library.packageGraph.allInheritableElements[iKey].add(newModelElement); + library.packageGraph.allInheritableElements + .putIfAbsent(iKey, () => {}) + .add(newModelElement); } } } @@ -557,10 +558,9 @@ abstract class ModelElement extends Canonicalization preferredClass: preferredClass); } + ModelElement _canonicalModelElement; // Returns the canonical ModelElement for this ModelElement, or null // if there isn't one. - ModelElement _canonicalModelElement; - ModelElement get canonicalModelElement => _canonicalModelElement ??= buildCanonicalModelElement(); diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart index cca0940719..db169efd77 100644 --- a/lib/src/model/package_graph.dart +++ b/lib/src/model/package_graph.dart @@ -184,13 +184,13 @@ class PackageGraph { return _extensions; } - Map> _findRefElementCache; - Map> get findRefElementCache { + HashMap> _findRefElementCache; + HashMap> get findRefElementCache { if (_findRefElementCache == null) { assert(packageGraph.allLibrariesAdded); - _findRefElementCache = {}; + _findRefElementCache = HashMap>(); for (final modelElement - in utils.filterNonDocumented(packageGraph.allLocalModelElements)) { + in utils.filterHasCanonical(packageGraph.allModelElements)) { _findRefElementCache.putIfAbsent( modelElement.fullyQualifiedNameWithoutLibrary, () => {}); _findRefElementCache.putIfAbsent( @@ -210,15 +210,15 @@ class PackageGraph { PackageWarningCounter _packageWarningCounter; /// All ModelElements constructed for this package; a superset of [allModelElements]. - final Map, ModelElement> - allConstructedModelElements = {}; + final allConstructedModelElements = + HashMap, ModelElement>(); /// Anything that might be inheritable, place here for later lookup. - final Map, Set> - allInheritableElements = {}; + final allInheritableElements = + HashMap, Set>(); /// A mapping of the list of classes which implement each class. - final Map> _implementors = LinkedHashMap( + final _implementors = LinkedHashMap>( equals: (Class a, Class b) => a.definingClass == b.definingClass, hashCode: (Class class_) => class_.definingClass.hashCode); @@ -544,8 +544,9 @@ class PackageGraph { referredFrom: [topLevelLibrary]); return; } - _libraryElementReexportedBy.putIfAbsent(libraryElement, () => {}); - _libraryElementReexportedBy[libraryElement].add(topLevelLibrary); + _libraryElementReexportedBy + .putIfAbsent(libraryElement, () => {}) + .add(topLevelLibrary); for (var exportedElement in libraryElement.exports) { _tagReexportsFor( topLevelLibrary, exportedElement.exportedLibrary, exportedElement); diff --git a/lib/src/model_utils.dart b/lib/src/model_utils.dart index 77a143247a..3890d27107 100644 --- a/lib/src/model_utils.dart +++ b/lib/src/model_utils.dart @@ -73,6 +73,11 @@ AstNode getAstNode( return null; } +Iterable filterHasCanonical( + Iterable maybeHasCanonicalItems) { + return maybeHasCanonicalItems.where((me) => me.canonicalModelElement != null); +} + /// Remove elements that aren't documented. Iterable filterNonDocumented( Iterable maybeDocumentedItems) { diff --git a/lib/src/package_config_provider.dart b/lib/src/package_config_provider.dart index 3221aa4ea8..845137e2f9 100644 --- a/lib/src/package_config_provider.dart +++ b/lib/src/package_config_provider.dart @@ -27,8 +27,9 @@ class FakePackageConfigProvider implements PackageConfigProvider { final _packageConfigData = >{}; void addPackageToConfigFor(String location, String name, Uri root) { - _packageConfigData.putIfAbsent(location, () => []); - _packageConfigData[location].add(package_config.Package(name, root)); + _packageConfigData + .putIfAbsent(location, () => []) + .add(package_config.Package(name, root)); } @override diff --git a/lib/src/warnings.dart b/lib/src/warnings.dart index b8a57561c0..de96ee860c 100644 --- a/lib/src/warnings.dart +++ b/lib/src/warnings.dart @@ -482,8 +482,7 @@ class PackageWarningCounter { errorCount += 1; } var warningData = Tuple2(kind, message); - countedWarnings.putIfAbsent(element?.element, () => {}); - countedWarnings[element?.element].add(warningData); + countedWarnings.putIfAbsent(element?.element, () => {}).add(warningData); _writeWarning(kind, warningMode, config.verboseWarnings, element?.fullyQualifiedName, fullMessage); } diff --git a/test/end2end/model_test.dart b/test/end2end/model_test.dart index 50bcf175a6..15f56129ba 100644 --- a/test/end2end/model_test.dart +++ b/test/end2end/model_test.dart @@ -15,6 +15,7 @@ import 'package:dartdoc/src/render/model_element_renderer.dart'; import 'package:dartdoc/src/render/parameter_renderer.dart'; import 'package:dartdoc/src/render/typedef_renderer.dart'; import 'package:dartdoc/src/special_elements.dart'; +import 'package:dartdoc/src/tuple.dart'; import 'package:dartdoc/src/warnings.dart'; import 'package:test/test.dart'; @@ -882,6 +883,24 @@ void main() { docsAsHtml = doAwesomeStuff.documentationAsHtml; }); + test('Verify links to inherited members inside class', () { + expect( + docsAsHtml, + contains( + 'ClassWithUnusualProperties.forInheriting')); + expect( + docsAsHtml, + contains( + 'ExtendedBaseReexported.action

')); + var doAwesomeStuffWarnings = packageGraph.packageWarningCounter + .countedWarnings[doAwesomeStuff.element] ?? + []; + expect( + doAwesomeStuffWarnings, + isNot(anyElement((Tuple2 e) => + e.item1 == PackageWarning.ambiguousDocReference))); + }); + test('can handle renamed imports', () { var aFunctionUsingRenamedLib = fakeLibrary.functions .firstWhere((f) => f.name == 'aFunctionUsingRenamedLib'); diff --git a/testing/test_package/lib/fake.dart b/testing/test_package/lib/fake.dart index 7aace41f5b..6cccca19b0 100644 --- a/testing/test_package/lib/fake.dart +++ b/testing/test_package/lib/fake.dart @@ -858,6 +858,7 @@ class ExtraSpecialList extends SpecialList {} /// {@subCategory Things and Such} /// {@image https://flutter.io/images/catalog-widget-placeholder.png} /// {@samples https://flutter.io} +/// class BaseForDocComments { /// Takes a [value] and returns a String. /// @@ -899,6 +900,10 @@ class BaseForDocComments { /// /// Reference to something that doesn't exist containing a type parameter [ThisIsNotHereNoWay] /// + /// Reference to an inherited member: [ClassWithUnusualProperties.forInheriting] + /// + /// Reference to an inherited member in another library via class name: [ExtendedBaseReexported.action] + /// /// Link to a nonexistent file (erroneously expects base href): [link](SubForDocComments/localMethod.html) /// /// Link to an existing file: [link](../SubForDocComments/localMethod.html) diff --git a/testing/test_package/lib/src/somelib.dart b/testing/test_package/lib/src/somelib.dart index 697ec1f2b6..921d4eef94 100644 --- a/testing/test_package/lib/src/somelib.dart +++ b/testing/test_package/lib/src/somelib.dart @@ -8,6 +8,13 @@ class YetAnotherClass {} class AUnicornClass {} +class BaseReexported { + String action; +} + +class ExtendedBaseReexported extends BaseReexported { +} + /// A private extension. extension _Unseen on Object { void doYouSeeMe() { } @@ -21,4 +28,4 @@ extension on List { /// [_Unseen] is not seen, but [DocumentMe] is. extension DocumentThisExtensionOnce on String { String get reportOnString => '$this is wonderful'; -} \ No newline at end of file +}