Added interfaces for code-formatting while applying replacements.

[ioeric]

Summary

Merge branch 'master' of http://llvm.org/git/clang

[klimek]

include/clang/Tooling/Core/Replacement.h

227–232

I think we'll need to mainly call out the differences to the function above in the comment.

241–243

I'd probably put the Style parameter last.

lib/Tooling/Core/Replacement.cpp

14–16

Nit: I'd put newlines between the #include groups here.

293

We'll either want to assert that !Replaces.empty() (and document that in the function comment), or do an early exit if that's the case.

305

This comment doesn't carry it's weight, I think. I'd delete it.

316–318

I think we'll want to cut that out for now and implement it so that it works for arbitrary sets of replacements in a follow-up cl.

338

If the inner functions handle this case correctly, we don't need to handle it here.

unittests/Tooling/RefactoringTest.cpp

171–177

If you want to test that breaks happen, it's often better to use a configuration with a smaller column limit.

[ioeric]

• removed applyAllReplacementsAndFormat with Rewritter interface. reduced the column limit in test case; some formatting.

[klimek]

LG

lib/Tooling/Core/Replacement.cpp

290

I'd return a default constructed Replacements() instead - I think that requires a bit less reasoning (as written currently, I'll need to reason that a copy is made to convince myself that this is correct).

[ioeric]

• return Replacements() instead of the empty Replaces.

[klimek]

Submitted as r262232

