Skip to content

Commit 0f6d495

Browse files
dmpolukhindmpolukhin
committed
[clang-tidy][readability-named-parameter] Add an option to print names without comment
Add InsertPlainNamesInForwardDecls option to readability-named-parameter check to insert parameter names without comments for forward declarations only. When enabled, forward declarations get plain parameter names (e.g., `int param`) while function definitions continue to use commented names (e.g., `int /*param*/`). Named parameters in forward decls don't cause compiler warnings and some developers prefer to have names without comments but in sync between declarations and the definition. Default behavior remains unchanged (InsertPlainNamesInForwardDecls=false). Example with InsertPlainNamesInForwardDecls=true: ```cpp // Forward declaration - gets plain name void func(int param); void func(int param) { ... = param; } ```
1 parent f7cdff7 commit 0f6d495

File tree

5 files changed

+85
-7
lines changed

5 files changed

+85
-7
lines changed

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

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,17 @@ using namespace clang::ast_matchers;
1515

1616
namespace clang::tidy::readability {
1717

18+
NamedParameterCheck::NamedParameterCheck(StringRef Name,
19+
ClangTidyContext *Context)
20+
: ClangTidyCheck(Name, Context),
21+
InsertPlainNamesInForwardDecls(
22+
Options.get("InsertPlainNamesInForwardDecls", false)) {}
23+
24+
void NamedParameterCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
25+
Options.store(Opts, "InsertPlainNamesInForwardDecls",
26+
InsertPlainNamesInForwardDecls);
27+
}
28+
1829
void NamedParameterCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
1930
Finder->addMatcher(functionDecl().bind("decl"), this);
2031
}
@@ -84,7 +95,8 @@ void NamedParameterCheck::check(const MatchFinder::MatchResult &Result) {
8495

8596
for (auto P : UnnamedParams) {
8697
// Fallback to an unused marker.
87-
StringRef NewName = "unused";
98+
constexpr StringRef FallbackName = "unused";
99+
StringRef NewName = FallbackName;
88100

89101
// If the method is overridden, try to copy the name from the base method
90102
// into the overrider.
@@ -105,12 +117,26 @@ void NamedParameterCheck::check(const MatchFinder::MatchResult &Result) {
105117
NewName = Name;
106118
}
107119

108-
// Now insert the comment. Note that getLocation() points to the place
120+
// Now insert the fix. Note that getLocation() points to the place
109121
// where the name would be, this allows us to also get complex cases like
110122
// function pointers right.
111123
const ParmVarDecl *Parm = P.first->getParamDecl(P.second);
112-
D << FixItHint::CreateInsertion(Parm->getLocation(),
113-
" /*" + NewName.str() + "*/");
124+
125+
// The fix depends on the InsertPlainNamesInForwardDecls option,
126+
// whether this is a forward declaration and whether the parameter has
127+
// a real name.
128+
bool IsForwardDeclaration = (!Definition || Function != Definition);
129+
if (InsertPlainNamesInForwardDecls && IsForwardDeclaration &&
130+
NewName != FallbackName) {
131+
// For forward declarations with InsertPlainNamesInForwardDecls enabled,
132+
// insert the parameter name without comments
133+
D << FixItHint::CreateInsertion(Parm->getLocation(),
134+
" " + NewName.str());
135+
} else {
136+
// Default behavior: insert the parameter name as a comment
137+
D << FixItHint::CreateInsertion(Parm->getLocation(),
138+
" /*" + NewName.str() + "*/");
139+
}
114140
}
115141
}
116142
}

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,16 @@ namespace clang::tidy::readability {
2626
/// Corresponding cpplint.py check name: 'readability/function'.
2727
class NamedParameterCheck : public ClangTidyCheck {
2828
public:
29-
NamedParameterCheck(StringRef Name, ClangTidyContext *Context)
30-
: ClangTidyCheck(Name, Context) {}
29+
NamedParameterCheck(StringRef Name, ClangTidyContext *Context);
3130
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
3231
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
32+
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
3333
std::optional<TraversalKind> getCheckTraversalKind() const override {
3434
return TK_IgnoreUnlessSpelledInSource;
3535
}
36+
37+
private:
38+
const bool InsertPlainNamesInForwardDecls;
3639
};
3740

3841
} // namespace clang::tidy::readability

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ Changes in existing checks
215215
- Improved :doc:`cppcoreguidelines-missing-std-forward
216216
<clang-tidy/checks/cppcoreguidelines/missing-std-forward>` check by adding a
217217
flag to specify the function used for forwarding instead of ``std::forward``.
218-
218+
219219
- Improved :doc:`cppcoreguidelines-pro-bounds-pointer-arithmetic
220220
<clang-tidy/checks/cppcoreguidelines/pro-bounds-pointer-arithmetic>` check by
221221
fixing false positives when calling indexing operators that do not perform
@@ -342,6 +342,11 @@ Changes in existing checks
342342
false negatives where math expressions are the operand of assignment operators
343343
or comparison operators.
344344

