From b866f799227b2591065ee1f18d52ea2e030cb70d Mon Sep 17 00:00:00 2001 From: Aleksander Boruch-Gruszecki Date: Sun, 7 Feb 2021 18:15:36 +0100 Subject: [PATCH 1/2] 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. --- scaladoc-testcases/src/tests/tests.scala | 14 ++- .../tasty/comments/MemberLookup.scala | 96 +++++++++++++------ .../tasty/comments/MemberLookupTests.scala | 6 ++ 3 files changed, 85 insertions(+), 31 deletions(-) diff --git a/scaladoc-testcases/src/tests/tests.scala b/scaladoc-testcases/src/tests/tests.scala index 12c805384226..4bb4f7e54786 100644 --- a/scaladoc-testcases/src/tests/tests.scala +++ b/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! diff --git a/scaladoc/src/dotty/tools/scaladoc/tasty/comments/MemberLookup.scala b/scaladoc/src/dotty/tools/scaladoc/tasty/comments/MemberLookup.scala index 4e420e5f4e2b..14893b51440e 100644 --- a/scaladoc/src/dotty/tools/scaladoc/tasty/comments/MemberLookup.scala +++ b/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,6 +80,7 @@ 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) = { @@ -74,7 +88,7 @@ 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] - 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 diff --git a/scaladoc/test/dotty/tools/scaladoc/tasty/comments/MemberLookupTests.scala b/scaladoc/test/dotty/tools/scaladoc/tasty/comments/MemberLookupTests.scala index b88e74c84817..db5c73482d6a 100644 --- a/scaladoc/test/dotty/tools/scaladoc/tasty/comments/MemberLookupTests.scala +++ b/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)) => From 65f32eafd9daf52b1e7b78938b585b2e21134727 Mon Sep 17 00:00:00 2001 From: Krzysztof Romanowski Date: Mon, 15 Feb 2021 10:37:53 +0100 Subject: [PATCH 2/2] Update logging --- .../tools/scaladoc/tasty/comments/MemberLookup.scala | 8 ++++---- .../tools/scaladoc/tasty/comments/MemberLookupTests.scala | 2 ++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/scaladoc/src/dotty/tools/scaladoc/tasty/comments/MemberLookup.scala b/scaladoc/src/dotty/tools/scaladoc/tasty/comments/MemberLookup.scala index 14893b51440e..dff2d6a33589 100644 --- a/scaladoc/src/dotty/tools/scaladoc/tasty/comments/MemberLookup.scala +++ b/scaladoc/src/dotty/tools/scaladoc/tasty/comments/MemberLookup.scala @@ -5,12 +5,12 @@ import scala.quoted._ trait MemberLookup { - def lookup(using Quotes)( + def lookup(using Quotes, DocContext)( query: Query, owner: quotes.reflect.Symbol, ): Option[(quotes.reflect.Symbol, String)] = lookupOpt(query, Some(owner)) - def lookupOpt(using Quotes)( + def lookupOpt(using Quotes, DocContext)( query: Query, ownerOpt: Option[quotes.reflect.Symbol], ): Option[(quotes.reflect.Symbol, String)] = @@ -79,8 +79,8 @@ trait MemberLookup { catch 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() + val msg = s"Unable to find a link for ${query} ${ownerOpt.fold("")(o => "in " + o.name)}" + report.warn(msg, e) None private def hackMembersOf(using Quotes)(rsym: quotes.reflect.Symbol) = { diff --git a/scaladoc/test/dotty/tools/scaladoc/tasty/comments/MemberLookupTests.scala b/scaladoc/test/dotty/tools/scaladoc/tasty/comments/MemberLookupTests.scala index db5c73482d6a..d975de32d3cf 100644 --- a/scaladoc/test/dotty/tools/scaladoc/tasty/comments/MemberLookupTests.scala +++ b/scaladoc/test/dotty/tools/scaladoc/tasty/comments/MemberLookupTests.scala @@ -9,6 +9,8 @@ import dotty.tools.scaladoc.tasty.util._ class LookupTestCases[Q <: Quotes](val q: Quotes) { + given DocContext = testDocContext() + def testAll(): Unit = { testOwnerlessLookup() testOwnedLookup()