Duplicate TextEdit in textDocument/rename response

[bstaletic]

Hello,

while refactoring ycmd's LSP client code, we wrote a test for clangd that sendstextDocument/rename request on a simple file. Instead of 4 unique TextEdits, clangd returned 5 with 3rd one duplicated

The exchanged messages, from the ycmd's side:

2019-01-20 13:06:44,206 - DEBUG - TX: Sending message: b'Content-Length: 528\r\n\r\n{"id": "1", "jsonrpc": "2.0", "method": "initialize", "params": {"capabilities": {"textDocument": {"completion": {"completionItemKind": {"valueSet": [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26]}}}}, "initializationOptions": {}, "processId": 7882, "rootPath": "/home/bstaletic/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/tests/clangd/testdata", "rootUri": "file:///home/bstaletic/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/tests/clangd/testdata"}}'
2019-01-20 13:06:44,207 - DEBUG - RX: Received message: b'{"id":"1","jsonrpc":"2.0","result":{"capabilities":{"codeActionProvider":true,"completionProvider":{"resolveProvider":false,"triggerCharacters":[".",">",":"]},"definitionProvider":true,"documentFormattingProvider":true,"documentHighlightProvider":true,"documentOnTypeFormattingProvider":{"firstTriggerCharacter":"}","moreTriggerCharacter":[]},"documentRangeFormattingProvider":true,"documentSymbolProvider":true,"executeCommandProvider":{"commands":["clangd.applyFix"]},"hoverProvider":true,"renameProvider":true,"signatureHelpProvider":{"triggerCharacters":["(",","]},"textDocumentSync":2,"workspaceSymbolProvider":true}}}'
2019-01-20 13:06:44,207 - DEBUG - TX: Sending notification: b'Content-Length: 57\r\n\r\n{"jsonrpc": "2.0", "method": "initialized", "params": {}}'
2019-01-20 13:06:44,207 - DEBUG - TX: Sending notification: b'Content-Length: 92\r\n\r\n{"jsonrpc": "2.0", "method": "workspace/didChangeConfiguration", "params": {"settings": {}}}'
2019-01-20 13:06:44,207 - DEBUG - TX: Sending notification: b'Content-Length: 546\r\n\r\n{"jsonrpc": "2.0", "method": "textDocument/didOpen", "params": {"textDocument": {"languageId": "cpp", "text": "struct Foo {\\n  int x;\\n  int y;\\n  char c;\\n};\\n\\nint main()\\n{\\n  Foo foo;\\n  // The location after the dot is line 11, col 7\\n  foo.\\n}\\n\\n\\nstatic Foo test_function_that_has_no_errors()\\n{\\n  Foo foo = { 1,2,\'c\'};\\n  if (foo.c ) {\\n    foo.x = 1;\\n    foo.y = 2;\\n  }\\n\\n  return foo;\\n}\\n", "uri": "file:///home/bstaletic/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/tests/clangd/testdata/basic.cpp", "version": 1}}}'
2019-01-20 13:06:44,213 - DEBUG - RX: Received message: b'{"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"diagnostics":[{"message":"expected unqualified-id","range":{"end":{"character":1,"line":11},"start":{"character":0,"line":11}},"severity":1}],"uri":"file:///home/bstaletic/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/tests/clangd/testdata/basic.cpp"}}'
2019-01-20 13:07:15,050 - DEBUG - TX: Sending message: b'Content-Length: 276\r\n\r\n{"id": "2", "jsonrpc": "2.0", "method": "textDocument/rename", "params": {"newName": "Bar", "position": {"character": 3, "line": 16}, "textDocument": {"uri": "file:///home/bstaletic/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/tests/clangd/testdata/basic.cpp"}}}'
2019-01-20 13:07:15,051 - DEBUG - RX: Received message: b'{"id":"2","jsonrpc":"2.0","result":{"changes":{"file:///home/bstaletic/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/tests/clangd/testdata/basic.cpp":[{"newText":"Bar","range":{"end":{"character":10,"line":0},"start":{"character":7,"line":0}}},{"newText":"Bar","range":{"end":{"character":5,"line":8},"start":{"character":2,"line":8}}},{"newText":"Bar","range":{"end":{"character":10,"line":14},"start":{"character":7,"line":14}}},{"newText":"Bar","range":{"end":{"character":10,"line":14},"start":{"character":7,"line":14}}},{"newText":"Bar","range":{"end":{"character":5,"line":16},"start":{"character":2,"line":16}}}]}}}'
2019-01-20 13:07:15,060 - DEBUG - TX: Sending notification: b'Content-Length: 549\r\n\r\n{"jsonrpc": "2.0", "method": "textDocument/didChange", "params": {"contentChanges": [{"text": "struct Bar {\\n  int x;\\n  int y;\\n  char c;\\n};\\n\\nint main()\\n{\\n  Bar foo;\\n  // The location after the dot is line 11, col 7\\n  foo.\\n}\\n\\n\\nstatic Bar test_function_that_has_no_errors()\\n{\\n  Bar foo = { 1,2,\'c\'};\\n  if (foo.c ) {\\n    foo.x = 1;\\n    foo.y = 2;\\n  }\\n\\n  return foo;\\n}\\n"}], "textDocument": {"uri": "file:///home/bstaletic/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/tests/clangd/testdata/basic.cpp", "version": 2}}}'
2019-01-20 13:07:15,565 - DEBUG - RX: Received message: b'{"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"diagnostics":[{"message":"expected unqualified-id","range":{"end":{"character":1,"line":11},"start":{"character":0,"line":11}},"severity":1}],"uri":"file:///home/bstaletic/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/tests/clangd/testdata/basic.cpp"}}'

