Merge pull request #13130 from romanowski/scaladoc/api-layout

Scaladoc generate flatten structure for API rather then put everything in `api` directory

Author: romanowski

project/Build.scala

@@ -1325,6 +1325,7 @@ object Build {
       generateScalaDocumentation := Def.inputTaskDyn {
         val extraArgs = spaceDelimited("[output]").parsed
         val dest = file(extraArgs.headOption.getOrElse("scaladoc/output/scala3")).getAbsoluteFile
+        val justAPI = extraArgs.drop(1).headOption == Some("--justAPI")
         val majorVersion = (LocalProject("scala3-library-bootstrapped") / scalaBinaryVersion).value
 
         val dottyJars: Seq[java.io.File] = Seq(
@@ -1346,21 +1347,24 @@ object Build {
 
         val dottyLibRoot = projectRoot.relativize(dottyManagesSources.toPath.normalize())
 
+        def generateDocTask =
+          generateDocumentation(
+            roots, "Scala 3", dest.getAbsolutePath, "master",
+            Seq(
+              "-comment-syntax", "wiki",
+              s"-source-links:docs=github://lampepfl/dotty/master#docs",
+              "-doc-root-content", docRootFile.toString,
+              "-Ydocument-synthetic-types"
+            ) ++ (if (justAPI) Nil else Seq("-siteroot", "docs", "-Yapi-subdirectory")))
+
         if (dottyJars.isEmpty) Def.task { streams.value.log.error("Dotty lib wasn't found") }
+        else if (justAPI) generateDocTask
         else Def.task{
           IO.write(dest / "versions" / "latest-nightly-base", majorVersion)
 
           // This file is used by GitHub Pages when the page is available in a custom domain
           IO.write(dest / "CNAME", "dotty.epfl.ch")
-        }.dependsOn(generateDocumentation(
-          roots, "Scala 3", dest.getAbsolutePath, "master",
-          Seq(
-            "-comment-syntax", "wiki",
-            "-siteroot", "docs",
-            s"-source-links:docs=github://lampepfl/dotty/master#docs",
-            "-doc-root-content", docRootFile.toString,
-            "-Ydocument-synthetic-types"
-          )))
+        }.dependsOn(generateDocTask)
       }.evaluated,
 
       generateTestcasesDocumentation := Def.taskDyn {

sbt-test/sbt-dotty/scaladoc/build.sbt

@@ -6,6 +6,6 @@ TaskKey[Unit]("checkScaladocOptions") := {
 }
 
 TaskKey[Unit]("checkHtmlFiles") := {
-  val helloHtml = (Compile / doc / target).value / "api" / "hello.html"
+  val helloHtml = (Compile / doc / target).value / "hello.html"
   assert(helloHtml.exists)
 }

scaladoc-testcases/src/docs/tests/Adoc.scala

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

scaladoc-testcases/src/resources/tests/Adoc.scala

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

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

@@ -58,7 +58,8 @@ object Scaladoc:
     snippetCompilerDebug: Boolean = false,
     noLinkWarnings: Boolean = false,
     versionsDictionaryUrl: Option[String] = None,
-    generateInkuire : Boolean = false
+    generateInkuire : Boolean = false,
+    apiSubdirectory : Boolean = false
   )
 
   def run(args: Array[String], rootContext: CompilerContext): Reporter =
@@ -223,7 +224,8 @@ object Scaladoc:
         noLinkWarnings.get,
         snippetCompilerDebug.get,
         versionsDictionaryUrl.nonDefault,
-        generateInkuire.get
+        generateInkuire.get,
+        apiSubdirectory.get,
       )
       (Some(docArgs), newContext)
     }

scaladoc/src/dotty/tools/scaladoc/ScaladocSettings.scala

@@ -120,5 +120,8 @@ class ScaladocSettings extends SettingGroup with AllScalaSettings:
   val generateInkuire: Setting[Boolean] =
     BooleanSetting("-Ygenerate-inkuire", "Generates InkuireDB and enables Hoogle-like searches", false)
 
+  val apiSubdirectory: Setting[Boolean] =
+    BooleanSetting("-Yapi-subdirectory", "Keep all api member inside `api` directory", false)
+
   def scaladocSpecificSettings: Set[Setting[_]] =
     Set(sourceLinks, syntax, revision, externalDocumentationMappings, socialLinks, skipById, skipByRegex, deprecatedSkipPackages, docRootContent, snippetCompiler, snippetCompilerDebug, generateInkuire)

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

@@ -64,13 +64,18 @@ class HtmlRenderer(rootPackage: Member, val members: Map[DRI, Member])(using ctx
           children = Nil
         ))
       case Some(siteContext) =>
