Merge pull request #14657 from jchyb/scaladoc/jekyll-escapes

Scaladoc: Warn about special characters in filenames according to the default Jekyll rules

Author: pikinier20

scaladoc-testcases/src/_docs/tests/Adoc.scala renamed to scaladoc-testcases/src/docs/tests/Adoc.scala

@@ -1,4 +1,4 @@
-package _docs.tests
+package docs.tests
 
 class Adoc:
   def foo = 123

scaladoc-testcases/src/example/JekyllIncompat.scala

@@ -0,0 +1,5 @@
+package example
+
+package jekyllIncompat:
+  object #~
+  object ~>~

scaladoc/src/dotty/tools/scaladoc/DocContext.scala

@@ -17,6 +17,8 @@ import java.io.PrintStream
 import scala.io.Codec
 import java.net.URL
 import scala.util.Try
+import scala.collection.mutable
+import dotty.tools.scaladoc.util.Check.checkJekyllIncompatPath
 
 type CompilerContext = dotty.tools.dotc.core.Contexts.Context
 
@@ -90,3 +92,18 @@ case class DocContext(args: Scaladoc.Args, compilerContext: CompilerContext):
 
 
   val externalDocumentationLinks = args.externalMappings
+
+
+  // We collect and report any generated files incompatible with Jekyll
+  private lazy val jekyllIncompatLinks = mutable.HashSet[String]()
+
+  def checkPathCompat(path: Seq[String]): Unit =
+    if checkJekyllIncompatPath(path) then jekyllIncompatLinks.add(path.mkString("/"))
+
+  def reportPathCompatIssues(): Unit =
+    if !jekyllIncompatLinks.isEmpty then
+      report.warning(
+        s"""Following generated file paths might not be compatible with Jekyll:
+          |${jekyllIncompatLinks.mkString("\n")}
+          |If using GitHub Pages consider adding a \".nojekyll\" file.
+        """.stripMargin)(using compilerContext)

scaladoc/src/dotty/tools/scaladoc/Scaladoc.scala

@@ -229,5 +229,6 @@ object Scaladoc:
     given docContext: DocContext = new DocContext(args, ctx)
     val module = ScalaModuleProvider.mkModule()
     new dotty.tools.scaladoc.renderers.HtmlRenderer(module.rootPackage, module.members).render()
+    docContext.reportPathCompatIssues()
     report.inform("generation completed successfully")
     docContext

scaladoc/src/dotty/tools/scaladoc/renderers/Locations.scala

@@ -47,6 +47,7 @@ trait Locations(using ctx: DocContext):
               case "<empty>" :: tail => "_empty_" :: tail
               case other => other
             if ctx.args.apiSubdirectory then "api" :: fqn else fqn
+        ctx.checkPathCompat(path)
         cache.put(dri, path)
         path
       case cached => cached

scaladoc/src/dotty/tools/scaladoc/renderers/Renderer.scala

@@ -120,7 +120,7 @@ abstract class Renderer(rootPackage: Member, val members: Map[DRI, Member], prot
     val all = navigablePage +: redirectPages
     // We need to check for conflicts only if we have top-level member called docs
     val hasPotentialConflict =
-      rootPackage.members.exists(m => m.name.startsWith("_docs"))
+      rootPackage.members.exists(m => m.name.startsWith("docs"))
 
     if hasPotentialConflict then
       def walk(page: Page): Unit =

scaladoc/src/dotty/tools/scaladoc/site/StaticSiteContext.scala

@@ -23,7 +23,12 @@ class StaticSiteContext(
   val docsPath = root.toPath.resolve("_docs")
   val blogPath = root.toPath.resolve("_blog")
 
-  val relativizeFrom = if args.apiSubdirectory then docsPath else root.toPath
+  def relativize(path: Path): Path = 
+    if args.apiSubdirectory then 
+      docsPath.relativize(path)
+    else
+      val relativised = docsPath.relativize(path)
+      Paths.get("docs").resolve(relativised)
 
   val siteExtensions = Set(".html", ".md")
 
@@ -47,7 +52,7 @@ class StaticSiteContext(
       val redirectFrom = loadedTemplate.templateFile.settings.getOrElse("page", Map.empty).asInstanceOf[Map[String, Object]].get("redirectFrom")
       def redirectToTemplate(redirectFrom: String) =
         val path = if redirectFrom.startsWith("/")
-          then relativizeFrom.resolve(redirectFrom.drop(1))
+          then docsPath.resolve(redirectFrom.drop(1))
           else loadedTemplate.file.toPath.resolveSibling(redirectFrom)
         val driFrom = driFor(path)
         val driTo = driFor(loadedTemplate.file.toPath)
@@ -93,7 +98,7 @@ class StaticSiteContext(
     }
 
   def driFor(dest: Path): DRI =
-    val rawFilePath = relativizeFrom.relativize(dest)
+    val rawFilePath = relativize(dest)
     val pageName = dest.getFileName.toString
     val dotIndex = pageName.lastIndexOf('.')
 

scaladoc/src/dotty/tools/scaladoc/util/check.scala

@@ -0,0 +1,14 @@
+package dotty.tools.scaladoc.util
+
+object Check:
+  /**  
+   *  Jekyll (also used by GitHub Pages) by default makes a couple characters 
+   *  illegal to use in file name beginnings.
+   */
+  def checkJekyllIncompatPath(path: Seq[String]): Boolean =
+    path.find( filename =>
+      filename.matches("^~.*")
+       || filename.matches("^\\_.*")
+       || filename.matches("^\\..*")
+       || filename.matches("^\\#.*")
+    ).isDefined

scaladoc/test/dotty/tools/scaladoc/ReportingTest.scala

@@ -76,11 +76,11 @@ class ReportingTest:
     val docsRoot = testDocPath.resolve("conflicts-pages").toString
     checkReportedDiagnostics(_.copy(
       docsRoot = Some(docsRoot),
-      tastyFiles = tastyFiles("tests", rootPck = "_docs")
+      tastyFiles = tastyFiles("tests", rootPck = "docs")
     )){ diag =>
       assertNoWarning(diag)
       val Seq(msg) = diag.errorMsgs.map(_.toLowerCase)
-      Seq("conflict","api", "static", "page", "_docs/tests/adoc.html")
+      Seq("conflict","api", "static", "page", "docs/tests/adoc.html")
       .foreach( word =>
           Assert.assertTrue(s"Error message: $msg should contains $word", msg.contains(word))
         )

scaladoc/test/dotty/tools/scaladoc/site/NavigationTest.scala

@@ -44,5 +44,5 @@ class NavigationTest extends BaseHtmlTest:
       )),
     ))
 
-    testNavMenu("_docs/Adoc.html", topLevelNav)
+    testNavMenu("docs/Adoc.html", topLevelNav)
   }