From 576bf9809733aa50f98297bf18bee742905ea36c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Sat, 5 Mar 2022 15:20:33 +0100 Subject: [PATCH 1/7] Go to definition of synthetic symbols. Previously, Metals didn't go the definition of symbols that the presentation compiler defines as synthetic. These symbols include auto-generated case class `apply` methods and regular class constructors. Now, we ignore the `isSynthetic` bit on the definition symbol so that "Go to definition" works as expected for cases where it didn't before. --- .../internal/pc/PcDefinitionProvider.scala | 3 +-- .../scala/tests/pc/PcDefinitionSuite.scala | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/mtags/src/main/scala-2/scala/meta/internal/pc/PcDefinitionProvider.scala b/mtags/src/main/scala-2/scala/meta/internal/pc/PcDefinitionProvider.scala index e92f54212a8..6efb85d2812 100644 --- a/mtags/src/main/scala-2/scala/meta/internal/pc/PcDefinitionProvider.scala +++ b/mtags/src/main/scala-2/scala/meta/internal/pc/PcDefinitionProvider.scala @@ -26,8 +26,7 @@ class PcDefinitionProvider(val compiler: MetalsGlobal, params: OffsetParams) { if ( tree.symbol == null || tree.symbol == NoSymbol || - tree.symbol.isErroneous || - tree.symbol.isSynthetic + tree.symbol.isErroneous ) { DefinitionResultImpl.empty } else if (tree.symbol.hasPackageFlag) { diff --git a/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala b/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala index 1bbcb8aa5ee..a98d3ffaa7b 100644 --- a/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala +++ b/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala @@ -351,4 +351,26 @@ class PcDefinitionSuite extends BasePcDefinitionSuite { |} |""".stripMargin ) + + check( + "synthetic-definition-case-class", + """| + |class Main { + | case class <<>>User(name: String, age: Int) + | def hello(u: User): Unit = () + | hello(Us@@er()) + |} + |""".stripMargin + ) + + check( + "synthetic-definition-class-constructor", + """| + |class Main { + | <> + | def hello(u: User): Unit = () + | hello(new Us@@er()) + |} + |""".stripMargin + ) } From 7871d17c7ea19d9b54c5af6025d242d7e5f5a0e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Sat, 5 Mar 2022 15:32:27 +0100 Subject: [PATCH 2/7] Allow definition on eta expanded wildcards This should be OK since the definition is at the same location as the call-site. --- .../src/test/scala/tests/pc/PcDefinitionSuite.scala | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala b/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala index a98d3ffaa7b..a074ef3483f 100644 --- a/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala +++ b/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala @@ -302,7 +302,16 @@ class PcDefinitionSuite extends BasePcDefinitionSuite { "eta", """| |object Main { - | List(1).map(@@_ + 2) + | List(1).map(<<>>@@_ + 2) + |} + |""".stripMargin + ) + + check( + "eta-2", + """| + |object Main { + | List(1).foldLeft(0)(_ + <<>>@@_) |} |""".stripMargin ) From 2c15b218d0d4785d102fd68446c1c479da966587 Mon Sep 17 00:00:00 2001 From: Vadim Chelyshov Date: Sat, 5 Mar 2022 22:47:56 +0300 Subject: [PATCH 3/7] Scala3 pc - go to definition for sythetic symbols --- .../internal/pc/PcDefinitionProvider.scala | 13 ++++++---- .../scala/tests/pc/PcDefinitionSuite.scala | 24 +++++++++++++++++-- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/PcDefinitionProvider.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/PcDefinitionProvider.scala index 862773aa2cd..22dd01d7cbd 100644 --- a/mtags/src/main/scala-3/scala/meta/internal/pc/PcDefinitionProvider.scala +++ b/mtags/src/main/scala-3/scala/meta/internal/pc/PcDefinitionProvider.scala @@ -130,13 +130,16 @@ class PcDefinitionProvider( case (imp: Import) :: _ => importedSymbols(imp, _.span.contains(pos.span)) - // For constructor calls, return the `` that was selected - case _ :: (_: New) :: (select: Select) :: _ => - List(select.symbol) - // wildcard param case head :: _ if (head.symbol.is(Param) && head.symbol.is(Synthetic)) => - Nil + List(head.symbol) + + case (head @ Select(target, name)) :: _ + if head.symbol.is(Synthetic) && name == StdNames.nme.apply => + println(head) + val sym = target.symbol + if sym.is(Synthetic) && sym.is(Module) then List(sym.companionClass) + else List(target.symbol) case head :: tl => if head.symbol.is(Synthetic) then enclosingSymbols(tl, pos, indexed) diff --git a/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala b/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala index a074ef3483f..2d32e92c34a 100644 --- a/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala +++ b/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala @@ -369,7 +369,17 @@ class PcDefinitionSuite extends BasePcDefinitionSuite { | def hello(u: User): Unit = () | hello(Us@@er()) |} - |""".stripMargin + |""".stripMargin, + compat = Map( + "3" -> + """| + |class Main { + | case class <>(name: String, age: Int) + | def hello(u: User): Unit = () + | hello(User()) + |} + |""".stripMargin + ) ) check( @@ -380,6 +390,16 @@ class PcDefinitionSuite extends BasePcDefinitionSuite { | def hello(u: User): Unit = () | hello(new Us@@er()) |} - |""".stripMargin + |""".stripMargin, + compat = Map( + "3" -> + """| + |class Main { + | class <>(name: String, age: Int) + | def hello(u: User): Unit = () + | hello(new Us@@er()) + |} + |""".stripMargin + ) ) } From 2392dec3dd888cfa16baf7d26225b1c05dc95528 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Mon, 7 Mar 2022 08:16:00 +0100 Subject: [PATCH 4/7] Focus the position from the presentation compiler --- .../scala-2/scala/meta/internal/pc/PcDefinitionProvider.scala | 2 +- tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mtags/src/main/scala-2/scala/meta/internal/pc/PcDefinitionProvider.scala b/mtags/src/main/scala-2/scala/meta/internal/pc/PcDefinitionProvider.scala index 6efb85d2812..7d5284e56e5 100644 --- a/mtags/src/main/scala-2/scala/meta/internal/pc/PcDefinitionProvider.scala +++ b/mtags/src/main/scala-2/scala/meta/internal/pc/PcDefinitionProvider.scala @@ -42,7 +42,7 @@ class PcDefinitionProvider(val compiler: MetalsGlobal, params: OffsetParams) { DefinitionResultImpl( semanticdbSymbol(tree.symbol), ju.Collections.singletonList( - new Location(params.uri().toString(), tree.symbol.pos.toLSP) + new Location(params.uri().toString(), tree.symbol.pos.focus.toLSP) ) ) } else { diff --git a/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala b/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala index a074ef3483f..0294d7e69ff 100644 --- a/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala +++ b/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala @@ -376,7 +376,7 @@ class PcDefinitionSuite extends BasePcDefinitionSuite { "synthetic-definition-class-constructor", """| |class Main { - | <> + | class <<>>User(name: String, age: Int) | def hello(u: User): Unit = () | hello(new Us@@er()) |} From 48f1f50f796c1c47085e7db4f392c4ab059bd810 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Mon, 7 Mar 2022 08:22:54 +0100 Subject: [PATCH 5/7] Make Scala 3 tests pass by adding compat assertions for the old behavior --- .../scala/tests/pc/PcDefinitionSuite.scala | 62 ++++++++++++++++--- 1 file changed, 54 insertions(+), 8 deletions(-) diff --git a/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala b/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala index 0294d7e69ff..54acd182534 100644 --- a/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala +++ b/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala @@ -30,13 +30,25 @@ class PcDefinitionSuite extends BasePcDefinitionSuite { """| |object Main { | for { - | <> <- List(1) + | <<>>x <- List(1) | y <- 1.to(x) | z = y + x | if y < @@x | } yield y |} - |""".stripMargin + |""".stripMargin, + compat = Map( + "3" -> + """|object Main { + | for { + | <> <- List(1) + | y <- 1.to(x) + | z = y + x + | if y < x + | } yield y + |} + |""".stripMargin + ) ) check( @@ -225,7 +237,7 @@ class PcDefinitionSuite extends BasePcDefinitionSuite { "named-arg-local", """| |object Main { - | <> + | def <<>>foo(arg: Int): Unit = () | | foo(a@@rg = 42) |} @@ -295,7 +307,15 @@ class PcDefinitionSuite extends BasePcDefinitionSuite { |object Main { | val n = ma@@th.max(1, 2) |} - |""".stripMargin + |""".stripMargin, + compat = Map( + "3" -> + """| + |object Main { + | val n = ma@@th.max(1, 2) + |} + |""".stripMargin + ) ) check( @@ -304,14 +324,22 @@ class PcDefinitionSuite extends BasePcDefinitionSuite { |object Main { | List(1).map(<<>>@@_ + 2) |} - |""".stripMargin + |""".stripMargin, + compat = Map( + "3" -> + """| + |object Main { + | List(1).map(@@_ + 2) + |} + |""".stripMargin + ) ) check( "eta-2", """| |object Main { - | List(1).foldLeft(0)(_ + <<>>@@_) + | List(1).foldLeft(0)(_ + @@_) |} |""".stripMargin ) @@ -369,7 +397,16 @@ class PcDefinitionSuite extends BasePcDefinitionSuite { | def hello(u: User): Unit = () | hello(Us@@er()) |} - |""".stripMargin + |""".stripMargin, + compat = Map( + "3" -> + """|class Main { + | case class User(name: String, age: Int) + | def <>(u: User): Unit = () + | hello(User()) + |} + |""".stripMargin + ) ) check( @@ -380,6 +417,15 @@ class PcDefinitionSuite extends BasePcDefinitionSuite { | def hello(u: User): Unit = () | hello(new Us@@er()) |} - |""".stripMargin + |""".stripMargin, + compat = Map( + "3" -> + """|class Main { + | class User(name: String, age: Int) + | def hello(u: User): Unit = () + | hello(new User()) + |} + |""".stripMargin + ) ) } From 1e03f807e052cbe47c6b62adee3e95b745b95812 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Mon, 7 Mar 2022 09:37:19 +0100 Subject: [PATCH 6/7] Remove unused compat assertion --- .../src/test/scala/tests/pc/PcDefinitionSuite.scala | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala b/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala index 694613edb82..1ed0c8e75c5 100644 --- a/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala +++ b/tests/cross/src/test/scala/tests/pc/PcDefinitionSuite.scala @@ -307,15 +307,7 @@ class PcDefinitionSuite extends BasePcDefinitionSuite { |object Main { | val n = ma@@th.max(1, 2) |} - |""".stripMargin, - compat = Map( - "3" -> - """| - |object Main { - | val n = ma@@th.max(1, 2) - |} - |""".stripMargin - ) + |""".stripMargin ) check( From a45d89482ce1a50eaf3e7a2cbde9b8bbb55ad337 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Mon, 7 Mar 2022 09:39:19 +0100 Subject: [PATCH 7/7] Remove println statement --- .../scala-3/scala/meta/internal/pc/PcDefinitionProvider.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/PcDefinitionProvider.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/PcDefinitionProvider.scala index 22dd01d7cbd..bac6fe49003 100644 --- a/mtags/src/main/scala-3/scala/meta/internal/pc/PcDefinitionProvider.scala +++ b/mtags/src/main/scala-3/scala/meta/internal/pc/PcDefinitionProvider.scala @@ -136,7 +136,6 @@ class PcDefinitionProvider( case (head @ Select(target, name)) :: _ if head.symbol.is(Synthetic) && name == StdNames.nme.apply => - println(head) val sym = target.symbol if sym.is(Synthetic) && sym.is(Module) then List(sym.companionClass) else List(target.symbol)