diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -589,7 +589,6 @@ }; bool fromJSON(const llvm::json::Value &, DocumentSymbolParams &); - /// Represents a related message and source code location for a diagnostic. /// This should be used to point to code locations that cause or related to a /// diagnostics, e.g when duplicating a symbol in a scope. @@ -639,11 +638,17 @@ /// A LSP-specific comparator used to find diagnostic in a container like /// std:map. -/// We only use the required fields of Diagnostic to do the comparsion to avoid -/// any regression issues from LSP clients (e.g. VScode), see -/// https://git.io/vbr29 +/// We only use as many fields of Diagnostic as is needed to make distinct +/// diagnostics unique in practice, to avoid regression issues from LSP clients +/// (e.g. VScode), see https://git.io/vbr29 struct LSPDiagnosticCompare { bool operator()(const Diagnostic &LHS, const Diagnostic &RHS) const { + if (!LHS.code.empty() && !RHS.code.empty()) { + // If the code is known for both, use the code to diambiguate, as e.g. + // two checkers could produce diagnostics with the same range and message. + return std::tie(LHS.range, LHS.message, LHS.code) < + std::tie(RHS.range, RHS.message, RHS.code); + } return std::tie(LHS.range, LHS.message) < std::tie(RHS.range, RHS.message); } }; diff --git a/clang-tools-extra/clangd/test/fixits-duplication.test b/clang-tools-extra/clangd/test/fixits-duplication.test new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clangd/test/fixits-duplication.test @@ -0,0 +1,221 @@ +# RUN: clangd -lit-test -clang-tidy-checks=modernize-use-nullptr,hicpp-use-nullptr < %s | FileCheck -strict-whitespace %s +{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"codeAction":{"codeActionLiteralSupport":{}}}},"trace":"off"}} +--- +{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.cpp","languageId":"cpp","version":1,"text":"void foo() { char* p = 0; }"}}} +# CHECK: "method": "textDocument/publishDiagnostics", +# CHECK-NEXT: "params": { +# CHECK-NEXT: "diagnostics": [ +# CHECK-NEXT: { +# CHECK-NEXT: "code": "hicpp-use-nullptr", +# CHECK-NEXT: "message": "Use nullptr (fix available)", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 24, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 23, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "severity": 2, +# CHECK-NEXT: "source": "clang-tidy" +# CHECK-NEXT: }, +# CHECK-NEXT: { +# CHECK-NEXT: "code": "modernize-use-nullptr", +# CHECK-NEXT: "message": "Use nullptr (fix available)", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 24, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 23, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "severity": 2, +# CHECK-NEXT: "source": "clang-tidy" +# CHECK-NEXT: } +# CHECK-NEXT: ], +# CHECK-NEXT: "uri": "file:///{{.*}}/foo.cpp" +# CHECK-NEXT: } +--- +{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.cpp"},"range":{"start":{"line":0,"character":23},"end":{"line":0,"character":24}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 23}, "end": {"line": 0, "character": 24}},"severity":2,"message":"Use nullptr (fix available)", "code": "hicpp-use-nullptr", "source": "clang-tidy"},{"range":{"start": {"line": 0, "character": 23}, "end": {"line": 0, "character": 24}},"severity":2,"message":"Use nullptr (fix available)", "code": "modernize-use-nullptr", "source": "clang-tidy"}]}}} +# CHECK: "id": 2, +# CHECK-NEXT: "jsonrpc": "2.0", +# CHECK-NEXT: "result": [ +# CHECK-NEXT: { +# CHECK-NEXT: "diagnostics": [ +# CHECK-NEXT: { +# CHECK-NEXT: "code": "hicpp-use-nullptr", +# CHECK-NEXT: "message": "Use nullptr (fix available)", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 24, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 23, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "severity": 2, +# CHECK-NEXT: "source": "clang-tidy" +# CHECK-NEXT: } +# CHECK-NEXT: ], +# CHECK-NEXT: "edit": { +# CHECK-NEXT: "changes": { +# CHECK-NEXT: "file://{{.*}}/foo.cpp": [ +# CHECK-NEXT: { +# CHECK-NEXT: "newText": "nullptr", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 24, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 23, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: ] +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "kind": "quickfix", +# CHECK-NEXT: "title": "change '0' to 'nullptr'" +# CHECK-NEXT: }, +# CHECK-NEXT: { +# CHECK-NEXT: "diagnostics": [ +# CHECK-NEXT: { +# CHECK-NEXT: "code": "modernize-use-nullptr", +# CHECK-NEXT: "message": "Use nullptr (fix available)", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 24, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 23, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "severity": 2, +# CHECK-NEXT: "source": "clang-tidy" +# CHECK-NEXT: } +# CHECK-NEXT: ], +# CHECK-NEXT: "edit": { +# CHECK-NEXT: "changes": { +# CHECK-NEXT: "file://{{.*}}/foo.cpp": [ +# CHECK-NEXT: { +# CHECK-NEXT: "newText": "nullptr", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 24, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 23, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: ] +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "kind": "quickfix", +# CHECK-NEXT: "title": "change '0' to 'nullptr'" +# CHECK-NEXT: } +# CHECK-NEXT: ] +--- +{"jsonrpc":"2.0","id":3,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.cpp"},"range":{"start":{"line":0,"character":23},"end":{"line":0,"character":24}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 23}, "end": {"line": 0, "character": 24}},"severity":2,"message":"Use nullptr (fix available)", "source": "clang-tidy"},{"range":{"start": {"line": 0, "character": 23}, "end": {"line": 0, "character": 24}},"severity":2,"message":"Use nullptr (fix available)", "source": "clang-tidy"}]}}} +# CHECK: "id": 3, +# CHECK-NEXT: "jsonrpc": "2.0", +# CHECK-NEXT: "result": [ +# CHECK-NEXT: { +# CHECK-NEXT: "diagnostics": [ +# CHECK-NEXT: { +# CHECK-NEXT: "message": "Use nullptr (fix available)", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 24, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 23, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "severity": 2, +# CHECK-NEXT: "source": "clang-tidy" +# CHECK-NEXT: } +# CHECK-NEXT: ], +# CHECK-NEXT: "edit": { +# CHECK-NEXT: "changes": { +# CHECK-NEXT: "file://{{.*}}/foo.cpp": [ +# CHECK-NEXT: { +# CHECK-NEXT: "newText": "nullptr", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 24, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 23, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: ] +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "kind": "quickfix", +# CHECK-NEXT: "title": "change '0' to 'nullptr'" +# CHECK-NEXT: }, +# CHECK-NEXT: { +# CHECK-NEXT: "diagnostics": [ +# CHECK-NEXT: { +# CHECK-NEXT: "message": "Use nullptr (fix available)", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 24, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 23, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "severity": 2, +# CHECK-NEXT: "source": "clang-tidy" +# CHECK-NEXT: } +# CHECK-NEXT: ], +# CHECK-NEXT: "edit": { +# CHECK-NEXT: "changes": { +# CHECK-NEXT: "file://{{.*}}/foo.cpp": [ +# CHECK-NEXT: { +# CHECK-NEXT: "newText": "nullptr", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 24, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 23, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: ] +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "kind": "quickfix", +# CHECK-NEXT: "title": "change '0' to 'nullptr'" +# CHECK-NEXT: } +# CHECK-NEXT: ] +--- +{"jsonrpc":"2.0","id":4,"method":"shutdown"} +--- +{"jsonrpc":"2.0","method":"exit"} +