clang-tidy and clang-query wont crash with invalid command line options

[njames93]

Summary

Motivated by clang-tidy crashed for unknown command line argument.

[njames93]

Fix compile error

[aaron.ballman]

LGTM, but can you add test coverage for this?

[njames93]

Added test coverage.

[thakis]

This breaks check-clang-tools on windows: http://45.33.8.238/win/16485/step_8.txt

Looks like you need to optionally allow a .exe suffix in the test. Please take a look, and revert if it takes a while until you can fix.

(I'm on a phone, else I'd fix myself.)

[njames93]

In D80879#2065350, @thakis wrote:

This breaks check-clang-tools on windows: http://45.33.8.238/win/16485/step_8.txt

Looks like you need to optionally allow a .exe suffix in the test. Please take a look, and revert if it takes a while until you can fix.

(I'm on a phone, else I'd fix myself.)

Just seen the buildbot, I've reverted it for now. Don't have windows to test so didnt want to fire away more breaking commits

[njames93]

Hopefully fix windows test cases

[thakis]

Test fix looks good as far as I can tell. Landing and seeing what the bot says is ok. It's fine for some of the bots to be red for a short while as long as it isn't hours. Thanks!

[njames93]

In D80879#2065364, @thakis wrote:

Test fix looks good as far as I can tell. Landing and seeing what the bot says is ok. It's fine for some of the bots to be red for a short while as long as it isn't hours. Thanks!

I'm unsure about one part(the CHECK-NEXT lines, just wanna see what the pre merge bot says.

[njames93]

Actually fix the windows builds

clang-tools-extra/clang-query/tool/ClangQuery.cpp

 #include "QuerySession.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/LineEditor/LineEditor.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Signals.h"
+#include "llvm/Support/WithColor.h"
 #include <fstream>
 #include <string>
 
 using namespace clang;
 using namespace clang::ast_matchers;
 using namespace clang::ast_matchers::dynamic;
 using namespace clang::query;
 using namespace clang::tooling;
while (!FileContentRef.empty()) {
     FileContentRef = Q->RemainingContent;
   }
   return false;
 }
 
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
 
-  CommonOptionsParser OptionsParser(argc, argv, ClangQueryCategory);
+  llvm::Expected<CommonOptionsParser> OptionsParser =
+      CommonOptionsParser::create(argc, argv, ClangQueryCategory,
+                                  llvm::cl::OneOrMore);
+
+  if (!OptionsParser) {
+    llvm::WithColor::error() << llvm::toString(OptionsParser.takeError());
+    return 1;
+  }
 
   if (!Commands.empty() && !CommandFiles.empty()) {
     llvm::errs() << argv[0] << ": cannot specify both -c and -f\n";
     return 1;
   }
 
   if ((!Commands.empty() || !CommandFiles.empty()) && !PreloadFile.empty()) {
     llvm::errs() << argv[0]
                  << ": cannot specify both -c or -f with --preload\n";
     return 1;
   }
 
-  ClangTool Tool(OptionsParser.getCompilations(),
-                 OptionsParser.getSourcePathList());
+  ClangTool Tool(OptionsParser->getCompilations(),
+                 OptionsParser->getSourcePathList());
   std::vector<std::unique_ptr<ASTUnit>> ASTs;
   int Status = Tool.buildASTs(ASTs);
   int ASTStatus = 0;
   if (Status == 1) {
     // Building ASTs failed.
     return 1;
   } else if (Status == 2) {
     ASTStatus |= 1;

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

 #include "../ClangTidy.h"
 #include "../ClangTidyForceLinker.h"
 #include "../GlobList.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
+#include "llvm/Support/WithColor.h"
 
 using namespace clang::ast_matchers;
 using namespace clang::driver;
 using namespace clang::tooling;
 using namespace llvm;
 
 static cl::OptionCategory ClangTidyCategory("clang-tidy options");
 
llvm::errs() << "Error: invalid virtual filesystem overlay file '"
                  << OverlayFile << "'.\n";
     return nullptr;
   }
   return FS;
 }
 
 int clangTidyMain(int argc, const char **argv) {
   llvm::InitLLVM X(argc, argv);
-  CommonOptionsParser OptionsParser(argc, argv, ClangTidyCategory,
-                                    cl::ZeroOrMore);
+  llvm::Expected<CommonOptionsParser> OptionsParser =
+      CommonOptionsParser::create(argc, argv, ClangTidyCategory,
+  if (!OptionsParser) {
+    llvm::WithColor::error() << llvm::toString(OptionsParser.takeError());
+    return 1;
+  }
+
   llvm::IntrusiveRefCntPtr<vfs::OverlayFileSystem> BaseFS(
       new vfs::OverlayFileSystem(vfs::getRealFileSystem()));
 
   if (!VfsOverlay.empty()) {
     IntrusiveRefCntPtr<vfs::FileSystem> VfsFromFile =
         getVfsFromFile(VfsOverlay, BaseFS);
     if (!VfsFromFile)
       return 1;
if (std::error_code EC = llvm::sys::fs::make_absolute(AbsolutePath)) {
                    << EC.message() << "\n";
     }
     return AbsolutePath;
   };
 
   SmallString<256> ProfilePrefix = MakeAbsolute(StoreCheckProfile);
 
   StringRef FileName("dummy");
-  auto PathList = OptionsParser.getSourcePathList();
+  auto PathList = OptionsParser->getSourcePathList();
   if (!PathList.empty()) {
     FileName = PathList.front();
   }
 
   SmallString<256> FilePath = MakeAbsolute(std::string(FileName));
 
   ClangTidyOptions EffectiveOptions = OptionsProvider->getOptions(FilePath);
   std::vector<std::string> EnabledChecks =
int clangTidyMain(int argc, const char **argv) {
 
   llvm::InitializeAllTargetInfos();
   llvm::InitializeAllTargetMCs();
   llvm::InitializeAllAsmParsers();
 
   ClangTidyContext Context(std::move(OwningOptionsProvider),
                            AllowEnablingAnalyzerAlphaCheckers);
   std::vector<ClangTidyError> Errors =
-      runClangTidy(Context, OptionsParser.getCompilations(), PathList, BaseFS,
+      runClangTidy(Context, OptionsParser->getCompilations(), PathList, BaseFS,
                    EnableCheckProfile, ProfilePrefix);
   bool FoundErrors = llvm::find_if(Errors, [](const ClangTidyError &E) {
                        return E.DiagLevel == ClangTidyError::Error;
                      }) != Errors.end();
 
   const bool DisableFixes = Fix && FoundErrors && !FixErrors;
 
   unsigned WErrorCount = 0;

clang-tools-extra/test/clang-query/invalid-command-line.cpp

+// RUN: not clang-query --invalid-arg 2>&1 | FileCheck %s
+
+// CHECK: error: [CommonOptionsParser]: clang-query{{(\.exe)?}}: Unknown command line argument '--invalid-arg'.  Try: 'clang-query{{(\.exe)?}} --help'
+// CHECK-NEXT: clang-query{{(\.exe)?}}: Did you mean '--extra-arg'?

clang-tools-extra/test/clang-tidy/infrastructure/invalid-command-line.cpp

+// RUN: not clang-tidy --invalid-arg 2>&1 | FileCheck %s
+
+// CHECK: error: [CommonOptionsParser]: clang-tidy{{(\.exe)?}}: Unknown command line argument '--invalid-arg'.  Try: 'clang-tidy{{(\.exe)?}} --help'
+// CHECK-NEXT: clang-tidy{{(\.exe)?}}: Did you mean '--extra-arg'?