Skip to content
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

Fix broken Scaladoc external references with JDK11 #8663

Merged
merged 1 commit into from Apr 16, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion project/ScalaOptionParser.scala
Expand Up @@ -124,7 +124,7 @@ object ScalaOptionParser {
private def scalaDocBooleanSettingNames = List("-implicits", "-implicits-debug", "-implicits-show-all", "-implicits-sound-shadowing", "-implicits-hide", "-author", "-diagrams", "-diagrams-debug", "-raw-output", "-no-prefixes", "-no-link-warnings", "-expand-all-types", "-groups")
private def scalaDocIntSettingNames = List("-diagrams-max-classes", "-diagrams-max-implicits", "-diagrams-dot-timeout", "-diagrams-dot-restart")
private def scalaDocChoiceSettingNames = Map("-doc-format" -> List("html"))
private def scaladocStringSettingNames = List("-doc-title", "-doc-version", "-doc-footer", "-doc-no-compile", "-doc-source-url", "-doc-generator", "-skip-packages")
private def scaladocStringSettingNames = List("-doc-title", "-doc-version", "-doc-footer", "-doc-no-compile", "-doc-source-url", "-doc-generator", "-skip-packages", "-jdk-api-doc-base")
private def scaladocPathSettingNames = List("-doc-root-content", "-diagrams-dot-path")
private def scaladocMultiStringSettingNames = List("-doc-external-doc")

Expand Down
6 changes: 5 additions & 1 deletion src/manual/scala/man1/scaladoc.scala
Expand Up @@ -77,7 +77,11 @@ object scaladoc extends Command {
"Define a URL to be concatenated with source locations for link to source files."),
Definition(
CmdOption("doc-external-doc", Argument("external-doc")),
"Define a comma-separated list of classpath_entry_path#doc_URL pairs describing external dependencies."))),
"Define a comma-separated list of classpath_entry_path#doc_URL pairs describing external dependencies."),
Definition(
CmdOption("jdk-api-doc-base", Argument("url")),
"Define a URL to be concatenated with source locations for link to Java API."))
),

