Repository: dart-lang/dartdoc (dart)

Issue:
canonicalization is sometimes failing to return any results for links to extension methods
I have an extension `CoreChecks` which is in scope, that extension defines a method `equals`. I hoped that a doc comment `/// [CoreChecks.equals]` would have a link to the docs for `equals`, but it renders as a code block instead of a link.

The analyzer jump to definition does work so this is a case where analyzer and dart doc disagree about comment references.

Pull request description:
Fixes #3325.

Also introduces a new test helper for reflective tests that should make it easier to test canonicalization edge cases.  There were some shortcuts in the code here that I do not think actually needed to be there given the current implementation -- they may have been parts of older extension method code that missed a cleanup pass.

Patch:
diff --git a/lib/src/model/container_member.dart b/lib/src/model/container_member.dart
index e91c1f7a8d..49d4b03b27 100644
--- a/lib/src/model/container_member.dart
+++ b/lib/src/model/container_member.dart
@@ -43,11 +43,8 @@ mixin ContainerMember on ModelElement implements EnclosedElement {
           as Container?;
     }
     // TODO(jcollins-g): move Extension specific code to [Extendable]
-    if (enclosingElement.isDocumented) {
-      return packageGraph.findCanonicalModelElementFor(enclosingElement.element)
-          as Container?;
-    }
-    return null;
+    return packageGraph.findCanonicalModelElementFor(enclosingElement.element)
+        as Container?;
   }
 
   @override
