Skip to content

Commit

Permalink
Merge pull request #8663 from steven-barnes/issue/11839
Browse files Browse the repository at this point in the history
Fix broken Scaladoc external references with JDK11
  • Loading branch information
SethTisue committed Apr 16, 2020
2 parents d433c3e + 8e5004a commit 98c1648
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 25 deletions.
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")
}
}

0 comments on commit 98c1648

Please sign in to comment.