[clang-tools-extra] [clangd] forward clang-tidy's readability-identifier-naming fix to textDocument/rename (PR #78454)

Tom Praschan via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 17 06:49:14 PST 2024


https://github.com/tom-anders updated https://github.com/llvm/llvm-project/pull/78454

>From 1fb6fb163bf2e63cb30571b179768bae2f43eb71 Mon Sep 17 00:00:00 2001
From: tom-anders <13141438+tom-anders at users.noreply.github.com>
Date: Wed, 17 Jan 2024 15:36:02 +0100
Subject: [PATCH] [clangd] forward clang-tidy's readability-identifier-naming
 fix to textDocument/rename

---
 clang-tools-extra/clangd/ClangdLSPServer.cpp  | 32 +++++++++++++++++++
 clang-tools-extra/clangd/ClangdLSPServer.h    |  1 +
 clang-tools-extra/clangd/ClangdServer.cpp     | 27 ++++++++++++----
 clang-tools-extra/clangd/ClangdServer.h       |  5 +++
 clang-tools-extra/clangd/Protocol.cpp         |  8 +++++
 clang-tools-extra/clangd/Protocol.h           |  1 +
 .../clangd/test/initialize-params.test        |  1 +
 7 files changed, 68 insertions(+), 7 deletions(-)

diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index a87da252b7a7e9..49d1eb0e8341c1 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -73,6 +73,23 @@ std::optional<int64_t> decodeVersion(llvm::StringRef Encoded) {
 
 const llvm::StringLiteral ApplyFixCommand = "clangd.applyFix";
 const llvm::StringLiteral ApplyTweakCommand = "clangd.applyTweak";
+const llvm::StringLiteral ApplyRenameCommand = "clangd.applyRename";
+
+CodeAction toCodeAction(const ClangdServer::CodeActionResult::Rename &R,
+                        const URIForFile &File) {
+  CodeAction CA;
+  CA.title = R.Diag.Message;
+  CA.kind = std::string(CodeAction::REFACTOR_KIND);
+  CA.command.emplace();
+  CA.command->title = R.Diag.Message;
+  CA.command->command = std::string(ApplyRenameCommand);
+  RenameParams Params;
+  Params.textDocument = TextDocumentIdentifier{File};
+  Params.position = R.Diag.Range.start;
+  Params.newName = R.NewName;
+  CA.command->argument = Params;
+  return CA;
+}
 
 /// Transforms a tweak into a code action that would apply it if executed.
 /// EXPECTS: T.prepare() was called and returned true.
@@ -808,6 +825,16 @@ void ClangdLSPServer::onCommandApplyTweak(const TweakArgs &Args,
                      std::move(Action));
 }
 
+void ClangdLSPServer::onCommandApplyRename(const RenameParams &R,
+                                           Callback<llvm::json::Value> Reply) {
+  onRename(R, [this, Reply = std::move(Reply)](
+                  llvm::Expected<WorkspaceEdit> Edit) mutable {
+    if (!Edit)
+      Reply(Edit.takeError());
+    applyEdit(std::move(*Edit), "Rename applied.", std::move(Reply));
+  });
+}
+
 void ClangdLSPServer::applyEdit(WorkspaceEdit WE, llvm::json::Value Success,
                                 Callback<llvm::json::Value> Reply) {
   ApplyWorkspaceEditParams Edit;
@@ -1043,6 +1070,10 @@ void ClangdLSPServer::onCodeAction(const CodeActionParams &Params,
         CAs.back().diagnostics = {It->second};
       }
     }
+
+    for (const auto &R : Fixits->Renames)
+      CAs.push_back(toCodeAction(R, File));
+
     for (const auto &TR : Fixits->TweakRefs)
       CAs.push_back(toCodeAction(TR, File, Selection));
 
