API pages are not top level

romanowski · romanowski · commit 05f73107a916 · 2021-07-22T23:02:02.000+02:00
diff --git a/scaladoc-testcases/src/docs/tests/Adoc.scala b/scaladoc-testcases/src/docs/tests/Adoc.scala
@@ -0,0 +1,4 @@
+package docs.tests
+
+class Adoc:
+  def foo = 123
diff --git a/scaladoc-testcases/src/resources/tests/Adoc.scala b/scaladoc-testcases/src/resources/tests/Adoc.scala
@@ -0,0 +1,4 @@
+package resources.tests
+
+class Adoc:
+  def foo = 123
diff --git a/scaladoc/src/dotty/tools/scaladoc/renderers/HtmlRenderer.scala b/scaladoc/src/dotty/tools/scaladoc/renderers/HtmlRenderer.scala
@@ -58,19 +58,18 @@ class HtmlRenderer(rootPackage: Member, val members: Map[DRI, Member])(using ctx
 
   val hiddenPages: Seq[Page] =
     staticSite match
-      case None =>
-        Seq(navigablePage.copy( // Add index page that is a copy of api/index.html
-          link = navigablePage.link.copy(dri = docsRootDRI),
-          children = Nil
-        ))
+      case None => Nil
       case Some(siteContext) =>
-        (siteContext.orphanedTemplates :+ siteContext.indexTemplate()).map(templateToPage(_, siteContext))
+        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 +82,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")
+          page.children.foreach(walk)
+
+      all.foreach (walk)
 
-  val allPages = getAllPages
+    all
 
   def renderContent(page: Page) = page.content match
     case m: Member =>
diff --git a/scaladoc/src/dotty/tools/scaladoc/renderers/Locations.scala b/scaladoc/src/dotty/tools/scaladoc/renderers/Locations.scala
@@ -19,6 +19,10 @@ 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]]()
 
   // TODO verify if location exisits
@@ -27,17 +31,16 @@ trait Locations(using ctx: DocContext):
       case null =>
         val path = dri match
           case `docsRootDRI` => List("docs", "index")
-          case `apiPageDRI` => List("api", "index")
+          case `apiPageDRI` =>
+            if 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 =>
             val loc = dri.location
-            val fqn = loc.split(Array('.')).toList match
+            loc.split(Array('.')).toList match
               case "<empty>" :: Nil  => "_empty_" :: Nil
               case "<empty>" :: tail => "_empty_" :: tail
               case other => other
-
-            Seq("api") ++ fqn
         cache.put(dri, path)
         path
       case cached => cached
diff --git a/scaladoc/src/dotty/tools/scaladoc/renderers/Resources.scala b/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}")
+      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))
diff --git a/scaladoc/src/dotty/tools/scaladoc/site/StaticSiteContext.scala b/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")
diff --git a/scaladoc/test-documentations/conflicts-pages/docs/tests/Adoc.md b/scaladoc/test-documentations/conflicts-pages/docs/tests/Adoc.md
@@ -0,0 +1 @@
+# Trying to override a api page!
diff --git a/scaladoc/test-documentations/conflicts-resources/resources/tests/Adoc.html b/scaladoc/test-documentations/conflicts-resources/resources/tests/Adoc.html
@@ -0,0 +1 @@
+<html><body>I am causing conflicts!</body></html>
diff --git a/scaladoc/test/dotty/tools/scaladoc/BaseHtmlTest.scala b/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._
 
diff --git a/scaladoc/test/dotty/tools/scaladoc/ExternalLocationProviderIntegrationTest.scala b/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))
diff --git a/scaladoc/test/dotty/tools/scaladoc/RaportingTest.scala b/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))
+        )
+    }
diff --git a/scaladoc/test/dotty/tools/scaladoc/signatures/AbstractMemberSignaturesTest.scala b/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))
diff --git a/scaladoc/test/dotty/tools/scaladoc/signatures/SignatureTest.scala b/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
diff --git a/scaladoc/test/dotty/tools/scaladoc/site/NavigationTest.scala b/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),
         ))
       )),
     ))
diff --git a/scaladoc/test/dotty/tools/scaladoc/site/SiteGeneratationTest.scala b/scaladoc/test/dotty/tools/scaladoc/site/SiteGeneratationTest.scala
@@ -45,18 +45,19 @@ class SiteGeneratationTest extends BaseHtmlTest:
 
   def testApiPages(
     mainTitle: String = "API",
-    parents: Seq[String] = Seq(projectName))(using ProjectContext) =
-      checkFile("api/index.html")(
+    parents: Seq[String] = Seq(projectName),
+    hasToplevelIndexIndex: Boolean = true)(using ProjectContext) =
+      checkFile((if hasToplevelIndexIndex then "api/" else "" )+ "index.html")(
         title = mainTitle,
         header = projectName,
         parents = parents
       )
