[clangd] Preserve diags between tweak enumeration and execution

[kadircet]

Summary

Depends on D98505.

[kadircet]

ping

[sammccall]

Sorry about taking forever to get around to this, I kept looking at it and not understanding it.

IIUC the idea is:

• tweaks depend on various data from the codeAction request
• but we only have the codeAction request when enumerating, not when executing
• so we jam all the data needed to recalculate the tweak into the command args
• now tweaks depend on codeactioncontext.diagnostics, so jam that in command args too

This definitely works and we can move forward with it but it raises some ideas I'd like to talk through...

---

Extending this pattern this way feels wasteful: while file/range/tweakID is always needed to recompute the tweak, diagnostics almost never are. This inefficiency probably isn't really important (though amusingly if you have N diagnostics overlapping a point where M code actions are available, the codeAction response will contain N*M diagnostics...)
And the advantage is, the logic to extract info out of diagnostics gets exactly reused between enumerate & apply, which is the Tweak design.

But it's worth considering the alternative. Fundamentally this is that enumerate extracts the info from diagnostics and passes it directly to apply, which is the Code Action design. In this world the tweak only sees the CodeActionContext when enumerating and can output some JSON data, and when applying it sees the JSON data instead.

The reason this is worth entertaining is the Code Action design supports multiple instances of the same tweak, where the Tweak design does not. To support three different "override abstract method X" actions, each needs to pass its own value of X as data from enumerate -> apply so that they end up doing different things. And if we had this mechanism, I think would feel like the natural way to handle diagnostics.

I think this probably looks something like

class Tweak {
  virtual bool prepare(const Selection&); // as now, if false then we can't use this tweak
  struct Case { std::string title; json::Object Data };
  vector<Case> cases(); // replaces title()
  Expected<Effect> apply(json::Object Data);
}
// enumeration = prepare() + cases()
// application = prepare() + apply()

Maybe it makes sense to drop prepare() and have tweaks factor out any common logic between cases & apply in whatever way suits them.

---

Anyway this is obviously yet another big yak-shave and this patch isn't the place to discuss it, but I wanted to brain dump a bit. Let's talk next week. Like I said we can go ahead with the approach in this patch, but I want to understand the long-term.

[sammccall]

Sorry I've lost my context - did we decide to move forward with this patch?

[kadircet]

Sorry I've lost my context - did we decide to move forward with this patch?

I don't think we've came to a conclusion, just decided to postpone until needed. I believe the cases design is really a good fit for making tweaks expose multiple code actions.

But we've actually missed one thing while discussing this patch. It actually allows data from clang::Diagnostic to be stashed into clangd::Diag for use later on. Modules can actually stash this info while the AST is being built, later on they can be retrieved directly from the ParsedAST::getDiagnostics(). But this creates the N*M problem again, and feels like a hack.
What we can do instead is during enumerateTweak, we can group data in diagnostics (making sure data stored in diagnostic is keyed by tweak::id) and pass an additional array of json objects in tweak::prepare. This will make the problem N+M again and make the data passing explicit.

[sammccall]

In D99540#2799159, @kadircet wrote:

Sorry I've lost my context - did we decide to move forward with this patch?

I don't think we've came to a conclusion, just decided to postpone until needed. I believe the cases design is really a good fit for making tweaks expose multiple code actions.

But we've actually missed one thing while discussing this patch. It actually allows data from clang::Diagnostic to be stashed into clangd::Diag for use later on. Modules can actually stash this info while the AST is being built, later on they can be retrieved directly from the ParsedAST::getDiagnostics(). But this creates the N*M problem again, and feels like a hack.
What we can do instead is during enumerateTweak, we can group data in diagnostics (making sure data stored in diagnostic is keyed by tweak::id) and pass an additional array of json objects in tweak::prepare. This will make the problem N+M again and make the data passing explicit.

Sorry, somehow I'm not following this (either the problem or the suggestion at all). Let's chat this week about it...

clang-tools-extra/clangd/ClangdLSPServer.cpp

 }
 
 const llvm::StringLiteral APPLY_FIX_COMMAND = "clangd.applyFix";
 const llvm::StringLiteral APPLY_TWEAK_COMMAND = "clangd.applyTweak";
 
 /// 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) {
