diff --git a/metals/src/main/scala/scala/meta/internal/metals/ScaladocDefinitionProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/ScaladocDefinitionProvider.scala index 49d7cf288422..79c8bf25451e 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ScaladocDefinitionProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ScaladocDefinitionProvider.scala @@ -10,6 +10,7 @@ import scala.meta.Term import scala.meta.Tree import scala.meta.inputs.Input import scala.meta.inputs.Position +import scala.meta.internal.docstrings.WikiLink import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.mtags import scala.meta.internal.parsing.Trees @@ -272,18 +273,16 @@ case class ScalaDocLink(rawSymbol: String, isScala3: Boolean) { } object ScalaDocLink { - private val irrelevantWhite = "[ \\n\\t\\r]" - private val regex = s"\\[\\[$irrelevantWhite*(.*?)$irrelevantWhite*\\]\\]".r + // Extraction is shared with the renderer (`WikiLink`), so source + // go-to-definition navigates exactly the links the renderer renders — + // including `[[[ ... ]]]` and a link's title (scalameta/metals#3383). def atOffset( text: String, offset: Int, isScala3: Boolean, ): Option[ScalaDocLink] = - regex.findAllMatchIn(text).collectFirst { - case m if m.start(1) <= offset && offset <= m.end(1) => - ScalaDocLink(m.group(1), isScala3) - } + WikiLink.atOffset(text, offset).map(ScalaDocLink(_, isScala3)) sealed trait SymbolType object SymbolType { diff --git a/mtags/src/main/scala/scala/meta/internal/metals/docstrings/ScaladocParser.scala b/mtags/src/main/scala/scala/meta/internal/metals/docstrings/ScaladocParser.scala index dce8ce22c6b7..03633e744a30 100644 --- a/mtags/src/main/scala/scala/meta/internal/metals/docstrings/ScaladocParser.scala +++ b/mtags/src/main/scala/scala/meta/internal/metals/docstrings/ScaladocParser.scala @@ -17,9 +17,6 @@ import scala.meta.Position */ object ScaladocParser { - // Used in link() and for converting Javadoc @see to links - private val LinkPattern = """(.+?#?.+?(\([^\)]*\)?)?)((?:\s+)(.*))?""".r - /* Creates comments with necessary arguments */ def createComment( body0: Option[Body] = None, @@ -56,16 +53,13 @@ object ScaladocParser { case Summary(inline) => // Make sure Javadoc text is converted into links Summary(inline match { + // A quoted `@see "..."` string is plain text per the Javadoc + // spec, not a link (scalameta/metals#3383). + case Text(text) if text.trim.startsWith("\"") => + Text(text) case Text(text) => - text match { - case LinkPattern(link, _, _, title) => - Link( - link, - Option(title) map (Text.apply) getOrElse Text(link) - ) - case text => - Link(text, Text(text)) - } + val (target, title) = WikiLink.splitTargetTitle(text) + Link(target, Text(title.getOrElse(target))) case x => x }) case x => x @@ -1282,12 +1276,8 @@ object ScaladocParser { val link = readUntil { check(stop) } jump(stop) - link match { - case LinkPattern(link, _, _, title) => - Link(link, Option(title) map (Text.apply) getOrElse Text(link)) - case text => - Link(text, Text(text)) - } + val (target, title) = WikiLink.splitTargetTitle(link) + Link(target, Text(title.getOrElse(target))) } /* UTILITY */ diff --git a/mtags/src/main/scala/scala/meta/internal/metals/docstrings/WikiLink.scala b/mtags/src/main/scala/scala/meta/internal/metals/docstrings/WikiLink.scala new file mode 100644 index 000000000000..84cf4995d8f8 --- /dev/null +++ b/mtags/src/main/scala/scala/meta/internal/metals/docstrings/WikiLink.scala @@ -0,0 +1,81 @@ +package scala.meta.internal.docstrings + +/** + * Shared parsing of scaladoc entity (wiki) links `[[ ... ]]`, so the renderer + * and source-position extraction (go-to-definition) agree on a link's + * boundaries and its target/title split — instead of separate regexes that + * disagree on backticks, bracket nesting and parenthesised signatures + * (scalameta/metals#3383). + */ +object WikiLink { + + /** + * Splits a link's inner content into its target and optional title. The split + * is the first run of whitespace that lies OUTSIDE a backtick-escaped name and + * outside a parenthesised signature or type-argument list, so a backticked + * target containing a space (`` `my type` ``), a member signature with spaces + * (`foo(a: Int)`) and a type application (`foo[A, B](a: A)`) stay part of the + * target rather than being cut off as a title (scalameta/metals#3383). + */ + def splitTargetTitle(content: String): (String, Option[String]) = { + val n = content.length + var i = 0 + while (i < n && content.charAt(i).isWhitespace) i += 1 + val start = i + var inBacktick = false + var depth = 0 + var split = -1 + while (i < n && split < 0) { + content.charAt(i) match { + case '`' => inBacktick = !inBacktick + case '(' | '[' if !inBacktick => depth += 1 + case ')' | ']' if !inBacktick && depth > 0 => depth -= 1 + case c if !inBacktick && depth == 0 && c.isWhitespace => split = i + case _ => + } + i += 1 + } + if (split < 0) (content.substring(start).trim, None) + else { + val title = content.substring(split).trim + val titleOpt = if (title.isEmpty) None else Some(title) + (content.substring(start, split), titleOpt) + } + } + + /** + * The target of the entity link whose brackets (`[[ ... ]]`, or any `n >= 2` + * matching brackets, mirroring the renderer's grammar) enclose `offset`, so + * source go-to-definition navigates exactly the links the renderer renders, + * including the triple-bracket form the old regex truncated + * (scalameta/metals#3383). + */ + def atOffset(text: String, offset: Int): Option[String] = { + val n = text.length + var i = 0 + var result: Option[String] = None + while (i < n && result.isEmpty) { + if (text.charAt(i) == '[') { + var open = 0 + while (i + open < n && text.charAt(i + open) == '[') open += 1 + if (open >= 2) { + val contentStart = i + open + val end = text.indexOf("]" * open, contentStart) + if (end < 0) i += open + else { + // The link occupies the half-open range `[i, end + open)`; an + // inclusive upper bound would let the char right after the closing + // brackets (e.g. the opening `[` of an adjacent `[[A]][[B]]`) resolve + // to this link (scalameta/metals#3383). + if (offset >= i && offset < end + open) + result = Some( + splitTargetTitle(text.substring(contentStart, end))._1 + ) + i = end + open + } + } else i += 1 + } else i += 1 + } + result + } +} diff --git a/project/TestGroups.scala b/project/TestGroups.scala index 26e51975438e..92368505c29a 100644 --- a/project/TestGroups.scala +++ b/project/TestGroups.scala @@ -44,9 +44,10 @@ object TestGroups { "tests.debug.BreakpointScalaCliDapSuite", "tests.CallHierarchyLspSuite", "tests.BspStatusSuite", "tests.ServerLivenessMonitorLspSuite", "tests.ToplevelWithInnerScala2Suite", "tests.ScaladocSymbolsSuite", - "tests.SkipCommentsSuite", "tests.JarSourcesProviderSuite", - "tests.inlayHints.InlayHintsHoverSuite", "tests.Java8Suite", - "tests.RequestRegistrySuite", "tests.inlayHints.InlayHintsExpectSuite", + "tests.WikiLinkSuite", "tests.SkipCommentsSuite", + "tests.JarSourcesProviderSuite", "tests.inlayHints.InlayHintsHoverSuite", + "tests.Java8Suite", "tests.RequestRegistrySuite", + "tests.inlayHints.InlayHintsExpectSuite", "tests.worksheets.WorksheetInfiniteLoopSuite", "tests.TimeoutSuite", "tests.SingleFileSuite", "tests.SupportedScalaSuite", "tests.bestEffort.BestEffortCompilationSuite", diff --git a/tests/unit/src/test/scala/tests/DefinitionLspSuite.scala b/tests/unit/src/test/scala/tests/DefinitionLspSuite.scala index dd5c923f6e79..909781657165 100644 --- a/tests/unit/src/test/scala/tests/DefinitionLspSuite.scala +++ b/tests/unit/src/test/scala/tests/DefinitionLspSuite.scala @@ -616,6 +616,74 @@ class DefinitionLspSuite } yield () } + // Scaladoc also permits `[[[ ... ]]]` (any n >= 2 matching brackets); the old + // `\[\[ ... \]\]` regex truncated it to `[scala.Double` (scalameta/metals#3383). + test("scaladoc-definition-triple-bracket") { + val testCase = + """|package a + | + |object O { + | /** + | * Returns a [[[scala.Do@@uble]]] representing yada yada yada... + | */ + | def f: Double = ??? + |} + |""".stripMargin + for { + _ <- initialize( + s""" + |/metals.json + |{ + | "a": { } + |} + |/a/src/main/scala/a/Main.scala + |${testCase.replace("@@", "")} + |""".stripMargin + ) + _ <- server.didOpen("a/src/main/scala/a/Main.scala") + locations <- server.definition( + "a/src/main/scala/a/Main.scala", + testCase, + workspace, + ) + _ = assert(locations.nonEmpty) + _ = assert(locations.head.getUri().endsWith("scala/Double.scala")) + } yield () + } + + // The link's title is not part of the target, so a titled link still navigates + // (scalameta/metals#3383). + test("scaladoc-definition-link-title") { + val testCase = + """|package a + | + |object O { + | /** Returns a [[scala.Do@@uble the number]]. */ + | def f: Double = ??? + |} + |""".stripMargin + for { + _ <- initialize( + s""" + |/metals.json + |{ + | "a": { } + |} + |/a/src/main/scala/a/Main.scala + |${testCase.replace("@@", "")} + |""".stripMargin + ) + _ <- server.didOpen("a/src/main/scala/a/Main.scala") + locations <- server.definition( + "a/src/main/scala/a/Main.scala", + testCase, + workspace, + ) + _ = assert(locations.nonEmpty) + _ = assert(locations.head.getUri().endsWith("scala/Double.scala")) + } yield () + } + test("scaladoc-definition-this") { for { _ <- initialize( diff --git a/tests/unit/src/test/scala/tests/JavadocSuite.scala b/tests/unit/src/test/scala/tests/JavadocSuite.scala index b0ba87445d4d..ed999773f63c 100644 --- a/tests/unit/src/test/scala/tests/JavadocSuite.scala +++ b/tests/unit/src/test/scala/tests/JavadocSuite.scala @@ -36,6 +36,18 @@ class JavadocSuite extends BaseSuite { |- [previousProblemsFromSuccessfulCompilation](previousProblemsFromSuccessfulCompilation)""".stripMargin, ) + // A quoted `@see "..."` is plain text per the Javadoc spec, not a link + // (scalameta/metals#3383). + check( + "see-quoted-text", + """/** + |* @see "Effective Java" + |*/ + """.stripMargin, + """|**See** + |- "Effective Java"""".stripMargin, + ) + check( "escapee", """/** diff --git a/tests/unit/src/test/scala/tests/WikiLinkSuite.scala b/tests/unit/src/test/scala/tests/WikiLinkSuite.scala new file mode 100644 index 000000000000..8b01cfd6cc3d --- /dev/null +++ b/tests/unit/src/test/scala/tests/WikiLinkSuite.scala @@ -0,0 +1,66 @@ +package tests + +import scala.meta.internal.docstrings.WikiLink + +class WikiLinkSuite extends BaseSuite { + + private def checkSplit( + name: String, + content: String, + target: String, + title: Option[String], + ): Unit = + test(name)( + assertEquals(WikiLink.splitTargetTitle(content), (target, title)) + ) + + private def checkOffset( + name: String, + text: String, + offset: Int, + expected: Option[String], + ): Unit = + test(name)(assertEquals(WikiLink.atOffset(text, offset), expected)) + + checkSplit("split-plain", "scala.Foo", "scala.Foo", None) + checkSplit("split-title", "scala.Foo the foo", "scala.Foo", Some("the foo")) + // A backticked target keeps an embedded space instead of being split as a title. + checkSplit("split-backtick-space", "`my type`", "`my type`", None) + checkSplit( + "split-backtick-space-title", + "`my type` the type", + "`my type`", + Some("the type"), + ) + // Whitespace inside a parenthesised signature is part of the target. + checkSplit("split-paren-space", "foo(a: Int) bar", "foo(a: Int)", Some("bar")) + checkSplit("split-leading-ws", " scala.Foo ", "scala.Foo", None) + // Whitespace inside a type-argument list `[...]` is part of the target too, not a + // title boundary (scalameta/metals#3383). + checkSplit( + "split-type-args", + "foo[A, B](a: A) label", + "foo[A, B](a: A)", + Some("label"), + ) + checkSplit("split-type-args-only", "Map[K, V]", "Map[K, V]", None) + + checkOffset("offset-double", "see [[scala.Foo]] now", 12, Some("scala.Foo")) + // The triple-bracket form the old `[[ ]]` regex truncated to `[scala.Foo`. + checkOffset("offset-triple", "see [[[scala.Foo]]] now", 12, Some("scala.Foo")) + checkOffset("offset-backtick-space", "[[`my type`]]", 5, Some("`my type`")) + checkOffset( + "offset-with-title", + "[[scala.Foo the foo]]", + 5, + Some("scala.Foo"), + ) + checkOffset("offset-outside", "[[scala.Foo]] tail", 15, None) + checkOffset("offset-none", "no link here", 3, None) + // The char right after `]]` belongs to no link (and to the next one if + // adjacent), not to this link. + checkOffset("offset-after-close", "[[scala.Foo]] tail", 13, None) + checkOffset("offset-adjacent-first", "[[a.A]][[b.B]]", 2, Some("a.A")) + checkOffset("offset-adjacent-second-open", "[[a.A]][[b.B]]", 7, Some("b.B")) + checkOffset("offset-closing-bracket", "[[a.A]]", 6, Some("a.A")) +}