Skip to content

Commit 27e3671

Browse files
njames93njames93
committed
[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
1 parent 369ea47 commit 27e3671

File tree

7 files changed

+312
-24
lines changed

7 files changed

+312
-24
lines changed

‎clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp

Lines changed: 71 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,13 @@
88

99
#include "IdentifierNamingCheck.h"
1010

11-
#include "../utils/ASTUtils.h"
12-
#include "clang/ASTMatchers/ASTMatchFinder.h"
1311
#include "clang/AST/CXXInheritance.h"
14-
#include "clang/Frontend/CompilerInstance.h"
1512
#include "clang/Lex/PPCallbacks.h"
1613
#include "clang/Lex/Preprocessor.h"
1714
#include "llvm/ADT/DenseMapInfo.h"
1815
#include "llvm/Support/Debug.h"
1916
#include "llvm/Support/Format.h"
17+
#include "llvm/Support/Regex.h"
2018

2119
#define DEBUG_TYPE "clang-tidy"
2220

@@ -126,7 +124,9 @@ class IdentifierNamingCheckPPCallbacks : public PPCallbacks {
126124

127125
IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name,
128126
ClangTidyContext *Context)
129-
: RenamerClangTidyCheck(Name, Context) {
127+
: RenamerClangTidyCheck(Name, Context),
128+
IgnoreFailedSplit(Options.get("IgnoreFailedSplit", 0)),
129+
IgnoreMainLikeFunctions(Options.get("IgnoreMainLikeFunctions", 0)) {
130130
auto const fromString = [](StringRef Str) {
131131
return llvm::StringSwitch<llvm::Optional<CaseType>>(Str)
132132
.Case("aNy_CasE", CT_AnyCase)
@@ -151,8 +151,6 @@ IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name,
151151
NamingStyles.push_back(llvm::None);
152152
}
153153
}
154-
155-
IgnoreFailedSplit = Options.get("IgnoreFailedSplit", 0);
156154
}
157155

158156
IdentifierNamingCheck::~IdentifierNamingCheck() = default;
@@ -193,6 +191,7 @@ void IdentifierNamingCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
193191
}
194192

195193
Options.store(Opts, "IgnoreFailedSplit", IgnoreFailedSplit);
194+
Options.store(Opts, "IgnoreMainLikeFunctions", IgnoreMainLikeFunctions);
196195
}
197196

