hokein updated this revision to Diff 185529.
hokein added a comment.

Move format to the tweak.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57739/new/

https://reviews.llvm.org/D57739

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/Compiler.h
  clangd/refactor/Tweak.cpp
  clangd/refactor/Tweak.h
  clangd/refactor/tweaks/SwapIfBranches.cpp
  unittests/clangd/TweakTests.cpp

Index: unittests/clangd/TweakTests.cpp
===================================================================
--- unittests/clangd/TweakTests.cpp
+++ unittests/clangd/TweakTests.cpp
@@ -53,7 +53,8 @@
   auto CheckOver = [&](Range Selection) {
     unsigned Begin = cantFail(positionToOffset(Code.code(), Selection.start));
     unsigned End = cantFail(positionToOffset(Code.code(), Selection.end));
-    auto T = prepareTweak(ID, Tweak::Selection(AST, Begin, End));
+    auto T = prepareTweak(
+        ID, Tweak::Selection(AST, Begin, End, clang::format::getLLVMStyle()));
     if (Available)
       EXPECT_THAT_EXPECTED(T, Succeeded())
           << "code is " << markRange(Code.code(), Selection);
@@ -93,7 +94,7 @@
   ParsedAST AST = TU.build();
   unsigned Begin = cantFail(positionToOffset(Code.code(), SelectionRng.start));
   unsigned End = cantFail(positionToOffset(Code.code(), SelectionRng.end));
-  Tweak::Selection S(AST, Begin, End);
+  Tweak::Selection S(AST, Begin, End, clang::format::getLLVMStyle());
 
   auto T = prepareTweak(ID, S);
   if (!T)
@@ -127,12 +128,40 @@
 
   llvm::StringLiteral Input = R"cpp(
     void test() {
-      ^if (true) { return 100; } else { continue; }
+      ^if (true) {
+        return 100;
+      } else {
+        continue;
+      }
     }
   )cpp";
   llvm::StringLiteral Output = R"cpp(
     void test() {
-      if (true) { continue; } else { return 100; }
+      if (true) {
+        continue;
+      } else {
+        return 100;
+      }
+    }
+  )cpp";
+  checkTransform(ID, Input, Output);
+
+  Input = R"cpp(
+    void test() {
+      ^if () {
+        return 100;
+      } else {
+        continue;
+      }
+    }
+  )cpp";
+  Output = R"cpp(
+    void test() {
+      if () {
+        continue;
+      } else {
+        return 100;
+      }
     }
   )cpp";
   checkTransform(ID, Input, Output);
@@ -144,7 +173,11 @@
   )cpp";
   Output = R"cpp(
     void test() {
-      if () { continue; } else { return 100; }
+      if () {
+        continue;
+      } else {
+        return 100;
+      }
     }
   )cpp";
   checkTransform(ID, Input, Output);
Index: clangd/refactor/tweaks/SwapIfBranches.cpp
===================================================================
--- clangd/refactor/tweaks/SwapIfBranches.cpp
+++ clangd/refactor/tweaks/SwapIfBranches.cpp
@@ -37,9 +37,11 @@
   const char *id() const override final;
 
   bool prepare(const Selection &Inputs) override;
-  Expected<tooling::Replacements> apply(const Selection &Inputs) override;
   std::string title() const override;
 
+protected:
+  Expected<tooling::Replacements> execute(const Selection &Inputs) override;
+
 private:
   const IfStmt *If = nullptr;
 };
@@ -60,7 +62,8 @@
          dyn_cast_or_null<CompoundStmt>(If->getElse());
 }
 