+                        Range Selection, llvm::ArrayRef<Diagnostic> Diags) {
   CodeAction CA;
   CA.title = T.Title;
   CA.kind = T.Kind.str();
   // 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 = std::string(APPLY_TWEAK_COMMAND);
   TweakArgs Args;
   Args.file = File;
   Args.tweakID = T.ID;
   Args.selection = Selection;
+  Args.diagnostics = Diags.vec();
   CA.command->argument = std::move(Args);
   return CA;
 }
 
 void adjustSymbolKinds(llvm::MutableArrayRef<DocumentSymbol> Syms,
                        SymbolKindBitset Kinds) {
   for (auto &S : Syms) {
     S.kind = adjustKindToCapability(S.kind, Kinds);
auto Action = [this, Reply = std::move(Reply),
     for (const auto &It : R->ApplyEdits) {
       (*WE.changes)[URI::createFile(It.first()).toString()] =
           It.second.asTextEdits();
     }
     // ApplyEdit will take care of calling Reply().
     return applyEdit(std::move(WE), "Tweak applied.", std::move(Reply));
   };
   Server->applyTweak(Args.file.file(), Args.selection, Args.tweakID,
-                     std::move(Action));
+                     Args.diagnostics, std::move(Action));
 }
 
 void ClangdLSPServer::applyEdit(WorkspaceEdit WE, llvm::json::Value Success,
                                 Callback<llvm::json::Value> Reply) {
   ApplyWorkspaceEditParams Edit;
   Edit.edit = std::move(WE);
   ApplyWorkspaceEdit(
       Edit, [Reply = std::move(Reply), SuccessMessage = std::move(Success)](
for (const Diagnostic &D : Params.context.diagnostics) {
         FixIts.back().diagnostics = {D};
       }
     }
   }
 
   // Now enumerate the semantic code actions.
   auto ConsumeActions =
       [Reply = std::move(Reply), File, Selection = Params.range,
-       FixIts = std::move(FixIts), this](
+       Diags = Params.context.diagnostics, FixIts = std::move(FixIts), this](
           llvm::Expected<std::vector<ClangdServer::TweakRef>> Tweaks) mutable {
         if (!Tweaks)
           return Reply(Tweaks.takeError());
 
         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));
+          Actions.push_back(toCodeAction(T, File, Selection, Diags));
 
         // If there's exactly one quick-fix, call it "preferred".
         // We never consider refactorings etc as preferred.
         CodeAction *OnlyFix = nullptr;
         for (auto &Action : Actions) {
           if (Action.kind && *Action.kind == CodeAction::QUICKFIX_KIND) {
             if (OnlyFix) {
               OnlyFix->isPreferred = false;
auto ConsumeActions =
         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,
+      File.file(), Params.range, Params.context.diagnostics,
       [this, KindAllowed(std::move(KindAllowed))](const Tweak &T) {
         return Opts.TweakFilter(T) && KindAllowed(T.kind());
       },
       std::move(ConsumeActions));
 }
 
 void ClangdLSPServer::onCompletion(const CompletionParams &Params,
                                    Callback<CompletionList> Reply) {

clang-tools-extra/clangd/ClangdServer.h

 #include "refactor/Tweak.h"
 #include "support/Cancellation.h"
 #include "support/Function.h"
 #include "support/MemoryTree.h"
 #include "support/Path.h"
 #include "support/ThreadsafeFS.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 #include <functional>
 #include <memory>
 #include <string>
 #include <type_traits>
public:
   struct TweakRef {
     std::string ID;    /// ID to pass for applyTweak.
     std::string Title; /// A single-line message to show in the UI.
     llvm::StringLiteral Kind;
   };
   /// Enumerate the code tweaks available to the user at a specified point.
   /// Tweaks where Filter returns false will not be checked or included.
   void enumerateTweaks(PathRef File, Range Sel,
+                       llvm::ArrayRef<Diagnostic> Diags,
                        llvm::unique_function<bool(const Tweak &)> Filter,
                        Callback<std::vector<TweakRef>> CB);
 
   /// Apply the code tweak with a specified \p ID.
   void applyTweak(PathRef File, Range Sel, StringRef ID,
-                  Callback<Tweak::Effect> CB);
+                  llvm::ArrayRef<Diagnostic> Diags, Callback<Tweak::Effect> CB);
 
   /// Called when an event occurs for a watched file in the workspace.
   void onFileEvent(const DidChangeWatchedFilesParams &Params);
 
   /// Get symbol info for given position.
   /// Clangd extension - not part of official LSP.
   void symbolInfo(PathRef File, Position Pos,
                   Callback<std::vector<SymbolDetails>> CB);

clang-tools-extra/clangd/ClangdServer.cpp

auto Action = [File = File.str(), NewName = NewName.str(), Pos, Opts,
     return CB(*R);
   };
   WorkScheduler->runWithAST("Rename", File, std::move(Action));
 }
 
 // May generate several candidate selections, due to SelectionTree ambiguity.
 // vector of pointers because GCC doesn't like non-copyable Selection.
 static llvm::Expected<std::vector<std::unique_ptr<Tweak::Selection>>>
