ilya-biryukov created this revision. ilya-biryukov added reviewers: kadircet, ioeric, hokein, sammccall. Herald added subscribers: arphaman, jkorous, MaskRay, mgorny.
The code actions can be run in two stages: - Stage 1. Decides whether the action is available to the user and collects all the information required to finish the action. Should be cheap, since this will run over all the actions known to clangd on each textDocument/codeAction request. - Stage 2. Uses information from stage 1 to produce the actual edits that the code action should perform. This stage can be expensive and will only run if the user chooses to perform the specified action in the UI. Trivial actions can also produce results in stage 1 to avoid an extra round-trip and read of the AST to reconstruct the action on the server. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D56267 Files: clangd/CMakeLists.txt clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/CodeActions.cpp clangd/CodeActions.h clangd/Protocol.cpp clangd/Protocol.h clangd/SourceCode.cpp clangd/SourceCode.h test/clangd/initialize-params.test
Index: test/clangd/initialize-params.test =================================================================== --- test/clangd/initialize-params.test +++ test/clangd/initialize-params.test @@ -25,7 +25,8 @@ # CHECK-NEXT: "documentSymbolProvider": true, # CHECK-NEXT: "executeCommandProvider": { # CHECK-NEXT: "commands": [ -# CHECK-NEXT: "clangd.applyFix" +# CHECK-NEXT: "clangd.applyFix", +# CHECK-NEXT: "clangd.applyCodeAction" # CHECK-NEXT: ] # CHECK-NEXT: }, # CHECK-NEXT: "hoverProvider": true, Index: clangd/SourceCode.h =================================================================== --- clangd/SourceCode.h +++ clangd/SourceCode.h @@ -65,6 +65,10 @@ std::pair<size_t, size_t> offsetToClangLineColumn(llvm::StringRef Code, size_t Offset); +/// Return the substring, corresponding to the passed range. +llvm::Expected<llvm::StringRef> rangeSubstr(llvm::StringRef Code, + const Range &R); + /// From "a::b::c", return {"a::b::", "c"}. Scope is empty if there's no /// qualifier. std::pair<llvm::StringRef, llvm::StringRef> Index: clangd/SourceCode.cpp =================================================================== --- clangd/SourceCode.cpp +++ clangd/SourceCode.cpp @@ -161,6 +161,20 @@ return {Lines + 1, Offset - StartOfLine + 1}; } +llvm::Expected<llvm::StringRef> rangeSubstr(llvm::StringRef Code, + const Range &R) { + auto Begin = positionToOffset(Code, R.start, false); + if (!Begin) + return Begin.takeError(); + auto End = positionToOffset(Code, R.end, false); + if (!End) + return End.takeError(); + if (*End < *Begin) + return createStringError(inconvertibleErrorCode(), + "invalid range passed to rangeSubstr"); + return Code.substr(*Begin, *End - *Begin); +} + std::pair<StringRef, StringRef> splitQualifiedName(StringRef QName) { size_t Pos = QName.rfind("::"); if (Pos == StringRef::npos) Index: clangd/Protocol.h =================================================================== --- clangd/Protocol.h +++ clangd/Protocol.h @@ -632,6 +632,14 @@ bool fromJSON(const llvm::json::Value &, WorkspaceEdit &); llvm::json::Value toJSON(const WorkspaceEdit &WE); +struct CodeActionArgs { + std::string file; + int64_t actionId; + Range selection; +}; +bool fromJSON(const llvm::json::Value &, CodeActionArgs &); +llvm::json::Value toJSON(const CodeActionArgs &A); + /// Exact commands are not specified in the protocol so we define the /// ones supported by Clangd here. The protocol specifies the command arguments /// to be "any[]" but to make this safer and more manageable, each command we @@ -643,12 +651,15 @@ struct ExecuteCommandParams { // Command to apply fix-its. Uses WorkspaceEdit as argument. const static llvm::StringLiteral CLANGD_APPLY_FIX_COMMAND; + // Command to apply the code action. Uses action id as the argument. + const static llvm::StringLiteral CLANGD_APPLY_CODE_ACTION; /// The command identifier, e.g. CLANGD_APPLY_FIX_COMMAND std::string command; // Arguments llvm::Optional<WorkspaceEdit> workspaceEdit; + llvm::Optional<CodeActionArgs> codeActionArgs; }; bool fromJSON(const llvm::json::Value &, ExecuteCommandParams &); @@ -670,6 +681,7 @@ /// Used to filter code actions. llvm::Optional<std::string> kind; const static llvm::StringLiteral QUICKFIX_KIND; + const static llvm::StringLiteral REFACTOR_KIND; /// The diagnostics that this code action resolves. llvm::Optional<std::vector<Diagnostic>> diagnostics; Index: clangd/Protocol.cpp =================================================================== --- clangd/Protocol.cpp +++ clangd/Protocol.cpp @@ -418,6 +418,9 @@ const StringLiteral ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND = "clangd.applyFix"; +const StringLiteral ExecuteCommandParams::CLANGD_APPLY_CODE_ACTION = + "clangd.applyCodeAction"; + bool fromJSON(const json::Value &Params, ExecuteCommandParams &R) { json::ObjectMapper O(Params); if (!O || !O.map("command", R.command)) @@ -428,6 +431,9 @@ return Args && Args->size() == 1 && fromJSON(Args->front(), R.workspaceEdit); } + if (R.command == ExecuteCommandParams::CLANGD_APPLY_CODE_ACTION) + return Args && Args->size() == 1 && + fromJSON(Args->front(), R.codeActionArgs); return false; // Unrecognized command. } @@ -493,10 +499,13 @@ auto Cmd = json::Object{{"title", C.title}, {"command", C.command}}; if (C.workspaceEdit) Cmd["arguments"] = {*C.workspaceEdit}; + if (C.codeActionArgs) + Cmd["arguments"] = {*C.codeActionArgs}; return std::move(Cmd); } const StringLiteral CodeAction::QUICKFIX_KIND = "quickfix"; +const StringLiteral CodeAction::REFACTOR_KIND = "refactor"; json::Value toJSON(const CodeAction &CA) { auto CodeAction = json::Object{{"title", CA.title}}; @@ -540,6 +549,17 @@ return json::Object{{"changes", std::move(FileChanges)}}; } +bool fromJSON(const llvm::json::Value &Params, CodeActionArgs &A) { + json::ObjectMapper O(Params); + return O && O.map("actionId", A.actionId) && + O.map("selection", A.selection) && O.map("file", A.file); +} + +llvm::json::Value toJSON(const CodeActionArgs &A) { + return json::Object{ + {"actionId", A.actionId}, {"selection", A.selection}, {"file", A.file}}; +} + json::Value toJSON(const ApplyWorkspaceEditParams &Params) { return json::Object{{"edit", Params.edit}}; } Index: clangd/CodeActions.h =================================================================== --- /dev/null +++ clangd/CodeActions.h @@ -0,0 +1,134 @@ +//===--- CodeActions.h -------------------------------------------*- C++-*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// The code actions are small refactoring-like actions that run over the AST and +// produce the set of edits as a result. They are local, i.e. they should take +// the current editor context, e.g. the cursor position and selection into +// account. A list of actions available in a current context can be obtained by +// calling CodeActions::prepareAll(). +// The actions are executed in two stages: +// - Stage 1 should check whether the action is available in a current +// context. It should be cheap and fast to compute as it is executed for all +// available actions on every client request, which happen quite frequently. +// - Stage 2 is performed after stage 1 and can be more expensive to compute. +// It is performed when the user actually chooses the action. +// To avoid extra round-trips and AST reads, actions can also produce results on +// stage 1 in cases when the actions are fast to compute. +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODEACTIONS_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODEACTIONS_H + +#include "Protocol.h" +#include "llvm/ADT/FunctionExtras.h" +#include <string> +#include <vector> + +namespace clang { +namespace clangd { + +class ParsedAST; + +/// The unique identifier of the code action. +using ActionID = size_t; + +struct ActionInputs; +class PreparedAction; +class ActionProvider; + +/// The collection of code actions available in clangd. +class CodeActions { +public: + CodeActions(); + + /// Produces a list of all actions available in the specified context. + std::vector<PreparedAction> prepareAll(const ActionInputs &Inputs) const; + + /// Prepares an action with a specified \p ID for the final execution. This + /// method is used to execute an action requested by a user in the UI. + llvm::Optional<PreparedAction> prepare(ActionID ID, + const ActionInputs &Inputs) const; + +private: + std::vector<std::unique_ptr<ActionProvider>> Actions; +}; + +/// A context used by the actions to identify +struct ActionInputs { + /// The path of an active document the action was invoked in. + llvm::StringRef File; + /// The text of the active document. + llvm::StringRef Code; + /// Selection range the action was invoked with. + Range Selection; + /// Parsed AST of the active file. + ParsedAST *AST = nullptr; + // FIXME: provide a way to get sources and ASTs for other files. + // FIXME: cache some commonly required information (e.g. AST nodes under + // cursor) to avoid redundant AST visit in every action. +}; + +/// Result of executing a first stage of the action. If computing the resulting +/// WorkspaceEdit is fast, the actions should produce it right away. +/// For non-trivial actions, a continuation function to compute the resulting +/// edits should be provided instead. It is expected that PreparedAction is +/// consumed immediately when created, so continuations can reference the AST, +/// so it's not safe to store the PreparedAction for long spans of time. +class PreparedAction { +public: + PreparedAction( + std::string Title, + llvm::unique_function<llvm::Expected<WorkspaceEdit>()> FinishAction); + PreparedAction(std::string Title, WorkspaceEdit Result); + + /// An id of the action that can be used to re-run it with + /// CodeActions::prepare(). + ActionID id() const { return ID; } + /// A title of the action for display in the UI. + llvm::StringRef title() const { return Title; } + /// Whether the results are immediately available. If this function returns + /// false, computeEdits() won't fail and is very cheap to compute. + bool needsMoreWork() const { return static_cast<bool>(FinishAction); } + /// Consume the action to compute the resulting edits. This function can be + /// expensive if needsMoreWork() returns true. + llvm::Expected<WorkspaceEdit> computeEdits() && { + if (FinishAction) + return FinishAction(); + return std::move(*Result); + } + +private: + void setID(ActionID V) { ID = V; } // only available to CodeActions. + friend class CodeActions; + + ActionID ID = std::numeric_limits<ActionID>::max(); // set by CodeActions(). + std::string Title; /// For display in the UI. + // Exactly one of FinishAction and Result will be set. + llvm::unique_function<llvm::Expected<WorkspaceEdit>()> FinishAction; + llvm::Optional<WorkspaceEdit> Result; +}; + +/// Base interface for writing a code action. +class ActionProvider { +public: + virtual ~ActionProvider() = default; + + /// Run the first stage of the action. The non-None result indicates that the + /// action is available and should be shown to the user. Returns None if the + /// action is not available. + /// This function should be fast, if the action requires non-trivial work it + /// should be moved into the second stage, i.e. the continuation function + /// provided in the resulting PreparedAction. + virtual llvm::Optional<PreparedAction> + prepare(const ActionInputs &Inputs) = 0; +}; + +} // namespace clangd +} // namespace clang + +#endif \ No newline at end of file Index: clangd/CodeActions.cpp =================================================================== --- /dev/null +++ clangd/CodeActions.cpp @@ -0,0 +1,59 @@ +//===--- CodeActions.cpp -----------------------------------------*- C++-*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +#include "CodeActions.h" +#include "Logger.h" +#include "SourceCode.h" +#include "llvm/Support/Error.h" +#include "llvm/Support/FormatVariadic.h" +#include <algorithm> + +using namespace llvm; + +namespace clang { +namespace clangd { + +PreparedAction::PreparedAction( + std::string Title, + llvm::unique_function<llvm::Expected<WorkspaceEdit>()> FinishAction) + : Title(std::move(Title)), FinishAction(std::move(FinishAction)) { + assert(this->FinishAction); +} + +PreparedAction::PreparedAction(std::string Title, WorkspaceEdit Result) + : Title(std::move(Title)), Result(std::move(Result)) {} + +CodeActions::CodeActions() { + // FIXME: Create all known actions here. + // FIXME: provide a registry of the providers. +} + +std::vector<PreparedAction> +CodeActions::prepareAll(const ActionInputs &Inputs) const { + std::vector<PreparedAction> Available; + for (size_t I = 0; I < Actions.size(); ++I) { + auto A = Actions[I]->prepare(Inputs); + if (!A) + continue; + A->setID(I); + Available.push_back(std::move(*A)); + } + return Available; +} + +llvm::Optional<PreparedAction> +CodeActions::prepare(ActionID ID, const ActionInputs &Inputs) const { + if (Actions.size() <= ID) { + elog("id of the code action ({0}) is invalid", ID); + return llvm::None; + } + return Actions[ID]->prepare(Inputs); +} + +} // namespace clangd +} // namespace clang \ No newline at end of file Index: clangd/ClangdServer.h =================================================================== --- clangd/ClangdServer.h +++ clangd/ClangdServer.h @@ -12,6 +12,7 @@ #include "Cancellation.h" #include "ClangdUnit.h" +#include "CodeActions.h" #include "CodeComplete.h" #include "FSProvider.h" #include "Function.h" @@ -202,6 +203,17 @@ void rename(PathRef File, Position Pos, llvm::StringRef NewName, Callback<std::vector<tooling::Replacement>> CB); + /// Enumerate the code actions available to the user at a specified point. + /// This runs the first stage of all the code actions known to clangd, see + /// CodeActions.h on distinction between the stages of the actions. + void enumerateCodeActions(PathRef File, Range R, + Callback<std::vector<PreparedAction>> CB); + + /// Apply the previously precomputed code action. This will always fully + /// execute the action. + void applyCodeAction(PathRef File, Range R, ActionID ID, + Callback<WorkspaceEdit> CB); + /// Only for testing purposes. /// Waits until all requests to worker thread are finished and dumps AST for /// \p File. \p File must be in the list of added documents. @@ -245,6 +257,7 @@ const FileSystemProvider &FSProvider; Path ResourceDir; + CodeActions Actions; // The index used to look up symbols. This could be: // - null (all index functionality is optional) // - the dynamic index owned by ClangdServer (DynamicIdx) Index: clangd/ClangdServer.cpp =================================================================== --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -324,6 +324,49 @@ "Rename", File, Bind(Action, File.str(), NewName.str(), std::move(CB))); } +void ClangdServer::enumerateCodeActions( + PathRef File, Range Selection, Callback<std::vector<PreparedAction>> CB) { + auto Action = [this, Selection](decltype(CB) CB, std::string File, + Expected<InputsAndAST> InpAST) { + if (!InpAST) + return CB(InpAST.takeError()); + + ActionInputs Inputs; + Inputs.File = File; + Inputs.Code = InpAST->Inputs.Contents; + Inputs.AST = &InpAST->AST; + Inputs.Selection = Selection; + + CB(Actions.prepareAll(Inputs)); + }; + + WorkScheduler.runWithAST("EnumerateCodeActions", File, + Bind(Action, std::move(CB), File.str())); +} + +void ClangdServer::applyCodeAction(PathRef File, Range R, ActionID ID, + Callback<WorkspaceEdit> CB) { + auto Action = [this, ID, R](decltype(CB) CB, std::string File, + Expected<InputsAndAST> InpAST) { + if (!InpAST) + return CB(InpAST.takeError()); + + ActionInputs Inputs; + Inputs.File = File; + Inputs.Code = InpAST->Inputs.Contents; + Inputs.AST = &InpAST->AST; + Inputs.Selection = R; + + auto A = Actions.prepare(ID, Inputs); + if (!A) + return CB(createStringError(inconvertibleErrorCode(), + "failed to recreate the code action")); + return CB(std::move(*A).computeEdits()); + }; + WorkScheduler.runWithAST("ApplyCodeAction", File, + Bind(Action, std::move(CB), File.str())); +} + void ClangdServer::dumpAST(PathRef File, unique_function<void(std::string)> Callback) { auto Action = [](decltype(Callback) Callback, Expected<InputsAndAST> InpAST) { Index: clangd/ClangdLSPServer.cpp =================================================================== --- clangd/ClangdLSPServer.cpp +++ clangd/ClangdLSPServer.cpp @@ -163,9 +163,9 @@ Server(Other.Server), TraceArgs(Other.TraceArgs) { Other.Server = nullptr; } - ReplyOnce& operator=(ReplyOnce&&) = delete; + ReplyOnce &operator=(ReplyOnce &&) = delete; ReplyOnce(const ReplyOnce &) = delete; - ReplyOnce& operator=(const ReplyOnce&) = delete; + ReplyOnce &operator=(const ReplyOnce &) = delete; ~ReplyOnce() { if (Server && !Replied) { @@ -325,7 +325,9 @@ {"referencesProvider", true}, {"executeCommandProvider", json::Object{ - {"commands", {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND}}, + {"commands", + {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND, + ExecuteCommandParams::CLANGD_APPLY_CODE_ACTION}}, }}, }}}}); } @@ -387,7 +389,7 @@ void ClangdLSPServer::onCommand(const ExecuteCommandParams &Params, Callback<json::Value> Reply) { - auto ApplyEdit = [&](WorkspaceEdit WE) { + auto ApplyEdit = [this](WorkspaceEdit WE) { ApplyWorkspaceEditParams Edit; Edit.edit = std::move(WE); // Ideally, we would wait for the response and if there is no error, we @@ -407,6 +409,18 @@ Reply("Fix applied."); ApplyEdit(*Params.workspaceEdit); + } else if (Params.command == ExecuteCommandParams::CLANGD_APPLY_CODE_ACTION && + Params.codeActionArgs) { + auto Action = [ApplyEdit](decltype(Reply) Reply, + Expected<WorkspaceEdit> WE) { + if (!WE) + return Reply(WE.takeError()); + Reply("Fix applied."); + ApplyEdit(std::move(*WE)); + }; + Server->applyCodeAction( + Params.codeActionArgs->file, Params.codeActionArgs->selection, + Params.codeActionArgs->actionId, Bind(Action, std::move(Reply))); } else { // We should not get here because ExecuteCommandParams would not have // parsed in the first place and this handler should not be called. But if @@ -588,49 +602,97 @@ void ClangdLSPServer::onCodeAction(const CodeActionParams &Params, Callback<json::Value> Reply) { - auto Code = DraftMgr.getDraft(Params.textDocument.uri.file()); + StringRef File = Params.textDocument.uri.file(); + auto Code = DraftMgr.getDraft(File); if (!Code) return Reply(make_error<LSPError>("onCodeAction called for non-added file", ErrorCode::InvalidParams)); // We provide a code action for Fixes on the specified diagnostics. - std::vector<CodeAction> Actions; + std::vector<CodeAction> FixIts; for (const Diagnostic &D : Params.context.diagnostics) { - for (auto &F : getFixes(Params.textDocument.uri.file(), D)) { - Actions.push_back(toCodeAction(F, Params.textDocument.uri)); - Actions.back().diagnostics = {D}; + for (auto &F : getFixes(File, D)) { + FixIts.push_back(toCodeAction(F, Params.textDocument.uri)); + FixIts.back().diagnostics = {D}; } } - if (SupportsCodeAction) - Reply(json::Array(Actions)); - else { - std::vector<Command> Commands; - for (const auto &Action : Actions) - if (auto Command = asCommand(Action)) - Commands.push_back(std::move(*Command)); - Reply(json::Array(Commands)); - } + // Now enumerate the semantic code actions. + auto ConsumeActions = + [this](decltype(Reply) Reply, std::string File, Range R, + std::vector<CodeAction> FixIts, + Expected<std::vector<PreparedAction>> SemActions) { + if (!SemActions) { + elog("error while getting semantic code actions: {0}", + SemActions.takeError()); + return Reply(json::Array(FixIts)); + } + + std::vector<CodeAction> Actions = std::move(FixIts); + auto toCodeAction = [&](PreparedAction &&A) -> CodeAction { + CodeAction CA; + CA.title = A.title(); + CA.kind = CodeAction::REFACTOR_KIND; + if (!A.needsMoreWork()) { + // This is an instant action, so we reply with a command to directly + // apply the edit that the action has already produced. + auto Edit = std::move(A).computeEdits(); + assert(Edit && "instant actions should not produce errors"); + CA.edit = std::move(*Edit); + return CA; + } + + // This action requires an expensive second stage, we only run it if + // the user actually chooses the action. So we reply with a command to + // run the full action when requested. + CA.command.emplace(); + CA.command->title = A.title(); + CA.command->command = Command::CLANGD_APPLY_CODE_ACTION; + CA.command->codeActionArgs.emplace(); + CA.command->codeActionArgs->file = File; + CA.command->codeActionArgs->actionId = A.id(); + CA.command->codeActionArgs->selection = R; + return CA; + }; + + Actions.reserve(Actions.size() + SemActions->size()); + for (PreparedAction &A : *SemActions) + Actions.push_back(toCodeAction(std::move(A))); + + if (SupportsCodeAction) + return Reply(json::Array(Actions)); + std::vector<Command> Commands; + for (const auto &Action : Actions) { + if (auto Command = asCommand(Action)) + Commands.push_back(std::move(*Command)); + } + return Reply(json::Array(Commands)); + }; + + Server->enumerateCodeActions(File, Params.range, + Bind(ConsumeActions, std::move(Reply), + File.str(), Params.range, + std::move(FixIts))); } void ClangdLSPServer::onCompletion(const TextDocumentPositionParams &Params, Callback<CompletionList> Reply) { - Server->codeComplete(Params.textDocument.uri.file(), Params.position, CCOpts, - Bind( - [this](decltype(Reply) Reply, - Expected<CodeCompleteResult> List) { - if (!List) - return Reply(List.takeError()); - CompletionList LSPList; - LSPList.isIncomplete = List->HasMore; - for (const auto &R : List->Completions) { - CompletionItem C = R.render(CCOpts); - C.kind = adjustKindToCapability( - C.kind, SupportedCompletionItemKinds); - LSPList.items.push_back(std::move(C)); - } - return Reply(std::move(LSPList)); - }, - std::move(Reply))); + Server->codeComplete( + Params.textDocument.uri.file(), Params.position, CCOpts, + Bind( + [this](decltype(Reply) Reply, Expected<CodeCompleteResult> List) { + if (!List) + return Reply(List.takeError()); + CompletionList LSPList; + LSPList.isIncomplete = List->HasMore; + for (const auto &R : List->Completions) { + CompletionItem C = R.render(CCOpts); + C.kind = + adjustKindToCapability(C.kind, SupportedCompletionItemKinds); + LSPList.items.push_back(std::move(C)); + } + return Reply(std::move(LSPList)); + }, + std::move(Reply))); } void ClangdLSPServer::onSignatureHelp(const TextDocumentPositionParams &Params, Index: clangd/CMakeLists.txt =================================================================== --- clangd/CMakeLists.txt +++ clangd/CMakeLists.txt @@ -13,6 +13,7 @@ ClangdLSPServer.cpp ClangdServer.cpp ClangdUnit.cpp + CodeActions.cpp CodeComplete.cpp CodeCompletionStrings.cpp Compiler.cpp
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits