Merge pull request #14516 from jchyb/scaladoc/fix-i14505-i14504

Scaladoc: fix issues with incorrect external links and special characters

Author: TheElectronWill

scaladoc-js/main/src/searchbar/PageEntry.scala

@@ -4,17 +4,19 @@ import scala.scalajs.js
 
 @js.native
 trait PageEntryJS extends js.Object {
-  val n: String = js.native
-  val t: String = js.native
-  val d: String = js.native
-  val l: String = js.native
-  val k: String = js.native
+  val n: String  = js.native
+  val t: String  = js.native
+  val d: String  = js.native
+  val l: String  = js.native
+  val e: Boolean = js.native
+  val k: String  = js.native
 }
 
 case class PageEntry(
   fullName: String,
   description: String,
   location: String,
+  isLocationExternal: Boolean,
   shortName: String,
   kind: String,
   tokens: List[String]
@@ -34,6 +36,7 @@ object PageEntry {
       jsObj.t,
       jsObj.d,
       jsObj.l,
+      jsObj.e,
       jsObj.n.toLowerCase,
       jsObj.k,
       StringUtils.createCamelCaseTokens(jsObj.n)

scaladoc-js/main/src/searchbar/SearchbarComponent.scala

@@ -22,7 +22,12 @@ class SearchbarComponent(engine: SearchbarEngine, inkuireEngine: InkuireJSSearch
       icon.classList.add(p.kind.take(2))
 
       val resultA = document.createElement("a").asInstanceOf[html.Anchor]
-      resultA.href = Globals.pathToRoot + p.location
+      resultA.href =
+        if (p.isLocationExternal) {
+          p.location
+        } else {
+          Globals.pathToRoot + p.location
+        }
       resultA.text = s"${p.fullName}"
       resultA.onclick = (event: Event) =>
         if (document.body.contains(rootDiv)) {
@@ -57,11 +62,14 @@ class SearchbarComponent(engine: SearchbarEngine, inkuireEngine: InkuireJSSearch
       icon.classList.add(m.entryType.take(2))
 
       val resultA = document.createElement("a").asInstanceOf[html.Anchor]
+      // Inkuire pageLocation should start with e (external)
+      // or i (internal). The rest of the string is an absolute
+      // or relative URL
       resultA.href = 
-        if(new URI(m.pageLocation).isAbsolute()) {
-          m.pageLocation
+        if (m.pageLocation(0) == 'e') {
+          m.pageLocation.substring(1)
         } else {
-          Globals.pathToRoot + m.pageLocation
+          Globals.pathToRoot + m.pageLocation.substring(1)
         } 
       resultA.text = m.functionName
       resultA.onclick = (event: Event) =>

scaladoc-js/main/test/dotty/tools/scaladoc/MatchersTest.scala

@@ -34,6 +34,7 @@ class MatchersTest:
     s"$kind $name",
     "",
     "",
+    false,
     s"$name",
     kind,
     StringUtils.createCamelCaseTokens(name)

scaladoc/src/dotty/tools/scaladoc/Inkuire.scala

@@ -114,6 +114,7 @@ object Inkuire {
     name:         String,
     packageName:  String,
     uri:          String,
+    isLocationExternal: Boolean,
     entryType:    String
   )
 
@@ -333,7 +334,7 @@ object Inkuire {
         ("signature", serialize(e.signature)),
         ("name", serialize(e.name)),
         ("packageName", serialize(e.packageName)),
-        ("uri", serialize(e.uri)),
+        ("uri", serialize((if e.isLocationExternal then "e" else "i") + e.uri)),
         ("entryType", serialize(e.entryType))
       )
     }

scaladoc/src/dotty/tools/scaladoc/renderers/Locations.scala

@@ -81,7 +81,12 @@ trait Locations(using ctx: DocContext):
 
   def resolveRoot(dri: DRI, path: String): String = resolveRoot(rawLocation(dri), path)
   def absolutePath(dri: DRI, extension: String = "html"): String = rawLocation(dri).mkString("", "/", s".$extension")
-  def absolutePathWithAnchor(dri: DRI, extension: String = "html"): String = s"${absolutePath(dri, extension)}#${dri.anchor}"
+
+  def escapedAbsolutePathWithAnchor(dri: DRI, extension: String = "html"): String =
+    s"${escapeUrl(absolutePath(dri, extension))}#${dri.anchor}"
+
+  def relativeInternalOrAbsoluteExternalPath(dri: DRI): String =
+    dri.externalLink.getOrElse(escapedAbsolutePathWithAnchor(dri))
 
   def resolveLink(dri: DRI, url: String): String =
     if URI(url).isAbsolute then url else resolveRoot(dri, url)

scaladoc/src/dotty/tools/scaladoc/renderers/Renderer.scala

@@ -120,7 +120,9 @@ abstract class Renderer(rootPackage: Member, val members: Map[DRI, Member], prot
       val signatureRenderer = new SignatureRenderer:
         def currentDri: DRI = page.link.dri
         def link(dri: DRI): Option[String] =
-          Some(pathToPage(currentDri, dri)).filter(_ != UnresolvedLocationLink)
+          dri.externalLink.orElse(
+            Some(pathToPage(currentDri, dri)).filter(_ != UnresolvedLocationLink)
+          )
 
       MemberRenderer(signatureRenderer).fullMember(m)
     case t: ResolvedTemplate => siteContent(page.link.dri, t)

scaladoc/src/dotty/tools/scaladoc/renderers/Resources.scala

@@ -133,7 +133,8 @@ trait Resources(using ctx: DocContext) extends Locations, Writer:
       }.mkString
 
     def mkEntry(dri: DRI, name: String, text: String, descr: String, kind: String) = jsonObject(
-        "l" -> jsonString(absolutePathWithAnchor(dri)),
+        "l" -> jsonString(relativeInternalOrAbsoluteExternalPath(dri)),
+        "e" -> (if dri.externalLink.isDefined then rawJSON("true") else rawJSON("false")),
         "n" -> jsonString(name),
         "t" -> jsonString(text),
         "d" -> jsonString(descr),

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

@@ -114,7 +114,8 @@ trait InkuireSupport(using DocContext) extends Resources:
             ),
             name = name,
             packageName = ownerName,
-            uri = methodSymbol.dri.externalLink.getOrElse(absolutePathWithAnchor(methodSymbol.dri)),
+            uri = methodSymbol.dri.externalLink.getOrElse(escapedAbsolutePathWithAnchor(methodSymbol.dri)),
+            isLocationExternal = methodSymbol.dri.externalLink.isDefined,
             entryType = "def"
           )
           val curriedSgn = sgn.copy(signature = Inkuire.curry(sgn.signature))
@@ -142,7 +143,8 @@ trait InkuireSupport(using DocContext) extends Resources:
             ),
             name = name,
             packageName = ownerName,
-            uri = valSymbol.dri.externalLink.getOrElse(absolutePathWithAnchor(valSymbol.dri)),
+            uri = valSymbol.dri.externalLink.getOrElse(escapedAbsolutePathWithAnchor(valSymbol.dri)),
+            isLocationExternal = valSymbol.dri.externalLink.isDefined,
             entryType = "val"
           )
           val curriedSgn = sgn.copy(signature = Inkuire.curry(sgn.signature))

scaladoc/test/dotty/tools/scaladoc/renderers/LocationTests.scala

@@ -37,3 +37,18 @@ class LocationTests:
       "../../annotation",
       path("api/scala/annotation/meta/beanGetter", "api/scala/annotation"),
     )
+
+  @Test
+  def testAnchorLinks() =
+    def pathWithAnchor(location: String, anchor: String) =
+      locations.escapedAbsolutePathWithAnchor(new DRI(location, anchor))
+
+    assertEquals(
+      "scala/%23::.html#abcde",
+      pathWithAnchor("scala.#::", "abcde")
+    )
+
+    assertEquals(
+      "scala/collection/immutable/LazyList$$%23$.html#abcde",
+      pathWithAnchor("scala.collection.immutable.LazyList$$#$", "abcde")
+    )