API doc directives: accept full character range in package names

According to the [Java Language Specification](https://docs.oracle.com/javase/specs/jls/se8/html/index.html) member of packages, e.g. subpackages are identifiers. Identifiers itself is ...an unlimited-length sequence of Java letters and Java digits, the first of which must be a Java letter. As a `Java letter` includes more than what is expressed by the regex `\b[a-z][a-z0-9_]*)\.`, package names which include uppercase or non ASCII characters cannot be processed. I know that there are some _naming conventions_ flying around but something like `aBc.DE.fg` is still a valid package name in my opinion. I have changed the regex in `packageDotsToSlash()` to `(\b\p{javaJavaIdentifierStart}\p{javaJavaIdentifierPart}*)\.`. Unfortunately this breaks with the changes applied for issue #397, #395, #98, #86 to handle inner classes. In order to support this notation, the regex can be relaxed to `(\b\p{javaLowerCase}\p{javaJavaIdentifierPart}*)\.`. I have added the variable 'scaladoc.strictPackageIdent' (which defaults to `false`) by which you can switch between the two regexp. Once set to true, the package name can conform to an identifier now.
lightbend · Apr 7, 2022 · 6f49d2b · 6f49d2b
1 parent 1a84ca8
commit 6f49d2b
Show file tree

Hide file tree

Showing 4 changed files with 242 additions and 21 deletions.
diff --git a/core/src/main/scala/com/lightbend/paradox/markdown/Directive.scala b/core/src/main/scala/com/lightbend/paradox/markdown/Directive.scala
@@ -312,25 +312,54 @@ abstract class ApiDocDirective(name: String)
 }
 
 object ApiDocDirective {
-  /** This relies on the naming convention of packages being all-ascii-lowercase (which is rarely broken), numbers and underscore. */
-  def packageDotsToSlash(s: String): String = s.replaceAll("(\\b[a-z][a-z0-9_]*)\\.", "$1/")
+  /**
+   * Converts package dot notation to a path, separated by '/'
+   * Allow all valid java characters and java numbers to be used, according to the java lang spec.
+   *
+   * @param s package or full qualified class name to be converted.
+   * @param packageNameStyle Setting `startWithLowercase`` will get it wrong when a package name
+   *                         starts with an uppercase letter or when an inner class starts with
+   *                         a lowercase character, while `startWithAnycase` will derive the wrong
+   *                         path whenever an inner class is encountered.
+   * @return Resulting path.
+   */
+  def packageDotsToSlash(s: String, packageNameStyle: String): String =
+    if (packageNameStyle == "startWithAnycase")
+      s.replaceAll("(\\b\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*)\\.", "$1/")
+    else
+      s.replaceAll("(\\b\\p{javaLowerCase}\\p{javaJavaIdentifierPart}*)\\.", "$1/")
+}
+
+object ScaladocDirective {
+  final val ScaladocPackageNameStyleProperty = raw"""scaladoc\.(.*)\.package_name_style""".r
+
+>>>>>>> 517023d (API doc directives: accept full character range in package names)
 }
 
 case class ScaladocDirective(ctx: Writer.Context)
   extends ApiDocDirective("scaladoc") {
 
+  import ScaladocDirective._
+
+  val defaultPackageNameStyle = variables.getOrElse("scaladoc.package_name_style", "startWithLowercase")
+  val packagePackageNameStyle: Map[String, String] = variables.collect {
+    case (property @ ScaladocPackageNameStyleProperty(pkg), url) => (pkg, variables(property))
+  }
+
   protected def resolveApiLink(link: String): Url = {
     val levels = link.split("[.]")
     val packages = (1 to levels.init.length).map(levels.take(_).mkString("."))
-    val baseUrl = packages.reverse.collectFirst(baseUrls).getOrElse(defaultBaseUrl).resolve()
-    url(link, baseUrl)
+    val packagesDeepestFirst = packages.reverse
+    val baseUrl = packagesDeepestFirst.collectFirst(baseUrls).getOrElse(defaultBaseUrl).resolve()
+    val packageNameStyle = packagesDeepestFirst.collectFirst(packagePackageNameStyle).getOrElse(defaultPackageNameStyle)
+    url(link, baseUrl, packageNameStyle)
   }
 
   private def classDotsToDollarDollar(s: String) = s.replaceAll("(\\b[A-Z].+)\\.", "$1\\$\\$")
 
-  private def url(link: String, baseUrl: Url): Url = {
+  private def url(link: String, baseUrl: Url, packageNameStyle: String): Url = {
     val url = Url(link).base
-    val path = classDotsToDollarDollar(ApiDocDirective.packageDotsToSlash(url.getPath)) + ".html"
+    val path = classDotsToDollarDollar(ApiDocDirective.packageDotsToSlash(url.getPath, packageNameStyle)) + ".html"
     (baseUrl / path).withFragment(url.getFragment)
   }
 
@@ -347,6 +376,7 @@ object JavadocDirective {
   // and including 8 starts with 1., so that's an easy way to tell if it's 9+ or not.
   val jdkDependentLinkStyle: LinkStyle = if (sys.props.get("java.specification.version").exists(_.startsWith("1."))) LinkStyleFrames else LinkStyleDirect
 
+<<<<<<< HEAD
   final val JavadocLinkStyleProperty: Regex = raw"""javadoc\.(.*).link_style""".r
 
   private[markdown] def url(link: String, baseUrl: Url, linkStyle: LinkStyle): Url = {
@@ -358,6 +388,9 @@ object JavadocDirective {
     }
   }
 
+  final val JavadocLinkStyleProperty = raw"""javadoc\.(.*).link_style""".r
+  final val JavadocPackageNameStyleProperty = raw"""javadoc\.(.*)\.package_name_style""".r
+
 }
 
 case class JavadocDirective(ctx: Writer.Context)
@@ -372,14 +405,31 @@ case class JavadocDirective(ctx: Writer.Context)
     case (property @ JavadocLinkStyleProperty(pkg), _) => (pkg, variables(property))
   }
 
+  val defaultPackageNameStyle = variables.getOrElse("javadoc.package_name_style", "startWithLowercase")
+  val packagePackageNameStyle: Map[String, String] = variables.collect {
+    case (property @ JavadocPackageNameStyleProperty(pkg), url) => (pkg, variables(property))
+  }
+
   override protected def resolveApiLink(link: String): Url = {
     val levels = link.split("[.]")
     val packages = (1 to levels.init.length).map(levels.take(_).mkString("."))
     val packagesDeepestFirst = packages.reverse
     val baseUrl = packagesDeepestFirst.collectFirst(baseUrls).getOrElse(defaultBaseUrl).resolve()
     val linkStyle = packagesDeepestFirst.collectFirst(packageLinkStyle).getOrElse(rootLinkStyle)
-    url(link, baseUrl, linkStyle)
+    val packageNameStyle = packagesDeepestFirst.collectFirst(packagePackageNameStyle).getOrElse(defaultPackageNameStyle)
+    url(link, baseUrl, linkStyle, packageNameStyle)
+
+  }
+
+  private def dollarDollarToClassDot(s: String) = s.replaceAll("\\$\\$", ".")
 
+  private[markdown] def url(link: String, baseUrl: Url, linkStyle: LinkStyle, packageNameStyle: String): Url = {
+    val url = Url(link).base
+    val path = dollarDollarToClassDot(ApiDocDirective.packageDotsToSlash(url.getPath, packageNameStyle)) + ".html"
+    linkStyle match {
+      case LinkStyleFrames => baseUrl.withEndingSlash.withQuery(path).withFragment(url.getFragment)
+      case LinkStyleDirect => (baseUrl / path).withFragment(url.getFragment)
+    }
   }
 }
 

