Autocomplete suggestion changes to something random once selected

[tgross35]

rust-analyzer version: rust-analyzer 1.84.0-nightly (b19329a3 2024-11-21)

rustc version: rustc 1.84.0-nightly (b19329a37 2024-11-21)

editor or extension: helix 24.7 (079f5442)

relevant settings: (eg. client settings, or environment variables like CARGO, RUSTC, RUSTUP_HOME or CARGO_HOME)

repository link (if public, optional): reproduced in multiple repositories

code snippet to reproduce: working on minimal repro

Since about the past week I have been dealing with an issue where autocomplete suggestions change when I highlight them. This video explains it better:

The video shows the following:

• I type let mut px = PCt
• Arrow down to the suggested and expected PCtx
• Once the suggestion is highlighted in the dropdown, it replaces my PCt with PCtx which is expected. However, it deletes the space between = and PCt - that is weird.
• I hit escape to cancel, which deletes PCt*
• I start typing PCt again. RA again suggests PCtx
• I arrow down and highlight PCtx in the dropdown. RA replaces it correctly this time
• However, after about a half second, the suggested PCtx in the dropdown has changed to Either. This is completely random and makes no sense in the context (though what it suggests does seem to be consistent)
• I hit escape to cancel. For some reason, it replaces PCtx with Either

My only RA config is pretty minimal:

[language-server.rust-analyzer.config]
diagnostics.disabled = [ "inactive-code", "unlinked-file" ]

I am assuming this came from an RA update since it started somewhat recently (past week or two) and Helix hasn't had an update in a few months, but I haven't yet had a chance to see if it still happens with older versions. This problem happens pretty consistently across multiple repos.

Edit: this happens with other kinds of autocomplete suggestions too, it is doing things like changing some_func to panic!().

[tgross35]

Doing some light bisection. I have not been able to reproduce with these versions:

• rust-analyzer 1.82.0 (f6e511ee 2024-10-15)
• rust-analyzer 1.84.0-nightly (1e4f10ba 2024-10-29)
• rust-analyzer 1.84.0-nightly (b91a3a05 2024-11-07)

With these, I have been able to cause the behavior pretty easily:

• rust-analyzer 1.84.0-nightly (90ab8eae 2024-11-14)
• rust-analyzer 1.83.0-beta.7 (b35ad722a4b 2024-11-22)
• rust-analyzer 1.84.0-nightly (b19329a3 2024-11-21)

[schrottkatze]

experiencing this too, on helix as well (but helix master, thought that might be causing it but apparently not), on the last mentioned rust-analyzer version

[TheBearodactyl]

same exact problem here

[boraarslan]

Same problem on nvim after updating my rust-analyzer. Solution on #18536 did not work for me.

neovim version:

NVIM v0.10.2
Build type: RelWithDebInfo
LuaJIT 2.1.1731601260

rust-analyzer version: rust-analyzer 1 (327ab2958f 2024-11-24)

[Veykril]