345+
- Improved :doc:`readability-named-parameter
346+
<clang-tidy/checks/readability/named-parameter>` check by adding the option
347+
`InsertPlainNamesInForwardDecls` to insert parameter names without comments
348+
for forward declarations only.
349+
345350
- Improved :doc:`readability-qualified-auto
346351
<clang-tidy/checks/readability/qualified-auto>` check by adding the option
347352
`AllowedTypes`, that excludes specified types from adding qualifiers.

‎clang-tools-extra/docs/clang-tidy/checks/readability/named-parameter.rst‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,12 @@ If a parameter is not utilized, its name can be commented out in a function defi
2323
}
2424

2525
Corresponding cpplint.py check name: `readability/function`.
26+
27+
Options
28+
-------
29+
30+
.. option:: InsertPlainNamesInForwardDecls
31+
32+
When `true`, the check will insert parameter names without comments for
33+
forward declarations only. When `false` (default), parameter names are
34+
always inserted as comments (e.g., ``/*param*/``).

‎clang-tools-extra/test/clang-tidy/checkers/readability/named-parameter.cpp‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,46 @@
11
// RUN: %check_clang_tidy %s readability-named-parameter %t
2+
// RUN: %check_clang_tidy -check-suffix=PLAIN-NAMES %s readability-named-parameter %t -- -config="{CheckOptions: [{key: readability-named-parameter.InsertPlainNamesInForwardDecls, value: true}]}"
23

34
void Method(char *) { /* */ }
45
// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: all parameters should be named in a function
56
// CHECK-FIXES: void Method(char * /*unused*/) { /* */ }
7+
// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:19: warning: all parameters should be named in a function
8+
// CHECK-FIXES-PLAIN-NAMES: void Method(char * /*unused*/) { /* */ }
69
void Method2(char *) {}
710
// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: all parameters should be named in a function
811
// CHECK-FIXES: void Method2(char * /*unused*/) {}
12+
// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:20: warning: all parameters should be named in a function
13+
// CHECK-FIXES-PLAIN-NAMES: void Method2(char * /*unused*/) {}
914
void Method3(char *, void *) {}
1015
// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: all parameters should be named in a function
1116
// CHECK-FIXES: void Method3(char * /*unused*/, void * /*unused*/) {}
17+
// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:20: warning: all parameters should be named in a function
18+
// CHECK-FIXES-PLAIN-NAMES: void Method3(char * /*unused*/, void * /*unused*/) {}
1219
void Method4(char *, int /*unused*/) {}
1320
// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: all parameters should be named in a function
1421
// CHECK-FIXES: void Method4(char * /*unused*/, int /*unused*/) {}
22+
// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:20: warning: all parameters should be named in a function
23+
// CHECK-FIXES-PLAIN-NAMES: void Method4(char * /*unused*/, int /*unused*/) {}
1524
void operator delete[](void *) throw() {}
1625
// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: all parameters should be named in a function
1726
// CHECK-FIXES: void operator delete[](void * /*unused*/) throw() {}
27+
// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:30: warning: all parameters should be named in a function
28+
// CHECK-FIXES-PLAIN-NAMES: void operator delete[](void * /*unused*/) throw() {}
1829
int Method5(int) { return 0; }
1930
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: all parameters should be named in a function
2031
// CHECK-FIXES: int Method5(int /*unused*/) { return 0; }
32+
// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:16: warning: all parameters should be named in a function
33+
// CHECK-FIXES-PLAIN-NAMES: int Method5(int /*unused*/) { return 0; }
2134
void Method6(void (*)(void *)) {}
2235
// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: all parameters should be named in a function
2336
// CHECK-FIXES: void Method6(void (* /*unused*/)(void *)) {}
37+
// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:21: warning: all parameters should be named in a function
38+
// CHECK-FIXES-PLAIN-NAMES: void Method6(void (* /*unused*/)(void *)) {}
2439
template <typename T> void Method7(T) {}
2540
// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: all parameters should be named in a function
2641
// CHECK-FIXES: template <typename T> void Method7(T /*unused*/) {}
42+
// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:37: warning: all parameters should be named in a function
43+
// CHECK-FIXES-PLAIN-NAMES: template <typename T> void Method7(T /*unused*/) {}
2744

2845
// Don't warn in macros.
2946
#define M void MethodM(int) {}
@@ -55,6 +72,8 @@ struct Y {
5572
void foo(T) {}
5673
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: all parameters should be named in a function
5774
// CHECK-FIXES: void foo(T /*unused*/) {}
75+
// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:13: warning: all parameters should be named in a function
76+
// CHECK-FIXES-PLAIN-NAMES: void foo(T /*unused*/) {}
5877
};
5978

6079
Y<int> y;
@@ -69,19 +88,27 @@ struct Derived : public Base {
6988
void foo(int);
7089
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: all parameters should be named in a function
7190
// CHECK-FIXES: void foo(int /*argname*/);
91+
// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:15: warning: all parameters should be named in a function
92+
// CHECK-FIXES-PLAIN-NAMES: void foo(int argname);
7293
};
7394

7495
void FDef(int);
7596
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: all parameters should be named in a function
7697
// CHECK-FIXES: void FDef(int /*n*/);
98+
// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:14: warning: all parameters should be named in a function
99+
// CHECK-FIXES-PLAIN-NAMES: void FDef(int n);
77100
void FDef(int n) {}
78101

79102
void FDef2(int, int);
80103
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: all parameters should be named in a function
81104
// CHECK-FIXES: void FDef2(int /*n*/, int /*unused*/);
105+
// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:15: warning: all parameters should be named in a function
106+
// CHECK-FIXES-PLAIN-NAMES: void FDef2(int n, int /*unused*/);
82107
void FDef2(int n, int) {}
83108
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: all parameters should be named in a function
84109
// CHECK-FIXES: void FDef2(int n, int /*unused*/) {}
110+
// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:22: warning: all parameters should be named in a function
111+
// CHECK-FIXES-PLAIN-NAMES: void FDef2(int n, int /*unused*/) {}
85112

86113
void FNoDef(int);
87114

@@ -91,18 +118,26 @@ Z the_z;
91118
Z &operator++(Z&) { return the_z; }
92119
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: all parameters should be named in a function
93120
// CHECK-FIXES: Z &operator++(Z& /*unused*/) { return the_z; }
121+
// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:17: warning: all parameters should be named in a function
122+
// CHECK-FIXES-PLAIN-NAMES: Z &operator++(Z& /*unused*/) { return the_z; }
94123

95124
Z &operator++(Z&, int) { return the_z; }
96125
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: all parameters should be named in a function
97126
// CHECK-FIXES: Z &operator++(Z& /*unused*/, int) { return the_z; }
127+
// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:17: warning: all parameters should be named in a function
128+
// CHECK-FIXES-PLAIN-NAMES: Z &operator++(Z& /*unused*/, int) { return the_z; }
98129

99130
Z &operator--(Z&) { return the_z; }
100131
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: all parameters should be named in a function
101132
// CHECK-FIXES: Z &operator--(Z& /*unused*/) { return the_z; }
133+
// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:17: warning: all parameters should be named in a function
134+
// CHECK-FIXES-PLAIN-NAMES: Z &operator--(Z& /*unused*/) { return the_z; }
102135

103136
Z &operator--(Z&, int) { return the_z; }
104137
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: all parameters should be named in a function
105138
// CHECK-FIXES: Z &operator--(Z& /*unused*/, int) { return the_z; }
139+
// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:17: warning: all parameters should be named in a function
140+
// CHECK-FIXES-PLAIN-NAMES: Z &operator--(Z& /*unused*/, int) { return the_z; }
106141

107142
namespace testing {
108143
namespace internal {

0 commit comments

Comments
 (0)