lsp: module versioning can lead to wrong package
Let's say we have modules A B and C.
- A imports [email protected]
- A imports [email protected]
- C imports [email protected]
I have A/x.cue and that has import "B@v0". The lsp "definitions" analysis code (which runs goto-dfn, completions, and hover) would behave as follows:
- When jumping from A to C, I would wind up in [email protected].
- When then jumping from C to B, I'd go to [email protected].
- But when jumping from A directly to B, I would go to [email protected].
This could be a real problem when trying to debug an issue. Consider this:
-- cue.mod/module.cue --
module: "A@v0"
language: version: "v0.12.0"
deps: {
"B@v0": v: "v0.0.5"
"C@v0": v: "v0.0.4"
}
-- main.cue --
package A
import "C"
x: C.#X & {
a: 4
}
-- _registry/[email protected]/cue.mod/module.cue --
module: "C@v0"
language: version: "v0.12.0"
deps: {
"B@v0": v: "v0.0.3"
}
-- _registry/[email protected]/c.cue --
package C
import "B"
#X: B.#X & {
z: 1
}
-- _registry/[email protected]/cue.mod/module.cue --
module: "B@v0"
language: version: "v0.12.0"
-- _registry/[email protected]/cue.mod/module.cue --
module: "B@v0"
language: version: "v0.12.0"
#X: {
a?: int
z!: int
}
-- _registry/[email protected]/cue.mod/module.cue --
module: "B@v0"
language: version: "v0.12.0"
-- _registry/[email protected]/cue.mod/module.cue --
module: "B@v0"
language: version: "v0.12.0"
#X: {
a?: int
z!: >2
}
- start at main.cue
- jumpdef from
C.#Xgoes to[email protected].#X - jumpdef from
B.#Xgoes to[email protected].#X - user sees
z!: intnotz!: >2, the thing that's causing the conflict
LSP is kinda stateless (it's really not due to the communication of text edits, but it is stateless from the pov of navigation). We could consider having some mechanism (possibly a code action or something) that allows the user to control which module.cue/cue.mod is the "root" and thus used to control version selection. We would need some sensible default behaviour, and we would need to warn the user when we detect ambiguity.
goto-dfn of course does support returning several different locations, but we are already using that to indicate "all of these locations are unified together". These could already come from several different packages and modules so keeping the semantics there simple - "it's ALL of these" - is important. Adding to this alternatives - "it's either this version or that version" - seems like a bad idea to me. Also the result of goto-dfn is a plain list of Location which doesn't have a label - it's just uri and range only, so we can't control what the editor shows the user.
Finally, as mentioned, this issue affects more than just goto-dfn, so I prefer a solution that does not rely on any unique aspect of the the result of goto-dfn, or completions, or hover docs (or anything else we implement in the future); this is why I think explicit user management of this piece of state is likely to be the right way to go.
Update: I believe that issue #4144 is a more accurate reflection of how the lsp currently behaves and although there are valid points in this issue, its description of how LSP behaves is not actually correct.