From 6da114016a3fda1069af195151c1c5581897646d Mon Sep 17 00:00:00 2001 From: Aleksander Boruch-Gruszecki Date: Mon, 22 Mar 2021 19:36:29 +0100 Subject: [PATCH 1/3] Scaladoc: in def lookup, at last stage prefer type to term This results in similar queries to what we had previously and prepares for listing members in a different way. --- .../tasty/comments/MemberLookup.scala | 73 +++++++++++++------ .../tasty/comments/MemberLookupTests.scala | 2 +- 2 files changed, 51 insertions(+), 24 deletions(-) diff --git a/scaladoc/src/dotty/tools/scaladoc/tasty/comments/MemberLookup.scala b/scaladoc/src/dotty/tools/scaladoc/tasty/comments/MemberLookup.scala index 2f338dc4a6d6..3f727ffef604 100644 --- a/scaladoc/src/dotty/tools/scaladoc/tasty/comments/MemberLookup.scala +++ b/scaladoc/src/dotty/tools/scaladoc/tasty/comments/MemberLookup.scala @@ -56,7 +56,7 @@ trait MemberLookup { query match { case Query.StrictMemberId(id) => - localLookup(id, nearest).nextOption.map(_ -> id) + downwardLookup(List(id), nearest).map(_ -> id) case Query.QualifiedId(Query.Qual.This, _, rest) => downwardLookup(rest.asList, nearestCls).map(_ -> rest.join) case Query.QualifiedId(Query.Qual.Package, _, rest) => @@ -101,26 +101,19 @@ trait MemberLookup { sym.isCompleted && sym.info.exists } - private def localLookup(using Quotes)(query: String, owner: quotes.reflect.Symbol): Iterator[quotes.reflect.Symbol] = { + private def localLookup(using Quotes)( + sel: MemberLookup.Selector, + owner: quotes.reflect.Symbol + ): Iterator[quotes.reflect.Symbol] = { import quotes.reflect._ def findMatch(syms: Iterator[Symbol]): Iterator[Symbol] = { - // Scaladoc overloading support allows terminal * (and they're meaningless) - val cleanQuery = query.stripSuffix("*") - val (q, forceTerm, forceType) = - if cleanQuery endsWith "$" then - (cleanQuery.init, true, false) - else if cleanQuery endsWith "!" then - (cleanQuery.init, false, true) - else - (cleanQuery, false, false) - def matches(s: Symbol): Boolean = - s.name == q && ( - if forceTerm then s.isTerm - else if forceType then s.isType - else true - ) + s.name == sel.ident && sel.kind.match { + case MemberLookup.SelectorKind.ForceTerm => s.isTerm + case MemberLookup.SelectorKind.ForceType => s.isType + case MemberLookup.SelectorKind.NoForce => true + } def hackResolveModule(s: Symbol): Symbol = if s.flags.is(Flags.Module) then s.moduleClass else s @@ -128,7 +121,7 @@ trait MemberLookup { // val syms0 = syms.toList // val matched0 = syms0.filter(matches) // if matched0.isEmpty then - // println(s"Failed to look up $q in $owner; all members: {{{") + // println(s"Failed to look up ${sel.ident} in $owner; all members: {{{") // syms0.foreach { s => println(s"\t$s") } // println("}}}") // val matched = matched0.iterator @@ -136,8 +129,9 @@ trait MemberLookup { // def showMatched() = matched0.foreach { s => // println(s"\t $s") // } - // println(s"localLookup in class ${owner} for `$q`{forceTerm=$forceTerm}:") + // println(s"localLookup in class ${owner} for `${sel.ident}`{kind=${sel.kind}}:{{{") // showMatched() + // println("}}}") val matched = syms.filter(matches) matched.map(hackResolveModule) @@ -172,10 +166,22 @@ trait MemberLookup { import quotes.reflect._ query match { case Nil => None - case q :: Nil => localLookup(q, owner).nextOption + case q :: Nil => + val sel = MemberLookup.Selector.fromString(q) + 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 + lookedUp.find(s => s.isType && !s.flags.is(Flags.Module)).orElse( + lookedUp.find(s => s.isTerm || s.flags.is(Flags.Module)) + ) + case _ => + localLookup(sel, owner).nextOption + + } case q :: qs => - val lookedUp = - localLookup(q, owner).toSeq + val sel = MemberLookup.Selector.fromString(q) + val lookedUp = localLookup(sel, owner).toSeq if lookedUp.isEmpty then None else { // tm/tp - term/type symbols which we looked up and which allow further lookup @@ -198,4 +204,25 @@ trait MemberLookup { } } -object MemberLookup extends MemberLookup +object MemberLookup extends MemberLookup { + enum SelectorKind { + case ForceTerm + case ForceType + case NoForce + } + + case class Selector(ident: String, kind: SelectorKind) + object Selector { + def fromString(str: String) = { + // Scaladoc overloading support allows terminal * (and they're meaningless) + val cleanStr = str.stripSuffix("*") + + if cleanStr endsWith "$" then + Selector(cleanStr.init, SelectorKind.ForceTerm) + else if cleanStr endsWith "!" then + Selector(cleanStr.init, SelectorKind.ForceType) + else + Selector(cleanStr, SelectorKind.NoForce) + } + } +} diff --git a/scaladoc/test/dotty/tools/scaladoc/tasty/comments/MemberLookupTests.scala b/scaladoc/test/dotty/tools/scaladoc/tasty/comments/MemberLookupTests.scala index 5a34a21d0dfe..c61445be7df6 100644 --- a/scaladoc/test/dotty/tools/scaladoc/tasty/comments/MemberLookupTests.scala +++ b/scaladoc/test/dotty/tools/scaladoc/tasty/comments/MemberLookupTests.scala @@ -65,7 +65,7 @@ class LookupTestCases[Q <: Quotes](val q: Quotes) { cls("tests.A") -> "AA!" -> cls("tests.A").tpe("AA"), cls("tests.A") -> "AA$" -> cls("tests.A").fld("AA"), - cls("tests.C") -> "CC" -> cls("tests.C").fld("CC"), + cls("tests.C") -> "CC" -> cls("tests.C").tpe("CC"), cls("tests.C") -> "CC$" -> cls("tests.C").fld("CC"), cls("tests.C") -> "CC!" -> cls("tests.C").tpe("CC"), From e4a63bf1cbd528b6da71a2267a3e4a23aa15ebaa Mon Sep 17 00:00:00 2001 From: Aleksander Boruch-Gruszecki Date: Mon, 22 Mar 2021 19:37:34 +0100 Subject: [PATCH 2/3] Scaladoc: look up inherited definitions as well --- scaladoc-testcases/src/tests/tests.scala | 6 +++++- .../dotty/tools/scaladoc/tasty/comments/MemberLookup.scala | 7 ++++--- .../tools/scaladoc/tasty/comments/MemberLookupTests.scala | 4 ++++ 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/scaladoc-testcases/src/tests/tests.scala b/scaladoc-testcases/src/tests/tests.scala index 77f38de225d5..3d8d532ff534 100644 --- a/scaladoc-testcases/src/tests/tests.scala +++ b/scaladoc-testcases/src/tests/tests.scala @@ -131,6 +131,10 @@ class B extends A { class BB } +class BModule { + def foo = "foo" +} + /** Companion object to test linking. * * This is my member: [[B$.Z]] @@ -141,7 +145,7 @@ class B extends A { * * And this is my term member, addressed differently: [[this.Z$]] */ -object B { +object B extends BModule { type Z = Int val Z: Int = 0 } diff --git a/scaladoc/src/dotty/tools/scaladoc/tasty/comments/MemberLookup.scala b/scaladoc/src/dotty/tools/scaladoc/tasty/comments/MemberLookup.scala index 3f727ffef604..f012aa931058 100644 --- a/scaladoc/src/dotty/tools/scaladoc/tasty/comments/MemberLookup.scala +++ b/scaladoc/src/dotty/tools/scaladoc/tasty/comments/MemberLookup.scala @@ -88,7 +88,10 @@ 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(s => hackIsNotAbsent(s.asInstanceOf[Symbol])) + val members = + sym.info.allMembers.iterator.map(_.symbol).filter( + s => hackIsNotAbsent(s.asInstanceOf[Symbol]) + ) // println(s"members of ${sym.show} : ${members.map(_.show).mkString(", ")}") members.asInstanceOf[Iterator[Symbol]] } @@ -144,8 +147,6 @@ trait MemberLookup { }) else owner.tree match { - case tree: ClassDef => - findMatch(tree.body.iterator.collect { case t: Definition if hackIsNotAbsent(t.symbol) => t.symbol }) case tree: TypeDef => val tpe = tree.rhs match { diff --git a/scaladoc/test/dotty/tools/scaladoc/tasty/comments/MemberLookupTests.scala b/scaladoc/test/dotty/tools/scaladoc/tasty/comments/MemberLookupTests.scala index c61445be7df6..62202b26af1c 100644 --- a/scaladoc/test/dotty/tools/scaladoc/tasty/comments/MemberLookupTests.scala +++ b/scaladoc/test/dotty/tools/scaladoc/tasty/comments/MemberLookupTests.scala @@ -28,6 +28,8 @@ class LookupTestCases[Q <: Quotes](val q: Quotes) { "???" -> cls("scala.Predef$").fun("???"), "scala.List" -> cls("scala.package$").tpe("List"), + "scala.List.lift" -> cls("scala.PartialFunction").fun("lift"), + "tests.A" -> cls("tests.A"), "tests.A$" -> cls("tests.A$"), "tests.Methods.simple" -> cls("tests.Methods").fun("simple"), @@ -91,6 +93,8 @@ class LookupTestCases[Q <: Quotes](val q: Quotes) { cls("tests.inner.B") -> "A" -> cls("tests.inner.A$"), + cls("tests.B$") -> "foo" -> cls("tests.BModule").fun("foo"), + cls("tests.D") -> "foo" -> cls("tests.package$").fld("foo"), cls("tests.D") -> "bar" -> cls("tests.tests$package$").fld("bar"), cls("tests.inner.A$") -> "foo" -> cls("tests.package$").fld("foo"), From 421af1e107bb4e8913561079e3560a0f86b2516a Mon Sep 17 00:00:00 2001 From: Andrzej Ratajczak Date: Mon, 22 Mar 2021 20:59:39 +0100 Subject: [PATCH 3/3] Scaladoc: resolve looked up inherited symbols to correct DRIs --- scaladoc-testcases/src/tests/links.scala | 3 +- .../src/tests/lookupInheritedMembers.scala | 20 +++++++++ .../dotty/tools/scaladoc/tasty/SymOps.scala | 5 +++ .../tools/scaladoc/tasty/TastyParser.scala | 8 +++- .../scaladoc/tasty/comments/Comments.scala | 8 +++- .../tasty/comments/MemberLookup.scala | 42 +++++++++++++------ .../tasty/comments/MemberLookupTests.scala | 23 +++++++--- 7 files changed, 85 insertions(+), 24 deletions(-) create mode 100644 scaladoc-testcases/src/tests/lookupInheritedMembers.scala diff --git a/scaladoc-testcases/src/tests/links.scala b/scaladoc-testcases/src/tests/links.scala index 542b1d85f13a..c51a14f75fdc 100644 --- a/scaladoc-testcases/src/tests/links.scala +++ b/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 \ No newline at end of file + = 123 diff --git a/scaladoc-testcases/src/tests/lookupInheritedMembers.scala b/scaladoc-testcases/src/tests/lookupInheritedMembers.scala new file mode 100644 index 000000000000..d39bd0fe4422 --- /dev/null +++ b/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 +} diff --git a/scaladoc/src/dotty/tools/scaladoc/tasty/SymOps.scala b/scaladoc/src/dotty/tools/scaladoc/tasty/SymOps.scala index c150a1b8e304..65fd33b30128 100644 --- a/scaladoc/src/dotty/tools/scaladoc/tasty/SymOps.scala +++ b/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 + ) diff --git a/scaladoc/src/dotty/tools/scaladoc/tasty/TastyParser.scala b/scaladoc/src/dotty/tools/scaladoc/tasty/TastyParser.scala index 033321f0b0d1..0e5871138926 100644 --- a/scaladoc/src/dotty/tools/scaladoc/tasty/TastyParser.scala +++ b/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) diff --git a/scaladoc/src/dotty/tools/scaladoc/tasty/comments/Comments.scala b/scaladoc/src/dotty/tools/scaladoc/tasty/comments/Comments.scala index 8a460c84e3eb..c60187c1b8b9 100644 --- a/scaladoc/src/dotty/tools/scaladoc/tasty/comments/Comments.scala +++ b/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" diff --git a/scaladoc/src/dotty/tools/scaladoc/tasty/comments/MemberLookup.scala b/scaladoc/src/dotty/tools/scaladoc/tasty/comments/MemberLookup.scala index f012aa931058..93aafa7cb80a 100644 --- a/scaladoc/src/dotty/tools/scaladoc/tasty/comments/MemberLookup.scala +++ b/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) diff --git a/scaladoc/test/dotty/tools/scaladoc/tasty/comments/MemberLookupTests.scala b/scaladoc/test/dotty/tools/scaladoc/tasty/comments/MemberLookupTests.scala index 62202b26af1c..dafd4003fa6f 100644 --- a/scaladoc/test/dotty/tools/scaladoc/tasty/comments/MemberLookupTests.scala +++ b/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) } }