[clang-tidy] Extend InheritParentConfig to CommandLineConfig

[njames93]

Summary

Extend the InheritParentConfig support introduced in D75184 for the command line option --config.
The current behaviour of --config is to when set, disable looking for .clang-tidy configuration files.
This new behaviour lets you set InheritParentConfig to true in the command line to then look for .clang-tidy configuration files to be merged with what's been specified on the command line.

[DmitryPolukhin]

LGTM

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

struct ClangTidyOptions {
   typedef std::vector<std::string> ArgList;
 
   /// Add extra compilation arguments to the end of the list.
   llvm::Optional<ArgList> ExtraArgs;
 
   /// Add extra compilation arguments to the start of the list.
   llvm::Optional<ArgList> ExtraArgsBefore;
 
-  /// Only used in the FileOptionsProvider. If true, FileOptionsProvider will
-  /// take a configuration file in the parent directory (if any exists) and
-  /// apply this config file on top of the parent one. If false or missing,
-  /// only this configuration file will be used.
+  /// Only used in the FileOptionsProvider and ConfigOptionsProvider. If true
+  /// and using a FileOptionsProvider, it will take a configuration file in the
+  /// parent directory (if any exists) and apply this config file on top of the
+  /// parent one. IF true and using a ConfigOptionsProvider, it will apply this
+  /// config on top of any configuation file it finds in the directory using the
+  /// same logic as FileOptionsProvider. If false or missing, only this
+  /// configuration file will be used.
   llvm::Optional<bool> InheritParentConfig;
 
   /// Use colors in diagnostics. If missing, it will be auto detected.
   llvm::Optional<bool> UseColor;
 };
 
 /// Abstract interface for retrieving various ClangTidy options.
 class ClangTidyOptionsProvider {
public:
   }
   std::vector<OptionsSource> getRawOptions(llvm::StringRef FileName) override;
 
 private:
   ClangTidyGlobalOptions GlobalOptions;
   ClangTidyOptions DefaultOptions;
 };
 
