[clang-tidy] Add support for loading configs for specific files.

[njames93]

Summary

There was already a basic version of this in IdentifierNamingCheck, controlled by an option GetConfigPerFile. This has now been changed to a global option UsePerFileConfig. As there hasn't been a release with GetConfigPerFile, This change shouldn't break anyones config.

Now there is support for this built into the base class directly, along with some basic caching.
As the clang-tidy providers used only have at best directory resolution, this caches based on directory rather than filename. This can potentially save alot of redundant config lookups.

[sammccall]

I get the motivation for wanting to have a good, accessible way to do this thing that is possible today, but at the same time... I'm not sure we should pave this cow path, or even keep it.

Clang-tidy's configuration system is *really* complicated, and I don't think the complexity justified by the value it brings.
Currently AFAICS there's just *one* check that ever reads the config for other files, and it could be redesigned not to do so - by conservatively renaming only identifiers declared in the current file.

As you're aware Nathan but others aren't - this ability to request config for other files was a significant source of difficulty in D91029 - i.e. complexity of embedding clang-tidy.
Other features like inheritance, local vs global names, check option value priority, ConfigFileHandlers etc have contributed to a really complex model and greatly hinder the prospect of replacing ClangTidyOptionsProvider with something simpler, which is sorely needed IMO.

[njames93]

In D92175#2418651, @sammccall wrote:

Currently AFAICS there's just *one* check that ever reads the config for other files, and it could be redesigned not to do so - by conservatively renaming only identifiers declared in the current file.

You've missed the point behind this. The purpose for this functionality is clang-tidy (usually) only gets ran on implementation files, This makes it hard to enforce specific checks on header files.
You're best bet is hoping that all the header files you include are using the same .clang-tidy config.
This gets even more challenging when using the run_clang_tidy script over a compilation database. If there isn't one single project wide configuration, You'll likely get conflicting errors and fixes.
Also there is only *one* check at the moment.

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

private:
   ClangTidyContext *Context;
 
 protected:
   OptionsView Options;
   /// Returns the main file name of the current translation unit.
   StringRef getCurrentMainFile() const { return Context->getCurrentFile(); }
   /// Returns the language options from the context.
   const LangOptions &getLangOpts() const { return Context->getLangOpts(); }
+  /// Returns the options if clang-tidy had been ran on \p Filename.
+  OptionsView getOptionsForFile(StringRef Filename) const;
+  /// Returns true if this check would be enabled if clang-tidy had been ran on
+  /// \p Filename.
+  bool isCheckEnabledForFile(StringRef Filename) const;
 };
 
 /// Read a named option from the ``Context`` and parse it as a bool.
 ///
 /// Reads the option with the check-local name \p LocalName from the
 /// ``CheckOptions``. If the corresponding key is not present, returns
 /// a ``MissingOptionError``. If the corresponding key can't be parsed as
 /// a bool, return an ``UnparseableIntegerOptionError``.

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

 //===--- ClangTidyCheck.cpp - clang-tidy ------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "ClangTidyCheck.h"
+#include "GlobList.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace clang {
 namespace tidy {
ClangTidyCheck::OptionsView::getOptionalLocalOrGlobal<std::string>(
     StringRef LocalName) const {
   if (auto ValueOr = getLocalOrGlobal(LocalName))
     return *ValueOr;
   else
     consumeError(ValueOr.takeError());
   return llvm::None;
 }
 
+ClangTidyCheck::OptionsView
+ClangTidyCheck::getOptionsForFile(StringRef Filename) const {
+  return OptionsView(CheckName,
+                     Context->getOptionsForFile(Filename).CheckOptions);
+}
+
+bool ClangTidyCheck::isCheckEnabledForFile(StringRef Filename) const {
+  auto Checks = Context->getOptionsForFile(Filename).Checks;
+  if (!Checks)
+    return false;
+  return GlobList(*Checks).contains(CheckName);
+}
 } // namespace tidy
 } // namespace clang

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

public:
 
   /// Returns options for \c CurrentFile.
   ///
   /// The \c CurrentFile can be changed using \c setCurrentFile.
   const ClangTidyOptions &getOptions() const;
 
   /// Returns options for \c File. Does not change or depend on
   /// \c CurrentFile.
