[clang-tidy] Return error code on config parse error

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp

@@ -265,8 +265,16 @@ const ClangTidyOptions &ClangTidyContext::getOptions() const {
 ClangTidyOptions ClangTidyContext::getOptionsForFile(StringRef File) const {
   // Merge options on top of getDefaults() as a safeguard against options with
   // unset values.
-  return ClangTidyOptions::getDefaults().merge(
-      OptionsProvider->getOptions(File), 0);
+  ClangTidyOptions defaultOptions = ClangTidyOptions::getDefaults();
+  llvm::ErrorOr<ClangTidyOptions> fileOptions =
+      OptionsProvider->getOptions(File);
+
+  // If there was an error parsing the options, just use the default options.
+  // Ideally, the options for each file should be validated before this point.
+  if (!fileOptions)
+    return defaultOptions;
+
+  return defaultOptions.merge(*fileOptions, 0);
 }
 
 void ClangTidyContext::setEnableProfiling(bool P) { Profile = P; }

clang-tools-extra/clang-tidy/ClangTidyOptions.cpp

@@ -267,16 +267,19 @@ const char
     ClangTidyOptionsProvider::OptionsSourceTypeConfigCommandLineOption[] =
         "command-line option '-config'";
 
-ClangTidyOptions
+llvm::ErrorOr<ClangTidyOptions>
 ClangTidyOptionsProvider::getOptions(llvm::StringRef FileName) {
   ClangTidyOptions Result;
   unsigned Priority = 0;
-  for (auto &Source : getRawOptions(FileName))
+  llvm::ErrorOr<std::vector<OptionsSource>> Options = getRawOptions(FileName);
+  if (!Options)
+    return Options.getError();
+  for (auto &Source : *Options)
     Result.mergeWith(Source.first, ++Priority);
   return Result;
 }
 
-std::vector<OptionsSource>
+llvm::ErrorOr<std::vector<OptionsSource>>
 DefaultOptionsProvider::getRawOptions(llvm::StringRef FileName) {
   std::vector<OptionsSource> Result;
   Result.emplace_back(DefaultOptions, OptionsSourceTypeDefaultBinary);
@@ -292,24 +295,26 @@ ConfigOptionsProvider::ConfigOptionsProvider(
                               std::move(OverrideOptions), std::move(FS)),
       ConfigOptions(std::move(ConfigOptions)) {}
 
-std::vector<OptionsSource>
+llvm::ErrorOr<std::vector<OptionsSource>>
 ConfigOptionsProvider::getRawOptions(llvm::StringRef FileName) {
-  std::vector<OptionsSource> RawOptions =
+  llvm::ErrorOr<std::vector<OptionsSource>> RawOptions =
       DefaultOptionsProvider::getRawOptions(FileName);
+  if (!RawOptions)
+    return RawOptions;
   if (ConfigOptions.InheritParentConfig.value_or(false)) {
     LLVM_DEBUG(llvm::dbgs()
                << "Getting options for file " << FileName << "...\n");
 
     llvm::ErrorOr<llvm::SmallString<128>> AbsoluteFilePath =
         getNormalizedAbsolutePath(FileName);
     if (AbsoluteFilePath) {
-      addRawFileOptions(AbsoluteFilePath->str(), RawOptions);
+      addRawFileOptions(AbsoluteFilePath->str(), *RawOptions);
     }
   }
-  RawOptions.emplace_back(ConfigOptions,
-                          OptionsSourceTypeConfigCommandLineOption);
-  RawOptions.emplace_back(OverrideOptions,
-                          OptionsSourceTypeCheckCommandLineOption);
+  RawOptions->emplace_back(ConfigOptions,
+                           OptionsSourceTypeConfigCommandLineOption);
+  RawOptions->emplace_back(OverrideOptions,
+                           OptionsSourceTypeCheckCommandLineOption);
   return RawOptions;
 }
 
@@ -345,21 +350,21 @@ FileOptionsBaseProvider::getNormalizedAbsolutePath(llvm::StringRef Path) {
   return NormalizedAbsolutePath;
 }
 
-void FileOptionsBaseProvider::addRawFileOptions(
+std::error_code FileOptionsBaseProvider::addRawFileOptions(
     llvm::StringRef AbsolutePath, std::vector<OptionsSource> &CurOptions) {
   auto CurSize = CurOptions.size();
   // Look for a suitable configuration file in all parent directories of the
   // file. Start with the immediate parent directory and move up.
   StringRef RootPath = llvm::sys::path::parent_path(AbsolutePath);
   auto MemorizedConfigFile =
-      [this, &RootPath](StringRef CurrentPath) -> std::optional<OptionsSource> {
+      [this, &RootPath](StringRef CurrentPath) -> llvm::ErrorOr<OptionsSource> {
     const auto Iter = CachedOptions.Memorized.find(CurrentPath);
     if (Iter != CachedOptions.Memorized.end())
       return CachedOptions.Storage[Iter->second];
-    std::optional<OptionsSource> OptionsSource = tryReadConfigFile(CurrentPath);
+    llvm::ErrorOr<OptionsSource> OptionsSource = tryReadConfigFile(CurrentPath);
     if (OptionsSource) {
       const size_t Index = CachedOptions.Storage.size();
-      CachedOptions.Storage.emplace_back(OptionsSource.value());
+      CachedOptions.Storage.emplace_back(*OptionsSource);
       while (RootPath != CurrentPath) {
         LLVM_DEBUG(llvm::dbgs()
                    << "Caching configuration for path " << RootPath << ".\n");
@@ -373,16 +378,21 @@ void FileOptionsBaseProvider::addRawFileOptions(
   };
   for (StringRef CurrentPath = RootPath; !CurrentPath.empty();
        CurrentPath = llvm::sys::path::parent_path(CurrentPath)) {
-    if (std::optional<OptionsSource> Result =
-            MemorizedConfigFile(CurrentPath)) {
-      CurOptions.emplace_back(Result.value());
+    llvm::ErrorOr<OptionsSource> Result = MemorizedConfigFile(CurrentPath);
+
+    if (Result) {
+      CurOptions.emplace_back(*Result);
       if (!Result->first.InheritParentConfig.value_or(false))
         break;
+    } else if (Result.getError() != llvm::errc::no_such_file_or_directory) {
+      return Result.getError();
     }
   }
   // Reverse order of file configs because closer configs should have higher
   // priority.
   std::reverse(CurOptions.begin() + CurSize, CurOptions.end());
+
+  return {};
 }
 
 FileOptionsProvider::FileOptionsProvider(
@@ -404,27 +414,31 @@ FileOptionsProvider::FileOptionsProvider(
 // FIXME: This method has some common logic with clang::format::getStyle().
 // Consider pulling out common bits to a findParentFileWithName function or
 // similar.
-std::vector<OptionsSource>
+llvm::ErrorOr<std::vector<OptionsSource>>
 FileOptionsProvider::getRawOptions(StringRef FileName) {
   LLVM_DEBUG(llvm::dbgs() << "Getting options for file " << FileName
                           << "...\n");
 
   llvm::ErrorOr<llvm::SmallString<128>> AbsoluteFilePath =
       getNormalizedAbsolutePath(FileName);
   if (!AbsoluteFilePath)
-    return {};
+    return std::vector<OptionsSource>{};
 
-  std::vector<OptionsSource> RawOptions =
+  llvm::ErrorOr<std::vector<OptionsSource>> RawOptions =
       DefaultOptionsProvider::getRawOptions(AbsoluteFilePath->str());
-  addRawFileOptions(AbsoluteFilePath->str(), RawOptions);
+  std::error_code Err = addRawFileOptions(AbsoluteFilePath->str(), *RawOptions);
+
+  if (Err)
+    return Err;
+
   OptionsSource CommandLineOptions(OverrideOptions,
                                    OptionsSourceTypeCheckCommandLineOption);
 
-  RawOptions.push_back(CommandLineOptions);
+  RawOptions->push_back(CommandLineOptions);
   return RawOptions;
 }
 
-std::optional<OptionsSource>
+llvm::ErrorOr<OptionsSource>
 FileOptionsBaseProvider::tryReadConfigFile(StringRef Directory) {
   assert(!Directory.empty());
 
@@ -433,7 +447,7 @@ FileOptionsBaseProvider::tryReadConfigFile(StringRef Directory) {
   if (!DirectoryStatus || !DirectoryStatus->isDirectory()) {
     llvm::errs() << "Error reading configuration from " << Directory
                  << ": directory doesn't exist.\n";
-    return std::nullopt;
+    return make_error_code(llvm::errc::not_a_directory);
   }
 
   for (const ConfigFileHandler &ConfigHandler : ConfigHandlers) {
@@ -464,11 +478,11 @@ FileOptionsBaseProvider::tryReadConfigFile(StringRef Directory) {
       if (ParsedOptions.getError())
         llvm::errs() << "Error parsing " << ConfigFile << ": "
                      << ParsedOptions.getError().message() << "\n";
-      continue;
+      return ParsedOptions.getError();
     }
     return OptionsSource(*ParsedOptions, std::string(ConfigFile));
   }
-  return std::nullopt;
+  return make_error_code(llvm::errc::no_such_file_or_directory);
 }
 
 /// Parses -line-filter option and stores it to the \c Options.

clang-tools-extra/clang-tidy/ClangTidyOptions.h

@@ -174,12 +174,12 @@ class ClangTidyOptionsProvider {
 
   /// Returns an ordered vector of OptionsSources, in order of increasing
   /// priority.
-  virtual std::vector<OptionsSource>
+  virtual llvm::ErrorOr<std::vector<OptionsSource>>
   getRawOptions(llvm::StringRef FileName) = 0;
 
   /// Returns options applying to a specific translation unit with the
   /// specified \p FileName.
-  ClangTidyOptions getOptions(llvm::StringRef FileName);
+  llvm::ErrorOr<ClangTidyOptions> getOptions(llvm::StringRef FileName);
 };
 
 /// Implementation of the \c ClangTidyOptionsProvider interface, which
@@ -193,7 +193,8 @@ class DefaultOptionsProvider : public ClangTidyOptionsProvider {
   const ClangTidyGlobalOptions &getGlobalOptions() override {
     return GlobalOptions;
   }
-  std::vector<OptionsSource> getRawOptions(llvm::StringRef FileName) override;
+  llvm::ErrorOr<std::vector<OptionsSource>>
+  getRawOptions(llvm::StringRef FileName) override;
 
 private:
   ClangTidyGlobalOptions GlobalOptions;
@@ -204,7 +205,9 @@ class FileOptionsBaseProvider : public DefaultOptionsProvider {
 protected:
   // A pair of configuration file base name and a function parsing
   // configuration from text in the corresponding format.
-  using ConfigFileHandler = std::pair<std::string, std::function<llvm::ErrorOr<ClangTidyOptions> (llvm::MemoryBufferRef)>>;
+  using ConfigFileHandler =
+      std::pair<std::string, std::function<llvm::ErrorOr<ClangTidyOptions>(
+                                 llvm::MemoryBufferRef)>>;
 
   /// Configuration file handlers listed in the order of priority.
   ///
@@ -235,15 +238,15 @@ class FileOptionsBaseProvider : public DefaultOptionsProvider {
                           ClangTidyOptions OverrideOptions,
                           ConfigFileHandlers ConfigHandlers);
 
-  void addRawFileOptions(llvm::StringRef AbsolutePath,
-                         std::vector<OptionsSource> &CurOptions);
+  std::error_code addRawFileOptions(llvm::StringRef AbsolutePath,
+                                    std::vector<OptionsSource> &CurOptions);
 
   llvm::ErrorOr<llvm::SmallString<128>>
   getNormalizedAbsolutePath(llvm::StringRef AbsolutePath);
 
   /// Try to read configuration files from \p Directory using registered
   /// \c ConfigHandlers.
-  std::optional<OptionsSource> tryReadConfigFile(llvm::StringRef Directory);
+  llvm::ErrorOr<OptionsSource> tryReadConfigFile(llvm::StringRef Directory);
 
   struct OptionsCache {
     llvm::StringMap<size_t> Memorized;
@@ -262,7 +265,8 @@ class ConfigOptionsProvider : public FileOptionsBaseProvider {
       ClangTidyGlobalOptions GlobalOptions, ClangTidyOptions DefaultOptions,
       ClangTidyOptions ConfigOptions, ClangTidyOptions OverrideOptions,
       llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS = nullptr);
-  std::vector<OptionsSource> getRawOptions(llvm::StringRef FileName) override;
+  llvm::ErrorOr<std::vector<OptionsSource>>
+  getRawOptions(llvm::StringRef FileName) override;
 
 private:
   ClangTidyOptions ConfigOptions;
@@ -315,7 +319,8 @@ class FileOptionsProvider : public FileOptionsBaseProvider {
                       ClangTidyOptions OverrideOptions,
                       ConfigFileHandlers ConfigHandlers);
 
-  std::vector<OptionsSource> getRawOptions(llvm::StringRef FileName) override;
+  llvm::ErrorOr<std::vector<OptionsSource>>
+  getRawOptions(llvm::StringRef FileName) override;
 };
 
 /// Parses LineFilter from JSON and stores it to the \p Options.

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

@@ -624,17 +624,37 @@ int clangTidyMain(int argc, const char **argv) {
   }
 
   SmallString<256> FilePath = makeAbsolute(FileName);
-  ClangTidyOptions EffectiveOptions = OptionsProvider->getOptions(FilePath);
+  llvm::ErrorOr<ClangTidyOptions> EffectiveOptions =
+      OptionsProvider->getOptions(FilePath);
+
+  if (!EffectiveOptions)
+    return 1;
+
+  // Validate the configuration files associated with all input files so we can
+  // return an error up front.
+  if (PathList.size() > 1) {
+    for (auto iter = PathList.begin() + 1; iter != PathList.end(); ++iter) {
+      llvm::ErrorOr<ClangTidyOptions> Options =
+          OptionsProvider->getOptions(*iter);
+      if (!Options)
+        return 1;
+    }
+  }
 
   std::vector<std::string> EnabledChecks =
-      getCheckNames(EffectiveOptions, AllowEnablingAnalyzerAlphaCheckers);
+      getCheckNames(*EffectiveOptions, AllowEnablingAnalyzerAlphaCheckers);
 
   if (ExplainConfig) {
     // FIXME: Show other ClangTidyOptions' fields, like ExtraArg.
-    std::vector<clang::tidy::ClangTidyOptionsProvider::OptionsSource>
+    llvm::ErrorOr<
+        std::vector<clang::tidy::ClangTidyOptionsProvider::OptionsSource>>
         RawOptions = OptionsProvider->getRawOptions(FilePath);
+
+    if (!RawOptions)
+      return 1;
+
     for (const std::string &Check : EnabledChecks) {
-      for (const auto &[Opts, Source] : llvm::reverse(RawOptions)) {
+      for (const auto &[Opts, Source] : llvm::reverse(*RawOptions)) {
         if (Opts.Checks && GlobList(*Opts.Checks).contains(Check)) {
           llvm::outs() << "'" << Check << "' is enabled in the " << Source
                        << ".\n";
@@ -658,21 +678,23 @@ int clangTidyMain(int argc, const char **argv) {
   }
 
   if (DumpConfig) {
-    EffectiveOptions.CheckOptions =
-        getCheckOptions(EffectiveOptions, AllowEnablingAnalyzerAlphaCheckers);
+    EffectiveOptions->CheckOptions =
+        getCheckOptions(*EffectiveOptions, AllowEnablingAnalyzerAlphaCheckers);
     llvm::outs() << configurationAsText(ClangTidyOptions::getDefaults().merge(
-                        EffectiveOptions, 0))
+                        *EffectiveOptions, 0))
                  << "\n";
     return 0;
   }
 
   if (VerifyConfig) {
-    std::vector<ClangTidyOptionsProvider::OptionsSource> RawOptions =
-        OptionsProvider->getRawOptions(FileName);
+    llvm::ErrorOr<std::vector<ClangTidyOptionsProvider::OptionsSource>>
+        RawOptions = OptionsProvider->getRawOptions(FileName);
+    if (!RawOptions)
+      return 1;
     ChecksAndOptions Valid =
         getAllChecksAndOptions(AllowEnablingAnalyzerAlphaCheckers);
     bool AnyInvalid = false;
-    for (const auto &[Opts, Source] : RawOptions) {
+    for (const auto &[Opts, Source] : *RawOptions) {
       if (Opts.Checks)
         AnyInvalid |= verifyChecks(Valid.Checks, *Opts.Checks, Source);
       if (Opts.HeaderFileExtensions && Opts.ImplementationFileExtensions)

clang-tools-extra/docs/ReleaseNotes.rst

@@ -114,6 +114,9 @@ Improvements to clang-tidy
 - Fixed bug in :program:`run_clang_tidy.py` where the program would not
   correctly display the checks enabled by the top-level `.clang-tidy` file.
 
+- Changed :program:`clang-tidy` to exit with an error code if it encounters an
+  invalid configuration file.
+
 New checks
 ^^^^^^^^^^