Scaladoc: Fix bugs found while setting up cats

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

@@ -169,6 +169,7 @@ object Scaladoc:
           CommentSyntax.default
         }
       }
+      val legacySourceLinkList = if legacySourceLink.get.nonEmpty then List(legacySourceLink.get) else Nil
       val externalMappings =
         externalDocumentationMappings.get.flatMap( s =>
             ExternalDocLink.parse(s).fold(left => {
@@ -208,7 +209,7 @@ object Scaladoc:
         projectLogo.nonDefault,
         projectFooter.nonDefault,
         parseSyntax,
-        sourceLinks.get,
+        sourceLinks.get ++ legacySourceLinkList,
         revision.nonDefault,
         externalMappings,
         socialLinksParsed,

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

@@ -34,13 +34,16 @@ class ScaladocSettings extends SettingGroup with AllScalaSettings:
     StringSetting("-project-version", "project version", "The current version of your project.", "", aliases = List("-doc-version"))
 
   val projectLogo: Setting[String] =
-    StringSetting("-project-logo", "project logo filename", "The file that contains the project's logo (in /images).", "", aliases = List("-doc-logo"))
+    StringSetting("-project-logo", "project logo filename", "Path to the file that contains the project's logo. Provided path can be absolute or relative to the project root directory.", "", aliases = List("-doc-logo"))
 
   val projectFooter: Setting[String] = StringSetting("-project-footer", "project footer", "A footer on every Scaladoc page.", "", aliases = List("-doc-footer"))
 
   val sourceLinks: Setting[List[String]] =
     MultiStringSetting("-source-links", "sources", SourceLinks.usage)
 
+  val legacySourceLink: Setting[String] =
+    StringSetting("-doc-source-url", "sources", "Legacy option from Scala 2. Use -source-links instead.", "")
+
   val syntax: Setting[String] =
     StringSetting("-comment-syntax", "syntax", "Syntax of the comment used", "")
 
@@ -124,4 +127,4 @@ class ScaladocSettings extends SettingGroup with AllScalaSettings:
     StringSetting("-scastie-configuration", "Scastie configuration", "Additional configuration passed to Scastie in code snippets", "")
 
   def scaladocSpecificSettings: Set[Setting[_]] =
-    Set(sourceLinks, syntax, revision, externalDocumentationMappings, socialLinks, skipById, skipByRegex, deprecatedSkipPackages, docRootContent, snippetCompiler, generateInkuire, scastieConfiguration)
+    Set(sourceLinks, legacySourceLink, syntax, revision, externalDocumentationMappings, socialLinks, skipById, skipByRegex, deprecatedSkipPackages, docRootContent, snippetCompiler, generateInkuire, scastieConfiguration)

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

@@ -22,10 +22,10 @@ case class TemplateSourceLink(val urlTemplate: String) extends SourceLink:
       "\\{\\{ path \\}\\}".r -> pathString,
       "\\{\\{ line \\}\\}".r -> line.fold("")(_.toString),
       "\\{\\{ ext \\}\\}".r -> Some(
-        pathString).filter(_.lastIndexOf(".") == -1).fold("")(p => p.substring(p.lastIndexOf("."))
+        pathString).filter(_.lastIndexOf(".") != -1).fold("")(p => p.substring(p.lastIndexOf("."))
       ),
       "\\{\\{ path_no_ext \\}\\}".r -> Some(
-        pathString).filter(_.lastIndexOf(".") == -1).fold(pathString)(p => p.substring(0, p.lastIndexOf("."))
+        pathString).filter(_.lastIndexOf(".") != -1).fold(pathString)(p => p.substring(0, p.lastIndexOf("."))
       ),
       "\\{\\{ name \\}\\}".r -> memberName
     )
@@ -43,7 +43,7 @@ case class WebBasedSourceLink(prefix: String, revision: String, subPath: String)
     s"$prefix/$action/$finalRevision$subPath/${pathToString(path)}$linePart"
 
 class SourceLinkParser(revision: Option[String]) extends ArgParser[SourceLink]:
-  val KnownProvider = raw"(\w+):\/\/([^\/#]+)\/([^\/#]+)(\/[^\/#]+)?(#.+)?".r
+  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
   val SupportedScalaDocPatternReplacements = Map(
@@ -63,6 +63,12 @@ class SourceLinkParser(revision: Option[String]) extends ArgParser[SourceLink]:
 
   def parse(string: String): Either[String, SourceLink] =
     val res = string match
+      case scaladocSetting if ScalaDocPatten.findFirstIn(scaladocSetting).nonEmpty =>
+        val all = ScalaDocPatten.findAllIn(scaladocSetting)
+        val (supported, unsupported) = all.partition(SupportedScalaDocPatternReplacements.contains)
+        if unsupported.nonEmpty then Left(s"Unsupported patterns from scaladoc format are used: ${unsupported.mkString(" ")}")
+        else Right(TemplateSourceLink(supported.foldLeft(string)((template, pattern) =>
+          template.replace(pattern, SupportedScalaDocPatternReplacements(pattern)))))
       case KnownProvider(name, organization, repo, rawRevision, rawSubPath) =>
         val subPath = Option(rawSubPath).fold("")("/" + _.drop(1))
         val pathRev = Option(rawRevision).map(_.drop(1)).orElse(revision)
@@ -81,12 +87,6 @@ class SourceLinkParser(revision: Option[String]) extends ArgParser[SourceLink]:
             Left(s"'$other' is not a known provider, please provide full source path template.")
       case BrokenKnownProvider("gitlab" | "github") =>
         Left(s"Does not match known provider syntax: `<name>://organization/repository`")
-      case scaladocSetting if ScalaDocPatten.findFirstIn(scaladocSetting).nonEmpty =>
-        val all = ScalaDocPatten.findAllIn(scaladocSetting)
-        val (supported, unsupported) = all.partition(SupportedScalaDocPatternReplacements.contains)
-        if unsupported.nonEmpty then Left(s"Unsupported patterns from scaladoc format are used: ${unsupported.mkString(" ")}")
-        else Right(TemplateSourceLink(supported.foldLeft(string)((template, pattern) =>
-          template.replace(pattern, SupportedScalaDocPatternReplacements(pattern)))))
       case other =>
         Left("Does not match any implemented source link syntax")
     res match {

scaladoc/src/dotty/tools/scaladoc/api.scala

@@ -195,6 +195,8 @@ extension[T] (member: Member)
 
   def withKind(kind: Kind): Member = member.copy(kind = kind)
 
+  def withDocs(docs: Option[Comment]) = member.copy(docs = docs)
+
   def withNewMembers(newMembers: Seq[Member]): Member =
     member.copy(members = member.members ++ newMembers)
 

scaladoc/src/dotty/tools/scaladoc/tasty/TastyParser.scala

@@ -160,7 +160,8 @@ case class ScaladocTastyInspector()(using ctx: DocContext) extends DocTastyInspe
     all.groupBy(_._1).map { case (pckName, members) =>
       val (pcks, rest) = members.map(_._2).partition(_.kind == Kind.Package)
       val basePck = pcks.reduce( (p1, p2) =>
-        p1.withNewMembers(p2.members) // TODO add doc
+        val withNewMembers = p1.withNewMembers(p2.members)
+        if withNewMembers.docs.isEmpty then withNewMembers.withDocs(p2.docs) else withNewMembers
       )
       basePck.withMembers((basePck.members ++ rest).sortBy(_.name))
     }.toList -> rootDoc

scaladoc/test/dotty/tools/scaladoc/source-links/SourceLinksTest.scala

@@ -33,6 +33,7 @@ class SourceLinkTest:
     Seq(
       "github://lampepfl/dotty",
       "gitlab://lampepfl/dotty",
+      "github://lampepfl/dotty/branch/withslash",
       "https://github.com/scala/scala/blob/2.13.x€{FILE_PATH_EXT}#€{FILE_LINE}"
     ).foreach{ template =>
       test(template)
@@ -45,12 +46,6 @@ class SourceLinkTest:
       val template = s"$provider://ala/ma"
       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 = SourceLinkParser(Some("develop")).parse(template)
-        assertTrue(s"Expected failure syntax info: $res", res.left.exists(_.contains("syntax")))
-      }
-
     }
 
 class SourceLinksTest:
@@ -121,7 +116,7 @@ class SourceLinksTest:
       ("project/Build.scala", 54, edit) -> "https://gitlab.com/lampepfl/dotty/-/edit/develop/project/Build.scala#L54",
     )
 
-    testLink(Seq("€{FILE_PATH}#€{FILE_LINE}"), Some("develop"))(
+    testLink(Seq("€{FILE_PATH}.scala#€{FILE_LINE}"), Some("develop"))(
       "project/Build.scala" -> "/project/Build.scala#",
       ("project/Build.scala", 54) -> "/project/Build.scala#54",
       ("project/Build.scala", edit) -> "/project/Build.scala#",
@@ -135,6 +130,20 @@ class SourceLinksTest:
       ("project/Build.scala", 54, edit) -> "https://github.com/scala/scala/blob/2.13.x/project/Build.scala#L54",
     )
 
+    testLink(Seq("https://github.com/scala/scala/blob/2.13.x€{FILE_PATH}.scala#L€{FILE_LINE}"), Some("develop"))(
+      "project/Build.scala" -> "https://github.com/scala/scala/blob/2.13.x/project/Build.scala#L",
+      ("project/Build.scala", 54) -> "https://github.com/scala/scala/blob/2.13.x/project/Build.scala#L54",
+      ("project/Build.scala", edit) -> "https://github.com/scala/scala/blob/2.13.x/project/Build.scala#L",
+      ("project/Build.scala", 54, edit) -> "https://github.com/scala/scala/blob/2.13.x/project/Build.scala#L54",
+    )
+
+    testLink(Seq("github://lampepfl/dotty/branch/withslash#src/lib"), None)(
+      "project/Build.scala" -> "https://github.com/lampepfl/dotty/blob/branch/withslash/src/lib/project/Build.scala",
+      ("project/Build.scala", 54) -> "https://github.com/lampepfl/dotty/blob/branch/withslash/src/lib/project/Build.scala#L54",
+      ("project/Build.scala", edit) -> "https://github.com/lampepfl/dotty/edit/branch/withslash/src/lib/project/Build.scala",
+      ("project/Build.scala", 54, edit) -> "https://github.com/lampepfl/dotty/edit/branch/withslash/src/lib/project/Build.scala#L54",
+    )
+
   @Test
   def testBasicPrefixedPaths =
     testLink(Seq("src=gitlab://lampepfl/dotty"), Some("develop"))(
@@ -148,7 +157,7 @@ class SourceLinksTest:
   @Test
   def prefixedPaths =
     testLink(Seq(
-     "src/generated=€{FILE_PATH}#€{FILE_LINE}",
+     "src/generated=€{FILE_PATH_EXT}#€{FILE_LINE}",
       "src=gitlab://lampepfl/dotty",
       "github://lampepfl/dotty"
       ), Some("develop"))(