njames93 updated this revision to Diff 281756. njames93 added a comment. Rebase from parent and address comments.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84814/new/ https://reviews.llvm.org/D84814 Files: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/.clang-tidy clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/header.h clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/.clang-tidy clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/header.h clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-multiple-styles.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-multiple-styles.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-multiple-styles.cpp @@ -0,0 +1,34 @@ +// 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. + +// RUN: %check_clang_tidy -std=c++11 %s readability-identifier-naming %t -- \ +// RUN: -config='{ InheritParentConfig: true, CheckOptions: [ \ +// RUN: {key: readability-identifier-naming.FunctionCase, value: camelBack} \ +// RUN: ]}' -header-filter='.*' -- -I%theaders + +#include "global-style1/header.h" +#include "global-style2/header.h" +// CHECK-MESSAGES-DAG: global-style1/header.h:5:6: warning: invalid case style for global function 'style1Bad' +// CHECK-MESSAGES-DAG: global-style2/header.h:5:6: warning: invalid case style for global function 'style2Bad' + +void goodStyle() { + style1_good(); + STYLE2_GOOD(); +} +// CHECK-MESSAGES-DAG: :[[@LINE+1]]:6: warning: invalid case style for function 'bad_style' +void bad_style() { + style1Bad(); + style2Bad(); +} + +// CHECK-FIXES: void badStyle() { +// CHECK-FIXES-NEXT: style1_bad(); +// CHECK-FIXES-NEXT: STYLE2_BAD(); +// CHECK-FIXES-NEXT: } Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/header.h =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/header.h @@ -0,0 +1,5 @@ + + +void STYLE2_GOOD(); + +void style2Bad(); Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/.clang-tidy =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/.clang-tidy @@ -0,0 +1,4 @@ +CheckOptions: + - key: readability-identifier-naming.GlobalFunctionCase + value: UPPER_CASE + Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/header.h =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/header.h @@ -0,0 +1,5 @@ + + +void style1_good(); + +void style1Bad(); Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/.clang-tidy =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/.clang-tidy @@ -0,0 +1,4 @@ +CheckOptions: + - key: readability-identifier-naming.GlobalFunctionCase + value: lower_case + Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h =================================================================== --- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h +++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h @@ -69,7 +69,16 @@ DiagInfo GetDiagInfo(const NamingCheckId &ID, const NamingCheckFailure &Failure) const override; - std::vector<llvm::Optional<NamingStyle>> NamingStyles; + ArrayRef<llvm::Optional<NamingStyle>> + 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; + + ClangTidyContext *const Context; + const std::string CheckName; const bool IgnoreFailedSplit; const bool IgnoreMainLikeFunctions; }; Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp +++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp @@ -15,7 +15,8 @@ #include "llvm/ADT/DenseMapInfo.h" #include "llvm/Support/Debug.h" #include "llvm/Support/Error.h" -#include "llvm/Support/Format.h" +#include "llvm/Support/FormatVariadic.h" +#include "llvm/Support/Path.h" #include "llvm/Support/Regex.h" #define DEBUG_TYPE "clang-tidy" @@ -119,41 +120,41 @@ #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; + Styles.reserve(StyleNames->size()); + for (auto const &StyleName : StyleNames) { + auto CaseOptional = Options.getOptional<IdentifierNamingCheck::CaseType>( + (StyleName + "Case").str()); + auto Prefix = Options.get((StyleName + "Prefix").str(), ""); + auto Postfix = Options.get((StyleName + "Suffix").str(), ""); + + if (CaseOptional || !Prefix.empty() || !Postfix.empty()) + Styles.emplace_back(IdentifierNamingCheck::NamingStyle{ + std::move(CaseOptional), std::move(Prefix), std::move(Postfix)}); + else + Styles.emplace_back(llvm::None); + } + return Styles; +} + IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name, ClangTidyContext *Context) - : RenamerClangTidyCheck(Name, Context), + : RenamerClangTidyCheck(Name, Context), Context(Context), CheckName(Name), IgnoreFailedSplit(Options.get("IgnoreFailedSplit", false)), IgnoreMainLikeFunctions(Options.get("IgnoreMainLikeFunctions", false)) { - for (auto const &Name : StyleNames) { - auto CaseOptional = [&]() -> llvm::Optional<CaseType> { - auto ValueOr = Options.get<CaseType>((Name + "Case").str()); - if (ValueOr) - return *ValueOr; - llvm::logAllUnhandledErrors( - llvm::handleErrors(ValueOr.takeError(), - [](const MissingOptionError &) -> llvm::Error { - return llvm::Error::success(); - }), - llvm::errs(), "warning: "); - return llvm::None; - }(); - - auto prefix = Options.get((Name + "Prefix").str(), ""); - auto postfix = Options.get((Name + "Suffix").str(), ""); - - if (CaseOptional || !prefix.empty() || !postfix.empty()) { - NamingStyles.push_back(NamingStyle(CaseOptional, prefix, postfix)); - } else { - NamingStyles.push_back(llvm::None); - } - } + NamingStylesCache[llvm::sys::path::parent_path(Context->getCurrentFile())] = + getNamingStyles(Options); } IdentifierNamingCheck::~IdentifierNamingCheck() = default; void IdentifierNamingCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { RenamerClangTidyCheck::storeOptions(Opts); + ArrayRef<llvm::Optional<NamingStyle>> NamingStyles = + getStyleForFile(Context->getCurrentFile()); for (size_t i = 0; i < SK_Count; ++i) { if (NamingStyles[i]) { if (NamingStyles[i]->Case) { @@ -374,8 +375,7 @@ static StyleKind findStyleKind( const NamedDecl *D, - const std::vector<llvm::Optional<IdentifierNamingCheck::NamingStyle>> - &NamingStyles, + ArrayRef<llvm::Optional<IdentifierNamingCheck::NamingStyle>> NamingStyles, bool IgnoreMainLikeFunctions) { assert(D && D->getIdentifier() && !D->getName().empty() && !D->isImplicit() && "Decl must be an explicit identifier with a name."); @@ -652,63 +652,56 @@ return SK_Invalid; } -llvm::Optional<RenamerClangTidyCheck::FailureInfo> -IdentifierNamingCheck::GetDeclFailureInfo(const NamedDecl *Decl, - const SourceManager &SM) const { - StyleKind SK = findStyleKind(Decl, NamingStyles, IgnoreMainLikeFunctions); - if (SK == SK_Invalid) - return None; - - if (!NamingStyles[SK]) +static llvm::Optional<RenamerClangTidyCheck::FailureInfo> getFailureInfo( + StringRef Name, SourceLocation Location, + ArrayRef<llvm::Optional<IdentifierNamingCheck::NamingStyle>> NamingStyles, + StyleKind SK, const SourceManager &SM, bool IgnoreFailedSplit) { + if (SK == SK_Invalid || !NamingStyles[SK]) return None; - const NamingStyle &Style = *NamingStyles[SK]; - StringRef Name = Decl->getName(); + const IdentifierNamingCheck::NamingStyle &Style = *NamingStyles[SK]; if (matchesStyle(Name, Style)) return None; - std::string KindName = fixupWithCase(StyleNames[SK], CT_LowerCase); + std::string KindName = + fixupWithCase(StyleNames[SK], IdentifierNamingCheck::CT_LowerCase); std::replace(KindName.begin(), KindName.end(), '_', ' '); std::string Fixup = fixupWithStyle(Name, Style); if (StringRef(Fixup).equals(Name)) { if (!IgnoreFailedSplit) { - LLVM_DEBUG(llvm::dbgs() - << Decl->getBeginLoc().printToString(SM) - << llvm::format(": unable to split words for %s '%s'\n", - KindName.c_str(), Name.str().c_str())); + LLVM_DEBUG(Location.print(llvm::dbgs(), SM); + llvm::dbgs() + << llvm::formatv(": unable to split words for {0} '{1}'\n", + KindName, Name)); } return None; } - return FailureInfo{std::move(KindName), std::move(Fixup)}; + 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); } llvm::Optional<RenamerClangTidyCheck::FailureInfo> IdentifierNamingCheck::GetMacroFailureInfo(const Token &MacroNameTok, const SourceManager &SM) const { - if (!NamingStyles[SK_MacroDefinition]) - return None; - - StringRef Name = MacroNameTok.getIdentifierInfo()->getName(); - const NamingStyle &Style = *NamingStyles[SK_MacroDefinition]; - if (matchesStyle(Name, Style)) - return None; + SourceLocation Loc = MacroNameTok.getLocation(); - std::string KindName = - fixupWithCase(StyleNames[SK_MacroDefinition], CT_LowerCase); - std::replace(KindName.begin(), KindName.end(), '_', ' '); - - std::string Fixup = fixupWithStyle(Name, Style); - if (StringRef(Fixup).equals(Name)) { - if (!IgnoreFailedSplit) { - LLVM_DEBUG(llvm::dbgs() - << MacroNameTok.getLocation().printToString(SM) - << llvm::format(": unable to split words for %s '%s'\n", - KindName.c_str(), Name.str().c_str())); - } - return None; - } - return FailureInfo{std::move(KindName), std::move(Fixup)}; + return getFailureInfo(MacroNameTok.getIdentifierInfo()->getName(), Loc, + getStyleForFile(SM.getFilename(Loc)), + SK_MacroDefinition, SM, IgnoreFailedSplit); } RenamerClangTidyCheck::DiagInfo @@ -720,6 +713,15 @@ }}; } +ArrayRef<llvm::Optional<IdentifierNamingCheck::NamingStyle>> +IdentifierNamingCheck::getStyleForFile(StringRef FileName) const { + auto &Styles = NamingStylesCache[llvm::sys::path::parent_path(FileName)]; + if (Styles.empty()) + Styles = getNamingStyles( + {CheckName, Context->getOptionsForFile(FileName).CheckOptions}); + return Styles; +} + } // namespace readability } // namespace tidy } // namespace clang
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits