From 3b9b90a1914f1e470ba7d333b26bd34787337806 Mon Sep 17 00:00:00 2001 From: Nathan James Date: Thu, 5 Nov 2020 19:51:04 +0000 Subject: [PATCH] [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 --- .../readability/IdentifierNamingCheck.cpp | 78 +++++++++++-------- .../readability/IdentifierNamingCheck.h | 29 +++++-- .../global-style-disabled/.clang-tidy | 5 -- .../global-style-disabled/header.h | 3 - .../global-style1/.clang-tidy | 2 + .../global-style1/header.h | 2 + .../global-style2/.clang-tidy | 2 + .../global-style2/header.h | 2 + ...lity-identifier-naming-multiple-styles.cpp | 32 ++++---- 9 files changed, 92 insertions(+), 63 deletions(-) delete mode 100644 clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style-disabled/.clang-tidy delete mode 100644 clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style-disabled/header.h diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp index 4af1b444cf02..5d63066490a6 100644 --- 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> -getNamingStyles(const ClangTidyCheck::OptionsView &Options) { - std::vector> Styles( +static IdentifierNamingCheck::FileStyle +getFileStyleFromOptions(const ClangTidyCheck::OptionsView &Options) { + SmallVector, 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> 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 IdentifierNamingCheck::GetDeclFailureInfo(const NamedDecl *Decl, const SourceManager &SM) const { SourceLocation Loc = Decl->getLocation(); - ArrayRef> 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 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> +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 *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)) { + 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 diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h index ad1c582d100b..77c03f77d91d 100644 --- 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> &&Styles, + bool IgnoreMainLike) + : Styles(std::move(Styles)), IsActive(true), + IgnoreMainLikeFunctions(IgnoreMainLike) {} + + ArrayRef> getStyles() const { + assert(IsActive); + return Styles; + } + bool isActive() const { return IsActive; } + bool isIgnoringMainLikeFunction() const { return IgnoreMainLikeFunctions; } + + private: + SmallVector, 0> Styles; + bool IsActive; + bool IgnoreMainLikeFunctions; + }; + private: llvm::Optional GetDeclFailureInfo(const NamedDecl *Decl, @@ -70,19 +90,16 @@ private: DiagInfo GetDiagInfo(const NamingCheckId &ID, const NamingCheckFailure &Failure) const override; - ArrayRef> - 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>> - NamingStylesCache; - ArrayRef> MainFileStyle; + mutable llvm::StringMap NamingStylesCache; + FileStyle *MainFileStyle; ClangTidyContext *const Context; const std::string CheckName; const bool GetConfigPerFile; const bool IgnoreFailedSplit; - const bool IgnoreMainLikeFunctions; }; } // namespace readability diff --git 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 deleted file mode 100644 index 6a704df8b7b1..000000000000 --- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style-disabled/.clang-tidy +++ /dev/null @@ -1,5 +0,0 @@ -Checks: -readability-identifier-naming -CheckOptions: - - key: readability-identifier-naming.GlobalFunctionCase - value: lower_case - diff --git 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 deleted file mode 100644 index e863f70f7fcb..000000000000 --- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style-disabled/header.h +++ /dev/null @@ -1,3 +0,0 @@ -void disabled_style_1(); -void disabledStyle2(); -void DISABLED_STYLE_3(); diff --git 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 index 85af9672b61d..fc68c6df4a80 100644 --- 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 diff --git 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 index b170bed7c3f6..abbf7dfa4839 100644 --- 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[]) {} diff --git 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 index b2e67ea9c87b..d77875c97e68 100644 --- 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 diff --git 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 index 6b78ad82a1fd..9d3e846a080b 100644 --- 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[]) {} diff --git 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 index 54880d2ca3d0..0608305a8225 100644 --- 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'