hokein created this revision.
hokein added a reviewer: kbobyrev.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

would make the rename API more nature, respecting the parameter 
Options.WantFormat.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75251

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/refactor/Rename.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -712,7 +712,7 @@
   clangd::RenameOptions RenameOpts;
   // Shall we allow to custimize the file limit?
   RenameOpts.AllowCrossFile = CrossFileRename;
-
+  RenameOpts.WantFormat = true;
   ClangdLSPServer LSPServer(
       *TransportLayer, FSProvider, CCOpts, RenameOpts, CompileCommandsDirPath,
       /*UseDirBasedCDB=*/CompileArgsFrom == FilesystemCompileArgs,
Index: clang-tools-extra/clangd/refactor/Rename.h
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.h
+++ clang-tools-extra/clangd/refactor/Rename.h
@@ -34,7 +34,7 @@
   /// when AllowCrossFile = true.
   /// If the actual number exceeds the limit, rename is forbidden.
   size_t LimitFiles = 50;
-  /// If true, format the rename edits, only meaningful in ClangdServer layer.
+  /// If true, format the rename edits.
   bool WantFormat = false;
 };
 
@@ -55,7 +55,7 @@
   DirtyBufferGetter GetDirtyBuffer = nullptr;
 };
 
-/// Renames all occurrences of the symbol. The result edits are unformatted.
+/// Renames all occurrences of the symbol.
 /// If AllowCrossFile is false, returns an error if rename a symbol that's used
 /// in another file (per the index).
 llvm::Expected<FileEdits> rename(const RenameInputs &RInputs);
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -18,6 +18,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Format/Format.h"
 #include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/None.h"
@@ -462,6 +463,16 @@
                LexedIndex + 1, Fuel, MatchedCB);
 }
 
+llvm::Expected<FileEdits> formatEdits(FileEdits FE,
+                                      const format::FormatStyle Style) {
+  llvm::Error Err = llvm::Error::success();
+  for (auto &E : FE)
+    Err = llvm::joinErrors(reformatEdit(E.getValue(), Style), std::move(Err));
+  if (Err)
+    return std::move(Err);
+  return FE;
+}
+
 } // namespace
 
 llvm::Expected<FileEdits> rename(const RenameInputs &RInputs) {
@@ -470,6 +481,9 @@
   ParsedAST &AST = RInputs.AST;
   const SourceManager &SM = AST.getSourceManager();
   llvm::StringRef MainFileCode = SM.getBufferData(SM.getMainFileID());
+  const auto FormatStyle =
+      getFormatStyleForFile(RInputs.MainFilePath, MainFileCode,
+                            &SM.getFileManager().getVirtualFileSystem());
   auto GetFileContent = [&RInputs,
                          &SM](PathRef AbsPath) -> llvm::Expected<std::string> {
     llvm::Optional<std::string> DirtyBuffer;
@@ -534,9 +548,11 @@
   // return the main file edit if this is a within-file rename or the symbol
   // being renamed is function local.
   if (!Opts.AllowCrossFile || RenameDecl.getParentFunctionOrMethod()) {
-    return FileEdits(
+    auto MainFileEdit = FileEdits(
         {std::make_pair(RInputs.MainFilePath,
                         Edit{MainFileCode, std::move(*MainFileRenameEdit)})});
+    return RInputs.Opts.WantFormat ? formatEdits(MainFileEdit, FormatStyle)
+                                   : MainFileEdit;
   }
 
   FileEdits Results;
@@ -555,7 +571,7 @@
   // Attach the rename edits for the main file.
   Results.try_emplace(RInputs.MainFilePath, MainFileCode,
                       std::move(*MainFileRenameEdit));
-  return Results;
+  return RInputs.Opts.WantFormat ? formatEdits(Results, FormatStyle) : Results;
 }
 
 llvm::Expected<Edit> buildRenameEdit(llvm::StringRef AbsFilePath,
Index: clang-tools-extra/clangd/ClangdServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -259,7 +259,7 @@
 
   /// Rename all occurrences of the symbol at the \p Pos in \p File to
   /// \p NewName.
-  /// If WantFormat is false, the final TextEdit will be not formatted,
+  /// If Opts.WantFormat is false, the final TextEdit will be not formatted,
   /// embedders could use this method to get all occurrences of the symbol (e.g.
   /// highlighting them in prepare stage).
   void rename(PathRef File, Position Pos, llvm::StringRef NewName,
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -373,23 +373,8 @@
         return llvm::None;
       return It->second;
     };
-    auto Edits = clangd::rename(
-        {Pos, NewName, InpAST->AST, File, Index, Opts, GetDirtyBuffer});
-    if (!Edits)
-      return CB(Edits.takeError());
-
-    if (Opts.WantFormat) {
-      auto Style = getFormatStyleForFile(File, InpAST->Inputs.Contents,
-                                         InpAST->Inputs.FS.get());
-      llvm::Error Err = llvm::Error::success();
-      for (auto &E : *Edits)
-        Err =
-            llvm::joinErrors(reformatEdit(E.getValue(), Style), std::move(Err));
-
-      if (Err)
-        return CB(std::move(Err));
-    }
-    return CB(std::move(*Edits));
+    return CB(clangd::rename(
+        {Pos, NewName, InpAST->AST, File, Index, Opts, GetDirtyBuffer}));
   };
   WorkScheduler.runWithAST("Rename", File, std::move(Action));
 }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D75251: [clangd] Mo... Haojian Wu via Phabricator via cfe-commits

Reply via email to