-tweakSelection(const Range &Sel, const InputsAndAST &AST) {
+tweakSelection(const Range &Sel, const InputsAndAST &AST,
+               llvm::ArrayRef<Diagnostic> Diags) {
   auto Begin = positionToOffset(AST.Inputs.Contents, Sel.start);
   if (!Begin)
     return Begin.takeError();
   auto End = positionToOffset(AST.Inputs.Contents, Sel.end);
   if (!End)
     return End.takeError();
   std::vector<std::unique_ptr<Tweak::Selection>> Result;
   SelectionTree::createEach(
       AST.AST.getASTContext(), AST.AST.getTokens(), *Begin, *End,
       [&](SelectionTree T) {
         Result.push_back(std::make_unique<Tweak::Selection>(
-            AST.Inputs.Index, AST.AST, *Begin, *End, std::move(T)));
+            AST.Inputs.Index, AST.AST, *Begin, *End, std::move(T), Diags));
         return false;
       });
   assert(!Result.empty() && "Expected at least one SelectionTree");
   return std::move(Result);
 }
 
 void ClangdServer::enumerateTweaks(
-    PathRef File, Range Sel, llvm::unique_function<bool(const Tweak &)> Filter,
+    PathRef File, Range Sel, llvm::ArrayRef<Diagnostic> Diags,
+    llvm::unique_function<bool(const Tweak &)> Filter,
     Callback<std::vector<TweakRef>> CB) {
   // Tracks number of times a tweak has been offered.
   static constexpr trace::Metric TweakAvailable(
       "tweak_available", trace::Metric::Counter, "tweak_id");
   auto Action = [File = File.str(), Sel, CB = std::move(CB),
-                 Filter = std::move(Filter),
+                 Filter = std::move(Filter), Diags = Diags.vec(),
                  FeatureModules(this->FeatureModules)](
                     Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
       return CB(InpAST.takeError());
-    auto Selections = tweakSelection(Sel, *InpAST);
+    auto Selections = tweakSelection(Sel, *InpAST, Diags);
     if (!Selections)
       return CB(Selections.takeError());
     std::vector<TweakRef> Res;
     // Don't allow a tweak to fire more than once across ambiguous selections.
     llvm::DenseSet<llvm::StringRef> PreparedTweaks;
     auto DeduplicatingFilter = [&](const Tweak &T) {
       return Filter(T) && !PreparedTweaks.count(T.id());
     };
     for (const auto &Sel : *Selections) {
       for (auto &T : prepareTweaks(*Sel, DeduplicatingFilter, FeatureModules)) {
         Res.push_back({T->id(), T->title(), T->kind()});
         PreparedTweaks.insert(T->id());
         TweakAvailable.record(1, T->id());
       }
     }
 
     CB(std::move(Res));
   };
 
   WorkScheduler->runWithAST("EnumerateTweaks", File, std::move(Action),
                             Transient);
 }
 
 void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID,
