diff --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.h b/clang-tools-extra/clang-tidy/ClangTidyCheck.h --- a/clang-tools-extra/clang-tidy/ClangTidyCheck.h +++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.h @@ -268,7 +268,7 @@ if (llvm::Expected ValueOr = get(LocalName)) return *ValueOr; else - logErrToStdErr(ValueOr.takeError()); + logOptionParsingError(ValueOr.takeError()); return Default; } @@ -314,7 +314,7 @@ if (llvm::Expected ValueOr = getLocalOrGlobal(LocalName)) return *ValueOr; else - logErrToStdErr(ValueOr.takeError()); + logOptionParsingError(ValueOr.takeError()); return Default; } @@ -353,7 +353,7 @@ if (auto ValueOr = get(LocalName, IgnoreCase)) return *ValueOr; else - logErrToStdErr(ValueOr.takeError()); + logOptionParsingError(ValueOr.takeError()); return Default; } @@ -395,10 +395,35 @@ if (auto ValueOr = getLocalOrGlobal(LocalName, IgnoreCase)) return *ValueOr; else - logErrToStdErr(ValueOr.takeError()); + logOptionParsingError(ValueOr.takeError()); return Default; } + /// Returns the value for the option \p LocalName represented as a ``T``. + /// If the option is missing returns None, if the option can't be parsed + /// as a ``T``, log that to stderr and return None. + template + llvm::Optional getOptional(StringRef LocalName) const { + if (auto ValueOr = get(LocalName)) + return *ValueOr; + else + logOptionParsingError(ValueOr.takeError()); + return llvm::None; + } + + /// Returns the value for the local or global option \p LocalName + /// represented as a ``T``. + /// If the option is missing returns None, if the + /// option can't be parsed as a ``T``, log that to stderr and return None. + template + llvm::Optional getOptionalLocalOrGlobal(StringRef LocalName) const { + if (auto ValueOr = getLocalOrGlobal(LocalName)) + return *ValueOr; + else + logOptionParsingError(ValueOr.takeError()); + return llvm::None; + } + /// Stores an option with the check-local name \p LocalName with /// string value \p Value to \p Options. void store(ClangTidyOptions::OptionMap &Options, StringRef LocalName, @@ -456,7 +481,8 @@ void storeInt(ClangTidyOptions::OptionMap &Options, StringRef LocalName, int64_t Value) const; - static void logErrToStdErr(llvm::Error &&Err); + /// Logs an Error to stderr if a \p Err is not a MissingOptionError. + static void logOptionParsingError(llvm::Error &&Err); std::string NamePrefix; const ClangTidyOptions::OptionMap &CheckOptions; @@ -524,6 +550,19 @@ ClangTidyOptions::OptionMap &Options, StringRef LocalName, bool Value) const; +/// Returns the value for the option \p LocalName. +/// If the option is missing returns None. +template <> +Optional ClangTidyCheck::OptionsView::getOptional( + StringRef LocalName) const; + +/// Returns the value for the local or global option \p LocalName. +/// If the option is missing returns None. +template <> +Optional +ClangTidyCheck::OptionsView::getOptionalLocalOrGlobal( + StringRef LocalName) const; + } // namespace tidy } // namespace clang diff --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp --- a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp @@ -10,6 +10,7 @@ #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" +#include "llvm/Support/WithColor.h" #include "llvm/Support/raw_ostream.h" namespace clang { @@ -126,7 +127,7 @@ llvm::Expected ValueOr = get(LocalName); if (ValueOr) return *ValueOr; - logErrToStdErr(ValueOr.takeError()); + logOptionParsingError(ValueOr.takeError()); return Default; } @@ -145,7 +146,7 @@ llvm::Expected ValueOr = getLocalOrGlobal(LocalName); if (ValueOr) return *ValueOr; - logErrToStdErr(ValueOr.takeError()); + logOptionParsingError(ValueOr.takeError()); return Default; } @@ -204,13 +205,36 @@ Iter->second.Value); } -void ClangTidyCheck::OptionsView::logErrToStdErr(llvm::Error &&Err) { - llvm::logAllUnhandledErrors( - llvm::handleErrors(std::move(Err), - [](const MissingOptionError &) -> llvm::Error { - return llvm::Error::success(); - }), - llvm::errs(), "warning: "); +void ClangTidyCheck::OptionsView::logOptionParsingError(llvm::Error &&Err) { + auto RemainingErrors = llvm::handleErrors( + std::move(Err), [](const MissingOptionError &) -> llvm::Error { + return llvm::Error::success(); + }); + if (RemainingErrors) + llvm::logAllUnhandledErrors(std::move(RemainingErrors), + llvm::WithColor::error()); } + +template <> +Optional ClangTidyCheck::OptionsView::getOptional( + StringRef LocalName) const { + if (auto ValueOr = get(LocalName)) + return *ValueOr; + else + consumeError(ValueOr.takeError()); + return llvm::None; +} + +template <> +Optional +ClangTidyCheck::OptionsView::getOptionalLocalOrGlobal( + StringRef LocalName) const { + if (auto ValueOr = getLocalOrGlobal(LocalName)) + return *ValueOr; + else + consumeError(ValueOr.takeError()); + return llvm::None; +} + } // namespace tidy } // namespace clang 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 @@ -59,6 +59,8 @@ std::string Suffix; }; + using OptionalStyle = llvm::Optional; + private: llvm::Optional GetDeclFailureInfo(const NamedDecl *Decl, @@ -69,7 +71,12 @@ DiagInfo GetDiagInfo(const NamingCheckId &ID, const NamingCheckFailure &Failure) const override; - std::vector> NamingStyles; + ArrayRef getStyleForFile(StringRef FileName) const; + + 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 void +populateNaimgStyle(std::vector &Styles, + const ClangTidyCheck::OptionsView &Options) { + assert(Styles.empty() && "Styles should be empty here"); + 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); + } +} + 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); - } - } + populateNaimgStyle(NamingStylesCache[llvm::sys::path::parent_path( + Context->getCurrentFile())], + 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) { @@ -372,11 +373,10 @@ return (Style.Prefix + Mid + Style.Suffix).str(); } -static StyleKind findStyleKind( - const NamedDecl *D, - const std::vector> - &NamingStyles, - bool IgnoreMainLikeFunctions) { +static StyleKind +findStyleKind(const NamedDecl *D, + 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::GetMacroFailureInfo(const Token &MacroNameTok, - const SourceManager &SM) const { - if (!NamingStyles[SK_MacroDefinition]) - return None; +IdentifierNamingCheck::GetDeclFailureInfo(const NamedDecl *Decl, + const SourceManager &SM) const { + SourceLocation Loc = Decl->getLocation(); + ArrayRef NamingStyles = getStyleForFile(SM.getFilename(Loc)); - StringRef Name = MacroNameTok.getIdentifierInfo()->getName(); - const NamingStyle &Style = *NamingStyles[SK_MacroDefinition]; - if (matchesStyle(Name, Style)) - return None; + return getFailureInfo( + Decl->getName(), Loc, NamingStyles, + findStyleKind(Decl, NamingStyles, IgnoreMainLikeFunctions), SM, + IgnoreFailedSplit); +} - std::string KindName = - fixupWithCase(StyleNames[SK_MacroDefinition], CT_LowerCase); - std::replace(KindName.begin(), KindName.end(), '_', ' '); +llvm::Optional +IdentifierNamingCheck::GetMacroFailureInfo(const Token &MacroNameTok, + const SourceManager &SM) const { + SourceLocation Loc = MacroNameTok.getLocation(); + ArrayRef NamingStyles = getStyleForFile(SM.getFilename(Loc)); - 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, + NamingStyles, 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()) + populateNaimgStyle( + Styles, {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(); \ No newline at end of file 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(); \ No newline at end of file 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: } \ No newline at end of file