diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h --- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h +++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h @@ -69,7 +69,15 @@ DiagInfo GetDiagInfo(const NamingCheckId &ID, const NamingCheckFailure &Failure) const override; - std::vector> NamingStyles; + ArrayRef> + getStyleForFile(StringRef FileName) const; + + /// Stores the style options for a given directory. + mutable llvm::StringMap>> + NamingStylesCache; + + ClangTidyContext *const Context; + const std::string CheckName; const bool IgnoreFailedSplit; const bool IgnoreMainLikeFunctions; }; diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp --- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp +++ b/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> +getNamingStyles(const ClangTidyCheck::OptionsView &Options) { + std::vector> Styles; + Styles.reserve(StyleNames->size()); + for (auto const &StyleName : StyleNames) { + auto CaseOptional = Options.getOptional( + (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 { - auto ValueOr = Options.get((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> 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> - &NamingStyles, + ArrayRef> 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 -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 getFailureInfo( + StringRef Name, SourceLocation Location, + ArrayRef> 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 +IdentifierNamingCheck::GetDeclFailureInfo(const NamedDecl *Decl, + const SourceManager &SM) const { + SourceLocation Loc = Decl->getLocation(); + ArrayRef> NamingStyles = + getStyleForFile(SM.getFilename(Loc)); + + return getFailureInfo( + Decl->getName(), Loc, NamingStyles, + findStyleKind(Decl, NamingStyles, IgnoreMainLikeFunctions), SM, + IgnoreFailedSplit); } llvm::Optional 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> +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 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 new file mode 100644 --- /dev/null +++ b/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 + 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 new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/header.h @@ -0,0 +1,5 @@ + + +void style1_good(); + +void style1Bad(); 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 new file mode 100644 --- /dev/null +++ b/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 + 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 new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/header.h @@ -0,0 +1,5 @@ + + +void STYLE2_GOOD(); + +void style2Bad(); diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp @@ -5,11 +5,11 @@ // RUN: {key: readability-identifier-naming.ClassCase, value: UUPER_CASE}, \ // RUN: {key: readability-identifier-naming.StructCase, value: CAMEL}, \ // RUN: {key: readability-identifier-naming.EnumCase, value: AnY_cASe}, \ -// RUN: ]}" 2>&1 | FileCheck %s --implicit-check-not warning +// RUN: ]}" 2>&1 | FileCheck %s --implicit-check-not error -// CHECK-DAG: warning: invalid configuration value 'camelback' for option 'readability-identifier-naming.FunctionCase'; did you mean 'camelBack'?{{$}} -// CHECK-DAG: warning: invalid configuration value 'UUPER_CASE' for option 'readability-identifier-naming.ClassCase'; did you mean 'UPPER_CASE'?{{$}} +// CHECK-DAG: error: invalid configuration value 'camelback' for option 'readability-identifier-naming.FunctionCase'; did you mean 'camelBack'?{{$}} +// CHECK-DAG: error: invalid configuration value 'UUPER_CASE' for option 'readability-identifier-naming.ClassCase'; did you mean 'UPPER_CASE'?{{$}} // Don't try to suggest an alternative for 'CAMEL' -// CHECK-DAG: warning: invalid configuration value 'CAMEL' for option 'readability-identifier-naming.StructCase'{{$}} +// CHECK-DAG: error: invalid configuration value 'CAMEL' for option 'readability-identifier-naming.StructCase'{{$}} // This fails on the EditDistance, but as it matches ignoring case suggest the correct value -// CHECK-DAG: warning: invalid configuration value 'AnY_cASe' for option 'readability-identifier-naming.EnumCase'; did you mean 'aNy_CasE'?{{$}} +// CHECK-DAG: error: invalid configuration value 'AnY_cASe' for option 'readability-identifier-naming.EnumCase'; did you mean 'aNy_CasE'?{{$}} 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 new file mode 100644 --- /dev/null +++ b/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 isnt 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: }