-  ClangTidyOptions getOptionsForFile(StringRef File) const;
+  const ClangTidyOptions &getOptionsForFile(StringRef File) const;
 
   /// Returns \c ClangTidyStats containing issued and ignored diagnostic
   /// counters.
   const ClangTidyStats &getStats() const { return Stats; }
 
   /// Control profile collection in clang-tidy.
   void setEnableProfiling(bool Profile);
   bool getEnableProfiling() const { return Profile; }
private:
   std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider;
 
   std::string CurrentFile;
   ClangTidyOptions CurrentOptions;
   class CachedGlobList;
   std::unique_ptr<CachedGlobList> CheckFilter;
   std::unique_ptr<CachedGlobList> WarningAsErrorFilter;
 
+  mutable llvm::StringMap<ClangTidyOptions> CachedDirectoryOptions;
+
   LangOptions LangOpts;
 
   ClangTidyStats Stats;
 
   std::string CurrentBuildDirectory;
 
   llvm::DenseMap<unsigned, std::string> CheckNamesByDiagnosticID;
 

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

 const ClangTidyGlobalOptions &ClangTidyContext::getGlobalOptions() const {
   return OptionsProvider->getGlobalOptions();
 }
 
 const ClangTidyOptions &ClangTidyContext::getOptions() const {
   return CurrentOptions;
 }
 
-ClangTidyOptions ClangTidyContext::getOptionsForFile(StringRef File) const {
+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);
+  StringRef Directory = llvm::sys::path::parent_path(File);
+  auto Iter = CachedDirectoryOptions.find(Directory);
+  bool Inserted;
+  if (Iter == CachedDirectoryOptions.end()) {
+    std::tie(Iter, Inserted) = CachedDirectoryOptions.insert_or_assign(
+        Directory, ClangTidyOptions::getDefaults().merge(
+                       OptionsProvider->getOptions(File), 0));
+    assert(Inserted);
+  }
+  return Iter->getValue();
 }
 
 void ClangTidyContext::setEnableProfiling(bool P) { Profile = P; }
 
 void ClangTidyContext::setProfileStoragePrefix(StringRef Prefix) {
   ProfilePrefix = std::string(Prefix);
 }
 

clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h

DiagInfo GetDiagInfo(const NamingCheckId &ID,
                        const NamingCheckFailure &Failure) const override;
 
   const FileStyle &getStyleForFile(StringRef FileName) const;
 
   /// Stores the style options as a vector, indexed by the specified \ref
   /// StyleKind, for a given directory.
   mutable llvm::StringMap<FileStyle> NamingStylesCache;
   FileStyle *MainFileStyle;
-  ClangTidyContext *const Context;
-  const std::string CheckName;
-  const bool GetConfigPerFile;
+  const bool UsePerFileConfig;
   const bool IgnoreFailedSplit;
 };
 
 } // namespace readability
 template <>
 struct OptionEnumMapping<readability::IdentifierNamingCheck::CaseType> {
   static llvm::ArrayRef<
       std::pair<readability::IdentifierNamingCheck::CaseType, StringRef>>
   getEnumMapping();
 };
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_IDENTIFIERNAMINGCHECK_H

clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp

if (CaseOptional || !Prefix.empty() || !Postfix.empty() ||
                         std::move(Postfix), std::move(IgnoredRegexpStr));
   }
   bool IgnoreMainLike = Options.get("IgnoreMainLikeFunctions", false);
   return {std::move(Styles), IgnoreMainLike};
 }
 
 IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name,
                                              ClangTidyContext *Context)
