Edits are applied twice

[Veykril]

This popped up in Rust-Analyzer and we are unsure whether its a bug in the client or us implementing it wrong rust-lang/rust-analyzer#7916

Basically when the user renames a module in the source via the rename action rust-analyzer replies with a WorkspaceEdit that renames all locations as well as file names that are affected. This in turn seems to cause vscode to trigger one or more WillRenameFiles requests for which we reply with the same WorkspaceEdit as this is basically the same rename request in this case. This causes two renames to be applied per reference causing incorrect text edits.

We assume that VSCode should not react with a WillRenameFiles request when the file renames are caused by the server, is this assumption wrong? If yes what should be done by the server to prevent this as the documentation doesn't make this clear to us https://microsoft.github.io/language-server-protocol/specifications/specification-3-17/#workspace_willRenameFiles

[dbaeumer]

This is actually expected. Otherwise another extension / server can never participate in a file rename. This is also the reason why these events are in the workspace realm (they trigger if something to the workspace happens). I will clarify that in the spec.

There is currently no way in the spec to prevent them. What we could think about is to add an indication to the event who triggered it.

In the VS Code client libs you could do of course filter them in the middleware.

[dbaeumer]

@Veykril can you give me an example were this behavior causes problems.

[lnicola]

@dbaeumer see rust-lang/rust-analyzer#7916 (comment).

[dbaeumer]

Can you please explain the flow to me since I am not familiar with what happens in Rust on a mod rename.

[Veykril]

Assuming we have two files here:
lib.rs

mod foobar;

foobar.rs

A rename on the foobar with the new name foo on the mod foobar; line will make rust-analyzer reply with an edit that changes the reference foobar to foo as well as a file system edit to rename the foobar.rs file to foo.rs. This causes vscode to trigger a WillRenameFiles request of foobar.rs which will cause rust-analyzer to generate the same rename reply for this as its basically the same rename action. Then VSCode applies both of these workspace edits causing mod foobar; to turn into mod foo(note the missing semicolon) as the renames are applied twice. The double rename causes problems like these as the second rename being applied now has invalid ranges for the rename.
Thats whats roughly happening.

So the result we get from this is

mod foo

foo.rs

which is incorrect as we lost a semicolon we shouldnt lose

Similarly if we rename foobar to something thats longer like foobar2 we get mod foobar22; instead of mod foobar2;.

[Veykril]

I can see how this is intentional so that other extensions can observe this, but the question is if it makes sense to send the WillRenameRequest to the server that caused the rename.

Maybe a simple solution on the rust-analyzer side would be to only send the file edits in a rename response if the client supports WillRenameFiles and do the actual rename part in the response of that instead?

[dbaeumer]

@Veykril OK. Understood.

Filtering the events without knowing exactly who has caused them by which operation is very difficult in an async environment. It could lead to drop an event although the server was not the source.

So the only fix for this would be that we can tag an workspace edit with an ID which is the repeated in an event to ensure correct interpretation. Requires work in VS Code side and the libraries.

So I like your simple solution

Maybe a simple solution on the rust-analyzer side would be to only send the file edits in a rename response if the client supports WillRenameFiles and do the actual rename part in the response of that instead?

[matklad]

Hm, while we are working-around this on our side, there's still behavior that surprises me -- vscode applies two conficing edits. Here's a log when I rename a to bb:

[Trace - 5:58:36 PM] Sending request 'textDocument/prepareRename - (14)'.
Params: {
    "textDocument": {
        "uri": "file:///home/matklad/tmp/hello/src/main.rs"
    },
    "position": { "line": 0, "character": 5 }
}


[Trace - 5:58:36 PM] Received response 'textDocument/prepareRename - (14)' in 1ms.
Result: {
    "start": { "line": 0, "character": 4 },
    "end": { "line": 0, "character": 5 }
}


[Trace - 5:58:39 PM] Sending request 'textDocument/rename - (15)'.
Params: {
    "textDocument": {
        "uri": "file:///home/matklad/tmp/hello/src/main.rs"
    },
    "position": { "line": 0, "character": 5 },
    "newName": "bb"
}


[Trace - 5:58:39 PM] Received response 'textDocument/rename - (15)' in 2ms.
Result: {
    "documentChanges": [
        {
            "kind": "rename",
            "oldUri": "file:///home/matklad/tmp/hello/src/a.rs",
            "newUri": "file:///home/matklad/tmp/hello/src/bb.rs"
        },
        {
            "textDocument": {
                "uri": "file:///home/matklad/tmp/hello/src/main.rs",
                "version": 1
            },
            "edits": [
                {
                    "range": {
                        "start": { "line": 0, "character": 4 },
                        "end": { "line": 0, "character": 5 }
                    },
                    "newText": "bb"
                }
            ]
        }
    ]
}


[Trace - 5:58:39 PM] Sending request 'workspace/willRenameFiles - (16)'.
Params: {
    "files": [
        {
            "oldUri": "file:///home/matklad/tmp/hello/src/a.rs",
            "newUri": "file:///home/matklad/tmp/hello/src/bb.rs"
        }
    ]
}


[Trace - 5:58:39 PM] Received response 'workspace/willRenameFiles - (16)' in 1ms.
Result: {
    "documentChanges": [
        {
            "textDocument": {
                "uri": "file:///home/matklad/tmp/hello/src/main.rs",
                "version": 1
            },
            "edits": [
                {
                    "range": {
                        "start": { "line": 0, "character": 4 },
                        "end": { "line": 0, "character": 5 }
                    },
                    "newText": "bb"
                }
            ]
        }
    ]
}


[Trace - 5:58:39 PM] Sending notification 'textDocument/didChange'.
Params: {
    "textDocument": {
        "uri": "file:///home/matklad/tmp/hello/src/main.rs",
        "version": 2
    },
    "contentChanges": [
        {
            "range": {
                "start": { "line": 0, "character": 4 },
                "end": { "line": 0, "character": 5 }
            },
            "rangeLength": 1,
            "text": "bb"
        }
    ]
}


[Trace - 5:58:39 PM] Sending notification 'textDocument/didChange'.
Params: {
    "textDocument": {
        "uri": "file:///home/matklad/tmp/hello/src/main.rs",
        "version": 3
    },
    "contentChanges": [
        {
            "range": {
                "start": { "line": 0, "character": 5 },
                "end": { "line": 0, "character": 5 }
            },
            "rangeLength": 0,
            "text": "b"
        }
    ]
}

Note that the server sends two identical edits for document version 1. I would expect the client to either error out because the edits are conflicting, or to notice that they are identical and apply only one (that is, to figure out that they OT-merge to a single edit).

Instead, the client sends two didChange events:

[Trace - 5:58:39 PM] Sending notification 'textDocument/didChange'.
Params: {
    "textDocument": {
        "uri": "file:///home/matklad/tmp/hello/src/main.rs",
        "version": 2
    },
    "contentChanges": [
        {
            "range": {
                "start": { "line": 0, "character": 4 },
                "end": { "line": 0, "character": 5 }
            },
            "rangeLength": 1,
            "text": "bb"
        }
    ]
}


[Trace - 5:58:39 PM] Sending notification 'textDocument/didChange'.
Params: {
    "textDocument": {
        "uri": "file:///home/matklad/tmp/hello/src/main.rs",
        "version": 3
    },
    "contentChanges": [
        {
            "range": {
                "start": { "line": 0, "character": 5 },
                "end": { "line": 0, "character": 5 }
            },
            "rangeLength": 0,
            "text": "b"
        }
    ]
}

So, it applies edit with version 1 to a document with version 2.

[dbaeumer]

@matklad thanks for reporting this!