Skip to content

Scaladoc: Fix links to members inside package objects, links containing hashes and links containg backticks #11383

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions scaladoc-testcases/src/tests/complexNames.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,21 @@ abstract class A:
def +:(other: Int): A
def :+(other: Int): A

// scaladoc has problems with names in backticks
// def `multi word name`: Int
// def `*** name with arbitrary chars ^%`: Int
// def `mischievous(param:Int)`(otherParam: Int): String
// def withMischievousParams(`param: String, param2`: String): String
def `multi word name`: Int
def `*** name with arbitrary chars ^%`: Int
def `mischievous(param:Int)`(otherParam: Int): String
def withMischievousParams(`param: String, param23`: String): String

def complexName_^*(param: String): A

def `completelyUnnecessaryBackticks`: Int //expected: def completelyUnnecessaryBackticks: Int
def `+++:`(other: Int): A //expected: def +++:(other: Int): A
def `:+++`(other: Int): A //expected: def :+++(other: Int): A

def `abc_^^_&&`: A //expected: def abc_^^_&&: A
def `abc_^^_&&`: A
def `abc_def`: A //expected: def abc_def: A
def `abc_def_++`: A //expected: def abc_def_++: A
// def `++_abc`: A
// def `abc_++_--`: A
def `++_abc`: A
def `abc_++_--`: A

class `class with backticks to check links`
6 changes: 6 additions & 0 deletions scaladoc-testcases/src/tests/functionDRI.scala
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,9 @@ class E:
def b = 10
object F:
def b = 11

object #:: {
def #:: = 10
}


3 changes: 2 additions & 1 deletion scaladoc/src/dotty/tools/scaladoc/DRI.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package dotty.tools.scaladoc

import java.nio.file.Path
import dotty.tools.scaladoc.util.Escape._

val staticFileSymbolUUID = "___staticFile___"

Expand All @@ -17,7 +18,7 @@ final case class DRI(

def isStaticFile = symbolUUID == staticFileSymbolUUID

def asFileLocation: String = location.replace(".","/")
def asFileLocation: String = escapeUrl(location).replace(".", "/")

object DRI:
def forPath(path: Path) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ class HtmlRenderer(rootPackage: Member, val members: Map[DRI, Member])(using ctx
private val args = summon[DocContext].args
val staticSite = summon[DocContext].staticSiteContext

val effectiveMembers = members.filter( (dri, member) => member.origin == Origin.RegularlyDefined && member.inheritedFrom.isEmpty)

private def needsOwnPage(member: Member): Boolean =
def properKind(kind: Kind): Boolean = kind match
case Kind.Package => true
Expand Down
7 changes: 4 additions & 3 deletions scaladoc/src/dotty/tools/scaladoc/renderers/Locations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import scala.util.matching._
val UnresolvedLocationLink = "#"

trait Locations(using ctx: DocContext):
def members: Map[DRI, Member]
def effectiveMembers: Map[DRI, Member]

var cache = new JHashMap[DRI, Seq[String]]()

Expand Down Expand Up @@ -48,7 +48,7 @@ trait Locations(using ctx: DocContext):
UnresolvedLocationLink

def pathToPage(from: DRI, to: DRI): String =
if to.isStaticFile || members.contains(to) then
if to.isStaticFile || effectiveMembers.contains(to) then
val anchor = if to.anchor.isEmpty then "" else "#" + to.anchor
pathToRaw(rawLocation(from), rawLocation(to)) +".html" + anchor
else
Expand All @@ -63,6 +63,7 @@ trait Locations(using ctx: DocContext):


def pathToRaw(from: Seq[String], to: Seq[String]): String =
import dotty.tools.scaladoc.util.Escape._
val fromDir = from.dropRight(1)
val commonPaths = to.zip(fromDir).takeWhile{ case (a, b) => a == b }.size

Expand All @@ -72,7 +73,7 @@ trait Locations(using ctx: DocContext):
case Nil => to.lastOption.fold(Seq("index"))(".." :: _ :: Nil)
case l => l

(contextPath ++ nodePath).mkString("/")
escapeUrl((contextPath ++ nodePath).mkString("/"))

def resolveRoot(from: Seq[String], to: String): String =
pathToRaw(from, to.split("/").toList)
Expand Down
15 changes: 14 additions & 1 deletion scaladoc/src/dotty/tools/scaladoc/tasty/NameNormalizer.scala
Original file line number Diff line number Diff line change
@@ -1,13 +1,26 @@
package dotty.tools.scaladoc.tasty

import dotty.tools.scaladoc._
import dotty.tools.dotc.core.StdNames.nme.keywords
import dotty.tools.dotc.core.Names.termName

trait NameNormalizer { self: TastyParser =>
import qctx.reflect._
extension (s: Symbol) def normalizedName: String = {
val withoutGivenPrefix = if s.isGiven then s.name.stripPrefix("given_") else s.name
val withoutObjectSuffix = if s.flags.is(Flags.Module) then withoutGivenPrefix.stripSuffix("$") else withoutGivenPrefix
val constructorNormalizedName = if s.isClassConstructor then "this" else withoutObjectSuffix
constructorNormalizedName
val escaped = escapedName(constructorNormalizedName)
escaped
}

private val ignoredKeywords: Set[String] = Set("this")

private def escapedName(name: String) =
val simpleIdentifierRegex = raw"(?:\w+_[^\[\(\s_]+)|\w+|[^\[\(\s\w_]+".r
name match
case n if ignoredKeywords(n) => n
case n if keywords(termName(n)) => s"`$n`"
case simpleIdentifierRegex() => name
case n => s"`$n`"
}
6 changes: 3 additions & 3 deletions scaladoc/src/dotty/tools/scaladoc/tasty/SymOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class SymOps[Q <: Quotes](val q: Q):
def className: Option[String] =
if (sym.isClassDef && !sym.flags.is(Flags.Package)) Some(
Some(sym.maybeOwner).filter(s => s.exists).flatMap(_.className).fold("")(cn => cn + "$") + sym.name
)
).filterNot(_.contains("package$"))
else if (sym.isPackageDef) None
else sym.maybeOwner.className

Expand Down Expand Up @@ -117,8 +117,8 @@ class SymOps[Q <: Quotes](val q: Q):
val csym = sym.asInstanceOf[dotc.core.Symbols.Symbol]
Option(csym.associatedFile).fold("")(_.path)
}
// We want package object to point to package
val className = sym.className.filter(_ != "package$")

val className = sym.className

DRI(
className.fold(sym.packageName)(cn => s"${sym.packageName}.${cn}"),
Expand Down
4 changes: 4 additions & 0 deletions scaladoc/src/dotty/tools/scaladoc/util/escape.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package dotty.tools.scaladoc.util

object Escape:
def escapeUrl(url: String) = url.replace("#","%23")
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class InheritanceLoop extends SignatureTest("inheritanceLoop", SignatureTest.all
class InheritedMembers extends SignatureTest("inheritedMembers2", SignatureTest.all.filter(_ != "class"),
sourceFiles = List("inheritedMembers1", "inheritedMembers2"))

class ComplexNames extends SignatureTest("complexNames", Seq("def"))
class ComplexNames extends SignatureTest("complexNames", Seq("def", "class"))

class WrongDocumentationLinks extends SignatureTest("links", Seq("def"))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import dotty.tools.scaladoc.util.HTML._
class LocationTests:
given DocContext = testDocContext()
object locations extends Locations:
val members = Map.empty
val effectiveMembers = Map.empty

@Test
def testPathToRoot() =
Expand Down