Index: clang-tidy/ClangTidy.h =================================================================== --- clang-tidy/ClangTidy.h +++ clang-tidy/ClangTidy.h @@ -118,10 +118,8 @@ std::vector getCheckNames(const ClangTidyOptions &Options); /// \brief Run a set of clang-tidy checks on a set of files. -/// -/// Takes ownership of the \c OptionsProvider. ClangTidyStats -runClangTidy(ClangTidyOptionsProvider *OptionsProvider, +runClangTidy(std::unique_ptr OptionsProvider, const tooling::CompilationDatabase &Compilations, ArrayRef InputFiles, std::vector *Errors); Index: clang-tidy/ClangTidy.cpp =================================================================== --- clang-tidy/ClangTidy.cpp +++ clang-tidy/ClangTidy.cpp @@ -313,17 +313,19 @@ std::vector getCheckNames(const ClangTidyOptions &Options) { clang::tidy::ClangTidyContext Context( - new DefaultOptionsProvider(ClangTidyGlobalOptions(), Options)); + llvm::make_unique(ClangTidyGlobalOptions(), + Options)); ClangTidyASTConsumerFactory Factory(Context); return Factory.getCheckNames(Context.getChecksFilter()); } -ClangTidyStats runClangTidy(ClangTidyOptionsProvider *OptionsProvider, - const tooling::CompilationDatabase &Compilations, - ArrayRef InputFiles, - std::vector *Errors) { +ClangTidyStats +runClangTidy(std::unique_ptr OptionsProvider, + const tooling::CompilationDatabase &Compilations, + ArrayRef InputFiles, + std::vector *Errors) { ClangTool Tool(Compilations, InputFiles); - clang::tidy::ClangTidyContext Context(OptionsProvider); + clang::tidy::ClangTidyContext Context(std::move(OptionsProvider)); ClangTidyDiagnosticConsumer DiagConsumer(Context); Tool.setDiagnosticConsumer(&DiagConsumer); Index: clang-tidy/ClangTidyDiagnosticConsumer.h =================================================================== --- clang-tidy/ClangTidyDiagnosticConsumer.h +++ clang-tidy/ClangTidyDiagnosticConsumer.h @@ -117,9 +117,7 @@ class ClangTidyContext { public: /// \brief Initializes \c ClangTidyContext instance. - /// - /// Takes ownership of the \c OptionsProvider. - ClangTidyContext(ClangTidyOptionsProvider *OptionsProvider); + ClangTidyContext(std::unique_ptr OptionsProvider); /// \brief Report any errors detected using this method. /// @@ -180,6 +178,7 @@ std::unique_ptr OptionsProvider; std::string CurrentFile; + ClangTidyOptions CurrentOptions; std::unique_ptr CheckFilter; ClangTidyStats Stats; Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp =================================================================== --- clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -160,8 +160,9 @@ return Contains; } -ClangTidyContext::ClangTidyContext(ClangTidyOptionsProvider *OptionsProvider) - : DiagEngine(nullptr), OptionsProvider(OptionsProvider) { +ClangTidyContext::ClangTidyContext( + std::unique_ptr OptionsProvider) + : DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)) { // Before the first translation unit we can get errors related to command-line // parsing, use empty string for the file name in this case. setCurrentFile(""); @@ -202,7 +203,10 @@ void ClangTidyContext::setCurrentFile(StringRef File) { CurrentFile = File; - CheckFilter.reset(new GlobList(getOptions().Checks)); + // Safeguard against options with unset values. + CurrentOptions = ClangTidyOptions::getDefaults().mergeWith( + OptionsProvider->getOptions(CurrentFile)); + CheckFilter.reset(new GlobList(*getOptions().Checks)); } void ClangTidyContext::setASTContext(ASTContext *Context) { @@ -214,7 +218,7 @@ } const ClangTidyOptions &ClangTidyContext::getOptions() const { - return OptionsProvider->getOptions(CurrentFile); + return CurrentOptions; } GlobList &ClangTidyContext::getChecksFilter() { @@ -328,7 +332,7 @@ const Preprocessor *PP) { // Before the first translation unit we don't need HeaderFilter, as we // shouldn't get valid source locations in diagnostics. - HeaderFilter.reset(new llvm::Regex(Context.getOptions().HeaderFilterRegex)); + HeaderFilter.reset(new llvm::Regex(*Context.getOptions().HeaderFilterRegex)); } bool ClangTidyDiagnosticConsumer::passesLineFilter(StringRef FileName, Index: clang-tidy/ClangTidyOptions.h =================================================================== --- clang-tidy/ClangTidyOptions.h +++ clang-tidy/ClangTidyOptions.h @@ -10,7 +10,10 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANG_TIDY_OPTIONS_H #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANG_TIDY_OPTIONS_H +#include "llvm/ADT/Optional.h" +#include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/ErrorOr.h" #include #include #include @@ -42,18 +45,31 @@ /// \brief Contains options for clang-tidy. These options may be read from /// configuration files, and may be different for different translation units. struct ClangTidyOptions { - /// \brief Allow all checks and no headers by default. - ClangTidyOptions() : Checks("*"), AnalyzeTemporaryDtors(false) {} + /// \brief These options are used for all settings that haven't been + /// overridden by the \c OptionsProvider. + /// + /// Allow no checks and no headers by default. + static ClangTidyOptions getDefaults() { + ClangTidyOptions Options; + Options.Checks = ""; + Options.HeaderFilterRegex = ""; + Options.AnalyzeTemporaryDtors = false; + return Options; + } + + /// \brief Creates a new \c ClangTidyOptions instance combined from all fields + /// of this instance overridden by the fields of \p Other that have a value. + ClangTidyOptions mergeWith(const ClangTidyOptions &Other) const; /// \brief Checks filter. - std::string Checks; + llvm::Optional Checks; /// \brief Output warnings from headers matching this filter. Warnings from /// main files will always be displayed. - std::string HeaderFilterRegex; + llvm::Optional HeaderFilterRegex; /// \brief Turns on temporary destructor-based analysis. - bool AnalyzeTemporaryDtors; + llvm::Optional AnalyzeTemporaryDtors; }; /// \brief Abstract interface for retrieving various ClangTidy options. @@ -79,7 +95,7 @@ const ClangTidyGlobalOptions &getGlobalOptions() override { return GlobalOptions; } - const ClangTidyOptions &getOptions(llvm::StringRef) override { + const ClangTidyOptions &getOptions(llvm::StringRef /*FileName*/) override { return DefaultOptions; } @@ -88,13 +104,45 @@ ClangTidyOptions DefaultOptions; }; +/// \brief Implementation of the \c ClangTidyOptionsProvider interface, which +/// tries to find a .clang-tidy file in the closest parent directory of each +/// file. +class FileOptionsProvider : public DefaultOptionsProvider { +public: + /// \brief Initializes the \c FileOptionsProvider instance. + /// + /// \param GlobalOptions are just stored and returned to the caller of + /// \c getGlobalOptions. + /// + /// \param FallbackOptions are used in case there's no corresponding + /// .clang-tidy file. + /// + /// If any of the \param OverrideOptions fields are set, they will override + /// whatever options are read from the configuration file. + FileOptionsProvider(const ClangTidyGlobalOptions &GlobalOptions, + const ClangTidyOptions &FallbackOptions, + const ClangTidyOptions &OverrideOptions); + const ClangTidyOptions &getOptions(llvm::StringRef FileName) override; + +private: + /// \brief Try to read configuration file from \p Directory. If \p Directory + /// is empty, use the fallback value. + llvm::ErrorOr TryReadConfigFile(llvm::StringRef Directory); + + llvm::StringMap CachedOptions; + ClangTidyOptions OverrideOptions; +}; + /// \brief Parses LineFilter from JSON and stores it to the \p Options. -std::error_code parseLineFilter(const std::string &LineFilter, - clang::tidy::ClangTidyGlobalOptions &Options); +std::error_code parseLineFilter(llvm::StringRef LineFilter, + ClangTidyGlobalOptions &Options); /// \brief Parses configuration from JSON and stores it to the \p Options. -std::error_code parseConfiguration(const std::string &Config, - clang::tidy::ClangTidyOptions &Options); +std::error_code parseConfiguration(llvm::StringRef Config, + ClangTidyOptions &Options); + +/// \brief Serializes configuration to a YAML-encoded string. +std::string configurationAsText(const ClangTidyOptions &Options); } // end namespace tidy } // end namespace clang Index: clang-tidy/ClangTidyOptions.cpp =================================================================== --- clang-tidy/ClangTidyOptions.cpp +++ clang-tidy/ClangTidyOptions.cpp @@ -8,7 +8,16 @@ //===----------------------------------------------------------------------===// #include "ClangTidyOptions.h" +#include "clang/Basic/LLVM.h" +#include "llvm/ADT/SmallString.h" +#include "llvm/Support/Debug.h" +#include "llvm/Support/FileSystem.h" +#include "llvm/Support/Path.h" +#include "llvm/Support/raw_ostream.h" #include "llvm/Support/YAMLTraits.h" +#include + +#define DEBUG_TYPE "clang-tidy-options" using clang::tidy::ClangTidyOptions; using clang::tidy::FileFilter; @@ -61,20 +70,131 @@ namespace clang { namespace tidy { +ClangTidyOptions +ClangTidyOptions::mergeWith(const ClangTidyOptions &Other) const { + ClangTidyOptions Result = *this; + + // Merge comma-separated glob lists by appending the new value after a comma. + if (Other.Checks) + Result.Checks = + (Result.Checks && !Result.Checks->empty() ? *Result.Checks + "," : "") + + *Other.Checks; + + if (Other.HeaderFilterRegex) \ + Result.HeaderFilterRegex = Other.HeaderFilterRegex; + if (Other.AnalyzeTemporaryDtors) \ + Result.AnalyzeTemporaryDtors = Other.AnalyzeTemporaryDtors; + return Result; +} + +FileOptionsProvider::FileOptionsProvider( + const ClangTidyGlobalOptions &GlobalOptions, + const ClangTidyOptions &FallbackOptions, + const ClangTidyOptions &OverrideOptions) + : DefaultOptionsProvider(GlobalOptions, FallbackOptions), + OverrideOptions(OverrideOptions) { + CachedOptions[""] = FallbackOptions.mergeWith(OverrideOptions); +} + +static const char ConfigFileName[] = ".clang-tidy"; + +const ClangTidyOptions &FileOptionsProvider::getOptions(StringRef FileName) { + DEBUG(llvm::dbgs() << "Getting options for file " << FileName << "...\n"); + SmallString<256> FilePath(FileName); + + if (std::error_code EC = llvm::sys::fs::make_absolute(FilePath)) { + llvm::errs() << "Can't make absolute path from " << FileName << ": " + << EC.message() << "\n"; + // FIXME: Figure out what to do. + } else { + FileName = FilePath; + } + + // 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(FileName); + for (StringRef CurrentPath = Path;; + CurrentPath = llvm::sys::path::parent_path(CurrentPath)) { + llvm::ErrorOr Result = std::error_code(); + + 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) { + DEBUG(llvm::dbgs() << "Caching configuration for path " << Path + << ".\n"); + CachedOptions.GetOrCreateValue(Path, *Result); + Path = llvm::sys::path::parent_path(Path); + } + return CachedOptions.GetOrCreateValue(Path, *Result).getValue(); + } + if (Result.getError() != std::errc::no_such_file_or_directory) { + llvm::errs() << "Error reading " << ConfigFileName << " from " << Path + << ": " << Result.getError().message() << "\n"; + } + } +} + +llvm::ErrorOr +FileOptionsProvider::TryReadConfigFile(StringRef Directory) { + assert(!Directory.empty()); + + ClangTidyOptions Options = DefaultOptionsProvider::getOptions(Directory); + if (!llvm::sys::fs::is_directory(Directory)) + return std::errc::not_a_directory; + + SmallString<128> ConfigFile(Directory); + llvm::sys::path::append(ConfigFile, ".clang-tidy"); + DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n"); + + bool IsFile = false; + // Ignore errors from is_regular_file: we only need to know if we can read + // the file or not. + llvm::sys::fs::is_regular_file(Twine(ConfigFile), IsFile); + + if (!IsFile) + return std::errc::no_such_file_or_directory; + + llvm::ErrorOr> Text = + llvm::MemoryBuffer::getFile(ConfigFile.c_str()); + if (std::error_code EC = Text.getError()) + return EC; + if (std::error_code EC = parseConfiguration((*Text)->getBuffer(), Options)) + return EC; + return Options.mergeWith(OverrideOptions); +} + /// \brief Parses -line-filter option and stores it to the \c Options. -std::error_code parseLineFilter(const std::string &LineFilter, +std::error_code parseLineFilter(StringRef LineFilter, clang::tidy::ClangTidyGlobalOptions &Options) { llvm::yaml::Input Input(LineFilter); Input >> Options.LineFilter; return Input.error(); } -std::error_code parseConfiguration(const std::string &Config, +std::error_code parseConfiguration(StringRef Config, clang::tidy::ClangTidyOptions &Options) { llvm::yaml::Input Input(Config); Input >> Options; return Input.error(); } +std::string configurationAsText(const ClangTidyOptions &Options) { + std::string Text; + llvm::raw_string_ostream Stream(Text); + llvm::yaml::Output Output(Stream); + // We use the same mapping method for input and output, so we need a non-const + // reference here. + ClangTidyOptions NonConstValue = Options; + Output << NonConstValue; + return Stream.str(); +} + } // namespace tidy } // namespace clang Index: clang-tidy/tool/ClangTidyMain.cpp =================================================================== --- clang-tidy/tool/ClangTidyMain.cpp +++ clang-tidy/tool/ClangTidyMain.cpp @@ -26,6 +26,13 @@ static cl::OptionCategory ClangTidyCategory("clang-tidy options"); static cl::extrahelp CommonHelp(CommonOptionsParser::HelpMessage); +static cl::extrahelp ClangTidyHelp( + "Configuration files:\n" + " clang-tidy attempts to read configuration for each source file from a\n" + " .clang-tidy file located in the closest parent directory of the source\n" + " file. If any configuration options have a corresponding command-line\n" + " option, command-line option takes precedence. The effective\n" + " configuration can be inspected using -dump-config.\n\n"); const char DefaultChecks[] = "*," // Enable all checks, except these: @@ -39,7 +46,9 @@ "in the list. Globs without '-' prefix add checks\n" "with matching names to the set, globs with the '-'\n" "prefix remove checks with matching names from the\n" - "set of enabled checks."), + "set of enabled checks.\n" + "This option's value is appended to the value read\n" + "from a .clang-tidy file, if any."), cl::init(""), cl::cat(ClangTidyCategory)); static cl::opt @@ -48,7 +57,9 @@ "headers to output diagnostics from. Diagnostics\n" "from the main file of each translation unit are\n" "always displayed.\n" - "Can be used together with -line-filter."), + "Can be used together with -line-filter.\n" + "This option overrides the value read from a\n" + ".clang-tidy file."), cl::init(""), cl::cat(ClangTidyCategory)); static cl::opt @@ -73,10 +84,17 @@ cl::init(false), cl::cat(ClangTidyCategory)); static cl::opt -AnalyzeTemporaryDtors("analyze-temporary-dtors", - cl::desc("Enable temporary destructor-aware analysis in\n" - "clang-analyzer- checks."), - cl::init(false), cl::cat(ClangTidyCategory)); +DumpConfig("dump-config", + cl::desc("Dumps configuration in the YAML format to stdout."), + cl::init(false), cl::cat(ClangTidyCategory)); + +static cl::opt AnalyzeTemporaryDtors( + "analyze-temporary-dtors", + cl::desc("Enable temporary destructor-aware analysis in\n" + "clang-analyzer- checks.\n" + "This option overrides the value read from a\n" + ".clang-tidy file."), + cl::init(false), cl::cat(ClangTidyCategory)); static void printStats(const clang::tidy::ClangTidyStats &Stats) { if (Stats.errorsIgnored()) { @@ -116,12 +134,25 @@ return 1; } - clang::tidy::ClangTidyOptions Options; - Options.Checks = DefaultChecks + Checks; - Options.HeaderFilterRegex = HeaderFilter; - Options.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors; - - std::vector EnabledChecks = clang::tidy::getCheckNames(Options); + clang::tidy::ClangTidyOptions FallbackOptions; + FallbackOptions.Checks = DefaultChecks; + FallbackOptions.HeaderFilterRegex = HeaderFilter; + FallbackOptions.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors; + + clang::tidy::ClangTidyOptions OverrideOptions; + if (Checks.getNumOccurrences() > 0) + OverrideOptions.Checks = Checks; + if (HeaderFilter.getNumOccurrences() > 0) + OverrideOptions.HeaderFilterRegex = HeaderFilter; + if (AnalyzeTemporaryDtors.getNumOccurrences() > 0) + OverrideOptions.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors; + + auto OptionsProvider = llvm::make_unique( + GlobalOptions, FallbackOptions, OverrideOptions); + + std::string FileName = OptionsParser.getSourcePathList().front(); + std::vector EnabledChecks = + clang::tidy::getCheckNames(OptionsProvider->getOptions(FileName)); // FIXME: Allow using --list-checks without positional arguments. if (ListChecks) { @@ -132,18 +163,23 @@ return 0; } + if (DumpConfig) { + llvm::outs() << clang::tidy::configurationAsText( + clang::tidy::ClangTidyOptions::getDefaults() + .mergeWith(OptionsProvider->getOptions(FileName))) + << "\n"; + return 0; + } + if (EnabledChecks.empty()) { llvm::errs() << "Error: no checks enabled.\n"; llvm::cl::PrintHelpMessage(/*Hidden=*/false, /*Categorized=*/true); return 1; } - // TODO: Implement configuration file reading and a "real" options provider. - auto OptionsProvider = - new clang::tidy::DefaultOptionsProvider(GlobalOptions, Options); std::vector Errors; clang::tidy::ClangTidyStats Stats = clang::tidy::runClangTidy( - OptionsProvider, OptionsParser.getCompilations(), + std::move(OptionsProvider), OptionsParser.getCompilations(), OptionsParser.getSourcePathList(), &Errors); clang::tidy::handleErrors(Errors, Fix); Index: test/clang-tidy/Inputs/config-files/.clang-tidy =================================================================== --- /dev/null +++ test/clang-tidy/Inputs/config-files/.clang-tidy @@ -0,0 +1,2 @@ +Checks: 'from-parent' +HeaderFilterRegex: 'parent' @@ -0,0 +1,2 @@ +Checks: 'from-parent' +HeaderFilterRegex: 'parent' Index: test/clang-tidy/Inputs/config-files/1/.clang-tidy =================================================================== --- /dev/null +++ test/clang-tidy/Inputs/config-files/1/.clang-tidy @@ -0,0 +1,2 @@ +Checks: 'from-child1' +HeaderFilterRegex: 'child1' @@ -0,0 +1,2 @@ +Checks: 'from-child1' +HeaderFilterRegex: 'child1' @@ -0,0 +1,2 @@ +Checks: 'from-child1' +HeaderFilterRegex: 'child1' Index: test/clang-tidy/config-files.cpp =================================================================== --- /dev/null +++ test/clang-tidy/config-files.cpp @@ -0,0 +1,12 @@ +// RUN: clang-tidy -dump-config %S/Inputs/config-files/- -- | FileCheck %s -check-prefix=CHECK-BASE +// CHECK-BASE: Checks: from-parent +// CHECK-BASE: HeaderFilterRegex: parent +// RUN: clang-tidy -dump-config %S/Inputs/config-files/1/- -- | FileCheck %s -check-prefix=CHECK-CHILD1 +// CHECK-CHILD1: Checks: from-child1 +// CHECK-CHILD1: HeaderFilterRegex: child1 +// RUN: clang-tidy -dump-config %S/Inputs/config-files/2/- -- | FileCheck %s -check-prefix=CHECK-CHILD2 +// CHECK-CHILD2: Checks: from-parent +// CHECK-CHILD2: HeaderFilterRegex: parent +// 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 Index: test/clang-tidy/diagnostic.cpp =================================================================== --- test/clang-tidy/diagnostic.cpp +++ test/clang-tidy/diagnostic.cpp @@ -1,5 +1,5 @@ // RUN: clang-tidy -checks='-*,misc-use-override' %s.nonexistent.cpp -- | FileCheck -check-prefix=CHECK1 %s -// RUN: clang-tidy -checks='-google-*,google-explicit-constructor' %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK2 %s +// RUN: clang-tidy -checks='-*,clang-diagnostic-*,google-explicit-constructor' %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK2 %s // RUN: clang-tidy -checks='-*,google-explicit-constructor,clang-diagnostic-literal-conversion' %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK3 %s // RUN: clang-tidy -checks='-*,misc-use-override,clang-diagnostic-macro-redefined' %s -- -DMACRO_FROM_COMMAND_LINE | FileCheck -check-prefix=CHECK4 %s Index: test/clang-tidy/validate-check-names.cpp =================================================================== --- /dev/null +++ test/clang-tidy/validate-check-names.cpp @@ -0,0 +1,2 @@ +// Check names may only contain alphanumeric characters, '-', '_', and '.'. +// RUN: clang-tidy -checks=* -list-checks - -- | grep '^ ' | cut -b5- | not grep -v '^[a-zA-Z0-9_.\-]\+$' Index: unittests/clang-tidy/ClangTidyOptionsTest.cpp =================================================================== --- unittests/clang-tidy/ClangTidyOptionsTest.cpp +++ unittests/clang-tidy/ClangTidyOptionsTest.cpp @@ -61,9 +61,9 @@ "AnalyzeTemporaryDtors: true\n", Options); EXPECT_FALSE(Error); - EXPECT_EQ("-*,misc-*", Options.Checks); - EXPECT_EQ(".*", Options.HeaderFilterRegex); - EXPECT_TRUE(Options.AnalyzeTemporaryDtors); + EXPECT_EQ("-*,misc-*", *Options.Checks); + EXPECT_EQ(".*", *Options.HeaderFilterRegex); + EXPECT_TRUE(*Options.AnalyzeTemporaryDtors); } } // namespace test Index: unittests/clang-tidy/ClangTidyTest.h =================================================================== --- unittests/clang-tidy/ClangTidyTest.h +++ unittests/clang-tidy/ClangTidyTest.h @@ -45,8 +45,10 @@ const Twine &Filename = "input.cc", ArrayRef ExtraArgs = None) { T Check; - ClangTidyContext Context( - new DefaultOptionsProvider(ClangTidyGlobalOptions(), ClangTidyOptions())); + ClangTidyOptions Options; + Options.Checks = "*"; + ClangTidyContext Context(llvm::make_unique( + ClangTidyGlobalOptions(), Options)); ClangTidyDiagnosticConsumer DiagConsumer(Context); Check.setContext(&Context); std::vector ArgCXX11(1, "-std=c++11");