This revision was automatically updated to reflect the committed changes.
Closed by commit rL363889: [clangd] Include the diagnostics's code when
comparing diagnostics (authored by nridge, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D63316?vs=205694&id=205695#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63316/new/
https://reviews.llvm.org/D63316
Files:
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/clangd/test/fixits-duplication.test
Index: clang-tools-extra/trunk/clangd/test/fixits-duplication.test
===================================================================
--- clang-tools-extra/trunk/clangd/test/fixits-duplication.test
+++ clang-tools-extra/trunk/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"}
+
Index: clang-tools-extra/trunk/clangd/Protocol.h
===================================================================
--- clang-tools-extra/trunk/clangd/Protocol.h
+++ clang-tools-extra/trunk/clangd/Protocol.h
@@ -616,7 +616,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.
@@ -666,11 +665,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);
}
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
