improvement: use pc for go to def when stale semanticdb #7028
Conversation
…emanticdb), add report when missing definition)
| ): Future[DefinitionResult] = | ||
| for { | ||
| fromCompiler <- | ||
| if (path.isScalaFilename && !isAmmonnite(path)) |
There was a problem hiding this comment.
Could we just keep the current order for Ammonite? With this change we would not use compiler for Ammonite, right?
I wonder if we could just adjust tests to take that into account?
There was a problem hiding this comment.
I can also fix it and map generated Ammonite files to source files like we do for Scala CLI, if we want to still have Ammonite support.
Reordering is only half of a fix since for local symbols using pc will lead to generated sources. But I can also do that, if don't know where we are with Ammonite support.
There was a problem hiding this comment.
I raised the topic on discord again and will start pushing it. In the meantime I don't think we want to work on any fixes, which we will be removing in the future.
So if it's too much work let's just leave it as is then.
| private var fallbackDefn: Option[DefinitionResult] = None | ||
| private var nonLocalGuesses: List[String] = List.empty | ||
|
|
||
| private var fundScaladocDef = false |
There was a problem hiding this comment.
| private var fundScaladocDef = false | |
| private var foundScaladocDef = false |
| @@ -233,7 +233,7 @@ class DefinitionLspSuite | |||
| |package a | |||
| |object B/*L1*/ { | |||
| | def main/*L2*/() = { | |||
| | println/*Predef.scala*/(A/*;A.scala:2;A.scala:3*/("John")) | |||
There was a problem hiding this comment.
I think the idea here was to show both object and case class. Any idea why this changed?
There was a problem hiding this comment.
I think that in SemanticDB the symbol found here is a synthetic apply, so we get both class and object, where in pc it will simply find the symbol of either class or object. I can create a followup to see how it behaves to make sure there is some reasonable consistency.
| error = Some(e) | ||
| } | ||
|
|
||
| def withNonLocalGuesses(guesses: List[String]): Unit = { |
There was a problem hiding this comment.
| def withNonLocalGuesses(guesses: List[String]): Unit = { | |
| def setNonLocalGuesses(guesses: List[String]): Unit = { |
This is more a setter than a builder method, no? Could we make them more consistent or rename as above?
resolves: #7027