Merge pull request #11804 from lampepfl/scaladoc/generic-path-based

Scaladoc: Add generic path-based arguments implementation

Author: romanowski

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

@@ -69,8 +69,7 @@ extension (r: report.type)
 case class NavigationNode(name: String, dri: DRI, nested: Seq[NavigationNode])
 
 case class DocContext(args: Scaladoc.Args, compilerContext: CompilerContext):
-  lazy val sourceLinks: SourceLinks = SourceLinks.load(using this)
-
+  lazy val sourceLinks = SourceLinks.load(args.sourceLinks, args.revision)(using compilerContext)
   lazy val staticSiteContext = args.docsRoot.map(path => StaticSiteContext(
       File(path).getAbsoluteFile(),
       args,

scaladoc/src/dotty/tools/scaladoc/PathBased.scala

@@ -0,0 +1,39 @@
+package dotty.tools.scaladoc
+
+import java.nio.file.{Path, Paths}
+
+case class PathBased[T](entries: List[PathBased.Entry[T]], projectRoot: Path):
+  def get(path: Path): Option[PathBased.Result[T]] =
+    if path.isAbsolute then
+      if path.startsWith(projectRoot) then get(projectRoot.relativize(path))
+      else None
+    else entries.find(_.path.forall(p => path.startsWith(p))).map(entry =>
+      PathBased.Result(entry.path.fold(path)(_.relativize(path)), entry.elem)
+    )
+
+trait ArgParser[T]:
+  def parse(s: String): Either[String, T]
+
+object PathBased:
+  case class Entry[T](path: Option[Path], elem: T)
+  case class ParsingResult[T](errors: List[String], result: PathBased[T])
+  case class Result[T](path: Path, elem: T)
+
+  private val PathExtractor = "([^=]+)=(.+)".r
+
+
+  def parse[T](args: Seq[String], projectRoot: Path = Paths.get("").toAbsolutePath())(using parser: ArgParser[T]): ParsingResult[T] = {
+    val parsed = args.map {
+      case PathExtractor(path, arg) => parser.parse(arg).map(elem => Entry(Some(Paths.get(path)), elem))
+      case arg => parser.parse(arg).map(elem => Entry(None, elem))
+    }
+    val errors = parsed.collect {
+      case Left(error) => error
+    }.toList
+
+    val entries = parsed.collect {
+      case Right(entry) => entry
+    }.toList
+
+    ParsingResult(errors, PathBased(entries, projectRoot))
+  }

scaladoc/src/dotty/tools/scaladoc/SourceLinks.scala

@@ -14,13 +14,6 @@ trait SourceLink:
   val path: Option[Path] = None
   def render(memberName: String, path: Path, operation: String, line: Option[Int]): String
 
-case class PrefixedSourceLink(val myPath: Path, nested: SourceLink) extends SourceLink:
-  val myPrefix = pathToString(myPath)
-  override val path = Some(myPath)
-  override def render(memberName: String, path: Path, operation: String, line: Option[Int]): String =
-    nested.render(memberName, myPath.relativize(path), operation, line)
-
-
 case class TemplateSourceLink(val urlTemplate: String) extends SourceLink:
   override val path: Option[Path] = None
   override def render(memberName: String, path: Path, operation: String, line: Option[Int]): String =
@@ -48,8 +41,7 @@ case class WebBasedSourceLink(prefix: String, revision: String, subPath: String)
     val linePart = line.fold("")(l => s"#L$l")
     s"$prefix/$action/$revision$subPath/${pathToString(path)}$linePart"
 
-object SourceLink:
-  val SubPath = "([^=]+)=(.+)".r
+class SourceLinkParser(revision: Option[String]) extends ArgParser[SourceLink]:
   val KnownProvider = raw"(\w+):\/\/([^\/#]+)\/([^\/#]+)(\/[^\/#]+)?(#.+)?".r
   val BrokenKnownProvider = raw"(\w+):\/\/.+".r
   val ScalaDocPatten = raw"€\{(TPL_NAME|TPL_OWNER|FILE_PATH|FILE_EXT|FILE_LINE|FILE_PATH_EXT)\}".r
@@ -68,9 +60,8 @@ object SourceLink:
 
   private def parseLinkDefinition(s: String): Option[SourceLink] = ???
 
-  def parse(string: String, revision: Option[String]): Either[String, SourceLink] =
-
-    string match
+  def parse(string: String): Either[String, SourceLink] =
+    val res = string match
       case KnownProvider(name, organization, repo, rawRevision, rawSubPath) =>
         val subPath = Option(rawSubPath).fold("")("/" + _.drop(1))
         val pathRev = Option(rawRevision).map(_.drop(1)).orElse(revision)
@@ -87,14 +78,6 @@ object SourceLink:
               WebBasedSourceLink(gitlabPrefix(organization, repo), rev, subPath))
           case other =>
             Left(s"'$other' is not a known provider, please provide full source path template.")
-
-      case SubPath(prefix, config) =>
-        parse(config, revision) match
-          case l: Left[String, _] => l
-          case Right(_:PrefixedSourceLink) =>
-            Left(s"Source path $string has duplicated subpath setting (scm template can not contains '=')")
-          case Right(nested) =>
-            Right(PrefixedSourceLink(Paths.get(prefix), nested))
       case BrokenKnownProvider("gitlab" | "github") =>
         Left(s"Does not match known provider syntax: `<name>://organization/repository`")
       case scaladocSetting if ScalaDocPatten.findFirstIn(scaladocSetting).nonEmpty =>
@@ -104,28 +87,23 @@ object SourceLink:
         else Right(TemplateSourceLink(supported.foldLeft(string)((template, pattern) =>
           template.replace(pattern, SupportedScalaDocPatternReplacements(pattern)))))
       case other =>
-        Right(TemplateSourceLink(""))
+        Left("Does not match any implemented source link syntax")
+    res match {
+      case Left(error) => Left(s"'$string': $error")
+      case other => other
+    }
 
 
 type Operation = "view" | "edit"
 
-case class SourceLinks(links: Seq[SourceLink], projectRoot: Path):
+class SourceLinks(val sourceLinks: PathBased[SourceLink]):
   def pathTo(rawPath: Path, memberName: String = "", line: Option[Int] = None, operation: Operation = "view"): Option[String] =
-    def resolveRelativePath(path: Path) =
-      links
-        .find(_.path.forall(p => path.startsWith(p)))
-        .map(_.render(memberName, path, operation, line))
-
-    if rawPath.isAbsolute then
-      if rawPath.startsWith(projectRoot) then resolveRelativePath(projectRoot.relativize(rawPath))
-      else None
-    else resolveRelativePath(rawPath)
+    sourceLinks.get(rawPath).map(res => res.elem.render(memberName, res.path, operation, line))
 
   def pathTo(member: Member): Option[String] =
     member.sources.flatMap(s => pathTo(s.path, member.name, Option(s.lineNumber).map(_ + 1)))
 
 object SourceLinks:
-
   val usage =
     """Source links provide a mapping between file in documentation and code repository.
       |
@@ -150,32 +128,16 @@ object SourceLinks:
       |Template can defined only by subset of sources defined by path prefix represented by `<sub-path>`.
       |In such case paths used in templates will be relativized against `<sub-path>`""".stripMargin
 
-  def load(
-      configs: Seq[String],
-      revision: Option[String],
-      projectRoot: Path)(
-      using Context): SourceLinks =
-    val mappings = configs.map(str => str -> SourceLink.parse(str, revision))
-
-    val errors = mappings.collect {
-      case (template, Left(message)) =>
-        s"'$template': $message"
-    }.mkString("\n")
-
-    if errors.nonEmpty then report.warning(
-      s"""Following templates has invalid format:
-         |$errors
-         |
-         |$usage
-         |""".stripMargin
-      )
-
-    SourceLinks(mappings.collect {case (_, Right(link)) => link}, projectRoot)
-
-  def load(using ctx: DocContext): SourceLinks =
-    load(
-      ctx.args.sourceLinks,
-      ctx.args.revision,
-      // TODO (https://github.com/lampepfl/scaladoc/issues/240): configure source root
-      Paths.get("").toAbsolutePath
-    )
+  def load(config: Seq[String], revision: Option[String], projectRoot: Path = Paths.get("").toAbsolutePath)(using CompilerContext): SourceLinks =
+    PathBased.parse(config, projectRoot)(using SourceLinkParser(revision)) match {
+      case PathBased.ParsingResult(errors, sourceLinks) =>
+        if errors.nonEmpty then report.warning(
+          s"""Following templates has invalid format:
+            |$errors
+            |
+            |${SourceLinks.usage}
+            |""".stripMargin
+        )
+        SourceLinks(sourceLinks)
+    }
+

scaladoc/test/dotty/tools/scaladoc/SourceLinksTests.scala

@@ -9,22 +9,22 @@ class SourceLinkTest:
   @Test
   def testBasicFailures() =
     def testFailure(template: String, messagePart: String) =
-      val res = SourceLink.parse(template, None)
+      val res = SourceLinkParser(None).parse(template)
       assertTrue(s"Expected failure containing $messagePart: $res", res.left.exists(_.contains(messagePart)))
 
-      val resWithVersion = SourceLink.parse(template, Some("develop"))
+      val resWithVersion = SourceLinkParser(Some("develop")).parse(template)
       assertEquals(res, resWithVersion)
 
     testFailure("ala://ma/kota", "known provider")
-    testFailure("ala=ala=ala://ma/kota", "known provider")
-    testFailure("ala=ala=ala", "subpath")
+    testFailure("ala=ala=ala://ma/kota", "source link syntax")
+    testFailure("ala=ala=ala", "source link syntax")
     testFailure("""€{TPL_OWNER}""", "scaladoc")
 
 
   @Test
   def testProperTemplates() =
     def test(template: String) =
-      val res = try SourceLink.parse(template, Some("develop")) catch
+      val res = try SourceLinkParser(Some("develop")).parse(template) catch
         case e: Exception => throw RuntimeException(s"When testing $template", e)
       assertTrue(s"Bad template: $template", res.isRight)
 
@@ -35,19 +35,18 @@ class SourceLinkTest:
       "https://github.com/scala/scala/blob/2.13.x€{FILE_PATH_EXT}#€{FILE_LINE}"
     ).foreach{ template =>
       test(template)
-      test(s"docs/dotty=$template")
     }
 
 
   @Test
   def testSourceProviderWithoutRevision() =
     Seq("github", "gitlab").foreach { provider =>
       val template = s"$provider://ala/ma"
-      val res = SourceLink.parse(template, None)
+      val res = SourceLinkParser(None).parse(template)
       assertTrue(s"Expected failure containing missing revision: $res", res.left.exists(_.contains("revision")))
 
       Seq(s"$provider://ala/ma/", s"$provider://ala", s"$provider://ala/ma/develop/on/master").foreach { template =>
-        val res = SourceLink.parse(template, Some("develop"))
+        val res = SourceLinkParser(Some("develop")).parse(template)
         assertTrue(s"Expected failure syntax info: $res", res.left.exists(_.contains("syntax")))
       }
 
@@ -62,7 +61,7 @@ class SourceLinksTest:
   type Args = String | (String, Operation) | (String, Int) | (String, Int, Operation)
 
   private def testLink(config: Seq[String], revision: Option[String])(cases: (Args, String | None.type)*): Unit =
-    val links = SourceLinks.load(config, revision, projectRoot)(using testContext)
+    val links = SourceLinks.load(config, revision)(using testContext)
     cases.foreach { case (args, expected) =>
       val res = args match
         case path: String => links.pathTo(projectRoot.resolve(path))