From 8e5004a05139e7cafa7422d4f64ecdba34bf512b Mon Sep 17 00:00:00 2001 From: Steven Barnes Date: Fri, 17 Jan 2020 20:16:42 -0800 Subject: [PATCH] Fix broken Scaladoc external Java references The problem was that PlainNioFile.underlyingSource was expanding "/modules/java.base" into a full path, where as the lookup in findExternalLink was using relative paths ("/modules/java.base.jmod") Added -jdk-api-doc-base for specifying the base URL for Java API references --- project/ScalaOptionParser.scala | 2 +- src/manual/scala/man1/scaladoc.scala | 6 +- .../scala/tools/nsc/doc/Settings.scala | 7 ++ .../tools/nsc/doc/model/MemberLookup.scala | 92 +++++++++++++++---- test/scaladoc/run/t191.scala | 16 ++-- 5 files changed, 98 insertions(+), 25 deletions(-) diff --git a/project/ScalaOptionParser.scala b/project/ScalaOptionParser.scala index 3bd7f4bbd2d7..3b8a1a4dc152 100644 --- a/project/ScalaOptionParser.scala +++ b/project/ScalaOptionParser.scala @@ -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") diff --git a/src/manual/scala/man1/scaladoc.scala b/src/manual/scala/man1/scaladoc.scala index 922b3d242aba..ed8bb0d1f291 100644 --- a/src/manual/scala/man1/scaladoc.scala +++ b/src/manual/scala/man1/scaladoc.scala @@ -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( diff --git a/src/scaladoc/scala/tools/nsc/doc/Settings.scala b/src/scaladoc/scala/tools/nsc/doc/Settings.scala index 393e30ea2239..0ec9db08353c 100644 --- a/src/scaladoc/scala/tools/nsc/doc/Settings.scala +++ b/src/scaladoc/scala/tools/nsc/doc/Settings.scala @@ -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", diff --git a/src/scaladoc/scala/tools/nsc/doc/model/MemberLookup.scala b/src/scaladoc/scala/tools/nsc/doc/model/MemberLookup.scala index cfaf5884162a..6b007462f2b6 100644 --- a/src/scaladoc/scala/tools/nsc/doc/model/MemberLookup.scala +++ b/src/scaladoc/scala/tools/nsc/doc/model/MemberLookup.scala @@ -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 { @@ -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 } diff --git a/test/scaladoc/run/t191.scala b/test/scaladoc/run/t191.scala index a50b715e8602..734c422ca3ca 100644 --- a/test/scaladoc/run/t191.scala +++ b/test/scaladoc/run/t191.scala @@ -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 :) @@ -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 @@ -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 = { @@ -50,7 +52,9 @@ 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) } @@ -58,7 +62,7 @@ object Test extends ScaladocModelTest { 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", @@ -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) => @@ -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") } }