[clangd] Interfaces for writing code actions

[ilya-biryukov]

Summary

The code actions can be run in two stages:

• Stage 1. Decides whether the action is available to the user and collects all the information required to finish the action. Should be cheap, since this will run over all the actions known to clangd on each textDocument/codeAction request.
• Stage 2. Uses information from stage 1 to produce the actual edits that the code action should perform. This stage can be expensive and will only run if the user chooses to perform the specified action in the UI.

Trivial actions can also produce results in stage 1 to avoid an extra
round-trip and read of the AST to reconstruct the action on the server.

[ilya-biryukov]

That's the result of me playing around with writing a small code action for clangd.
It's not final yet, e.g. missing tests and example actions, but still wanted to drop the change here to make sure it's not lost, I believe we'll need something like this in the near future anyway.

[ilya-biryukov]

• Put more AST-centric information into ActionInputs
• Restructure and refactor the code

[ilya-biryukov]

• Remove 'using namespace llvm'

[ilya-biryukov]

I've added a few sample actions, please take a look.

The major thing that's missing from the design is how we can define an interface for actions to get the nodes they interested in from the AST without doing an AST traversal in each of the actions separately. I have a few options in mind, will be happy to explore more of this.
The other major problem that I've run into while playing around with the action in VSCode is increased latency: my suspicion is that the codeAction requests are not getting cancelled, I'll confirm and file an issue against VSCode if that's the case.

[sammccall]

High level comments:

• this is an important concept and in hindsight it's amazing we didn't have it yet!
• I don't get a clear sense of the boundaries between "code action", "refactoring", and "action". There are also "prepared actions" and "instant actions". I think some combination of fewer concepts, clearer names, and more consistent usage would help a lot.
• it's pretty heavy on registries/factories/preparedX, maybe we can find a way to simplify
• many (most?) actions are going to do some "walk up AST looking for X", the interface could help make that easy

clangd/ClangdLSPServer.cpp

368

Seems a bit more direct to give each one its own command name? Maybe under a namespace like refactor/swapIfBranches

682

This seems like it could just be a helper function... what does it capture?
I think it might belong next to PreparedActions just like we have toLSPDiags in Diag.h - it's not ideal dependency-wise, but inlining everything into ClangdLSPServer seems dubious too. Happy to discuss offline...

688

This optimization seems inessential, and complicates the model a bit. Can we leave it out for now?

clangd/ClangdServer.h

222

Unclear what the second sentence means, I'd suggest dropping it. Most obvious (to me) interpretation is that this can't fail, which isn't true.
nit: "Previously precomputed" is redundant :-)

clangd/CodeActions.h

16 ↗

