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)
   }

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

@@ -32,13 +32,13 @@ class SiteGeneratationTest extends BaseHtmlTest:
       }
 
   def testDocPages()(using ProjectContext) =
-    checkFile("_docs/Adoc.html")(title = "Adoc", header = "Header in Adoc", parents = Seq(projectName))
-    checkFile("_docs/dir/index.html")(title = "A directory", header = "A directory", parents = Seq(projectName))
-    checkFile("_docs/dir/nested.html")(
+    checkFile("docs/Adoc.html")(title = "Adoc", header = "Header in Adoc", parents = Seq(projectName))
+    checkFile("docs/dir/index.html")(title = "A directory", header = "A directory", parents = Seq(projectName))
+    checkFile("docs/dir/nested.html")(
       title = "Nested in a directory", header = "Nested in a directory", parents = Seq(projectName, "A directory"))
 
   def testDocIndexPage()(using ProjectContext) =
-    checkFile("_docs/index.html")(title = projectName, header = s"$projectName in header")
+    checkFile("docs/index.html")(title = projectName, header = s"$projectName in header")
 
   def testApiPages(
     mainTitle: String = "API",
@@ -66,13 +66,13 @@ class SiteGeneratationTest extends BaseHtmlTest:
     testDocIndexPage()
     testApiPages()
 
-    withHtmlFile("_docs/Adoc.html"){ content  =>
+    withHtmlFile("docs/Adoc.html"){ content  =>
         content.assertAttr("p a","href", "../tests/site/SomeClass.html")
     }
 
     withHtmlFile("tests/site/SomeClass.html"){ content  =>
       content.assertAttr(".breadcrumbs a","href",
-        "../../_docs/index.html", "../../index.html", "../site.html", "SomeClass.html"
+        "../../docs/index.html", "../../index.html", "../site.html", "SomeClass.html"
       )
     }
   }
@@ -98,7 +98,7 @@ class SiteGeneratationTest extends BaseHtmlTest:
   @Test
   def staticLinking() = withGeneratedSite(testDocPath.resolve("static-links")){
 
-    withHtmlFile("_docs/Adoc.html"){ content  =>
+    withHtmlFile("docs/Adoc.html"){ content  =>
         content.assertAttr("p a","href",
         "dir/html.html",
         "dir/name...with..dots..html",