[clang-tools-extra] r352494 - [clangd] Interfaces for writing code tweaks
Ilya Biryukov via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 30 01:41:58 PST 2019
Should be fixed by r352612, let me know if it pops up again.
This was a use after move, probably gcc and clang prefer different
evaluation order, hence the difference in behaviours.
On Wed, Jan 30, 2019 at 12:12 PM Ilya Biryukov <ibiryukov at google.com> wrote:
> Hi Mikael,
>
> I suspect an undefined value creeps in for protocol capabilities, given
> that we have responses in different formats.
> I'll investigate, thanks for the report!
>
> On Wed, Jan 30, 2019 at 12:05 PM Mikael Holmén <mikael.holmen at ericsson.com>
> wrote:
>
>> Hi Ilya,
>>
>> I've no idea why, but when I compile this commit with gcc (7.4.0) the
>> test fixits-command.test fails, and the output from clangd is rather
>> different from when I compile it with clang (3.6.0).
>>
>> So I run
>>
>> build-all/bin/clangd -lit-test <
>>
>> /data/repo/master/llvm-master/tools/clang/tools/extra/test/clangd/fixits-command.test
>>
>> and the first diff that I think is significant is that with gcc it says
>>
>> E[09:44:29.470] error while getting semantic code actions: -32602:
>> trying to get AST for non-added document
>>
>> Before this patch the output from gcc and clang built clangd is identical.
>>
>> Attaching logs with gcc and clang.
>>
>> Any idea what the problem is?
>>
>> Regards,
>> Mikael
>>
>> On 1/29/19 3:17 PM, Ilya Biryukov via cfe-commits wrote:
>> > Author: ibiryukov
>> > Date: Tue Jan 29 06:17:36 2019
>> > New Revision: 352494
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=352494&view=rev
>> > Log:
>> > [clangd] Interfaces for writing code tweaks
>> >
>> > Summary:
>> > The code tweaks are an implementation of mini-refactorings exposed
>> > via the LSP code actions. They 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 from the client.
>> >
>> > - 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.
>> >
>> > One unfortunate consequence of this change is increased latency of
>> > processing the textDocument/codeAction requests, which now wait for an
>> > AST. However, we cannot avoid this with what we have available in the
>> LSP
>> > today.
>> >
>> > Reviewers: kadircet, ioeric, hokein, sammccall
>> >
>> > Reviewed By: sammccall
>> >
>> > Subscribers: mgrang, mgorny, MaskRay, jkorous, arphaman, cfe-commits
>> >
>> > Differential Revision: https://reviews.llvm.org/D56267
>> >
>> > Added:
>> > clang-tools-extra/trunk/clangd/refactor/
>> > clang-tools-extra/trunk/clangd/refactor/Tweak.cpp
>> > clang-tools-extra/trunk/clangd/refactor/Tweak.h
>> > clang-tools-extra/trunk/clangd/refactor/tweaks/
>> > clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt
>> > clang-tools-extra/trunk/clangd/refactor/tweaks/Dummy.cpp
>> > Modified:
>> > clang-tools-extra/trunk/clangd/CMakeLists.txt
>> > clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
>> > clang-tools-extra/trunk/clangd/ClangdServer.cpp
>> > clang-tools-extra/trunk/clangd/ClangdServer.h
>> > clang-tools-extra/trunk/clangd/Protocol.cpp
>> > clang-tools-extra/trunk/clangd/Protocol.h
>> > clang-tools-extra/trunk/clangd/SourceCode.cpp
>> > clang-tools-extra/trunk/clangd/SourceCode.h
>> > clang-tools-extra/trunk/clangd/tool/CMakeLists.txt
>> > clang-tools-extra/trunk/test/clangd/fixits-command.test
>> > clang-tools-extra/trunk/test/clangd/initialize-params.test
>> >
>> > Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
>> > URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=352494&r1=352493&r2=352494&view=diff
>> >
>> ==============================================================================
>> > --- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
>> > +++ clang-tools-extra/trunk/clangd/CMakeLists.txt Tue Jan 29 06:17:36
>> 2019
>> > @@ -71,6 +71,8 @@ add_clang_library(clangDaemon
>> > index/dex/PostingList.cpp
>> > index/dex/Trigram.cpp
>> >
>> > + refactor/Tweak.cpp
>> > +
>> > LINK_LIBS
>> > clangAST
>> > clangASTMatchers
>> > @@ -108,6 +110,7 @@ add_clang_library(clangDaemon
>> > ${CLANGD_ATOMIC_LIB}
>> > )
>> >
>> > +add_subdirectory(refactor/tweaks)
>> > if( LLVM_LIB_FUZZING_ENGINE OR LLVM_USE_SANITIZE_COVERAGE )
>> > add_subdirectory(fuzzer)
>> > endif()
>> >
>> > Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=352494&r1=352493&r2=352494&view=diff
>> >
>> ==============================================================================
>> > --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
>> > +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Tue Jan 29
>> 06:17:36 2019
>> > @@ -8,11 +8,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"
>> > @@ -30,6 +33,28 @@ public:
>> > }
>> > };
>> >
>> > +/// Transforms a tweak into a code action that would apply it if
>> executed.
>> > +/// EXPECTS: T.prepare() was called and returned true.
>> > +CodeAction toCodeAction(const ClangdServer::TweakRef &T, const
>> URIForFile &File,
>> > + Range Selection) {
>> > + CodeAction CA;
>> > + CA.title = T.Title;
>> > + CA.kind = CodeAction::REFACTOR_KIND;
>> > + // This tweak may have an expensive second stage, we only run it if
>> the user
>> > + // actually chooses it in the UI. We reply with a command that would
>> run the
>> > + // corresponding tweak.
>> > + // FIXME: for some tweaks, computing the edits is cheap and we could
>> send them
>> > + // directly.
>> > + CA.command.emplace();
>> > + CA.command->title = T.Title;
>> > + CA.command->command = Command::CLANGD_APPLY_TWEAK;
>> > + CA.command->tweakArgs.emplace();
>> > + CA.command->tweakArgs->file = File;
>> > + CA.command->tweakArgs->tweakID = T.ID;
>> > + CA.command->tweakArgs->selection = Selection;
>> > + return CA;
>> > +};
>> > +
>> > void adjustSymbolKinds(llvm::MutableArrayRef<DocumentSymbol> Syms,
>> > SymbolKindBitset Kinds) {
>> > for (auto &S : Syms) {
>> > @@ -338,7 +363,9 @@ void ClangdLSPServer::onInitialize(const
>> > {"referencesProvider", true},
>> > {"executeCommandProvider",
>> > llvm::json::Object{
>> > - {"commands",
>> {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND}},
>> > + {"commands",
>> > + {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND,
>> > + ExecuteCommandParams::CLANGD_APPLY_TWEAK}},
>> > }},
>> > }}}});
>> > }
>> > @@ -400,7 +427,7 @@ void ClangdLSPServer::onFileEvent(const
>> >
>> > void ClangdLSPServer::onCommand(const ExecuteCommandParams &Params,
>> > Callback<llvm::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
>> > @@ -420,6 +447,31 @@ void ClangdLSPServer::onCommand(const Ex
>> >
>> > Reply("Fix applied.");
>> > ApplyEdit(*Params.workspaceEdit);
>> > + } else if (Params.command ==
>> ExecuteCommandParams::CLANGD_APPLY_TWEAK &&
>> > + Params.tweakArgs) {
>> > + auto Code = DraftMgr.getDraft(Params.tweakArgs->file.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, URIForFile File,
>> > + std::string Code,
>> > + llvm::Expected<tooling::Replacements> R)
>> {
>> > + if (!R)
>> > + return Reply(R.takeError());
>> > +
>> > + WorkspaceEdit WE;
>> > + WE.changes.emplace();
>> > + (*WE.changes)[File.uri()] = replacementsToEdits(Code, *R);
>> > +
>> > + Reply("Fix applied.");
>> > + ApplyEdit(std::move(WE));
>> > + };
>> > + Server->applyTweak(Params.tweakArgs->file.file(),
>> > + Params.tweakArgs->selection,
>> Params.tweakArgs->tweakID,
>> > + Bind(Action, std::move(Reply),
>> Params.tweakArgs->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 +653,53 @@ static llvm::Optional<Command> asCommand
>> >
>> > void ClangdLSPServer::onCodeAction(const CodeActionParams &Params,
>> > Callback<llvm::json::Value> Reply)
>> {
>> > - auto Code = DraftMgr.getDraft(Params.textDocument.uri.file());
>> > + URIForFile File = Params.textDocument.uri;
>> > + auto Code = DraftMgr.getDraft(File.file());
>> > if (!Code)
>> > return Reply(llvm::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.file(), D)) {
>> > + FixIts.push_back(toCodeAction(F, Params.textDocument.uri));
>> > + FixIts.back().diagnostics = {D};
>> > }
>> > }
>> >
>> > - if (SupportsCodeAction)
>> > - Reply(llvm::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(llvm::json::Array(Commands));
>> > - }
>> > + // Now enumerate the semantic code actions.
>> > + auto ConsumeActions =
>> > + [this](decltype(Reply) Reply, URIForFile File, std::string Code,
>> > + Range Selection, std::vector<CodeAction> FixIts,
>> > + llvm::Expected<std::vector<ClangdServer::TweakRef>>
>> Tweaks) {
>> > + if (!Tweaks) {
>> > + auto Err = Tweaks.takeError();
>> > + if (Err.isA<CancelledError>())
>> > + return Reply(std::move(Err)); // do no logging, this is
>> expected.
>> > + elog("error while getting semantic code actions: {0}",
>> > + std::move(Err));
>> > + return Reply(llvm::json::Array(FixIts));
>> > + }
>> > +
>> > + std::vector<CodeAction> Actions = std::move(FixIts);
>> > + Actions.reserve(Actions.size() + Tweaks->size());
>> > + for (const auto &T : *Tweaks)
>> > + Actions.push_back(toCodeAction(T, File, Selection));
>> > +
>> > + if (SupportsCodeAction)
>> > + return Reply(llvm::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(llvm::json::Array(Commands));
>> > + };
>> > +
>> > + Server->enumerateTweaks(File.file(), Params.range,
>> > + Bind(ConsumeActions, std::move(Reply),
>> > + std::move(File), std::move(*Code),
>> Params.range,
>> > + std::move(FixIts)));
>> > }
>> >
>> > void ClangdLSPServer::onCompletion(const CompletionParams &Params,
>> >
>> > Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=352494&r1=352493&r2=352494&view=diff
>> >
>> ==============================================================================
>> > --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
>> > +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Jan 29 06:17:36
>> 2019
>> > @@ -16,11 +16,13 @@
>> > #include "XRefs.h"
>> > #include "index/FileIndex.h"
>> > #include "index/Merge.h"
>> > +#include "refactor/Tweak.h"
>> > #include "clang/Format/Format.h"
>> > #include "clang/Frontend/CompilerInstance.h"
>> > #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 +30,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 <future>
>> > +#include <memory>
>> > #include <mutex>
>> >
>> > namespace clang {
>> > @@ -325,6 +329,56 @@ void ClangdServer::rename(PathRef File,
>> > "Rename", File, Bind(Action, File.str(), NewName.str(),
>> std::move(CB)));
>> > }
>> >
>> > +void ClangdServer::enumerateTweaks(PathRef File, Range Sel,
>> > + Callback<std::vector<TweakRef>> CB)
>> {
>> > + auto Action = [Sel](decltype(CB) CB, std::string File,
>> > + Expected<InputsAndAST> InpAST) {
>> > + if (!InpAST)
>> > + return CB(InpAST.takeError());
>> > +
>> > + auto &AST = InpAST->AST;
>> > + auto CursorLoc = sourceLocationInMainFile(
>> > + AST.getASTContext().getSourceManager(), Sel.start);
>> > + if (!CursorLoc)
>> > + return CB(CursorLoc.takeError());
>> > + Tweak::Selection Inputs = {InpAST->Inputs.Contents, InpAST->AST,
>> > + *CursorLoc};
>> > +
>> > + std::vector<TweakRef> Res;
>> > + for (auto &T : prepareTweaks(Inputs))
>> > + Res.push_back({T->id(), T->title()});
>> > + CB(std::move(Res));
>> > + };
>> > +
>> > + WorkScheduler.runWithAST("EnumerateTweaks", File,
>> > + Bind(Action, std::move(CB), File.str()));
>> > +}
>> > +
>> > +void ClangdServer::applyTweak(PathRef File, Range Sel, TweakID ID,
>> > + Callback<tooling::Replacements> CB) {
>> > + auto Action = [ID, Sel](decltype(CB) CB, std::string File,
>> > + Expected<InputsAndAST> InpAST) {
>> > + if (!InpAST)
>> > + return CB(InpAST.takeError());
>> > +
>> > + auto &AST = InpAST->AST;
>> > + auto CursorLoc = sourceLocationInMainFile(
>> > + AST.getASTContext().getSourceManager(), Sel.start);
>> > + if (!CursorLoc)
>> > + return CB(CursorLoc.takeError());
>> > + Tweak::Selection Inputs = {InpAST->Inputs.Contents, InpAST->AST,
>> > + *CursorLoc};
>> > +
>> > + auto A = prepareTweak(ID, Inputs);
>> > + if (!A)
>> > + return CB(A.takeError());
>> > + // FIXME: run formatter on top of resulting replacements.
>> > + return CB((*A)->apply(Inputs));
>> > + };
>> > + WorkScheduler.runWithAST("ApplyTweak", File,
>> > + Bind(Action, std::move(CB), File.str()));
>> > +}
>> > +
>> > void ClangdServer::dumpAST(PathRef File,
>> > llvm::unique_function<void(std::string)>
>> Callback) {
>> > auto Action = [](decltype(Callback) Callback,
>> >
>> > Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
>> > URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=352494&r1=352493&r2=352494&view=diff
>> >
>> ==============================================================================
>> > --- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
>> > +++ clang-tools-extra/trunk/clangd/ClangdServer.h Tue Jan 29 06:17:36
>> 2019
>> > @@ -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"
>> > @@ -211,6 +213,18 @@ public:
>> > void rename(PathRef File, Position Pos, llvm::StringRef NewName,
>> > Callback<std::vector<tooling::Replacement>> CB);
>> >
>> > + struct TweakRef {
>> > + TweakID ID; /// ID to pass for applyTweak.
>> > + std::string Title; /// A single-line message to show in the UI.
>> > + };
>> > + /// Enumerate the code tweaks available to the user at a specified
>> point.
>> > + void enumerateTweaks(PathRef File, Range Sel,
>> > + Callback<std::vector<TweakRef>> CB);
>> > +
>> > + /// Apply the code tweak with a specified \p ID.
>> > + void applyTweak(PathRef File, Range Sel, TweakID ID,
>> > + Callback<tooling::Replacements> 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.
>> >
>> > Modified: clang-tools-extra/trunk/clangd/Protocol.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=352494&r1=352493&r2=352494&view=diff
>> >
>> ==============================================================================
>> > --- clang-tools-extra/trunk/clangd/Protocol.cpp (original)
>> > +++ clang-tools-extra/trunk/clangd/Protocol.cpp Tue Jan 29 06:17:36 2019
>> > @@ -421,6 +421,9 @@ bool fromJSON(const llvm::json::Value &P
>> >
>> > const llvm::StringLiteral
>> ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND =
>> > "clangd.applyFix";
>> > +const llvm::StringLiteral ExecuteCommandParams::CLANGD_APPLY_TWEAK =
>> > + "clangd.applyTweak";
>> > +
>> > bool fromJSON(const llvm::json::Value &Params, ExecuteCommandParams
>> &R) {
>> > llvm::json::ObjectMapper O(Params);
>> > if (!O || !O.map("command", R.command))
>> > @@ -431,6 +434,8 @@ bool fromJSON(const llvm::json::Value &P
>> > return Args && Args->size() == 1 &&
>> > fromJSON(Args->front(), R.workspaceEdit);
>> > }
>> > + if (R.command == ExecuteCommandParams::CLANGD_APPLY_TWEAK)
>> > + return Args && Args->size() == 1 && fromJSON(Args->front(),
>> R.tweakArgs);
>> > return false; // Unrecognized command.
>> > }
>> >
>> > @@ -497,10 +502,13 @@ llvm::json::Value toJSON(const Command &
>> > auto Cmd = llvm::json::Object{{"title", C.title}, {"command",
>> C.command}};
>> > if (C.workspaceEdit)
>> > Cmd["arguments"] = {*C.workspaceEdit};
>> > + if (C.tweakArgs)
>> > + Cmd["arguments"] = {*C.tweakArgs};
>> > 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}};
>> > @@ -544,6 +552,17 @@ llvm::json::Value toJSON(const Workspace
>> > return llvm::json::Object{{"changes", std::move(FileChanges)}};
>> > }
>> >
>> > +bool fromJSON(const llvm::json::Value &Params, TweakArgs &A) {
>> > + llvm::json::ObjectMapper O(Params);
>> > + return O && O.map("file", A.file) && O.map("selection", A.selection)
>> &&
>> > + O.map("tweakID", A.tweakID);
>> > +}
>> > +
>> > +llvm::json::Value toJSON(const TweakArgs &A) {
>> > + return llvm::json::Object{
>> > + {"tweakID", A.tweakID}, {"selection", A.selection}, {"file",
>> A.file}};
>> > +}
>> > +
>> > llvm::json::Value toJSON(const ApplyWorkspaceEditParams &Params) {
>> > return llvm::json::Object{{"edit", Params.edit}};
>> > }
>> >
>> > Modified: clang-tools-extra/trunk/clangd/Protocol.h
>> > URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=352494&r1=352493&r2=352494&view=diff
>> >
>> ==============================================================================
>> > --- clang-tools-extra/trunk/clangd/Protocol.h (original)
>> > +++ clang-tools-extra/trunk/clangd/Protocol.h Tue Jan 29 06:17:36 2019
>> > @@ -631,6 +631,21 @@ struct WorkspaceEdit {
>> > bool fromJSON(const llvm::json::Value &, WorkspaceEdit &);
>> > llvm::json::Value toJSON(const WorkspaceEdit &WE);
>> >
>> > +/// 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
>> > +/// tweak.
>> > +struct TweakArgs {
>> > + /// A file provided by the client on a textDocument/codeAction
>> request.
>> > + URIForFile file;
>> > + /// A selection provided by the client on a textDocument/codeAction
>> request.
>> > + Range selection;
>> > + /// ID of the tweak that should be executed. Corresponds to
>> Tweak::id().
>> > + std::string tweakID;
>> > +};
>> > +bool fromJSON(const llvm::json::Value &, TweakArgs &);
>> > +llvm::json::Value toJSON(const TweakArgs &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
>> > @@ -642,12 +657,15 @@ llvm::json::Value toJSON(const Workspace
>> > 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 TweakArgs as argument.
>> > + const static llvm::StringLiteral CLANGD_APPLY_TWEAK;
>> >
>> > /// The command identifier, e.g. CLANGD_APPLY_FIX_COMMAND
>> > std::string command;
>> >
>> > // Arguments
>> > llvm::Optional<WorkspaceEdit> workspaceEdit;
>> > + llvm::Optional<TweakArgs> tweakArgs;
>> > };
>> > bool fromJSON(const llvm::json::Value &, ExecuteCommandParams &);
>> >
>> > @@ -669,6 +687,7 @@ struct CodeAction {
>> > /// 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;
>> >
>> > Modified: clang-tools-extra/trunk/clangd/SourceCode.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.cpp?rev=352494&r1=352493&r2=352494&view=diff
>> >
>> ==============================================================================
>> > --- clang-tools-extra/trunk/clangd/SourceCode.cpp (original)
>> > +++ clang-tools-extra/trunk/clangd/SourceCode.cpp Tue Jan 29 06:17:36
>> 2019
>> > @@ -141,6 +141,16 @@ Position sourceLocToPosition(const Sourc
>> > return P;
>> > }
>> >
>> > +llvm::Expected<SourceLocation> sourceLocationInMainFile(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());
>> >
>> > Modified: clang-tools-extra/trunk/clangd/SourceCode.h
>> > URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.h?rev=352494&r1=352493&r2=352494&view=diff
>> >
>> ==============================================================================
>> > --- clang-tools-extra/trunk/clangd/SourceCode.h (original)
>> > +++ clang-tools-extra/trunk/clangd/SourceCode.h Tue Jan 29 06:17:36 2019
>> > @@ -56,6 +56,11 @@ Position offsetToPosition(llvm::StringRe
>> > /// 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<SourceLocation> sourceLocationInMainFile(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);
>> >
>> > Added: clang-tools-extra/trunk/clangd/refactor/Tweak.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/Tweak.cpp?rev=352494&view=auto
>> >
>> ==============================================================================
>> > --- clang-tools-extra/trunk/clangd/refactor/Tweak.cpp (added)
>> > +++ clang-tools-extra/trunk/clangd/refactor/Tweak.cpp Tue Jan 29
>> 06:17:36 2019
>> > @@ -0,0 +1,74 @@
>> > +//===--- Tweak.cpp -----------------------------------------------*-
>> C++-*-===//
>> > +//
>> > +// Part of the LLVM Project, under the Apache License v2.0 with LLVM
>> Exceptions.
>> > +// See https://llvm.org/LICENSE.txt for license information.
>> > +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
>> > +//
>> >
>> +//===----------------------------------------------------------------------===//
>> > +#include "Tweak.h"
>> > +#include "Logger.h"
>> > +#include "llvm/ADT/STLExtras.h"
>> > +#include "llvm/ADT/StringMap.h"
>> > +#include "llvm/Support/Error.h"
>> > +#include "llvm/Support/Registry.h"
>> > +#include <functional>
>> > +#include <memory>
>> > +
>> > +LLVM_INSTANTIATE_REGISTRY(llvm::Registry<clang::clangd::Tweak>);
>> > +
>> > +namespace clang {
>> > +namespace clangd {
>> > +
>> > +/// A handy typedef to save some typing.
>> > +typedef llvm::Registry<Tweak> TweakRegistry;
>> > +
>> > +namespace {
>> > +/// Asserts invariants on TweakRegistry. No-op with assertion disabled.
>> > +void validateRegistry() {
>> > +#ifndef NDEBUG
>> > + llvm::StringSet<> Seen;
>> > + for (const auto &E : TweakRegistry::entries()) {
>> > + // REGISTER_TWEAK ensures E.getName() is equal to the tweak class
>> name.
>> > + // We check that id() matches it.
>> > + assert(E.instantiate()->id() == E.getName() &&
>> > + "id should be equal to class name");
>> > + assert(Seen.try_emplace(E.getName()).second && "duplicate check
>> id");
>> > + }
>> > +#endif
>> > +}
>> > +} // namespace
>> > +
>> > +std::vector<std::unique_ptr<Tweak>> prepareTweaks(const
>> Tweak::Selection &S) {
>> > + validateRegistry();
>> > +
>> > + std::vector<std::unique_ptr<Tweak>> Available;
>> > + for (const auto &E : TweakRegistry::entries()) {
>> > + std::unique_ptr<Tweak> T = E.instantiate();
>> > + if (!T->prepare(S))
>> > + continue;
>> > + Available.push_back(std::move(T));
>> > + }
>> > + // Ensure deterministic order of the results.
>> > + llvm::sort(Available,
>> > + [](const std::unique_ptr<Tweak> &L,
>> > + const std::unique_ptr<Tweak> &R) { return L->id() <
>> R->id(); });
>> > + return Available;
>> > +}
>> > +
>> > +llvm::Expected<std::unique_ptr<Tweak>> prepareTweak(TweakID ID,
>> > + const
>> Tweak::Selection &S) {
>> > + auto It = llvm::find_if(
>> > + TweakRegistry::entries(),
>> > + [ID](const TweakRegistry::entry &E) { return E.getName() == ID;
>> });
>> > + if (It == TweakRegistry::end())
>> > + return llvm::createStringError(llvm::inconvertibleErrorCode(),
>> > + "id of the tweak is invalid");
>> > + std::unique_ptr<Tweak> T = It->instantiate();
>> > + if (!T->prepare(S))
>> > + return llvm::createStringError(llvm::inconvertibleErrorCode(),
>> > + "failed to prepare() a check");
>> > + return T;
>> > +}
>> > +
>> > +} // namespace clangd
>> > +} // namespace clang
>> >
>> > Added: clang-tools-extra/trunk/clangd/refactor/Tweak.h
>> > URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/Tweak.h?rev=352494&view=auto
>> >
>> ==============================================================================
>> > --- clang-tools-extra/trunk/clangd/refactor/Tweak.h (added)
>> > +++ clang-tools-extra/trunk/clangd/refactor/Tweak.h Tue Jan 29 06:17:36
>> 2019
>> > @@ -0,0 +1,98 @@
>> > +//===--- Tweak.h -------------------------------------------------*-
>> C++-*-===//
>> > +//
>> > +// Part of the LLVM Project, under the Apache License v2.0 with LLVM
>> Exceptions.
>> > +// See https://llvm.org/LICENSE.txt for license information.
>> > +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
>> > +//
>> >
>> +//===----------------------------------------------------------------------===//
>> > +// 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.
>> >
>> +//===----------------------------------------------------------------------===//
>> > +
>> > +#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.
>> > +/// To implement a new tweak use the following pattern in a .cpp file:
>> > +/// class MyTweak : public Tweak {
>> > +/// public:
>> > +/// TweakID id() const override final; // definition provided by
>> > +/// // REGISTER_TWEAK.
>> > +/// // implement other methods here.
>> > +/// };
>> > +/// REGISTER_TWEAK(MyTweak);
>> > +class Tweak {
>> > +public:
>> > + /// Input to prepare and apply tweaks.
>> > + struct Selection {
>> > + /// 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, it is always equal to the name of the
>> class
>> > + /// defining the Tweak. Definition is provided automatically by
>> > + /// REGISTER_TWEAK.
>> > + virtual TweakID id() const = 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<tooling::Replacements> 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;
>> > +};
>> > +
>> > +// All tweaks must be registered in the .cpp file next to their
>> definition.
>> > +#define REGISTER_TWEAK(Subclass)
>> \
>> > + ::llvm::Registry<::clang::clangd::Tweak>::Add<Subclass>
>> \
>> > + TweakRegistrationFor##Subclass(#Subclass, /*Description=*/"");
>> \
>> > + ::clang::clangd::TweakID Subclass::id() const {
>> \
>> > + return llvm::StringLiteral(#Subclass);
>> \
>> > + }
>> > +
>> > +/// Calls prepare() on all tweaks, returning those that can run on the
>> > +/// selection.
>> > +std::vector<std::unique_ptr<Tweak>> prepareTweaks(const
>> Tweak::Selection &S);
>> > +
>> > +// Calls prepare() on the tweak with a given ID.
>> > +// If prepare() returns false, returns an error.
>> > +// If prepare() returns true, returns the corresponding tweak.
>> > +llvm::Expected<std::unique_ptr<Tweak>> prepareTweak(TweakID ID,
>> > + const
>> Tweak::Selection &S);
>> > +
>> > +} // namespace clangd
>> > +} // namespace clang
>> > +
>> > +#endif
>> >
>> > Added: clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt
>> > URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt?rev=352494&view=auto
>> >
>> ==============================================================================
>> > --- clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt
>> (added)
>> > +++ clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt Tue
>> Jan 29 06:17:36 2019
>> > @@ -0,0 +1,13 @@
>> > +include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../..)
>> > +
>> > +# A target containing all code tweaks (i.e. mini-refactorings)
>> provided by
>> > +# clangd.
>> > +# Built as an object library to make sure linker does not remove global
>> > +# constructors that register individual tweaks in a global registry.
>> > +# To enable these tweaks in exectubales or shared libraries, add
>> > +# $<TARGET_OBJECTS:obj.clangDaemonTweaks> to a list of sources, see
>> > +# clangd/tool/CMakeLists.txt for an example.
>> > +add_clang_library(clangDaemonTweaks OBJECT
>> > + Dummy.cpp # FIXME: to avoid CMake errors due to empty inputs, remove
>> when a
>> > + # first tweak lands.
>> > + )
>> >
>> > Added: clang-tools-extra/trunk/clangd/refactor/tweaks/Dummy.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/tweaks/Dummy.cpp?rev=352494&view=auto
>> >
>> ==============================================================================
>> > --- clang-tools-extra/trunk/clangd/refactor/tweaks/Dummy.cpp (added)
>> > +++ clang-tools-extra/trunk/clangd/refactor/tweaks/Dummy.cpp Tue Jan 29
>> 06:17:36 2019
>> > @@ -0,0 +1,9 @@
>> > +//===--- Dummy.cpp -----------------------------------------------*-
>> C++-*-===//
>> > +//
>> > +// The LLVM Compiler Infrastructure
>> > +//
>> > +// This file is distributed under the University of Illinois Open
>> Source
>> > +// License. See LICENSE.TXT for details.
>> > +//
>> >
>> +//===----------------------------------------------------------------------===//
>> > +// Does nothing, only here to avoid cmake errors for empty libraries.
>> > \ No newline at end of file
>> >
>> > Modified: clang-tools-extra/trunk/clangd/tool/CMakeLists.txt
>> > URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/CMakeLists.txt?rev=352494&r1=352493&r2=352494&view=diff
>> >
>> ==============================================================================
>> > --- clang-tools-extra/trunk/clangd/tool/CMakeLists.txt (original)
>> > +++ clang-tools-extra/trunk/clangd/tool/CMakeLists.txt Tue Jan 29
>> 06:17:36 2019
>> > @@ -3,6 +3,7 @@ include_directories(${CMAKE_CURRENT_BINA
>> >
>> > add_clang_tool(clangd
>> > ClangdMain.cpp
>> > + $<TARGET_OBJECTS:obj.clangDaemonTweaks>
>> > )
>> >
>> > set(LLVM_LINK_COMPONENTS
>> >
>> > Modified: clang-tools-extra/trunk/test/clangd/fixits-command.test
>> > URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/fixits-command.test?rev=352494&r1=352493&r2=352494&view=diff
>> >
>> ==============================================================================
>> > --- clang-tools-extra/trunk/test/clangd/fixits-command.test (original)
>> > +++ clang-tools-extra/trunk/test/clangd/fixits-command.test Tue Jan 29
>> 06:17:36 2019
>> > @@ -23,7 +23,7 @@
>> > # CHECK-NEXT: "uri": "file://{{.*}}/foo.c"
>> > # CHECK-NEXT: }
>> > ---
>> >
>> -{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start":
>> {"line": 0, "character": 32}, "end": {"line": 0, "character":
>> 37}},"severity":2,"message":"Using the result of an assignment as a
>> condition without parentheses"}]}}}
>> >
>> +{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":0,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start":
>> {"line": 0, "character": 32}, "end": {"line": 0, "character":
>> 37}},"severity":2,"message":"Using the result of an assignment as a
>> condition without parentheses"}]}}}
>> > # CHECK: "id": 2,
>> > # CHECK-NEXT: "jsonrpc": "2.0",
>> > # CHECK-NEXT: "result": [
>> > @@ -92,7 +92,7 @@
>> > # CHECK-NEXT: }
>> > # CHECK-NEXT: ]
>> > ---
>> >
>> -{"jsonrpc":"2.0","id":3,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start":
>> {"line": 0, "character": 32}, "end": {"line": 0, "character":
>> 37}},"severity":2,"message":"Using the result of an assignment as a
>> condition without parentheses"}]}}}
>> >
>> +{"jsonrpc":"2.0","id":3,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":0,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start":
>> {"line": 0, "character": 32}, "end": {"line": 0, "character":
>> 37}},"severity":2,"message":"Using the result of an assignment as a
>> condition without parentheses"}]}}}
>> > # Make sure unused "code" and "source" fields ignored gracefully
>> > # CHECK: "id": 3,
>> > # CHECK-NEXT: "jsonrpc": "2.0",
>> >
>> > Modified: clang-tools-extra/trunk/test/clangd/initialize-params.test
>> > URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/initialize-params.test?rev=352494&r1=352493&r2=352494&view=diff
>> >
>> ==============================================================================
>> > --- clang-tools-extra/trunk/test/clangd/initialize-params.test
>> (original)
>> > +++ clang-tools-extra/trunk/test/clangd/initialize-params.test Tue Jan
>> 29 06:17:36 2019
>> > @@ -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.applyTweak"
>> > # CHECK-NEXT: ]
>> > # CHECK-NEXT: },
>> > # CHECK-NEXT: "hoverProvider": true,
>> >
>> >
>> > _______________________________________________
>> > cfe-commits mailing list
>> > cfe-commits at lists.llvm.org
>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>> >
>>
>
>
> --
> Regards,
> Ilya Biryukov
>
--
Regards,
Ilya Biryukov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190130/11cc2bcd/attachment-0001.html>
More information about the cfe-commits
mailing list