Added applyAllReplacementsAndFormat that works for multiple files.

[ioeric]

Summary

Added applyAllReplacementsAndFormat that works for multiple files.

[ioeric]

• Fixed commenting issue due to merge.

[ioeric]

• Added getOrCreateFileID interface in SourceManager.

[djasper]

I don't think this is the right abstraction:

• Formatting so far is fundamentally per file. Grouping replacements per file is something that can be done outside of libFormat.
• The logic to sort and deduplicate Replacements is important independent of whether formatting is done on them. So this should also go outside of libFormat.

[klimek]

Ok, after thinking about it a bit, here is a new plan:

• get rid of applyAllReplacementsAndFormat (in all its forms)
• implement a formatRewrittenCode(Rewriter&, ...); as Daniel wants libFormat to stay file focused, we can put that into non-core libTooling; it'll figure out what to reformat from the rewriter state

That should leave us with a set of single purpose functions we can nicely combine.

[ioeric]

• Removed all applyAllReplacementsAndFormat(*); added formatRewrittenCode in clangTooling.

[ioeric]

• Removed definitions that were forgotten.

[ioeric]

Start a new revision to put applyAllReplacementsAndFormat into non-core libTooling .

include/clang/Basic/SourceManager.h

public:
   FileID createFileID(std::unique_ptr<llvm::MemoryBuffer> Buffer,
                       SrcMgr::CharacteristicKind FileCharacter = SrcMgr::C_User,
                       int LoadedID = 0, unsigned LoadedOffset = 0,
                       SourceLocation IncludeLoc = SourceLocation()) {
     return createFileID(createMemBufferContentCache(std::move(Buffer)),
                         IncludeLoc, FileCharacter, LoadedID, LoadedOffset);
   }
 
+  /// \brief Get the FileID for \p SourceFile if it exists. Otherwise, create a
+  /// new FileID for the \p SourceFile.
+  FileID getOrCreateFileID(const FileEntry *SourceFile,
+                           SrcMgr::CharacteristicKind FileCharacter) {
+    FileID ID = translateFile(SourceFile);
+    return ID.isValid() ? ID : createFileID(SourceFile, SourceLocation(),
+                                            FileCharacter);
+  }
+
   /// \brief Return a new SourceLocation that encodes the
   /// fact that a token from SpellingLoc should actually be referenced from
   /// ExpansionLoc, and that it represents the expansion of a macro argument
   /// into the function-like macro body.
   SourceLocation createMacroArgExpansionLoc(SourceLocation Loc,
                                             SourceLocation ExpansionLoc,
                                             unsigned TokLength);
 

include/clang/Format/Format.h

tooling::Replacements sortIncludes(const FormatStyle &Style, StringRef Code,
                                    unsigned *Cursor = nullptr);
 
 /// \brief Returns the replacements corresponding to applying and formatting
 /// \p Replaces.
 tooling::Replacements formatReplacements(StringRef Code,
                                          const tooling::Replacements &Replaces,
                                          const FormatStyle &Style);
 
-/// \brief In addition to applying all replacements in \p Replaces to \p Code,
-/// 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 with all replacements applied and formatted, if
-/// successful. An empty string otherwise.
-///
-/// See also "include/clang/Tooling/Core/Replacements.h".
-std::string applyAllReplacementsAndFormat(StringRef Code,
-                                          const tooling::Replacements &Replaces,
-                                          const FormatStyle &Style);
-
 /// \brief Reformats the given \p Ranges in the file \p ID.
 ///
 /// Each range is extended on either end to its next bigger logic unit, i.e.
 /// everything that might influence its formatting or might be influenced by its
 /// formatting.
 ///
 /// Returns the ``Replacements`` necessary to make all \p Ranges comply with
 /// \p Style.

