[clang-tools-extra] r363889 - [clangd] Include the diagnostics's code when comparing diagnostics
Nathan Ridge via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 19 16:11:02 PDT 2019
Author: nridge
Date: Wed Jun 19 16:11:02 2019
New Revision: 363889
URL: http://llvm.org/viewvc/llvm-project?rev=363889&view=rev
Log:
[clangd] Include the diagnostics's code when comparing diagnostics
Summary: This fixes https://github.com/clangd/clangd/issues/60
Reviewers: kadircet
Reviewed By: kadircet
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D63316
Added:
clang-tools-extra/trunk/clangd/test/fixits-duplication.test
Modified:
clang-tools-extra/trunk/clangd/Protocol.h
Modified: clang-tools-extra/trunk/clangd/Protocol.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=363889&r1=363888&r2=363889&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Protocol.h (original)
+++ clang-tools-extra/trunk/clangd/Protocol.h Wed Jun 19 16:11:02 2019
@@ -616,7 +616,6 @@ struct DocumentSymbolParams {
};
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 @@ llvm::json::Value toJSON(const Diagnosti
/// 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);
}
};
Added: clang-tools-extra/trunk/clangd/test/fixits-duplication.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/fixits-duplication.test?rev=363889&view=auto
==============================================================================
--- clang-tools-extra/trunk/clangd/test/fixits-duplication.test (added)
+++ clang-tools-extra/trunk/clangd/test/fixits-duplication.test Wed Jun 19 16:11:02 2019
@@ -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"}
+
More information about the cfe-commits
mailing list