Skip to content

Commit

Permalink
fix: try pc symbol before symbol alternatives in go to definition
Browse files Browse the repository at this point in the history
  • Loading branch information
kasiaMarek committed Apr 25, 2024
1 parent 811d738 commit ed9d23d
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,7 @@ class Compilers(
c.symbol(),
definitionPath,
None,
c.symbol(),
)
}
}.getOrElse(Future.successful(DefinitionResult.empty))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,21 @@ final class DefinitionProvider(
DefinitionResult.empty
}
val fromCompilerOrSemanticdb =
if (fromSnapshot.isEmpty && path.isScalaFilename) {
compilers().definition(params, token)
} else {
if (fromSemanticdb.isEmpty) {
warnings.noSemanticdb(path)
}
Future.successful(fromSnapshot)
fromSnapshot match {
case defn if defn.isEmpty && path.isScalaFilename =>
compilers().definition(params, token)
case defn @ DefinitionResult(_, symbol, _, _, querySymbol)
if symbol != querySymbol && path.isScalaFilename =>
compilers().definition(params, token).map { compilerDefn =>
if (compilerDefn.isEmpty || compilerDefn.querySymbol == querySymbol)
defn
else compilerDefn.copy(semanticdb = defn.semanticdb)
}
case defn =>
if (fromSemanticdb.isEmpty) {
warnings.noSemanticdb(path)
}
Future.successful(defn)
}

fromCompilerOrSemanticdb.map { definition =>
Expand Down Expand Up @@ -202,6 +210,7 @@ final class DefinitionProvider(
ident.value,
None,
None,
ident.value,
)
)
} else None
Expand Down Expand Up @@ -369,6 +378,7 @@ final class DefinitionProvider(
occ.symbol,
None,
dirtyPosition.getTextDocument.getUri,
occ.symbol,
).toResult
} else {
// symbol is global so it is defined in an external destination buffer.
Expand Down Expand Up @@ -404,6 +414,7 @@ case class DefinitionDestination(
symbol: String,
path: Option[AbsolutePath],
uri: String,
querySymbol: String,
) {

/**
Expand All @@ -423,6 +434,7 @@ case class DefinitionDestination(
symbol,
path,
Some(snapshot),
querySymbol,
)
}

Expand Down Expand Up @@ -532,9 +544,10 @@ class DestinationProvider(
Some(
DefinitionResult(
ju.Collections.singletonList(range.toLocation(uri)),
symbol,
defn.definitionSymbol.value,
Some(defn.path),
None,
defn.querySymbol.value,
)
)
case _ =>
Expand All @@ -551,6 +564,7 @@ class DestinationProvider(
defn.definitionSymbol.value,
Some(destinationPath),
uri,
defn.querySymbol.value,
).toResult
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,20 @@ case class DefinitionResult(
symbol: String,
definition: Option[AbsolutePath],
semanticdb: Option[TextDocument],
querySymbol: String,
) {
def isEmpty: Boolean = locations.isEmpty()
def ++(other: DefinitionResult) = DefinitionResult(
(locations.asScala ++ other.locations.asScala).asJava,
symbol,
definition,
semanticdb,
querySymbol,
)
}

object DefinitionResult {
def empty(symbol: String): DefinitionResult =
DefinitionResult(Collections.emptyList(), symbol, None, None)
DefinitionResult(Collections.emptyList(), symbol, None, None, symbol)
def empty: DefinitionResult = empty(Symbols.None)
}
Original file line number Diff line number Diff line change
Expand Up @@ -2704,6 +2704,7 @@ class MetalsLspService(
symbol = results.head.symbol,
definition = None,
semanticdb = None,
querySymbol = results.head.symbol,
)
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ class ScaladocDefinitionProvider(
results.head.symbol,
None,
None,
results.head.querySymbol,
)
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,12 @@ final class RenameProvider(
Future
.sequence(allReferences)
.map(locs =>
(locs.flatten, symbolOccurrence, definition, newName)
(
locs.flatten.filterNot(_.getRange().isOffset),
symbolOccurrence,
definition,
newName,
)
)
}
.map {
Expand Down
43 changes: 43 additions & 0 deletions tests/unit/src/test/scala/tests/DefinitionLspSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -750,4 +750,47 @@ class DefinitionLspSuite
} yield ()
}

test("go-to-reexported-symbol") {
val testCase =
"""|package a
|class Test extends A {
| assert("Hello".fo@@o == "HelloFoo")
|}
|
|trait A {
| export B.*
|}
|""".stripMargin
for {
_ <- initialize(
s"""
|/metals.json
|{
| "a": { "scalaVersion": "3.4.1" }
|}
|/a/src/main/scala/a/Main.scala
|${testCase.replace("@@", "")}
|/a/src/main/scala/a/Other.scala
|package a
|
|object B {
| extension (value: String) def foo: String = s"$${value}Foo"
|}
|""".stripMargin
)
_ <- server.didOpen("a/src/main/scala/a/Other.scala")
locations <- server.definition(
"a/src/main/scala/a/Main.scala",
testCase,
workspace,
)
_ = assert(locations.length == 1)
_ = assert(locations.forall(_.getUri().endsWith("a/Other.scala")))
_ = assertEquals(
locations.map(_.getRange().getStart().getLine()),
List(3),
)
} yield ()
}

}

0 comments on commit ed9d23d

Please sign in to comment.