+                              llvm::ArrayRef<Diagnostic> Diags,
                               Callback<Tweak::Effect> CB) {
   // Tracks number of times a tweak has been attempted.
   static constexpr trace::Metric TweakAttempt(
       "tweak_attempt", trace::Metric::Counter, "tweak_id");
   // Tracks number of times a tweak has failed to produce edits.
   static constexpr trace::Metric TweakFailed(
       "tweak_failed", trace::Metric::Counter, "tweak_id");
   TweakAttempt.record(1, TweakID);
   auto Action = [File = File.str(), Sel, TweakID = TweakID.str(),
-                 CB = std::move(CB),
+                 CB = std::move(CB), Diags = Diags.vec(),
                  this](Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
       return CB(InpAST.takeError());
-    auto Selections = tweakSelection(Sel, *InpAST);
+    auto Selections = tweakSelection(Sel, *InpAST, Diags);
     if (!Selections)
       return CB(Selections.takeError());
     llvm::Optional<llvm::Expected<Tweak::Effect>> Effect;
     // Try each selection, take the first one that prepare()s.
     // If they all fail, Effect will hold get the last error.
     for (const auto &Selection : *Selections) {
       auto T = prepareTweak(TweakID, *Selection, FeatureModules);
       if (T) {

clang-tools-extra/clangd/Protocol.h

 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_PROTOCOL_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PROTOCOL_H
 
 #include "URI.h"
 #include "index/SymbolID.h"
 #include "support/MemoryTree.h"
 #include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/raw_ostream.h"
 #include <bitset>
 #include <memory>
 #include <string>
 #include <vector>
 

 /// 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;
+  /// Set of diagnostics attached to the codeAction request when this tweak was
+  /// emitted.
+  std::vector<Diagnostic> diagnostics;
 };
 bool fromJSON(const llvm::json::Value &, TweakArgs &, llvm::json::Path);
 llvm::json::Value toJSON(const TweakArgs &A);
 
 struct ExecuteCommandParams {
   /// The identifier of the actual command handler.
   std::string command;
 

clang-tools-extra/clangd/Protocol.cpp

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::Path P) {
   llvm::json::ObjectMapper O(Params, P);
   return O && O.map("file", A.file) && O.map("selection", A.selection) &&
-         O.map("tweakID", A.tweakID);
+         O.map("tweakID", A.tweakID) && O.map("diagnostics", A.diagnostics);
 }
 
 llvm::json::Value toJSON(const TweakArgs &A) {
-  return llvm::json::Object{
-      {"tweakID", A.tweakID}, {"selection", A.selection}, {"file", A.file}};
+  return llvm::json::Object{{"tweakID", A.tweakID},
+                            {"selection", A.selection},
+                            {"file", A.file},
+                            {"diagnostics", A.diagnostics}};
 }
 
 llvm::json::Value toJSON(const ApplyWorkspaceEditParams &Params) {
   return llvm::json::Object{{"edit", Params.edit}};
 }
 
 bool fromJSON(const llvm::json::Value &Response, ApplyWorkspaceEditResponse &R,
               llvm::json::Path P) {

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

 ///     // implement other methods here.
 ///   };
 ///   REGISTER_TWEAK(MyTweak);
 class Tweak {
 public:
   /// Input to prepare and apply tweaks.
   struct Selection {
     Selection(const SymbolIndex *Index, ParsedAST &AST, unsigned RangeBegin,
-              unsigned RangeEnd, SelectionTree ASTSelection);
+              unsigned RangeEnd, SelectionTree ASTSelection,
+              llvm::ArrayRef<Diagnostic> Diags);
     /// The text of the active document.
     llvm::StringRef Code;
     /// The Index for handling codebase related queries.
     const SymbolIndex *Index = nullptr;
     /// The parsed active file. Never null. (Pointer so Selection is movable).
     ParsedAST *AST;
     /// A location of the cursor in the editor.
     // FIXME: Cursor is redundant and should be removed
     SourceLocation Cursor;
     /// The begin offset of the selection
     unsigned SelectionBegin;
     /// The end offset of the selection
     unsigned SelectionEnd;
     /// The AST nodes that were selected.
     SelectionTree ASTSelection;
+    /// Diagnostics related to this selection.
+    llvm::ArrayRef<clangd::Diagnostic> Diags;
     // FIXME: provide a way to get sources and ASTs for other files.
   };
 
   struct Effect {
     /// A message to be displayed to the user.
     llvm::Optional<std::string> ShowMessage;
     FileEdits ApplyEdits;
 

clang-tools-extra/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 "FeatureModule.h"
+#include "Protocol.h"
 #include "SourceCode.h"
 #include "index/Index.h"
 #include "support/Logger.h"
 #include "support/Path.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/Registry.h"
 #include <functional>
for (auto &M : *Modules)
       M.contributeTweaks(All);
   }
   return All;
 }
 } // namespace
 
 Tweak::Selection::Selection(const SymbolIndex *Index, ParsedAST &AST,
                             unsigned RangeBegin, unsigned RangeEnd,
-                            SelectionTree ASTSelection)
+                            SelectionTree ASTSelection,
+                            llvm::ArrayRef<Diagnostic> Diags)
     : Index(Index), AST(&AST), SelectionBegin(RangeBegin),
-      SelectionEnd(RangeEnd), ASTSelection(std::move(ASTSelection)) {
+      SelectionEnd(RangeEnd), ASTSelection(std::move(ASTSelection)),
+      Diags(Diags) {
   auto &SM = AST.getSourceManager();
   Code = SM.getBufferData(SM.getMainFileID());
   Cursor = SM.getComposedLoc(SM.getMainFileID(), RangeBegin);
 }
 
 std::vector<std::unique_ptr<Tweak>>
 prepareTweaks(const Tweak::Selection &S,
               llvm::function_ref<bool(const Tweak &)> Filter,

clang-tools-extra/clangd/test/code-action-request.test

"params": {
     }
 }
 #      CHECK:  "id": 1,
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": [
 # CHECK-NEXT:    {
 # CHECK-NEXT:      "arguments": [
 # CHECK-NEXT:        {
+# CHECK-NEXT:          "diagnostics": [],
 # CHECK-NEXT:          "file": "file://{{.*}}/clangd-test/main.cpp",
 # CHECK-NEXT:          "selection": {
 # CHECK-NEXT:            "end": {
 # CHECK-NEXT:              "character": 4,
 # CHECK-NEXT:              "line": 0
 # CHECK-NEXT:            },
 # CHECK-NEXT:            "start": {
 # CHECK-NEXT:              "character": 0,
"textDocument": { "uri": "test:///main.cpp" },
         }
     }
 }
 #      CHECK:  "id": 3,
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": [
 # CHECK-NEXT:    {
 ---
