[clang-tidy] Extend IdentifierNamingCheck per file config

[njames93]

Summary

Add IgnoreMainLikeFunctions to the per file config. This can be extended for new options added to the check easily.

[aaron.ballman]

Is there a way to test this functionality out?

[njames93]

In D90832#2375934, @aaron.ballman wrote:

Is there a way to test this functionality out?

There's already tests for loading the individual styles from other files, I'm sure I could extend that

[aaron.ballman]

In D90832#2376009, @njames93 wrote:

In D90832#2375934, @aaron.ballman wrote:

Is there a way to test this functionality out?

There's already tests for loading the individual styles from other files, I'm sure I could extend that

I'd appreciate that, otherwise the changes here all seem reasonable to me so far (though if you wanted to split the formatting changes out into an NFC, those don't even require a review).

[njames93]

Didn't mean for there to be any formatting changes. I'll remove those later too

[njames93]

Removed unnecessary formatting changes
Added test cases.

With the test cases I removed some cases that actually were never tested due to the order in which clang-tidy handles applying configurations. Right now there just isnt the infrastructure needed to fully test those cases.

[aaron.ballman]

LGTM aside from some tiny nits!

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

172

Don't use auto when the type isn't spelled out in the initialization.

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

63

Should end the newline back to the end of the file.

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

NamingStyle(llvm::Optional<CaseType> Case, const std::string &Prefix,
                 const std::string &Suffix)
         : Case(Case), Prefix(Prefix), Suffix(Suffix) {}
 
     llvm::Optional<CaseType> Case;
     std::string Prefix;
     std::string Suffix;
   };
 
+  struct FileStyle {
+    FileStyle() : IsActive(false), IgnoreMainLikeFunctions(false) {}
+    FileStyle(SmallVectorImpl<Optional<NamingStyle>> &&Styles,
+              bool IgnoreMainLike)
+        : Styles(std::move(Styles)), IsActive(true),
+          IgnoreMainLikeFunctions(IgnoreMainLike) {}
+
+    ArrayRef<Optional<NamingStyle>> getStyles() const {
+      assert(IsActive);
+      return Styles;
+    }
+    bool isActive() const { return IsActive; }
+    bool isIgnoringMainLikeFunction() const { return IgnoreMainLikeFunctions; }
+
+  private:
+    SmallVector<Optional<NamingStyle>, 0> Styles;
+    bool IsActive;
+    bool IgnoreMainLikeFunctions;
+  };
+
 private:
   llvm::Optional<FailureInfo>
   GetDeclFailureInfo(const NamedDecl *Decl,
                      const SourceManager &SM) const override;
   llvm::Optional<FailureInfo>
   GetMacroFailureInfo(const Token &MacroNameTok,
                       const SourceManager &SM) const override;
   DiagInfo GetDiagInfo(const NamingCheckId &ID,
                        const NamingCheckFailure &Failure) const override;
 
-  ArrayRef<llvm::Optional<NamingStyle>>
-  getStyleForFile(StringRef FileName) const;
+  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<std::vector<llvm::Optional<NamingStyle>>>
-      NamingStylesCache;
-  ArrayRef<llvm::Optional<NamingStyle>> MainFileStyle;
+  mutable llvm::StringMap<FileStyle> NamingStylesCache;
+  FileStyle *MainFileStyle;
   ClangTidyContext *const Context;
   const std::string CheckName;
   const bool GetConfigPerFile;
   const bool IgnoreFailedSplit;
-  const bool IgnoreMainLikeFunctions;
 };
 
 } // 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

 #define STRINGIZE(v) #v,
   NAMING_KEYS(STRINGIZE)
 #undef STRINGIZE
 };
 
 #undef NAMING_KEYS
 // clang-format on
 