-    : RenamerClangTidyCheck(Name, Context), Context(Context), CheckName(Name),
-      GetConfigPerFile(Options.get("GetConfigPerFile", true)),
+    : RenamerClangTidyCheck(Name, Context),
+      UsePerFileConfig(Options.getLocalOrGlobal("UsePerFileConfig", true)),
       IgnoreFailedSplit(Options.get("IgnoreFailedSplit", false)) {
 
   auto IterAndInserted = NamingStylesCache.try_emplace(
       llvm::sys::path::parent_path(Context->getCurrentFile()),
       getFileStyleFromOptions(Options));
   assert(IterAndInserted.second && "Couldn't insert Style");
   // Holding a reference to the data in the vector is safe as it should never
   // move.
for (size_t I = 0; I < SK_Count; ++I) {
     Options.store(Opts, StyleString, Styles[I]->Suffix);
     if (Styles[I]->Case) {
       memcpy(&StyleString[StyleSize], "Case", 4);
       StyleString.pop_back();
       StyleString.pop_back();
       Options.store(Opts, StyleString, *Styles[I]->Case);
     }
   }
-  Options.store(Opts, "GetConfigPerFile", GetConfigPerFile);
+  Options.store(Opts, "UsePerFileConfig", UsePerFileConfig);
   Options.store(Opts, "IgnoreFailedSplit", IgnoreFailedSplit);
   Options.store(Opts, "IgnoreMainLikeFunctions",
                 MainFileStyle->isIgnoringMainLikeFunction());
 }
 
 static bool matchesStyle(StringRef Name,
                          const IdentifierNamingCheck::NamingStyle &Style) {
   static llvm::Regex Matchers[] = {
IdentifierNamingCheck::GetDiagInfo(const NamingCheckId &ID,
   return DiagInfo{"invalid case style for %0 '%1'",
                   [&](DiagnosticBuilder &Diag) {
                     Diag << Failure.Info.KindName << ID.second;
                   }};
 }
 
 const IdentifierNamingCheck::FileStyle &
 IdentifierNamingCheck::getStyleForFile(StringRef FileName) const {
-  if (!GetConfigPerFile)
+  if (!UsePerFileConfig)
     return *MainFileStyle;
   StringRef Parent = llvm::sys::path::parent_path(FileName);
   auto Iter = NamingStylesCache.find(Parent);
   if (Iter != NamingStylesCache.end())
     return Iter->getValue();
 
-  ClangTidyOptions Options = Context->getOptionsForFile(FileName);
-  if (Options.Checks && GlobList(*Options.Checks).contains(CheckName)) {
+  if (isCheckEnabledForFile(FileName)) {
     auto It = NamingStylesCache.try_emplace(
-        Parent, getFileStyleFromOptions({CheckName, Options.CheckOptions}));
+        Parent, getFileStyleFromOptions(getOptionsForFile(FileName)));
     assert(It.second);
     return It.first->getValue();
   }
   // Default construction gives an empty style.
   auto It = NamingStylesCache.try_emplace(Parent);
   assert(It.second);
   return It.first->getValue();
 }
 
 } // namespace readability
 } // namespace tidy
 } // namespace clang

clang-tools-extra/clangd/TidyProvider.cpp

 #include "Config.h"
 #include "support/Logger.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/VirtualFileSystem.h"