@@ -1664,6 +1695,7 @@ void ClangdLSPServer::bindMethods(LSPBinder &Bind,
   Bind.method("textDocument/foldingRange", this, &ClangdLSPServer::onFoldingRange);
   Bind.command(ApplyFixCommand, this, &ClangdLSPServer::onCommandApplyEdit);
   Bind.command(ApplyTweakCommand, this, &ClangdLSPServer::onCommandApplyTweak);
+  Bind.command(ApplyRenameCommand, this, &ClangdLSPServer::onCommandApplyRename);
 
   ApplyWorkspaceEdit = Bind.outgoingMethod("workspace/applyEdit");
   PublishDiagnostics = Bind.outgoingNotification("textDocument/publishDiagnostics");
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h
index 79579c22b788a9..69a349c6a5e087 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.h
+++ b/clang-tools-extra/clangd/ClangdLSPServer.h
@@ -174,6 +174,7 @@ class ClangdLSPServer : private ClangdServer::Callbacks,
   /// Implement commands.
   void onCommandApplyEdit(const WorkspaceEdit &, Callback<llvm::json::Value>);
   void onCommandApplyTweak(const TweakArgs &, Callback<llvm::json::Value>);
+  void onCommandApplyRename(const RenameParams &, Callback<llvm::json::Value>);
 
   /// Outgoing LSP calls.
   LSPBinder::OutgoingMethod<ApplyWorkspaceEditParams,
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index 6fb2641e8793db..a2a93ccee9480e 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -668,16 +668,29 @@ void ClangdServer::codeAction(const CodeActionInputs &Params,
     CodeActionResult Result;
     Result.Version = InpAST->AST.version().str();
     if (KindAllowed(CodeAction::QUICKFIX_KIND)) {
-      auto FindMatchedFixes =
-          [&InpAST](const DiagRef &DR) -> llvm::ArrayRef<Fix> {
+      auto FindMatchedDiag = [&InpAST](const DiagRef &DR) -> const Diag * {
         for (const auto &Diag : InpAST->AST.getDiagnostics())
           if (Diag.Range == DR.Range && Diag.Message == DR.Message)
-            return Diag.Fixes;
-        return {};
+            return &Diag;
+        return nullptr;
       };
-      for (const auto &Diag : Params.Diagnostics)
-        for (const auto &Fix : FindMatchedFixes(Diag))
-          Result.QuickFixes.push_back({Diag, Fix});
+      for (const auto &DiagRef : Params.Diagnostics) {
+        if (const auto *Diag = FindMatchedDiag(DiagRef))
+          for (const auto &Fix : Diag->Fixes) {
+            bool IsClangTidyRename =
+                Diag->Source == Diag::ClangTidy &&
+                Diag->Name == "readability-identifier-naming" &&
+                !Fix.Edits.empty();
+            if (IsClangTidyRename) {
+              CodeActionResult::Rename R;
+              R.NewName = Fix.Edits.front().newText;
+              R.Diag = DiagRef;
+              Result.Renames.emplace_back(std::move(R));
+            } else {
+              Result.QuickFixes.push_back({DiagRef, Fix});
+            }
+          }
+      }
     }
 
     // Collect Tweaks
diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h
index a416602251428b..46245205e5239d 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -380,6 +380,11 @@ class ClangdServer {
     };
     std::vector<QuickFix> QuickFixes;
     std::vector<TweakRef> TweakRefs;
+    struct Rename {
+      DiagRef Diag;
+      std::string NewName;
+    };
+    std::vector<Rename> Renames;
   };
   /// Surface code actions (quick-fixes for diagnostics, or available code
   /// tweaks) for a given range in a file.
diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp
index a6370649f5ad1c..7cc248cd1da553 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -1187,6 +1187,14 @@ bool fromJSON(const llvm::json::Value &Params, RenameParams &R,
          O.map("position", R.position) && O.map("newName", R.newName);
 }
 
+llvm::json::Value toJSON(const RenameParams &R) {
+  return llvm::json::Object{
+      {"textDocument", R.textDocument},
+      {"position", R.position},
+      {"newName", R.newName},
+  };
+}
+
 llvm::json::Value toJSON(const DocumentHighlight &DH) {
   return llvm::json::Object{
       {"range", toJSON(DH.range)},
diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h
index e88c804692568f..d2812ae6b78ac6 100644
--- a/clang-tools-extra/clangd/Protocol.h
+++ b/clang-tools-extra/clangd/Protocol.h
@@ -1435,6 +1435,7 @@ struct RenameParams {
   std::string newName;
 };
 bool fromJSON(const llvm::json::Value &, RenameParams &, llvm::json::Path);
+llvm::json::Value toJSON(const RenameParams &);
 
 enum class DocumentHighlightKind { Text = 1, Read = 2, Write = 3 };
 
diff --git a/clang-tools-extra/clangd/test/initialize-params.test b/clang-tools-extra/clangd/test/initialize-params.test
index a1fdae9870ab6e..7c96eb9835b713 100644
--- a/clang-tools-extra/clangd/test/initialize-params.test
+++ b/clang-tools-extra/clangd/test/initialize-params.test
@@ -40,6 +40,7 @@
 # CHECK-NEXT:      "executeCommandProvider": {
 # CHECK-NEXT:        "commands": [
 # CHECK-NEXT:          "clangd.applyFix",
+# CHECK-NEXT:          "clangd.applyRename"
 # CHECK-NEXT:          "clangd.applyTweak"
 # CHECK-NEXT:        ]
 # CHECK-NEXT:      },



More information about the cfe-commits mailing list