Skip to content

Commit 60a0ed7

Browse files
committed
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.
1 parent d698cba commit 60a0ed7

File tree

11 files changed

+57
-14
lines changed

11 files changed

+57
-14
lines changed
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package _docs.tests
1+
package docs.tests
22

33
class Adoc:
44
def foo = 123
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
package example
2+
3+
package jekyllIncompat:
4+
object #~
5+
object ~>~

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ import java.io.PrintStream
1717
import scala.io.Codec
1818
import java.net.URL
1919
import scala.util.Try
20+
import scala.collection.mutable
21+
import dotty.tools.scaladoc.util.Check._
2022

2123
type CompilerContext = dotty.tools.dotc.core.Contexts.Context
2224

@@ -88,3 +90,18 @@ case class DocContext(args: Scaladoc.Args, compilerContext: CompilerContext):
8890

8991

9092
val externalDocumentationLinks = args.externalMappings
93+
94+
95+
// We collect and report any generated files incompatible with Jekyll
96+
private lazy val jekyllIncompatLinks = mutable.HashSet[String]()
97+
98+
def checkPathCompat(path: Seq[String]): Unit =
99+
if checkJekyllIncompatPath(path) then jekyllIncompatLinks.add(path.mkString("/"))
100+
101+
def reportPathCompatIssues(): Unit =
102+
if !jekyllIncompatLinks.isEmpty then
103+
report.warning(
104+
s"""Following generated file paths might not be compatible with Jekyll:
105+
|${jekyllIncompatLinks.mkString("\n")}
106+
|If using GitHub Pages consider adding a \".nojekyll\" file.
107+
""".stripMargin)(using compilerContext)

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,5 +247,6 @@ object Scaladoc:
247247
given docContext: DocContext = new DocContext(args, ctx)
248248
val module = ScalaModuleProvider.mkModule()
249249
new dotty.tools.scaladoc.renderers.HtmlRenderer(module.rootPackage, module.members).render()
250+
docContext.reportPathCompatIssues()
250251
report.inform("generation completed successfully")
251252
docContext

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ trait Locations(using ctx: DocContext):
4747
case "<empty>" :: tail => "_empty_" :: tail
4848
case other => other
4949
if ctx.args.apiSubdirectory then "api" :: fqn else fqn
50+
ctx.checkPathCompat(path)
5051
cache.put(dri, path)
5152
path
5253
case cached => cached

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ abstract class Renderer(rootPackage: Member, val members: Map[DRI, Member], prot
101101
val all = navigablePage +: redirectPages
102102
// We need to check for conflicts only if we have top-level member called docs
103103
val hasPotentialConflict =
104-
rootPackage.members.exists(m => m.name.startsWith("_docs"))
104+
rootPackage.members.exists(m => m.name.startsWith("docs"))
105105

106106
if hasPotentialConflict then
107107
def walk(page: Page): Unit =

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,12 @@ class StaticSiteContext(
2323
val docsPath = root.toPath.resolve("_docs")
2424
val blogPath = root.toPath.resolve("_blog")
2525

26-
val relativizeFrom = if args.apiSubdirectory then docsPath else root.toPath
26+
def relativize(path: Path): Path =
27+
if args.apiSubdirectory then
28+
docsPath.relativize(path)
29+
else
30+
val relativised = docsPath.relativize(path)
31+
Paths.get("docs").resolve(relativised)
2732

2833
val siteExtensions = Set(".html", ".md")
2934

@@ -91,7 +96,7 @@ class StaticSiteContext(
9196
}
9297

9398
def driFor(dest: Path): DRI =
94-
val rawFilePath = relativizeFrom.relativize(dest)
99+
val rawFilePath = relativize(dest)
95100
val pageName = dest.getFileName.toString
96101
val dotIndex = pageName.lastIndexOf('.')
97102

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package dotty.tools.scaladoc.util
2+
3+
object Check:
4+
/**
5+
* Jekyll (also used by GitHub Pages) by default makes a couple characters
6+
* illegal to use in file name beginnings.
7+
*/
8+
def checkJekyllIncompatPath(path: Seq[String]): Boolean =
9+
path.find( filename =>
10+
filename.matches("^~.*")
11+
|| filename.matches("^\\_.*")
12+
|| filename.matches("^\\..*")
13+
|| filename.matches("^\\#.*")
14+
).isDefined

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,11 @@ class ReportingTest:
7676
val docsRoot = testDocPath.resolve("conflicts-pages").toString
7777
checkReportedDiagnostics(_.copy(
7878
docsRoot = Some(docsRoot),
79-
tastyFiles = tastyFiles("tests", rootPck = "_docs")
79+
tastyFiles = tastyFiles("tests", rootPck = "docs")
8080
)){ diag =>
8181
assertNoWarning(diag)
8282
val Seq(msg) = diag.errorMsgs.map(_.toLowerCase)
83-
Seq("conflict","api", "static", "page", "_docs/tests/adoc.html")
83+
Seq("conflict","api", "static", "page", "docs/tests/adoc.html")
8484
.foreach( word =>
8585
Assert.assertTrue(s"Error message: $msg should contains $word", msg.contains(word))
8686
)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,5 +44,5 @@ class NavigationTest extends BaseHtmlTest:
4444
)),
4545
))
4646

