[clang-tidy] Format code around applied fixes

Summary:
Add -format option (disabled by default for now) to trigger formatting
of replacements.

Reviewers: ioeric

Reviewed By: ioeric

Subscribers: kimgr, malcolm.parsons, JDevlieghere, cfe-commits

Differential Revision: https://reviews.llvm.org/D30564

llvm-svn: 296864

6 files changed, 85 insertions, 15 deletions

diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp
index 75aaefd56a1..2da9e142957 100644
--- a/clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -183,7 +183,6 @@ public:
   }
 
   void Finish() {
-    // FIXME: Run clang-format on changes.
     if (ApplyFixes && TotalFixes > 0) {
       Rewriter Rewrite(SourceMgr, LangOpts);
       for (const auto &FileAndReplacements : FileReplacements) {
@@ -197,19 +196,28 @@ public:
           continue;
         }
         StringRef Code = Buffer.get()->getBuffer();
-        auto Style = format::getStyle("file", File, FormatStyle);
+        auto Style = format::getStyle(FormatStyle, File, "none");
         if (!Style) {
           llvm::errs() << llvm::toString(Style.takeError()) << "\n";
           continue;
         }
-        llvm::Expected<Replacements> CleanReplacements =
+        llvm::Expected<tooling::Replacements> Replacements =
             format::cleanupAroundReplacements(Code, FileAndReplacements.second,
                                               *Style);
-        if (!CleanReplacements) {
-          llvm::errs() << llvm::toString(CleanReplacements.takeError()) << "\n";
+        if (!Replacements) {
+          llvm::errs() << llvm::toString(Replacements.takeError()) << "\n";
           continue;
         }
-        if (!tooling::applyAllReplacements(CleanReplacements.get(), Rewrite)) {
+        if (llvm::Expected<tooling::Replacements> FormattedReplacements =
+                format::formatReplacements(Code, *Replacements, *Style)) {
+          Replacements = std::move(FormattedReplacements);
+          if (!Replacements)
+            llvm_unreachable("!Replacements");
+        } else {
+          llvm::errs() << llvm::toString(FormattedReplacements.takeError())
+                       << ". Skipping formatting.\n";
+        }
+        if (!tooling::applyAllReplacements(Replacements.get(), Rewrite)) {
           llvm::errs() << "Can't apply replacements for file " << File << "\n";
         }
       }
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index b6684d023a2..4dc86e1aa39 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -60,7 +60,7 @@ appearance in the list. Globs without '-'
 prefix add checks with matching names to the
 set, globs with the '-' prefix remove checks
 with matching names from the set of enabled
-checks.  This option's value is appended to the
+checks. This option's value is appended to the
 value of the 'Checks' option in .clang-tidy
 file, if any.
 )"),
@@ -120,12 +120,20 @@ well.
 )"),
                                cl::init(false), cl::cat(ClangTidyCategory));
 