(On Diff #181321)

why is ActionProvider in refactor/ but CodeActions/ is here?

23 ↗

(On Diff #181321)

I'm not sure this indirection is necessary, would a StringMap<u_p<ActionProvider>> suffice?

Since it's not actually complicated and there's just one caller, I think knowing what prepareAll is doing at the callsite would be an improvement.

clangd/refactor/ActionProvider.h

67 ↗

(On Diff #181321)

I think we should consider (not necessarily adopt) designing the interface around walking up the AST node chain, rather than bolting it on later.

71 ↗

(On Diff #181321)

I'd prefer a string here, this will make the protocol easier to understand for human readers.

78 ↗

(On Diff #181321)

This is a concrete class that holds a type-erased unique_function with the actual logic. Similarly, the factories are just wrappers of unique_function.
This method of abstraction is harder (for me at least) to follow than expressing the commonality of the important objects (the refactorings) as inheritance. And more concretely, stack traces are going to be terrible :-)

The factory/prepared duality makes it hard to represent a refactoring as a single class, but let's have a think about this part.

87 ↗

(On Diff #181321)

Why is this dynamic and not a constructor param?

[sammccall]

To be a bit more concrete...

One approach which won't win any OO design points but might be quite easy to navigate: combine the ActionProvider and PreparedAction into one class.

class SwapIfBranches : public MiniRefactoringThing {
  SwapIfBranches(); // must have a default constructor.
  const char* id() const override { return "swapIfBranches"; } // must be constant for the subclass.
  bool prepare(const Inputs&) override; // may only be called once. Returns false if this operation doesn't apply.
  std::string title() override; // REQUIRES: prepare() returned true
  Expected<tooling::Replacements> apply() override; // REQUIRES: prepare() returned true
}

note that the "subclasses of a common interface, have a default constructor" fits exactly with the LLVM Registry pattern, if we want to move to that.
Otherwise, a StringMap<unique_ptr<MiniRefactoringThing>>

A more... complicated version could use something like CRTP to give a generic external interface while keeping the internal dataflow/signatures clean:

class SwapIfBranches : public MiniRefactoringThing<SwapIfBranches> {
  struct State { ... }; // completing forward declaration from CRTP base
  Optional<State> prepare(const Inputs&) const override;
  std::string getTitle(const State&) const;
  Expected<tooling::Replacements> apply(const State&) const override;
}
const char MiniRefactoringThing<SwapIfBranches>::ID[] = "swapIfBranches";

Similarly we could use Registry or a StringMap<unique_ptr<MiniRefactoringThingIface>> where the CRTP base implements the interface.

[sammccall]

Name ideas:

• "Refactoring" - I think the main drawback is the conflicts with potential global-refactorings that we do in clangd. I don't think the conflict with tooling/refactor is bad enough to avoid this. I also don't think "some of these don't look like refactoring to me" is a strong reason to avoid it.
• "Code action" - I think the partial overlap with the protocol concept is confusing. (Worse, the protocol has two things called code action, that partially overlap). It's also not really a standard term outside LSP/VSCode, so we should be able to come up with something as good.
• "Action" - much too vague, I think
• "RefactoringAction" (or refactoring/Action, or refactoring::Action...) - not terrible but I think dominated by just "Refactoring" or "Refactor".
• "AutoEdit" or something equally descriptive - this is bland along the lines of Code Action, may be ok if it's sufficiently short and unique
• "Swizzle" or something equally unique-and-semantics-free - I'd be happy with this kind of alternative if we can't find something that clearly means what we want

[sammccall]

Offline discussion:

• CRTP doesn't buy anything. Simple inheritance is less clean than current function-passing version but probably more familiar.
• Tweak is a good name: short, evocative but not commonly used, works as a noun and a verb.
• Registry might be a good enough fit to use directly

Here's the result of my local, er, tweaking to ActionProvider.h.

//===--- Tweak.h -------------------------------------------------*- C++-*-===//
//
//                     The LLVM Compiler Infrastructure
//
// This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details.
//
//===----------------------------------------------------------------------===//
// A tweak is a small context-sensitive refactoring-like action that is
// triggered at a cursor location, examines the AST and produces a set of edits.
//
// It has two main pieces of logic:
//   - prepare() quickly checks whether the action is applicable at the cursor.
//     It must be cheap as all tweaks are prepared to list them for the user.
//   - apply() computes the actual edits.
//     This can be more expensive, only the chosen tweak is applied.
//===----------------------------------------------------------------------===//

#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_ACTIONPROVIDER_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_ACTIONPROVIDER_H

#include "ClangdUnit.h"
#include "Protocol.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringRef.h"
namespace clang {
namespace clangd {

// Abstract base for small context-sensitive refactoring actions.
class Tweak {
public:
  // Information about the location where the tweak was triggered.
  struct Selection {
    // The path of an active document the action was invoked in.
    llvm::StringRef File;
    // The text of the active document.
    llvm::StringRef Code;
    // Parsed AST of the active file.
    ParsedAST *
    // A location of the cursor in the editor.
    SourceLocation Cursor;
    // FIXME: full selection bounds.
    // FIXME: AST node under cursor.
    // FIXME: allow tweaks to specify matchers, and include match results.
    // FIXME: allow access to index and other source files.
  };

  // Tweaks must have a no-op default constructor.
  virtual ~Tweak() = default;

  virtual const char* id() = 0;
  // Determines whether this tweak can run at the selection.
  // This function may record information to be used later.
  // It should run as fast as possible, particularly when returning false.
  virtual bool prepare(const Selection &) = 0;

  // Generate the edits for this tweak.
  // REQUIRES: prepare() was called and returned true.
  virtual llvm::Expected<tooling::Replacements>
  apply(const Selection &) = 0;

  // Description of this tweak at the selected location.
  // e.g. "Out-line definition of toString()".
  // REQUIRES: prepare() was called and returned true.
  virtual std::string describe() const;
};

// All tweaks must be registered, next to their definition.
#define REGISTER_TWEAK(Subclass)                                               \
  static ::llvm::Registry<::clang::clangd::Tweak>::Add<Subclass>                 \
      TweakRegistrationFor##Subclass(Subclass{}.id(), #Subclass);

// Calls prepare() on all tweaks, returning those that can run on the selection.
std::vector<std::unique_ptr<Tweak>> prepareTweaks(const Tweak::Selection &);
// Calls prepare() on the tweak with a given ID.
// If prepare() returns false, returns an error.
Expected<std::unique_ptr<Tweak>> prepareTweak(StringRef ID,
                                              const Tweak::Selection &);

} // namespace clangd
} // namespace clang

#endif

[ilya-biryukov]

• Rename ActionProvider to Tweak
• Update an interface of Tweak
• ActionInputs -> Tweak::Selection
• Remove CodeAction.h, use tweak registry instead

[sammccall]

(Mostly just comments on Tweak, I think the comments on other parts still apply)

clangd/SourceCode.h

59

naming nits:

• this should mention "main-file"
• doesn't need to mention "position" as it's just the param type
• 'sourceLoc' seems an odd abbreviation

Maybe sourceLocationInMainFile?

(Current name is consistent with sourceLocToPosition but that name is bad too...)

clangd/refactor/Tweak.cpp

39

Can we drop this indirection and use the registry directly?

63

(nit: I'd make this operator< of Tweak? e.g. if priority is added, it should be private)

clangd/refactor/Tweak.h

79

nit: can we move this to the cpp file and out of the public interface?
(or keep the registry public, but drop prepareTweak[s] and just make callers do it)

94

should this return ErrorOr<unique_ptr<Tweak>> so we can treat both missing ID and prepare failed as errors with messages?

[ilya-biryukov]

• Build tweaks as an object library. To make sure linker does not optimize away global constructors.

[ilya-biryukov]

Haven't yet addressed all the comments, but switched to use the "object library" (i.e. a collection of .o files) to make sure linker does not optimize away global ctors required by registry.

clangd/refactor/Tweak.cpp

39

Sure, would mean linear time for prepareTweak, but we probably don't care.

[ilya-biryukov]

• Remove intermediate StringMap, use registry directly
• Rename codeAction to codeTweak in ClangdServer
• Rename a helper to get position to sourceLocationInMainFile
• Move the Registry typedef into the .cpp file
• Make toCodeAction a helper function
• Update comments

[ilya-biryukov]

clangd/ClangdLSPServer.cpp

368

I'm not sure what we're winning here.

Currently we have: command:{ name: "applyCodeAction", args: ["swap-if-branches", "…"] }
In the proposed approach we'd have: command: { name: "swap-if-branches", args: […] }

The latter would definitely mean more fiddling at least to communicate the list of supported actions. Both approaches don't seem to require any changes on the client side, even though they affect the messages we send over the protocol.

682

I've moved it into a helper function, moved away from using a lambda.
However, kept it in ClangdLSPServer.cpp, Diag.h does feel like the wrong place for it, it would be nice to find a better place for both functions.

clangd/refactor/Tweak.cpp

63

My personal preference would be to keep it in .cpp file to avoid complicating the public interface with a function and a friend declaration for prepareTweaks (let me know if I misinterpreted your proposal).
Let me know if you have strong preference wrt to this one

clangd/refactor/Tweak.h

79

Done.
I don't think that makes a big difference, though: hiding the typedef would hide the fact we're using the Registry, though. We still have REGISTER_TWEAK that mentions it in the public interface.

[ilya-biryukov]

This should be ready for another round of review. Let me know if I missed any of the older comments.
There are two more things that I'd like to do before this lands:

• go through the docs again and update/simplify them to make sure they're in sync with the new interface,
• publish the tests I have, they're not 100% ready yet, so I'm keeping the, local for now. This could be a separate change, though I don't think this should matter much if the rest is dealt with.

[sammccall]

This looks really good, everything else is pretty superficial I think.

I think we'll want to add one lit test for the whole two-step tweak workflow, as well as TestTU/annotation-based unit-tests for each tweak.
As this patch has no actual tweaks in it, we can work out the details when landing the first one.

clangd/ClangdLSPServer.cpp

37

comment: tweak must have been successfully prepared?

42

Comment should be more tentative (calculating edits *may* be expensive) since we don't have the immediate-edit optimization yet.
Decent place for a FIXME or comment regarding the optimization too.

clangd/ClangdServer.h

217

The returned Tweak object contains references to the AST, which may be dangling, so it's not actually safe to use.
It would be nice to allow callers to either directly apply a returned tweak (with progress saved) or to apply one "later" by name, but the way that transactions work in ClangdServer only the latter is really possible.

So I'd suggest this should return vector<pair<TweakID, string>>, which makes it clear(ish) how this relates to applyCodeTweak

217

enumerateTweaks?

221

tweak? It's a verb...

clangd/Protocol.cpp

425

tweak

clangd/Protocol.h

634

TweakArgs?

636

"tweakId" or just "id"

661

CLANGD_TWEAK_COMMAND

clangd/refactor/Tweak.h

41

Not convinced about this helper function.

• much of it is just a passthrough to struct initialization. I think the code calling it would be clearer if it was initialising the fields one-by one
• the only part that's not passthrough is already a function call with a clear name, calling it seems reasonable
• I'm not sure it makes sense to have the Range -> SourceLocation conversion in this file, but the Tweak -> CodeAction conversion outside this file (and not unit-testable). There's an argument to be make to keep this file independent of LSP protocol structs, but I think that argument applies equally to this function.

41

Expected<Selection>? Passing an invalid range is always an error I guess.

60

nit: one of my lessons from clang-tidy is that mapping between IDs and implementations is annoying.
Since IDs are 1:1 with classes, can we just require this to be the class name?

(If you wanted, I think you could adapt REGISTER_TWEAK so that it goes inside the class defn, and then it could provide the override of id() itself)

[sammccall]

clangd/ClangdServer.cpp

343

(action context?)

367

we should format::cleanUpAroundReplacements... fine to leave this as a FIXME

clangd/refactor/Tweak.h

46

Hmm, maybe we should drop this until we know how cross-file tweaks will work?

[ilya-biryukov]

• Update comments
• Return an ID+Title instead of a Tweak object
• Rename codeAction -> tweak everywhere
• Remove Tweak::Selection::File
• Return Expected<Selection> instead of Optional<Selection>
• Add a FIXME to apply the formatter on top of the actions

clangd/ClangdServer.cpp

367

Added a FIXME. We probably want a helper that does this, to reuse in tests and ClangdServer.

clangd/ClangdServer.h

221

That's too short for my taste, I went with applyTweak and it also feels helpful if we don't switch between a verb and a noun to keep clear a parallel between a Tweak returned by enumerateTweaks and a tweak applied in applyTweak.

Let me know if you feel strongly about this :-)

clangd/Protocol.cpp

425

Used applyTweak for symmetry with applyFix (and other reasons mentioned in the comments for ClangdServer::tweak)

clangd/refactor/Tweak.h

41

The reason I added it is to avoid duplication between in the test code and ClangdServer, which are the only two clients we have.
I expect this to be more useful when we add a way to traverse the subset of the AST in the checks

60

That would mean no two tweaks are allowed to have the same class name. This is probably fine, but somewhat contradicts C++, which would solve it with namespaces.
To be fair, there's a simple trick to grep for the id to find its class, so I'd keep as is.

If we choose to adapt REGISTER_TWEAK, that would mean we force everyone to put their tweaks only in .cpp files. That creates arbitrary restrictions on how one should write a check and I'm somewhat opposed to this. But happy to reconsider if you feel strongly about this.

[sammccall]

Can live with applyTweak etc, though it makes me sad.

clangd/refactor/Tweak.h

41

I understand. I think as things stand both callers would be clearer (if a couple of lines longer) without this helper.

What the API should be in the future - happy to talk about that then.

60

I don't care about the details (e.g. whether REGISTER_TWEAK sets the name, asserts the name, or none of the above).

I do care that we don't add a second ID for a class that's not equal to the class name. This is both a bad idea from first principles and from experience with clang-tidy.
If this were ever to become a real problem, I'm happy to include the namespace name in the ID.

[ilya-biryukov]

• Inline Tweak::Selection::create()
• Define Tweak::id() in REGISTER_TWEAK.

[ilya-biryukov]

clangd/refactor/Tweak.h

41

I'd keep this function, here is my reasoning:

1. There are 4 usages already (2 in this patch, 2 in the tests), keeping them in sync means an annoying (to my taste) amount of copy-paste.
2. This function is not completely trivial, as it makes a decision on what a CursorLoc is and documents it.
3. Making changes is simpler: e.g. if we add more fields to Tweak::Selection, we'd end up changing this function (possibly its signature too) and all of its callers. There's almost no room for error. If we remove it, one would have to find and update all usages by hand and make sure they're the same.

I did update the patch to inline it, but let me know if any of this convinces you.

60

Done, REGISTER_TWEAK now defines id.

Out of curiosity, what are the problems with clang-tidy checks that you mention? Finding a check by its name being hard is something that comes to mind. Anything else?

[ilya-biryukov]

clang-tools-extra/clangd/refactor/Tweak.cpp

27 ↗

(On Diff #182870)

Please note I added these assertions here.

It feels weird to traverse twice on every call to prepareTweaks, but that's the simplest option I could come up with.

[sammccall]

clang-tools-extra/clangd/Protocol.cpp

425 ↗

(On Diff #182870)

tweak or applyTweak

clang-tools-extra/clangd/Protocol.h

634 ↗

(On Diff #182870)

comment here?

clang-tools-extra/clangd/SourceCode.h

59 ↗

(On Diff #182870)

(can we add a unit test for this function?)

clang-tools-extra/clangd/refactor/Tweak.cpp

27 ↗

(On Diff #182870)

Looks fine - you could consider extracting to a separate function validateRegistry or so, but up to you

[ilya-biryukov]

• s/applyCodeAction/applyTweak
• Added a comment to TweakArgs
• Added a unit test for sourceLocationInMainFile
• Extract a 'validateRegistry' function
• Do not log CancelledError.

[ilya-biryukov]

clang-tools-extra/clangd/Protocol.cpp

425 ↗

(On Diff #182870)

Done. Thanks for spotting this, totally missed it.

clang-tools-extra/clangd/SourceCode.h

59 ↗

(On Diff #182870)

Added a test in ClangdUnitTests for a lack of a better place. SourceCodeTests only test non-source-manager-related things, so adding a dependency on SourceManager there seemed weird.

Happy to move them elsewhere, let me know what you think.

[ilya-biryukov]

• Update license headers in new files
• Add an empty cpp file to avoid cmake errors due to empty inputs
• clang-format
• Update the 'fixits-command.test' to unbreak it (the line number was larger than the number of lines in the file)

clangd/CMakeLists.txt

add_clang_library(clangDaemon
   index/SymbolCollector.cpp
   index/YAMLSerialization.cpp
 
   index/dex/Dex.cpp
   index/dex/Iterator.cpp
   index/dex/PostingList.cpp
   index/dex/Trigram.cpp
 
+  refactor/Tweak.cpp
+
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangDriver
   clangFormat
   clangFrontend
   clangIndex
add_clang_library(clangDaemon
   clangTooling
   clangToolingCore
   clangToolingInclusions
   clangToolingRefactor
   ${LLVM_PTHREAD_LIB}
   ${CLANGD_ATOMIC_LIB}
   )
 
+add_subdirectory(refactor/tweaks)
 if( LLVM_LIB_FUZZING_ENGINE OR LLVM_USE_SANITIZE_COVERAGE )
   add_subdirectory(fuzzer)
 endif()
 add_subdirectory(tool)
 add_subdirectory(indexer)
 add_subdirectory(index/dex/dexp)
 
 if (LLVM_INCLUDE_BENCHMARKS)
   add_subdirectory(benchmarks)
 endif()
 if ( CLANGD_BUILD_XPC )
   add_subdirectory(xpc)
 endif ()

clangd/ClangdLSPServer.cpp

 //===--- ClangdLSPServer.cpp - LSP server ------------------------*- 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 "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"
 
 namespace clang {
 namespace clangd {
 namespace {
 class IgnoreCompletionError : public llvm::ErrorInfo<CancelledError> {
 public:
   void log(llvm::raw_ostream &OS) const override {
     OS << "ignored auto-triggered completion, preceding char did not match";
   }
   std::error_code convertToErrorCode() const override {
     return std::make_error_code(std::errc::operation_canceled);
   }
 };
 
+/// 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) {
     S.kind = adjustKindToCapability(S.kind, Kinds);
     adjustSymbolKinds(S.children, Kinds);
   }
 }
 
Reply(llvm::json::Object{
             {"documentHighlightProvider", true},
             {"hoverProvider", true},
             {"renameProvider", true},
             {"documentSymbolProvider", true},
             {"workspaceSymbolProvider", true},
             {"referencesProvider", true},
             {"executeCommandProvider",
              llvm::json::Object{
-                 {"commands", {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND}},
+                 {"commands",
+                  {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND,
+                   ExecuteCommandParams::CLANGD_APPLY_TWEAK}},
              }},
         }}}});
 }
 
 void ClangdLSPServer::onShutdown(const ShutdownParams &Params,
                                  Callback<std::nullptr_t> Reply) {
   // Do essentially nothing, just say we're ready to exit.
   ShutdownRequestReceived = true;

 }
 
 void ClangdLSPServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
   Server->onFileEvent(Params);
 }
 
 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
     // would reply success/failure to the original RPC.
     call("workspace/applyEdit", Edit);
   };
   if (Params.command == ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND &&
       Params.workspaceEdit) {
     // The flow for "apply-fix" :
     // 1. We publish a diagnostic, including fixits
     // 2. The user clicks on the diagnostic, the editor asks us for code actions
     // 3. We send code actions, with the fixit embedded as context
     // 4. The user selects the fixit, the editor asks us to apply it
     // 5. We unwrap the changes and send them back to the editor
     // 6. The editor applies the changes (applyEdit), and sends us a reply (but
     // we ignore it)
 
     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
     // more commands are added, this will be here has a safe guard.
     Reply(llvm::make_error<LSPError>(
         llvm::formatv("Unsupported command \"{0}\".", Params.command).str(),
         ErrorCode::InvalidParams));
   }
static llvm::Optional<Command> asCommand(const CodeAction &Action) {
   Cmd.title = Action.title;
   if (Action.kind && *Action.kind == CodeAction::QUICKFIX_KIND)
     Cmd.title = "Apply fix: " + Cmd.title;
   return Cmd;
 }
 
 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};
     }
   }
 
+  // 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)
-    Reply(llvm::json::Array(Actions));
-  else {
+          return Reply(llvm::json::Array(Actions));
         std::vector<Command> Commands;
-    for (const auto &Action : Actions)
+        for (const auto &Action : Actions) {
           if (auto Command = asCommand(Action))
             Commands.push_back(std::move(*Command));
-    Reply(llvm::json::Array(Commands));
         }
+        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,
                                    Callback<CompletionList> Reply) {
   if (!shouldRunCompletion(Params))
     return Reply(llvm::make_error<IgnoreCompletionError>());
   Server->codeComplete(Params.textDocument.uri.file(), Params.position, CCOpts,
                        Bind(

clangd/ClangdServer.h

 #include "FSProvider.h"
 #include "Function.h"
 #include "GlobalCompilationDatabase.h"
 #include "Protocol.h"
 #include "TUScheduler.h"
 #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"
 #include <functional>
 #include <future>
 #include <string>
 #include <type_traits>
 #include <utility>
public:
   llvm::Expected<tooling::Replacements>
   formatOnType(StringRef Code, PathRef File, Position Pos);
 
   /// Rename all occurrences of the symbol at the \p Pos in \p File to
   /// \p NewName.
   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.
   void dumpAST(PathRef File, llvm::unique_function<void(std::string)> Callback);
   /// Called when an event occurs for a watched file in the workspace.
   void onFileEvent(const DidChangeWatchedFilesParams &Params);
 
   /// Get symbol info for given position.

clangd/ClangdServer.cpp

 #include "CodeComplete.h"
 #include "FindSymbols.h"
 #include "Headers.h"
 #include "SourceCode.h"
 #include "Trace.h"
 #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"
 #include "llvm/ADT/Optional.h"
 #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 {
 namespace clangd {
 namespace {
 
 class RefactoringResultCollector final
     : public tooling::RefactoringResultConsumer {
auto Action = [Pos](Path File, std::string NewName,
     }
     return CB(std::move(Replacements));
   };
 
   WorkScheduler.runWithAST(
       "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,
                    llvm::Expected<InputsAndAST> InpAST) {
     if (!InpAST) {
       llvm::consumeError(InpAST.takeError());
       return Callback("<no-ast>");
     }

clangd/Protocol.h

struct WorkspaceEdit {
   llvm::Optional<std::map<std::string, std::vector<TextEdit>>> changes;
 
   /// Note: "documentChanges" is not currently used because currently there is
   /// no support for versioned edits.
 };
 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
 /// handle maps to a certain llvm::Optional of some struct to contain its
 /// arguments. Different commands could reuse the same llvm::Optional as
 /// arguments but a command that needs different arguments would simply add a
 /// new llvm::Optional and not use any other ones. In practice this means only
 /// one argument type will be parsed and set.
 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 &);
 
 struct Command : public ExecuteCommandParams {
   std::string title;
 };
 llvm::json::Value toJSON(const Command &C);
 
 /// A code action represents a change that can be performed in code, e.g. to fix
 /// a problem or to refactor code.
 ///
 /// A CodeAction must set either `edit` and/or a `command`. If both are
 /// supplied, the `edit` is applied first, then the `command` is executed.
 struct CodeAction {
   /// A short, human-readable, title for this code action.
   std::string title;
 
   /// The kind of the code action.
   /// 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;
 
   /// The workspace edit this code action performs.
   llvm::Optional<WorkspaceEdit> edit;
 
   /// A command this code action executes. If a code action provides an edit

clangd/Protocol.cpp

 
 bool fromJSON(const llvm::json::Value &Params, WorkspaceEdit &R) {
   llvm::json::ObjectMapper O(Params);
   return O && O.map("changes", R.changes);
 }
 
 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))
     return false;
 
   auto Args = Params.getAsObject()->getArray("arguments");
   if (R.command == ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND) {
     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.
 }
 
 llvm::json::Value toJSON(const SymbolInformation &P) {
   return llvm::json::Object{
       {"name", P.name},
       {"kind", static_cast<int>(P.kind)},
       {"location", P.location},
bool fromJSON(const llvm::json::Value &Params, WorkspaceSymbolParams &R) {
   llvm::json::ObjectMapper O(Params);
   return O && O.map("query", R.query);
 }
 
 llvm::json::Value toJSON(const Command &C) {
   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}};
   if (CA.kind)
     CodeAction["kind"] = *CA.kind;
   if (CA.diagnostics)
     CodeAction["diagnostics"] = llvm::json::Array(*CA.diagnostics);
   if (CA.edit)
llvm::json::Value toJSON(const WorkspaceEdit &WE) {
   if (!WE.changes)
     return llvm::json::Object{};
   llvm::json::Object FileChanges;
   for (auto &Change : *WE.changes)
     FileChanges[Change.first] = llvm::json::Array(Change.second);
   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}};
 }
 
 bool fromJSON(const llvm::json::Value &Params, TextDocumentPositionParams &R) {
   llvm::json::ObjectMapper O(Params);
   return O && O.map("textDocument", R.textDocument) &&
          O.map("position", R.position);

clangd/SourceCode.h

 /// Turn an offset in Code into a [line, column] pair.
 /// The offset must be in range [0, Code.size()].
 Position offsetToPosition(llvm::StringRef Code, size_t Offset);
 
 /// Turn a SourceLocation into a [line, column] pair.
 /// 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);
 
 // Converts an offset to a clang line/column (1-based, columns are bytes).
 // The offset must be in range [0, Code.size()].
 // Prefer to use SourceManager if one is available.
 std::pair<size_t, size_t> offsetToClangLineColumn(llvm::StringRef Code,

clangd/SourceCode.cpp

Position sourceLocToPosition(const SourceManager &SM, SourceLocation Loc) {
   if (!Invalid) {
     auto ColumnInBytes = SM.getColumnNumber(FID, Offset) - 1;
     auto LineSoFar = Code.substr(Offset - ColumnInBytes, ColumnInBytes);
     P.character = lspLength(LineSoFar);
   }
   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());
   Position End = sourceLocToPosition(SM, R.getEnd());
 
   return {Begin, End};
 }
 

clangd/refactor/Tweak.h

+//===--- 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 *
+    /// 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

clangd/refactor/Tweak.cpp

+//===--- 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

clangd/refactor/tweaks/CMakeLists.txt

+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.
+  )

clangd/refactor/tweaks/Dummy.cpp

+//===--- 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

clangd/tool/CMakeLists.txt

 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
 include_directories(${CMAKE_CURRENT_BINARY_DIR}/..)
 
 add_clang_tool(clangd
   ClangdMain.cpp
+  $<TARGET_OBJECTS:obj.clangDaemonTweaks>
   )
 
 set(LLVM_LINK_COMPONENTS
   support
   )
 
 set(CLANGD_XPC_LIBS "")
 if(CLANGD_BUILD_XPC)

test/clangd/fixits-command.test

 # CHECK-NEXT:          }
 # CHECK-NEXT:        },
 # CHECK-NEXT:        "severity": 2
 # CHECK-NEXT:      }
 # CHECK-NEXT:    ],
 # 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": [
 # CHECK-NEXT:    {
 # CHECK-NEXT:      "arguments": [
 # CHECK-NEXT:        {
 # CHECK-NEXT:          "changes": {
 # CHECK-NEXT:            "file://{{.*}}/foo.c": [

 # CHECK-NEXT:          }
 # CHECK-NEXT:        }
 # CHECK-NEXT:      ],
 # CHECK-NEXT:      "command": "clangd.applyFix",
 # CHECK-NEXT:      "title": "Apply fix: use '==' to turn this assignment into an equality comparison"
 # 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",
 # CHECK-NEXT:  "result": [
 # CHECK-NEXT:    {
 # CHECK-NEXT:      "arguments": [
 # CHECK-NEXT:        {
 # CHECK-NEXT:          "changes": {

test/clangd/initialize-params.test

 # CHECK-NEXT:      "documentOnTypeFormattingProvider": {
 # CHECK-NEXT:        "firstTriggerCharacter": "}",
 # CHECK-NEXT:        "moreTriggerCharacter": []
 # CHECK-NEXT:      },
 # CHECK-NEXT:      "documentRangeFormattingProvider": true,
 # 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,
 # CHECK-NEXT:      "referencesProvider": true,
 # CHECK-NEXT:      "renameProvider": true,
 # CHECK-NEXT:      "signatureHelpProvider": {
 # CHECK-NEXT:        "triggerCharacters": [
 # CHECK-NEXT:          "(",