Scaladoc: resolve looked up inherited symbols to correct DRIs

Author: BarkingBad

scaladoc-testcases/src/tests/links.scala

@@ -9,8 +9,7 @@ object AnObject:
 /**
  * Broken link, that should result a warning not break compilation
  * [[tests.links.AnObject]]
-
  */
 class LinksTest:
   def verifyIfLinksTestIsGenerated(b: Int): Int
-    = 123
+    = 123

scaladoc-testcases/src/tests/lookupInheritedMembers.scala

@@ -0,0 +1,20 @@
+package tests
+package lookupInheritedMembers.pack1 {
+  class A:
+    def x = 1
+    val y = 1
+    type MyType
+
+}
+package lookupInheritedMembers.pack2 {
+  class B extends tests.lookupInheritedMembers.pack1.A
+}
+
+package lookupInheritedMembers {
+  /**
+   * [[tests.lookupInheritedMembers.pack2.B.x]]
+   * [[tests.lookupInheritedMembers.pack2.B.y]]
+   * [[tests.lookupInheritedMembers.pack2.B.MyType]]
+   */
+  class LookupInheritedMembers
+}

scaladoc/src/dotty/tools/scaladoc/tasty/SymOps.scala

@@ -190,3 +190,8 @@ class SymOps[Q <: Quotes](val q: Q) extends JavadocAnchorCreator with Scaladoc2A
           // For some reason it contains `$$$` instrad of symbol name
           s"${sym.name}${sym.fullName}/${sym.signature.resultSig}/[${sym.signature.paramSigs.mkString("/")}]"
         )
+
+    def driInContextOfInheritingParent(par: Symbol)(using dctx: DocContext): DRI = sym.dri.copy(
+      location = par.dri.location,
+      externalLink = None
+    )

scaladoc/src/dotty/tools/scaladoc/tasty/TastyParser.scala

@@ -80,8 +80,12 @@ case class ScaladocTastyInspector()(using ctx: DocContext) extends DocTastyInspe
     def driFor(link: String): Option[DRI] =
       val symOps = new SymOps[q.type](q)
       import symOps._