+#include <limits>
 #include <memory>
 
 namespace clang {
 namespace clangd {
 
 static void mergeCheckList(llvm::Optional<std::string> &Checks,
                            llvm::StringRef List) {
   if (List.empty())
if (LLVM_LIKELY(Str.front() != '-'))
       DisableGlob.push_back('-');
     DisableGlob += Str;
   }
 
   return [DisableList(std::move(DisableGlob))](tidy::ClangTidyOptions &Opts,
                                                llvm::StringRef) {
     if (Opts.Checks && !Opts.Checks->empty())
       Opts.Checks->append(DisableList);
+
+    // Force any tidy check that uses UsePreFileConfig to disable it. We only
+    // care about diagnostics in the main file so looking for configs in other
+    // files is redundant.
+    Opts.CheckOptions.insert_or_assign(
+        "UsePerFileConfig", tidy::ClangTidyOptions::ClangTidyValue(
+                                "false", std::numeric_limits<unsigned>::max()));
   };
 }
 
 TidyProviderRef provideClangdConfig() {
   return [](tidy::ClangTidyOptions &Opts, llvm::StringRef) {
     const auto &CurTidyConfig = Config::current().ClangTidy;
     if (!CurTidyConfig.Checks.empty())
       mergeCheckList(Opts.Checks, CurTidyConfig.Checks);

clang-tools-extra/docs/ReleaseNotes.rst

 - Improved :doc:`modernize-loop-convert
   <clang-tidy/checks/modernize-loop-convert>` check.
 
   Now able to transform iterator loops using ``rbegin`` and ``rend`` methods.
 
 - Improved :doc:`readability-identifier-naming
   <clang-tidy/checks/readability-identifier-naming>` check.
 
-  Added an option `GetConfigPerFile` to support including files which use
+  Added an option `UsePerFileConfig` to support including files which use
   different naming styles.
 
   Now renames overridden virtual methods if the method they override has a
   style violation.
   
   Added support for specifying the style of scoped ``enum`` constants. If 
   unspecified, will fall back to the style for regular ``enum`` constants.
 

clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst

  - :option:`ConstantParameterCase`, :option:`ConstantParameterPrefix`, :option:`ConstantParameterSuffix`, :option:`ConstantParameterIgnoredRegexp`
  - :option:`ConstantPointerParameterCase`, :option:`ConstantPointerParameterPrefix`, :option:`ConstantPointerParameterSuffix`, :option:`ConstantPointerParameterIgnoredRegexp`
  - :option:`ConstexprFunctionCase`, :option:`ConstexprFunctionPrefix`, :option:`ConstexprFunctionSuffix`, :option:`ConstexprFunctionIgnoredRegexp`
  - :option:`ConstexprMethodCase`, :option:`ConstexprMethodPrefix`, :option:`ConstexprMethodSuffix`, :option:`ConstexprMethodIgnoredRegexp`
  - :option:`ConstexprVariableCase`, :option:`ConstexprVariablePrefix`, :option:`ConstexprVariableSuffix`, :option:`ConstexprVariableIgnoredRegexp`
  - :option:`EnumCase`, :option:`EnumPrefix`, :option:`EnumSuffix`, :option:`EnumIgnoredRegexp`
  - :option:`EnumConstantCase`, :option:`EnumConstantPrefix`, :option:`EnumConstantSuffix`, :option:`EnumConstantIgnoredRegexp`
  - :option:`FunctionCase`, :option:`FunctionPrefix`, :option:`FunctionSuffix`, :option:`FunctionIgnoredRegexp`
- - :option:`GetConfigPerFile`
  - :option:`GlobalConstantCase`, :option:`GlobalConstantPrefix`, :option:`GlobalConstantSuffix`, :option:`GlobalConstantIgnoredRegexp`
  - :option:`GlobalConstantPointerCase`, :option:`GlobalConstantPointerPrefix`, :option:`GlobalConstantPointerSuffix`, :option:`GlobalConstantPointerIgnoredRegexp`
  - :option:`GlobalFunctionCase`, :option:`GlobalFunctionPrefix`, :option:`GlobalFunctionSuffix`, :option:`GlobalFunctionIgnoredRegexp`
  - :option:`GlobalPointerCase`, :option:`GlobalPointerPrefix`, :option:`GlobalPointerSuffix`, :option:`GlobalPointerIgnoredRegexp`
  - :option:`GlobalVariableCase`, :option:`GlobalVariablePrefix`, :option:`GlobalVariableSuffix`, :option:`GlobalVariableIgnoredRegexp`
  - :option:`IgnoreMainLikeFunctions`
  - :option:`InlineNamespaceCase`, :option:`InlineNamespacePrefix`, :option:`InlineNamespaceSuffix`, :option:`InlineNamespaceIgnoredRegexp`
  - :option:`LocalConstantCase`, :option:`LocalConstantPrefix`, :option:`LocalConstantSuffix`, :option:`LocalConstantIgnoredRegexp`

  - :option:`StaticVariableCase`, :option:`StaticVariablePrefix`, :option:`StaticVariableSuffix`, :option:`StaticVariableIgnoredRegexp`
  - :option:`StructCase`, :option:`StructPrefix`, :option:`StructSuffix`, :option:`StructIgnoredRegexp`
  - :option:`TemplateParameterCase`, :option:`TemplateParameterPrefix`, :option:`TemplateParameterSuffix`, :option:`TemplateParameterIgnoredRegexp`
  - :option:`TemplateTemplateParameterCase`, :option:`TemplateTemplateParameterPrefix`, :option:`TemplateTemplateParameterSuffix`, :option:`TemplateTemplateParameterIgnoredRegexp`
  - :option:`TypeAliasCase`, :option:`TypeAliasPrefix`, :option:`TypeAliasSuffix`, :option:`TypeAliasIgnoredRegexp`
  - :option:`TypedefCase`, :option:`TypedefPrefix`, :option:`TypedefSuffix`, :option:`TypedefIgnoredRegexp`
  - :option:`TypeTemplateParameterCase`, :option:`TypeTemplateParameterPrefix`, :option:`TypeTemplateParameterSuffix`, :option:`TypeTemplateParameterIgnoredRegexp`
  - :option:`UnionCase`, :option:`UnionPrefix`, :option:`UnionSuffix`, :option:`UnionIgnoredRegexp`
+ - :option:`UsePerFileConfig`
  - :option:`ValueTemplateParameterCase`, :option:`ValueTemplateParameterPrefix`, :option:`ValueTemplateParameterSuffix`, :option:`ValueTemplateParameterIgnoredRegexp`
  - :option:`VariableCase`, :option:`VariablePrefix`, :option:`VariableSuffix`, :option:`VariableIgnoredRegexp`
  - :option:`VirtualMethodCase`, :option:`VirtualMethodPrefix`, :option:`VirtualMethodSuffix`, :option:`VirtualMethodIgnoredRegexp`
 
 .. option:: AbstractClassCase
 
     When defined, the check will ensure abstract class names conform to the
     selected casing.
.. code-block:: c++
     char MY_Function_string();
 
 After:
 
 .. code-block:: c++
 
     char pre_my_function_string_post();
 
-.. option:: GetConfigPerFile
-
-    When `true` the check will look for the configuration for where an
-    identifier is declared. Useful for when included header files use a 
-    different style. 
-    Default value is `true`.
-
 .. option:: GlobalConstantCase
 
     When defined, the check will ensure global constant names conform to the
     selected casing.
 
 .. option:: GlobalConstantPrefix
 
     When defined, the check will ensure global constant names will add the

 
 .. code-block:: c++
 
     union pre_foo_post {
       int a;
       char b;
     };
 
+.. option:: UsePerFileConfig
+
+    When `true` the check will look for the configuration for where an
+    identifier is declared. Useful for when included header files use a 
+    different style. 
+    Default value is `true`.
+
 .. option:: ValueTemplateParameterCase
 
     When defined, the check will ensure value template parameter names conform to the
     selected casing.
 
 .. option:: ValueTemplateParameterPrefix
 
     When defined, the check will ensure value template parameter names will add the

clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-multiple-styles.cpp

 // Setup header directory
 
 // RUN: rm -rf %theaders
 // RUN: mkdir %theaders
 // RUN: cp -R %S/Inputs/readability-identifier-naming/. %theaders
 
 // C++11 isn't explicitly required, but failing to specify a standard means the
 // check will run multiple times for different standards. This will cause the
 // second test to fail as the header file will be changed during the first run.
 // InheritParentConfig is needed to look for the clang-tidy configuration files.
 
 // RUN: %check_clang_tidy -check-suffixes=ENABLED,SHARED -std=c++11 %s \
 // RUN: readability-identifier-naming %t -- \
 // RUN:  -config='{ InheritParentConfig: true, CheckOptions: [ \
 // RUN:   {key: readability-identifier-naming.FunctionCase, value: camelBack}, \
 // RUN:   {key: readability-identifier-naming.ParameterCase, value: CamelCase}, \
-// RUN:   {key: readability-identifier-naming.GetConfigPerFile, value: true} \
+// RUN:   {key: readability-identifier-naming.UsePerFileConfig, value: true} \
 // RUN:  ]}' -header-filter='.*' -- -I%theaders
 
 // On DISABLED run, everything should be made 'camelBack'.
 
 // RUN: cp -R %S/Inputs/readability-identifier-naming/. %theaders
 // RUN: %check_clang_tidy -check-suffixes=DISABLED,SHARED -std=c++11 %s \
 // RUN: readability-identifier-naming %t -- \
 // RUN:  -config='{ InheritParentConfig: false, CheckOptions: [ \
 // RUN:   {key: readability-identifier-naming.FunctionCase, value: camelBack}, \
 // RUN:   {key: readability-identifier-naming.ParameterCase, value: CamelCase}, \
-// RUN:   {key: readability-identifier-naming.GetConfigPerFile, value: false} \
+// RUN:   {key: readability-identifier-naming.UsePerFileConfig, value: false} \
 // RUN:  ]}' -header-filter='.*' -- -I%theaders
 
 #include "global-style1/header.h"
 #include "global-style2/header.h"
 
 void goodStyle() {
   style_first_good();
   STYLE_SECOND_GOOD();