47-
testNavMenu("_docs/Adoc.html", topLevelNav)
47+
testNavMenu("docs/Adoc.html", topLevelNav)
4848
}

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,13 @@ class SiteGeneratationTest extends BaseHtmlTest:
3232
}
3333

3434
def testDocPages()(using ProjectContext) =
35-
checkFile("_docs/Adoc.html")(title = "Adoc", header = "Header in Adoc", parents = Seq(projectName))
36-
checkFile("_docs/dir/index.html")(title = "A directory", header = "A directory", parents = Seq(projectName))
37-
checkFile("_docs/dir/nested.html")(
35+
checkFile("docs/Adoc.html")(title = "Adoc", header = "Header in Adoc", parents = Seq(projectName))
36+
checkFile("docs/dir/index.html")(title = "A directory", header = "A directory", parents = Seq(projectName))
37+
checkFile("docs/dir/nested.html")(
3838
title = "Nested in a directory", header = "Nested in a directory", parents = Seq(projectName, "A directory"))
3939

4040
def testDocIndexPage()(using ProjectContext) =
41-
checkFile("_docs/index.html")(title = projectName, header = s"$projectName in header")
41+
checkFile("docs/index.html")(title = projectName, header = s"$projectName in header")
4242

4343
def testApiPages(
4444
mainTitle: String = "API",
@@ -66,13 +66,13 @@ class SiteGeneratationTest extends BaseHtmlTest:
6666
testDocIndexPage()
6767
testApiPages()
6868

69-
withHtmlFile("_docs/Adoc.html"){ content =>
69+
withHtmlFile("docs/Adoc.html"){ content =>
7070
content.assertAttr("p a","href", "../tests/site/SomeClass.html")
7171
}
7272

7373
withHtmlFile("tests/site/SomeClass.html"){ content =>
7474
content.assertAttr(".breadcrumbs a","href",
75-
"../../_docs/index.html", "../../index.html", "../site.html", "SomeClass.html"
75+
"../../docs/index.html", "../../index.html", "../site.html", "SomeClass.html"
7676
)
7777
}
7878
}
@@ -98,7 +98,7 @@ class SiteGeneratationTest extends BaseHtmlTest:
9898
@Test
9999
def staticLinking() = withGeneratedSite(testDocPath.resolve("static-links")){
100100

101-
withHtmlFile("_docs/Adoc.html"){ content =>
101+
withHtmlFile("docs/Adoc.html"){ content =>
102102
content.assertAttr("p a","href",
103103
"dir/html.html",
104104
"dir/name...with..dots..html",

0 commit comments

Comments
 (0)