hokein created this revision.
hokein added a reviewer: kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
hokein requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Fixes https://github.com/clangd/clangd/issues/1633
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D150548
Files:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/Protocol.cpp
clang-tools-extra/clangd/Protocol.h
clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
Index: clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
===================================================================
--- clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
+++ clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
@@ -308,6 +308,100 @@
# CHECK-NEXT: {
# CHECK-NEXT: "arguments": [
# CHECK-NEXT: {
+# CHECK-NEXT: "changeAnnotations": {
+# CHECK-NEXT: "AddAllMissingIncludes0": {
+# CHECK-NEXT: "description": "Provides {{.*}}",
+# CHECK-NEXT: "label": "{{.*}}",
+# CHECK-NEXT: "needsConfirmation": true
+# CHECK-NEXT: },
+# CHECK-NEXT: "AddAllMissingIncludes1": {
+# CHECK-NEXT: "description": "Provides {{.*}}",
+# CHECK-NEXT: "label": "{{.*}}",
+# CHECK-NEXT: "needsConfirmation": true
+# CHECK-NEXT: },
+# CHECK-NEXT: "RemoveAllUnusedIncludes0": {
+# CHECK-NEXT: "label": "Remove {{.*}}",
+# CHECK-NEXT: "needsConfirmation": true
+# CHECK-NEXT: },
+# CHECK-NEXT: "RemoveAllUnusedIncludes1": {
+# CHECK-NEXT: "label": "Remove {{.*}}",
+# CHECK-NEXT: "needsConfirmation": true
+# CHECK-NEXT: }
+# CHECK-NEXT: },
+# CHECK-NEXT: "documentChanges": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "edits": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "annotationId": "RemoveAllUnusedIncludes0",
+# CHECK-NEXT: "newText": "",
+# CHECK-NEXT: "range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT: "character": 0,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT: "character": 0,
+# CHECK-NEXT: "line": 0
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: },
+# CHECK-NEXT: {
+# CHECK-NEXT: "annotationId": "RemoveAllUnusedIncludes1",
+# CHECK-NEXT: "newText": "",
+# CHECK-NEXT: "range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT: "character": 0,
+# CHECK-NEXT: "line": 2
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT: "character": 0,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: },
+# CHECK-NEXT: {
+# CHECK-NEXT: "annotationId": "AddAllMissingIncludes0",
+# CHECK-NEXT: "newText": "#include {{.*}}foo.h{{.*}}",
+# CHECK-NEXT: "range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT: "character": 0,
+# CHECK-NEXT: "line": 2
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT: "character": 0,
+# CHECK-NEXT: "line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: },
+# CHECK-NEXT: {
+# CHECK-NEXT: "annotationId": "AddAllMissingIncludes1",
+# CHECK-NEXT: "newText": "#include {{.*}}bar.h{{.*}}",
+# CHECK-NEXT: "range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT: "character": 0,
+# CHECK-NEXT: "line": 2
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT: "character": 0,
+# CHECK-NEXT: "line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: ],
+# CHECK-NEXT: "textDocument": {
+# CHECK-NEXT: "uri": "file://{{.*}}/simple.cpp",
+# CHECK-NEXT: "version": 0
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: ]
+# CHECK-NEXT: }
+# CHECK-NEXT: ],
+# CHECK-NEXT: "command": "clangd.applyFix",
+# CHECK-NEXT: "title": "Apply fix: fix all includes"
+# CHECK-NEXT: },
+# CHECK-NEXT: {
+# CHECK-NEXT: "arguments": [
+# CHECK-NEXT: {
# CHECK-NEXT: "documentChanges": [
# CHECK-NEXT: {
# CHECK-NEXT: "edits": [
@@ -391,7 +485,13 @@
# CHECK-NEXT: ],
# CHECK-NEXT: "command": "clangd.applyFix",
# CHECK-NEXT: "title": "Apply fix: remove all unused includes"
-# CHECK-NEXT: },
+# CHECK-NEXT: }
+# CHECK-NEXT: ]
+---
+{"jsonrpc":"2.0","id":4,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///simple.cpp"},"range":{"start":{"line":0,"character":0},"end":{"line":0,"character":17}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 0}, "end": {"line": 0, "character": 17}},"severity":2,"message":"Included header all1.h is not used directly (fixes available)", "code": "unused-includes", "source": "clangd"},{"range":{"start": {"line": 1, "character": 0}, "end": {"line": 1, "character": 17}},"severity":2,"message":"Included header all2.h is not used directly (fixes available)", "code": "unused-includes", "source": "clangd"}]}}}
+# CHECK: "id": 4,
+# CHECK-NEXT: "jsonrpc": "2.0",
+# CHECK-NEXT: "result": [
# CHECK-NEXT: {
# CHECK-NEXT: "arguments": [
# CHECK-NEXT: {
@@ -485,9 +585,127 @@
# CHECK-NEXT: ],
# CHECK-NEXT: "command": "clangd.applyFix",
# CHECK-NEXT: "title": "Apply fix: fix all includes"
+# CHECK-NEXT: },
+# CHECK-NEXT: {
+# CHECK-NEXT: "arguments": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "documentChanges": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "edits": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "newText": "",
+# CHECK-NEXT: "range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT: "character": 0,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT: "character": 0,
+# CHECK-NEXT: "line": 0
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: ],
+# CHECK-NEXT: "textDocument": {
+# CHECK-NEXT: "uri": "file://{{.*}}/simple.cpp",
+# CHECK-NEXT: "version": 0
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: ]
+# CHECK-NEXT: }
+# CHECK-NEXT: ],
+# CHECK-NEXT: "command": "clangd.applyFix",
+# CHECK-NEXT: "title": "Apply fix: remove #include directive"
+# CHECK-NEXT: },
+# CHECK-NEXT: {
+# CHECK-NEXT: "arguments": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "documentChanges": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "edits": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "newText": "",
+# CHECK-NEXT: "range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT: "character": 0,
+# CHECK-NEXT: "line": 2
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT: "character": 0,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: ],
+# CHECK-NEXT: "textDocument": {
+# CHECK-NEXT: "uri": "file://{{.*}}/simple.cpp",
+# CHECK-NEXT: "version": 0
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: ]
+# CHECK-NEXT: }
+# CHECK-NEXT: ],
+# CHECK-NEXT: "command": "clangd.applyFix",
+# CHECK-NEXT: "title": "Apply fix: remove #include directive"
+# CHECK-NEXT: },
+# CHECK-NEXT: {
+# CHECK-NEXT: "arguments": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "changeAnnotations": {
+# CHECK-NEXT: "RemoveAllUnusedIncludes0": {
+# CHECK-NEXT: "label": "Remove '#include \"all1.h\"'",
+# CHECK-NEXT: "needsConfirmation": true
+# CHECK-NEXT: },
+# CHECK-NEXT: "RemoveAllUnusedIncludes1": {
+# CHECK-NEXT: "label": "Remove '#include \"all2.h\"'",
+# CHECK-NEXT: "needsConfirmation": true
+# CHECK-NEXT: }
+# CHECK-NEXT: },
+# CHECK-NEXT: "documentChanges": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "edits": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "annotationId": "RemoveAllUnusedIncludes0",
+# CHECK-NEXT: "newText": "",
+# CHECK-NEXT: "range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT: "character": 0,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT: "character": 0,
+# CHECK-NEXT: "line": 0
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: },
+# CHECK-NEXT: {
+# CHECK-NEXT: "annotationId": "RemoveAllUnusedIncludes1",
+# CHECK-NEXT: "newText": "",
+# CHECK-NEXT: "range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT: "character": 0,
+# CHECK-NEXT: "line": 2
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT: "character": 0,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: ],
+# CHECK-NEXT: "textDocument": {
+# CHECK-NEXT: "uri": "file://{{.*}}/simple.cpp",
+# CHECK-NEXT: "version": 0
+# CHECK-NEXT: }
+# CHECK-NEXT: }
+# CHECK-NEXT: ]
+# CHECK-NEXT: }
+# CHECK-NEXT: ],
+# CHECK-NEXT: "command": "clangd.applyFix",
+# CHECK-NEXT: "title": "Apply fix: remove all unused includes"
# CHECK-NEXT: }
# CHECK-NEXT: ]
---
-{"jsonrpc":"2.0","id":4,"method":"shutdown"}
+{"jsonrpc":"2.0","id":5,"method":"shutdown"}
---
{"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/Protocol.h
===================================================================
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -257,6 +257,7 @@
return std::tie(L.newText, L.range, L.annotationId) ==
std::tie(R.newText, R.range, L.annotationId);
}
+bool operator<(const TextEdit &, const TextEdit &);
bool fromJSON(const llvm::json::Value &, TextEdit &, llvm::json::Path);
llvm::json::Value toJSON(const TextEdit &);
llvm::raw_ostream &operator<<(llvm::raw_ostream &, const TextEdit &);
@@ -276,6 +277,7 @@
};
bool fromJSON(const llvm::json::Value &, ChangeAnnotation &, llvm::json::Path);
llvm::json::Value toJSON(const ChangeAnnotation &);
+bool operator<(const ChangeAnnotation&, const ChangeAnnotation&);
struct TextDocumentEdit {
/// The text document to change.
@@ -287,6 +289,7 @@
};
bool fromJSON(const llvm::json::Value &, TextDocumentEdit &, llvm::json::Path);
llvm::json::Value toJSON(const TextDocumentEdit &);
+bool operator<(const TextDocumentEdit &, const TextDocumentEdit &);
struct TextDocumentItem {
/// The text document's URI.
@@ -1029,7 +1032,7 @@
};
bool fromJSON(const llvm::json::Value &, WorkspaceEdit &, llvm::json::Path);
llvm::json::Value toJSON(const WorkspaceEdit &WE);
-
+bool operator<(const WorkspaceEdit &, const WorkspaceEdit &);
/// Arguments for the 'applyTweak' command. The server sends these commands as a
/// response to the textDocument/codeAction request. The client can later send a
/// command back to the server if the user requests to execute a particular code
Index: clang-tools-extra/clangd/Protocol.cpp
===================================================================
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -21,6 +21,7 @@
#include "llvm/Support/JSON.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/raw_ostream.h"
+#include <tuple>
namespace clang {
namespace clangd {
@@ -184,6 +185,11 @@
O.map("version", R.version) && O.map("text", R.text);
}
+bool operator<(const TextEdit &L, const TextEdit &R) {
+ return std::tie(L.newText, L.range, L.annotationId) <
+ std::tie(R.newText, R.range, L.annotationId);
+}
+
bool fromJSON(const llvm::json::Value &Params, TextEdit &R,
llvm::json::Path P) {
llvm::json::ObjectMapper O(Params, P);
@@ -216,6 +222,10 @@
Result["description"] = CA.description;
return Result;
}
+bool operator<(const ChangeAnnotation& LHS, const ChangeAnnotation& RHS) {
+ return std::tie(LHS.label, LHS.description, LHS.needsConfirmation) < std::tie(
+ LHS.label, LHS.description, LHS.needsConfirmation);
+}
bool fromJSON(const llvm::json::Value &Params, TextDocumentEdit &R,
llvm::json::Path P) {
@@ -227,6 +237,10 @@
{"edits", P.edits}};
return Result;
}
+inline bool operator<(const TextDocumentEdit &L, const TextDocumentEdit &R) {
+ return std::tie(L.textDocument.uri, L.textDocument.version, L.edits) <
+ std::tie(R.textDocument.uri, L.textDocument.version, R.edits);
+}
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const TextEdit &TE) {
OS << TE.range << " => \"";
@@ -920,6 +934,10 @@
}
return Result;
}
+bool operator<(const WorkspaceEdit &LHS, const WorkspaceEdit &RHS) {
+ return std::tie(LHS.changes, LHS.documentChanges, LHS.changeAnnotations) <
+ std::tie(RHS.changes, RHS.documentChanges, RHS.changeAnnotations);
+}
bool fromJSON(const llvm::json::Value &Params, TweakArgs &A,
llvm::json::Path P) {
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -289,6 +289,7 @@
// FIXME(kirillbobyrev): Add fix suggestion for adding IWYU pragmas
// (keep/export) remove the warning once we support IWYU pragmas.
auto &F = D.Fixes.emplace_back();
+ // FIXME: make the message more distinct by spelling the header.
F.Message = "remove #include directive";
F.Edits.emplace_back();
F.Edits.back().range.start.line = Inc->HashLine;
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -41,10 +41,12 @@
#include <cstddef>
#include <cstdint>
#include <functional>
+#include <map>
#include <memory>
#include <mutex>
#include <optional>
#include <string>
+#include <tuple>
#include <utility>
#include <vector>
@@ -1007,6 +1009,8 @@
return Cmd;
}
+
+
void ClangdLSPServer::onCodeAction(const CodeActionParams &Params,
Callback<llvm::json::Value> Reply) {
URIForFile File = Params.textDocument.uri;
@@ -1022,10 +1026,27 @@
// We provide a code action for Fixes on the specified diagnostics.
std::vector<CodeAction> FixIts;
if (KindAllowed(CodeAction::QUICKFIX_KIND)) {
+ // A case-specific comparator used to deduplicate diagnostic fixes.
+ // The lessthan is mainly based on fix edit.
+ // A fixit might fix multiple diagnostics (e.g. "remove all unused includes"
+ // fix all unused-includes diagnostics), we don't want to show this
+ // "remove all" fix multiple times in the UI.
+ struct CodeActionLessthan {
+ bool operator()(const CodeAction &LHS, const CodeAction &RHS) const {
+ return std::tie(LHS.title, LHS.edit) < std::tie(RHS.title, RHS.edit);
+ }
+ };
+ std::map<CodeAction, std::vector<const Diagnostic *>, CodeActionLessthan>
+ FixToDiagnostic;
for (const Diagnostic &D : Params.context.diagnostics) {
- for (auto &CA : getFixes(File.file(), D)) {
- FixIts.push_back(CA);
- FixIts.back().diagnostics = {D};
+ for (auto &CA : getFixes(File.file(), D))
+ FixToDiagnostic[CA].push_back(&D);
+ }
+ for (auto& [CA, Diags] : FixToDiagnostic) {
+ FixIts.push_back(CA);
+ FixIts.back().diagnostics.emplace();
+ for (auto* Diag: Diags) {
+ FixIts.back().diagnostics->push_back(*Diag);
}
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
