[clang-tidy] Extend InheritParentConfig to CommandLineConfig

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.

Reviewed By: DmitryPolukhin

Differential Revision: https://reviews.llvm.org/D81949

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

@@ -194,14 +194,27 @@ ConfigOptionsProvider::ConfigOptionsProvider(
     const ClangTidyGlobalOptions &GlobalOptions,
     const ClangTidyOptions &DefaultOptions,
     const ClangTidyOptions &ConfigOptions,
-    const ClangTidyOptions &OverrideOptions)
+    const ClangTidyOptions &OverrideOptions,
-    : DefaultOptionsProvider(GlobalOptions, DefaultOptions),
+    llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS)
-      ConfigOptions(ConfigOptions), OverrideOptions(OverrideOptions) {}
+    : 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,
@@ -209,7 +222,7 @@ ConfigOptionsProvider::getRawOptions(llvm::StringRef FileName) {
   return RawOptions;
 }
 
-FileOptionsProvider::FileOptionsProvider(
+FileOptionsBaseProvider::FileOptionsBaseProvider(
     const ClangTidyGlobalOptions &GlobalOptions,
     const ClangTidyOptions &DefaultOptions,
     const ClangTidyOptions &OverrideOptions,
@@ -221,36 +234,21 @@ FileOptionsProvider::FileOptionsProvider(
   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().
+void FileOptionsBaseProvider::addRawFileOptions(
-// Consider pulling out common bits to a findParentFileWithName function or
+    llvm::StringRef AbsolutePath, std::vector<OptionsSource> &CurOptions) {
-// similar.
+  auto CurSize = CurOptions.size();
-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());
-  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;
@@ -273,21 +271,58 @@ FileOptionsProvider::getRawOptions(StringRef FileName) {
       }
       CachedOptions[Path] = *Result;
 
-      RawOptions.push_back(*Result);
+      CurOptions.push_back(*Result);
-      if (!Result->first.InheritParentConfig ||
+      if (!Result->first.InheritParentConfig.getValueOr(false))
-          !*Result->first.InheritParentConfig)
         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)) {

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

@@ -121,10 +121,13 @@ struct ClangTidyOptions {
   /// Add extra compilation arguments to the start of the list.
   llvm::Optional<ArgList> ExtraArgsBefore;
 
-  /// Only used in the FileOptionsProvider. If true, FileOptionsProvider will
+  /// Only used in the FileOptionsProvider and ConfigOptionsProvider. If true
-  /// take a configuration file in the parent directory (if any exists) and
+  /// and using a FileOptionsProvider, it will take a configuration file in the
-  /// apply this config file on top of the parent one. If false or missing,
+  /// parent directory (if any exists) and apply this config file on top of the
-  /// only this configuration file will be used.
+  /// 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.
@@ -180,30 +183,7 @@ private:
   ClangTidyOptions DefaultOptions;
 };
 
-/// Implementation of ClangTidyOptions interface, which is used for
+class FileOptionsBaseProvider : public DefaultOptionsProvider {
-/// '-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 {
 public:
   // A pair of configuration file base name and a function parsing
   // configuration from text in the corresponding format.
@@ -230,6 +210,57 @@ public:
   /// 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
@@ -269,16 +300,6 @@ public:
                       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.

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

@@ -310,7 +310,7 @@ static std::unique_ptr<ClangTidyOptionsProvider> createOptionsProvider(
       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";

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

@@ -18,7 +18,7 @@
 // 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
@@ -30,3 +30,23 @@
 // 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