diff --git a/lib/src/model/inheritable.dart b/lib/src/model/inheritable.dart
index ac454cc355..be1998c97d 100644
--- a/lib/src/model/inheritable.dart
+++ b/lib/src/model/inheritable.dart
@@ -48,10 +48,6 @@ mixin Inheritable on ContainerMember {
     if (canonicalEnclosingContainer == null) {
       return null;
     }
-    // TODO(jcollins-g): factor out extension logic into [Extendable]
-    if (canonicalEnclosingContainer is Extension) {
-      return this;
-    }
     return canonicalEnclosingContainer.allCanonicalModelElements
         .firstWhereOrNull((m) =>
             m.name == name &&


Test patch:
diff --git a/test/dartdoc_test_base.dart b/test/dartdoc_test_base.dart
index ad4dca0909..d4a492fe06 100644
--- a/test/dartdoc_test_base.dart
+++ b/test/dartdoc_test_base.dart
@@ -11,6 +11,16 @@ import 'package:meta/meta.dart';
 import 'src/test_descriptor_utils.dart' as d;
 import 'src/utils.dart';
 
+/// Exception thrown for invalid use of [DartdocTestBase]'s api.
+class DartdocTestBaseFailure implements Exception {
+  final String message;
+
+  DartdocTestBaseFailure(this.message);
+
+  @override
+  String toString() => message;
+}
+
 abstract class DartdocTestBase {
   late final PackageMetaProvider packageMetaProvider;
   late final MemoryResourceProvider resourceProvider;
@@ -65,6 +75,64 @@ analyzer:
       d.file('lib.dart', '''
 library $libraryName;
 
+$libraryContent
+'''),
+    ]).createInMemory(resourceProvider, packagePath);
+
+    var packageGraph = await bootBasicPackage(
+      packagePath,
+      packageMetaProvider,
+      packageConfigProvider,
+    );
+    return packageGraph.libraries.named(libraryName);
+  }
+
+  /// Similar to [bootPackageWithLibrary], but allows for more complex
+  /// cases to test the edges of canonicalization.
+  ///
+  /// - Puts [reexportedContent] in a library named [libraryName]_src in
+  ///   `lib/src` (if [reexportPrivate] is true), or 'lib/subdir'.
+  /// - Creates a reexporting library named [libraryName]_lib in `lib` that
+  ///   reexports [libraryName]_src.
+  /// - Creates [libraryName] containing [libraryContent] that can optionally
+  ///   import 'lib.dart' to import the reexporting library.
+  ///
+  /// Optionally, specify [show] or [hide] to change whether the reexport
+  /// gives access to the full namespace.
+  Future<Library> bootPackageWithReexportedLibrary(
+      String reexportedContent, String libraryContent,
+      {bool reexportPrivate = false,
+      List<String> show = const [],
+      List<String> hide = const []}) async {
+    final subdir = reexportPrivate ? 'src' : 'subdir';
+    await d.dir('lib', [
+      d.dir(subdir, [
+        d.file('lib.dart', '''
+library ${libraryName}_src;
+
+$reexportedContent
+'''),
+      ])
+    ]).createInMemory(resourceProvider, packagePath);
+
+    if (show.isNotEmpty && hide.isNotEmpty) {
+      throw DartdocTestBaseFailure('Can not specify show and hide');
+    }
+
+    final showHideString = '${show.isNotEmpty ? 'show ${show.join(', ')}' : ''}'
+        '${hide.isNotEmpty ? 'hide ${hide.join(', ')}' : ''}';
+    await d.dir('lib', [
+      d.file('lib.dart', '''
+library ${libraryName}_lib;
+
+export '$subdir/lib.dart' $showHideString;
+'''),
+    ]).createInMemory(resourceProvider, packagePath);
+
+    await d.dir('lib', [
+      d.file('importing_lib.dart', '''
+library $libraryName;
+
 $libraryContent
 '''),
     ]).createInMemory(resourceProvider, packagePath);
diff --git a/test/extension_methods_test.dart b/test/extension_methods_test.dart
new file mode 100644
index 0000000000..dc290d1a35
--- /dev/null
+++ b/test/extension_methods_test.dart
@@ -0,0 +1,115 @@
+// Copyright (c) 2023, 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/markdown_processor.dart';
+import 'package:dartdoc/src/model/extension.dart';
+import 'package:dartdoc/src/model/method.dart';
+import 'package:dartdoc/src/model/model_element.dart';
+import 'package:test/test.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import 'dartdoc_test_base.dart';
+import 'src/utils.dart';
+
+void main() {
+  defineReflectiveSuite(() {
+    if (classModifiersAllowed) {
+      defineReflectiveTests(ExtensionMethodsTest);
+    }
+  });
+}
+
+@reflectiveTest
+class ExtensionMethodsTest extends DartdocTestBase {
+  @override
+  String get libraryName => 'extension_methods';
+
+  static const String reexportedContent = '''
+class AClassNotNeedingExtending {}
+
+class AClassNeedingExtending {}
+
+extension AnExtension on AClassNeedingExtending {
+  void aMethod() {}
+}
+''';
+
+  static const String libraryContent = '''
+/// This is an amazing public function.
+var aPublicFunction() {}
+''';
+
+  void expectReferenceValid(
+      ModelElement reference, ModelElement expected, String href) {
+    expect(identical(reference.canonicalModelElement, expected), isTrue);
+    expect(expected.isCanonical, isTrue);
+    expect(expected.href, endsWith(href));
+  }
+
+  void test_reexportWithShow() async {
+    var library = await bootPackageWithReexportedLibrary(
+        reexportedContent, libraryContent,
+        reexportPrivate: true, show: ['AClassNeedingExtending', 'AnExtension']);
+    var aPublicFunction = library.functions.named('aPublicFunction');
+    var anExtension = library.package.publicLibraries
+        .named('${libraryName}_lib')
+        .extensions
+        .named('AnExtension');
+    var anExtensionMethod = anExtension.instanceMethods.named('aMethod');
+    var anExtensionReference =
+        getMatchingLinkElement('AnExtension', aPublicFunction).commentReferable
+            as Extension;
+    expectReferenceValid(anExtensionReference, anExtension,
+        '%extension_methods_lib/AnExtension.html');
+    var anExtensionMethodReference =
+        getMatchingLinkElement('AnExtension.aMethod', aPublicFunction)
+            .commentReferable as Method;
+    expectReferenceValid(anExtensionMethodReference, anExtensionMethod,
+        '%extension_methods_lib/AnExtension/aMethod.html');
+  }
+
+  void test_reexportWithHide() async {
+    var library = await bootPackageWithReexportedLibrary(
+        reexportedContent, libraryContent,
+        reexportPrivate: true, hide: ['AClassNotNeedingExtending']);
+    var aPublicFunction = library.functions.named('aPublicFunction');
+    var anExtension = library.package.publicLibraries
+        .named('${libraryName}_lib')
+        .extensions
+        .named('AnExtension');
+    var anExtensionMethod = anExtension.instanceMethods.named('aMethod');
+    var anExtensionReference =
+        getMatchingLinkElement('AnExtension', aPublicFunction).commentReferable
+            as Extension;
+    expectReferenceValid(anExtensionReference, anExtension,
+        '%extension_methods_lib/AnExtension.html');
+    var anExtensionMethodReference =
+        getMatchingLinkElement('AnExtension.aMethod', aPublicFunction)
+            .commentReferable as Method;
+    expectReferenceValid(anExtensionMethodReference, anExtensionMethod,
+        '%extension_methods_lib/AnExtension/aMethod.html');
+  }
+
+  void test_reexportFull() async {
+    var library = await bootPackageWithReexportedLibrary(
+        reexportedContent, libraryContent,
+        reexportPrivate: true);
+    var aPublicFunction = library.functions.named('aPublicFunction');
+    var anExtension = library.package.publicLibraries
+        .named('${libraryName}_lib')
+        .extensions
+        .named('AnExtension');
+    var anExtensionMethod = anExtension.instanceMethods.named('aMethod');
+    var anExtensionReference =
+        getMatchingLinkElement('AnExtension', aPublicFunction).commentReferable
+            as Extension;
+    expectReferenceValid(anExtensionReference, anExtension,
+        '%extension_methods_lib/AnExtension.html');
+    var anExtensionMethodReference =
+        getMatchingLinkElement('AnExtension.aMethod', aPublicFunction)
+            .commentReferable as Method;
+    expectReferenceValid(anExtensionMethodReference, anExtensionMethod,
+        '%extension_methods_lib/AnExtension/aMethod.html');
+  }
+}