-      Try(QueryParser(link).readQuery()).toOption.flatMap(q =>
-        MemberLookup.lookupOpt(q, None).map{ case (sym, _) => sym.dri}
+      Try(QueryParser(link).readQuery()).toOption.flatMap(query =>
+        MemberLookup.lookupOpt(query, None).map {
+          case (sym, _, inheritingParent) => inheritingParent match
+            case Some(parent) => sym.driInContextOfInheritingParent(parent)
+            case None => sym.dri
+        }
       )
 
     ctx.staticSiteContext.foreach(_.memberLinkResolver = driFor)

scaladoc/src/dotty/tools/scaladoc/tasty/comments/Comments.scala

@@ -83,6 +83,7 @@ abstract class MarkupConversion[T](val repr: Repr)(using DocContext) {
 
   object SymOps extends SymOps[qctx.type](qctx)
   export SymOps.dri
+  export SymOps.driInContextOfInheritingParent
 
   def resolveLink(queryStr: String): DocLink =
     if SchemeUri.matches(queryStr) then DocLink.ToURL(queryStr)
@@ -94,8 +95,11 @@ abstract class MarkupConversion[T](val repr: Repr)(using DocContext) {
         DocLink.UnresolvedDRI(queryStr, msg)
       case Right(query) =>
         MemberLookup.lookup(using qctx)(query, owner) match
-          case Some((sym, targetText)) =>
-            DocLink.ToDRI(sym.dri, targetText)
+          case Some((sym, targetText, inheritingParent)) =>
+            var dri = inheritingParent match
+              case Some(parent) => sym.driInContextOfInheritingParent(parent)
+              case None => sym.dri
+            DocLink.ToDRI(dri, targetText)
           case None =>
             val txt = s"No DRI found for query"
             val msg = s"$txt: $queryStr"

scaladoc/src/dotty/tools/scaladoc/tasty/comments/MemberLookup.scala

@@ -5,15 +5,22 @@ import scala.quoted._
 
 trait MemberLookup {
 
+  def memberLookupResult(using Quotes)(
+    symbol: quotes.reflect.Symbol,
+    label: String,
+    inheritingParent: Option[quotes.reflect.Symbol] = None
+  ): (quotes.reflect.Symbol, String, Option[quotes.reflect.Symbol]) =
+    (symbol, label, inheritingParent)
+
   def lookup(using Quotes, DocContext)(
     query: Query,
     owner: quotes.reflect.Symbol,
-  ): Option[(quotes.reflect.Symbol, String)] = lookupOpt(query, Some(owner))
+  ): Option[(quotes.reflect.Symbol, String, Option[quotes.reflect.Symbol])] = lookupOpt(query, Some(owner))
 
   def lookupOpt(using Quotes, DocContext)(
     query: Query,
     ownerOpt: Option[quotes.reflect.Symbol],
-  ): Option[(quotes.reflect.Symbol, String)] =
+  ): Option[(quotes.reflect.Symbol, String, Option[quotes.reflect.Symbol])] =
     try
       import quotes.reflect._
 
@@ -26,7 +33,7 @@ trait MemberLookup {
       def nearestMembered(sym: Symbol): Symbol =
         if sym.isClassDef || sym.flags.is(Flags.Package) then sym else nearestMembered(sym.owner)
 
-      val res: Option[(Symbol, String)] = {
+      val res: Option[(Symbol, String, Option[Symbol])] = {
         def toplevelLookup(querystrings: List[String]) =
           downwardLookup(querystrings, defn.PredefModule.moduleClass)
           .orElse(downwardLookup(querystrings, defn.ScalaPackage))
@@ -38,7 +45,7 @@ trait MemberLookup {
             val nearest = nearestMembered(owner)
             val nearestCls = nearestClass(owner)
             val nearestPkg = nearestPackage(owner)
-            def relativeLookup(querystrings: List[String], owner: Symbol): Option[Symbol] = {
+            def relativeLookup(querystrings: List[String], owner: Symbol): Option[(Symbol, Option[Symbol])] = {
               val isMeaningful =
                 owner.exists
                 // those are just an optimisation, they can be dropped if problems show up
@@ -56,20 +63,20 @@ trait MemberLookup {
 
             query match {
               case Query.StrictMemberId(id) =>
-                downwardLookup(List(id), nearest).map(_ -> id)
+                downwardLookup(List(id), nearest).map(memberLookupResult(_, id, _))
               case Query.QualifiedId(Query.Qual.This, _, rest) =>
-                downwardLookup(rest.asList, nearestCls).map(_ -> rest.join)
+                downwardLookup(rest.asList, nearestCls).map(memberLookupResult(_, rest.join, _))
               case Query.QualifiedId(Query.Qual.Package, _, rest) =>
-                downwardLookup(rest.asList, nearestPkg).map(_ -> rest.join)
+                downwardLookup(rest.asList, nearestPkg).map(memberLookupResult(_, rest.join, _))
               case query =>
                 val ql = query.asList
                 toplevelLookup(ql)
                 .orElse(relativeLookup(ql, nearest))
-                .map(_ -> query.join)
+                .map(memberLookupResult(_, query.join, _))
             }
 
           case None =>
-            toplevelLookup(query.asList).map(_ -> query.join)
+            toplevelLookup(query.asList).map(memberLookupResult(_, query.join, _))
         }
       }
 
@@ -163,13 +170,15 @@ trait MemberLookup {
       }
   }
 
-  private def downwardLookup(using Quotes)(query: List[String], owner: quotes.reflect.Symbol): Option[quotes.reflect.Symbol] = {
+  private def downwardLookup(using Quotes)(
+    query: List[String], owner: quotes.reflect.Symbol
+  ): Option[(quotes.reflect.Symbol, Option[quotes.reflect.Symbol])] = {
     import quotes.reflect._
     query match {
       case Nil => None
       case q :: Nil =>
         val sel = MemberLookup.Selector.fromString(q)
-        sel.kind match {
+        val res = sel.kind match {
           case MemberLookup.SelectorKind.NoForce =>
             val lookedUp = localLookup(sel, owner).toSeq
             // note: those flag lookups are necessary b/c for objects we return their classes
@@ -178,7 +187,16 @@ trait MemberLookup {
             )
           case _ =>
             localLookup(sel, owner).nextOption
-
+        }
+        res match {
+          case None => None
+          case Some(sym) =>
+            val externalOwner: Option[quotes.reflect.Symbol] =
+              if owner eq sym.owner then None
+              else if owner.flags.is(Flags.Module) then Some(owner.moduleClass)
+              else if owner.isClassDef then Some(owner)
+              else None
+            Some(sym -> externalOwner)
         }
       case q :: qs =>
         val sel = MemberLookup.Selector.fromString(q)

scaladoc/test/dotty/tools/scaladoc/tasty/comments/MemberLookupTests.scala

@@ -38,16 +38,27 @@ class LookupTestCases[Q <: Quotes](val q: Quotes) {
 
       "java.util.AbstractCollection" -> cls("java.util.AbstractCollection"),
       "java.lang.String" -> cls("java.lang.String"),
+
+      "tests.lookupInheritedMembers.pack1.A.x" ->
+        cls("tests.lookupInheritedMembers.pack1.A").fun("x"),
+
+      "tests.lookupInheritedMembers.pack2.B.x" ->
+        cls("tests.lookupInheritedMembers.pack1.A").fun("x"),
     )
 
-    cases.foreach { case (query, Sym(sym)) =>
-      val lookupRes = MemberLookup.lookupOpt(parseQuery(query), None)
-      assertTrue(s"Couldn't look up: $query", lookupRes.nonEmpty)
-      val Some((lookedUp, _)) = lookupRes
-      assertSame(query, sym, lookedUp)
+    cases.foreach { case (query, sym) =>
+      testOwnerlessLookup(query, sym)
     }
   }
 
+  def testOwnerlessLookup(query: String, wrappedTarget: Sym): Unit = {
+    val target = wrappedTarget.symbol
+    val lookupRes = MemberLookup.lookupOpt(parseQuery(query), None)
+    assertTrue(s"Couldn't look up: $query", lookupRes.nonEmpty)
+    val Some((lookedUp, _, _)) = lookupRes
+    assertSame(query, target, lookedUp)
+  }
+
   def testOwnedLookup(): Unit = {
     val cases = List[((Sym, String), Sym)](
       cls("tests.A") -> "tests.Methods.simple" -> cls("tests.Methods").fun("simple"),
@@ -102,7 +113,7 @@ class LookupTestCases[Q <: Quotes](val q: Quotes) {
     )
 
     cases.foreach { case ((Sym(owner), query), Sym(target)) =>
-      val Some((lookedUp, _)) = MemberLookup.lookup(parseQuery(query), owner)
+      val Some((lookedUp, _, _)) = MemberLookup.lookup(parseQuery(query), owner)
       assertSame(s"$owner / $query", target, lookedUp)
     }
   }