Skip to content

Commit

Permalink
fix: use alternative with stripped synthetic filename$package. for …
Browse files Browse the repository at this point in the history
…go to def and docs (#7000)
  • Loading branch information
kasiaMarek authored Dec 9, 2024
1 parent 55ba017 commit ac01eef
Show file tree
Hide file tree
Showing 11 changed files with 107 additions and 22 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package scala.meta.internal.metals

import java.util
import java.util.Collections

import scala.meta.pc.SymbolDocumentation

case class ProxySymbolDocumentation(orginalSymbol: String)
extends SymbolDocumentation {
override def symbol(): String = ""
override def displayName(): String = ""
override def docstring(): String = ""
override def defaultValue(): String = ""
override def typeParameters(): util.List[SymbolDocumentation] =
Collections.emptyList()
override def parameters(): util.List[SymbolDocumentation] =
Collections.emptyList()
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class CompletionItemResolver(
}
} else {
val companionDoc = docs(companion)
if (companionDoc.isEmpty) gsymDoc
if (companionDoc.isEmpty || companionDoc == gsymDoc) gsymDoc
else if (gsymDoc.isEmpty) companionDoc
else {
List(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,9 +230,10 @@ object MtagsEnrichments extends ScalametaCommonEnrichments:
def stripBackticks: String = s.stripPrefix("`").stripSuffix("`")

extension (search: SymbolSearch)
def symbolDocumentation(symbol: Symbol, contentType: ContentType = ContentType.MARKDOWN)(using
Context
): Option[SymbolDocumentation] =
def symbolDocumentation(
symbol: Symbol,
contentType: ContentType = ContentType.MARKDOWN
)(using Context): Option[SymbolDocumentation] =
def toSemanticdbSymbol(symbol: Symbol) =
SemanticdbSymbols.symbolName(
if !symbol.is(JavaDefined) && symbol.isPrimaryConstructor then
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ object CompletionItemResolver extends ItemResolver:
else gsymDoc
else
val companionDoc = docs(companion)
if companionDoc.isEmpty then gsymDoc
if companionDoc.isEmpty || companionDoc == gsymDoc then gsymDoc
else if gsymDoc.isEmpty then companionDoc
else
List(
Expand Down
43 changes: 34 additions & 9 deletions mtags/src/main/scala/scala/meta/internal/metals/Docstrings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,15 @@ class Docstrings(index: GlobalSymbolIndex) {
parents: ParentSymbols,
contentType: ContentType
): Optional[SymbolDocumentation] = {
val content = Content.from(symbol, contentType)
val result = cache.get(content) match {
val result = getFromCacheWithProxy(symbol, contentType) match {
case Some(value) =>
if (value == EmptySymbolDocumentation) None
else Some(value)
case None =>
indexSymbol(symbol, contentType)
val result = cache.get(content)
val result = getFromCacheWithProxy(symbol, contentType)
if (result.isEmpty)
cache(content) = EmptySymbolDocumentation
cache(Content.from(symbol, contentType)) = EmptySymbolDocumentation
result
}
/* Fall back to parent javadocs/scaladocs if nothing is specified for the current symbol
Expand Down Expand Up @@ -83,11 +82,9 @@ class Docstrings(index: GlobalSymbolIndex) {
.parents()
.asScala
.flatMap { s =>
val content = Content.from(s, contentType)
if (cache.contains(content)) cache.get(content)
else {
getFromCacheWithProxy(s, contentType).orElse {
indexSymbol(s, contentType)
cache.get(content)
getFromCacheWithProxy(s, contentType)
}
}
.find(_.docstring().nonEmpty)
Expand All @@ -100,6 +97,17 @@ class Docstrings(index: GlobalSymbolIndex) {
}
}

private def getFromCacheWithProxy(
symbol: String,
contentType: ContentType
): Option[SymbolDocumentation] = {
cache.get(Content.from(symbol, contentType)) match {
case Some(ProxySymbolDocumentation(alternativeSymbol)) =>
cache.get(Content.from(alternativeSymbol, contentType))
case res => res
}
}

/**
* Expire all symbols showed in the given scala source file.
*
Expand Down Expand Up @@ -130,6 +138,7 @@ class Docstrings(index: GlobalSymbolIndex) {
case Some(defn) =>
try {
indexSymbolDefinition(defn, contentType)
maybeCacheAlternative(defn, contentType)
} catch {
case NonFatal(e) =>
logger.log(Level.SEVERE, defn.path.toURI.toString, e)
Expand All @@ -138,6 +147,22 @@ class Docstrings(index: GlobalSymbolIndex) {
}
}

private def maybeCacheAlternative(
defn: SymbolDefinition,
contentType: ContentType
) = {
val defSymbol = defn.definitionSymbol.value
val querySymbol = defn.querySymbol.value
lazy val queryContent = Content.from(querySymbol, contentType)

if (
defSymbol != querySymbol && cache.get(queryContent).forall {
case EmptySymbolDocumentation | _: ProxySymbolDocumentation => true
case _ => false
}
) cache.put(queryContent, new ProxySymbolDocumentation(defSymbol))
}

private def indexSymbolDefinition(
defn: SymbolDefinition,
contentType: ContentType
Expand Down Expand Up @@ -166,7 +191,7 @@ class Docstrings(index: GlobalSymbolIndex) {
): Unit = {
for {
contentType <- ContentType.values()
} cache.remove(Content.from(occ.symbol, contentType))
} cache.remove(Content.from(sinfo.symbol, contentType))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ object DefinitionAlternatives {
*/
def apply(symbol: Symbol): List[Symbol] = {
List(
stripSyntheticPackageObjectFromObject(symbol),
caseClassCompanionToType(symbol),
caseClassApplyOrCopy(symbol),
caseClassApplyOrCopyParams(symbol),
Expand All @@ -26,6 +27,21 @@ object DefinitionAlternatives {
Some(sym.owner -> sym.value.desc)
}

private def stripSyntheticPackageObjectFromObject(
symbol: Symbol
): Option[Symbol] = {
val toplevel = symbol.toplevel
Option(toplevel).flatMap {
case GlobalSymbol(owner, Descriptor.Term(pkgName))
if pkgName.endsWith("$package") =>
val newSymbol =
Symbol(s"${owner.value}${symbol.value.stripPrefix(toplevel.value)}")
if (newSymbol.toplevel.isTerm) Some(newSymbol)
else None
case _ => None
}
}

/**
* If `case class A(a: Int)` and there is no companion object, resolve
* `A` in `A(1)` to the class definition.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,9 @@ trait ScalametaCommonEnrichments extends CommonMtagsEnrichments {

def filename: String = path.toNIO.filename

def scalaFileName: String =
path.filename.stripSuffix(".scala").stripSuffix(".sc")

def toIdeallyRelativeURI(sourceItemOpt: Option[AbsolutePath]): String =
sourceItemOpt match {
case Some(sourceItem) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,7 @@ class SymbolIndexBucket(
}
if (!definitions.contains(symbol.value)) {
// Fallback 3: guess related symbols from the enclosing class.
DefinitionAlternatives(symbol)
.flatMap(alternative => query0(querySymbol, alternative))
DefinitionAlternatives(symbol).flatMap(query0(querySymbol, _))
} else {
definitions
.get(symbol.value)
Expand Down
16 changes: 11 additions & 5 deletions tests/cross/src/main/scala/tests/pc/BaseHoverSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@ abstract class BaseHoverSuite
expected: String,
includeRange: Boolean = false,
automaticPackage: Boolean = true,
compat: Map[String, String] = Map.empty
compat: Map[String, String] = Map.empty,
repeat: Int = 1
)(implicit loc: Location): Unit = {
test(testOpt) {
assert(repeat > 0)
val filename = "Hover.scala"
val pkg = scala.meta.Term.Name(testOpt.name).syntax
val noRange = original
Expand All @@ -42,10 +44,14 @@ abstract class BaseHoverSuite
} else {
CompilerRangeParams(Paths.get(filename).toUri(), code, so, eo)
}
val hover = presentationCompiler
.hover(pcParams)
.get()
.asScala
val hover = (0 to repeat)
.map(_ =>
presentationCompiler
.hover(pcParams)
.get()
.asScala
)
.last
.map(_.toLsp())
val obtained: String = renderAsString(code, hover, includeRange)
assertNoDiff(
Expand Down
17 changes: 17 additions & 0 deletions tests/cross/src/test/scala/tests/hover/HoverScala3TypeSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -595,4 +595,21 @@ class HoverScala3TypeSuite extends BaseHoverSuite {
|""".stripMargin
)

check(
"i6852",
"""
|type Foo = String
|
|/** some doc */
|object Foo:
| /** doc doc */
| val fo@@o = ""
|""".stripMargin,
"""|```scala
|val foo: String
|```
|doc doc""".stripMargin,
repeat = 2
)

}
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite {
|
|
""".stripMargin,
"""|apply(viewId: String, nodeUri: String, label: String, command: String = ..., icon: String = ..., tooltip: String = ..., collapseState: String = ...): TreeViewNode
"""|apply(viewId: String, nodeUri: String, label: String, command: String = null, icon: String = null, tooltip: String = null, collapseState: String = null): TreeViewNode
| ^^^^^^^^^^^^^^
|""".stripMargin,
compat = Map(
Expand Down

0 comments on commit ac01eef

Please sign in to comment.