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

scaladoc -doc-external-uris requires incorrect file URI format #6803

Closed
scabug opened this issue Dec 12, 2012 · 3 comments
Closed

scaladoc -doc-external-uris requires incorrect file URI format #6803

scabug opened this issue Dec 12, 2012 · 3 comments
Assignees
Milestone

Comments

@scabug
Copy link

@scabug scabug commented Dec 12, 2012

The syntax for -doc-external-uris is:

  -doc-external-uris:<external-doc>  comma-separated list of file://classpath_entry_path#doc_URL URIs for external dependencies

The test for this feature is broken because it incorrectly constructs a file URI:

https://github.com/scala/scala/blob/master/test/scaladoc/run/SI-191.scala#L34

  override def scaladocSettings = {
    val scalaLibUri = getClass.getClassLoader.getResource("scala/Function1.class").toURI.getSchemeSpecificPart.split("!")(0)
    val scalaLib = new File(new URL(scalaLibUri).getPath).getPath
    val extArg = new URI("file", scalaLib, scalaURL).toString
    "-no-link-warnings -doc-external-uris " + extArg
  }

The scheme specific part of a local file URI is "//" + file.getPath, not just file.getPath. Because the code compares the scheme specific part of the user-provided URI against the path, this will not match the specified file URI in practice.

https://github.com/scala/scala/blob/master/src/compiler/scala/tools/nsc/doc/Settings.scala#L266

  lazy val extUrlMapping: Map[String, String] = docExternalUris.value map { s =>
    val uri = new URI(s)
    uri.getSchemeSpecificPart -> appendIndex(uri.getFragment)
  } toMap

https://github.com/scala/scala/blob/master/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala#L1040

    Option(sym1.associatedFile) flatMap (_.underlyingSource) flatMap { src =>
      val path = src.path
      settings.extUrlMapping get path map { url =>
        LinkToExternal(name, url + "#" + name)
      }
    }

Given that the scheme of the classpath entry URI is ignored and assumed to be file:, File<->URI/URL conversions are tricky due to things like Windows UNC filenames, and the -doc-external-uris feature doesn't currently work as specified, I think the proper fix is to change the syntax and not go through URIs or URLs at all. Because users can figure out that an incorrect file URI like file:/home/user/demo.jar is accepted, it might be considered a breaking change, though.

@scabug
Copy link
Author

@scabug scabug commented Dec 12, 2012

Imported From: https://issues.scala-lang.org/browse/SI-6803?orig=1
Reporter: @harrah
Affected Versions: 2.10.1-RC1

Loading

@scabug
Copy link
Author

@scabug scabug commented Dec 18, 2012

@vigdorchik said:
Since this feature is only available in 2.10.1 it's not too late to change the implementation. Thank you for bringing this up.

Loading

@scabug
Copy link
Author

@scabug scabug commented Jan 8, 2013

Loading

@scabug scabug closed this Jan 8, 2013
@scabug scabug added this to the 2.10.1 milestone Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants