[clang-tidy] Added -fix-errors option

[alexfh]

Summary

Added -fix-errors option to allow applying fixes when compiler errors
are present. Without this flag -fix would bail out if there are compiler errors.
This is needed to avoid applying wrong fixes if Clang fails to recover from
compilation errors correctly.

[klimek]

clang-tidy/tool/ClangTidyMain.cpp

295–299

It seems strange to first have to compute DisableFixes based on FixErrors, and then use FixErrors again.
If we want FixErrors to imply Fix, why not have the if (FixErrors) Fix = true; before the whole block?

[alexfh]

Make the relation between -fix-errors and -fix more explicit. Slightly changed error messages.

[klimek]

clang-tidy/tool/ClangTidyMain.cpp

300–301

Why don't we directly do llvm::errs() ...; here and skip the rest of the function (perhaps apart from the stat printing and profile checking)?

[alexfh]

clang-tidy/tool/ClangTidyMain.cpp

300–301

We need to call handleErrors to display the errors in any case. We also need to print the stats and profile data, and only then the message that fixes had not been applied.

[klimek]

lg

clang-tidy/tool/ClangTidyMain.cpp

300–301

Ok, then I'd optionally do Fix && !DisableFixes below instead of using the flag like a global variable...

[alexfh]

clang-tidy/tool/ClangTidyMain.cpp

300–301

Done. Committed.

clang-tidy/tool/ClangTidyMain.cpp

HeaderFilter("header-filter",
                       "always displayed.\n"
                       "Can be used together with -line-filter.\n"
                       "This option overrides the value read from a\n"
                       ".clang-tidy file."),
              cl::init(""), cl::cat(ClangTidyCategory));
 
 static cl::opt<bool>
     SystemHeaders("system-headers",
-                  cl::desc("Display the errors from system headers"),
+                  cl::desc("Display the errors from system headers."),
                   cl::init(false), cl::cat(ClangTidyCategory));
 static cl::opt<std::string>
 LineFilter("line-filter",
            cl::desc("List of files with line ranges to filter the\n"
                     "warnings. Can be used together with\n"
                     "-header-filter. The format of the list is a JSON\n"
                     "array of objects:\n"
                     "  [\n"
                     "    {\"name\":\"file1.cpp\",\"lines\":[[1,3],[5,7]]},\n"
                     "    {\"name\":\"file2.h\"}\n"
                     "  ]"),
            cl::init(""), cl::cat(ClangTidyCategory));
 
-static cl::opt<bool> Fix("fix", cl::desc("Fix detected errors if possible."),
-                         cl::init(false), cl::cat(ClangTidyCategory));
+static cl::opt<bool>
+    Fix("fix", cl::desc("Apply suggested fixes. Without -fix-errors\n"
+                        "clang-tidy will bail out if any compilation\n"
+                        "errors were found."),
+        cl::init(false), cl::cat(ClangTidyCategory));
+
+static cl::opt<bool>
+    FixErrors("fix-errors",
+              cl::desc("Apply suggested fixes even if compilation errors\n"
+                       "were found. If compiler errors have attached\n"
+                       "fix-its, clang-tidy will apply them as well."),
 
 static cl::opt<bool>
 ListChecks("list-checks",
            cl::desc("List all enabled checks and exit. Use with\n"
                     "-checks='*' to list all available checks."),
            cl::init(false), cl::cat(ClangTidyCategory));
 
 static cl::opt<std::string> Config(
int clangTidyMain(int argc, const char **argv) {
 
   ProfileData Profile;
 
   std::vector<ClangTidyError> Errors;
   ClangTidyStats Stats =
       runClangTidy(std::move(OptionsProvider), OptionsParser.getCompilations(),
                    OptionsParser.getSourcePathList(), &Errors,
                    EnableCheckProfile ? &Profile : nullptr);
+  bool FoundErrors =
+      std::find_if(Errors.begin(), Errors.end(), [](const ClangTidyError &E) {
+        return E.DiagLevel == ClangTidyError::Error;
+      }) != Errors.end();
+
+  // -fix-errors implies -fix.
+  if (FixErrors)
+    Fix = true;
+
+  const bool DisableFixes = Fix && FoundErrors && !FixErrors;
+  if (DisableFixes)
+    Fix = false;
+
   handleErrors(Errors, Fix);
 
   if (!ExportFixes.empty() && !Errors.empty()) {
     std::error_code EC;
     llvm::raw_fd_ostream OS(ExportFixes, EC, llvm::sys::fs::F_None);
     if (EC) {
       llvm::errs() << "Error opening output file: " << EC.message() << '\n';
       return 1;
     }
     exportReplacements(Errors, OS);
   }
 
   printStats(Stats);
+  if (DisableFixes)
+    llvm::errs() << "Found compiler errors, but -fix-error was not specified.\n"
+                    "Fixes have NOT been applied.\n\n";
+
   if (EnableCheckProfile)
     printProfileData(Profile, llvm::errs());
 
   return 0;
 }
 
 // This anchor is used to force the linker to link the LLVMModule.
 extern volatile int LLVMModuleAnchorSource;

test/clang-tidy/fix-errors.cpp

+// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
+// RUN: clang-tidy %t.cpp -checks='-*,google-explicit-constructor' -fix -- > %t.msg 2>&1
+// RUN: FileCheck -input-file=%t.cpp -check-prefix=CHECK-FIX %s
+// RUN: FileCheck -input-file=%t.msg -check-prefix=CHECK-MESSAGES %s
+// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
+// RUN: clang-tidy %t.cpp -checks='-*,google-explicit-constructor' -fix-errors -- > %t.msg 2>&1
+// RUN: FileCheck -input-file=%t.cpp -check-prefix=CHECK-FIX2 %s
+// RUN: FileCheck -input-file=%t.msg -check-prefix=CHECK-MESSAGES2 %s
+
+class A { A(int i); }
+// CHECK-FIX: class A { A(int i); }{{$}}
+// CHECK-MESSAGES: Fixes have NOT been applied.
+// CHECK-FIX2: class A { explicit A(int i); };
+// CHECK-MESSAGES2: note: FIX-IT applied suggested code changes
+// CHECK-MESSAGES2: clang-tidy applied 2 of 2 suggested fixes.