Clangd stderr:

I[13:06:44.206] <-- initialize("1")
I[13:06:44.206] --> reply("1")
I[13:06:44.207] <-- initialized
E[13:06:44.207] Error -32601: method not found
I[13:06:44.207] <-- workspace/didChangeConfiguration
I[13:06:44.207] <-- textDocument/didOpen
I[13:06:44.209] Updating file /home/bstaletic/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/tests/clangd/testdata/basic.cpp with command [/home/bstaletic/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/tests/clangd/testdata] clang-tool -xc++ -std=c++11 -iquotequote/ -Isystem/ /home/bstaletic/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/tests/clangd/testdata/basic.cpp -resource-dir=/home/bstaletic/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/clang_includes
E[13:06:44.211] Could not build a preamble for file /home/bstaletic/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/tests/clangd/testdata/basic.cpp
I[13:07:15.050] <-- textDocument/rename("2")
I[13:07:15.051] --> reply("2")
I[13:07:15.061] <-- textDocument/didChange
I[13:07:15.561] Updating file /home/bstaletic/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/tests/clangd/testdata/basic.cpp with command [/home/bstaletic/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/tests/clangd/testdata] clang-tool -xc++ -std=c++11 -iquotequote/ -Isystem/ /home/bstaletic/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/tests/clangd/testdata/basic.cpp -resource-dir=/home/bstaletic/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/clang_includes
E[13:07:15.563] Could not build a preamble for file /home/bstaletic/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/tests/clangd/testdata/basic.cpp

The nicely formatted textDocument/rename request:

'{
	"id": "2", 
	"jsonrpc": "2.0", 
	"method": "textDocument/rename", 
	"params": {
		"newName": "Bar", 
		"position": {
			"character": 3, 
			"line": 16
		}, 
		"textDocument": {
			"uri": "file:///home/bstaletic/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/tests/clangd/testdata/basic.cpp"
		}
	}
}'

The nicely formatted textDocument/rename response:

'{
	"id": "2",
	"jsonrpc": "2.0",
	"result": {
		"changes": {
			"file: ///home/bstaletic/.vim/pack/bundle/start/YouCompleteMe/third_party/ycmd/ycmd/tests/clangd/testdata/basic.cpp": [
				{
					"newText": "Bar",
					"range": {"end": {"character": 10,"line": 0},"start": {"character": 7,"line": 0}}
				},
				{
					"newText": "Bar",
					"range": {"end": {"character": 5,"line": 8},"start": {"character": 2,"line": 8}}
				},
				{
					"newText": "Bar",
					"range": {"end": {"character": 10,"line": 14},"start": {"character": 7,"line": 14}}
				},
				{
					"newText": "Bar",
					"range": {"end": {"character": 10,"line": 14},"start": {"character": 7,"line": 14}}
				},
				{
					"newText": "Bar",
					"range": {"end": {"character": 5,"line": 16},"start": {"character": 2,"line": 16}}
				}
			]
		}
	}
}'

[kadircet]

It can be reproduced with the following test case:

TEST_F(ClangdVFSTest, TestRandom) {
  MockFSProvider FS;
  ErrorCheckingDiagConsumer DiagConsumer;
  MockCompilationDatabase CDB;

  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());

  auto FooCpp = testPath("foo.cpp");
  Annotations Code(R"cpp(
    struct Foo{};
    Foo test() {
     F^oo x;
     return x;
    }
    )cpp");

  runAddDocument(Server, FooCpp, Code.code());

  auto X = runRename(Server, FooCpp, Code.point(), "new_name");
  ASSERT_TRUE(!!X);
  size_t PrevOffset = -1;
  for (const auto &T : X.get()) {
    EXPECT_NE(PrevOffset, T.getOffset());
    PrevOffset = T.getOffset();
  }
}

Unfortunately issue seems to be rather related to clang api we use, https://github.com/llvm/llvm-project/blob/master/clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp#L84

Because visitor itself returns duplicate entries for the return type of function test()

[kadircet]

Another case this happens:

void foo() {
    if(auto add_xxx = 5) {
        add_xxx = 3;
    }
}

try renaming add_xxx to added_xxx(either of them results in same breakage). Which simply fails on vscode by saying "overlapping edits" and results in malformed text in vim like added_xxxxx.

[bstaletic]

Ycmd can cope with this as long as the old and new name have the same length, otherwise a duplicated edit does make a mess. Requesting a RefactorRename foo_bar would correctly replace the text.

[sam-mccall]

Fixed in r360116.