-{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}}
+{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"diagnostics": [], "file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}}
 #      CHECK:    "newText": "int",
 # CHECK-NEXT:    "range": {
 # CHECK-NEXT:      "end": {
 # CHECK-NEXT:        "character": 4,
 # CHECK-NEXT:        "line": 0
 # CHECK-NEXT:      },
 # CHECK-NEXT:      "start": {
 # CHECK-NEXT:        "character": 0,
 # CHECK-NEXT:        "line": 0
 # CHECK-NEXT:      }
 # CHECK-NEXT:    }
 ---
 {"jsonrpc":"2.0","id":5,"method":"shutdown"}
 ---
 {"jsonrpc":"2.0","method":"exit"}
 ---

clang-tools-extra/clangd/test/request-reply.test

 # RUN: clangd -log=verbose -lit-test < %s | FileCheck -strict-whitespace %s
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"auto i = 0;"}}}
 ---
-{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}}
+{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"diagnostics": [], "file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}}
 #      CHECK:  "id": 0,
 #      CHECK:  "method": "workspace/applyEdit",
 #      CHECK:  "newText": "int",
 # CHECK-NEXT:  "range": {
 # CHECK-NEXT:    "end": {
 # CHECK-NEXT:      "character": 4,
 # CHECK-NEXT:      "line": 0
 # CHECK-NEXT:    },
 # CHECK-NEXT:    "start": {
 # CHECK-NEXT:      "character": 0,
 # CHECK-NEXT:      "line": 0
 # CHECK-NEXT:    }
 # CHECK-NEXT:  }
 ---
 {"jsonrpc":"2.0","id":0,"result":{"applied":false}}
 #      CHECK:  "error": {
 # CHECK-NEXT:    "code": -32001,
 # CHECK-NEXT:    "message": "edits were not applied: unknown reason"
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "id": 4,
 ---
-{"jsonrpc":"2.0","id":5,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}}
+{"jsonrpc":"2.0","id":5,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"diagnostics": [], "file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}}
 #      CHECK:  "id": 1,
 #      CHECK:  "method": "workspace/applyEdit",
 ---
 {"jsonrpc":"2.0","id":1,"result":{"applied":true}}
 #      CHECK:  "id": 5,
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": "Tweak applied."
 ---
 # a reply with an invalid id.
 {"jsonrpc":"2.0","id":"invalid","result":{"applied":true}}
 # clangd doesn't reply, just emits an elog.
 ---
 {"jsonrpc":"2.0","id":6,"method":"shutdown"}
 ---
 {"jsonrpc":"2.0","method":"exit"}

clang-tools-extra/clangd/test/tweaks-format.test

 # RUN: clangd -lit-test < %s | FileCheck %s
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cc","languageId":"cpp","version":1,"text":"int f() { if (true) { return 1; } else {} }"}}}
 ---
 {"jsonrpc":"2.0","id":5,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///main.cc"},"range":{"start":{"line":0,"character":11},"end":{"line":0,"character":11}},"context":{"diagnostics":[]}}}
 ---
-{"jsonrpc":"2.0","id":6,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cc","selection":{"end":{"character":11,"line":0},"start":{"character":11,"line":0}},"tweakID":"SwapIfBranches"}]}}
+{"jsonrpc":"2.0","id":6,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"diagnostics": [], "file":"test:///main.cc","selection":{"end":{"character":11,"line":0},"start":{"character":11,"line":0}},"tweakID":"SwapIfBranches"}]}}
 #      CHECK:    "newText": "\n  ",
 # CHECK-NEXT:    "range": {
 # CHECK-NEXT:      "end": {
 # CHECK-NEXT:        "character": 10,
 # CHECK-NEXT:        "line": 0
 # CHECK-NEXT:      },
 # CHECK-NEXT:      "start": {
 # CHECK-NEXT:        "character": 9,

