Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 27, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions project/Build.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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", "-Ylegacy-api-layout")))

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 {
Expand Down
4 changes: 4 additions & 0 deletions scaladoc-testcases/src/docs/tests/Adoc.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package docs.tests

class Adoc:
def foo = 123
4 changes: 4 additions & 0 deletions scaladoc-testcases/src/resources/tests/Adoc.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package resources.tests

class Adoc:
def foo = 123
6 changes: 4 additions & 2 deletions scaladoc/src/dotty/tools/scaladoc/Scaladoc.scala
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ object Scaladoc:
snippetCompilerDebug: Boolean = false,
noLinkWarnings: Boolean = false,
versionsDictionaryUrl: Option[String] = None,
generateInkuire : Boolean = false
generateInkuire : Boolean = false,
legacyAPILayout : Boolean = false
)

def run(args: Array[String], rootContext: CompilerContext): Reporter =
Expand Down Expand Up @@ -223,7 +224,8 @@ object Scaladoc:
noLinkWarnings.get,
snippetCompilerDebug.get,
versionsDictionaryUrl.nonDefault,
generateInkuire.get
generateInkuire.get,
legacyAPILayout.get,
)
(Some(docArgs), newContext)
}
Expand Down
3 changes: 3 additions & 0 deletions scaladoc/src/dotty/tools/scaladoc/ScaladocSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 legacyAPILayout: Setting[Boolean] =
BooleanSetting("-Ylegacy-api-layout", "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)
32 changes: 22 additions & 10 deletions scaladoc/src/dotty/tools/scaladoc/renderers/HtmlRenderer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment explaining what this does? I’m a bit lost.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems it just a rewrite (no new logic is added here).

Before:
we got one file by calling siteContext.indexTemplate()

Now:
we call siteContext.indexTemplates() which returns collection (effectively only one file), then we pattern match on it just to replace it with emptyIndextemplate (which was embedded in old siteContext.indexTemplate()) if it was empty or get identity otherwise and then append it as a collection.

Can you @romanowski explain if I get something wrong here? The only thing is granularity (we have more designated functions to call), but on the other hand main misconception here is that indexTemplates() actually can yield 0 or 1 file, never 2.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will rewrite to Option


/**
* 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)
Expand All @@ -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 =>
Expand Down
12 changes: 9 additions & 3 deletions scaladoc/src/dotty/tools/scaladoc/renderers/Locations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -27,7 +31,10 @@ 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.args.legacyAPILayout || 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 =>
Expand All @@ -36,8 +43,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.legacyAPILayout then "api" :: fqn else fqn
cache.put(dri, path)
path
case cached => cached
Expand Down
36 changes: 20 additions & 16 deletions scaladoc/src/dotty/tools/scaladoc/renderers/Resources.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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))
14 changes: 9 additions & 5 deletions scaladoc/src/dotty/tools/scaladoc/site/StaticSiteContext.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be indexFile as we get only one file (either html or md) but not both at once. Moreover it would never be files becuase of assertion in line 25


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")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Trying to override a api page!
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<html><body>I am causing conflicts!</body></html>
2 changes: 0 additions & 2 deletions scaladoc/test/dotty/tools/scaladoc/BaseHtmlTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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._

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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))
Expand Down
34 changes: 33 additions & 1 deletion scaladoc/test/dotty/tools/scaladoc/RaportingTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 =>
Expand All @@ -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))
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 10 additions & 10 deletions scaladoc/test/dotty/tools/scaladoc/site/NavigationTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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),
))
)),
))
Expand Down
Loading