Scaladoc external references JDK11

[ashawley]

Linking to Javadoc with JDK11 doesn't work in Scala 2.13.0-RC1 and later.

This is a build.sbt:

scalaVersion := "2.13.1"

scalacOptions in (Compile, doc) += "-Xfatal-warnings"

apiMappings ++= {
  Map(
    scalaInstance.value.libraryJar
      -> url(s"http://www.scala-lang.org/api/${scalaVersion.value}/")
  ) ++ Map(
    file("/modules/java.base")
      -> url("https://docs.oracle.com/en/java/javase/11/docs/api/java.base"),
    file("/modules/java.xml")
      -> url("https://docs.oracle.com/en/java/javase/11/docs/api/java.xml")
  )
}

A simple Scala file, A.scala:

/**
 * [[java.lang.Throwable]]
 * [[scala.AnyRef]]
 */
class A

Running doc in sbt:

[error] A.scala:1:1: Could not find any member to link for "java.lang.Throwable".
[error] /**
[error] ^
[error] one error found
[error] (Compile / doc) Scaladoc generation failed

Repository with minimal example:

https://github.com/ashawley/scala-issue-11839

[ashawley]

A git bisect tracks it down to scala/scala#7782

It could be.

There is a reference to underlyingSource in scaladoc:

https://github.com/scala/scala/blob/2d9e6ac/src/scaladoc/scala/tools/nsc/doc/model/MemberLookup.scala#L61

[ashawley]

This is a difficult problem to work on because there isn't a good testing story with Scaladoc in the Scala build, and even more the case for JDK11 issues.

[ashawley]

% sbt -Dstarr.version=2.13.2-bin-SNAPSHOT

Oh, the starr.version trick will only work in the sbt build for Scala, and not for any sbt project, including my example repo.

[steven-barnes]

Is the lack of unit tests laziness, or difficulty?

[ashawley]

Yes, it is perhaps both. I'd phrase it that the Scaladoc is not a priority compared to the compiler or the standard library. However, it is also a complex tool that is difficult to test.

[steven-barnes]

Alright, another attempt. steven-barnes/scala@d2ec0a5

I can attempt to write tests for some of this. Also test suites could be run with different JDKs, using a tool such as Jenkins

[ashawley]

@retronym wrote the following proposal in scala/scala#8647:

Thanks for contributing a fix here. I'll see if I can help reconcile different uses of underlyingSource to help figure this out.

First of all, I was curious how Javadoc users deal with external links into Java SDK API docs. Looks like the Maven Javadoc plugin has is clever enough to automatically configure the base URL of the SDK docs and also ships with config files (example) that map packages (e.g. java.util.function to the defining JPMS module (e.g java.base). 💯 for usability, no manual mapping configuration needed by build tool users.

Okay, as to testing, we do have functional tests that exercise external links in Scaladoc, such as https://github.com/scala/scala/blob/2.13.x/test/scaladoc/run/t191.scala. That seems focused on linking to the Scala SDK docs. But it could be extended or duplicated to test linking to Java SDK docs.

Our CI builds currently do not run on a multiple versions of Java. We'll add Java 11 to the mix soon. Tests that are Java version dependent (ie, should only run on Java 9+, or have assert different things on different Java versions) can be usually be expressed by adding logic to the test. You can then at least run the test manually on Java 11 on your workstation, even though CI won't exercise those paths.

AbstractFile.underlyingSource was originally a way to navigate from a link to a JAR entry to the JAR itself. Java 9 changes things a bit here by removing rt.jar in favour of the virtual jrt:// virtual file system. Zinc (the Scala incremental compiler used in SBT and elsewhere) uses this map the references from compiled classed to JARs on the classpath. I figured the least-bad option for jrt:// backed classes was to have underlyingSource return the .jmod file -- if that was updated or removed (however unliikely that is for a JDK install) Zinc ought to detect the change.

But I didn't consider that this path would need to be exposed to Scaladoc users trying valiantly to configure external URL mappings. The prior answer of "/modules/java.base" was a bit nicer still somewhat confusing to Scaladoc users IMO as it looks like an on-disk path that doesn't actually exist.

How about we add a new Scaladoc option --jdk-api-doc-base that the user could configure to https://docs.oracle.com/en/java/javase/$N/docs/api. findExternalLink could then have a special case when it detects that a symbol is part of the JDK:

val isJDK = isChildOf(sym.associatedFile.underlyingSource, sys.props("java.home"))

It could then detect the JPMS module of the symbol (I can help with the details of this) and construct the correct URL from the base.

WDYT?

[ashawley]

The current proposal by Jason above for -jdk-api-doc-base might fix links for the JDK specifically, but might not be a solution for Java modules generally. My bug is about linking to code in any Java module, not just the java.base module. The JDK even contains more than one module.

The apiMappings setting in sbt is a Map[File,URL] that can support multiple mappings for this reason. I'm reading the proposal is for a single tuple of (File, URL), or maybe a command-line arg for a single URL? That would only fix Scaladoc links to the JDK base module.

[SethTisue]

leaving the issue open because the linked PR (merged now for 2.13.2) only supports mappings to the java.base module, so we're not all the way to a complete fix yet

[steven-barnes]

It has been a while, but I believe it works with any JDK module.

…

Sent from my iPhone

On Apr 16, 2020, at 6:55 AM, Seth Tisue ***@***.***> wrote:  leaving the issue open because the linked PR only supports mappings to the java.base module, so we're not all the way to a complete fix yet — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[SethTisue]

@ashawley also didn't seem completely sure one way or the other. can somebody actually confirm?

[ashawley]

I'm suggesting Scaladoc should support multiple mappings to modules, not just one mapping.

[steven-barnes]

I have a lot of time on my hands now. If you write up a spec, then I can implement it. Or work on some other issue.

…

On Apr 16, 2020, at 7:29 AM, Aaron S. Hawley ***@***.***> wrote: I'm suggesting Scaladoc should support multiple mappings to modules, not just one mapping. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#11839 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJFBLGUXDRMVEQ5R77VPCE3RM4I3JANCNFSM4KAFOAGQ>.

[ashawley]

It would be nice to have the apiMappings work as it used to in 2.12 (see bug description and the example code repo). Whether that's even possible after the change in 16a3216, I'm not sure.

[eed3si9n]

There a report #11955 that says Scala 2.13.2 produces "java.net.URISyntaxException: Illegal character in path at index 53: https://docs.oracle.com/en/java/javase/11/docs/api/C:\opt\jdk\jmods\java.base/java/io/Serializable.html".

Not sure if OP is using apiMappings, but it seems like java.base isn't working?

[steven-barnes]

I can look at this, however I do not have a Windows machine.

…

On Apr 24, 2020, at 11:05 AM, eugene yokota ***@***.***> wrote: There a report #11955 <#11955> that says Scala 2.13.2 produces "java.net.URISyntaxException: Illegal character in path at index 53: https://docs.oracle.com/en/java/javase/11/docs/api/C:\opt\jdk\jmods\java.base/java/io/Serializable.html <https://docs.oracle.com/en/java/javase/11/docs/api/C:%5Copt%5Cjdk%5Cjmods%5Cjava.base/java/io/Serializable.html>". Not sure if OP is using apiMappings, but it seems like java.base isn't working? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#11839 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJFBLGUC7MKBHLLJFQFUR6LROHIHBANCNFSM4KAFOAGQ>.

[steven-barnes]

Eyeballing the code, this is probably the issue:
val tokens = path.split("/")

This needs to be "\" on Windows. java.io.File.separator would work

My apologies, but lacking a Windows machine, I cannot test this

[martijnhoekstra]

Eyeballing the code, this is probably the issue:
val tokens = path.split("/")

This needs to be "" on Windows. java.io.File.separator would work

My apologies, but lacking a Windows machine, I cannot test this

Careful, split takes a regex. You need to quote it.

[steven-barnes]

Ah yes, the famous quadruple-backslash regex. Thanks for catching that.

[SethTisue]

fixed by scala/scala#8663