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 @@ -10,6 +10,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_IDENTIFIERNAMINGCHECK_H #include "../utils/RenamerClangTidyCheck.h" +#include "llvm/ADT/Optional.h" namespace clang { class MacroInfo; @@ -69,7 +70,17 @@ DiagInfo GetDiagInfo(const NamingCheckId &ID, const NamingCheckFailure &Failure) const override; - std::vector> NamingStyles; + ArrayRef> + 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; + ClangTidyContext *const Context; + const std::string CheckName; + const bool GetConfigPerFile; 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 @@ -8,6 +8,7 @@ #include "IdentifierNamingCheck.h" +#include "../GlobList.h" #include "clang/AST/CXXInheritance.h" #include "clang/Lex/PPCallbacks.h" #include "clang/Lex/Preprocessor.h" @@ -15,7 +16,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 +121,47 @@ #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), + GetConfigPerFile(Options.get("GetConfigPerFile", true)), 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); - } - } + auto IterAndInserted = NamingStylesCache.try_emplace( + llvm::sys::path::parent_path(Context->getCurrentFile()), + getNamingStyles(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(); } 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) { @@ -166,7 +174,7 @@ NamingStyles[i]->Suffix); } } - + Options.store(Opts, "GetConfigPerFile", GetConfigPerFile); Options.store(Opts, "IgnoreFailedSplit", IgnoreFailedSplit); Options.store(Opts, "IgnoreMainLikeFunctions", IgnoreMainLikeFunctions); } @@ -374,8 +382,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 +659,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) +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; - if (!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 +720,21 @@ }}; } +ArrayRef> +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 Styles; +} + } // namespace readability } // namespace tidy } // namespace clang diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -67,7 +67,14 @@ Improvements to clang-tidy -------------------------- -The improvements are... +Changes in existing checks +^^^^^^^^^^^^^^^^^^^^^^^^^^ + +- Improved :doc:`readability-identifier-naming + ` check. + + Added an option `GetConfigPerFile` to support including files which use + different naming styles. Improvements to include-fixer ----------------------------- diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst b/clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst --- a/clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst @@ -51,6 +51,7 @@ - :option:`EnumCase`, :option:`EnumPrefix`, :option:`EnumSuffix` - :option:`EnumConstantCase`, :option:`EnumConstantPrefix`, :option:`EnumConstantSuffix` - :option:`FunctionCase`, :option:`FunctionPrefix`, :option:`FunctionSuffix` + - :option:`GetConfigPerFile` - :option:`GlobalConstantCase`, :option:`GlobalConstantPrefix`, :option:`GlobalConstantSuffix` - :option:`GlobalConstantPointerCase`, :option:`GlobalConstantPointerPrefix`, :option:`GlobalConstantPointerSuffix` - :option:`GlobalFunctionCase`, :option:`GlobalFunctionPrefix`, :option:`GlobalFunctionSuffix` @@ -713,6 +714,13 @@ char pre_my_function_string_post(); +.. option:: GetConfigPerFile + + When `true` the check will look for the configuration for where an + identifier is declared. Useful for when included header files use a + different style. + Default value is `true`. + .. option:: GlobalConstantCase When defined, the check will ensure global constant names conform to the 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 new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style-disabled/.clang-tidy @@ -0,0 +1,5 @@ +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 new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style-disabled/header.h @@ -0,0 +1,3 @@ +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 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,5 @@ +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-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 style_first_good(); + +void styleFirstBad(); 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,5 @@ +Checks: readability-identifier-naming +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 STYLE_SECOND_GOOD(); + +void styleSecondBad(); 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,64 @@ +// 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. +// InheritParentConfig is needed to look for the clang-tidy configuration files. + +// RUN: %check_clang_tidy -check-suffixes=ENABLED,SHARED -std=c++11 %s \ +// RUN: readability-identifier-naming %t -- \ +// RUN: -config='{ InheritParentConfig: true, CheckOptions: [ \ +// RUN: {key: readability-identifier-naming.FunctionCase, value: camelBack}, \ +// RUN: {key: readability-identifier-naming.GetConfigPerFile, value: true} \ +// RUN: ]}' -header-filter='.*' -- -I%theaders + +// On DISABLED run, everything should be made 'camelBack'. + +// 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: {key: readability-identifier-naming.FunctionCase, value: camelBack}, \ +// 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(); + STYLE_SECOND_GOOD(); + // CHECK-FIXES-DISABLED: styleFirstGood(); + // CHECK-FIXES-DISABLED-NEXT: styleSecondGood(); +} +// CHECK-MESSAGES-SHARED-DAG: :[[@LINE+1]]:6: warning: invalid case style for function 'bad_style' +void bad_style() { + styleFirstBad(); + styleSecondBad(); +} +// CHECK-FIXES-SHARED: void badStyle() { +// CHECK-FIXES-DISABLED-NEXT: styleFirstBad(); +// CHECK-FIXES-ENABLED-NEXT: style_first_bad(); +// CHECK-FIXES-DISABLED-NEXT: styleSecondBad(); +// 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(); +}