improvement: Further heuristic improvements

Couple of related improvements: - escape operators, they are escaped for semanticdb symbols ( otherwise ??? will not be found) - add standard Scala imports (needed for scala.Predef.`???`().) - also try and guess method with (). ending - check if the returned symbol is exactly the same as we were looking for (could not reproduce it 100% the same as I encountered in the wild, but it's close enough)
scalameta · Aug 20, 2024 · b7b3ef5 · b7b3ef5
1 parent ead4e2e
commit b7b3ef5
Show file tree

Hide file tree

Showing 4 changed files with 91 additions and 10 deletions.
diff --git a/metals/src/main/scala/scala/meta/internal/metals/FallbackDefinitionProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/FallbackDefinitionProvider.scala
@@ -56,7 +56,7 @@ class FallbackDefinitionProvider(
           .value
         if (isInSelectPosition) List(symbolPrefix + ".")
         else if (isInTypePosition) List(symbolPrefix + "#")
-        else List(".", "#").map(ending => symbolPrefix + ending)
+        else List(".", "#", "().").map(ending => symbolPrefix + ending)
       }
 
       // Get all select parts to build symbol from it later
@@ -114,16 +114,26 @@ class FallbackDefinitionProvider(
             }.flatten
         }.flatten
 
+      val standardScalaImports = guessObjectOrClass(
+        List("scala", "Predef") ++ proposedNameParts
+      )
       val fullyScopedName =
         guessObjectOrClass(proposedNameParts)
 
+      def findInIndex(proposedSymbol: String) = {
+        index
+          .definition(mtags.Symbol(proposedSymbol))
+          // Make sure we don't return unrelated definitions
+          .filter { _.definitionSymbol.value == proposedSymbol }
+      }
       val nonLocalGuesses =
-        (proposedImportedSymbols ++ fullyScopedName).distinct
+        (proposedImportedSymbols ++ fullyScopedName ++ standardScalaImports).distinct
           .flatMap { proposedSymbol =>
-            index.definition(mtags.Symbol(proposedSymbol))
+            findInIndex(proposedSymbol)
           }
 
       def toDefinition(guesses: List[mtags.SymbolDefinition]) = {
+
         DefinitionResult(
           guesses
             .flatMap(guess =>
@@ -143,8 +153,10 @@ class FallbackDefinitionProvider(
       } else {
         // otherwise might be symbol in a local package, starting from enclosing
         proposedCurrentPackageSymbols.reverse
-          .map(proposedSymbol => index.definition(mtags.Symbol(proposedSymbol)))
-          .collectFirst { case Some(dfn) => toDefinition(List(dfn)) }
+          .map(proposedSymbol => findInIndex(proposedSymbol))
+          .collectFirst { case Some(dfn) =>
+            toDefinition(List(dfn))
+          }
       }
 
       result.foreach { _ =>

diff --git a/mtags-shared/src/main/scala/scala/meta/internal/mtags/KeywordWrapper.scala b/mtags-shared/src/main/scala/scala/meta/internal/mtags/KeywordWrapper.scala
@@ -11,9 +11,12 @@ trait KeywordWrapper {
 
   def keywords: Set[String]
 
-  final def needsBacktick(s: String): Boolean = {
+  final def needsBacktick(
+      s: String,
+      wrapOperators: Boolean = false
+  ): Boolean = {
     val chunks = s.split("_", -1)
-    def validOperator(c: Char) = {
+    def validOperator(c: Char) = !wrapOperators && {
       c.getType == Character.MATH_SYMBOL ||
       c.getType == Character.OTHER_SYMBOL ||
       "!#%&*+-/:<=>?@\\^|~".contains(c)
@@ -45,12 +48,13 @@ trait KeywordWrapper {
 
   final def backtickWrap(
       s: String,
-      exclusions: Set[String] = Set.empty
+      exclusions: Set[String] = Set.empty,
+      wrapOperators: Boolean = false
   ): String = {
     if (exclusions.contains(s)) s
     else if (s.isEmpty) "``"
     else if (s(0) == '`' && s.last == '`') s
-    else if (needsBacktick(s)) "" + ('`') + s + '`'
+    else if (needsBacktick(s, wrapOperators)) "" + ('`') + s + '`'
     else s
   }
 }

diff --git a/mtags/src/main/scala/scala/meta/internal/mtags/Symbol.scala b/mtags/src/main/scala/scala/meta/internal/mtags/Symbol.scala
@@ -161,7 +161,8 @@ object Symbol {
       case ((prefix, insideObject), next) =>
         val name = keywordWrapper.backtickWrap(
           next.replace("\\", ""),
-          Set("this", "package")
+          Set("this", "package"),
+          wrapOperators = true
         )
         if (prefix.isEmpty())
           (name, /*insideObject =*/ name.find(_.isLetter).exists(_.isUpper))

diff --git a/tests/slow/src/test/scala/tests/feature/DefinitionCrossLspSuite.scala b/tests/slow/src/test/scala/tests/feature/DefinitionCrossLspSuite.scala
@@ -638,5 +638,69 @@ class DefinitionCrossLspSuite
         )
       } yield ()
     }
+
+    test(s"fallback-object-imports-$version") {
+      cleanWorkspace()
+      for {
+        _ <- initialize(
+          s"""
+             |/metals.json
+             |{
+             |  "a": { "scalaVersion" : "${version}" }
+             |}
+             |/a/src/main/scala/a/Main.scala
+             |package a
+             |import b.Foo._
+             |
+             |object Main {
+             |  val name: Version = ???
+             |}
+             |
+             |/a/src/main/scala/a/b/Foo.scala
+             |package a.b
+             |
+             |trait Foo
+             |object Foo extends Foo{
+             |  val a = 123
+             |}
+             |
+             |""".stripMargin
+        )
+        _ <- server.didOpen("a/src/main/scala/a/Main.scala")
+        _ <- server.didOpen("a/src/main/scala/a/b/Foo.scala")
+        _ = assertNoDiff(
+          client.workspaceDiagnostics,
+          if (version == BuildInfo.scala3)
+            """|a/src/main/scala/a/Main.scala:5:13: error: Not found: type Version
+               |  val name: Version = ???
+               |            ^^^^^^^
+               |""".stripMargin
+          else
+            """|a/src/main/scala/a/Main.scala:5:13: error: not found: type Version
+               |  val name: Version = ???
+               |            ^^^^^^^
+               |""".stripMargin,
+        )
+        _ = assertNoDiff(
+          server.workspaceDefinitions,
+          """|/a/src/main/scala/a/Main.scala
+             |package a
+             |import b/*<no symbol>*/.Foo/*Foo.scala:3*/._
+             |
+             |object Main/*L3*/ {
+             |  val name/*L4*/: Version/*<no symbol>*/ = ???/*Predef.scala*/
+             |}
+             |
+             |/a/src/main/scala/a/b/Foo.scala
+             |package a.b
+             |
+             |trait Foo/*L2*/
+             |object Foo/*L3*/ extends Foo/*L2*/{
+             |  val a/*L4*/ = 123
+             |}
+             |""".stripMargin,
+        )
+      } yield ()
+    }
   }
 }