Section("Compiler Options",
DefinitionList(
Expand Down
7 changes: 7 additions & 0 deletions src/scaladoc/scala/tools/nsc/doc/Settings.scala
Expand Up @@ -87,6 +87,13 @@ class Settings(error: String => Unit, val printMsg: String => Unit = println(_),
"comma-separated list of classpath_entry_path#doc_URL pairs describing external dependencies."
)

val jdkApiDocBase = StringSetting (
"-jdk-api-doc-base",
"url",
"URL used to link Java API references.",
""
)

val docgenerator = StringSetting (
"-doc-generator",
"class-name",
Expand Down
92 changes: 75 additions & 17 deletions src/scaladoc/scala/tools/nsc/doc/model/MemberLookup.scala
Expand Up @@ -14,7 +14,10 @@ package scala.tools.nsc
package doc
package model

import java.nio.file.Paths

import base._
import scala.tools.nsc.io.AbstractFile

/** This trait extracts all required information for documentation from compilation units */
trait MemberLookup extends base.MemberLookupBase {
Expand Down Expand Up @@ -57,28 +60,83 @@ trait MemberLookup extends base.MemberLookupBase {
/* Get package object which has associatedFile ne null */
sym.info.member(newTermName("package"))
else sym
def classpathEntryFor(s: Symbol): Option[String] = {
Option(s.associatedFile).flatMap(_.underlyingSource).map { src =>
val path = src.canonicalPath
if(path.endsWith(".class")) { // Individual class file -> Classpath entry is root dir
val nesting = s.ownerChain.count(_.hasPackageFlag)
if(nesting > 0) {
val p = 0.until(nesting).foldLeft(src) {
case (null, _) => null
case (f, _) => f.container
}
if(p eq null) path else p.canonicalPath
} else path
} else path // JAR file (and fallback option)
}
}
classpathEntryFor(sym1) flatMap { path =>
settings.extUrlMapping get path map { url => {
LinkToExternalTpl(name, url, makeTemplate(sym))
if (isJDK(sym1)) {
Some(LinkToExternalTpl(name, jdkUrl(path), makeTemplate(sym)))
}
else {
settings.extUrlMapping get path map { url =>
LinkToExternalTpl(name, url, makeTemplate(sym))
}
}
}
}

private def classpathEntryFor(s: Symbol): Option[String] = {
Option(s.associatedFile).flatMap(_.underlyingSource).map { src =>
val path = src.canonicalPath
if(path.endsWith(".class")) { // Individual class file -> Classpath entry is root dir
val nesting = s.ownerChain.count(_.hasPackageFlag)
if(nesting > 0) {
val p = 0.until(nesting).foldLeft(src) {
case (null, _) => null
case (f, _) => f.container
}
if(p eq null) path else p.canonicalPath
} else path
} else path // JAR file (and fallback option)
}
}

/**
* Check if this file is a child of the given directory string. Can only be used
* on directories that actually exist in the file system.
*/
def isChildOf(f: AbstractFile, dir: String): Boolean = {
val parent = Paths.get(dir).toAbsolutePath().toString
f.canonicalPath.startsWith(parent)
}

private def isJDK(sym: Symbol) =
sym.associatedFile.underlyingSource.map(f => isChildOf(f, (sys.props("java.home")))).getOrElse(false)

def jdkUrl(path: String): String = {
if (path.endsWith(".jmod")) {
val tokens = path.split("/")
val module = tokens.last.stripSuffix(".jmod")
s"$jdkUrl/$module"
}
else {
jdkUrl
}
}

def jdkUrl: String = {
if (settings.jdkApiDocBase.isDefault)
defaultJdkUrl
else
settings.jdkApiDocBase.value
}

lazy val defaultJdkUrl = {
if (javaVersion < 11) {
s"https://docs.oracle.com/javase/$javaVersion/docs/api"
}
else {
s"https://docs.oracle.com/en/java/javase/$javaVersion/docs/api"
}
}

lazy val javaVersion: Int = {
var version = System.getProperty("java.version")
(if (version.startsWith("1.")) {
version.substring(2, 3)
}
else {
val dot = version.indexOf(".")
version.substring(0, dot)
}).toInt
}

override def warnNoLink = !settings.docNoLinkWarnings.value
}
16 changes: 10 additions & 6 deletions test/scaladoc/run/t191.scala
Expand Up @@ -19,6 +19,7 @@ object Test extends ScaladocModelTest {
* - [[scala]] Linking to a package
* - [[scala.AbstractMethodError]] Linking to a member in the package object
* - [[scala.Predef.String]] Linking to a member in an object
* - [[java.lang.Throwable]] Linking to a class in the JDK
*
* Don't look at:
* - [[scala.NoLink]] Not linking :)
Expand All @@ -32,6 +33,7 @@ object Test extends ScaladocModelTest {
"""

def scalaURL = "http://bog.us"
val jdkURL = "http://java.us"

override def scaladocSettings = {
val samplePath = getClass.getClassLoader.getResource("scala/Function1.class").getPath
Expand All @@ -41,7 +43,7 @@ object Test extends ScaladocModelTest {
} else { // individual class files on disk
samplePath.replace('\\', '/').dropRight("scala/Function1.class".length)
}
s"-no-link-warnings -doc-external-doc $scalaLibPath#$scalaURL"
s"-no-link-warnings -doc-external-doc $scalaLibPath#$scalaURL -jdk-api-doc-base $jdkURL"
}

def testModel(rootPackage: Package): Unit = {
Expand All @@ -50,15 +52,17 @@ object Test extends ScaladocModelTest {

def check(memberDef: Def, expected: Int): Unit = {
val externals = memberDef.valueParams(0)(0).resultType.refEntity collect {
case (_, (LinkToExternalTpl(name, url, _), _)) => assert(url.contains(scalaURL)); name
case (_, (LinkToExternalTpl(name, url, _), _)) =>
assert(url.contains(scalaURL) || url.contains(jdkURL))
name
}
assert(externals.size == expected)
}

check(test._method("foo"), 1)
check(test._method("bar"), 0)
check(test._method("barr"), 2)
check(test._method("baz"), 0)
check(test._method("baz"), 1)

val expectedUrls = collection.mutable.Set[String](
"scala/collection/Map",
Expand All @@ -72,7 +76,7 @@ object Test extends ScaladocModelTest {
).map( _.split("#").toSeq ).map({
case Seq(one) => scalaURL + "/" + one + ".html"
case Seq(one, two) => scalaURL + "/" + one + ".html#" + two
})
}) ++ Set(s"$jdkURL/java/lang/Throwable.html")

def isExpectedExternalLink(l: EntityLink) = l.link match {
case LinkToExternalTpl(name, baseUrlString, tpl: TemplateEntity) =>
Expand All @@ -84,7 +88,7 @@ object Test extends ScaladocModelTest {
case _ => false
}

assert(countLinks(test.comment.get, isExpectedExternalLink) == 8,
"${countLinks(test.comment.get, isExpectedExternalLink)} == 8")
assert(countLinks(test.comment.get, isExpectedExternalLink) == 9,
"${countLinks(test.comment.get, isExpectedExternalLink)} == 9")
}
}