198197
static bool matchesStyle(StringRef Name,
@@ -324,6 +323,67 @@ static std::string fixupWithCase(StringRef Name,
324323
return Fixup;
325324
}
326325

326+
static bool isParamInMainLikeFunction(const ParmVarDecl &ParmDecl,
327+
bool IncludeMainLike) {
328+
const auto *FDecl =
329+
dyn_cast_or_null<FunctionDecl>(ParmDecl.getParentFunctionOrMethod());
330+
if (!FDecl)
331+
return false;
332+
if (FDecl->isMain())
333+
return true;
334+
if (!IncludeMainLike)
335+
return false;
336+
if (FDecl->getAccess() != AS_public && FDecl->getAccess() != AS_none)
337+
return false;
338+
enum MainType { None, Main, WMain };
339+
auto IsCharPtrPtr = [](QualType QType) -> MainType {
340+
if (QType.isNull())
341+
return None;
342+
if (QType = QType->getPointeeType(), QType.isNull())
343+
return None;
344+
if (QType = QType->getPointeeType(), QType.isNull())
345+
return None;
346+
if (QType->isCharType())
347+
return Main;
348+
if (QType->isWideCharType())
349+
return WMain;
350+
return None;
351+
};
352+
auto IsIntType = [](QualType QType) {
353+
if (QType.isNull())
354+
return false;
355+
if (const auto *Builtin =
356+
dyn_cast<BuiltinType>(QType->getUnqualifiedDesugaredType())) {
357+
return Builtin->getKind() == BuiltinType::Int;
358+
}
359+
return false;
360+
};
361+
if (!IsIntType(FDecl->getReturnType()))
362+
return false;
363+
if (FDecl->getNumParams() < 2 || FDecl->getNumParams() > 3)
364+
return false;
365+
if (!IsIntType(FDecl->parameters()[0]->getType()))
366+
return false;
367+
MainType Type = IsCharPtrPtr(FDecl->parameters()[1]->getType());
368+
if (Type == None)
369+
return false;
370+
if (FDecl->getNumParams() == 3 &&
371+
IsCharPtrPtr(FDecl->parameters()[2]->getType()) != Type)
372+
return false;
373+
374+
if (Type == Main) {
375+
static llvm::Regex Matcher(
376+
"(^[Mm]ain([_A-Z]|$))|([a-z0-9_]Main([_A-Z]|$))|(_main(_|$))");
377+
assert(Matcher.isValid() && "Invalid Matcher for main like functions.");
378+
return Matcher.match(FDecl->getName());
379+
} else {
380+
static llvm::Regex Matcher("(^((W[Mm])|(wm))ain([_A-Z]|$))|([a-z0-9_]W[Mm]"
381+
"ain([_A-Z]|$))|(_wmain(_|$))");
382+
assert(Matcher.isValid() && "Invalid Matcher for wmain like functions.");
383+
return Matcher.match(FDecl->getName());
384+
}
385+
}
386+
327387
static std::string
328388
fixupWithStyle(StringRef Name,
329389
const IdentifierNamingCheck::NamingStyle &Style) {
@@ -338,7 +398,8 @@ fixupWithStyle(StringRef Name,
338398
static StyleKind findStyleKind(
339399
const NamedDecl *D,
340400
const std::vector<llvm::Optional<IdentifierNamingCheck::NamingStyle>>
341-
&NamingStyles) {
401+
&NamingStyles,
402+
bool IgnoreMainLikeFunctions) {
342403
assert(D && D->getIdentifier() && !D->getName().empty() && !D->isImplicit() &&
343404
"Decl must be an explicit identifier with a name.");
344405

@@ -434,6 +495,8 @@ static StyleKind findStyleKind(
434495
}
435496

436497
if (const auto *Decl = dyn_cast<ParmVarDecl>(D)) {
498+
if (isParamInMainLikeFunction(*Decl, IgnoreMainLikeFunctions))
499+
return SK_Invalid;
437500
QualType Type = Decl->getType();
438501

439502
if (Decl->isConstexpr() && NamingStyles[SK_ConstexprVariable])
@@ -615,7 +678,7 @@ static StyleKind findStyleKind(
615678
llvm::Optional<RenamerClangTidyCheck::FailureInfo>
616679
IdentifierNamingCheck::GetDeclFailureInfo(const NamedDecl *Decl,
617680
const SourceManager &SM) const {
618-
StyleKind SK = findStyleKind(Decl, NamingStyles);
681+
StyleKind SK = findStyleKind(Decl, NamingStyles, IgnoreMainLikeFunctions);
619682
if (SK == SK_Invalid)
620683
return None;
621684

‎clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ class IdentifierNamingCheck final : public RenamerClangTidyCheck {
7070
const NamingCheckFailure &Failure) const override;
7171

7272
std::vector<llvm::Optional<NamingStyle>> NamingStyles;
73-
bool IgnoreFailedSplit;
73+
const bool IgnoreFailedSplit;
74+
const bool IgnoreMainLikeFunctions;
7475
};
7576

7677
} // namespace readability

‎clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@
1414
#include "clang/Lex/PPCallbacks.h"
1515
#include "clang/Lex/Preprocessor.h"
1616
#include "llvm/ADT/DenseMapInfo.h"
17-
#include "llvm/Support/Debug.h"
18-
#include "llvm/Support/Format.h"
1917

2018
#define DEBUG_TYPE "clang-tidy"
2119

@@ -106,20 +104,22 @@ void RenamerClangTidyCheck::registerMatchers(MatchFinder *Finder) {
106104
this);
107105
Finder->addMatcher(typeLoc().bind("typeLoc"), this);
108106
Finder->addMatcher(nestedNameSpecifierLoc().bind("nestedNameLoc"), this);
107+
auto MemberOrDependent =
108+
expr(eachOf(memberExpr().bind("memberExpr"),
109+
cxxDependentScopeMemberExpr().bind("depMemberExpr")));
109110
Finder->addMatcher(
110111
functionDecl(unless(cxxMethodDecl(isImplicit())),
111-
hasBody(forEachDescendant(memberExpr().bind("memberExpr")))),
112+
hasBody(forEachDescendant(MemberOrDependent))),
112113
this);
113114
Finder->addMatcher(
114-
cxxConstructorDecl(
115-
unless(isImplicit()),
116-
forEachConstructorInitializer(
117-
allOf(isWritten(), withInitializer(forEachDescendant(
118-
memberExpr().bind("memberExpr")))))),
115+
cxxConstructorDecl(unless(isImplicit()),
116+
forEachConstructorInitializer(allOf(
117+
isWritten(), withInitializer(forEachDescendant(
118+
MemberOrDependent))))),
119+
this);
120+
Finder->addMatcher(
121+
fieldDecl(hasInClassInitializer(forEachDescendant(MemberOrDependent))),
119122
this);
120-
Finder->addMatcher(fieldDecl(hasInClassInitializer(
121-
forEachDescendant(memberExpr().bind("memberExpr")))),
122-
this);
123123
}
124124

125125
void RenamerClangTidyCheck::registerPPCallbacks(
@@ -271,6 +271,39 @@ void RenamerClangTidyCheck::check(const MatchFinder::MatchResult &Result) {
271271
return;
272272
}
273273

274+
if (const auto *DepMemberRef =
275+
Result.Nodes.getNodeAs<CXXDependentScopeMemberExpr>(
276+
"depMemberExpr")) {
277+
QualType BaseType = DepMemberRef->isArrow()
278+
? DepMemberRef->getBaseType()->getPointeeType()
279+
: DepMemberRef->getBaseType();
280+
if (BaseType.isNull())
281+
return;
282+
const CXXRecordDecl *Base = BaseType.getTypePtr()->getAsCXXRecordDecl();
283+
if (!Base)
284+
return;
285+
DeclarationName DeclName = DepMemberRef->getMemberNameInfo().getName();
286+
if (!DeclName.isIdentifier())
287+
return;
288+
StringRef DependentName = DeclName.getAsIdentifierInfo()->getName();
289+
for (const FieldDecl *Field : Base->fields()) {
290+
if (Field->getParent() == Base && Field->getDeclName().isIdentifier() &&
291+
Field->getName().equals(DependentName)) {
292+
SourceRange Range = DepMemberRef->getMemberNameInfo().getSourceRange();
293+
addUsage(NamingCheckFailures, Field, Range, Result.SourceManager);
294+
return;
295+
}
296+
}
297+
for (const CXXMethodDecl *Method : Base->methods()) {
298+
if (Method->getParent() == Base && Method->getDeclName().isIdentifier() &&
299+
Method->getName().equals(DependentName)) {
300+
SourceRange Range = DepMemberRef->getMemberNameInfo().getSourceRange();
301+
addUsage(NamingCheckFailures, Method, Range, Result.SourceManager);
302+
return;
303+
}
304+
}
305+
}
306+
274307
if (const auto *Decl = Result.Nodes.getNodeAs<NamedDecl>("decl")) {
275308
if (!Decl->getIdentifier() || Decl->getName().empty() || Decl->isImplicit())
276309
return;

‎clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,11 @@ New aliases
104104
Changes in existing checks
105105
^^^^^^^^^^^^^^^^^^^^^^^^^^
106106

107+
- Improved :doc:'readability-identifier-naming
108+
<clang-tidy/checks/readability-identifier-naming>` check.
109+
110+
Now able to rename member references in class template definitions with
111+
explicit access.
107112

108113
Renamed checks
109114
^^^^^^^^^^^^^^

‎clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ The following options are describe below:
5555
- :option:`GlobalFunctionCase`, :option:`GlobalFunctionPrefix`, :option:`GlobalFunctionSuffix`
5656
- :option:`GlobalPointerCase`, :option:`GlobalPointerPrefix`, :option:`GlobalPointerSuffix`
5757
- :option:`GlobalVariableCase`, :option:`GlobalVariablePrefix`, :option:`GlobalVariableSuffix`
58+
- :option:`IgnoreMainLikeFunctions`
5859
- :option:`InlineNamespaceCase`, :option:`InlineNamespacePrefix`, :option:`InlineNamespaceSuffix`
5960
- :option:`LocalConstantCase`, :option:`LocalConstantPrefix`, :option:`LocalConstantSuffix`
6061
- :option:`LocalConstantPointerCase`, :option:`LocalConstantPointerPrefix`, :option:`LocalConstantPointerSuffix`
@@ -827,6 +828,12 @@ After:
827828

828829
int pre_global3_post;
829830

831+
.. option:: IgnoreMainLikeFunctions
832+
833+
When set to `1` functions that have a similar signature to ``main`` or
834+
``wmain`` won't enforce checks on the names of their parameters.
835+
Default value is `0`.
836+
830837
.. option:: InlineNamespaceCase
831838

832839
When defined, the check will ensure inline namespaces names conform to the

‎clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-main-like.cpp

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
2+
// RUN: -config='{CheckOptions: [ \
3+
// RUN: {key: readability-identifier-naming.ParameterCase, value: CamelCase}, \
4+
// RUN: {key: readability-identifier-naming.IgnoreMainLikeFunctions, value: 1} \
5+
// RUN: ]}'
6+
7+
int mainLike(int argc, char **argv);
8+
int mainLike(int argc, char **argv, const char **env);
9+
int mainLike(int argc, const char **argv);
10+
int mainLike(int argc, const char **argv, const char **env);
11+
int mainLike(int argc, char *argv[]);
12+
int mainLike(int argc, const char *argv[]);
13+
int mainLike(int argc, char *argv[], char *env[]);
14+
int mainLike(int argc, const char *argv[], const char *env[]);
15+
void notMain(int argc, char **argv);
16+
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for parameter 'argc'
17+
// CHECK-MESSAGES: :[[@LINE-2]]:31: warning: invalid case style for parameter 'argv'
18+
void notMain(int argc, char **argv, char **env);
19+
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for parameter 'argc'
20+
// CHECK-MESSAGES: :[[@LINE-2]]:31: warning: invalid case style for parameter 'argv'
21+
// CHECK-MESSAGES: :[[@LINE-3]]:44: warning: invalid case style for parameter 'env'
22+
int notMain(int argc, char **argv, char **env, int Extra);
23+
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for parameter 'argc'
24+
// CHECK-MESSAGES: :[[@LINE-2]]:30: warning: invalid case style for parameter 'argv'
25+
// CHECK-MESSAGES: :[[@LINE-3]]:43: warning: invalid case style for parameter 'env'
26+
int notMain(int argc, char **argv, int Extra);
27+
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for parameter 'argc'
28+
// CHECK-MESSAGES: :[[@LINE-2]]:30: warning: invalid case style for parameter 'argv'
29+
int notMain(int argc, char *argv);
30+
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for parameter 'argc'
31+
// CHECK-MESSAGES: :[[@LINE-2]]:29: warning: invalid case style for parameter 'argv'
32+
int notMain(unsigned argc, char **argv);
33+
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: invalid case style for parameter 'argc'
34+
// CHECK-MESSAGES: :[[@LINE-2]]:35: warning: invalid case style for parameter 'argv'
35+
int notMain(long argc, char *argv);
36+
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for parameter 'argc'
37+
// CHECK-MESSAGES: :[[@LINE-2]]:30: warning: invalid case style for parameter 'argv'
38+
int notMain(int argc, char16_t **argv);
39+
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for parameter 'argc'
40+
// CHECK-MESSAGES: :[[@LINE-2]]:34: warning: invalid case style for parameter 'argv'
41+
int notMain(int argc, char argv[]);
42+
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for parameter 'argc'
43+
// CHECK-MESSAGES: :[[@LINE-2]]:28: warning: invalid case style for parameter 'argv'
44+
typedef char myFunChar;
45+
typedef int myFunInt;
46+
typedef char **myFunCharPtr;
47+
typedef long myFunLong;
48+
myFunInt mainLikeTypedef(myFunInt argc, myFunChar **argv);
49+
int mainLikeTypedef(int argc, myFunCharPtr argv);
50+
int notMainTypedef(myFunLong argc, char **argv);
51+
// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: invalid case style for parameter 'argc'
52+
// CHECK-MESSAGES: :[[@LINE-2]]:43: warning: invalid case style for parameter 'argv'
53+
54+
// Don't flag as name contains the word main
55+
int myMainFunction(int argc, char *argv[]);
56+
57+
// This is fine, named with wmain and has wchar ptr.
58+
int wmainLike(int argc, wchar_t *argv[]);
59+
60+
// Flag this as has signature of main, but named as wmain.
61+
int wmainLike(int argc, char *argv[]);
62+
// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: invalid case style for parameter 'argc'
63+
// CHECK-MESSAGES: :[[@LINE-2]]:31: warning: invalid case style for parameter 'argv'
64+
65+
struct Foo {
66+
Foo(int argc, char *argv[]) {}
67+
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: invalid case style for parameter 'argc'
68+
// CHECK-MESSAGES: :[[@LINE-2]]:23: warning: invalid case style for parameter 'argv'
69+
70+
int mainPub(int argc, char *argv[]);
71+
static int mainPubStatic(int argc, char *argv[]);
72+
73+
protected:
74+
int mainProt(int argc, char *argv[]);
75+
// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for parameter 'argc'
76+
// CHECK-MESSAGES: :[[@LINE-2]]:32: warning: invalid case style for parameter 'argv'
77+
static int mainProtStatic(int argc, char *argv[]);
78+
// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: invalid case style for parameter 'argc'
79+
// CHECK-MESSAGES: :[[@LINE-2]]:45: warning: invalid case style for parameter 'argv'
80+
81+
private:
82+
int mainPriv(int argc, char *argv[]);
83+
// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for parameter 'argc'
84+
// CHECK-MESSAGES: :[[@LINE-2]]:32: warning: invalid case style for parameter 'argv'
85+
static int mainPrivStatic(int argc, char *argv[]);
86+
// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: invalid case style for parameter 'argc'
87+
// CHECK-MESSAGES: :[[@LINE-2]]:45: warning: invalid case style for parameter 'argv'
88+
};

0 commit comments

Comments
 (0)