Index: clangd/CMakeLists.txt =================================================================== --- clangd/CMakeLists.txt +++ clangd/CMakeLists.txt @@ -70,6 +70,8 @@ index/dex/PostingList.cpp index/dex/Trigram.cpp + refactor/Tweak.cpp + LINK_LIBS clangAST clangASTMatchers Index: clangd/ClangdLSPServer.cpp =================================================================== --- clangd/ClangdLSPServer.cpp +++ clangd/ClangdLSPServer.cpp @@ -9,11 +9,14 @@ #include "ClangdLSPServer.h" #include "Diagnostics.h" +#include "Protocol.h" #include "SourceCode.h" #include "Trace.h" #include "URI.h" +#include "clang/Tooling/Core/Replacement.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/Support/Errc.h" +#include "llvm/Support/Error.h" #include "llvm/Support/FormatVariadic.h" #include "llvm/Support/Path.h" #include "llvm/Support/ScopedPrinter.h" @@ -338,7 +341,9 @@ {"referencesProvider", true}, {"executeCommandProvider", llvm::json::Object{ - {"commands", {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND}}, + {"commands", + {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND, + ExecuteCommandParams::CLANGD_APPLY_CODE_ACTION}}, }}, }}}}); } @@ -400,7 +405,7 @@ void ClangdLSPServer::onCommand(const ExecuteCommandParams &Params, Callback 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 @@ -420,6 +425,33 @@ Reply("Fix applied."); ApplyEdit(*Params.workspaceEdit); + } else if (Params.command == ExecuteCommandParams::CLANGD_APPLY_CODE_ACTION && + Params.codeActionArgs) { + auto Code = DraftMgr.getDraft(Params.codeActionArgs->file); + if (!Code) + return Reply(llvm::createStringError( + llvm::inconvertibleErrorCode(), + "trying to apply a code action for a non-added file")); + + auto Action = [ApplyEdit](decltype(Reply) Reply, std::string File, + std::string Code, + Expected R) { + if (!R) + return Reply(R.takeError()); + + WorkspaceEdit WE; + WE.changes.emplace(); + (*WE.changes)[URI::createFile(File).toString()] = + replacementsToEdits(Code, *R); + + Reply("Fix applied."); + ApplyEdit(std::move(WE)); + }; + Server->applyCodeAction( + Params.codeActionArgs->file, Params.codeActionArgs->selection, + Params.codeActionArgs->actionId, + Bind(Action, std::move(Reply), Params.codeActionArgs->file, + std::move(*Code))); } 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 @@ -601,28 +633,67 @@ void ClangdLSPServer::onCodeAction(const CodeActionParams &Params, Callback Reply) { - auto Code = DraftMgr.getDraft(Params.textDocument.uri.file()); + StringRef File = Params.textDocument.uri.file(); + auto Code = DraftMgr.getDraft(File); if (!Code) return Reply(llvm::make_error( "onCodeAction called for non-added file", ErrorCode::InvalidParams)); // We provide a code action for Fixes on the specified diagnostics. - std::vector Actions; + std::vector 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(llvm::json::Array(Actions)); - else { - std::vector Commands; - for (const auto &Action : Actions) - if (auto Command = asCommand(Action)) - Commands.push_back(std::move(*Command)); - Reply(llvm::json::Array(Commands)); - } + // Now enumerate the semantic code actions. + auto ConsumeActions = + [this](decltype(Reply) Reply, std::string File, std::string Code, Range R, + std::vector FixIts, + llvm::Expected>> Tweaks) { + if (!Tweaks) { + elog("error while getting semantic code actions: {0}", + Tweaks.takeError()); + return Reply(llvm::json::Array(FixIts)); + } + + std::vector Actions = std::move(FixIts); + auto toCodeAction = [&](std::unique_ptr T) -> CodeAction { + CodeAction CA; + CA.title = T->title(); + CA.kind = CodeAction::REFACTOR_KIND; + // 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 = T->title(); + CA.command->command = Command::CLANGD_APPLY_CODE_ACTION; + CA.command->codeActionArgs.emplace(); + CA.command->codeActionArgs->file = File; + CA.command->codeActionArgs->actionId = T->id(); + CA.command->codeActionArgs->selection = R; + return CA; + }; + + Actions.reserve(Actions.size() + Tweaks->size()); + for (auto &T : *Tweaks) + Actions.push_back(toCodeAction(std::move(T))); + + if (SupportsCodeAction) + return Reply(llvm::json::Array(Actions)); + std::vector Commands; + for (const auto &Action : Actions) { + if (auto Command = asCommand(Action)) + Commands.push_back(std::move(*Command)); + } + return Reply(llvm::json::Array(Commands)); + }; + + Server->enumerateCodeActions(File, Params.range, + Bind(ConsumeActions, std::move(Reply), + File.str(), std::move(*Code), Params.range, + std::move(FixIts))); } void ClangdLSPServer::onCompletion(const CompletionParams &Params, Index: clangd/ClangdServer.h =================================================================== --- clangd/ClangdServer.h +++ clangd/ClangdServer.h @@ -21,8 +21,10 @@ #include "index/Background.h" #include "index/FileIndex.h" #include "index/Index.h" +#include "refactor/Tweak.h" #include "clang/Tooling/CompilationDatabase.h" #include "clang/Tooling/Core/Replacement.h" +#include "llvm/ADT/FunctionExtras.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/ADT/Optional.h" #include "llvm/ADT/StringRef.h" @@ -202,6 +204,17 @@ void rename(PathRef File, Position Pos, llvm::StringRef NewName, Callback> 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>> CB); + + /// Apply the previously precomputed code action. This will always fully + /// execute the action. + void applyCodeAction(PathRef File, Range R, TweakID ID, + Callback 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. Index: clangd/ClangdServer.cpp =================================================================== --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -21,6 +21,7 @@ #include "clang/Frontend/CompilerInvocation.h" #include "clang/Lex/Preprocessor.h" #include "clang/Tooling/CompilationDatabase.h" +#include "clang/Tooling/Core/Replacement.h" #include "clang/Tooling/Refactoring/RefactoringResultConsumer.h" #include "clang/Tooling/Refactoring/Rename/RenamingAction.h" #include "llvm/ADT/ArrayRef.h" @@ -28,10 +29,12 @@ #include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Errc.h" +#include "llvm/Support/Error.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" #include "llvm/Support/raw_ostream.h" #include +#include #include namespace clang { @@ -322,6 +325,49 @@ "Rename", File, Bind(Action, File.str(), NewName.str(), std::move(CB))); } +void ClangdServer::enumerateCodeActions( + PathRef File, Range Selection, + Callback>> CB) { + auto Action = [Selection](decltype(CB) CB, std::string File, + Expected InpAST) { + if (!InpAST) + return CB(InpAST.takeError()); + + auto Inputs = Tweak::Selection::create(File, InpAST->Inputs.Contents, + InpAST->AST, Selection); + if (!Inputs) + return CB(llvm::createStringError(llvm::inconvertibleErrorCode(), + "could not create action context")); + CB(prepareTweaks(*Inputs)); + }; + + WorkScheduler.runWithAST("EnumerateCodeActions", File, + Bind(Action, std::move(CB), File.str())); +} + +void ClangdServer::applyCodeAction(PathRef File, Range R, TweakID ID, + Callback CB) { + auto Action = [ID, R](decltype(CB) CB, std::string File, + Expected InpAST) { + if (!InpAST) + return CB(InpAST.takeError()); + + auto Inputs = + Tweak::Selection::create(File, InpAST->Inputs.Contents, InpAST->AST, R); + if (!Inputs) + return CB(llvm::createStringError(llvm::inconvertibleErrorCode(), + "could not create action context")); + + auto A = prepareTweak(ID, *Inputs); + if (!A) + return CB(llvm::createStringError(llvm::inconvertibleErrorCode(), + "failed to recreate the code action")); + return CB(A->apply(*Inputs)); + }; + WorkScheduler.runWithAST("ApplyCodeAction", File, + Bind(Action, std::move(CB), File.str())); +} + void ClangdServer::dumpAST(PathRef File, llvm::unique_function Callback) { auto Action = [](decltype(Callback) Callback, 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; + std::string 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; + llvm::Optional codeActionArgs; }; bool fromJSON(const llvm::json::Value &, ExecuteCommandParams &); @@ -670,6 +681,7 @@ /// Used to filter code actions. llvm::Optional kind; const static llvm::StringLiteral QUICKFIX_KIND; + const static llvm::StringLiteral REFACTOR_KIND; /// The diagnostics that this code action resolves. llvm::Optional> diagnostics; Index: clangd/Protocol.cpp =================================================================== --- clangd/Protocol.cpp +++ clangd/Protocol.cpp @@ -422,6 +422,9 @@ const llvm::StringLiteral ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND = "clangd.applyFix"; +const llvm::StringLiteral ExecuteCommandParams::CLANGD_APPLY_CODE_ACTION = + "clangd.applyCodeAction"; + bool fromJSON(const llvm::json::Value &Params, ExecuteCommandParams &R) { llvm::json::ObjectMapper O(Params); if (!O || !O.map("command", R.command)) @@ -432,6 +435,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. } @@ -498,10 +504,13 @@ auto Cmd = llvm::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 llvm::StringLiteral CodeAction::QUICKFIX_KIND = "quickfix"; +const llvm::StringLiteral CodeAction::REFACTOR_KIND = "refactor"; llvm::json::Value toJSON(const CodeAction &CA) { auto CodeAction = llvm::json::Object{{"title", CA.title}}; @@ -545,6 +554,17 @@ return llvm::json::Object{{"changes", std::move(FileChanges)}}; } +bool fromJSON(const llvm::json::Value &Params, CodeActionArgs &A) { + llvm::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 llvm::json::Object{ + {"actionId", A.actionId}, {"selection", A.selection}, {"file", A.file}}; +} + llvm::json::Value toJSON(const ApplyWorkspaceEditParams &Params) { return llvm::json::Object{{"edit", Params.edit}}; } Index: clangd/SourceCode.h =================================================================== --- clangd/SourceCode.h +++ clangd/SourceCode.h @@ -55,6 +55,11 @@ /// FIXME: This should return an error if the location is invalid. Position sourceLocToPosition(const SourceManager &SM, SourceLocation Loc); +/// Return the file location, corresponding to \p P. Note that one should take +/// care to avoid comparing the result with expansion locations. +llvm::Expected positionToSourceLoc(const SourceManager &SM, + Position P); + // Converts a half-open clang source range to an LSP range. // Note that clang also uses closed source ranges, which this can't handle! Range halfOpenToRange(const SourceManager &SM, CharSourceRange R); Index: clangd/SourceCode.cpp =================================================================== --- clangd/SourceCode.cpp +++ clangd/SourceCode.cpp @@ -142,6 +142,16 @@ return P; } +llvm::Expected positionToSourceLoc(const SourceManager &SM, + Position P) { + llvm::StringRef Code = SM.getBuffer(SM.getMainFileID())->getBuffer(); + auto Offset = + positionToOffset(Code, P, /*AllowColumnBeyondLineLength=*/false); + if (!Offset) + return Offset.takeError(); + return SM.getLocForStartOfFile(SM.getMainFileID()).getLocWithOffset(*Offset); +} + Range halfOpenToRange(const SourceManager &SM, CharSourceRange R) { // Clang is 1-based, LSP uses 0-based indexes. Position Begin = sourceLocToPosition(SM, R.getBegin()); Index: clangd/refactor/Tweak.h =================================================================== --- /dev/null +++ clangd/refactor/Tweak.h @@ -0,0 +1,98 @@ +//===--- Tweak.h -------------------------------------------------*- C++-*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// Tweaks 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. +// 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_REFACTOR_ACTIONS_TWEAK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_TWEAK_H + +#include "ClangdUnit.h" +#include "Protocol.h" +#include "clang/Tooling/Core/Replacement.h" +#include "llvm/ADT/Optional.h" +#include "llvm/ADT/StringRef.h" +namespace clang { +namespace clangd { + +using TweakID = llvm::StringRef; + +/// An interface base for small context-sensitive refactoring actions. +class Tweak { +public: + /// Input to prepare and apply tweaks. + struct Selection { + static llvm::Optional create(llvm::StringRef File, + llvm::StringRef Code, + ParsedAST &AST, Range Selection); + + /// The path of an active document the action was invoked in. + llvm::StringRef File; + /// The text of the active document. + llvm::StringRef Code; + /// Parsed AST of the active file. + ParsedAST &AST; + /// A location of the cursor in the editor. + SourceLocation Cursor; + // FIXME: add selection when there are checks relying on it. + // 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. + }; + virtual ~Tweak() = default; + /// A unique id of the action. The convention is to + /// lower-case-with-dashes for the identifier. + virtual TweakID id() = 0; + /// 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 'apply'. + /// Returns true iff the action is available and apply() can be called on it. + virtual bool prepare(const Selection &Sel) = 0; + /// Run the second stage of the action that would produce the actual changes. + /// EXPECTS: prepare() was called and returned true. + virtual Expected apply(const Selection &Sel) = 0; + /// A one-line title of the action that should be shown to the users in the + /// UI. + /// EXPECTS: prepare() was called and returned true. + virtual std::string title() const = 0; +}; + +/// Contains all actions supported by clangd. +typedef llvm::Registry TweakRegistry; + +// All tweaks must be registered, next to their definition. +#define REGISTER_TWEAK(Subclass) \ + static ::clang::clangd::TweakRegistry::Add \ + TweakRegistrationFor##Subclass(Subclass{}.id(), #Subclass); + +/// Calls prepare() on all tweaks, returning those that can run on the +/// selection. +std::vector> +prepareTweaks(const Tweak::Selection &S); + +// Calls prepare() on the tweak with a given ID. +// If prepare() returns false, returns null. +// If prepare() returns true, return the corresponding tweak. +std::unique_ptr prepareTweak(TweakID ID, const Tweak::Selection &S); + +} // namespace clangd +} // namespace clang + +#endif Index: clangd/refactor/Tweak.cpp =================================================================== --- /dev/null +++ clangd/refactor/Tweak.cpp @@ -0,0 +1,82 @@ +//===--- Tweak.cpp --------------------------------------*- C++-*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +#include "Tweak.h" +#include "Logger.h" +#include "llvm/ADT/StringMap.h" +#include "llvm/Support/Registry.h" +#include +#include + +LLVM_INSTANTIATE_REGISTRY(clang::clangd::TweakRegistry); + +namespace clang { +namespace clangd { + +llvm::Optional Tweak::Selection::create(llvm::StringRef File, + llvm::StringRef Code, + ParsedAST &AST, + Range Selection) { + + auto &Ctx = AST.getASTContext(); + // Approximate the cursor position as the start of the selection. The LSP does + // not provide a separate field for this. + auto Cursor = positionToSourceLoc(Ctx.getSourceManager(), Selection.start); + if (!Cursor) { + llvm::consumeError(Cursor.takeError()); + return llvm::None; + } + return Tweak::Selection{File, Code, AST, *Cursor}; +} + +namespace { +const llvm::StringMap()>> & +tweaksRegistry() { + static llvm::StringMap()>> *TM = []() { + llvm::StringMap()>> TM; + for (auto E : llvm::Registry::entries()) { + assert(E.getName() == E.instantiate()->id() && + "id() and name in registry are different. Did you use " + "REGISTER_TWEAK used?"); + TM[E.getName()] = [E]() { return E.instantiate(); }; + } + return new auto(std::move(TM)); + }(); + return *TM; +} +} // namespace + +std::vector> prepareTweaks(const Tweak::Selection &S) { + std::vector> Available; + for (const auto &KV : tweaksRegistry()) { + std::unique_ptr T = KV.second(); + if (!T->prepare(S)) + continue; + Available.push_back(std::move(T)); + } + // Ensure deterministic order of the results. + llvm::sort(Available, + [](const std::unique_ptr &L, + const std::unique_ptr &R) { return L->id() < R->id(); }); + return Available; +} + +std::unique_ptr prepareTweak(TweakID ID, const Tweak::Selection &S) { + auto It = tweaksRegistry().find(ID); + if (It == tweaksRegistry().end()) { + elog("id of the tweak ({0}) is invalid", ID); + return nullptr; + } + std::unique_ptr T = It->second(); + if (!T->prepare(S)) + return nullptr; + return T; +} + +} // namespace clangd +} // namespace clang \ No newline at end of file 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,