Skip to content

fix: [SemanticDB] Emit SymbolInformation and Occurrence for anonymous class #15865

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 13, 2022

Conversation

tanishiking
Copy link
Member

fix #15852

This commit enables ExtractSemanticDB to emit SymbolInformation and
SymbolOccurrence of anonymous classes.

e.g.

trait Foo:
  def foo: String

val foo1: Foo = new Foo { def foo: String = ??? }
val foo2: Foo = new { def foo: String = ??? }

While Scala2 semanticdb-plugin emit symbols for the anonymous class.

Lack of the symbol information / occurrence led Metals not being able to
find those anonymous classes as implementations by go-to-implementation from trait Foo.
see: scalameta/metals#4246

Confirmed this change fix the metals's go-to-implementation issue

by the following diff

diff --git a/project/V.scala b/project/V.scala
index 19fabea1db..be984e0bdd 100644
--- a/project/V.scala
+++ b/project/V.scala
@@ -6,7 +6,7 @@ object V {
   val scala212 = "2.12.16"
   val scala213 = "2.13.8"
   val scala3 = "3.1.3"
-  val nextScala3RC = "3.2.0-RC3"
+  val nextScala3RC = "3.2.1-RC1-bin-SNAPSHOT"
   val sbtScala = "2.12.14"
   val ammonite212Version = "2.12.16"
   val ammonite213Version = "2.13.8"
diff --git a/tests/unit/src/main/scala/tests/BaseRangesSuite.scala b/tests/unit/src/main/scala/tests/BaseRangesSuite.scala
index 8304e59eab..7bef87163a 100644
--- a/tests/unit/src/main/scala/tests/BaseRangesSuite.scala
+++ b/tests/unit/src/main/scala/tests/BaseRangesSuite.scala
@@ -50,8 +50,7 @@ abstract class BaseRangesSuite(name: String) extends BaseLspSuite(name) {
           s"""/metals.json
              |{"a":
              |  {
-             |    "scalaVersion" : "$actualScalaVersion",
-             |    "libraryDependencies": ${toJsonArray(libraryDependencies)}
+             |    "scalaVersion" : "$actualScalaVersion"
              |  }
              |}
              |${input
diff --git a/tests/unit/src/test/scala/tests/ImplementationLspSuite.scala b/tests/unit/src/test/scala/tests/ImplementationLspSuite.scala
index 4bc5dd599a..2da9827c0a 100644
--- a/tests/unit/src/test/scala/tests/ImplementationLspSuite.scala
+++ b/tests/unit/src/test/scala/tests/ImplementationLspSuite.scala
@@ -493,9 +493,22 @@ class ImplementationLspSuite extends BaseRangesSuite("implementation") {
        |package a
        |trait A@@nimal
        |object Main{
-       |  val animal = new <<>>Animal{ def field(d : String) : Int = 123 }
+       |  val animal = <<>>new Animal{ def field(d : String) : Int = 123 }
        |}
        |""".stripMargin,
+    scalaVersion = Some("3.2.1-RC3-bin-SNAPSHOT"),
+  )
+
+  check(
+    "anon2",
+    """|/a/src/main/scala/a/Main.scala
+       |package a
+       |trait A@@nimal
+       |object Main{
+       |  val animal: Animal = <<>>new <<>>{ def field(d : String) : Int = 123 }
+       |}
+       |""".stripMargin,
+    scalaVersion = Some("3.2.1-RC3-bin-SNAPSHOT"),
   )
 
   check(

@@ -178,7 +180,7 @@ class ExtractSemanticDB extends Phase:
if !excludeChildren(tree.symbol) then
traverseChildren(tree)
}
if !excludeDef(tree.symbol) && tree.span.hasLength then
if !excludeDef(tree.symbol) && (tree.span.hasLength || tree.symbol.isAnonymousClass) then
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change seems kind of monkey patch, maybe we should refactor ExtractSemanticDB at some point.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A redesign would be welcome - The only specification I used in the original implementation was comparing with the expect files from scalameta's semanticdb tests, so everything grew organically around that

fix scala#15852

This commit enables ExtractSemanticDB to emit SymbolInformation and
SymbolOccurrence of anonymous classes.

e.g.

```scala
trait Foo:
  def foo: String

val foo1: Foo = new Foo { def foo: String = ??? }
val foo2: Foo = new { def foo: String = ??? }
```

While Scala2 semanticdb-plugin emit symbols for the anonymous class.

Lack of the symbol information / occurrence led Metals not being able to
find those anonymous classes as implementations by `go-to-implementation` from `trait Foo`.
@bishabosha bishabosha force-pushed the semanticdb-anon-class branch from 0d5b775 to 3604d3c Compare September 13, 2022 14:50
@bishabosha
Copy link
Member

did a rebase due to conflict

@bishabosha bishabosha merged commit a3c0bef into scala:main Sep 13, 2022
@Kordyjan Kordyjan added this to the 3.2.2 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SemanticDB] ExtractSemanticDB doesn't emit symbol for anonymous class
3 participants