[PATCH] clang-apply-replacements: Add code-formatting post processing step

[Guillaume Papin]

  A question. Parts of this code is duplicated from clang-modernize, is the intent for clang-modernize to use this inteface after? I think you should consider adding the ChangedRangesTest unittests that currently reside in unittests/clang-modernize/FileOverridesTest.cpp


================
Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:324
@@ +323,3 @@
+
+  // cleanups unecessary ranges to finish
+  coalesceRanges(ChangedRanges);
----------------
Maybe correct the mistakes in this comment or simply remove it since the code that follows is self-explanatory.

================
Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:328-330
@@ +327,5 @@
+
+void reformatRanges(const llvm::StringRef FileName, const RangeVec &Ranges,
+                    clang::SourceManager &SM, clang::format::FormatStyle Style,
+                    std::vector<tooling::Replacement> &Replacements) {
+  const clang::FileEntry *Entry = SM.getFileManager().getFile(FileName);
----------------
nit: I think we can get rid of the extra qualifications `clang::` and `llvm::`.

================
Comment at: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp:37-42
@@ -35,1 +36,8 @@
 
+static cl::opt<std::string> FormatStyleOpt(
+    "format-style",
+    cl::desc("Coding style to use on the replacements, either a builtin style\n"
+             "or a YAML config file (see: clang-format -dump-config).\n"
+             "Currently supports 4 builtins style:\n"
+             "  LLVM, Google, Chromium, Mozilla.\n"),
+    cl::value_desc("string"));
----------------
See: https://cpp11-migrate.atlassian.net/browse/CM-103


================
Comment at: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp:81
@@ +80,3 @@
+    Style.Standard = clang::format::FormatStyle::LS_Cpp11;
+    return true;
+  }
----------------
Not needed.

================
Comment at: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp:80
@@ +79,3 @@
+    // force mode to C++11
+    Style.Standard = clang::format::FormatStyle::LS_Cpp11;
+    return true;
----------------
Is it still relevant? Maybe we can use `FormatStyle::LS_Auto` now. 
http://clang.llvm.org/doxygen/structclang_1_1format_1_1FormatStyle.html#a54639a373da7c4e1e7f4296b9c2adc39

================
Comment at: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp:109
@@ +108,3 @@
+                        Rewriter &FormattingRewriter) {
+
+  SourceManager &SM = FormattingRewriter.getSourceMgr();
----------------
nit: why the empty newline to start?

================
Comment at: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp:145
@@ +144,3 @@
+  format::FormatStyle FormatStyle;
+  if (FormatStyleOpt.getNumOccurrences() > 0) {
+    if (!handleFormatStyle(argv[0], FormatStyle, Diagnostics))
----------------
This line is redundant with `handleFormatStyle()`'s first line.

================
Comment at: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp:213-221
@@ +212,11 @@
+    assert(Entry && "expected an existing file");
+    FileID ID = FormattingSM.translateFile(Entry);
+    const RewriteBuffer *FormattedBuffer = 0;
+    if (!ID.isInvalid())
+      FormattedBuffer = FormattingRewriter.getRewriteBufferFor(ID);
+
+    if (FormattedBuffer)
+      FormattedBuffer->write(FileStream);
+    else
+      BufferI->second.write(FileStream);
+  }
----------------
Eventually:
```
  const RewriteBuffer *Buffer = &BufferI->second;
  FileID ID = FormattingSM.translateFile(Entry);

  if (!ID.isInvalid())
    Buffer = FormattingRewriter.getRewriteBufferFor(ID);
  Buffer->write(FileStream);
```

================
Comment at: test/clang-apply-replacements/format.cpp:3
@@ +2,3 @@
+//
+// yes.cpp requires formatting after replacements are applied. no.cpp does not.
+//
----------------
Is the no.cpp test really needed?


http://llvm-reviews.chandlerc.com/D1730