Completion replaces the word cursor is at instead of inserting

[suguruwataru]

rust-analyzer version: commit 11d6212

rustc version: rustc 1.92.0

editor or extension: I use NeoVim with vim-lsp but this should affect everything

relevant settings: None

repository link (if public, optional): None

code snippet to reproduce:

Code snippet is based on this test

rust-analyzer/crates/ide-completion/src/tests/special.rs

Line 15 in 11d6212

fn completes_if_prefix_is_keyword() {

You can also reproduce using this test.

fn main() {
    let wherewolf = 92;
    drop(where$0sheep) // Note that compared to original test, `sheep` is added behind the cursor
}

After the text edit from the completion, it's expected that the code becomes

fn main() {
    let wherewolf = 92;
    drop(wherewolfsheep)
}

However, the actual behavior is that, the sheep gets eaten by the wolf:

fn main() {
    let wherewolf = 92;
    drop(wherewolf)
}

I thought this current behavior was the expected behavior, because it's the default behavior of Intellij IDEA (and has been hated by people for years), but looking at this comment makes me think this might be unintentional:

text_edit should start with what source_range points to

as in this case, source_range points to wheresheep, but text_edit is wherewolf.

I think the problem here is that, here

rust-analyzer/crates/ide-completion/src/render.rs

Lines 508 to 510 in 11d6212

	let mut item = CompletionItem::new(
	kind,
	ctx.source_range(),

the text to delete in this completion is decided to be ctx.source_range(), which is determined here

rust-analyzer/crates/ide-completion/src/context.rs

Line 514 in 11d6212

_ if kind.is_any_identifier() => self.original_token.text_range(),

to be the original_token's text_range(), and the original_token has been decided here

rust-analyzer/crates/ide-completion/src/context.rs

Line 724 in 11d6212

let original_token = original_file.syntax().token_at_offset(offset).left_biased()?;

to be wheresheep instead of where.

There's also a discussion about this: #21029, where an example that better illustrates why this behavior is annoying in practice is given.

[lnicola]

Interestingly, in VS Code, Helix and Zed I get wherewolfsheep.

[A4-Tacks]

AFAIK, this can be changed to insertion instead of replace, such as suggest.insertMode: insert in coc.nvim

This may be related to InsertReplaceEdit, which provides both insertion and replacement

[suguruwataru]

Interestingly, in VS Code, Helix and Zed I get wherewolfsheep.

Might be why this flew under the radar for so many years...

This may be related to InsertReplaceEdit

So I teed the output of rust-analyzer, and the completion item given looks like this:

      {
        "label": "wherewolf",
        "kind": 6,
        "detail": "i32",
        "sortText": "7ffffff9",
        "filterText": "wherewolf",
        "textEdit": {
          "range": {
            "start": {
              "line": 2,
              "character": 9
            },
            "end": {
              "line": 2,
              "character": 20
            }
          },
          "newText": "wherewolf"
        }
      },

This text edit is of the LSP interface TextEdit, and the range indeed indicates that rust-analyzer wants to delete the whole of wheresheep.

So my guess is (just a guess, didn't look at the code), per LSP, the server returns InsertReplaceEdit or TextEdit based on insertReplaceSupport capability reported by the client. So perhaps rust-analyzer returns correct InsertReplaceEdit if the client reports insertReplaceSupport = true, but returns this buggy TextEdit when this capability isn't reported?

[suguruwataru]

There we go

Here we can see that, with insertReplaceSupport, the range to delete on the "insert" case is set to end at cursor pos

rust-analyzer/crates/rust-analyzer/src/lsp/to_proto.rs

Lines 182 to 197 in 5d5bb40

	pub(crate) fn completion_text_edit(
	line_index: &LineIndex,
	insert_replace_support: Option<lsp_types::Position>,
	indel: Indel,
	) -> lsp_types::CompletionTextEdit {
	let text_edit = text_edit(line_index, indel);
	match insert_replace_support {
	Some(cursor_pos) => lsp_types::InsertReplaceEdit {
	new_text: text_edit.new_text,
	insert: lsp_types::Range { start: text_edit.range.start, end: cursor_pos },
	replace: text_edit.range,
	}
	.into(),
	None => text_edit.into(),
	}
	}

and without it, the range to delete ends at source_range's end.

rust-analyzer/crates/rust-analyzer/src/lsp/to_proto.rs

Lines 319 to 330 in 5d5bb40

	if indel.delete.contains_range(source_range) {
	// Extract this indel as the main edit
	text_edit = Some(if indel.delete == source_range {
	self::completion_text_edit(line_index, insert_replace_support, indel.clone())
	} else {
	assert!(source_range.end() == indel.delete.end());
	let range1 = TextRange::new(indel.delete.start(), source_range.start());
	let range2 = source_range;
	let indel1 = Indel::delete(range1);
	let indel2 = Indel::replace(range2, indel.insert.clone());
	additional_text_edits.push(self::text_edit(line_index, indel1));
	self::completion_text_edit(line_index, insert_replace_support, indel2)

[ZXY595]

Might be why this flew under the radar for so many years...

This may be related to InsertReplaceEdit

So my guess is (just a guess, didn't look at the code), per LSP, the server returns InsertReplaceEdit or TextEdit based on insertReplaceSupport capability reported by the client. So perhaps rust-analyzer returns correct InsertReplaceEdit if the client reports insertReplaceSupport = true, but returns this buggy TextEdit when this capability isn't reported?

I think I've found the reason. In NVIM v0.11.5, vim.lsp.protocol.make_client_capabilities() actually returns capabilities like this:
https://github.com/neovim/neovim/blob/9fa7c0a04638ee3d5fefbdcec347b1c07a5ce9ce/runtime/lua/vim/lsp/protocol.lua#L456-L473

As you can see, there is no insertReplaceSupport in completionItem.
Instead, it was added after fix(lsp): enable insertReplaceSupport for use in adjust_start_col .

I think the solution is quite simple: either add insertReplaceSupport = true after make_client_capabilities() in latest release version of NVIM (I tried it and it just works!), or install the latest unstable version of NVIM.

[suguruwataru]

I'd argue that this is still something that should be fixed in r-a. If the client doesn't have insertReplaceSupport, r-a should use a saner default, which is insert instead of replace.

Of course, since both insert and replace are considered valid completion actions in the LSP world, guess it's also valid if r-a maintainer folks decide that the current behavior is to be kept and if one wants the insert behavior git a gud client.