-        (siteContext.orphanedTemplates :+ siteContext.indexTemplate()).map(templateToPage(_, siteContext))
+        // In case that we do not have an index page and we do not have any API entries 
+        // we want to create empty index page, so there is one
+        val actualIndexTemplate = siteContext.indexTemplates() match
+          case Nil if effectiveMembers.isEmpty => Seq(siteContext.emptyIndexTemplate)
+          case templates => templates
+
+        (siteContext.orphanedTemplates ++ actualIndexTemplate).map(templateToPage(_, siteContext))
 
   /**
    * Here we have to retrive index pages from hidden pages and replace fake index pages in navigable page tree.
    */
-  private def getAllPages: Seq[Page] =
-
+  val allPages: Seq[Page] =
     def traversePages(page: Page): (Page, Seq[Page]) =
       val (newChildren, newPagesToRemove): (Seq[Page], Seq[Page]) = page.children.map(traversePages(_)).foldLeft((Seq[Page](), Seq[Page]())) {
         case ((pAcc, ptrAcc), (p, ptr)) => (pAcc :+ p, ptrAcc ++ ptr)
@@ -83,9 +88,22 @@ class HtmlRenderer(rootPackage: Member, val members: Map[DRI, Member])(using ctx
 
     val (newNavigablePage, pagesToRemove) = traversePages(navigablePage)
 
-    newNavigablePage +: hiddenPages.filterNot(pagesToRemove.contains)
+    val all = newNavigablePage +: hiddenPages.filterNot(pagesToRemove.contains)
+    // We need to check for conflicts only if we have top-level member called blog or docs
+    val hasPotentialConflict =
+      rootPackage.members.exists(m => m.name.startsWith("docs") || m.name.startsWith("blog"))
+
+    if hasPotentialConflict then
+      def walk(page: Page): Unit =
+        if page.link.dri.isStaticFile then
+          val dest = absolutePath(page.link.dri)
+          if apiPaths.contains(dest) then
+            report.error(s"Conflict between static page and API member for $dest. $pathsConflictResoultionMsg")
+          page.children.foreach(walk)
+
+      all.foreach (walk)
 
-  val allPages = getAllPages
+    all
 
   def renderContent(page: Page) = page.content match
     case m: Member =>

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

@@ -19,15 +19,25 @@ val UnresolvedLocationLink = "#"
 trait Locations(using ctx: DocContext):
   def effectiveMembers: Map[DRI, Member]
 
+  // We generate this collection only if there may be a conflict with resources.
+  // Potentially can be quite big.
+  lazy val apiPaths = effectiveMembers.keySet.filterNot(_.isStaticFile).map(absolutePath)
+
   var cache = new JHashMap[DRI, Seq[String]]()
 
+  private[renderers] def pathsConflictResoultionMsg =
+    "Using `-Yapi-subdirectory` flag will move all API documentation into `api` subdirectory and will fix this conflict."
+
   // TODO verify if location exisits
   def rawLocation(dri: DRI): Seq[String] =
     cache.get(dri) match
       case null =>
         val path = dri match
           case `docsRootDRI` => List("docs", "index")
-          case `apiPageDRI` => List("api", "index")
+          case `apiPageDRI` =>
+            if ctx.args.apiSubdirectory || ctx.staticSiteContext.fold(false)(_.hasIndexFile)
+              then List("api", "index")
+              else List("index")
           case dri if dri.isStaticFile =>
             Paths.get(dri.location).iterator.asScala.map(_.toString).toList
           case dri =>
@@ -36,8 +46,7 @@ trait Locations(using ctx: DocContext):
               case "<empty>" :: Nil  => "_empty_" :: Nil
               case "<empty>" :: tail => "_empty_" :: tail
               case other => other
-
-            Seq("api") ++ fqn
+            if ctx.args.apiSubdirectory then "api" :: fqn else fqn
         cache.put(dri, path)
         path
       case cached => cached

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

@@ -171,19 +171,23 @@ trait Resources(using ctx: DocContext) extends Locations, Writer:
   )
 
   def renderResource(resource: Resource): Seq[String] =
-    resource match
-      case Resource.Text(path, content) =>
-        Seq(write(path, content))
-      case Resource.Classpath(path, name) =>
-        getClass.getClassLoader.getResourceAsStream(name) match
-          case null =>
-            report.error(s"Unable to find $name on classpath")
-            Nil
-          case is =>
-            try Seq(copy(is, path)) finally is.close()
-      case Resource.File(path, file) =>
-        Seq(copy(file, path))
-      case Resource.URL(url) =>
-        Nil
-      case Resource.URLToCopy(url, dest) =>
-        Seq(copy(new URL(url).openStream(), dest))
+    if resource.path.endsWith(".html") && apiPaths.contains(resource.path) then
+      report.error(s"Conflict between resource and API member for ${resource.path}. $pathsConflictResoultionMsg")
+      Nil
+    else
+      resource match
+        case Resource.Text(path, content) =>
+          Seq(write(path, content))
+        case Resource.Classpath(path, name) =>
+          getClass.getClassLoader.getResourceAsStream(name) match
+            case null =>
+              report.error(s"Unable to find $name on classpath")
+              Nil
+            case is =>
+              try Seq(copy(is, path)) finally is.close()
+        case Resource.File(path, file) =>
+          Seq(copy(file, path))
+        case Resource.URL(url) =>
+          Nil
+        case Resource.URLToCopy(url, dest) =>
+          Seq(copy(new URL(url).openStream(), dest))

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

@@ -19,17 +19,21 @@ class StaticSiteContext(
 
   var memberLinkResolver: String => Option[DRI] = _ => None
 
-  def indexTemplate(): LoadedTemplate =
+  private def indexFiles =
     val files = List(new File(root, "index.html"), new File(root, "index.md")).filter { _.exists() }
 
     if files.size > 1 then
       val msg = s"ERROR: Multiple root index pages found: ${files.map(_.getAbsolutePath)}"
       report.error(msg)
+    files
 
-    files.flatMap(loadTemplate(_, isBlog = false)).headOption.getOrElse {
-      val fakeFile = new File(root, "index.html")
-      LoadedTemplate(emptyTemplate(fakeFile, "index"), List.empty, fakeFile)
-    }
+  def hasIndexFile = indexFiles.nonEmpty
+
+  def emptyIndexTemplate =
+    val fakeFile = new File(root, "index.html")
+    LoadedTemplate(emptyTemplate(fakeFile, "index"), List.empty, fakeFile)
+
+  def indexTemplates(): Seq[LoadedTemplate] = indexFiles.flatMap(loadTemplate(_, isBlog = false))
 
   lazy val layouts: Map[String, TemplateFile] =
     val layoutRoot = new File(root, "_layouts")

scaladoc/test-documentations/conflicts-pages/docs/tests/Adoc.md

@@ -0,0 +1 @@
+# Trying to override a api page!

scaladoc/test-documentations/conflicts-resources/resources/tests/Adoc.html

@@ -0,0 +1 @@
+<html><body>I am causing conflicts!</body></html>

scaladoc/test/dotty/tools/scaladoc/BaseHtmlTest.scala

@@ -41,8 +41,6 @@ class BaseHtmlTest:
 
       finally IO.delete(dest.toFile)
 
-  val testDocPath = Paths.get(BuildInfo.testDocumentationRoot)
-
   class DocumentContext(d: Document, path: Path):
     import collection.JavaConverters._
 

scaladoc/test/dotty/tools/scaladoc/ExternalLocationProviderIntegrationTest.scala

@@ -60,7 +60,7 @@ abstract class ExternalLocationProviderIntegrationTest(
   )
 
   override def runTest = afterRendering {
-    val output = summon[DocContext].args.output.toPath.resolve("api")
+    val output = summon[DocContext].args.output.toPath
     val linksBuilder = List.newBuilder[String]
 
     def processFile(path: Path): Unit =
@@ -72,7 +72,6 @@ abstract class ExternalLocationProviderIntegrationTest(
         linksBuilder ++= hrefValues
       }
 
-    println(output)
     IO.foreachFileIn(output, processFile)
     val links = linksBuilder.result
     val errors = expectedLinks.flatMap(expect => Option.when(!links.contains(expect))(expect))

scaladoc/test/dotty/tools/scaladoc/RaportingTest.scala

@@ -44,7 +44,7 @@ class ReportingTest:
     finally Files.delete(notTasty)
 
   @Test
-  def verbosePrintsDokkaMessage =
+  def testSuccessfulDocsGeneration =
     val ctx = testContext
     ctx.setSetting(ctx.settings.verbose, true)
     checkReportedDiagnostics(ctx = ctx){ diag =>
@@ -53,3 +53,35 @@ class ReportingTest:
 
       assertMessagesAbout(diag.infoMsgs)("generation completed successfully")
     }
+
+  @Test
+  def testErrorInCaseOfAssetShadowing =
+    val ctx = testContext
+    ctx.setSetting(ctx.settings.verbose, true)
+    val docsRoot = testDocPath.resolve("conflicts-resources").toString
+    checkReportedDiagnostics(_.copy(
+      docsRoot = Some(docsRoot),
+      tastyFiles = tastyFiles("tests", rootPck = "resources")
+    )){ diag =>
+      assertNoWarning(diag)
+      val Seq(msg) = diag.errorMsgs.map(_.toLowerCase)
+      Seq("conflict","api", "resource", "resources/tests/adoc.html").foreach(word =>
+        Assert.assertTrue(s"Error message: $msg should contains $word", msg.contains(word)))
+    }
+
+  @Test
+  def testErrorInCaseOfDocsShadowing =
+    val ctx = testContext
+    ctx.setSetting(ctx.settings.verbose, true)
+    val docsRoot = testDocPath.resolve("conflicts-pages").toString
+    checkReportedDiagnostics(_.copy(
+      docsRoot = Some(docsRoot),
+      tastyFiles = tastyFiles("tests", rootPck = "docs")
+    )){ diag =>
+      assertNoWarning(diag)
+      val Seq(msg) = diag.errorMsgs.map(_.toLowerCase)
+      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/signatures/AbstractMemberSignaturesTest.scala

@@ -26,7 +26,7 @@ class AbstractMembers extends ScaladocTest("abstractmembersignatures"):
   }
 
   private def signaturesFromDocumentation()(using DocContext): Map[String, List[(String, String)]] =
-    val output = summon[DocContext].args.output.toPath.resolve("api")
+    val output = summon[DocContext].args.output.toPath
     val signatures = List.newBuilder[(String, (String, String))]
     def processFile(path: Path): Unit =
       val document = Jsoup.parse(IO.read(path))

scaladoc/test/dotty/tools/scaladoc/signatures/SignatureTest.scala

@@ -104,7 +104,7 @@ abstract class SignatureTest(
         }
 
   private def signaturesFromDocumentation()(using DocContext): Seq[String] =
-    val output = summon[DocContext].args.output.toPath.resolve("api")
+    val output = summon[DocContext].args.output.toPath
     val signatures = List.newBuilder[String]
 
     def processFile(path: Path): Unit = if filterFunc(path) then

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

@@ -30,17 +30,17 @@ class NavigationTest extends BaseHtmlTest:
       )),
       NavMenuTestEntry("Adoc", "Adoc.html", Seq()),
       NavMenuTestEntry("API", "../api/index.html", Seq(
-        NavMenuTestEntry("tests.site", "../api/tests/site.html", Seq(
-          NavMenuTestEntry("BrokenLink", "../api/tests/site/BrokenLink.html", Nil),
-          NavMenuTestEntry("BrokenLinkWiki", "../api/tests/site/BrokenLinkWiki.html", Nil),
-          NavMenuTestEntry("OtherPackageLink", "../api/tests/site/OtherPackageLink.html", Nil),
-          NavMenuTestEntry("OtherPackageLinkWiki", "../api/tests/site/OtherPackageLinkWiki.html", Nil),
-          NavMenuTestEntry("SamePackageLink", "../api/tests/site/SamePackageLink.html", Nil),
-          NavMenuTestEntry("SamePackageLinkWiki", "../api/tests/site/SamePackageLinkWiki.html", Nil),
-          NavMenuTestEntry("SomeClass", "../api/tests/site/SomeClass.html", Nil)
+        NavMenuTestEntry("tests.site", "../tests/site.html", Seq(
+          NavMenuTestEntry("BrokenLink", "../tests/site/BrokenLink.html", Nil),
+          NavMenuTestEntry("BrokenLinkWiki", "../tests/site/BrokenLinkWiki.html", Nil),
+          NavMenuTestEntry("OtherPackageLink", "../tests/site/OtherPackageLink.html", Nil),
+          NavMenuTestEntry("OtherPackageLinkWiki", "../tests/site/OtherPackageLinkWiki.html", Nil),
+          NavMenuTestEntry("SamePackageLink", "../tests/site/SamePackageLink.html", Nil),
+          NavMenuTestEntry("SamePackageLinkWiki", "../tests/site/SamePackageLinkWiki.html", Nil),
+          NavMenuTestEntry("SomeClass", "../tests/site/SomeClass.html", Nil)
         )),
-        NavMenuTestEntry("tests.site.some.other", "../api/tests/site/some/other.html", Seq(
-          NavMenuTestEntry("SomeOtherPackage", "../api/tests/site/some/other/SomeOtherPackage.html", Nil),
+        NavMenuTestEntry("tests.site.some.other", "../tests/site/some/other.html", Seq(
+          NavMenuTestEntry("SomeOtherPackage", "../tests/site/some/other/SomeOtherPackage.html", Nil),
         ))
       )),
     ))