-static std::vector<llvm::Optional<IdentifierNamingCheck::NamingStyle>>
-getNamingStyles(const ClangTidyCheck::OptionsView &Options) {
-  std::vector<llvm::Optional<IdentifierNamingCheck::NamingStyle>> Styles(
+static IdentifierNamingCheck::FileStyle
+getFileStyleFromOptions(const ClangTidyCheck::OptionsView &Options) {
+  SmallVector<llvm::Optional<IdentifierNamingCheck::NamingStyle>, 0> Styles(
       SK_Count);
   SmallString<64> StyleString;
   for (unsigned I = 0; I < SK_Count; ++I) {
     StyleString = StyleNames[I];
     size_t StyleSize = StyleString.size();
     StyleString.append("Prefix");
     std::string Prefix(Options.get(StyleString, ""));
     // Fast replacement of [Pre]fix -> [Suf]fix.
     memcpy(&StyleString[StyleSize], "Suf", 3);
     std::string Postfix(Options.get(StyleString, ""));
     memcpy(&StyleString[StyleSize], "Case", 4);
     StyleString.pop_back();
     StyleString.pop_back();
     auto CaseOptional =
         Options.getOptional<IdentifierNamingCheck::CaseType>(StyleString);
 
     if (CaseOptional || !Prefix.empty() || !Postfix.empty())
       Styles[I].emplace(std::move(CaseOptional), std::move(Prefix),
                         std::move(Postfix));
   }
-  return Styles;
+  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)),
-      IgnoreFailedSplit(Options.get("IgnoreFailedSplit", false)),
-      IgnoreMainLikeFunctions(Options.get("IgnoreMainLikeFunctions", false)) {
+      IgnoreFailedSplit(Options.get("IgnoreFailedSplit", false)) {
 
   auto IterAndInserted = NamingStylesCache.try_emplace(
       llvm::sys::path::parent_path(Context->getCurrentFile()),
-      getNamingStyles(Options));
+      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.
-  MainFileStyle = IterAndInserted.first->getValue();
+  MainFileStyle = &IterAndInserted.first->getValue();
 }
 
 IdentifierNamingCheck::~IdentifierNamingCheck() = default;
 
 void IdentifierNamingCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   RenamerClangTidyCheck::storeOptions(Opts);
   SmallString<64> StyleString;
+  auto Styles = MainFileStyle->getStyles();
   for (size_t I = 0; I < SK_Count; ++I) {
-    if (!MainFileStyle[I])
+    if (!Styles[I])
       continue;
     StyleString = StyleNames[I];
     size_t StyleSize = StyleString.size();
     StyleString.append("Prefix");
-    Options.store(Opts, StyleString, MainFileStyle[I]->Prefix);
+    Options.store(Opts, StyleString, Styles[I]->Prefix);
     // Fast replacement of [Pre]fix -> [Suf]fix.
     memcpy(&StyleString[StyleSize], "Suf", 3);
-    Options.store(Opts, StyleString, MainFileStyle[I]->Suffix);
-    if (MainFileStyle[I]->Case) {
+    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, *MainFileStyle[I]->Case);
+      Options.store(Opts, StyleString, *Styles[I]->Case);
     }
   }
   Options.store(Opts, "GetConfigPerFile", GetConfigPerFile);
   Options.store(Opts, "IgnoreFailedSplit", IgnoreFailedSplit);
-  Options.store(Opts, "IgnoreMainLikeFunctions", IgnoreMainLikeFunctions);
+  Options.store(Opts, "IgnoreMainLikeFunctions",
+                MainFileStyle->isIgnoringMainLikeFunction());
 }
 
 static bool matchesStyle(StringRef Name,
                          IdentifierNamingCheck::NamingStyle Style) {
   static llvm::Regex Matchers[] = {
       llvm::Regex("^.*$"),
       llvm::Regex("^[a-z][a-z0-9_]*$"),
       llvm::Regex("^[a-z][a-zA-Z0-9]*$"),
static llvm::Optional<RenamerClangTidyCheck::FailureInfo> getFailureInfo(
   return RenamerClangTidyCheck::FailureInfo{std::move(KindName),
                                             std::move(Fixup)};
 }
 
 llvm::Optional<RenamerClangTidyCheck::FailureInfo>
 IdentifierNamingCheck::GetDeclFailureInfo(const NamedDecl *Decl,
                                           const SourceManager &SM) const {
   SourceLocation Loc = Decl->getLocation();
-  ArrayRef<llvm::Optional<NamingStyle>> NamingStyles =
-      getStyleForFile(SM.getFilename(Loc));
-
-  return getFailureInfo(
-      Decl->getName(), Loc, NamingStyles,
-      findStyleKind(Decl, NamingStyles, IgnoreMainLikeFunctions), SM,
-      IgnoreFailedSplit);
+  const FileStyle &FileStyle = getStyleForFile(SM.getFilename(Loc));
+  if (!FileStyle.isActive())
+    return llvm::None;
+
+  return getFailureInfo(Decl->getName(), Loc, FileStyle.getStyles(),
+                        findStyleKind(Decl, FileStyle.getStyles(),
+                                      FileStyle.isIgnoringMainLikeFunction()),
+                        SM, IgnoreFailedSplit);
 }
 
 llvm::Optional<RenamerClangTidyCheck::FailureInfo>
 IdentifierNamingCheck::GetMacroFailureInfo(const Token &MacroNameTok,
                                            const SourceManager &SM) const {
   SourceLocation Loc = MacroNameTok.getLocation();
+  const FileStyle &Style = getStyleForFile(SM.getFilename(Loc));
+  if (!Style.isActive())
+    return llvm::None;
 
   return getFailureInfo(MacroNameTok.getIdentifierInfo()->getName(), Loc,
-                        getStyleForFile(SM.getFilename(Loc)),
-                        SK_MacroDefinition, SM, IgnoreFailedSplit);
+                        Style.getStyles(), SK_MacroDefinition, SM,
+                        IgnoreFailedSplit);
 }
 
 RenamerClangTidyCheck::DiagInfo
 IdentifierNamingCheck::GetDiagInfo(const NamingCheckId &ID,
                                    const NamingCheckFailure &Failure) const {
   return DiagInfo{"invalid case style for %0 '%1'",
                   [&](DiagnosticBuilder &Diag) {
                     Diag << Failure.Info.KindName << ID.second;
                   }};
 }
 
-ArrayRef<llvm::Optional<IdentifierNamingCheck::NamingStyle>>
+const IdentifierNamingCheck::FileStyle &
 IdentifierNamingCheck::getStyleForFile(StringRef FileName) const {
   if (!GetConfigPerFile)
-    return MainFileStyle;
-  auto &Styles = NamingStylesCache[llvm::sys::path::parent_path(FileName)];
-  if (Styles.empty()) {
-    ClangTidyOptions Options = Context->getOptionsForFile(FileName);
-    if (Options.Checks && GlobList(*Options.Checks).contains(CheckName))
-      Styles = getNamingStyles({CheckName, Options.CheckOptions});
-    else
-      Styles.resize(SK_Count, None);
-  }
-  return Styles;
+    return *MainFileStyle;
+  StringRef Parent = llvm::sys::path::parent_path(FileName);
+  auto Iter = NamingStylesCache.find(Parent);
+  if (Iter != NamingStylesCache.end())
+    return Iter->getValue();
+
+  if (Options.Checks && GlobList(*Options.Checks).contains(CheckName)) {
+    auto It = NamingStylesCache.try_emplace(
+        Parent, getFileStyleFromOptions({CheckName, Options.CheckOptions}));
+    assert(It.second);
+    return It.first->getValue();
+  }
+  assert(false);
+  // 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/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style-disabled/.clang-tidy

-Checks: -readability-identifier-naming
-CheckOptions:
-  - key:             readability-identifier-naming.GlobalFunctionCase
-    value:           lower_case
-

clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style-disabled/header.h

-void disabled_style_1();
-void disabledStyle2();
-void DISABLED_STYLE_3();

clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/.clang-tidy

 Checks: readability-identifier-naming
 CheckOptions:
   - key:             readability-identifier-naming.GlobalFunctionCase
     value:           lower_case
+  - key:             readability-identifier-naming.IgnoreMainLikeFunctions
+    value:           true
 

clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/header.h

 
 
 void style_first_good();
 
 void styleFirstBad();
+
+int thisIsMainLikeIgnored(int argc, const char *argv[]) {}

clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/.clang-tidy

 Checks: readability-identifier-naming
 CheckOptions:
   - key:             readability-identifier-naming.GlobalFunctionCase
     value:           UPPER_CASE
+  - key:             readability-identifier-naming.IgnoreMainLikeFunctions
+    value:           false
 

clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/header.h

 
 
 void STYLE_SECOND_GOOD();
 
 void styleSecondBad();
+
+int thisIsMainLikeNotIgnored(int argc, const char *argv[]) {}

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:  ]}' -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: true, CheckOptions: [ \
+// 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:  ]}' -header-filter='.*' -- -I%theaders
 
