Index: clang-tidy/readability/IdentifierNamingCheck.h =================================================================== --- clang-tidy/readability/IdentifierNamingCheck.h +++ clang-tidy/readability/IdentifierNamingCheck.h @@ -36,6 +36,7 @@ void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void registerPPCallbacks(CompilerInstance &Compiler) override; void onEndOfTranslationUnit() override; enum CaseType { @@ -64,7 +65,7 @@ /// \brief Holds an identifier name check failure, tracking the kind of the /// identifer, its possible fixup and the starting locations of all the - /// idenfiier usages. + /// identifier usages. struct NamingCheckFailure { std::string KindName; std::string Fixup; @@ -81,9 +82,17 @@ NamingCheckFailure() : ShouldFix(true) {} }; - typedef llvm::DenseMap + + struct NamingCheckId : std::pair { + typedef std::pair Parent; + using Parent::Parent; + }; + + typedef llvm::DenseMap NamingCheckFailureMap; + void checkMacro(SourceManager &sourceMgr, const Token &MacroNameTok); + private: std::vector NamingStyles; bool IgnoreFailedSplit; Index: clang-tidy/readability/IdentifierNamingCheck.cpp =================================================================== --- clang-tidy/readability/IdentifierNamingCheck.cpp +++ clang-tidy/readability/IdentifierNamingCheck.cpp @@ -12,11 +12,53 @@ #include "llvm/Support/Debug.h" #include "llvm/Support/Format.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Lex/PPCallbacks.h" +#include "clang/Lex/Preprocessor.h" +#include "llvm/ADT/DenseMapInfo.h" #define DEBUG_TYPE "clang-tidy" using namespace clang::ast_matchers; +namespace llvm { +/// Specialisation of DenseMapInfo to allow NamingCheckId objects in DenseMaps +template <> +struct DenseMapInfo< + clang::tidy::readability::IdentifierNamingCheck::NamingCheckId> { + using NamingCheckId = + clang::tidy::readability::IdentifierNamingCheck::NamingCheckId; + + static inline NamingCheckId getEmptyKey() { + return NamingCheckId( + clang::SourceLocation::getFromRawEncoding(static_cast(-1)), + "EMPTY"); + } + + static inline NamingCheckId getTombstoneKey() { + return NamingCheckId( + clang::SourceLocation::getFromRawEncoding(static_cast(-2)), + "TOMBSTONE"); + } + + static unsigned getHashValue(NamingCheckId Val) { + assert(Val != getEmptyKey() && "Cannot hash the empty key!"); + assert(Val != getTombstoneKey() && "Cannot hash the tombstone key!"); + + std::hash SecondHash; + return Val.first.getRawEncoding() + SecondHash(Val.second); + } + + static bool isEqual(NamingCheckId LHS, NamingCheckId RHS) { + if (RHS == getEmptyKey()) + return LHS == getEmptyKey(); + if (RHS == getTombstoneKey()) + return LHS == getTombstoneKey(); + return LHS == RHS; + } +}; +} // namespace llvm + namespace clang { namespace tidy { namespace readability { @@ -65,6 +107,7 @@ m(ValueTemplateParameter) \ m(TemplateTemplateParameter) \ m(TemplateParameter) \ + m(MacroDefinition) enum StyleKind { #define ENUMERATE(v) SK_ ## v, @@ -83,6 +126,32 @@ #undef NAMING_KEYS // clang-format on +namespace { +/// Callback supplies macros to IdentifierNamingCheck::checkMacro +class IdentifierNamingCheckPPCallbacks : public PPCallbacks { +public: + IdentifierNamingCheckPPCallbacks(Preprocessor *PP, + IdentifierNamingCheck *Check) + : PP(PP), Check(Check) {} + + /// MacroDefined calls checkMacro for macros in the main file + void MacroDefined(const Token &MacroNameTok, + const MacroDirective *MD) override { + (void)MD; + SourceManager &SM(PP->getSourceManager()); + if (!SM.isInMainFile(MacroNameTok.getLocation())) + return; + if (SM.isInSystemHeader(MacroNameTok.getLocation())) + return; + Check->checkMacro(SM, MacroNameTok); + } + +private: + Preprocessor *PP; + IdentifierNamingCheck *Check; +}; +} // namespace + IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context) { @@ -145,6 +214,12 @@ Finder->addMatcher(nestedNameSpecifierLoc().bind("nestedNameLoc"), this); } +void IdentifierNamingCheck::registerPPCallbacks(CompilerInstance &Compiler) { + Compiler.getPreprocessor().addPPCallbacks( + llvm::make_unique( + &Compiler.getPreprocessor(), this)); +} + static bool matchesStyle(StringRef Name, IdentifierNamingCheck::NamingStyle Style) { static llvm::Regex Matchers[] = { @@ -502,8 +577,8 @@ } static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures, - const NamedDecl *Decl, SourceRange Range, - const SourceManager *SM) { + const IdentifierNamingCheck::NamingCheckId &Decl, + SourceRange Range) { // Do nothing if the provided range is invalid. if (Range.getBegin().isInvalid() || Range.getEnd().isInvalid()) return; @@ -518,6 +593,14 @@ !Range.getEnd().isMacroID(); } +/// Convenience method when the usage to be added is a NamedDecl +static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures, + const NamedDecl *Decl, SourceRange Range) { + return addUsage(Failures, IdentifierNamingCheck::NamingCheckId( + Decl->getLocation(), Decl->getNameAsString()), + Range); +} + void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) { if (const auto *Decl = Result.Nodes.getNodeAs("classRef")) { @@ -525,7 +608,7 @@ return; addUsage(NamingCheckFailures, Decl->getParent(), - Decl->getNameInfo().getSourceRange(), Result.SourceManager); + Decl->getNameInfo().getSourceRange()); return; } @@ -541,8 +624,7 @@ // we want instead to replace the next token, that will be the identifier. Range.setBegin(CharSourceRange::getTokenRange(Range).getEnd()); - addUsage(NamingCheckFailures, Decl->getParent(), Range, - Result.SourceManager); + addUsage(NamingCheckFailures, Decl->getParent(), Range); return; } @@ -559,8 +641,7 @@ } if (Decl) { - addUsage(NamingCheckFailures, Decl, Loc->getSourceRange(), - Result.SourceManager); + addUsage(NamingCheckFailures, Decl, Loc->getSourceRange()); return; } @@ -570,8 +651,7 @@ SourceRange Range(Ref.getTemplateNameLoc(), Ref.getTemplateNameLoc()); if (const auto *ClassDecl = dyn_cast(Decl)) { - addUsage(NamingCheckFailures, ClassDecl->getTemplatedDecl(), Range, - Result.SourceManager); + addUsage(NamingCheckFailures, ClassDecl->getTemplatedDecl(), Range); return; } } @@ -579,7 +659,7 @@ if (const auto &Ref = Loc->getAs()) { addUsage(NamingCheckFailures, Ref.getTypePtr()->getAsTagDecl(), - Loc->getSourceRange(), Result.SourceManager); + Loc->getSourceRange()); return; } } @@ -588,8 +668,7 @@ Result.Nodes.getNodeAs("nestedNameLoc")) { if (NestedNameSpecifier *Spec = Loc->getNestedNameSpecifier()) { if (NamespaceDecl *Decl = Spec->getAsNamespace()) { - addUsage(NamingCheckFailures, Decl, Loc->getLocalSourceRange(), - Result.SourceManager); + addUsage(NamingCheckFailures, Decl, Loc->getLocalSourceRange()); return; } } @@ -598,15 +677,14 @@ if (const auto *Decl = Result.Nodes.getNodeAs("using")) { for (const auto &Shadow : Decl->shadows()) { addUsage(NamingCheckFailures, Shadow->getTargetDecl(), - Decl->getNameInfo().getSourceRange(), Result.SourceManager); + Decl->getNameInfo().getSourceRange()); } return; } if (const auto *DeclRef = Result.Nodes.getNodeAs("declRef")) { SourceRange Range = DeclRef->getNameInfo().getSourceRange(); - addUsage(NamingCheckFailures, DeclRef->getDecl(), Range, - Result.SourceManager); + addUsage(NamingCheckFailures, DeclRef->getDecl(), Range); return; } @@ -614,6 +692,33 @@ if (!Decl->getIdentifier() || Decl->getName().empty() || Decl->isImplicit()) return; + // Fix type aliases in value declarations + if (const auto *Value = Result.Nodes.getNodeAs("decl")) { + if (const auto *Typedef = + Value->getType().getTypePtr()->getAs()) { + addUsage(NamingCheckFailures, Typedef->getDecl(), + Value->getSourceRange()); + } + } + + // Fix type aliases in function declarations + if (const auto *Value = Result.Nodes.getNodeAs("decl")) { + if (const auto *Typedef = + Value->getReturnType().getTypePtr()->getAs()) { + addUsage(NamingCheckFailures, Typedef->getDecl(), + Value->getSourceRange()); + } + for (unsigned i = 0; i < Value->getNumParams(); ++i) { + if (const auto *Typedef = Value->parameters()[i] + ->getType() + .getTypePtr() + ->getAs()) { + addUsage(NamingCheckFailures, Typedef->getDecl(), + Value->getSourceRange()); + } + } + } + // Ignore ClassTemplateSpecializationDecl which are creating duplicate // replacements with CXXRecordDecl if (isa(Decl)) @@ -640,29 +745,61 @@ KindName.c_str(), Name)); } } else { - NamingCheckFailure &Failure = NamingCheckFailures[Decl]; + NamingCheckFailure &Failure = NamingCheckFailures[NamingCheckId( + Decl->getLocation(), Decl->getNameAsString())]; SourceRange Range = DeclarationNameInfo(Decl->getDeclName(), Decl->getLocation()) .getSourceRange(); Failure.Fixup = std::move(Fixup); Failure.KindName = std::move(KindName); - addUsage(NamingCheckFailures, Decl, Range, Result.SourceManager); + addUsage(NamingCheckFailures, Decl, Range); } } } +void IdentifierNamingCheck::checkMacro(SourceManager &SourceMgr, + const Token &MacroNameTok) { + StringRef Name = MacroNameTok.getIdentifierInfo()->getName(); + NamingStyle Style = NamingStyles[SK_MacroDefinition]; + if (matchesStyle(Name, Style)) + return; + + 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) { + DEBUG( + llvm::dbgs() << MacroNameTok.getLocation().printToString(SourceMgr) + << llvm::format(": unable to split words for %s '%s'\n", + KindName.c_str(), Name)); + } + } else { + NamingCheckId ID(MacroNameTok.getLocation(), Name); + NamingCheckFailure &Failure = NamingCheckFailures[ID]; + SourceRange Range = + SourceRange(MacroNameTok.getLocation(), MacroNameTok.getEndLoc()); + + Failure.Fixup = std::move(Fixup); + Failure.KindName = std::move(KindName); + addUsage(NamingCheckFailures, ID, Range); + } +} + void IdentifierNamingCheck::onEndOfTranslationUnit() { for (const auto &Pair : NamingCheckFailures) { - const NamedDecl &Decl = *Pair.first; + const NamingCheckId &Decl = Pair.first; const NamingCheckFailure &Failure = Pair.second; if (Failure.KindName.empty()) continue; if (Failure.ShouldFix) { - auto Diag = diag(Decl.getLocation(), "invalid case style for %0 '%1'") - << Failure.KindName << Decl.getName(); + auto Diag = diag(Decl.first, "invalid case style for %0 '%1'") + << Failure.KindName << Decl.second; for (const auto &Loc : Failure.RawUsageLocs) { // We assume that the identifier name is made of one token only. This is Index: test/clang-tidy/readability-identifier-naming.cpp =================================================================== --- test/clang-tidy/readability-identifier-naming.cpp +++ test/clang-tidy/readability-identifier-naming.cpp @@ -61,6 +61,7 @@ // RUN: {key: readability-identifier-naming.VariableCase, value: lower_case}, \ // RUN: {key: readability-identifier-naming.VirtualMethodCase, value: UPPER_CASE}, \ // RUN: {key: readability-identifier-naming.VirtualMethodPrefix, value: 'v_'}, \ +// RUN: {key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE}, \ // RUN: {key: readability-identifier-naming.IgnoreFailedSplit, value: 0} \ // RUN: ]}' -- -std=c++11 -fno-delayed-template-parsing \ // RUN: -I%S/Inputs/readability-identifier-naming \ @@ -305,6 +306,12 @@ // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for typedef 'struct_type' // CHECK-FIXES: {{^}}typedef this_structure struct_type_t;{{$}} +struct_type GlobalTypedefTestFunction(struct_type a_argument1) { +// CHECK-FIXES: {{^}}struct_type_t GlobalTypedefTestFunction(struct_type_t a_argument1) { + struct_type typedef_test_1; +// CHECK-FIXES: {{^}} struct_type_t typedef_test_1; +} + static void static_Function() { // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: invalid case style for function 'static_Function' // CHECK-FIXES: {{^}}static void staticFunction() {{{$}} @@ -318,5 +325,9 @@ // CHECK-FIXES: {{^}} using ::foo_ns::inline_namespace::ce_function;{{$}} } +#define MY_TEST_Macro(X) X() +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for macro definition 'MY_TEST_Macro' +// CHECK-FIXES: {{^}}#define MY_TEST_MACRO(X) X() + } }