-      checkFile("api/tests/site.html")(
+      checkFile("tests/site.html")(
         title = "tests.site",
         header = "tests.site",
         parents = parents :+ mainTitle
       )
-      checkFile("api/tests/site/SomeClass.html")(
+      checkFile("tests/site/SomeClass.html")(
         title = "SomeClass",
         header = "SomeClass",
         parents = parents ++ Seq(mainTitle, "tests.site")
@@ -70,12 +71,12 @@ class SiteGeneratationTest extends BaseHtmlTest:
     testApiPages()
 
     withHtmlFile("docs/Adoc.html"){ content  =>
-        content.assertAttr("p a","href", "../api/tests/site/SomeClass.html")
+        content.assertAttr("p a","href", "../tests/site/SomeClass.html")
     }
 
-    withHtmlFile("api/tests/site/SomeClass.html"){ content  =>
+    withHtmlFile("tests/site/SomeClass.html"){ content  =>
       content.assertAttr(".breadcrumbs a","href",
-        "../../../docs/index.html", "../../index.html", "../site.html", "SomeClass.html"
+        "../../docs/index.html", "../../api/index.html", "../site.html", "SomeClass.html"
       )
     }
   }
@@ -84,18 +85,18 @@ class SiteGeneratationTest extends BaseHtmlTest:
   def noGlobalIndexTest() = withGeneratedSite(testDocPath.resolve("noGlobalIndex")){
     testDocPages()
     testDocIndexPage()
-    testApiPages()
+    testApiPages(hasToplevelIndexIndex = false)
   }
 
   @Test
   def noIndexesTest() = withGeneratedSite(testDocPath.resolve("noIndexes")){
     testDocPages()
-    testApiPages()
+    testApiPages(hasToplevelIndexIndex = false)
   }
 
   @Test
   def noExistingDocs() = withGeneratedSite(testDocPath.resolve("noExisting")){
-    testApiPages(mainTitle = projectName, parents = Nil)
+    testApiPages(mainTitle = projectName, parents = Nil, hasToplevelIndexIndex = false)
   }
 
   @Test
diff --git a/scaladoc/test/dotty/tools/scaladoc/tasty/comments/IntegrationTest.scala b/scaladoc/test/dotty/tools/scaladoc/tasty/comments/IntegrationTest.scala
@@ -18,11 +18,11 @@ abstract class BaseIntegrationTest(pck: String) extends BaseHtmlTest:
         "", "" // each represent a link
         )
 
-    withHtmlFile(s"api/tests/$pck/BrokenLinks.html")(checkUnresolved)
+    withHtmlFile(s"tests/$pck/BrokenLinks.html")(checkUnresolved)
     val otherPackagePath = "../commonlinks/SomeOtherPackage.html"
-    withHtmlFile(s"api/tests/$pck/OtherPackageLink.html")(checkDocLinks(otherPackagePath))
+    withHtmlFile(s"tests/$pck/OtherPackageLink.html")(checkDocLinks(otherPackagePath))
     // OtherPackageMembers - does not work, TODO?
-    withHtmlFile(s"api/tests/$pck/SamePackageLink.html")(checkDocLinks("SomeClass.html"))
+    withHtmlFile(s"tests/$pck/SamePackageLink.html")(checkDocLinks("SomeClass.html"))
     // SamePackageMembers - does not work, TODO?
   }
 
diff --git a/scaladoc/test/dotty/tools/scaladoc/testUtils.scala b/scaladoc/test/dotty/tools/scaladoc/testUtils.scala
@@ -7,6 +7,7 @@ import dotty.tools.dotc.interfaces.Diagnostic.{ERROR, INFO, WARNING}
 import dotty.tools.scaladoc.test.BuildInfo
 import org.junit.Assert._
 import java.io.File
+import java.nio.file.Paths
 
 
 case class ReportedDiagnostics(errors: List[Diagnostic], warnings: List[Diagnostic], infos: List[Diagnostic]):
@@ -57,14 +58,14 @@ def testArgs(files: Seq[File] = Nil, dest: File = new File("notUsed")) = Scalado
           docsRoot = Some(""),
         )
 
-def testContext = 
+def testContext =
   val ctx = (new ContextBase).initialCtx.fresh.setReporter(new TestReporter)
   ctx.setSetting(ctx.settings.usejavacp, true)
   ctx
 
 def testDocContext(files: Seq[File] = Nil) = DocContext(testArgs(files), testContext)
 
-def tastyFiles(name: String, allowEmpty: Boolean = false) =
+def tastyFiles(name: String, allowEmpty: Boolean = false, rootPck: String = "tests") =
   def listFilesSafe(dir: File) = Option(dir.listFiles).getOrElse {
     throw AssertionError(s"$dir not found. The test name is incorrect or scaladoc-testcases were not recompiled.")
   }
@@ -73,7 +74,9 @@ def tastyFiles(name: String, allowEmpty: Boolean = false) =
       case f if f.getName endsWith ".tasty" => f :: Nil
       case _ => Nil
     }
-  val files = BuildInfo.test_testcasesOutputDir.flatMap(p => collectFiles(File(s"$p/tests/$name")))
+  val outputDir = BuildInfo.test_testcasesOutputDir
+  val files = outputDir.flatMap(p => collectFiles(File(s"$p/$rootPck/$name")))
   assert(files.nonEmpty || allowEmpty)
   files.toSeq
 
+def testDocPath = Paths.get(BuildInfo.testDocumentationRoot)

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+<html><body>I am causing conflicts!</body></html>`
Original file line number	Diff line number	Diff line change
`@@ -60,7 +60,7 @@ abstract class ExternalLocationProviderIntegrationTest(`
`60`	`60`	`)`
`61`	`61`
`62`	`62`	`override def runTest = afterRendering {`
`63`		`- val output = summon[DocContext].args.output.toPath.resolve("api")`
	`63`	`+ val output = summon[DocContext].args.output.toPath`
`64`	`64`	`val linksBuilder = List.newBuilder[String]`
`65`	`65`
`66`	`66`	`def processFile(path: Path): Unit =`
`@@ -72,7 +72,6 @@ abstract class ExternalLocationProviderIntegrationTest(`
`72`	`72`	`linksBuilder ++= hrefValues`
`73`	`73`	`}`
`74`	`74`
`75`		`- println(output)`
`76`	`75`	`IO.foreachFileIn(output, processFile)`
`77`	`76`	`val links = linksBuilder.result`
`78`	`77`	`val errors = expectedLinks.flatMap(expect => Option.when(!links.contains(expect))(expect))`