-#include "global-style-disabled/header.h"
 #include "global-style1/header.h"
 #include "global-style2/header.h"
-// CHECK-MESSAGES-ENABLED-DAG: global-style1/header.h:5:6: warning: invalid case style for global function 'styleFirstBad'
-// CHECK-MESSAGES-ENABLED-DAG: global-style2/header.h:5:6: warning: invalid case style for global function 'styleSecondBad'
-// CHECK-MESSAGES-DISABLED-DAG: global-style1/header.h:3:6: warning: invalid case style for function 'style_first_good'
-// CHECK-MESSAGES-DISABLED-DAG: global-style2/header.h:3:6: warning: invalid case style for function 'STYLE_SECOND_GOOD'
-// CHECK-MESSAGES-DISABLED-DAG: global-style-disabled/header.h:1:6: warning: invalid case style for function 'disabled_style_1'
-// CHECK-MESSAGES-DISABLED-DAG: global-style-disabled/header.h:3:6: warning: invalid case style for function 'DISABLED_STYLE_3'
 
 void goodStyle() {
   style_first_good();
   STYLE_SECOND_GOOD();
   //      CHECK-FIXES-DISABLED: styleFirstGood();
   // CHECK-FIXES-DISABLED-NEXT: styleSecondGood();
 }
-// CHECK-MESSAGES-SHARED-DAG: :[[@LINE+1]]:6: warning: invalid case style for function 'bad_style'
+// CHECK-MESSAGES-SHARED: :[[@LINE+1]]:6: warning: invalid case style for function 'bad_style'
 void bad_style() {
   styleFirstBad();
   styleSecondBad();
 }
 //        CHECK-FIXES-SHARED: void badStyle() {
 // CHECK-FIXES-DISABLED-NEXT:   styleFirstBad();
 //  CHECK-FIXES-ENABLED-NEXT:   style_first_bad();
 // CHECK-FIXES-DISABLED-NEXT:   styleSecondBad();
 //  CHECK-FIXES-ENABLED-NEXT:   STYLE_SECOND_BAD();
 //   CHECK-FIXES-SHARED-NEXT: }
 
