Correct links to remote, canonical libraries (#3912)

srawlins · web-flow · commit 2e64c2bf13b6 · 2024-10-22T15:30:35.000-07:00
Fixes #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.
diff --git 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<Library> libraries) {
+  Library canonicalLibraryCandidate(Iterable<Library> 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
@@ -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
@@ -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
@@ -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
@@ -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,
+        '<a href="$dartSdkUrlPrefix/dart-io/FileSystemEntity-class.html">FileSystemEntity</a>');
+  }
+
   void test_publicLibrary_unnamed() async {
     var library =
         (await bootPackageFromFiles([d.file('lib/lib1.dart', 'library;')]))
