From daa99345d480210ab108bbfffe461700fb0722ab Mon Sep 17 00:00:00 2001 From: Lucas Leblanc <44496264+Dedelweiss@users.noreply.github.com> Date: Mon, 12 Jun 2023 09:09:45 +0200 Subject: [PATCH] Refactor: Improve error messaging in sidebar YAML parser (#17229) Fixes: #15326 --- .../tools/scaladoc/site/SidebarParser.scala | 30 +++++-- .../scaladoc/site/SidebarParserTest.scala | 86 ++++++++++++++++++- 2 files changed, 108 insertions(+), 8 deletions(-) diff --git a/scaladoc/src/dotty/tools/scaladoc/site/SidebarParser.scala b/scaladoc/src/dotty/tools/scaladoc/site/SidebarParser.scala index 1aefeaa21032..1916c32e53dc 100644 --- a/scaladoc/src/dotty/tools/scaladoc/site/SidebarParser.scala +++ b/scaladoc/src/dotty/tools/scaladoc/site/SidebarParser.scala @@ -7,6 +7,8 @@ import com.fasterxml.jackson.core.`type`.TypeReference; import scala.jdk.CollectionConverters._ import java.util.Optional import scala.beans._ +import java.nio.file.{Files, Paths} +import scala.io.Source enum Sidebar: case Category( @@ -30,16 +32,31 @@ object Sidebar: private object RawInputTypeRef extends TypeReference[RawInput] - private def toSidebar(r: RawInput)(using CompilerContext): Sidebar = r match + private def toSidebar(r: RawInput, content: String | java.io.File)(using CompilerContext): Sidebar = r match case RawInput(title, page, index, subsection, dir, hidden) if page.nonEmpty && index.isEmpty && subsection.isEmpty() => + val sidebarPath = content match + case s: String => Paths.get(s) + case f: java.io.File => f.toPath() + val basePath = sidebarPath.getParent().resolve("_docs") + val pagePath = basePath.resolve(page) + if !Files.exists(pagePath) then + report.error(s"Page $page does not exist.") Sidebar.Page(Option.when(title.nonEmpty)(title), page, hidden) case RawInput(title, page, index, subsection, dir, hidden) if page.isEmpty && (!subsection.isEmpty() || !index.isEmpty()) => - Sidebar.Category(Option.when(title.nonEmpty)(title), Option.when(index.nonEmpty)(index), subsection.asScala.map(toSidebar).toList, Option.when(dir.nonEmpty)(dir)) + Sidebar.Category(Option.when(title.nonEmpty)(title), Option.when(index.nonEmpty)(index), subsection.asScala.map(toSidebar(_, content)).toList, Option.when(dir.nonEmpty)(dir)) case RawInput(title, page, index, subsection, dir, hidden) => - report.error(s"Error parsing YAML configuration file.\n$schemaMessage") + if title.isEmpty() && index.isEmpty() then + val msg = "`title` property is missing for some page." + report.error(s"$msg\n$schemaMessage") + else if title.nonEmpty && (page.isEmpty() || index.isEmpty()) then + val msg = s"Error parsing YAML configuration file: 'index' or 'page' path is missing for title '$title'." + report.error(s"$msg\n$schemaMessage") + else + val msg = "Problem when parsing YAML configuration file." + report.warning(s"$msg\n$schemaMessage") Sidebar.Page(None, page, hidden) - private def schemaMessage: String = + def schemaMessage: String = s"""Static site YAML configuration file should comply with the following description: |The root element of static site needs to be |`title` and `directory` properties are ignored in root subsection. @@ -57,8 +74,7 @@ object Sidebar: | hidden: # optional - Default value is false. | |For more information visit: - |https://docs.scala-lang.org/scala3/guides/scaladoc/static-site.html - |""".stripMargin + |https://docs.scala-lang.org/scala3/guides/scaladoc/static-site.html""".stripMargin def load(content: String | java.io.File)(using CompilerContext): Sidebar.Category = import scala.util.Try @@ -75,7 +91,7 @@ object Sidebar: }, identity ) - toSidebar(root) match + toSidebar(root, content) match case c: Sidebar.Category => c case _ => report.error(s"Root element is not a subsection.\n$schemaMessage") diff --git a/scaladoc/test/dotty/tools/scaladoc/site/SidebarParserTest.scala b/scaladoc/test/dotty/tools/scaladoc/site/SidebarParserTest.scala index 72fc6515fdee..f30990d07cfa 100644 --- a/scaladoc/test/dotty/tools/scaladoc/site/SidebarParserTest.scala +++ b/scaladoc/test/dotty/tools/scaladoc/site/SidebarParserTest.scala @@ -3,8 +3,12 @@ package site import org.junit.Test import org.junit.Assert._ +import dotty.tools.scaladoc.site.Sidebar +import dotty.tools.scaladoc.site.Sidebar.RawInput +import java.io.ByteArrayOutputStream +import java.io.PrintStream -// TODO add negaitve and more details tests +// TODO add negative and more details tests class SidebarParserTest: private val sidebar = @@ -34,6 +38,64 @@ class SidebarParserTest: | - page: my-page6/my-page6/my-page6.md """.stripMargin + private val sidebarNoTitle = + """index: index.md + |subsection: + | - title: My title + | page: my-page1.md + | - page: my-page2.md + | - page: my-page3/subsection + | - title: Reference + | subsection: + | - page: my-page3.md + | hidden: true + | - index: my-page4/index.md + | subsection: + | - page: my-page4/my-page4.md + | - title: My subsection + | index: my-page5/index.md + | subsection: + | - page: my-page5/my-page5.md + | - subsection: + | page: my-page7/my-page7.md + | - index: my-page6/index.md + | subsection: + | - index: my-page6/my-page6/index.md + | subsection: + | - page: my-page6/my-page6/my-page6.md + """.stripMargin + + private val sidebarErrorNoPage = + """index: index.md + |subsection: + | - title: My title + | - page: my-page2.md + | - page: my-page3/subsection + | - title: Reference + | subsection: + | - page: my-page3.md + | hidden: true + | - index: my-page4/index.md + | subsection: + | - page: my-page4/my-page4.md + | - title: My subsection + | index: my-page5/index.md + | subsection: + | - page: my-page5/my-page5.md + | - subsection: + | - page: my-page7/my-page7.md + | - index: my-page6/index.md + | subsection: + | - index: my-page6/my-page6/index.md + | subsection: + | - page: my-page6/my-page6/my-page6.md + """.stripMargin + + private val msgNoTitle = "`title` property is missing for some page." + private val msgNoPage = "Error parsing YAML configuration file: 'index' or 'page' path is missing for title 'My title'." + + private def schemaMessage: String = Sidebar.schemaMessage + @Test def loadSidebar(): Unit = assertEquals( Sidebar.Category( @@ -53,3 +115,25 @@ class SidebarParserTest: ), Sidebar.load(sidebar)(using testContext) ) + + @Test + def loadSidebarNoPageError: Unit = + val out = new ByteArrayOutputStream() + Console.withErr(new PrintStream(out)) { + Sidebar.load(sidebarErrorNoPage)(using testContext) + } + println(out.toString()) + val error = out.toString().trim() + + assert(error.contains(msgNoPage) && error.contains(schemaMessage)) + + + @Test + def loadSidebarNoTitleError(): Unit = + val out = new ByteArrayOutputStream() + Console.withErr(new PrintStream(out)) { + Sidebar.load(sidebarNoTitle)(using testContext) + } + val error = out.toString().trim() + + assert(error.contains(msgNoTitle) && error.contains(schemaMessage))