-void expectNoStyle() {
-  disabled_style_1();
-  disabledStyle2();
-  DISABLED_STYLE_3();
-  //      CHECK-FIXES-DISABLED: disabledStyle1();
-  // CHECK-FIXES-DISABLED-NEXT: disabledStyle2();
-  // CHECK-FIXES-DISABLED-NEXT: disabledStyle3();
-}
+// CHECK-MESSAGES-DISABLED: global-style1/header.h:3:6: warning: invalid case style for function 'style_first_good'
+// CHECK-MESSAGES-ENABLED:  global-style1/header.h:5:6: warning: invalid case style for global function 'styleFirstBad'
+// CHECK-MESSAGES-ENABLED:  global-style1/header.h:7:5: warning: invalid case style for global function 'thisIsMainLikeIgnored'
+// CHECK-MESSAGES-DISABLED: global-style1/header.h:7:31: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES-DISABLED: global-style1/header.h:7:49: warning: invalid case style for parameter 'argv'
+
+// CHECK-MESSAGES-DISABLED: global-style2/header.h:3:6: warning: invalid case style for function 'STYLE_SECOND_GOOD'
+// CHECK-MESSAGES-ENABLED:  global-style2/header.h:5:6: warning: invalid case style for global function 'styleSecondBad'
+// CHECK-MESSAGES-ENABLED:  global-style2/header.h:7:5: warning: invalid case style for global function 'thisIsMainLikeNotIgnored'
+// CHECK-MESSAGES-SHARED:   global-style2/header.h:7:34: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES-SHARED:   global-style2/header.h:7:52: warning: invalid case style for parameter 'argv'
 No newline at end of file