Add symbol occurrence for types inside of classOf
#2457
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tgodzik
requested changes
Aug 24, 2021
| @@ -56,7 +56,7 @@ object Test/*<=types.Test.*/ { | |||
| val compoundType2/*<=types.Test.C#compoundType2.*/: M/*=>types.Test.M#*/ with N/*=>types.Test.N#*/ = ???/*=>scala.Predef.`???`().*/ | |||
| val compoundType3/*<=types.Test.C#compoundType3.*/: M/*=>types.Test.M#*/ with N/*=>types.Test.N#*/ { def k/*<=local1*/: Int/*=>scala.Int#*/ } = ???/*=>scala.Predef.`???`().*/ | |||
| val compoundType4/*<=types.Test.C#compoundType4.*/ = new /*<=local2*/{ def k/*<=local3*/: Int/*=>scala.Int#*/ = ???/*=>scala.Predef.`???`().*/ } | |||
| val compoundType5/*<=types.Test.C#compoundType5.*/ = new M/*=>types.Test.M#*/ with N/*=>types.Test.N#*/ | |||
| val compoundType5/*<=types.Test.C#compoundType5.*/ = new /*<=local4*/M/*=>types.Test.M#*/ with N/*=>types.Test.N#*/ | |||
There was a problem hiding this comment.
Suggested change
| val compoundType5/*<=types.Test.C#compoundType5.*/ = new /*<=local4*/M/*=>types.Test.M#*/ with N/*=>types.Test.N#*/ | |
| val compoundType5/*<=types.Test.C#compoundType5.*/ = new M/*=>types.Test.M#*/ with N/*=>types.Test.N#*/ |
This should not be added, we might need to filter it out in the g.Constant match case.
There was a problem hiding this comment.
Looks like this is irrelevant to this change 🤔
I'm not sure when there's symbol occurrence for new literal, but I just added the following two lines of code (in the latest main branch) to Types.scala and run ++2.12.14 -> save-expect.
final val clazzOfJStr = classOf[java.lang.String]
final val clazzOfM = classOf[M]And I still get
val compoundType5/*<=types.Test.C#compoundType5.*/ = new /*<=local4*/M/*=>types.Test.M#*/ with N/*=>types.Test.N#*/Maybe we should figure out what's going on and fix it in another issue/PR?
diff
diff --git a/semanticdb/integration/src/main/scala/example/Types.scala b/semanticdb/integration/src/main/scala/example/Types.scala
index be2826873..d63a65cca 100644
--- a/semanticdb/integration/src/main/scala/example/Types.scala
+++ b/semanticdb/integration/src/main/scala/example/Types.scala
@@ -114,5 +114,7 @@ object Test {
final val clazzOfInt = classOf[Int]
final val clazzOfOption = classOf[Option[Int]]
+ final val clazzOfJStr = classOf[java.lang.String]
+ final val clazzOfM = classOf[M]
}
}
diff --git a/tests/jvm/src/test/resources/example/Types.scala b/tests/jvm/src/test/resources/example/Types.scala
index 1ebeb095a..5fe4bae11 100644
--- a/tests/jvm/src/test/resources/example/Types.scala
+++ b/tests/jvm/src/test/resources/example/Types.scala
@@ -56,7 +56,7 @@ object Test/*<=types.Test.*/ {
val compoundType2/*<=types.Test.C#compoundType2.*/: M/*=>types.Test.M#*/ with N/*=>types.Test.N#*/ = ???/*=>scala.Predef.`???`().*/
val compoundType3/*<=types.Test.C#compoundType3.*/: M/*=>types.Test.M#*/ with N/*=>types.Test.N#*/ { def k/*<=local1*/: Int/*=>scala.Int#*/ } = ???/*=>scala.Predef.`???`().*/
val compoundType4/*<=types.Test.C#compoundType4.*/ = new /*<=local2*/{ def k/*<=local3*/: Int/*=>scala.Int#*/ = ???/*=>scala.Predef.`???`().*/ }
- val compoundType5/*<=types.Test.C#compoundType5.*/ = new M/*=>types.Test.M#*/ with N/*=>types.Test.N#*/
+ val compoundType5/*<=types.Test.C#compoundType5.*/ = new /*<=local4*/M/*=>types.Test.M#*/ with N/*=>types.Test.N#*/
val compoundType6/*<=types.Test.C#compoundType6.*/ = new /*<=local5*/M/*=>types.Test.M#*/ with N/*=>types.Test.N#*/ { def k/*<=local6*/: Int/*=>scala.Int#*/ = ???/*=>scala.Predef.`???`().*/ }
val annType1/*<=types.Test.C#annType1.*/: T/*=>types.T#*/ @ann/*=>types.ann#*/(42) = ???/*=>scala.Predef.`???`().*/
@@ -114,5 +114,7 @@ object Test/*<=types.Test.*/ {
final val clazzOfInt/*<=types.Test.Literal.clazzOfInt.*/ = classOf/*=>scala.Predef.classOf().*/[Int]
final val clazzOfOption/*<=types.Test.Literal.clazzOfOption.*/ = classOf/*=>scala.Predef.classOf().*/[Option[Int]]
+ final val clazzOfJStr/*<=types.Test.Literal.clazzOfJStr.*/ = classOf/*=>scala.Predef.classOf().*/[java.lang.String]
+ final val clazzOfM/*<=types.Test.Literal.clazzOfM.*/ = classOf/*=>scala.Predef.classOf().*/[M]
}
}
diff --git a/tests/jvm/src/test/resources/metac.expect b/tests/jvm/src/test/resources/metac.expect
index d957bd119..fc1309faa 100644
--- a/tests/jvm/src/test/resources/metac.expect
+++ b/tests/jvm/src/test/resources/metac.expect
@@ -4053,8 +4053,8 @@ Schema => SemanticDB v4
Uri => semanticdb/integration/src/main/scala/example/Types.scala
Text => non-empty
Language => Scala
-Symbols => 140 entries
-Occurrences => 252 entries
+Symbols => 142 entries
+Occurrences => 256 entries
Symbols:
local0 => abstract method k: Int
@@ -4322,13 +4322,19 @@ types/Test.C#typeRef4. => val method typeRef4: List[Int]
types/Test.C#x. => val method x: p.X
p => types/Test.C#p.
X => types/P#X#
-types/Test.Literal. => final object Literal extends AnyRef { +12 decls }
+types/Test.Literal. => final object Literal extends AnyRef { +14 decls }
AnyRef => scala/AnyRef#
types/Test.Literal.bool. => final val method bool: true
types/Test.Literal.char. => final val method char: 'a'
types/Test.Literal.clazzOfInt. => final val method clazzOfInt: Class[Int]
Class => java/lang/Class#
Int => scala/Int#
+types/Test.Literal.clazzOfJStr. => final val method clazzOfJStr: Class[String]
+ Class => java/lang/Class#
+ String => java/lang/String#
+types/Test.Literal.clazzOfM. => final val method clazzOfM: Class[M]
+ Class => java/lang/Class#
+ M => types/Test.M#
types/Test.Literal.clazzOfOption. => final val method clazzOfOption: Class[Option[Int]]
Class => java/lang/Class#
Option => scala/Option#
@@ -4626,6 +4632,10 @@ Occurrences:
[113:27..113:34): classOf => scala/Predef.classOf().
[114:14..114:27): clazzOfOption <= types/Test.Literal.clazzOfOption.
[114:30..114:37): classOf => scala/Predef.classOf().
+[116:14..116:25): clazzOfJStr <= types/Test.Literal.clazzOfJStr.
+[116:28..116:35): classOf => scala/Predef.classOf().
+[117:14..117:22): clazzOfM <= types/Test.Literal.clazzOfM.
+[117:25..117:32): classOf => scala/Predef.classOf().
semanticdb/integration/src/main/scala/example/ValPattern.scala
--------------------------------------------------------------
diff --git a/tests/jvm/src/test/resources/metacp.expect b/tests/jvm/src/test/resources/metacp.expect
index 389f531d5..62c4c9c7a 100644
--- a/tests/jvm/src/test/resources/metacp.expect
+++ b/tests/jvm/src/test/resources/metacp.expect
@@ -3760,7 +3760,7 @@ Schema => SemanticDB v4
Uri => types/Test.class
Text => empty
Language => Scala
-Symbols => 102 entries
+Symbols => 104 entries
Symbols:
types/Test. => final object Test extends AnyRef { +4 decls }
@@ -3973,13 +3973,19 @@ types/Test.C#typeRef4. => val method typeRef4: List[Int]
types/Test.C#x. => val method x: p.X
p => types/Test.C#p.
X => types/P#X#
-types/Test.Literal. => final object Literal extends AnyRef { +12 decls }
+types/Test.Literal. => final object Literal extends AnyRef { +14 decls }
AnyRef => scala/AnyRef#
types/Test.Literal.bool. => final val method bool: true
types/Test.Literal.char. => final val method char: 'a'
types/Test.Literal.clazzOfInt. => final val method clazzOfInt: Class[Int]
Class => java/lang/Class#
Int => scala/Int#
+types/Test.Literal.clazzOfJStr. => final val method clazzOfJStr: Class[String]
+ Class => java/lang/Class#
+ String => java/lang/String#
+types/Test.Literal.clazzOfM. => final val method clazzOfM: Class[M]
+ Class => java/lang/Class#
+ M => types/Test.M#
types/Test.Literal.clazzOfOption. => final val method clazzOfOption: Class[Option[Int]]
Class => java/lang/Class#
Option => scala/Option#There was a problem hiding this comment.
This might be an unrelated issue then. Let's follow up with an issue and point to this exact place. Thanks for digging into it!
|
How do you usually run sbt:scalametaRoot> ++ 2.12.14
sbt:scalametaRoot> save-expectIs this a correct way to generate expect files? I found that if I run e.g. (save-expect with 2.12.13) --- a/tests/jvm/src/test/resources/example/InstrumentTyper.scala
+++ b/tests/jvm/src/test/resources/example/InstrumentTyper.scala
@@ -23,6 +23,6 @@ class InstrumentTyper/*<=example.InstrumentTyper#*/ { self/*<=local0*/: AnyRef =
)
def existential/*<=example.InstrumentTyper#existential().*/: U/*=>local1*/[Int/*=>scala.Int#*/]
forSome { type U/*<=local1*/[T/*<=local2*/ <: Int] } = ???/*=>scala.Predef.`???`().*/
type AnnotatedType/*<=example.InstrumentTyper#AnnotatedType#*/ = Int/*=>scala.Int#*/ @param
- def singletonType/*<=example.InstrumentTyper#singletonType().*/(x/*<=example.InstrumentTyper#sin
gletonType().(x)*/: Predef/*=>scala.Predef.*/.type) = ???/*=>scala.Predef.`???`().*/
+ def singletonType/*<=example.InstrumentTyper#singletonType().*/(x/*<=example.InstrumentTyper#sin
gletonType().(x)*/: Predef.type) = ???/*=>scala.Predef.`???`().*/
final val clazzOf/*<=example.InstrumentTyper#clazzOf.*/ = classOf/*=>scala.Predef.classOf().*/[O
ption[Int]]
}sbt:scalametaRoot> scalaVersion
[info] testsJS / scalaVersion
[info] 2.13.6
[info] parsersJS / scalaVersion
[info] 2.13.6
[info] treesJS / scalaVersion
[info] 2.13.6
[info] treesNative / scalaVersion
[info] 2.13.6
[info] testsJVM / scalaVersion
[info] 2.13.6
[info] testkitJS / scalaVersion
[info] 2.13.6
[info] commonJS / scalaVersion
[info] 2.13.6
[info] treesJVM / scalaVersion
[info] 2.13.6
[info] communitytest / scalaVersion
[info] 2.13.6
[info] semanticdbIntegration / scalaVersion
[info] 2.13.6
[info] semanticdbIntegrationMacros / scalaVersion
[info] 2.13.6
[info] semanticdbScalacCore / scalaVersion
[info] 2.13.6
[info] docs / scalaVersion
[info] 2.13.6
[info] scalametaJS / scalaVersion
[info] 2.13.6
[info] semanticdbScalacPlugin / scalaVersion
[info] 2.13.6
[info] scalametaNative / scalaVersion
[info] 2.13.6
[info] parsersJVM / scalaVersion
[info] 2.13.6
[info] testkitNative / scalaVersion
[info] 2.13.6
[info] metac / scalaVersion
[info] 2.13.6
[info] bench / scalaVersion
[info] 2.13.6
[info] testkitJVM / scalaVersion
[info] 2.13.6
[info] testsNative / scalaVersion
[info] 2.13.6
[info] parsersNative / scalaVersion
[info] 2.13.6
[info] scalametaJVM / scalaVersion
[info] 2.13.6
[info] commonNative / scalaVersion
[info] 2.13.6
[info] commonJVM / scalaVersion
[info] 2.13.6
[info] scalaVersion
[info] 2.13.6sbt:scalametaRoot> save-expect
...
[info] running scala.meta.tests.semanticdb.SaveExpectTest
[error] (run-main-0) java.lang.ClassNotFoundException: scala.meta.tests.semanticdb.SaveExpectTest
[error] java.lang.ClassNotFoundException: scala.meta.tests.semanticdb.SaveExpectTest
[error] at java.base/java.net.URLClassLoader.findClass(URLClassLoader.java:471)
[error] stack trace is suppressed; run last testsJVM / Test / bgRunMain for the full output
[error] Nonzero exit code: 1
[error] (testsJVM / Test / runMain) Nonzero exit code: 1
[error] Total time: 22 s, completed Aug 25, 2021, 12:58:38 PM |
|
So it generates different expect files for 2.12.13 and 2.12.14? That shouldn't be the case unless it's caused by the changes in this PR. Maybe we need some additional conditions to make it work with older 2.12 versions? |
Limitation: can't extract occurrence from types inside classOf if it's not a TypeRef without any type applications. because `classOf[...]` is encoded as `Literal(Constant(...))` while typing and positinal information inside of `classOf` disappear after the typechecking.
tanishiking
force-pushed
the
occurrence-classof
branch
from
f652356 to
a733c0a
Compare
|
I found that symbol occurrence for singleton types behave differently between 2.12.14 and ~2.12.13, and they're aligned in And I fixed the test by adjusting the number of occurrences in ~2.12.13 👍 |
tgodzik
approved these changes
Aug 30, 2021
| @@ -56,7 +56,7 @@ object Test/*<=types.Test.*/ { | |||
| val compoundType2/*<=types.Test.C#compoundType2.*/: M/*=>types.Test.M#*/ with N/*=>types.Test.N#*/ = ???/*=>scala.Predef.`???`().*/ | |||
| val compoundType3/*<=types.Test.C#compoundType3.*/: M/*=>types.Test.M#*/ with N/*=>types.Test.N#*/ { def k/*<=local1*/: Int/*=>scala.Int#*/ } = ???/*=>scala.Predef.`???`().*/ | |||
| val compoundType4/*<=types.Test.C#compoundType4.*/ = new /*<=local2*/{ def k/*<=local3*/: Int/*=>scala.Int#*/ = ???/*=>scala.Predef.`???`().*/ } | |||
| val compoundType5/*<=types.Test.C#compoundType5.*/ = new M/*=>types.Test.M#*/ with N/*=>types.Test.N#*/ | |||
| val compoundType5/*<=types.Test.C#compoundType5.*/ = new /*<=local4*/M/*=>types.Test.M#*/ with N/*=>types.Test.N#*/ | |||
There was a problem hiding this comment.
This might be an unrelated issue then. Let's follow up with an issue and point to this exact place. Thanks for digging into it!
|
Thank you for reviewing! Issue created #2459 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Partially fix #1909
As I mentioned in #1909 (comment) It's impossible to retrieve positional information from types inside of
classOfas long as we work on scala compiler (it might be a big change).This PR work around the issue and supports occurrences inside of
classOfiff the type is "simple" type likeclassOf[Int]which isTypeRefwith no type arguments.On the other hand, this PR doesn't support more complicated types like
classOf[Option[Int]]because we can't get positional information for bothOptionandInt, and we can't extract accurate symbol occurrences for them.I think extracting symbol occurrence with this limitation is practically quite useful (because I personally never written something like
classOf[List[Int]], usually justclassOf[Int]), and it's definitely better than nothing.What do you think about this?