Scaladoc: further definition lookup fixes

Lookup supports DFS now, and duplicates more of Scaladoc's old behaviour.

Fixes majority of remaining lookup warnings in stdlib.

Author: abgruszecki

scaladoc-testcases/src/tests/tests.scala

@@ -85,7 +85,11 @@ class A {
 }
 
 /** Companion object to test linking */
-object A
+object A {
+  /** Apparently this should work: [[A.bar]]. */
+  def foo() = 0
+  def bar() = 0
+}
 
 /** = An important Wiki test class =
   *
@@ -150,6 +154,14 @@ class C extends A {
 class D[T]
 class E[T] extends D[T]
 
+package inner {
+  object A
+  class B {
+    /** This resolves: [[A]]. */
+    def foo() = ()
+  }
+}
+
 /** A class with a semi-non-trivial constructor.
   *
   * @param a Hello!

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

@@ -26,39 +26,52 @@ trait MemberLookup {
       def nearestMembered(sym: Symbol): Symbol =
         if sym.isClassDef || sym.flags.is(Flags.Package) then sym else nearestMembered(sym.owner)
 
-      val res =
+      val res: Option[(Symbol, String)] = {
         def toplevelLookup(querystrings: List[String]) =
           downwardLookup(querystrings, defn.PredefModule.moduleClass)
           .orElse(downwardLookup(querystrings, defn.ScalaPackage))
           .orElse(downwardLookup(querystrings, defn.RootPackage))
+          .orElse(downwardLookup(querystrings, defn.EmptyPackageClass))
 
         ownerOpt match {
           case Some(owner) =>
             val nearest = nearestMembered(owner)
             val nearestCls = nearestClass(owner)
             val nearestPkg = nearestPackage(owner)
-            def relativeLookup(querystrings: List[String]) =
-              // TODO walk the owner chain?
-              downwardLookup(querystrings, nearestPkg).orElse(toplevelLookup(querystrings))
+            def relativeLookup(querystrings: List[String], owner: Symbol): Option[Symbol] = {
+              val isMeaningful =
+                owner.exists
+                // those are just an optimisation, they can be dropped if problems show up
+                && owner.ne(defn.ScalaPackage)
+                && owner.ne(defn.RootClass)
+                && owner.ne(defn.EmptyPackageClass)
+
+              if !isMeaningful then None else {
+                downwardLookup(querystrings, owner) match {
+                  case None => relativeLookup(querystrings, owner.owner)
+                  case some => some
+                }
+              }
+            }
+
             query match {
-              case Query.StrictMemberId(id) => localLookup(id, nearest).map(_ -> id)
-              case Query.Id(id) =>
-                (localLookup(id, nearest) orElse relativeLookup(List(id))).map(_ -> id)
+              case Query.StrictMemberId(id) =>
+                localLookup(id, nearest).nextOption.map(_ -> id)
               case Query.QualifiedId(Query.Qual.This, _, rest) =>
                 downwardLookup(rest.asList, nearestCls).map(_ -> rest.join)
               case Query.QualifiedId(Query.Qual.Package, _, rest) =>
                 downwardLookup(rest.asList, nearestPkg).map(_ -> rest.join)
-              case Query.QualifiedId(Query.Qual.Id(id), _, rest) if id == nearestCls.name =>
-                downwardLookup(rest.asList, nearestCls).map(_ -> rest.join)
-              case Query.QualifiedId(Query.Qual.Id(id), _, rest) if id == nearestPkg.name =>
-                downwardLookup(rest.asList, nearestPkg).map(_ -> rest.join)
-              case query: Query.QualifiedId =>
-                relativeLookup(query.asList).map(_ -> query.join)
+              case query =>
+                val ql = query.asList
+                toplevelLookup(ql)
+                .orElse(relativeLookup(ql, nearest))
+                .map(_ -> query.join)
             }
 
           case None =>
             toplevelLookup(query.asList).map(_ -> query.join)
         }
+      }
 
       // println(s"looked up `$query` in ${owner.show}[${owner.flags.show}] as ${res.map(_.show)}")
 
@@ -67,14 +80,15 @@ trait MemberLookup {
       case e: Exception =>
         // TODO (https://github.com/lampepfl/scala3doc/issues/238): proper reporting
         println(s"[WARN] Unable to find a link for ${query} ${ownerOpt.fold("")(o => "in " + o.name)}")
+        e.printStackTrace()
         None
 
   private def hackMembersOf(using Quotes)(rsym: quotes.reflect.Symbol) = {
     import quotes.reflect._
     import dotty.tools.dotc
     given dotc.core.Contexts.Context = quotes.asInstanceOf[scala.quoted.runtime.impl.QuotesImpl].ctx
     val sym = rsym.asInstanceOf[dotc.core.Symbols.Symbol]
-    val members = sym.info.decls.iterator.filter(_.isCompleted)
+    val members = sym.info.decls.iterator.filter(s => hackIsNotAbsent(s.asInstanceOf[Symbol]))
     // println(s"members of ${sym.show} : ${members.map(_.show).mkString(", ")}")
     members.asInstanceOf[Iterator[Symbol]]
   }
@@ -83,16 +97,14 @@ trait MemberLookup {
     import dotty.tools.dotc
     given dotc.core.Contexts.Context = quotes.asInstanceOf[scala.quoted.runtime.impl.QuotesImpl].ctx
     val sym = rsym.asInstanceOf[dotc.core.Symbols.Symbol]
-    sym.isCompleted
+    // note: Predef has .info = NoType for some reason
+    sym.isCompleted && sym.info.exists
   }
 
-  private def localLookup(using Quotes)(query: String, owner: quotes.reflect.Symbol): Option[quotes.reflect.Symbol] = {
+  private def localLookup(using Quotes)(query: String, owner: quotes.reflect.Symbol): Iterator[quotes.reflect.Symbol] = {
     import quotes.reflect._
 
-    inline def whenExists(s: Symbol)(otherwise: => Option[Symbol]): Option[Symbol] =
-      if s.exists then Some(s) else otherwise
-
-    def findMatch(syms: Iterator[Symbol]): Option[Symbol] = {
+    def findMatch(syms: Iterator[Symbol]): Iterator[Symbol] = {
       // Scaladoc overloading support allows terminal * (and they're meaningless)
       val cleanQuery = query.stripSuffix("*")
       val (q, forceTerm, forceType) =
@@ -114,23 +126,24 @@ trait MemberLookup {
         if s.flags.is(Flags.Module) then s.moduleClass else s
 
       // val syms0 = syms.toList
-      // val matched0 = syms0.find(matches)
+      // val matched0 = syms0.filter(matches)
       // if matched0.isEmpty then
       //   println(s"Failed to look up $q in $owner; all members below:")
       //   syms0.foreach { s => println(s"\t$s") }
-      // val matched = matched0
+      // val matched = matched0.iterator
 
       // def showMatched() = matched.foreach { s =>
-      //   println(s">>> ${s.show}")
+      //   println(s">>> $s")
       //   println(s">>> ${s.pos}")
       //   println(s">>> [${s.flags.show}]")
       //   println(s">>> {${if s.isTerm then "isterm" else ""};${if s.isType then "istype" else ""}}")
       //   println(s">>> moduleClass = ${if hackResolveModule(s) == s then hackResolveModule(s).show else "none"}")
       // }
-      // println(s"localLookup for class ${owner.show} of `$q`{forceTerm=$forceTerm}")
+      // println(s"localLookup in class ${owner} for `$q`{forceTerm=$forceTerm}")
+      // println(s"\t${matched0.mkString(", ")}")
       // showMatched()
 
-      val matched = syms.find(matches)
+      val matched = syms.filter(matches)
       matched.map(hackResolveModule)
     }
 
@@ -147,20 +160,43 @@ trait MemberLookup {
               case tpt: TypeTree => tpt.tpe
             }
 
-          tpe.classSymbol.flatMap { s =>
-            findMatch(hackMembersOf(s))
+          tpe.classSymbol match {
+            case Some(s) => findMatch(hackMembersOf(s))
+            case None => Iterator.empty
           }
         case _ =>
           findMatch(hackMembersOf(owner))
       }
   }
 
-  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] = {
+    import quotes.reflect._
     query match {
       case Nil => None
-      case q :: Nil => localLookup(q, owner)
-      case q :: qs => localLookup(q, owner).flatMap(downwardLookup(qs, _))
+      case q :: Nil => localLookup(q, owner).nextOption
+      case q :: qs =>
+        val lookedUp =
+          localLookup(q, owner).toSeq
+
+        if lookedUp.isEmpty then None else {
+          // tm/tp - term/type symbols which we looked up and which allow further lookup
+          // pk - package symbol
+          // Note: packages collide with both term and type definitions
+          // Note: classes and types collide
+          var pk: Option[Symbol] = None
+          var tp: Option[Symbol] = None
+          var tm: Option[Symbol] = None
+          lookedUp.foreach { s =>
+            if s.isPackageDef then pk = Some(s)
+            else if s.flags.is(Flags.Module) then tm = Some(s)
+            else if s.isClassDef || s.isTypeDef then tp = Some(s)
+          }
+          pk.flatMap(downwardLookup(qs, _))
+          .orElse(tp.flatMap(downwardLookup(qs, _)))
+          .orElse(tm.flatMap(downwardLookup(qs, _)))
+        }
     }
+  }
 }
 
 object MemberLookup extends MemberLookup

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

@@ -19,7 +19,9 @@ class LookupTestCases[Q <: Quotes](val q: Quotes) {
     val cases = List[(String, Sym)](
       "Array" -> cls("scala.Array"),
       "Option" -> cls("scala.Option"),
+      "Predef$" -> cls("scala.Predef$"),
       "Predef$.identity" -> cls("scala.Predef$").fun("identity"),
+      "Predef.identity" -> cls("scala.Predef$").fun("identity"),
       "Array$.from" -> cls("scala.Array$").fun("from"),
       "???" -> cls("scala.Predef$").fun("???"),
       "tests.A" -> cls("tests.A"),
@@ -75,6 +77,10 @@ class LookupTestCases[Q <: Quotes](val q: Quotes) {
       /*sanity*/ cls("tests.A") -> "this.Y" -> cls("tests.A").tpe("Y"),
       cls("tests.A") -> "this.X.method" -> cls("tests.B").fun("method"),
       cls("tests.A") -> "this.Y.method" -> cls("tests.B").fun("method"),
+
+      cls("tests.A") -> "A.foo" -> cls("tests.A$").fun("foo"),
+
+      cls("tests.inner.B") -> "A" -> cls("tests.inner.A$"),
     )
 
     cases.foreach { case ((Sym(owner), query), Sym(target)) =>