clang-tools-extra/clangd/tool/Check.cpp

for (const auto &Tok : SpelledTokens) {
       unsigned Start = AST->getSourceManager().getFileOffset(Tok.location());
       unsigned End = Start + Tok.length();
       Position Pos = offsetToPosition(Inputs.Contents, Start);
       // FIXME: dumping the tokens may leak sensitive code into bug reports.
       // Add an option to turn this off, once we decide how options work.
       vlog("  {0} {1}", Pos, Tok.text(AST->getSourceManager()));
       auto Tree = SelectionTree::createRight(AST->getASTContext(),
                                              AST->getTokens(), Start, End);
-      Tweak::Selection Selection(&Index, *AST, Start, End, std::move(Tree));
+      Tweak::Selection Selection(&Index, *AST, Start, End, std::move(Tree), {});
       for (const auto &T :
            prepareTweaks(Selection, Opts.TweakFilter, Opts.FeatureModules)) {
         auto Result = T->apply(Selection);
         if (!Result) {
           elog("    tweak: {0} ==> FAIL: {1}", T->id(), Result.takeError());
           ++ErrCount;
         } else {
           vlog("    tweak: {0}", T->id());

clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp

 //===-- ClangdLSPServerTests.cpp ------------------------------------------===//
 //
 // 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 "Annotations.h"
 #include "ClangdLSPServer.h"
 #include "LSPClient.h"
 #include "Protocol.h"
 #include "TestFS.h"
+#include "URI.h"
 #include "support/Logger.h"
 #include "support/TestTracer.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Testing/Support/Error.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
TEST_F(LSPTest, DiagModuleTest) {
   };
   FeatureModules.add(std::make_unique<DiagModule>());
 
   auto &Client = start();
   Client.didOpen("foo.cpp", "test;");
   EXPECT_THAT(Client.diagnostics("foo.cpp"),
               llvm::ValueIs(testing::ElementsAre(DiagMessage(DiagMsg))));
 }
+
+TEST_F(LSPTest, TweakForDiag) {
+  static constexpr const char *TweakID = "ModuleTweak";
+  struct TweakContributingModule final : public FeatureModule {
+    struct ModuleTweak final : public Tweak {
+      const char *id() const override { return TweakID; }
+      bool prepare(const Selection &Sel) override { return !Sel.Diags.empty(); }
+      Expected<Effect> apply(const Selection &Sel) override {
+        return error("not implemented");
+      }
+      std::string title() const override { return id(); }
+      llvm::StringLiteral kind() const override { return ""; };
+    };
+
+    void contributeTweaks(std::vector<std::unique_ptr<Tweak>> &Out) override {
+      Out.emplace_back(new ModuleTweak);
+    }
+  };
+
+  FeatureModules.add(std::make_unique<TweakContributingModule>());
+  auto &Client = start();
+  Client.didOpen("foo.cpp", "test;");
+  auto Diags = Client.diagnostics("foo.cpp");
+  ASSERT_TRUE(Diags.hasValue());
+  ASSERT_EQ(Diags->size(), 1U);
+  Diagnostic Diag;
+  llvm::json::Path::Root R;
+
+  ASSERT_TRUE(fromJSON(Diags->front(), Diag, R));
+
+  llvm::json::Object CodeActionParams;
+  CodeActionParams["textDocument"] = llvm::json::Object{
+      {"uri", URI::createFile(testPath("foo.cpp")).toString()}};
+  CodeActionParams["range"] = Diag.range;
+  CodeActionParams["context"] =
+      llvm::json::Object{{"diagnostics", llvm::json::Array{Diag}}};
+
+  // Check diags is propagated to code action.
+  auto ActionsStorage =
+      Client.call("textDocument/codeAction", std::move(CodeActionParams))
+          .takeValue();
+  auto *Actions = ActionsStorage.getAsArray();
+  ASSERT_TRUE(Actions);
+  ASSERT_EQ(Actions->size(), 1U);
+  auto *Action = Actions->front().getAsObject();
+  ASSERT_TRUE(Action);
+  EXPECT_THAT(Action->getString("title"), llvm::ValueIs(TweakID));
+
+  // Ensure diags is also propagated on execute.
+  llvm::json::Object ExecParams;
+  ExecParams["command"] = *Action->getString("command");
+  ExecParams["arguments"] = llvm::json::Array(*Action->getArray("arguments"));
+  auto Res =
+      Client.call("workspace/executeCommand", std::move(ExecParams)).take();
+  EXPECT_EQ(llvm::toString(Res.takeError()), "not implemented");
+}
 } // namespace
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp

TEST(FeatureModulesTest, ContributesTweak) {
 
   FeatureModuleSet Set;
   Set.add(std::make_unique<TweakContributingModule>());
 
   auto AST = TestTU::withCode("").build();
   auto Tree =
       SelectionTree::createRight(AST.getASTContext(), AST.getTokens(), 0, 0);
   auto Actual = prepareTweak(
-      TweakID, Tweak::Selection(nullptr, AST, 0, 0, std::move(Tree)), &Set);
+      TweakID, Tweak::Selection(nullptr, AST, 0, 0, std::move(Tree), {}), &Set);
   ASSERT_TRUE(bool(Actual));
   EXPECT_EQ(Actual->get()->id(), TweakID);
 }
 
 } // namespace
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp

 llvm::Optional<llvm::Expected<Tweak::Effect>>
 applyTweak(ParsedAST &AST, const Annotations &Input, StringRef TweakID,
            const SymbolIndex *Index) {
   auto Range = rangeOrPoint(Input);
   llvm::Optional<llvm::Expected<Tweak::Effect>> Result;
   SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), Range.first,
                             Range.second, [&](SelectionTree ST) {
                               Tweak::Selection S(Index, AST, Range.first,
-                                                 Range.second, std::move(ST));
+                                                 Range.second, std::move(ST),
+                                                 {});
                               if (auto T = prepareTweak(TweakID, S, nullptr)) {
                                 Result = (*T)->apply(S);
                                 return true;
                               } else {
                                 llvm::consumeError(T.takeError());
                                 return false;
                               }
                             });