-/// Implementation of ClangTidyOptions interface, which is used for
-/// '-config' command-line option.
-class ConfigOptionsProvider : public DefaultOptionsProvider {
-public:
-  ConfigOptionsProvider(const ClangTidyGlobalOptions &GlobalOptions,
-                        const ClangTidyOptions &DefaultOptions,
-                        const ClangTidyOptions &ConfigOptions,
-                        const ClangTidyOptions &OverrideOptions);
-  std::vector<OptionsSource> getRawOptions(llvm::StringRef FileName) override;
-
-private:
-  ClangTidyOptions ConfigOptions;
-  ClangTidyOptions OverrideOptions;
-};
-
-/// Implementation of the \c ClangTidyOptionsProvider interface, which
-/// tries to find a configuration file in the closest parent directory of each
-/// source file.
-///
-/// By default, files named ".clang-tidy" will be considered, and the
-/// \c clang::tidy::parseConfiguration function will be used for parsing, but a
-/// custom set of configuration file names and parsing functions can be
-/// specified using the appropriate constructor.
-class FileOptionsProvider : public DefaultOptionsProvider {
+class FileOptionsBaseProvider : public DefaultOptionsProvider {
 public:
   // A pair of configuration file base name and a function parsing
   // configuration from text in the corresponding format.
   typedef std::pair<std::string, std::function<llvm::ErrorOr<ClangTidyOptions>(
                                      llvm::StringRef)>>
       ConfigFileHandler;
 
   /// Configuration file handlers listed in the order of priority.
public:
   /// return std::make_unique<FileOptionsProvider>(
   ///     GlobalOptions, DefaultOptions, OverrideOptions, ConfigHandlers);
   /// \endcode
   ///
   /// With the order of handlers shown above, the ".my-tidy-config" file would
   /// take precedence over ".clang-tidy" if both reside in the same directory.
   typedef std::vector<ConfigFileHandler> ConfigFileHandlers;
 
+  FileOptionsBaseProvider(
+      const ClangTidyGlobalOptions &GlobalOptions,
+      const ClangTidyOptions &DefaultOptions,
+      const ClangTidyOptions &OverrideOptions,
+      llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS = nullptr);
+
+  FileOptionsBaseProvider(const ClangTidyGlobalOptions &GlobalOptions,
+                          const ClangTidyOptions &DefaultOptions,
+                          const ClangTidyOptions &OverrideOptions,
+                          const ConfigFileHandlers &ConfigHandlers);
+
+protected:
+  void addRawFileOptions(llvm::StringRef AbsolutePath,
+                         std::vector<OptionsSource> &CurOptions);
+
+  /// Try to read configuration files from \p Directory using registered
+  /// \c ConfigHandlers.
+  llvm::Optional<OptionsSource> tryReadConfigFile(llvm::StringRef Directory);
+
+  llvm::StringMap<OptionsSource> CachedOptions;
+  ClangTidyOptions OverrideOptions;
+  ConfigFileHandlers ConfigHandlers;
+  llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS;
+};
+
+/// Implementation of ClangTidyOptions interface, which is used for
+/// '-config' command-line option.
+class ConfigOptionsProvider : public FileOptionsBaseProvider {
+public:
+  ConfigOptionsProvider(
+      const ClangTidyGlobalOptions &GlobalOptions,
+      const ClangTidyOptions &DefaultOptions,
+      const ClangTidyOptions &ConfigOptions,
+      const ClangTidyOptions &OverrideOptions,
+      llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS = nullptr);
+  std::vector<OptionsSource> getRawOptions(llvm::StringRef FileName) override;
+
+private:
+  ClangTidyOptions ConfigOptions;
+};
+
+/// Implementation of the \c ClangTidyOptionsProvider interface, which
+/// tries to find a configuration file in the closest parent directory of each
+/// source file.
+///
+/// By default, files named ".clang-tidy" will be considered, and the
+/// \c clang::tidy::parseConfiguration function will be used for parsing, but a
+/// custom set of configuration file names and parsing functions can be
+/// specified using the appropriate constructor.
+class FileOptionsProvider : public FileOptionsBaseProvider {
+public:
   /// Initializes the \c FileOptionsProvider instance.
   ///
   /// \param GlobalOptions are just stored and returned to the caller of
   /// \c getGlobalOptions.
   ///
   /// \param DefaultOptions are used for all settings not specified in a
   /// configuration file.
   ///
public:
   /// in each directory are searched for in the order of appearance in
   /// \p ConfigHandlers.
   FileOptionsProvider(const ClangTidyGlobalOptions &GlobalOptions,
                       const ClangTidyOptions &DefaultOptions,
                       const ClangTidyOptions &OverrideOptions,
                       const ConfigFileHandlers &ConfigHandlers);
 
   std::vector<OptionsSource> getRawOptions(llvm::StringRef FileName) override;
-
-protected:
-  /// Try to read configuration files from \p Directory using registered
-  /// \c ConfigHandlers.
-  llvm::Optional<OptionsSource> tryReadConfigFile(llvm::StringRef Directory);
-
-  llvm::StringMap<OptionsSource> CachedOptions;
-  ClangTidyOptions OverrideOptions;
-  ConfigFileHandlers ConfigHandlers;
-  llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS;
 };
 
 /// Parses LineFilter from JSON and stores it to the \p Options.
 std::error_code parseLineFilter(llvm::StringRef LineFilter,
                                 ClangTidyGlobalOptions &Options);
 
 /// Parses configuration from JSON and returns \c ClangTidyOptions or an
 /// error.

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

DefaultOptionsProvider::getRawOptions(llvm::StringRef FileName) {
   Result.emplace_back(DefaultOptions, OptionsSourceTypeDefaultBinary);
   return Result;
 }
 
 ConfigOptionsProvider::ConfigOptionsProvider(
     const ClangTidyGlobalOptions &GlobalOptions,
     const ClangTidyOptions &DefaultOptions,
     const ClangTidyOptions &ConfigOptions,
-    const ClangTidyOptions &OverrideOptions)
-    : DefaultOptionsProvider(GlobalOptions, DefaultOptions),
-      ConfigOptions(ConfigOptions), OverrideOptions(OverrideOptions) {}
+    const ClangTidyOptions &OverrideOptions,
+    llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS)
+    : FileOptionsBaseProvider(GlobalOptions, DefaultOptions, OverrideOptions,
+                              FS),
+      ConfigOptions(ConfigOptions) {}
 
 std::vector<OptionsSource>
 ConfigOptionsProvider::getRawOptions(llvm::StringRef FileName) {
   std::vector<OptionsSource> RawOptions =
       DefaultOptionsProvider::getRawOptions(FileName);
+  if (ConfigOptions.InheritParentConfig.getValueOr(false)) {
+    LLVM_DEBUG(llvm::dbgs()
+               << "Getting options for file " << FileName << "...\n");
+    assert(FS && "FS must be set.");
+
+    llvm::SmallString<128> AbsoluteFilePath(FileName);
+
+    if (!FS->makeAbsolute(AbsoluteFilePath)) {
+      addRawFileOptions(AbsoluteFilePath, RawOptions);
+    }
+  }
   RawOptions.emplace_back(ConfigOptions,
                           OptionsSourceTypeConfigCommandLineOption);
   RawOptions.emplace_back(OverrideOptions,
                           OptionsSourceTypeCheckCommandLineOption);
   return RawOptions;
 }
 
