[clang-tools-extra] 4836188 - [clang-tidy] Extend InheritParentConfig to CommandLineConfig
Nathan James via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 19 04:02:30 PDT 2020
Author: Nathan James
Date: 2020-06-19T12:02:19+01:00
New Revision: 4836188ad9b3334b0c1e055d45ccaa54ed797e4b
URL: https://github.com/llvm/llvm-project/commit/4836188ad9b3334b0c1e055d45ccaa54ed797e4b
DIFF: https://github.com/llvm/llvm-project/commit/4836188ad9b3334b0c1e055d45ccaa54ed797e4b.diff
LOG: [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
Added:
Modified:
clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
clang-tools-extra/clang-tidy/ClangTidyOptions.h
clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
index 76cb663c8e2e..4fa78e2803f1 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ b/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)
- : 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,
@@ -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().
-// 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;
@@ -273,21 +271,58 @@ FileOptionsProvider::getRawOptions(StringRef FileName) {
}
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)) {
diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.h b/clang-tools-extra/clang-tidy/ClangTidyOptions.h
index 3eb2fa607cbf..0f3c1d413ec3 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.h
+++ b/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
- /// 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.
@@ -180,30 +183,7 @@ class DefaultOptionsProvider : public ClangTidyOptionsProvider {
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.
@@ -230,6 +210,57 @@ class FileOptionsProvider : public DefaultOptionsProvider {
/// 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 @@ class FileOptionsProvider : public DefaultOptionsProvider {
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.
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index cb1352a7edff..82939faf1ea8 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/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";
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
index 158daad032c5..ee1ed49472ba 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
+++ b/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
More information about the cfe-commits
mailing list