-Expected<tooling::Replacements> SwapIfBranches::apply(const Selection &Inputs) {
+Expected<tooling::Replacements>
+SwapIfBranches::execute(const Selection &Inputs) {
   auto &Ctx = Inputs.AST.getASTContext();
   auto &SrcMgr = Ctx.getSourceManager();
 
Index: clangd/refactor/Tweak.h
===================================================================
--- clangd/refactor/Tweak.h
+++ clangd/refactor/Tweak.h
@@ -40,16 +40,20 @@
 public:
   /// Input to prepare and apply tweaks.
   struct Selection {
-    Selection(ParsedAST &AST, unsigned RangeBegin, unsigned RangeEnd);
+    Selection(ParsedAST &AST, unsigned RangeBegin, unsigned RangeEnd,
+              format::FormatStyle Style);
     /// The text of the active document.
     llvm::StringRef Code;
     /// Parsed AST of the active file.
     ParsedAST &AST;
     /// A location of the cursor in the editor.
     SourceLocation Cursor;
-    // The AST nodes that were selected.
+    /// The AST nodes that were selected.
     SelectionTree ASTSelection;
     // FIXME: provide a way to get sources and ASTs for other files.
+
+    /// The style to format generated changes.
+    format::FormatStyle Style;
   };
   virtual ~Tweak() = default;
   /// A unique id of the action, it is always equal to the name of the class
@@ -63,13 +67,18 @@
   /// 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;
+  /// Format and apply the actual changes generated from the second stage of the
+  /// action.
+  Expected<tooling::Replacements> apply(const Selection &Sel);
   /// 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;
+
+protected:
+  /// Run the second stage of the action that would produce the actual changes.
+  /// EXPECTS: prepare() was called and returned true.
+  virtual Expected<tooling::Replacements> execute(const Selection &Sel) = 0;
 };
 
 // All tweaks must be registered in the .cpp file next to their definition.
Index: clangd/refactor/Tweak.cpp
===================================================================
--- clangd/refactor/Tweak.cpp
+++ clangd/refactor/Tweak.cpp
@@ -6,6 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 #include "Tweak.h"
+#include "Format.h"
 #include "Logger.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringMap.h"
@@ -39,13 +40,21 @@
 } // namespace
 
 Tweak::Selection::Selection(ParsedAST &AST, unsigned RangeBegin,
-                            unsigned RangeEnd)
-    : AST(AST), ASTSelection(AST.getASTContext(), RangeBegin, RangeEnd) {
+                            unsigned RangeEnd, format::FormatStyle Style)
+    : AST(AST), ASTSelection(AST.getASTContext(), RangeBegin, RangeEnd),
+      Style(std::move(Style)) {
   auto &SM = AST.getASTContext().getSourceManager();
   Code = SM.getBufferData(SM.getMainFileID());
   Cursor = SM.getComposedLoc(SM.getMainFileID(), RangeBegin);
 }
 
+Expected<tooling::Replacements> Tweak::apply(const Selection &Sel) {
+  auto RawReplacements = execute(Sel);
+  if (!RawReplacements)
+    return RawReplacements;
+  return cleanupAndFormat(Sel.Code, *RawReplacements, Sel.Style);
+}
+
 std::vector<std::unique_ptr<Tweak>> prepareTweaks(const Tweak::Selection &S) {
   validateRegistry();
 
Index: clangd/Compiler.h
===================================================================
--- clangd/Compiler.h
+++ clangd/Compiler.h
@@ -17,6 +17,7 @@
 
 #include "../clang-tidy/ClangTidyOptions.h"
 #include "index/Index.h"
+#include "clang/Format/Format.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
@@ -38,6 +39,7 @@
 struct ParseOptions {
   tidy::ClangTidyOptions ClangTidyOpts;
   bool SuggestMissingIncludes = false;
+  format::FormatStyle Style;
 };
 
 /// Information required to run clang, e.g. to parse AST or do code completion.
Index: clangd/ClangdUnit.cpp
===================================================================
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -309,9 +309,8 @@
   llvm::Optional<IncludeFixer> FixIncludes;
   auto BuildDir = VFS->getCurrentWorkingDirectory();
   if (Opts.SuggestMissingIncludes && Index && !BuildDir.getError()) {
-    auto Style = getFormatStyleForFile(MainInput.getFile(), Content, VFS.get());
     auto Inserter = std::make_shared<IncludeInserter>(
-        MainInput.getFile(), Content, Style, BuildDir.get(),
+        MainInput.getFile(), Content, Opts.Style, BuildDir.get(),
         Clang->getPreprocessor().getHeaderSearchInfo());
     if (Preamble) {
       for (const auto &Inc : Preamble->Includes.MainFileIncludes)
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -152,6 +152,9 @@
   Opts.ClangTidyOpts = tidy::ClangTidyOptions::getDefaults();
   if (ClangTidyOptProvider)
     Opts.ClangTidyOpts = ClangTidyOptProvider->getOptions(File);
+  // FIXME: cache this
+  Opts.Style =
+      getFormatStyleForFile(File, Contents, FSProvider.getFileSystem().get());
   Opts.SuggestMissingIncludes = SuggestMissingIncludes;
   // FIXME: some build systems like Bazel will take time to preparing
   // environment to build the file, it would be nice if we could emit a
@@ -338,7 +341,7 @@
   auto End = positionToOffset(AST.Inputs.Contents, Sel.end);
   if (!End)
     return End.takeError();
-  return Tweak::Selection(AST.AST, *Begin, *End);
+  return Tweak::Selection(AST.AST, *Begin, *End, AST.Inputs.Opts.Style);
 }
 
 void ClangdServer::enumerateTweaks(PathRef File, Range Sel,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to