diff options
4 files changed, 205 insertions, 22 deletions
diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp index 567dbddaa6f..9d562943152 100644 --- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp +++ b/clang-tools-extra/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<unsigned>(-1)), + "EMPTY"); + } + + static inline NamingCheckId getTombstoneKey() { + return NamingCheckId( + clang::SourceLocation::getFromRawEncoding(static_cast<unsigned>(-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<NamingCheckId::second_type> 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 { @@ -66,6 +108,7 @@ namespace readability { m(TemplateTemplateParameter) \ m(TemplateParameter) \ m(TypeAlias) \ + m(MacroDefinition) \ enum StyleKind { #define ENUMERATE(v) SK_ ## v, @@ -84,6 +127,33 @@ static StringRef const StyleNames[] = { #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 { + Check->checkMacro(PP->getSourceManager(), MacroNameTok, MD->getMacroInfo()); + } + + /// MacroExpands calls expandMacro for macros in the main file + void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD, + SourceRange /*Range*/, + const MacroArgs * /*Args*/) override { + Check->expandMacro(MacroNameTok, MD.getMacroInfo()); + } + +private: + Preprocessor *PP; + IdentifierNamingCheck *Check; +}; +} // namespace + IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context) { @@ -146,6 +216,12 @@ void IdentifierNamingCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher(nestedNameSpecifierLoc().bind("nestedNameLoc"), this); } +void IdentifierNamingCheck::registerPPCallbacks(CompilerInstance &Compiler) { + Compiler.getPreprocessor().addPPCallbacks( + llvm::make_unique<IdentifierNamingCheckPPCallbacks>( + &Compiler.getPreprocessor(), this)); +} + static bool matchesStyle(StringRef Name, IdentifierNamingCheck::NamingStyle Style) { static llvm::Regex Matchers[] = { @@ -506,8 +582,8 @@ static StyleKind findStyleKind( } 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; @@ -522,6 +598,14 @@ static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures, !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<CXXConstructorDecl>("classRef")) { @@ -529,7 +613,7 @@ void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) { return; addUsage(NamingCheckFailures, Decl->getParent(), - Decl->getNameInfo().getSourceRange(), Result.SourceManager); + Decl->getNameInfo().getSourceRange()); return; } @@ -545,8 +629,7 @@ void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) { // 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; } @@ -563,8 +646,7 @@ void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) { } if (Decl) { - addUsage(NamingCheckFailures, Decl, Loc->getSourceRange(), - Result.SourceManager); + addUsage(NamingCheckFailures, Decl, Loc->getSourceRange()); return; } @@ -574,8 +656,7 @@ void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) { SourceRange Range(Ref.getTemplateNameLoc(), Ref.getTemplateNameLoc()); if (const auto *ClassDecl = dyn_cast<TemplateDecl>(Decl)) { - addUsage(NamingCheckFailures, ClassDecl->getTemplatedDecl(), Range, - Result.SourceManager); + addUsage(NamingCheckFailures, ClassDecl->getTemplatedDecl(), Range); return; } } @@ -583,7 +664,7 @@ void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) { if (const auto &Ref = Loc->getAs<DependentTemplateSpecializationTypeLoc>()) { addUsage(NamingCheckFailures, Ref.getTypePtr()->getAsTagDecl(), - Loc->getSourceRange(), Result.SourceManager); + Loc->getSourceRange()); return; } } @@ -592,8 +673,7 @@ void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) { Result.Nodes.getNodeAs<NestedNameSpecifierLoc>("nestedNameLoc")) { if (NestedNameSpecifier *Spec = Loc->getNestedNameSpecifier()) { if (NamespaceDecl *Decl = Spec->getAsNamespace()) { - addUsage(NamingCheckFailures, Decl, Loc->getLocalSourceRange(), - Result.SourceManager); + addUsage(NamingCheckFailures, Decl, Loc->getLocalSourceRange()); return; } } @@ -602,15 +682,14 @@ void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) { if (const auto *Decl = Result.Nodes.getNodeAs<UsingDecl>("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<DeclRefExpr>("declRef")) { SourceRange Range = DeclRef->getNameInfo().getSourceRange(); - addUsage(NamingCheckFailures, DeclRef->getDecl(), Range, - Result.SourceManager); + addUsage(NamingCheckFailures, DeclRef->getDecl(), Range); return; } @@ -618,6 +697,33 @@ void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) { if (!Decl->getIdentifier() || Decl->getName().empty() || Decl->isImplicit()) return; + // Fix type aliases in value declarations + if (const auto *Value = Result.Nodes.getNodeAs<ValueDecl>("decl")) { + if (const auto *Typedef = + Value->getType().getTypePtr()->getAs<TypedefType>()) { + addUsage(NamingCheckFailures, Typedef->getDecl(), + Value->getSourceRange()); + } + } + + // Fix type aliases in function declarations + if (const auto *Value = Result.Nodes.getNodeAs<FunctionDecl>("decl")) { + if (const auto *Typedef = + Value->getReturnType().getTypePtr()->getAs<TypedefType>()) { + addUsage(NamingCheckFailures, Typedef->getDecl(), + Value->getSourceRange()); + } + for (unsigned i = 0; i < Value->getNumParams(); ++i) { + if (const auto *Typedef = Value->parameters()[i] + ->getType() + .getTypePtr() + ->getAs<TypedefType>()) { + addUsage(NamingCheckFailures, Typedef->getDecl(), + Value->getSourceRange()); + } + } + } + // Ignore ClassTemplateSpecializationDecl which are creating duplicate // replacements with CXXRecordDecl if (isa<ClassTemplateSpecializationDecl>(Decl)) @@ -644,29 +750,74 @@ void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) { 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, + const MacroInfo *MI) { + 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(MI->getDefinitionLoc(), Name); + NamingCheckFailure &Failure = NamingCheckFailures[ID]; + SourceRange Range(MacroNameTok.getLocation(), MacroNameTok.getEndLoc()); + + Failure.Fixup = std::move(Fixup); + Failure.KindName = std::move(KindName); + addUsage(NamingCheckFailures, ID, Range); + } +} + +void IdentifierNamingCheck::expandMacro(const Token &MacroNameTok, + const MacroInfo *MI) { + StringRef Name = MacroNameTok.getIdentifierInfo()->getName(); + NamingCheckId ID(MI->getDefinitionLoc(), Name); + + auto Failure = NamingCheckFailures.find(ID); + if (Failure == NamingCheckFailures.end()) + return; + + SourceRange Range(MacroNameTok.getLocation(), MacroNameTok.getEndLoc()); + 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 diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h index 550b334194d..953803ca8fc 100644 --- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h +++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h @@ -13,6 +13,9 @@ #include "../ClangTidy.h" namespace clang { + +class MacroInfo; + namespace tidy { namespace readability { @@ -36,6 +39,7 @@ public: 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 +68,7 @@ public: /// \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 +85,19 @@ public: NamingCheckFailure() : ShouldFix(true) {} }; - typedef llvm::DenseMap<const NamedDecl *, NamingCheckFailure> + + typedef std::pair<SourceLocation, std::string> NamingCheckId; + + typedef llvm::DenseMap<NamingCheckId, NamingCheckFailure> NamingCheckFailureMap; + /// Check Macros for style violations. + void checkMacro(SourceManager &sourceMgr, const Token &MacroNameTok, + const MacroInfo *MI); + + /// Add a usage of a macro if it already has a violation. + void expandMacro(const Token &MacroNameTok, const MacroInfo *MI); + private: std::vector<NamingStyle> NamingStyles; bool IgnoreFailedSplit; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 827601d78ef..e153b60a941 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -249,6 +249,11 @@ identified. The improvements since the 3.8 release include: Warns about defaulted constructors and assignment operators that are actually deleted. +- Updated `readability-identifier-naming-check + <http://clang.llvm.org/extra/clang-tidy/checks/readability-identifier-naming.html>`_ + + Added support for enforcing the case of macro statements. + - New `readability-redundant-control-flow <http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-control-flow.html>`_ check diff --git a/clang-tools-extra/test/clang-tidy/readability-identifier-naming.cpp b/clang-tools-extra/test/clang-tidy/readability-identifier-naming.cpp index 659736303e9..42532489551 100644 --- a/clang-tools-extra/test/clang-tidy/readability-identifier-naming.cpp +++ b/clang-tools-extra/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.TypeAliasCase, value: lower_case}, \ // RUN: {key: readability-identifier-naming.TypeAliasSuffix, value: '_t'}, \ // RUN: {key: readability-identifier-naming.IgnoreFailedSplit, value: 0} \ @@ -307,6 +308,12 @@ typedef THIS___Structure struct_type; // 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; +} + using my_struct_type = THIS___Structure; // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for type alias 'my_struct_type' // CHECK-FIXES: {{^}}using my_struct_type_t = this_structure;{{$}} @@ -329,5 +336,11 @@ static void static_Function() { // 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() + +void MY_TEST_Macro(function) {} +// CHECK-FIXES: {{^}}void MY_TEST_MACRO(function) {} } } |
