Skip to content

Commit

Permalink
improvement: Handle multiple package statements
Browse files Browse the repository at this point in the history
  • Loading branch information
tgodzik committed Aug 1, 2024
1 parent d9220f8 commit da5c5f4
Show file tree
Hide file tree
Showing 7 changed files with 180 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,9 @@ final class DefinitionProvider(
Future.successful(defn)
}

// pprint.log(params)
fromCompilerOrSemanticdb.map { definition =>
// pprint.log(definition)
if (definition.isEmpty && !definition.symbol.endsWith("/")) {
val isScala3 =
ScalaVersions.isScala3Version(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,21 @@ class FallbackDefinitionProvider(
}
.getOrElse(List(ident.value))

val proposedCurrentPackageSymbol =
guessObjectOrClass(
trees
.packageAtPosition(path, pos)
.getOrElse("_empty_") +: proposedNameParts
val currentPackageStatements = trees
.packageStatementsAtPosition(path, pos) match {
case None => List("_empty_")
case Some(value) =>
// generate packages from all the package statements
value.foldLeft(Seq.empty[String]) { case (pre, suffix) =>
if (pre.isEmpty) List(suffix) else pre :+ (pre.last + "." + suffix)
}
}

val proposedCurrentPackageSymbols =
currentPackageStatements.flatMap(pkg =>
guessObjectOrClass(
(pkg.split("\\.").toList ++ proposedNameParts)
)
)

// First name in select is the one that must be imported or in scope
Expand Down Expand Up @@ -107,14 +117,13 @@ class FallbackDefinitionProvider(
val fullyScopedName =
guessObjectOrClass(proposedNameParts)

val guesses =
(proposedImportedSymbols ++ proposedCurrentPackageSymbol ++ fullyScopedName).distinct
val nonLocalGuesses =
(proposedImportedSymbols ++ fullyScopedName).distinct
.flatMap { proposedSymbol =>
index.definition(mtags.Symbol(proposedSymbol))
}

if (guesses.nonEmpty) {
scribe.warn(s"Using indexes to guess the definition of ${ident.value}")
def toDefinition(guesses: List[mtags.SymbolDefinition]) = {
Some(
DefinitionResult(
guesses
Expand All @@ -130,9 +139,28 @@ class FallbackDefinitionProvider(
ident.value,
)
)
} else None
}
val result = if (nonLocalGuesses.nonEmpty) {
toDefinition(nonLocalGuesses)
} else {
// otherwise might be symbol in a local package, starting from enclosing
proposedCurrentPackageSymbols.reverse
.map(proposedSymbol => index.definition(mtags.Symbol(proposedSymbol)))
.find(_.nonEmpty)
.flatten
.map(dfn => toDefinition(List(dfn)))
.getOrElse(None)
}

result.foreach { _ =>
scribe.warn(
s"Could not find '${ident.value}' using presentation compiler nor semanticdb. " +
s"Trying to guess the definition using available information from local class context. "
)
}
result
}

defResult.flatten
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ final class FileSystemSemanticdbs(
}
} yield {
if (!targetroot.exists)
scribe.warn(s"Target root $targetroot does not exist")
scribe.debug(s"Target root $targetroot does not exist")
val optScalaVersion =
if (file.toLanguage.isJava) None
else buildTargets.scalaTarget(buildTarget).map(_.scalaVersion)
Expand Down
18 changes: 11 additions & 7 deletions metals/src/main/scala/scala/meta/internal/parsing/Trees.scala
Original file line number Diff line number Diff line change
Expand Up @@ -54,31 +54,35 @@ final class Trees(
}
}

def packageAtPosition(
def packageStatementsAtPosition(
source: AbsolutePath,
lspPos: l.Position,
): Option[String] = {
): Option[List[String]] = {

def loop(t: Tree, pos: Position, acc: String): Option[String] = {
def loop(
t: Tree,
pos: Position,
acc: List[String],
): Option[List[String]] = {
t match {
case t: Pkg =>
val enclosed = enclosedChildren(t.children, pos)
enclosed
.map(loop(_, pos, acc + "." + t.ref.toString()))
.map(loop(_, pos, acc :+ t.ref.toString()))
.headOption
.flatten
case other =>
val enclosed = enclosedChildren(other.children, pos)
if (enclosed.isEmpty) Some(acc)
else enclosed.map(loop(_, pos, acc)).headOption.flatten
else enclosed.flatMap(loop(_, pos, acc)).headOption
}
}

for {
tree <- get(source)
pos <- lspPos.toMeta(tree.pos.input)
lastEnc <- loop(tree, pos, "")
} yield lastEnc.stripPrefix(".")
lastEnc <- loop(tree, pos, Nil)
} yield lastEnc
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class PcDefinitionProvider(val compiler: MetalsGlobal, params: OffsetParams) {
if (
symbol == null ||
symbol == NoSymbol ||
symbol.isErroneous
symbol.isError
) {
DefinitionResultImpl.empty
} else if (symbol.hasPackageFlag) {
Expand Down
6 changes: 3 additions & 3 deletions mtags/src/main/scala/scala/meta/internal/mtags/Symbol.scala
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,9 @@ object Symbol {
*
* We assume that any lower case parts at the start are packages,
* everything later is either a class/object and then things
* the can belong to them. We don't care about things inside of
* classes, since they need to be accessed with a select on a typed
* variable instead.
* the can belong to them. Currently we will not guess if
* a identifier actually belongs to a class, we assume that
* we can only check fields inside objects.
*
* @param path a.b.c.MyClass.myMethod
* @param isScala3 true if the path is from a Scala 3 file
Expand Down
131 changes: 124 additions & 7 deletions tests/unit/src/test/scala/tests/DefinitionLspSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ class DefinitionLspSuite
|import java.util.concurrent.Future/*Future.java*/ // unused
|import scala.util.Failure/*Try.scala*/ // unused
|object Main/*L5*/ extends App/*App.scala*/ {
| val helloMessage/*<no symbol>*/ = Message/*Message.java:1*/.message/*Message.java:2*/
| val helloMessage/*L6*/ = Message/*Message.java:1*/.message/*Message.java:2*/
| new java.io.PrintStream/*PrintStream.java*/(new java.io.ByteArrayOutputStream/*ByteArrayOutputStream.java*/())
| println/*Predef.scala*/(message/*<no symbol>*/)
|}
Expand Down Expand Up @@ -422,10 +422,10 @@ class DefinitionLspSuite
|import a/*<no symbol>*/.Main/*Main.scala:3*/.Bar/*Main.scala:6*/
|
|object Foo/*L4*/{
| val ver/*<no symbol>*/: Version/*<no symbol>*/ = null
| val nm/*<no symbol>*/ = Main/*Main.scala:3*/.name/*Main.scala:5*/
| val foo/*<no symbol>*/ = Bar/*Main.scala:6*/()
| val m/*<no symbol>*/: Main/*Main.scala:2*/ = new Main/*Main.scala:2*/()
| val ver/*L5*/: Version/*<no symbol>*/ = null
| val nm/*L6*/ = Main/*Main.scala:3*/.name/*Main.scala:5*/
| val foo/*L7*/ = Bar/*Main.scala:6*/()
| val m/*L8*/: Main/*Main.scala:2*/ = new Main/*Main.scala:2*/()
|}
|""".stripMargin,
)
Expand Down Expand Up @@ -505,12 +505,12 @@ class DefinitionLspSuite
|
|object Foo/*L2*/ {
| import a/*<no symbol>*/.Main/*Main.scala:2*/.Other/*Main.scala:4*/
| val other/*<no symbol>*/ = Other/*Main.scala:4*/
| val other/*L4*/ = Other/*Main.scala:4*/
| a/*<no symbol>*/.Main/*Main.scala:2*/.`na-me`/*Main.scala:3*/
|}
|object Foo2/*L7*/ {
| import a/*<no symbol>*/.Main2/*Main.scala:7*/.Other/*Main.scala:8*/
| val other/*<no symbol>*/ = Other/*Main.scala:8*/
| val other/*L9*/ = Other/*Main.scala:8*/
|}
|object Foo3/*L11*/ {
| val Foo/*L12*/ = ""
Expand All @@ -521,6 +521,123 @@ class DefinitionLspSuite
} yield ()
}

test("fallback-to-workspace-search-packages") {
cleanWorkspace()
for {
_ <- initialize(
"""
|/metals.json
|{
| "a": {}
|}
|/a/src/main/scala/a/Main.scala
|package a
|
|object Main {
| val name: Int = "John"
|}
|
|/a/src/main/scala/a/b/Foo.scala
|package a
|package b
|
|object Foo {
| Main.name
|}
|
|/a/src/main/scala/a/b/Bar.scala
|package a.b
|
|object Bar {
| Main.name
|}
|
|/a/src/main/scala/a/b/c/OtherMain.scala
|package a.b.c
|
|object Main {
| val name: Int = "Lemon"
|}
|
|/a/src/main/scala/a/b/c/d/Baz.scala
|package a
|package b
|package c
|package d
|
|object Baz {
| Main.name
|}
|
|""".stripMargin
)
_ <- server.didOpen("a/src/main/scala/a/Main.scala")
_ <- server.didOpen("a/src/main/scala/a/b/Foo.scala")
_ <- server.didOpen("a/src/main/scala/a/b/Bar.scala")
_ <- server.didOpen("a/src/main/scala/a/b/c/OtherMain.scala")
_ <- server.didOpen("a/src/main/scala/a/b/c/d/Baz.scala")
_ = assertNoDiff(
client.workspaceDiagnostics,
"""|a/src/main/scala/a/Main.scala:4:19: error: type mismatch;
| found : String("John")
| required: Int
| val name: Int = "John"
| ^^^^^^
|a/src/main/scala/a/b/Bar.scala:4:3: error: not found: value Main
| Main.name
| ^^^^
|a/src/main/scala/a/b/c/OtherMain.scala:4:19: error: type mismatch;
| found : String("Lemon")
| required: Int
| val name: Int = "Lemon"
| ^^^^^^^
|""".stripMargin,
)
_ = assertNoDiff(
server.workspaceDefinitions,
"""|/a/src/main/scala/a/Main.scala
|package a
|
|object Main/*L2*/ {
| val name/*L3*/: Int/*Int.scala*/ = "John"
|}
|
|/a/src/main/scala/a/b/Bar.scala
|package a.b
|
|object Bar/*L2*/ {
| Main/*<no symbol>*/.name/*<no symbol>*/
|}
|
|/a/src/main/scala/a/b/Foo.scala
|package a
|package b
|
|object Foo/*L3*/ {
| Main/*Main.scala:2*/.name/*Main.scala:3*/
|}
|
|/a/src/main/scala/a/b/c/OtherMain.scala
|package a.b.c
|
|object Main/*L2*/ {
| val name/*L3*/: Int/*Int.scala*/ = "Lemon"
|}
|
|/a/src/main/scala/a/b/c/d/Baz.scala
|package a
|package b
|package c
|package d
|
|object Baz/*L5*/ {
| Main/*OtherMain.scala:2*/.name/*OtherMain.scala:3*/
|}
|""".stripMargin,
)
} yield ()
}

test("rambo", withoutVirtualDocs = true) {
cleanDatabase()
for {
Expand Down

0 comments on commit da5c5f4

Please sign in to comment.