[clang-tidy] Extend IdentifierNamingCheck per file config

Add IgnoreMainLikeFunctions to the per file config. This can be extended for new options added to the check easily. Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D90832
2020-11-05 19:51:04 +00:00 · 2020-11-05 19:51:04 +00:00 · 3b9b90a191
parent 40140e122f
commit 3b9b90a191
9 changed files with 92 additions and 63 deletions
--- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@ -122,9 +122,9 @@ static StringRef const StyleNames[] = {
 #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) {
@ -145,23 +145,23 @@ getNamingStyles(const ClangTidyCheck::OptionsView &Options) {
      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;
@ -169,26 +169,28 @@ IdentifierNamingCheck::~IdentifierNamingCheck() = default;
 void IdentifierNamingCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
  RenamerClangTidyCheck::storeOptions(Opts);
  SmallString<64> StyleString;
+  ArrayRef<llvm::Optional<NamingStyle>> 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,
@ -704,23 +706,27 @@ 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));
+  const FileStyle &FileStyle = getStyleForFile(SM.getFilename(Loc));
+  if (!FileStyle.isActive())
+    return llvm::None;

-  return getFailureInfo(
-      Decl->getName(), Loc, NamingStyles,
-      findStyleKind(Decl, NamingStyles, IgnoreMainLikeFunctions), SM,
-      IgnoreFailedSplit);
+  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
@ -732,19 +738,27 @@ IdentifierNamingCheck::GetDiagInfo(const NamingCheckId &ID,
                  }};
 }

-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()) {
+    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))
-      Styles = getNamingStyles({CheckName, Options.CheckOptions});
-    else
-      Styles.resize(SK_Count, None);
+  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();
  }
-  return Styles;
+  assert(false);
+  // Default construction gives an empty style.
+  auto It = NamingStylesCache.try_emplace(Parent);
+  assert(It.second);
+  return It.first->getValue();
 }

 } // namespace readability
--- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
@ -60,6 +60,26 @@ public:
    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,
@ -70,19 +90,16 @@ private:
  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
--- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style-disabled/.clang-tidy
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style-disabled/.clang-tidy
@ -1,5 +0,0 @@
-Checks: -readability-identifier-naming
-CheckOptions:
-  - key:             readability-identifier-naming.GlobalFunctionCase
-    value:           lower_case
-
--- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style-disabled/header.h
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style-disabled/header.h
@ -1,3 +0,0 @@
-void disabled_style_1();
-void disabledStyle2();
-void DISABLED_STYLE_3();
--- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/.clang-tidy
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/.clang-tidy
@ -2,4 +2,6 @@ Checks: readability-identifier-naming
 CheckOptions:
  - key:             readability-identifier-naming.GlobalFunctionCase
    value:           lower_case
+  - key:             readability-identifier-naming.IgnoreMainLikeFunctions
+    value:           true

--- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/header.h
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/header.h
@ -3,3 +3,5 @@
 void style_first_good();

 void styleFirstBad();
+
+int thisIsMainLikeIgnored(int argc, const char *argv[]) {}
--- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/.clang-tidy
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/.clang-tidy
@ -2,4 +2,6 @@ Checks: readability-identifier-naming
 CheckOptions:
  - key:             readability-identifier-naming.GlobalFunctionCase
    value:           UPPER_CASE
+  - key:             readability-identifier-naming.IgnoreMainLikeFunctions
+    value:           false

--- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/header.h
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/header.h
@ -3,3 +3,5 @@
 void STYLE_SECOND_GOOD();

 void styleSecondBad();
+
+int thisIsMainLikeNotIgnored(int argc, const char *argv[]) {}
--- a/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-multiple-styles.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-multiple-styles.cpp
@ -13,6 +13,7 @@
 // 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

@ -21,20 +22,14 @@
 // 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();
@ -42,7 +37,7 @@ void goodStyle() {
  //      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();
@ -54,11 +49,14 @@ void bad_style() {
 //  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'