include/clang/Tooling/Core/Replacement.h

 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLING_CORE_REPLACEMENT_H
 #define LLVM_CLANG_TOOLING_CORE_REPLACEMENT_H
 
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/StringRef.h"
+#include <map>
 #include <set>
 #include <string>
 #include <vector>
 
 namespace clang {
 
+class FileEntry;
+class FileManager;
 class Rewriter;
 
 namespace tooling {
 
 /// \brief A source range independent of the \c SourceManager.
 class Range {
 public:
   Range() : Offset(0), Length(0) {}
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 Calculate the ranges in a single file that are affected by the
+/// \brief Calculates 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);
+std::vector<Range> calculateChangedRangesInFile(const Replacements &Replaces);
+
+typedef std::map<const FileEntry *, Replacements>
+    FileToReplacementsMap;
+
+/// \brief Groups a random set of replacements by file path. Replacements
+/// related to the same file entry are put into the same vector.
+FileToReplacementsMap groupReplacementsByFile(const Replacements &Replaces,
+                                              FileManager &Files);
 
 /// \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>

include/clang/Tooling/ReplacementsFormat.h

+//===-- ReplacementsFormat.h -- Format code changed by replacements -*- C++ -*//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// \brief This file defines functions that reformat code changed by
+/// replacemnts.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLING_REPLACEMENTSFORMAT_H
+#define LLVM_CLANG_TOOLING_REPLACEMENTSFORMAT_H
+
+#include "clang/Format/Format.h"
+#include "clang/Tooling/Core/Replacement.h"
+
+namespace clang {
+namespace tooling {
+
+/// \brief Reformat code changed by \p Replaces in \p Rewrite.
+///
+/// \pre \p Replaces must have applied on \p Rewrite, and no other replacements
+/// apply after that. Also, \p Replaces must be conflict-free.
+///
+/// Code reformatting happens independently of the success of other files.
+///
+/// \returns true if we successfully reformatted all changed code in \p Rewrite.
+/// false otherwise.
+bool formatRewrittenCode(Rewriter &Rewrite, const Replacements &Replaces,
+                         const format::FormatStyle &Style);
+
+} // namespace tooling
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLING_REPLACEMENTSFORMAT_H

lib/Format/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangFormat
   BreakableToken.cpp
   ContinuationIndenter.cpp
   Format.cpp
   FormatToken.cpp
   TokenAnnotator.cpp
   UnwrappedLineFormatter.cpp
   UnwrappedLineParser.cpp
   WhitespaceManager.cpp
 
   LINK_LIBS
   clangBasic
   clangLex
+  clangRewrite
   clangToolingCore
   )

lib/Format/Format.cpp

 #include "TokenAnnotator.h"
 #include "UnwrappedLineFormatter.h"
 #include "UnwrappedLineParser.h"
 #include "WhitespaceManager.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
+#include "clang/Rewrite/Core/Rewriter.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/YAMLTraits.h"
 #include <queue>
 #include <string>
tooling::Replacements formatReplacements(StringRef Code,
   tooling::Replacements FormatReplaces =
       reformat(Style, NewCode, ChangedRanges, FileName);
 
   tooling::Replacements MergedReplacements =
       mergeReplacements(Replaces, FormatReplaces);
   return MergedReplacements;
 }
 
-std::string applyAllReplacementsAndFormat(StringRef Code,
-                                          const tooling::Replacements &Replaces,
-                                          const FormatStyle &Style) {
-  tooling::Replacements NewReplacements =
-      formatReplacements(Code, Replaces, Style);
-  if (NewReplacements.empty())
-    return Code; // Exit early to avoid overhead in `applyAllReplacements`.
-  return applyAllReplacements(Code, NewReplacements);
-}
-
 tooling::Replacements reformat(const FormatStyle &Style,
                                SourceManager &SourceMgr, FileID ID,
                                ArrayRef<CharSourceRange> Ranges,
                                bool *IncompleteFormat) {
   FormatStyle Expanded = expandPresets(Style);
   if (Expanded.DisableFormat)
     return tooling::Replacements();
   Formatter formatter(Expanded, SourceMgr, ID, Ranges);

lib/Tooling/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_subdirectory(Core)
 
 add_clang_library(clangTooling
   ArgumentsAdjusters.cpp
   CommonOptionsParser.cpp
   CompilationDatabase.cpp
   FileMatchTrie.cpp
   JSONCompilationDatabase.cpp
   Refactoring.cpp
   RefactoringCallbacks.cpp
+  ReplacementsFormat.cpp
   Tooling.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangDriver
   clangFrontend
+  clangFormat
   clangLex
   clangRewrite
   clangToolingCore
   )

lib/Tooling/Core/Replacement.cpp

bool Replacement::isApplicable() const {
   return FilePath != InvalidLocation;
 }
 
 bool Replacement::apply(Rewriter &Rewrite) const {
   SourceManager &SM = Rewrite.getSourceMgr();
   const FileEntry *Entry = SM.getFileManager().getFile(FilePath);
   if (!Entry)
     return false;
-  FileID ID;
-  // FIXME: Use SM.translateFile directly.
-  SourceLocation Location = SM.translateFileLineCol(Entry, 1, 1);
-  ID = Location.isValid() ?
-    SM.getFileID(Location) :
-    SM.createFileID(Entry, SourceLocation(), SrcMgr::C_User);
-  // FIXME: We cannot check whether Offset + Length is in the file, as
-  // the remapping API is not public in the RewriteBuffer.
+
+  FileID ID = SM.getOrCreateFileID(Entry, SrcMgr::C_User);
   const SourceLocation Start =
     SM.getLocForStartOfFile(ID).
     getLocWithOffset(ReplacementRange.getOffset());
   // ReplaceText returns false on success.
   // ReplaceText only fails if the source location is not a file location, in
   // which case we already returned false earlier.
   bool RewriteSucceeded = !Rewrite.ReplaceText(
       Start, ReplacementRange.getLength(), ReplacementText);
for (std::vector<Replacement>::const_iterator I = Replaces.begin(),
     } else {
       Result = false;
     }
   }
   return Result;
 }
 
 std::string applyAllReplacements(StringRef Code, const Replacements &Replaces) {
+  if (Replaces.empty()) return Code;
+
   IntrusiveRefCntPtr<vfs::InMemoryFileSystem> InMemoryFileSystem(
       new vfs::InMemoryFileSystem);
   FileManager Files(FileSystemOptions(), InMemoryFileSystem);
   DiagnosticsEngine Diagnostics(
       IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs),
       new DiagnosticOptions);
   SourceManager SourceMgr(Diagnostics, Files);
   Rewriter Rewrite(SourceMgr, LangOptions());
private:
   // as the element is only extended to the right.
   const StringRef FilePath;
   const unsigned Offset;
   unsigned Length;
   std::string Text;
 };
 } // namespace
 
+FileToReplacementsMap
+groupReplacementsByFile(const Replacements &Replaces, FileManager &Files) {
+  FileToReplacementsMap FileToReplaces;
+  for (const auto &Replace : Replaces) {
+    const FileEntry *Entry = Files.getFile(Replace.getFilePath());
+    assert(Entry && "Expected an existing file.");
+    FileToReplaces[Entry].insert(Replace);
+  }
+  return FileToReplaces;
+}
+
 Replacements mergeReplacements(const Replacements &First,
                                const Replacements &Second) {
   if (First.empty() || Second.empty())
     return First.empty() ? Second : First;
 
   // Delta is the amount of characters that replacements from 'Second' need to
   // be shifted so that their offsets refer to the original text.
   int Delta = 0;

lib/Tooling/ReplacementsFormat.cpp

+//===--- ReplacementFormat.cpp - Format code changed by replacemnts -------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// \file
+// \brief This file implements functions that reformat code changed by
+// replacements.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Tooling/ReplacementsFormat.h"
+
+#include "clang/Basic/SourceManager.h"
+#include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/Tooling/Core/Replacement.h"
+
+namespace clang {
+namespace tooling {
+
+/// \brief Reformat code changed by \p Replaces in \p Rewrite.
+bool formatRewrittenCode(Rewriter &Rewrite, const Replacements &Replaces,
+                         const format::FormatStyle &Style) {
+  SourceManager &SM = Rewrite.getSourceMgr();
+  FileManager &Files = SM.getFileManager();
+
+  tooling::FileToReplacementsMap FileToReplaces =
+      groupReplacementsByFile(Replaces, Files);
+
+  bool Result = true;
+  for (auto &FileAndReplaces : FileToReplaces) {
+    std::vector<Range> Ranges =
+        calculateChangedRangesInFile(FileAndReplaces.second);
+
+    const FileEntry *Entry = FileAndReplaces.first;
+    assert(Entry != nullptr && "Invalid file entry!");
+
+    FileID ID = SM.translateFile(Entry);
+    // ID must be valid since `Replaces` have applied.
+    assert(ID.isValid() && "Expect valid FileID!");
+
+    // Retrive the changed code from `Rewrite`, reformat the retrieved code, and
+    // override the `RewriteBuffer` corresponding to the current file in
+    // `Rewrite`.
+    // FIXME: this seems to be the only way to work on a modified
+    // `RewriteBuffer` since the changes of the replacements do not reflect on
+    // the `SourceManager`. Even if we override content of the file entry in
+    // `SourceManager`, it still doesn't work since Source Location's ID will
+    // become inconsistent. To cope with that, we need to create a new FileID
+    // for the overriden file entry, which will leave 2 FileIDs for the same
+    // `FileEntry` in the `SourceManager`. But when we reference FileID for the
+    // same `FileEntry` in `applyAllReplacements`, the current implementation
+    // will get the old FileID(the first occurrence), which means that the
+    // inconsistency of SourceLocation IDs still exists.
+    auto *Buffer = Rewrite.getRewriteBufferFor(ID);
+    std::string Code(Buffer->begin(), Buffer->end());
+
+    Replacements NewReplaces =
+        format::reformat(Style, Code, Ranges, Entry->getName());
+    std::string NewCode = applyAllReplacements(Code, NewReplaces);
+    if (NewCode.empty()) {
+      Result = false;
+      continue;
+    }
+    Rewrite.getEditBuffer(ID).Initialize(NewCode);
+  }
+  return Result;
+}
+
+} // namespace tooling
+} // namespace clang

unittests/Format/FormatTest.cpp

Replaces.insert(tooling::Replacement(
       Context.Sources, Context.getLocation(ID, 3, 10), 1, "nullptr"));
   Replaces.insert(tooling::Replacement(
       Context.Sources, Context.getLocation(ID, 4, 3), 1, "nullptr"));
   Replaces.insert(tooling::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));
+  EXPECT_EQ(Expected, applyAllReplacements(
+                          Code, formatReplacements(Code, Replaces, Style)));
 }
 
 } // end namespace
 } // end namespace format
 } // end namespace clang

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/ReplacementsFormat.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/Path.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace tooling {
 
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, MultipleFilesReplaceAndFormat) {
+  // Column limit is 20.
+  std::string Code1 = "Long *a =\n"
+                      "    new Long();\n"
+                      "long x = 1;";
+  std::string Expected1 = "auto a = new Long();\n"
+                          "long x =\n"
+                          "    12345678901;";
+  std::string Code2 = "int x = 123;\n"
+                      "int y = 0;";
+  std::string Expected2 = "int x =\n"
+                          "    1234567890123;\n"
+                          "int y = 10;";
+  FileID ID1 = Context.createInMemoryFile("format_1.cpp", Code1);
+  FileID ID2 = Context.createInMemoryFile("format_2.cpp", Code2);
+
+  tooling::Replacements Replaces;
+  // Scrambled the order of replacements.
+  Replaces.insert(tooling::Replacement(
+      Context.Sources, Context.getLocation(ID2, 1, 12), 0, "4567890123"));
+  Replaces.insert(tooling::Replacement(
+      Context.Sources, Context.getLocation(ID1, 1, 1), 6, "auto "));
+  Replaces.insert(tooling::Replacement(
+      Context.Sources, Context.getLocation(ID2, 2, 9), 1, "10"));
+  Replaces.insert(tooling::Replacement(
+      Context.Sources, Context.getLocation(ID1, 3, 10), 1, "12345678901"));
+
+  format::FormatStyle Style = format::getLLVMStyle();
+  Style.ColumnLimit = 20; // Set column limit to 20 to increase readibility.
+
+  EXPECT_TRUE(applyAllReplacements(Replaces, Context.Rewrite));
+  EXPECT_TRUE(formatRewrittenCode(Context.Rewrite, Replaces, Style));
+  EXPECT_EQ(Expected1, Context.getRewrittenText(ID1));
+  EXPECT_EQ(Expected2, Context.getRewrittenText(ID2));
+}
+
 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;