[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:12:53 PST 2019


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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190130/39509fe2/attachment-0001.html>


More information about the cfe-commits mailing list