Author: Nathan Date: 2020-01-27T23:47:51Z New Revision: c3d20fd472000c7abdc2c9cad051a2ab339709d9
URL: https://github.com/llvm/llvm-project/commit/c3d20fd472000c7abdc2c9cad051a2ab339709d9 DIFF: https://github.com/llvm/llvm-project/commit/c3d20fd472000c7abdc2c9cad051a2ab339709d9.diff LOG: [clang-tidy] readability-identifier-naming disregards parameters restrictions on main like functions Summary: Typically most main functions have the signature: ``` int main(int argc, char *argv[]) ``` To stick with convention when renaming parameters we should ignore the `argc` and `argv` names even if the parameter style says they should be renamed. This patch addresses this by checking all ParmVarDecls if they form part of a function with a signature that matches main `int name(int argc, char * argv[], (optional char *env[]))` Reviewers: aaron.ballman, JonasToth, alexfh, hokein Reviewed By: aaron.ballman Subscribers: Mordante, merge_guards_bot, xazax.hun, kristof.beyls, cfe-commits Tags: #clang, #clang-tools-extra Differential Revision: https://reviews.llvm.org/D73098 Added: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-main-like.cpp Modified: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp index 6a39ece09777..119502e91ffe 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<llvm::Optional<CaseType>>(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<FunctionDecl>(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<BuiltinType>(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<llvm::Optional<IdentifierNamingCheck::NamingStyle>> - &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<ParmVarDecl>(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<RenamerClangTidyCheck::FailureInfo> 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 86cd52f5e751..04bf53fe16b5 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<llvm::Optional<NamingStyle>> NamingStyles; - bool IgnoreFailedSplit; + const bool IgnoreFailedSplit; + const bool IgnoreMainLikeFunctions; }; } // namespace readability 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 c419b0304d4b..0998bf28c278 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 000000000000..6336ffb6b1ff --- /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' +}; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