-static cl::opt<std::string> FormatStyle("style", cl::desc(R"(
-Fallback style for reformatting after inserting fixes
-if there is no clang-format config file found.
+static cl::opt<std::string> FormatStyle("format-style", cl::desc(R"(
+Style for formatting code around applied fixes:
+  - 'none' (default) turns off formatting
+  - 'file' (literally 'file', not a placeholder)
+    uses .clang-format file in the closest parent
+    directory
+  - '{ <json> }' specifies options inline, e.g.
+    -format-style='{BasedOnStyle: llvm, IndentWidth: 8}'
+  - 'llvm', 'google', 'webkit', 'mozilla'
+See clang-format documentation for the up-to-date
+information about formatting styles and options.
 )"),
-                                        cl::init("llvm"),
-                                        cl::cat(ClangTidyCategory));
+                                   cl::init("none"),
+                                   cl::cat(ClangTidyCategory));
 
 static cl::opt<bool> ListChecks("list-checks", cl::desc(R"(
 List all enabled checks and exit. Use with
@@ -189,7 +197,7 @@ code with clang-apply-replacements.
                                         cl::cat(ClangTidyCategory));
 
 static cl::opt<bool> Quiet("quiet", cl::desc(R"(
-Run clang-tidy in quiet mode.  This suppresses
+Run clang-tidy in quiet mode. This suppresses
 printing statistics about ignored warnings and
 warnings treated as errors if the respective
 options are specified.
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 323bb7446b6..a13c22743a3 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -87,6 +87,9 @@ Improvements to clang-tidy
   Finds and replaces explicit calls to the constructor in a return statement by
   a braced initializer list so that the return type is not needlessly repeated.
 
+- Support clang-formatting of the code around applied fixes (``-format-style``
+  command-line option).
+
 Improvements to include-fixer
 -----------------------------
 
diff --git a/clang-tools-extra/docs/clang-tidy/index.rst b/clang-tools-extra/docs/clang-tidy/index.rst
index 7ca66c62d1d..c8fec37cdbe 100644
--- a/clang-tools-extra/docs/clang-tidy/index.rst
+++ b/clang-tools-extra/docs/clang-tidy/index.rst
@@ -158,6 +158,17 @@ An overview of all the command-line options:
                                    errors were found. If compiler errors have
                                    attached fix-its, clang-tidy will apply them as
                                    well.
+    -format-style=<string>       -
+                                   Style for formatting code around applied fixes:
+                                     - 'none' (default) turns off formatting
+                                     - 'file' (literally 'file', not a placeholder)
+                                       uses .clang-format file in the closest parent
+                                       directory
+                                     - '{ <json> }' specifies options inline, e.g.
+                                       -format-style='{BasedOnStyle: llvm, IndentWidth: 8}'
+                                     - 'llvm', 'google', 'webkit', 'mozilla'
+                                   See clang-format documentation for the up-to-date
+                                   information about formatting styles and options.
     -header-filter=<string>      -
                                    Regular expression matching the names of the
                                    headers to output diagnostics from. Diagnostics
@@ -179,6 +190,11 @@ An overview of all the command-line options:
                                    List all enabled checks and exit. Use with
                                    -checks=* to list all available checks.
     -p=<string>                  - Build path
+    -quiet                       -
+                                   Run clang-tidy in quiet mode. This suppresses
+                                   printing statistics about ignored warnings and
+                                   warnings treated as errors if the respective
+                                   options are specified.
     -style=<string>              -
                                    Fallback style for reformatting after inserting fixes
                                    if there is no clang-format config file found.
@@ -558,10 +574,10 @@ and once with the directive prefix set to ``CHECK-FIXES``, running
 against the fixed code (i.e., the code after generated fix-its are
 applied). In particular, ``CHECK-FIXES:`` can be used to check
 that code was not modified by fix-its, by checking that it is present
-unchanged in the fixed code.  The full set of `FileCheck`_ directives
+unchanged in the fixed code. The full set of `FileCheck`_ directives
 is available (e.g., ``CHECK-MESSAGES-SAME:``, ``CHECK-MESSAGES-NOT:``), though
 typically the basic ``CHECK`` forms (``CHECK-MESSAGES`` and ``CHECK-FIXES``)
-are sufficient for clang-tidy tests.  Note that the `FileCheck`_
+are sufficient for clang-tidy tests. Note that the `FileCheck`_
 documentation mostly assumes the default prefix (``CHECK``), and hence
 describes the directive as ``CHECK:``, ``CHECK-SAME:``, ``CHECK-NOT:``, etc.
 Replace ``CHECK`` by either ``CHECK-FIXES`` or ``CHECK-MESSAGES`` for
diff --git a/clang-tools-extra/test/clang-tidy/clean-up-code.cpp b/clang-tools-extra/test/clang-tidy/clean-up-code.cpp
index 96b3a45178b..15873ed168d 100644
--- a/clang-tools-extra/test/clang-tidy/clean-up-code.cpp
+++ b/clang-tools-extra/test/clang-tidy/clean-up-code.cpp
@@ -1,4 +1,6 @@
 // RUN: %check_clang_tidy %s misc-unused-using-decls %t
+// RUN: %check_clang_tidy %s misc-unused-using-decls %t -- -format-style=none --
+// RUN: %check_clang_tidy %s misc-unused-using-decls %t -- -format-style=llvm --
 namespace a { class A {}; }
 namespace b {
 using a::A;
diff --git a/clang-tools-extra/test/clang-tidy/readability-braces-around-statements-format.cpp b/clang-tools-extra/test/clang-tidy/readability-braces-around-statements-format.cpp
new file mode 100644
index 00000000000..cc1ce9226bf
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/readability-braces-around-statements-format.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s readability-braces-around-statements %t -- -format-style="{IndentWidth: 3}" --
+
+void do_something(const char *) {}
+
+bool cond(const char *) {
+  return false;
+}
+
+void test() {
+  if (cond("if0") /*comment*/) do_something("same-line");
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: statement should be inside braces
+  // CHECK-FIXES: {{^}}   if (cond("if0") /*comment*/) {{{$}}
+  // CHECK-FIXES-NEXT: {{^}}      do_something("same-line");{{$}}
+  // CHECK-FIXES-NEXT: {{^}}   }{{$}}
+
+  if (1) while (2) if (3) for (;;) do ; while(false) /**/;/**/
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: statement should be inside braces
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning: statement should be inside braces
+  // CHECK-MESSAGES: :[[@LINE-3]]:26: warning: statement should be inside braces
+  // CHECK-MESSAGES: :[[@LINE-4]]:35: warning: statement should be inside braces
+  // CHECK-MESSAGES: :[[@LINE-5]]:38: warning: statement should be inside braces
+  // CHECK-FIXES:      {{^}}   if (1) {{{$}}
+  // CHECK-FIXES-NEXT: {{^}}      while (2) {
+  // CHECK-FIXES-NEXT: {{^}}         if (3) {
+  // CHECK-FIXES-NEXT: {{^}}            for (;;) {
+  // CHECK-FIXES-NEXT: {{^}}               do {
+  // CHECK-FIXES-NEXT: {{^}}                  ;
+  // CHECK-FIXES-NEXT: {{^}}               } while (false) /**/; /**/
+  // CHECK-FIXES-NEXT: {{^}}            }
+  // CHECK-FIXES-NEXT: {{^}}         }
+  // CHECK-FIXES-NEXT: {{^}}      }
+  // CHECK-FIXES-NEXT: {{^}}   }
+}