[clangd] Move the format rename edits from clangdserver to rename API, NFC.

[hokein]

Summary

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

[kbobyrev]

LGTM with few nits

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

466

nit: Edits

469

nit: Edit

473

maybe return Err ? std::move(Err) : FE;, but up to you

clang-tools-extra/clangd/ClangdServer.h

public:
 
   /// Test the validity of a rename operation.
   void prepareRename(PathRef File, Position Pos,
                      const RenameOptions &RenameOpts,
                      Callback<llvm::Optional<Range>> CB);
 
   /// 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,
               const RenameOptions &Opts, Callback<FileEdits> CB);
   // FIXME: remove this compatibility method in favor above.
   void rename(PathRef File, Position Pos, llvm::StringRef NewName,
               bool WantFormat, Callback<FileEdits> CB);
 

clang-tools-extra/clangd/ClangdServer.cpp

if (!InpAST)
       return CB(InpAST.takeError());
     auto GetDirtyBuffer =
         [&Snapshot](PathRef AbsPath) -> llvm::Optional<std::string> {
       auto It = Snapshot.find(AbsPath);
       if (It == Snapshot.end())
         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));
 }
 
 void ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName,
                           bool WantFormat, Callback<FileEdits> CB) {
   RenameOptions Opts;
   Opts.WantFormat = WantFormat;

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

 struct RenameOptions {
   /// If true, enable cross-file rename; otherwise, only allows to rename a
   /// symbol that's only used in the current file.
   bool AllowCrossFile = false;
   /// The mamimum number of affected files (0 means no limit), only meaningful
   /// 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;
 };
 
 struct RenameInputs {
   Position Pos; // the position triggering the rename
   llvm::StringRef NewName;
 
   ParsedAST *
   llvm::StringRef MainFilePath;
 
   const SymbolIndex *Index = nullptr;
 
   RenameOptions Opts = {};
   // When set, used by the rename to get file content for all rename-related
   // files.
   // If there is no corresponding dirty buffer, we will use the file content
   // from disk.
   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);
 
 /// Generates rename edits that replaces all given occurrences with the
 /// NewName.
 /// Exposed for testing only.
 /// REQUIRED: Occurrences is sorted and doesn't have duplicated ranges.

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

 #include "ParsedAST.h"
 #include "Selection.h"
 #include "SourceCode.h"
 #include "Trace.h"
 #include "index/SymbolCollector.h"
 #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"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"
 #include <algorithm>
if (impliesSimpleEdit(IndexedRest.front().start, LexedRest.front().start)) {
     findNearMiss(PartialMatch, IndexedRest.drop_front(), LexedRest.drop_front(),
                  LexedIndex + 1, Fuel, MatchedCB);
     PartialMatch.pop_back();
   }
   findNearMiss(PartialMatch, IndexedRest, LexedRest.drop_front(),
                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) {
   trace::Span Tracer("Rename flow");
   const auto &Opts = RInputs.Opts;
   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;
     if (RInputs.GetDirtyBuffer &&
         (DirtyBuffer = RInputs.GetDirtyBuffer(AbsPath)))
       return std::move(*DirtyBuffer);
 
     auto Content =
llvm::Expected<FileEdits> rename(const RenameInputs &RInputs) {
   //   - run index-based rename on other affected files;
   auto MainFileRenameEdit = renameWithinFile(AST, RenameDecl, RInputs.NewName);
   if (!MainFileRenameEdit)
     return MainFileRenameEdit.takeError();
 
   // 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;
   // Renameable safely guards us that at this point we are renaming a local
   // symbol if we don't have index.
   if (RInputs.Index) {
     auto OtherFilesEdits = renameOutsideFile(
         RenameDecl, RInputs.MainFilePath, RInputs.NewName, *RInputs.Index,
         Opts.LimitFiles == 0 ? std::numeric_limits<size_t>::max()
                              : Opts.LimitFiles - 1,
         GetFileContent);
     if (!OtherFilesEdits)
       return OtherFilesEdits.takeError();
     Results = std::move(*OtherFilesEdits);
   }
   // 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,
                                      llvm::StringRef InitialCode,
                                      std::vector<Range> Occurrences,
                                      llvm::StringRef NewName) {
   trace::Span Tracer("BuildRenameEdit");
   SPAN_ATTACH(Tracer, "file_path", AbsFilePath);

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

#endif
   };
   llvm::Optional<OffsetEncoding> OffsetEncodingFromFlag;
   if (ForceOffsetEncoding != OffsetEncoding::UnsupportedEncoding)
     OffsetEncodingFromFlag = ForceOffsetEncoding;
 
   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,
       OffsetEncodingFromFlag, Opts);
   llvm::set_thread_name("clangd.main");
   int ExitCode = LSPServer.run()
                      ? 0
                      : static_cast<int>(ErrorResultCode::NoShutdownRequest);