[clang-tidy] Format code around applied fixes

[alexfh]

Summary

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

[malcolm.parsons]

test/clang-tidy/readability-braces-around-statements-format.cpp

1 ↗

(On Diff #90440)

Will clang-tidy look for a .clang-format file?
Does this test rely on a .clang-format file being found / not found?

[alexfh]

Pacify llvm::Expected<> debug checks.

[alexfh]

test/clang-tidy/readability-braces-around-statements-format.cpp

1 ↗

(On Diff #90440)

Good question. No idea ;) Lemme check...

[kimgr]

docs/clang-tidy/index.rst

184 ↗

(On Diff #90442)

This looks like a separate patch?

[alexfh]

docs/clang-tidy/index.rst

184 ↗

(On Diff #90442)

I've just pasted up-to-date -help output here. This should have been done when the -quiet option was added.

[ioeric]

test/clang-tidy/readability-braces-around-statements-format.cpp

1 ↗

(On Diff #90440)

Yes, it will look for .clang-format file in the working directory and its parents. If there happens to be a .clang-format with different style, the test might fail. We might want to explicitly set the style or add a .clang-format file in the test directory.

[alexfh]

Replace the separate -format and -style options with -format-style (default is
'none', which means no formatting).

[malcolm.parsons]

In D30564#691441, @alexfh wrote:

Replace the separate -format and -style options with -format-style (default is
'none', which means no formatting).

Is there a style that means use my .clang-format file?

[alexfh]

Expanded -format-style option description. Run cleanup tests with different format styles, just in case.

[alexfh]

In D30564#691446, @malcolm.parsons wrote:

In D30564#691441, @alexfh wrote:

Replace the separate -format and -style options with -format-style (default is
'none', which means no formatting).

Is there a style that means use my .clang-format file?

Yes, -format-style=file, see the updated docs.

[alexfh]

Clarify the 'file' option a bit.

[alexfh]

PTAL

[malcolm.parsons]

LGTM.

[ioeric]

Lg

[ioeric]

clang-tidy/ClangTidy.cpp

214 ↗

(On Diff #90448)

Maybe still apply the replacements if formatting fails?

[alexfh]

Apply changes even when formatting fails.

clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp

for (auto Fix : FixLocations) {
       Diags.Report(Fix.first, Fix.second ? diag::note_fixit_applied
                                          : diag::note_fixit_failed);
     }
     for (const auto &Note : Error.Notes)
       reportNote(Note);
   }
 
   void Finish() {
-    // FIXME: Run clang-format on changes.
     if (ApplyFixes && TotalFixes > 0) {
       Rewriter Rewrite(SourceMgr, LangOpts);
       for (const auto &FileAndReplacements : FileReplacements) {
         StringRef File = FileAndReplacements.first();
         llvm::ErrorOr<std::unique_ptr<MemoryBuffer>> Buffer =
             SourceMgr.getFileManager().getBufferForFile(File);
         if (!Buffer) {
           llvm::errs() << "Can't get buffer for file " << File << ": "
                        << Buffer.getError().message() << "\n";
           // FIXME: Maybe don't apply fixes for other files as well.
           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";
         }
       }
       if (Rewrite.overwriteChangedFiles()) {
         llvm::errs() << "clang-tidy failed to apply suggested fixes.\n";
       } else {
         llvm::errs() << "clang-tidy applied " << AppliedFixes << " of "
                      << TotalFixes << " suggested fixes.\n";

clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp

 
 static cl::opt<std::string> Checks("checks", cl::desc(R"(
 Comma-separated list of globs with optional '-'
 prefix. Globs are processed in order of
 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.
 )"),
                                    cl::init(""), cl::cat(ClangTidyCategory));
 
 static cl::opt<std::string> WarningsAsErrors("warnings-as-errors", cl::desc(R"(
 Upgrades warnings to errors. Same format as
 '-checks'.

 static cl::opt<bool> FixErrors("fix-errors", cl::desc(R"(
 Apply suggested fixes even if compilation
 errors were found. If compiler errors have
 attached fix-its, clang-tidy will apply them as
 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"),
 
 static cl::opt<bool> ListChecks("list-checks", cl::desc(R"(
 List all enabled checks and exit. Use with
 -checks=* to list all available checks.
 )"),
                                 cl::init(false), cl::cat(ClangTidyCategory));
 
 static cl::opt<bool> ExplainConfig("explain-config", cl::desc(R"(

 YAML file to store suggested fixes in. The
 stored fixes can be applied to the input source
 code with clang-apply-replacements.
 )"),
                                         cl::value_desc("filename"),
                                         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.
 )"),
                            cl::init(false),
                            cl::cat(ClangTidyCategory));
 
 namespace clang {

clang-tools-extra/trunk/docs/ReleaseNotes.rst

- New `safety-no-assembler
   Finds uses of inline assembler.
 
 - New `modernize-return-braced-init-list
   <http://clang.llvm.org/extra/clang-tidy/checks/modernize-return-braced-init-list.html>`_ check
 
   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
 -----------------------------
 
 The improvements are...
 
 Improvements to modularize
 --------------------------
 
 The improvements are...

clang-tools-extra/trunk/docs/clang-tidy/index.rst

-fix                         -
                                    Apply suggested fixes. Without -fix-errors
                                    clang-tidy will bail out if any compilation
                                    errors were found.
     -fix-errors                  -
                                    Apply suggested fixes even if compilation
                                    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
                                    from the main file of each translation unit are
                                    always displayed.
                                    Can be used together with -line-filter.
                                    This option overrides the 'HeaderFilter' option
                                    in .clang-tidy file, if any.
     -line-filter=<string>        -
                                    List of files with line ranges to filter the
                                    warnings. Can be used together with
                                    -header-filter. The format of the list is a
                                    JSON array of objects:
                                      [
                                        {"name":"file1.cpp","lines":[[1,3],[5,7]]},
                                        {"name":"file2.h"}
                                      ]
     -list-checks                 -
                                    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.
     -system-headers              - Display the errors from system headers.
     -warnings-as-errors=<string> -
                                    Upgrades warnings to errors. Same format as
                                    '-checks'.
                                    This option's value is appended to the value of

 diagnostic messages and fix-its. It filters out ``CHECK`` lines from the test
 file, runs :program:`clang-tidy` and verifies messages and fixes with two
 separate `FileCheck`_ invocations: once with FileCheck's directive
 prefix set to ``CHECK-MESSAGES``, validating the diagnostic messages,
 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
 clang-tidy tests.
 
 An additional check enabled by ``check_clang_tidy.py`` ensures that
 if `CHECK-MESSAGES:` is used in a file then every warning or error
 must have an associated CHECK in that file.

clang-tools-extra/trunk/test/clang-tidy/clean-up-code.cpp

 // 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;
 }
 namespace c {}
 // CHECK-MESSAGES: :[[@LINE-3]]:10: warning: using decl 'A' is unused [misc-unused-using-decls]
 // CHECK-FIXES: {{^namespace a { class A {}; }$}}
 // CHECK-FIXES-NOT: namespace
 // CHECK-FIXES: {{^namespace c {}$}}
 // FIXME: cleanupAroundReplacements leaves whitespace. Otherwise we could just
 // check the next line.

clang-tools-extra/trunk/test/clang-tidy/readability-braces-around-statements-format.cpp

+// 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: {{^}}   }
+}