diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp index 6a39ece097771..119502e91ffe7 100644 --- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp @@ -8,15 +8,13 @@ #include "IdentifierNamingCheck.h" -#include "../utils/ASTUtils.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/AST/CXXInheritance.h" -#include "clang/Frontend/CompilerInstance.h" #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" +#include "llvm/Support/Regex.h" #define DEBUG_TYPE "clang-tidy" @@ -126,7 +124,9 @@ class IdentifierNamingCheckPPCallbacks : public PPCallbacks { IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name, ClangTidyContext *Context) - : RenamerClangTidyCheck(Name, Context) { + : RenamerClangTidyCheck(Name, Context), + IgnoreFailedSplit(Options.get("IgnoreFailedSplit", 0)), + IgnoreMainLikeFunctions(Options.get("IgnoreMainLikeFunctions", 0)) { auto const fromString = [](StringRef Str) { return llvm::StringSwitch>(Str) .Case("aNy_CasE", CT_AnyCase) @@ -151,8 +151,6 @@ IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name, NamingStyles.push_back(llvm::None); } } - - IgnoreFailedSplit = Options.get("IgnoreFailedSplit", 0); } IdentifierNamingCheck::~IdentifierNamingCheck() = default; @@ -193,6 +191,7 @@ void IdentifierNamingCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { } Options.store(Opts, "IgnoreFailedSplit", IgnoreFailedSplit); + Options.store(Opts, "IgnoreMainLikeFunctions", IgnoreMainLikeFunctions); } static bool matchesStyle(StringRef Name, @@ -324,6 +323,67 @@ static std::string fixupWithCase(StringRef Name, return Fixup; } +static bool isParamInMainLikeFunction(const ParmVarDecl &ParmDecl, + bool IncludeMainLike) { + const auto *FDecl = + dyn_cast_or_null(ParmDecl.getParentFunctionOrMethod()); + if (!FDecl) + return false; + if (FDecl->isMain()) + return true; + if (!IncludeMainLike) + return false; + if (FDecl->getAccess() != AS_public && FDecl->getAccess() != AS_none) + return false; + enum MainType { None, Main, WMain }; + auto IsCharPtrPtr = [](QualType QType) -> MainType { + if (QType.isNull()) + return None; + if (QType = QType->getPointeeType(), QType.isNull()) + return None; + if (QType = QType->getPointeeType(), QType.isNull()) + return None; + if (QType->isCharType()) + return Main; + if (QType->isWideCharType()) + return WMain; + return None; + }; + auto IsIntType = [](QualType QType) { + if (QType.isNull()) + return false; + if (const auto *Builtin = + dyn_cast(QType->getUnqualifiedDesugaredType())) { + return Builtin->getKind() == BuiltinType::Int; + } + return false; + }; + if (!IsIntType(FDecl->getReturnType())) + return false; + if (FDecl->getNumParams() < 2 || FDecl->getNumParams() > 3) + return false; + if (!IsIntType(FDecl->parameters()[0]->getType())) + return false; + MainType Type = IsCharPtrPtr(FDecl->parameters()[1]->getType()); + if (Type == None) + return false; + if (FDecl->getNumParams() == 3 && + IsCharPtrPtr(FDecl->parameters()[2]->getType()) != Type) + return false; + + if (Type == Main) { + static llvm::Regex Matcher( + "(^[Mm]ain([_A-Z]|$))|([a-z0-9_]Main([_A-Z]|$))|(_main(_|$))"); + assert(Matcher.isValid() && "Invalid Matcher for main like functions."); + return Matcher.match(FDecl->getName()); + } else { + static llvm::Regex Matcher("(^((W[Mm])|(wm))ain([_A-Z]|$))|([a-z0-9_]W[Mm]" + "ain([_A-Z]|$))|(_wmain(_|$))"); + assert(Matcher.isValid() && "Invalid Matcher for wmain like functions."); + return Matcher.match(FDecl->getName()); + } +} + static std::string fixupWithStyle(StringRef Name, const IdentifierNamingCheck::NamingStyle &Style) { @@ -338,7 +398,8 @@ fixupWithStyle(StringRef Name, static StyleKind findStyleKind( const NamedDecl *D, const std::vector> - &NamingStyles) { + &NamingStyles, + bool IgnoreMainLikeFunctions) { assert(D && D->getIdentifier() && !D->getName().empty() && !D->isImplicit() && "Decl must be an explicit identifier with a name."); @@ -434,6 +495,8 @@ static StyleKind findStyleKind( } if (const auto *Decl = dyn_cast(D)) { + if (isParamInMainLikeFunction(*Decl, IgnoreMainLikeFunctions)) + return SK_Invalid; QualType Type = Decl->getType(); if (Decl->isConstexpr() && NamingStyles[SK_ConstexprVariable]) @@ -615,7 +678,7 @@ static StyleKind findStyleKind( llvm::Optional IdentifierNamingCheck::GetDeclFailureInfo(const NamedDecl *Decl, const SourceManager &SM) const { - StyleKind SK = findStyleKind(Decl, NamingStyles); + StyleKind SK = findStyleKind(Decl, NamingStyles, IgnoreMainLikeFunctions); if (SK == SK_Invalid) return None; diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h index 86cd52f5e751b..04bf53fe16b56 100644 --- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h +++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h @@ -70,7 +70,8 @@ class IdentifierNamingCheck final : public RenamerClangTidyCheck { const NamingCheckFailure &Failure) const override; std::vector> NamingStyles; - bool IgnoreFailedSplit; + const bool IgnoreFailedSplit; + const bool IgnoreMainLikeFunctions; }; } // namespace readability diff --git a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp index dfcd0818ea958..33c01d7cfefeb 100644 --- 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 @@ void RenamerClangTidyCheck::registerMatchers(MatchFinder *Finder) { 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 @@ void RenamerClangTidyCheck::check(const MatchFinder::MatchResult &Result) { 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 index 196ab3fe8c119..15f0b3ad0fb5e 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -104,6 +104,11 @@ New aliases 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/docs/clang-tidy/checks/readability-identifier-naming.rst b/clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst index c419b0304d4b0..0998bf28c2784 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst @@ -55,6 +55,7 @@ The following options are describe below: - :option:`GlobalFunctionCase`, :option:`GlobalFunctionPrefix`, :option:`GlobalFunctionSuffix` - :option:`GlobalPointerCase`, :option:`GlobalPointerPrefix`, :option:`GlobalPointerSuffix` - :option:`GlobalVariableCase`, :option:`GlobalVariablePrefix`, :option:`GlobalVariableSuffix` + - :option:`IgnoreMainLikeFunctions` - :option:`InlineNamespaceCase`, :option:`InlineNamespacePrefix`, :option:`InlineNamespaceSuffix` - :option:`LocalConstantCase`, :option:`LocalConstantPrefix`, :option:`LocalConstantSuffix` - :option:`LocalConstantPointerCase`, :option:`LocalConstantPointerPrefix`, :option:`LocalConstantPointerSuffix` @@ -827,6 +828,12 @@ After: int pre_global3_post; +.. option:: IgnoreMainLikeFunctions + + When set to `1` functions that have a similar signature to ``main`` or + ``wmain`` won't enforce checks on the names of their parameters. + Default value is `0`. + .. option:: InlineNamespaceCase When defined, the check will ensure inline namespaces names conform to the diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-main-like.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-main-like.cpp new file mode 100644 index 0000000000000..6336ffb6b1ff1 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-main-like.cpp @@ -0,0 +1,88 @@ +// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \ +// RUN: -config='{CheckOptions: [ \ +// RUN: {key: readability-identifier-naming.ParameterCase, value: CamelCase}, \ +// RUN: {key: readability-identifier-naming.IgnoreMainLikeFunctions, value: 1} \ +// RUN: ]}' + +int mainLike(int argc, char **argv); +int mainLike(int argc, char **argv, const char **env); +int mainLike(int argc, const char **argv); +int mainLike(int argc, const char **argv, const char **env); +int mainLike(int argc, char *argv[]); +int mainLike(int argc, const char *argv[]); +int mainLike(int argc, char *argv[], char *env[]); +int mainLike(int argc, const char *argv[], const char *env[]); +void notMain(int argc, char **argv); +// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for parameter 'argc' +// CHECK-MESSAGES: :[[@LINE-2]]:31: warning: invalid case style for parameter 'argv' +void notMain(int argc, char **argv, char **env); +// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for parameter 'argc' +// CHECK-MESSAGES: :[[@LINE-2]]:31: warning: invalid case style for parameter 'argv' +// CHECK-MESSAGES: :[[@LINE-3]]:44: warning: invalid case style for parameter 'env' +int notMain(int argc, char **argv, char **env, int Extra); +// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for parameter 'argc' +// CHECK-MESSAGES: :[[@LINE-2]]:30: warning: invalid case style for parameter 'argv' +// CHECK-MESSAGES: :[[@LINE-3]]:43: warning: invalid case style for parameter 'env' +int notMain(int argc, char **argv, int Extra); +// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for parameter 'argc' +// CHECK-MESSAGES: :[[@LINE-2]]:30: warning: invalid case style for parameter 'argv' +int notMain(int argc, char *argv); +// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for parameter 'argc' +// CHECK-MESSAGES: :[[@LINE-2]]:29: warning: invalid case style for parameter 'argv' +int notMain(unsigned argc, char **argv); +// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: invalid case style for parameter 'argc' +// CHECK-MESSAGES: :[[@LINE-2]]:35: warning: invalid case style for parameter 'argv' +int notMain(long argc, char *argv); +// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for parameter 'argc' +// CHECK-MESSAGES: :[[@LINE-2]]:30: warning: invalid case style for parameter 'argv' +int notMain(int argc, char16_t **argv); +// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for parameter 'argc' +// CHECK-MESSAGES: :[[@LINE-2]]:34: warning: invalid case style for parameter 'argv' +int notMain(int argc, char argv[]); +// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for parameter 'argc' +// CHECK-MESSAGES: :[[@LINE-2]]:28: warning: invalid case style for parameter 'argv' +typedef char myFunChar; +typedef int myFunInt; +typedef char **myFunCharPtr; +typedef long myFunLong; +myFunInt mainLikeTypedef(myFunInt argc, myFunChar **argv); +int mainLikeTypedef(int argc, myFunCharPtr argv); +int notMainTypedef(myFunLong argc, char **argv); +// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: invalid case style for parameter 'argc' +// CHECK-MESSAGES: :[[@LINE-2]]:43: warning: invalid case style for parameter 'argv' + +// Don't flag as name contains the word main +int myMainFunction(int argc, char *argv[]); + +// This is fine, named with wmain and has wchar ptr. +int wmainLike(int argc, wchar_t *argv[]); + +// Flag this as has signature of main, but named as wmain. +int wmainLike(int argc, char *argv[]); +// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: invalid case style for parameter 'argc' +// CHECK-MESSAGES: :[[@LINE-2]]:31: warning: invalid case style for parameter 'argv' + +struct Foo { + Foo(int argc, char *argv[]) {} + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: invalid case style for parameter 'argc' + // CHECK-MESSAGES: :[[@LINE-2]]:23: warning: invalid case style for parameter 'argv' + + int mainPub(int argc, char *argv[]); + static int mainPubStatic(int argc, char *argv[]); + +protected: + int mainProt(int argc, char *argv[]); + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for parameter 'argc' + // CHECK-MESSAGES: :[[@LINE-2]]:32: warning: invalid case style for parameter 'argv' + static int mainProtStatic(int argc, char *argv[]); + // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: invalid case style for parameter 'argc' + // CHECK-MESSAGES: :[[@LINE-2]]:45: warning: invalid case style for parameter 'argv' + +private: + int mainPriv(int argc, char *argv[]); + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for parameter 'argc' + // CHECK-MESSAGES: :[[@LINE-2]]:32: warning: invalid case style for parameter 'argv' + static int mainPrivStatic(int argc, char *argv[]); + // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: invalid case style for parameter 'argc' + // CHECK-MESSAGES: :[[@LINE-2]]:45: warning: invalid case style for parameter 'argv' +}; 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 index 177b3e2116a82..4e9234d2c793f 100644 --- 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 @@ class vector { //NOLINT }; //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 @@ class Container { 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