@SomeoneToIgnore is this issue what was caused by your changes last month? (and hence was this fixed by #18503?). Pinging mainly because I have not been following up on the completions issues that occured while I was away :)

[SomeoneToIgnore]

Yes, this got exposed due to my changes, alas.
#18503 fixed things for all editors like Emacs (lsp-mode), Kate, Zed, etc.
Things were not really fixed in this PR for Helix and NeoVim because

• Helix does not support partial completions and does not re-query completions on the document changes, causing r-a to get a different completions list for resolving, messing up all indices and returning nonsense
  More context: Completion items change when cycling helix-editor/helix#12119 (comment)

• NeoVim looks anti-spec and odd, it does edit the text document on completion selection (before applying anything) and is declaring a very peculiar set of completion item properties it can resolve, which they do care to fix even after the PR is there: Make LSP completions resolve capabilities more spec-compliant hrsh7th/cmp-nvim-lsp#75

While being caused by different approaches, both cases boil down to a microsoft/language-server-protocol#2060 question but that is not answered still.

(Also, to me, it's not very clear how the additionalTextEdits were correctly resolved for them before (by sheer luck it seems)).

More general context can be found from
https://rust-lang.zulipchat.com/#narrow/channel/185405-t-compiler.2Frust-analyzer/topic/Completion.20IDs/near/484684205
message and below.

[Veykril]

Do both of these editors announce what they are in the handshake? If yes we could temporarily disable resolve support for them within the server.

[SomeoneToIgnore]

Helix does: https://github.com/helix-editor/helix/blob/fc9968bd4bbc5adbcc35bb2fa40515dbb96a3a36/helix-lsp/src/client.rs#L682
NVim seems to follow its anti-spec motto and does not, but I cannot be sure, alas.

I love the approach too, had proposed it in https://rust-lang.zulipchat.com/#narrow/channel/185405-t-compiler.2Frust-analyzer/topic/Completion.20IDs/near/484683744

[pascalkuthe]

While adding incomplete completion requests to helix (in part as a response to this breakage) I investigated this closer and don't think what you are doing here is spec compliant:

• requiring completions to be requested at the same edit does not make any sense for servers that don't use incomplete completion requests so it can't be a general rule
• it's essentially a race condition whether the client resolves a completion before or after an incomplete completion response

the second point is the most relevant. When a client needs to apply a completion it will send a resolve request to the server (especially if the edit itself needs to be resolved). However, clients do not and cannot wait for incomplete completion list requests to complete at this point:

• it's not specified in the spec it's required (and no client including vscode waits)
• it's not practical to do since we would need to find the completion item in the new completion list that corresponds to the one the user accepted (otherwise you risk randomly applying a different item or would randomly error out when accepting a completion). However, it's not practical for the client to fiend the item in the new completion list that corresponds to the selected item (it' hard for the server to do this which triggered this whole thing but it's entirely impossible for the client to do).

To demonstrate that this is also how VSCode behaves I have pushed a small demonstrator: d2f69ee

• I modified the completion request handler to never return a result for incomplete completion requests (simulating an arbitrary delay either in the request handler or in the client who is asynchronously processing the response)
• I modified the completion request and resolver request to shuffle completion order based on version so it's easier to trigger (but that ofcourse happens in practice)

with this change I can observe this issue in VSCode (if you look at the log completions clearly get resolved to the wrong thing), demonstrating that this only works in vscode because the response to incomplete completion responses happens to be handled fast enough (aka a race condition).

This makes sense to me because from the clients perspective, I expect the server to really have some stable definition of completion items that does not change between edits (in simple/most cases the label) that allows me to perform a lookup/completion resolve.

Instead, You could use a combination of label plus some extra data (for disambiguation) to look up more similar to the old code. You are already shipping the imports along in the item data, that may be sufficient. Otherwise, you could hash some properties of the item with a reasonably collision-resistant hash like sha1 or tenthash

[ChayimFriedman2]

Frankly if this is that complex I'd prefer to just revert the change.

[TheBearodactyl]

what's the pr that merged the changes to the completion so that i can just clone and manually remove the specific changes

[workingjubilee]

Hello everyone!

There's been a fair amount of useful discussion but I think it might be best to avoid making statements that might be confused with factual claims about what the spec is or is not, when the ambiguity in the spec might be resolved in a different direction: microsoft/language-server-protocol#2060

This is less about what anyone has said so far here and more about noticing that this has been a tempting distraction in other discussions about this.

[SomeoneToIgnore]

To note, rust tree has all the changes reverted already, and rust-analyzer falls back to the old way of things for Helix: #18630

So, while things are being clarified for the spec part, nothing should be broken for the client anymore.

[pascalkuthe]

You are unlikely to grt an answer from the spec anytime soon given past precedence.

What I saying is that:

• it's impractical/impossible for clients to reliably work the way you expect them to
• things are broken, you are relying on a race condition jn vscode. You are just usually fast enough. Also you disabled completion resolve for other clients which does slow down other clients

[Veykril]

Also you disabled completion resolve for other clients which does slow down other clients

Fwiw, r-a does all the eager work whether you enable resolving or not currently, so the only thing that's worse off for clients that don't resolve is that the completion result is bigger in terms of json size. (for now)

Also just to clarify I hope you don't take us force disabling completion resolve for helix in a bad intent, it was just the fasted way to get things working for people again for now until we sort this out!

[pascalkuthe]

Fwiw, r-a does all the eager work whether you enable resolving or not currently, so the only thing that's worse off for clients that don't resolve is that the completion result is bigger in terms of json size. (for now)

yeah I understand but with incomplete completion requests that is still a bunch of back and fourth so would still be nice to avoid.

Also just to clarify I hope you don't take us force disabling completion resolve for helix in a bad intent, it was just the fasted way to get things working for people again for now until we sort this out!

No I actually think that was a good initial response, it stops the flood of bug reports for us and keeps things working for people. So no worries about that! I just wouldn't want things to stay this way permanently (particularly since it's also an issue for vscode).

[SomeoneToIgnore]

@pascalkuthe , I have created a hash proposal impl (thank you for the idea), if you want to double check I'm not doing anything ridiculous there: #18653