From d3ce1295e829df5bc122b517b64cd4da920f1216 Mon Sep 17 00:00:00 2001 From: Jan Chyb Date: Thu, 10 Mar 2022 09:20:43 +0100 Subject: [PATCH] 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. --- .../src/{_docs => docs}/tests/Adoc.scala | 2 +- .../src/example/JekyllIncompat.scala | 5 +++++ .../src/dotty/tools/scaladoc/DocContext.scala | 17 +++++++++++++++++ .../src/dotty/tools/scaladoc/Scaladoc.scala | 1 + .../tools/scaladoc/renderers/Locations.scala | 1 + .../tools/scaladoc/renderers/Renderer.scala | 2 +- .../tools/scaladoc/site/StaticSiteContext.scala | 11 ++++++++--- .../src/dotty/tools/scaladoc/util/check.scala | 14 ++++++++++++++ .../dotty/tools/scaladoc/ReportingTest.scala | 4 ++-- .../tools/scaladoc/site/NavigationTest.scala | 2 +- .../scaladoc/site/SiteGeneratationTest.scala | 14 +++++++------- 11 files changed, 58 insertions(+), 15 deletions(-) rename scaladoc-testcases/src/{_docs => docs}/tests/Adoc.scala (59%) create mode 100644 scaladoc-testcases/src/example/JekyllIncompat.scala create mode 100644 scaladoc/src/dotty/tools/scaladoc/util/check.scala diff --git a/scaladoc-testcases/src/_docs/tests/Adoc.scala b/scaladoc-testcases/src/docs/tests/Adoc.scala similarity index 59% rename from scaladoc-testcases/src/_docs/tests/Adoc.scala rename to scaladoc-testcases/src/docs/tests/Adoc.scala index 04b2c3918023..af93f9eddfa0 100644 --- a/scaladoc-testcases/src/_docs/tests/Adoc.scala +++ b/scaladoc-testcases/src/docs/tests/Adoc.scala @@ -1,4 +1,4 @@ -package _docs.tests +package docs.tests class Adoc: def foo = 123 diff --git a/scaladoc-testcases/src/example/JekyllIncompat.scala b/scaladoc-testcases/src/example/JekyllIncompat.scala new file mode 100644 index 000000000000..65c735804d57 --- /dev/null +++ b/scaladoc-testcases/src/example/JekyllIncompat.scala @@ -0,0 +1,5 @@ +package example + +package jekyllIncompat: + object #~ + object ~>~ diff --git a/scaladoc/src/dotty/tools/scaladoc/DocContext.scala b/scaladoc/src/dotty/tools/scaladoc/DocContext.scala index 64c072aa3de3..366b5b8c162c 100644 --- a/scaladoc/src/dotty/tools/scaladoc/DocContext.scala +++ b/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) diff --git a/scaladoc/src/dotty/tools/scaladoc/Scaladoc.scala b/scaladoc/src/dotty/tools/scaladoc/Scaladoc.scala index 0b746f19035b..9d263441ace1 100644 --- a/scaladoc/src/dotty/tools/scaladoc/Scaladoc.scala +++ b/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 diff --git a/scaladoc/src/dotty/tools/scaladoc/renderers/Locations.scala b/scaladoc/src/dotty/tools/scaladoc/renderers/Locations.scala index 9490693fb759..12615671e538 100644 --- a/scaladoc/src/dotty/tools/scaladoc/renderers/Locations.scala +++ b/scaladoc/src/dotty/tools/scaladoc/renderers/Locations.scala @@ -47,6 +47,7 @@ trait Locations(using ctx: DocContext): case "" :: 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 diff --git a/scaladoc/src/dotty/tools/scaladoc/renderers/Renderer.scala b/scaladoc/src/dotty/tools/scaladoc/renderers/Renderer.scala index 6d177e113c24..4ce81450afbb 100644 --- a/scaladoc/src/dotty/tools/scaladoc/renderers/Renderer.scala +++ b/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 = diff --git a/scaladoc/src/dotty/tools/scaladoc/site/StaticSiteContext.scala b/scaladoc/src/dotty/tools/scaladoc/site/StaticSiteContext.scala index 48c2acb745af..51ee9e30e933 100644 --- a/scaladoc/src/dotty/tools/scaladoc/site/StaticSiteContext.scala +++ b/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('.') diff --git a/scaladoc/src/dotty/tools/scaladoc/util/check.scala b/scaladoc/src/dotty/tools/scaladoc/util/check.scala new file mode 100644 index 000000000000..cbbeaded312e --- /dev/null +++ b/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 \ No newline at end of file diff --git a/scaladoc/test/dotty/tools/scaladoc/ReportingTest.scala b/scaladoc/test/dotty/tools/scaladoc/ReportingTest.scala index e64eac41ac66..fc215b1de229 100644 --- a/scaladoc/test/dotty/tools/scaladoc/ReportingTest.scala +++ b/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)) ) diff --git a/scaladoc/test/dotty/tools/scaladoc/site/NavigationTest.scala b/scaladoc/test/dotty/tools/scaladoc/site/NavigationTest.scala index 39bd7acf84f6..0a2d1891a3cc 100644 --- a/scaladoc/test/dotty/tools/scaladoc/site/NavigationTest.scala +++ b/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) } diff --git a/scaladoc/test/dotty/tools/scaladoc/site/SiteGeneratationTest.scala b/scaladoc/test/dotty/tools/scaladoc/site/SiteGeneratationTest.scala index 1b3c53aba3c0..c46875c7d0cd 100644 --- a/scaladoc/test/dotty/tools/scaladoc/site/SiteGeneratationTest.scala +++ b/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",