From 042ffd5a79f26719cd931479e84cb284a186a36c Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Tue, 22 Oct 2024 11:25:33 -0700 Subject: [PATCH] Correct links to remote, canonical libraries Fixes https://github.com/dart-lang/dartdoc/issues/3891 In this example, the **file** package exports elements from `dart:io`. So when `--link-to-remote` is used, the canonical library for those exported elements should be `api.dart.dev`. Also some cleaning: * Move `_searchForCanonicalLibrary` from ModelElement to `canonicalization.dart`, keeping more canonicalization logic together. * This allows us to make `Canonicalization` private. * Make some comments in ModelElement doc-comments. --- lib/src/model/canonicalization.dart | 63 +++++++++++++++++++++++++++-- lib/src/model/model_element.dart | 56 +++---------------------- lib/src/model/package_graph.dart | 11 ++--- test/dartdoc_test_base.dart | 4 +- test/libraries_test.dart | 10 +++++ 5 files changed, 85 insertions(+), 59 deletions(-) diff --git a/lib/src/model/canonicalization.dart b/lib/src/model/canonicalization.dart index 761f4a5b81..157c88352e 100644 --- a/lib/src/model/canonicalization.dart +++ b/lib/src/model/canonicalization.dart @@ -2,21 +2,78 @@ // 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:analyzer/dart/element/element.dart'; import 'package:dartdoc/src/model/model.dart'; import 'package:dartdoc/src/warnings.dart'; +/// Searches [PackageGraph.libraryExports] for a public, documented library +/// which exports this [ModelElement], ideally in its library's package. +Library? canonicalLibraryCandidate(ModelElement modelElement) { + var thisAndExported = + modelElement.packageGraph.libraryExports[modelElement.library.element]; + if (thisAndExported == null) { + return null; + } + + // Since we're looking for a library, find the [Element] immediately + // contained by a [CompilationUnitElement] in the tree. + var topLevelElement = modelElement.element; + while (topLevelElement.enclosingElement3 is! LibraryElement && + topLevelElement.enclosingElement3 is! CompilationUnitElement && + topLevelElement.enclosingElement3 != null) { + topLevelElement = topLevelElement.enclosingElement3!; + } + var topLevelElementName = topLevelElement.name; + if (topLevelElementName == null) { + // Any member of an unnamed extension is not public, and has no + // canonical library. + return null; + } + + final candidateLibraries = thisAndExported.where((l) { + if (!l.isPublic) return false; + if (l.package.documentedWhere == DocumentLocation.missing) return false; + if (modelElement is Library) return true; + var lookup = l.element.exportNamespace.definedNames[topLevelElementName]; + return topLevelElement == + (lookup is PropertyAccessorElement ? lookup.variable2 : lookup); + }).toList(growable: true); + + if (candidateLibraries.isEmpty) { + return null; + } + if (candidateLibraries.length == 1) { + return candidateLibraries.single; + } + + var remoteLibraries = candidateLibraries + .where((l) => l.package.documentedWhere == DocumentLocation.remote); + if (remoteLibraries.length == 1) { + // If one or more local libraries export code from a remotely documented + // library (and we're linking to remote libraries), then just use the remote + // library. + return remoteLibraries.single; + } + + var topLevelModelElement = + ModelElement.forElement(topLevelElement, modelElement.packageGraph); + + return _Canonicalization(topLevelModelElement) + .canonicalLibraryCandidate(candidateLibraries); +} + /// Canonicalization support in Dartdoc. /// /// This provides heuristic scoring to determine which library a human likely /// considers this element to be primarily 'from', and therefore, canonical. /// Still warn if the heuristic isn't very confident. -final class Canonicalization { +final class _Canonicalization { final ModelElement _element; - Canonicalization(this._element); + _Canonicalization(this._element); /// Calculates a candidate for the canonical library of [_element], among [libraries]. - Library calculateCanonicalCandidate(Iterable libraries) { + Library canonicalLibraryCandidate(Iterable libraries) { var locationPieces = _element.element.location .toString() .split(_locationSplitter) diff --git a/lib/src/model/model_element.dart b/lib/src/model/model_element.dart index c3d166876d..86da73271e 100644 --- a/lib/src/model/model_element.dart +++ b/lib/src/model/model_element.dart @@ -466,14 +466,14 @@ abstract class ModelElement .join(); } - // True if this is a function, or if it is an type alias to a function. + /// Whether this is a function, or if it is an type alias to a function. bool get isCallable => element is FunctionTypedElement || (element is TypeAliasElement && (element as TypeAliasElement).aliasedType is FunctionType); - // The canonical ModelElement for this ModelElement, - // or null if there isn't one. + /// The canonical ModelElement for this ModelElement, or null if there isn't + /// one. late final ModelElement? canonicalModelElement = () { final enclosingElement = this.enclosingElement; var preferredClass = switch (enclosingElement) { @@ -514,8 +514,9 @@ abstract class ModelElement var definingLibraryIsLocalPublic = packageGraph.localPublicLibraries.contains(library); - var possibleCanonicalLibrary = - definingLibraryIsLocalPublic ? library : _searchForCanonicalLibrary(); + var possibleCanonicalLibrary = definingLibraryIsLocalPublic + ? library + : canonicalLibraryCandidate(this); if (possibleCanonicalLibrary != null) return possibleCanonicalLibrary; @@ -532,51 +533,6 @@ abstract class ModelElement return null; }(); - /// Searches [PackageGraph.libraryExports] for a public, documented library - /// which exports this [ModelElement], ideally in [library]'s package. - Library? _searchForCanonicalLibrary() { - var thisAndExported = packageGraph.libraryExports[library.element]; - if (thisAndExported == null) { - return null; - } - - // Since we're looking for a library, find the [Element] immediately - // contained by a [CompilationUnitElement] in the tree. - var topLevelElement = element; - while (topLevelElement.enclosingElement3 is! LibraryElement && - topLevelElement.enclosingElement3 is! CompilationUnitElement && - topLevelElement.enclosingElement3 != null) { - topLevelElement = topLevelElement.enclosingElement3!; - } - var topLevelElementName = topLevelElement.name; - if (topLevelElementName == null) { - // Any member of an unnamed extension is not public, and has no - // canonical library. - return null; - } - - final candidateLibraries = thisAndExported.where((l) { - if (!l.isPublic) return false; - if (l.package.documentedWhere == DocumentLocation.missing) return false; - if (this is Library) return true; - var lookup = l.element.exportNamespace.definedNames[topLevelElementName]; - return topLevelElement == - (lookup is PropertyAccessorElement ? lookup.variable2 : lookup); - }).toList(growable: true); - - if (candidateLibraries.isEmpty) { - return null; - } - if (candidateLibraries.length == 1) { - return candidateLibraries.single; - } - - var topLevelModelElement = - ModelElement.forElement(topLevelElement, packageGraph); - return Canonicalization(topLevelModelElement) - .calculateCanonicalCandidate(candidateLibraries); - } - @override bool get isCanonical { if (!isPublic) return false; diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart index e80a9d0fb4..9d26f50b29 100644 --- a/lib/src/model/package_graph.dart +++ b/lib/src/model/package_graph.dart @@ -751,8 +751,9 @@ class PackageGraph with CommentReferable, Nameable { return buffer.toString(); } - /// Tries to find a canonical [ModelElement] for this [modelElement]. If we - /// know this element is related to a particular class, pass a + /// Tries to find a canonical [ModelElement] for [modelElement]. + /// + /// If we know the element is related to a particular class, pass a /// [preferredClass] to disambiguate. /// /// This doesn't know anything about [PackageGraph.inheritThrough] and @@ -795,10 +796,10 @@ class PackageGraph with CommentReferable, Nameable { ModelElement? canonicalModelElement; if (declaration != null && (element is ClassMemberElement || element is PropertyAccessorElement)) { - modelElement = getModelForElement(declaration); - element = modelElement.element; + var declarationModelElement = getModelForElement(declaration); + element = declarationModelElement.element; canonicalModelElement = _findCanonicalModelElementForAmbiguous( - modelElement, library, + declarationModelElement, library, preferredClass: preferredClass as InheritingContainer?); } else { if (library != null) { diff --git a/test/dartdoc_test_base.dart b/test/dartdoc_test_base.dart index 122db3bb49..f305778659 100644 --- a/test/dartdoc_test_base.dart +++ b/test/dartdoc_test_base.dart @@ -39,7 +39,9 @@ abstract class DartdocTestBase { String get dartAsyncUrlPrefix => 'https://api.dart.dev/stable/3.2.0/dart-async'; - String get dartCoreUrlPrefix => 'https://api.dart.dev/stable/3.2.0/dart-core'; + String get dartSdkUrlPrefix => 'https://api.dart.dev/stable/3.2.0'; + + String get dartCoreUrlPrefix => '$dartSdkUrlPrefix/dart-core'; String get sdkConstraint => '>=3.7.0 <4.0.0'; diff --git a/test/libraries_test.dart b/test/libraries_test.dart index fcef888d2d..8d110d579c 100644 --- a/test/libraries_test.dart +++ b/test/libraries_test.dart @@ -194,6 +194,16 @@ class LibrariesTest extends DartdocTestBase { expect(library.href, '${placeholder}libraries'); } + void test_library_containsClassWithSameNameAsDartSdk() async { + var library = await bootPackageWithLibrary( + 'export "dart:io";', + libraryFilePath: 'lib/library.dart', + ); + + expect(library.classes.named('FileSystemEntity').linkedName, + 'FileSystemEntity'); + } + void test_publicLibrary_unnamed() async { var library = (await bootPackageFromFiles([d.file('lib/lib1.dart', 'library;')]))