diff --git a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp --- a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp @@ -14,8 +14,6 @@ #include "clang/Lex/PPCallbacks.h" #include "clang/Lex/Preprocessor.h" #include "llvm/ADT/DenseMapInfo.h" -#include "llvm/Support/Debug.h" -#include "llvm/Support/Format.h" #define DEBUG_TYPE "clang-tidy" @@ -106,20 +104,22 @@ this); Finder->addMatcher(typeLoc().bind("typeLoc"), this); Finder->addMatcher(nestedNameSpecifierLoc().bind("nestedNameLoc"), this); + auto MemberOrDependent = + expr(eachOf(memberExpr().bind("memberExpr"), + cxxDependentScopeMemberExpr().bind("depMemberExpr"))); Finder->addMatcher( functionDecl(unless(cxxMethodDecl(isImplicit())), - hasBody(forEachDescendant(memberExpr().bind("memberExpr")))), + hasBody(forEachDescendant(MemberOrDependent))), this); Finder->addMatcher( - cxxConstructorDecl( - unless(isImplicit()), - forEachConstructorInitializer( - allOf(isWritten(), withInitializer(forEachDescendant( - memberExpr().bind("memberExpr")))))), + cxxConstructorDecl(unless(isImplicit()), + forEachConstructorInitializer(allOf( + isWritten(), withInitializer(forEachDescendant( + MemberOrDependent))))), + this); + Finder->addMatcher( + fieldDecl(hasInClassInitializer(forEachDescendant(MemberOrDependent))), this); - Finder->addMatcher(fieldDecl(hasInClassInitializer( - forEachDescendant(memberExpr().bind("memberExpr")))), - this); } void RenamerClangTidyCheck::registerPPCallbacks( @@ -271,6 +271,39 @@ return; } + if (const auto *DepMemberRef = + Result.Nodes.getNodeAs( + "depMemberExpr")) { + QualType BaseType = DepMemberRef->isArrow() + ? DepMemberRef->getBaseType()->getPointeeType() + : DepMemberRef->getBaseType(); + if (BaseType.isNull()) + return; + const CXXRecordDecl *Base = BaseType.getTypePtr()->getAsCXXRecordDecl(); + if (!Base) + return; + DeclarationName DeclName = DepMemberRef->getMemberNameInfo().getName(); + if (!DeclName.isIdentifier()) + return; + StringRef DependentName = DeclName.getAsIdentifierInfo()->getName(); + for (const FieldDecl *Field : Base->fields()) { + if (Field->getParent() == Base && Field->getDeclName().isIdentifier() && + Field->getName().equals(DependentName)) { + SourceRange Range = DepMemberRef->getMemberNameInfo().getSourceRange(); + addUsage(NamingCheckFailures, Field, Range, Result.SourceManager); + return; + } + } + for (const CXXMethodDecl *Method : Base->methods()) { + if (Method->getParent() == Base && Method->getDeclName().isIdentifier() && + Method->getName().equals(DependentName)) { + SourceRange Range = DepMemberRef->getMemberNameInfo().getSourceRange(); + addUsage(NamingCheckFailures, Method, Range, Result.SourceManager); + return; + } + } + } + if (const auto *Decl = Result.Nodes.getNodeAs("decl")) { if (!Decl->getIdentifier() || Decl->getName().empty() || Decl->isImplicit()) return; 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 @@ -97,6 +97,11 @@ Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ +- Improved :doc:'readability-identifier-naming + ` check. + + Now able to rename member references in class template definitions with + explicit access. Renamed checks ^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp @@ -1,7 +1,8 @@ // RUN: %check_clang_tidy %s readability-identifier-naming %t -- \ // RUN: -config='{CheckOptions: [ \ // RUN: {key: readability-identifier-naming.MemberCase, value: CamelCase}, \ -// RUN: {key: readability-identifier-naming.ParameterCase, value: CamelCase} \ +// RUN: {key: readability-identifier-naming.ParameterCase, value: CamelCase}, \ +// RUN: {key: readability-identifier-naming.MethodCase, value: camelBack} \ // RUN: ]}' -- -fno-delayed-template-parsing int set_up(int); @@ -83,12 +84,12 @@ }; //NOLINT }; // namespace std -class Foo { +class Foo { std::vector &stack; // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: invalid case style for member 'stack' [readability-identifier-naming] public: Foo(std::vector &stack) - // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for parameter 'stack' [readability-identifier-naming] + // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for parameter 'stack' [readability-identifier-naming] // CHECK-FIXES: {{^}} Foo(std::vector &Stack) : stack(stack) { // CHECK-FIXES: {{^}} : Stack(Stack) { @@ -134,4 +135,94 @@ void foo() { Container container; } -}; // namespace CtorInits +} // namespace CtorInits + +namespace resolved_dependance { +template +struct A0 { + int value; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for member 'value' + A0 &operator=(const A0 &Other) { + value = Other.value; // A0 + this->value = Other.value; // A0 + // CHECK-FIXES: {{^}} Value = Other.Value; // A0 + // CHECK-FIXES-NEXT: {{^}} this->Value = Other.Value; // A0 + return *this; + } + void outOfLineReset(); +}; + +template +void A0::outOfLineReset() { + this->value -= value; // A0 + // CHECK-FIXES: {{^}} this->Value -= Value; // A0 +} + +template +struct A1 { + int value; // A1 + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for member 'value' + // CHECK-FIXES: {{^}} int Value; // A1 + int GetValue() const { return value; } // A1 + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for method 'GetValue' + // CHECK-FIXES {{^}} int getValue() const { return Value; } // A1 + void SetValue(int Value) { this->value = Value; } // A1 + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for method 'SetValue' + // CHECK-FIXES {{^}} void setValue(int Value) { this->Value = Value; } // A1 + A1 &operator=(const A1 &Other) { + this->SetValue(Other.GetValue()); // A1 + this->value = Other.value; // A1 + // CHECK-FIXES: {{^}} this->setValue(Other.getValue()); // A1 + // CHECK-FIXES-NEXT: {{^}} this->Value = Other.Value; // A1 + return *this; + } + void outOfLineReset(); +}; + +template +void A1::outOfLineReset() { + this->value -= value; // A1 + // CHECK-FIXES: {{^}} this->Value -= Value; // A1 +} + +template +struct A2 { + int value; // A2 + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for member 'value' + // CHECK-FIXES: {{^}} int Value; // A2 + A2 &operator=(const A2 &Other) { + value = Other.value; // A2 + this->value = Other.value; // A2 + // CHECK-FIXES: {{^}} Value = Other.Value; // A2 + // CHECK-FIXES-NEXT: {{^}} this->Value = Other.Value; // A2 + return *this; + } +}; + +// create some instances to check it works when instantiated. +A1 AInt{}; +A1 BInt = (AInt.outOfLineReset(), AInt); +A1 AUnsigned{}; +A1 BUnsigned = AUnsigned; +} // namespace resolved_dependance + +namespace unresolved_dependance { +template +struct DependentBase { + int depValue; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for member 'depValue' +}; + +template +struct Derived : DependentBase { + Derived &operator=(const Derived &Other) { + this->depValue = Other.depValue; + // CHECK-FIXES-NOT: {{^}} this->DepValue = Other.DepValue; + return *this; + } +}; + +Derived AInt{}; +Derived BInt = AInt; + +} // namespace unresolved_dependance