include/clang/Tooling/Core/Replacement.h

 #include <set>
 #include <string>
 #include <vector>
 
 namespace clang {
 
 class Rewriter;
 
+namespace format {
+struct FormatStyle;
+} // namespace format
+
 namespace tooling {
 
 /// \brief A source range independent of the \c SourceManager.
 class Range {
 public:
   Range() : Offset(0), Length(0) {}
   Range(unsigned Offset, unsigned Length) : Offset(Offset), Length(Length) {}
 
bool applyAllReplacements(const std::vector<Replacement> &Replaces,
                           Rewriter &Rewrite);
 
 /// \brief Applies all replacements in \p Replaces to \p Code.
 ///
 /// This completely ignores the path stored in each replacement. If one or more
 /// replacements cannot be applied, this returns an empty \c string.
 std::string applyAllReplacements(StringRef Code, const Replacements &Replaces);
 
+/// \brief Applies all replacements in \p Replaces to \p Code.
+///
+/// This completely ignores the path stored in each replacement. If one or more
+/// replacements cannot be applied, this returns an empty \c string.
+std::string applyAllReplacements(StringRef Code,
+                                 const std::vector<Replacements> &Replaces);
+
+/// \brief Calculate the ranges in a single file that are affected by the
+/// Replacements.
+///
+/// \pre Replacements must be for the same file.
+std::vector<tooling::Range>
+calculateChangedRangesInFile(const tooling::Replacements &Replaces);
+
+/// \brief Return replacements that are merged from orginal replacements
+/// and the replacements for formatting the code after applying the orginal
+/// replacements.
+tooling::Replacements formatReplacements(StringRef Code,
+                                         const tooling::Replacements &Replaces,
+                                         const format::FormatStyle &Style);
+
+/// \brief In addition to applying replacements as in `applyAllReplacements`,
+/// this function also reformats the changed code after applying replacements.
+///
+/// \pre Replacements must be for the same file and conflict-free.
+///
+/// Replacement applications happen independently of the success of
+/// other applications.
+///
+/// \returns the changed code if all replacements apply and code is fixed.
+/// empty string otherwise.
+std::string applyAllReplacementsAndFormat(StringRef Code,
+                                          const Replacements &Replaces,
+                                          const format::FormatStyle &Style);
+
 /// \brief Merges two sets of replacements with the second set referring to the
 /// code after applying the first set. Within both 'First' and 'Second',
 /// replacements must not overlap.
 Replacements mergeReplacements(const Replacements &First,
                                const Replacements &Second);
 
 template <typename Node>
 Replacement::Replacement(const SourceManager &Sources,

lib/Tooling/Core/Replacement.cpp

 //===--- Replacement.cpp - Framework for clang refactoring tools ----------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 //  Implements classes to support/store refactorings.
 //
 //===----------------------------------------------------------------------===//
 
+#include "clang/Tooling/Core/Replacement.h"
+
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Format/Format.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Rewrite/Core/Rewriter.h"
-#include "clang/Tooling/Core/Replacement.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_os_ostream.h"
+#include <iterator>
 
 namespace clang {
 namespace tooling {
 
 static const char * const InvalidLocation = "";
 
 Replacement::Replacement()
   : FilePath(InvalidLocation) {}
std::string applyAllReplacements(StringRef Code, const Replacements &Replaces) {
   }
   std::string Result;
   llvm::raw_string_ostream OS(Result);
   Rewrite.getEditBuffer(ID).write(OS);
   OS.flush();
   return Result;
 }
 
+tooling::Replacements formatReplacements(StringRef Code,
+                                         const tooling::Replacements &Replaces,
+                                         const format::FormatStyle &Style) {
+  if (Replaces.empty()) return Replacements();
+
+  std::string NewCode = applyAllReplacements(Code, Replaces);
+  std::vector<tooling::Range> ChangedRanges =
+      calculateChangedRangesInFile(Replaces);
+  StringRef FileName = Replaces.begin()->getFilePath();
+  tooling::Replacements FormatReplaces =
+      format::reformat(Style, NewCode, ChangedRanges, FileName);
+
+  tooling::Replacements MergedReplacements =
+      mergeReplacements(Replaces, FormatReplaces);
+  return MergedReplacements;
+}
+
+std::vector<Range> calculateChangedRangesInFile(const Replacements &Replaces) {
+  std::vector<Range> ChangedRanges;
+  int Shift = 0;
+  for (const tooling::Replacement &R : Replaces) {
+    unsigned Offset = R.getOffset() + Shift;
+    unsigned Length = R.getReplacementText().size();
+    Shift += Length - R.getLength();
+    ChangedRanges.push_back(tooling::Range(Offset, Length));
+  }
+  return ChangedRanges;
+}
+
+std::string applyAllReplacementsAndFormat(StringRef Code,
+                                          const Replacements &Replaces,
+                                          const format::FormatStyle &Style) {
+  Replacements NewReplacements = formatReplacements(Code, Replaces, Style);
+  if (NewReplacements.empty())
+    return Code; // Exit early to avoid overhead in `applyAllReplacements`.
+  return applyAllReplacements(Code, NewReplacements);
+}
+
 namespace {
 // Represents a merged replacement, i.e. a replacement consisting of multiple
 // overlapping replacements from 'First' and 'Second' in mergeReplacements.
 //
 // Position projection:
 // Offsets and lengths of the replacements can generally refer to two different
 // coordinate spaces. Replacements from 'First' refer to the original text
 // whereas replacements from 'Second' refer to the text after applying 'First'.
 //
 // MergedReplacement always operates in the coordinate space of the original
 // text, i.e. transforms elements from 'Second' to take into account what was
 // changed based on the elements from 'First'.
 //
 // We can correctly calculate this projection as we look at the replacements in
 // order of strictly increasing offsets.
 //
 // Invariants:
 // * We always merge elements from 'First' into elements from 'Second' and vice
 //   versa. Within each set, the replacements are non-overlapping.
 // * We only extend to the right, i.e. merge elements with strictly increasing
 //   offsets.
 class MergedReplacement {
 public:
   MergedReplacement(const Replacement &R, bool MergeSecond, int D)
       : MergeSecond(MergeSecond), Delta(D), FilePath(R.getFilePath()),
         Offset(R.getOffset() + (MergeSecond ? 0 : Delta)), Length(R.getLength()),
         Text(R.getReplacementText()) {
     Delta += MergeSecond ? 0 : Text.size() - Length;
     DeltaFirst = MergeSecond ? Text.size() - Length : 0;
   }
 
   // Merges the next element 'R' into this merged element. As we always merge
   // from 'First' into 'Second' or vice versa, the MergedReplacement knows what
-  // set the next element is coming from. 
+  // set the next element is coming from.
   void merge(const Replacement &R) {
     if (MergeSecond) {
       unsigned REnd = R.getOffset() + Delta + R.getLength();
       unsigned End = Offset + Text.size();
       if (REnd > End) {
         Length += REnd - End;
         MergeSecond = false;
       }

unittests/Tooling/CMakeLists.txt

add_clang_unittest(ToolingTests
   ReplacementsYamlTest.cpp
   QualTypeNamesTest.cpp
   )
 
 target_link_libraries(ToolingTests
   clangAST
   clangASTMatchers
   clangBasic
+  clangFormat
   clangFrontend
   clangLex
   clangRewrite
   clangTooling
   clangToolingCore
   )

unittests/Tooling/RefactoringTest.cpp

 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclGroup.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Format/Format.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/Path.h"
Replaces.insert(Replacement(Context.Sources, SourceLocation(),
                               5, "2"));
   Replaces.insert(Replacement(Context.Sources, Context.getLocation(IDz, 1, 1),
                               4, "z"));
   EXPECT_FALSE(applyAllReplacements(Replaces, Context.Rewrite));
   EXPECT_EQ("a", Context.getRewrittenText(IDa));
   EXPECT_EQ("z", Context.getRewrittenText(IDz));
 }
 
+TEST_F(ReplacementTest, FormatCodeAfterReplacements) {
+  // Column limit is 20.
+  std::string Code = "Type *a =\n"
+                     "    new Type();\n"
+                     "g(iiiii, 0, jjjjj,\n"
+                     "  0, kkkkk, 0, mm);\n"
+                     "int  bad     = format   ;";
+  std::string Expected = "auto a = new Type();\n"
+                         "g(iiiii, nullptr,\n"
+                         "  jjjjj, nullptr,\n"
+                         "  kkkkk, nullptr,\n"
+                         "  mm);\n"
+                         "int  bad     = format   ;";
+  FileID ID = Context.createInMemoryFile("format.cpp", Code);
+  Replacements Replaces;
+  Replaces.insert(
+      Replacement(Context.Sources, Context.getLocation(ID, 1, 1), 6, "auto "));
+  Replaces.insert(Replacement(Context.Sources, Context.getLocation(ID, 3, 10),
+                              1, "nullptr"));
+  Replaces.insert(Replacement(Context.Sources, Context.getLocation(ID, 4, 3), 1,
+                              "nullptr"));
+  Replaces.insert(Replacement(Context.Sources, Context.getLocation(ID, 4, 13),
+                              1, "nullptr"));
+
+  format::FormatStyle Style = format::getLLVMStyle();
+  Style.ColumnLimit = 20; // Set column limit to 20 to increase readibility.
+  EXPECT_EQ(Expected, applyAllReplacementsAndFormat(Code, Replaces, Style));
+}
+
 TEST(ShiftedCodePositionTest, FindsNewCodePosition) {
   Replacements Replaces;
   Replaces.insert(Replacement("", 0, 1, ""));
   Replaces.insert(Replacement("", 4, 3, " "));
   // Assume ' int   i;' is turned into 'int i;' and cursor is located at '|'.
   EXPECT_EQ(0u, shiftedCodePosition(Replaces, 0)); // |int   i;
   EXPECT_EQ(0u, shiftedCodePosition(Replaces, 1)); //  |nt   i;
   EXPECT_EQ(1u, shiftedCodePosition(Replaces, 2)); //  i|t   i;

 
 TEST(Range, contains) {
   EXPECT_TRUE(Range(0, 10).contains(Range(0, 10)));
   EXPECT_TRUE(Range(0, 10).contains(Range(2, 6)));
   EXPECT_FALSE(Range(2, 6).contains(Range(0, 10)));
   EXPECT_FALSE(Range(0, 10).contains(Range(0, 11)));
 }
 
+TEST(Range, CalculateRangesOfReplacements) {
+  // Before: aaaabbbbbbz
+  // After : bbbbbbzzzzzzoooooooooooooooo
+  Replacements Replaces;
+  Replaces.insert(Replacement("foo", 0, 4, ""));
+  Replaces.insert(Replacement("foo", 10, 1, "zzzzzz"));
+  Replaces.insert(Replacement("foo", 11, 0, "oooooooooooooooo"));
+
+  std::vector<Range> Ranges = calculateChangedRangesInFile(Replaces);
+
+  EXPECT_EQ(3, Ranges.size());
+  EXPECT_TRUE(Ranges[0].getOffset() == 0);
+  EXPECT_TRUE(Ranges[0].getLength() == 0);
+  EXPECT_TRUE(Ranges[1].getOffset() == 6);
+  EXPECT_TRUE(Ranges[1].getLength() == 6);
+  EXPECT_TRUE(Ranges[2].getOffset() == 12);
+  EXPECT_TRUE(Ranges[2].getLength() == 16);
+}
+
 TEST(DeduplicateTest, removesDuplicates) {
   std::vector<Replacement> Input;
   Input.push_back(Replacement("fileA", 50, 0, " foo "));
   Input.push_back(Replacement("fileA", 10, 3, " bar "));
   Input.push_back(Replacement("fileA", 10, 2, " bar ")); // Length differs
   Input.push_back(Replacement("fileA", 9,  3, " bar ")); // Offset differs
   Input.push_back(Replacement("fileA", 50, 0, " foo ")); // Duplicate
   Input.push_back(Replacement("fileA", 51, 3, " bar "));