diff --git a/docs/src/main/paradox/directives/linking.md b/docs/src/main/paradox/directives/linking.md
@@ -83,12 +83,46 @@ URL.
 
 The `@scaladoc` directive also supports site root relative base URLs using the `.../` syntax.
 
+The directive will identify inner classes and resolve a reference like
+`@scaladoc[Consumer.Control](akka.kafka.scaladsl.Consumer.Control)` to
+<https://doc.akka.io/api/alpakka-kafka/current/akka/kafka/scaladsl/Consumer$$Control.html>. 
+This is working fine as long as all (sub)package names are starting with a lowercase
+character while class names start with an uppercase character -- which is most often
+the case.
+
+In a situation where a (sub)package name starts with an uppercase character the
+reference is resolved incorrectly. This can be fixed by configuring the properties
+`scaladoc.<package-prefix>.package_name_style` or the default 
+`scaladoc.package_name_style` and set it to `startWithAnycase`.
+The directive will match the link text with the longest common package prefix
+and use the default style as a fall-back if nothing else matches. Keep in mind
+that the `OuterClass.InnerClass` notation is no longer working then and has
+to be replaced by `OuterClass$$InnerClass`.
+
+For example, given:
+
+```sbt
+paradoxProperties in Compile ++= Map(
+  //...
+  "scaladoc.com.example.package_name_style" -> s"startWithAnycase"
+)
+```
+
+```markdown
+ @scaladoc[SomeClass](com.example.Some.Library.SomeClass)
+ @scaladoc[Outer.Inner](com.example.Some.Library.Outer$$Inner)
+ @scaladoc[Consumer.Control](akka.kafka.scaladsl.Consumer.Control)
+```
+
+Then all are being resolved to the correct URL.
+
 @@@ Note
 
 The [sbt-paradox-apidoc](https://github.com/lightbend/sbt-paradox-apidoc) plugin creates `@scaladoc` and `@javadoc` API links by searching the class paths for the appropriate class to link to.
 
 @@@
 
+
 #### @javadoc directive
 
 Use the `@javadoc` directives to link to Javadoc sites based on the package
@@ -120,6 +154,42 @@ associated with the `java.specification.version` system property.
 
 The `@javadoc` directive also supports site root relative base URLs using the `.../` syntax.
 
+The directive will identify inner classes and resolve a reference like
+`@javadoc[Flow.Subscriber](java.util.concurrent.Flow.Subscriber)` to
+<https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/concurrent/Flow.Subscriber.html>. 
+This is working fine as long as all (sub)package names are starting with a lowercase
+character while class names start with an uppercase character -- which is most often
+the case.
+
+In a situation where a (sub)package name starts with an uppercase character the
+reference is resolved incorrectly. This can be fixed by configuring the properties
+`javadoc.<package-prefix>.package_name_style` or the default 
+`javadoc.package_name_style` and set it to `startWithAnycase`.
+The directive will match the link text with the longest common package prefix
+and use the default style as a fall-back if nothing else matches. Keep in mind
+that the `OuterClass.InnerClass` notation is no longer working then. In this case
+the class has to be referenced as `OuterClass$$InnerClass` which is being resolved
+back to the `.`-notation.
+
+For example, given:
+
+```sbt
+paradoxProperties in Compile ++= Map(
+  //...
+  "javadoc.com.example.package_name_style" -> s"startWithAnycase"
+)
+```
+
+```markdown
+ @javadoc[SomeClass](com.example.Some.Library.SomeClass)
+ @javadoc[Outer.Inner](com.example.Some.Library.Outer$$Inner)
+ @javadoc[outer.Inner](com.example.Some.Library.outer$$Inner)
+ @javadoc[Outer.inner](com.example.Some.Library.Outer$$inner)
+ @javadoc[Consumer.Control](akka.kafka.scaladsl.Consumer.Control)
+```
+
+Then all are being resolved to the correct URL.
+
 @@@ Note
 
 The [sbt-paradox-apidoc](https://github.com/lightbend/sbt-paradox-apidoc) plugin creates `@scaladoc` and `@javadoc` API links by searching the class paths for the appropriate class to link to.

diff --git a/tests/src/test/scala/com/lightbend/paradox/markdown/JavadocDirectiveSpec.scala b/tests/src/test/scala/com/lightbend/paradox/markdown/JavadocDirectiveSpec.scala
@@ -20,23 +20,53 @@ import com.lightbend.paradox.ParadoxException
 
 class JavadocDirectiveSpec extends MarkdownBaseSpec {
 
-  import JavadocDirective._
-
   implicit val context = writerContextWithProperties(
     "javadoc.base_url" -> "http://www.reactive-streams.org/reactive-streams-1.0.0-javadoc/",
     "javadoc.link_style" -> "frames",
     "javadoc.java.base_url" -> "https://docs.oracle.com/javase/8/docs/api/",
     "javadoc.akka.base_url" -> "http://doc.akka.io/japi/akka/2.4.10",
     "javadoc.akka.http.base_url" -> "http://doc.akka.io/japi/akka-http/10.0.0/index.html",
     "javadoc.root.relative.base_url" -> ".../javadoc/api/",
-    "javadoc.broken.base_url" -> "https://c|"
+    "javadoc.broken.base_url" -> "https://c|",
+    "javadoc.org.example.base_url" -> "http://example.org/api/0.1.2/"
   )
 
+  def renderedMd(url: String, title: String, name: String, prefix: String = "", suffix: String = "") =
+    html(Seq(prefix, """<p><a href="""", url, """" title="""", title, """"><code>""", name, """</code></a></p>""", suffix).mkString(""))
+
   "javadoc directive" should "create links using configured URL templates" in {
     markdown("@javadoc[Publisher](org.reactivestreams.Publisher)") shouldEqual
       html("""<p><a href="http://www.reactive-streams.org/reactive-streams-1.0.0-javadoc/?org/reactivestreams/Publisher.html" title="org.reactivestreams.Publisher"><code>Publisher</code></a></p>""")
   }
 
+  it should "create accept digits in package names" in {
+    markdown("@javadoc[ObjectMetadata](akka.s3.ObjectMetadata)") shouldEqual
+      renderedMd("http://doc.akka.io/japi/akka/2.4.10/?akka/s3/ObjectMetadata.html", "akka.s3.ObjectMetadata", "ObjectMetadata")
+  }
+
+  it should "create accept also non ascii characters (java letters) in package names" in {
+    markdown("@javadoc[S0meTHing](org.example.some.stränµè.ıãß.S0meTHing)") shouldEqual
+      renderedMd("http://example.org/api/0.1.2/?org/example/some/stränµè/ıãß/S0meTHing.html", "org.example.some.stränµè.ıãß.S0meTHing", "S0meTHing")
+  }
+
+  it should "create accept also non ascii characters (java letters) in class names" in {
+    markdown("@javadoc[Grüße](org.example.some.Grüße)") shouldEqual
+      renderedMd("http://example.org/api/0.1.2/?org/example/some/Grüße.html", "org.example.some.Grüße", "Grüße")
+  }
+
+  it should "create accept uppercase in package names" in {
+    markdown("@javadoc[S0meTHing](org.example.soME.stränµè.ıãß.S0meTHing)") shouldEqual
+      renderedMd("http://example.org/api/0.1.2/?org/example/soME/stränµè/ıãß/S0meTHing.html", "org.example.soME.stränµè.ıãß.S0meTHing", "S0meTHing")
+  }
+
+  it should "create accept subpackages starting with uppercase" in {
+    implicit val context = writerContextWithProperties(
+      "javadoc.package_name_style" -> "startWithAnycase",
+      "javadoc.org.example.base_url" -> "http://example.org/api/0.1.2/")
+    markdown("@javadoc[S0meTHing](org.example.soME.stränµè.ıãß.你好.S0meTHing)") shouldEqual
+      renderedMd("http://example.org/api/0.1.2/?org/example/soME/stränµè/ıãß/你好/S0meTHing.html", "org.example.soME.stränµè.ıãß.你好.S0meTHing", "S0meTHing")
+  }
+
   it should "support 'javadoc:' as an alternative name" in {
     markdown("@javadoc:[Publisher](org.reactivestreams.Publisher)") shouldEqual
       html("""<p><a href="http://www.reactive-streams.org/reactive-streams-1.0.0-javadoc/?org/reactivestreams/Publisher.html" title="org.reactivestreams.Publisher"><code>Publisher</code></a></p>""")
@@ -118,19 +148,42 @@ class JavadocDirectiveSpec extends MarkdownBaseSpec {
   }
 
   it should "correctly link to an inner JRE class" in {
-    url(
-      "java.util.concurrent.Flow.Subscriber",
-      Url("https://docs.oracle.com/en/java/javase/11/docs/api/java.base/"),
-      LinkStyleDirect
-    ) should be(Url("https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/concurrent/Flow.Subscriber.html"))
+    val ctx = context.andThen(c => c.copy(properties = c.properties
+      .updated("javadoc.java.link_style", "direct")
+      .updated("javadoc.java.base_url", "https://docs.oracle.com/en/java/javase/11/docs/api/java.base/")
+    ))
+    markdown("@javadoc:[Flow.Subscriber](java.util.concurrent.Flow.Subscriber)")(ctx) shouldEqual
+      html("""<p><a href="https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/concurrent/Flow.Subscriber.html" title="java.util.concurrent.Flow.Subscriber"><code>Flow.Subscriber</code></a></p>""")
   }
 
   it should "correctly link to an inner Akka class" in {
-    url(
-      "akka.actor.testkit.typed.Effect.MessageAdapter",
-      Url("https://doc.akka.io/japi/akka/current/"),
-      LinkStyleDirect
-    ) should be(Url("https://doc.akka.io/japi/akka/current/akka/actor/testkit/typed/Effect.MessageAdapter.html"))
+    val ctx = context.andThen(c => c.copy(properties = c.properties
+      .updated("javadoc.akka.link_style", "direct")
+      .updated("javadoc.akka.base_url", "https://doc.akka.io/japi/akka/current/")
+    ))
+    markdown("@javadoc:[Effect.MessageAdapter](akka.actor.testkit.typed.Effect.MessageAdapter)")(ctx) shouldEqual
+      html("""<p><a href="https://doc.akka.io/japi/akka/current/akka/actor/testkit/typed/Effect.MessageAdapter.html" title="akka.actor.testkit.typed.Effect.MessageAdapter"><code>Effect.MessageAdapter</code></a></p>""")
+  }
+
+  it should "correctly link to an inner class if a subpackage starts with an uppercase character" in {
+    val ctx = context.andThen(c => c.copy(properties = c.properties
+      .updated("javadoc.org.example.package_name_style", "startWithAnycase")
+    ))
+    markdown("@javadoc:[Outer.Inner](org.example.Lib.Outer$$Inner)")(ctx) shouldEqual
+      renderedMd("http://example.org/api/0.1.2/?org/example/Lib/Outer.Inner.html", "org.example.Lib.Outer.Inner", "Outer.Inner")
+  }
+
+  it should "correctly link to an inner class if the outer class starts with a lowercase character" in {
+    markdown("@javadoc:[outer.Inner](org.example.lib.outer$$Inner)") shouldEqual
+      renderedMd("http://example.org/api/0.1.2/?org/example/lib/outer.Inner.html", "org.example.lib.outer.Inner", "outer.Inner")
+  }
+
+  it should "correctly link to an inner class if the inner class starts with a lowercase character" in {
+    val ctx = context.andThen(c => c.copy(properties = c.properties
+      .updated("javadoc.org.example.package_name_style", "startWithAnycase")
+    ))
+    markdown("@javadoc:[Outer.inner](org.example.lib.Outer$$inner)")(ctx) shouldEqual
+      renderedMd("http://example.org/api/0.1.2/?org/example/lib/Outer.inner.html", "org.example.lib.Outer.inner", "Outer.inner")
   }
 
 }