diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -41,10 +41,12 @@ #include #include #include +#include #include #include #include #include +#include #include #include @@ -1007,6 +1009,8 @@ return Cmd; } + + void ClangdLSPServer::onCodeAction(const CodeActionParams &Params, Callback Reply) { URIForFile File = Params.textDocument.uri; @@ -1022,10 +1026,27 @@ // We provide a code action for Fixes on the specified diagnostics. std::vector 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, 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); } } } diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/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; 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 @@ -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 diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/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 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) { diff --git a/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test b/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test --- a/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test +++ b/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"}