[clang-tools-extra] r352494 - [clangd] Interfaces for writing code tweaks
Mikael Holmén via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 30 01:53:48 PST 2019
On 1/30/19 10:41 AM, Ilya Biryukov wrote:
> 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.
Awesome! Now it passes also when I compile with gcc.
Thanks,
Mikael
>
> On Wed, Jan 30, 2019 at 12:12 PM Ilya Biryukov <ibiryukov at google.com
> <mailto: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 <mailto: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 <http://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 <mailto:cfe-commits at lists.llvm.org>
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
>
>
>
> --
> Regards,
> Ilya Biryukov
>
>
>
> --
> Regards,
> Ilya Biryukov
More information about the cfe-commits
mailing list