Add file path escaping to Scaladoc

As Jekyll (and in extension GitHub pages) makes starting a file/folder
name with a couple characters illegal, an additional check is added for
those and problematic paths are reported.
To avoid always having "_docs" if not using -Yapi_subdirectory in url by
default in the static site, we also replace that with "docs".
Some tests concerning links were adjusted to accomodate the changes.
Example testcase for illegal jekyll chars in Scaladoc was also added.

Author: jchyb

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",