-FileOptionsProvider::FileOptionsProvider(
+FileOptionsBaseProvider::FileOptionsBaseProvider(
     const ClangTidyGlobalOptions &GlobalOptions,
     const ClangTidyOptions &DefaultOptions,
     const ClangTidyOptions &OverrideOptions,
     llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS)
     : DefaultOptionsProvider(GlobalOptions, DefaultOptions),
       OverrideOptions(OverrideOptions), FS(std::move(VFS)) {
   if (!FS)
     FS = llvm::vfs::getRealFileSystem();
   ConfigHandlers.emplace_back(".clang-tidy", parseConfiguration);
 }
 
-FileOptionsProvider::FileOptionsProvider(
+FileOptionsBaseProvider::FileOptionsBaseProvider(
     const ClangTidyGlobalOptions &GlobalOptions,
     const ClangTidyOptions &DefaultOptions,
     const ClangTidyOptions &OverrideOptions,
-    const FileOptionsProvider::ConfigFileHandlers &ConfigHandlers)
+    const FileOptionsBaseProvider::ConfigFileHandlers &ConfigHandlers)
     : DefaultOptionsProvider(GlobalOptions, DefaultOptions),
       OverrideOptions(OverrideOptions), ConfigHandlers(ConfigHandlers) {}
 
-// 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>
-FileOptionsProvider::getRawOptions(StringRef FileName) {
-  LLVM_DEBUG(llvm::dbgs() << "Getting options for file " << FileName
-                          << "...\n");
-  assert(FS && "FS must be set.");
-
-  llvm::SmallString<128> AbsoluteFilePath(FileName);
+void FileOptionsBaseProvider::addRawFileOptions(
+    llvm::StringRef AbsolutePath, std::vector<OptionsSource> &CurOptions) {
+  auto CurSize = CurOptions.size();
 
-  if (FS->makeAbsolute(AbsoluteFilePath))
-    return {};
-
-  std::vector<OptionsSource> RawOptions =
-      DefaultOptionsProvider::getRawOptions(AbsoluteFilePath.str());
-  OptionsSource CommandLineOptions(OverrideOptions,
-                                   OptionsSourceTypeCheckCommandLineOption);
-  size_t FirstFileConfig = RawOptions.size();
   // Look for a suitable configuration file in all parent directories of the
   // file. Start with the immediate parent directory and move up.
-  StringRef Path = llvm::sys::path::parent_path(AbsoluteFilePath.str());
+  StringRef Path = llvm::sys::path::parent_path(AbsolutePath);
   for (StringRef CurrentPath = Path; !CurrentPath.empty();
        CurrentPath = llvm::sys::path::parent_path(CurrentPath)) {
     llvm::Optional<OptionsSource> Result;
 
     auto Iter = CachedOptions.find(CurrentPath);
     if (Iter != CachedOptions.end())
       Result = Iter->second;
 
     if (!Result)
       Result = tryReadConfigFile(CurrentPath);
 
     if (Result) {
       // Store cached value for all intermediate directories.
       while (Path != CurrentPath) {
         LLVM_DEBUG(llvm::dbgs()
                    << "Caching configuration for path " << Path << ".\n");
         if (!CachedOptions.count(Path))
           CachedOptions[Path] = *Result;
         Path = llvm::sys::path::parent_path(Path);
       }
       CachedOptions[Path] = *Result;
 
-      RawOptions.push_back(*Result);
-      if (!Result->first.InheritParentConfig ||
-          !*Result->first.InheritParentConfig)
+      CurOptions.push_back(*Result);
+      if (!Result->first.InheritParentConfig.getValueOr(false))
         break;
     }
   }
   // Reverse order of file configs because closer configs should have higher
   // priority.
-  std::reverse(RawOptions.begin() + FirstFileConfig, RawOptions.end());
+  std::reverse(CurOptions.begin() + CurSize, CurOptions.end());
+}
+
+FileOptionsProvider::FileOptionsProvider(
+    const ClangTidyGlobalOptions &GlobalOptions,
+    const ClangTidyOptions &DefaultOptions,
+    const ClangTidyOptions &OverrideOptions,
+    llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS)
+    : FileOptionsBaseProvider(GlobalOptions, DefaultOptions, OverrideOptions,
+                              VFS){};
+
+FileOptionsProvider::FileOptionsProvider(
+    const ClangTidyGlobalOptions &GlobalOptions,
+    const ClangTidyOptions &DefaultOptions,
+    const ClangTidyOptions &OverrideOptions,
+    const FileOptionsBaseProvider::ConfigFileHandlers &ConfigHandlers)
+    : FileOptionsBaseProvider(GlobalOptions, DefaultOptions, OverrideOptions,
+                              ConfigHandlers) {}
+
+// 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>
+FileOptionsProvider::getRawOptions(StringRef FileName) {
+  LLVM_DEBUG(llvm::dbgs() << "Getting options for file " << FileName
+                          << "...\n");
+  assert(FS && "FS must be set.");
+
+  llvm::SmallString<128> AbsoluteFilePath(FileName);
+
+  if (FS->makeAbsolute(AbsoluteFilePath))
+    return {};
+
+  std::vector<OptionsSource> RawOptions =
+      DefaultOptionsProvider::getRawOptions(AbsoluteFilePath.str());
+  addRawFileOptions(AbsoluteFilePath, RawOptions);
+  OptionsSource CommandLineOptions(OverrideOptions,
+                                   OptionsSourceTypeCheckCommandLineOption);
+
   RawOptions.push_back(CommandLineOptions);
   return RawOptions;
 }
 
 llvm::Optional<OptionsSource>
-FileOptionsProvider::tryReadConfigFile(StringRef Directory) {
+FileOptionsBaseProvider::tryReadConfigFile(StringRef Directory) {
   assert(!Directory.empty());
 
   if (!llvm::sys::fs::is_directory(Directory)) {
     llvm::errs() << "Error reading configuration from " << Directory
                  << ": directory doesn't exist.\n";
     return llvm::None;
   }
 

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

if (UseColor.getNumOccurrences() > 0)
     OverrideOptions.UseColor = UseColor;
 
   if (!Config.empty()) {
     if (llvm::ErrorOr<ClangTidyOptions> ParsedConfig =
             parseConfiguration(Config)) {
       return std::make_unique<ConfigOptionsProvider>(
           GlobalOptions,
           ClangTidyOptions::getDefaults().mergeWith(DefaultOptions, 0),
-          *ParsedConfig, OverrideOptions);
+          *ParsedConfig, OverrideOptions, std::move(FS));
     } else {
       llvm::errs() << "Error: invalid configuration specified.\n"
                    << ParsedConfig.getError().message() << "\n";
       return nullptr;
     }
   }
   return std::make_unique<FileOptionsProvider>(GlobalOptions, DefaultOptions,
                                                 OverrideOptions, std::move(FS));

clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp

 // RUN: clang-tidy -dump-config -checks='from-command-line' -header-filter='from command line' %S/Inputs/config-files/- -- | FileCheck %s -check-prefix=CHECK-COMMAND-LINE
 // CHECK-COMMAND-LINE: Checks: {{.*}}from-parent,from-command-line
 // CHECK-COMMAND-LINE: HeaderFilterRegex: from command line
 
 // For this test we have to use names of the real checks because otherwise values are ignored.
 // RUN: clang-tidy -dump-config %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD4
 // CHECK-CHILD4: Checks: {{.*}}modernize-loop-convert,modernize-use-using,llvm-qualified-auto
 // CHECK-CHILD4: - key: llvm-qualified-auto.AddConstToQualified
-// CHECK-CHILD4-NEXT: value: '1
+// CHECK-CHILD4-NEXT: value: '1'
 // CHECK-CHILD4: - key: modernize-loop-convert.MaxCopySize
 // CHECK-CHILD4-NEXT: value: '20'
 // CHECK-CHILD4: - key: modernize-loop-convert.MinConfidence
 // CHECK-CHILD4-NEXT: value: reasonable
 // CHECK-CHILD4: - key: modernize-use-using.IgnoreMacros
 // CHECK-CHILD4-NEXT: value: '0'
 
 // RUN: clang-tidy --explain-config %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-EXPLAIN
 // CHECK-EXPLAIN: 'llvm-qualified-auto' is enabled in the {{.*}}{{[/\\]}}Inputs{{[/\\]}}config-files{{[/\\]}}4{{[/\\]}}44{{[/\\]}}.clang-tidy.
 // CHECK-EXPLAIN: 'modernize-loop-convert' is enabled in the {{.*}}{{[/\\]}}Inputs{{[/\\]}}config-files{{[/\\]}}4{{[/\\]}}.clang-tidy.
 // CHECK-EXPLAIN: 'modernize-use-using' is enabled in the {{.*}}{{[/\\]}}Inputs{{[/\\]}}config-files{{[/\\]}}4{{[/\\]}}.clang-tidy.
+
+// RUN: clang-tidy -dump-config \
+// RUN: --config='{InheritParentConfig: true, \
+// RUN: Checks: -llvm-qualified-auto, \
+// RUN: CheckOptions: [{key: modernize-loop-convert.MaxCopySize, value: 21}]}' \
+// RUN: %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD5
+// CHECK-CHILD5: Checks: {{.*}}modernize-loop-convert,modernize-use-using,llvm-qualified-auto,-llvm-qualified-auto
+// CHECK-CHILD5: - key: modernize-loop-convert.MaxCopySize
+// CHECK-CHILD5-NEXT: value: '21'
+// CHECK-CHILD5: - key: modernize-loop-convert.MinConfidence
+// CHECK-CHILD5-NEXT: value: reasonable
+// CHECK-CHILD5: - key: modernize-use-using.IgnoreMacros
+// CHECK-CHILD5-NEXT: value: '0'
+
+// RUN: clang-tidy -dump-config \
+// RUN: --config='{InheritParentConfig: false, \
+// RUN: Checks: -llvm-qualified-auto}' \
+// RUN: %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD6
+// CHECK-CHILD6: Checks: {{.*}}-llvm-qualified-auto
+// CHECK-CHILD6-NOT: - key: modernize-use-using.IgnoreMacros