[clang-tidy] Add an option in 'readability-named-parameter' to print names without comment (#147953)

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 because the definition has name.
void func(int param);
void func(int param) { ... = param; }
```

Author: dmpolukhin

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

@@ -15,6 +15,17 @@ using namespace clang::ast_matchers;
 
 namespace clang::tidy::readability {
 
+NamedParameterCheck::NamedParameterCheck(StringRef Name,
+                                         ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      InsertPlainNamesInForwardDecls(
+          Options.get("InsertPlainNamesInForwardDecls", false)) {}
+
+void NamedParameterCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "InsertPlainNamesInForwardDecls",
+                InsertPlainNamesInForwardDecls);
+}
+
 void NamedParameterCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
   Finder->addMatcher(functionDecl().bind("decl"), this);
 }
@@ -84,7 +95,8 @@ void NamedParameterCheck::check(const MatchFinder::MatchResult &Result) {
 
     for (auto P : UnnamedParams) {
       // Fallback to an unused marker.
-      StringRef NewName = "unused";
+      static constexpr StringRef FallbackName = "unused";
+      StringRef NewName = FallbackName;
 
       // If the method is overridden, try to copy the name from the base method
       // into the overrider.
@@ -105,12 +117,25 @@ void NamedParameterCheck::check(const MatchFinder::MatchResult &Result) {
           NewName = Name;
       }
 
-      // Now insert the comment. Note that getLocation() points to the place
+      // Now insert the fix. Note that getLocation() points to the place
       // where the name would be, this allows us to also get complex cases like
       // function pointers right.
       const ParmVarDecl *Parm = P.first->getParamDecl(P.second);
-      D << FixItHint::CreateInsertion(Parm->getLocation(),
-                                      " /*" + NewName.str() + "*/");
+
+      // The fix depends on the InsertPlainNamesInForwardDecls option,
+      // whether this is a forward declaration and whether the parameter has
+      // a real name.
+      const bool IsForwardDeclaration = (!Definition || Function != Definition);
+      if (InsertPlainNamesInForwardDecls && IsForwardDeclaration &&
+          NewName != FallbackName) {
+        // For forward declarations with InsertPlainNamesInForwardDecls enabled,
+        // insert the parameter name without comments.
+        D << FixItHint::CreateInsertion(Parm->getLocation(),
+                                        " " + NewName.str());
+      } else {
+        D << FixItHint::CreateInsertion(Parm->getLocation(),
+                                        " /*" + NewName.str() + "*/");
+      }
     }
   }
 }

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

@@ -26,13 +26,16 @@ namespace clang::tidy::readability {
 /// Corresponding cpplint.py check name: 'readability/function'.
 class NamedParameterCheck : public ClangTidyCheck {
 public:
-  NamedParameterCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  NamedParameterCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   std::optional<TraversalKind> getCheckTraversalKind() const override {
     return TK_IgnoreUnlessSpelledInSource;
   }
+
+private:
+  const bool InsertPlainNamesInForwardDecls;
 };
 
 } // namespace clang::tidy::readability

clang-tools-extra/docs/ReleaseNotes.rst

@@ -215,7 +215,7 @@ Changes in existing checks
 - Improved :doc:`cppcoreguidelines-missing-std-forward
   <clang-tidy/checks/cppcoreguidelines/missing-std-forward>` check by adding a
   flag to specify the function used for forwarding instead of ``std::forward``.
-   
+
 - Improved :doc:`cppcoreguidelines-pro-bounds-pointer-arithmetic
   <clang-tidy/checks/cppcoreguidelines/pro-bounds-pointer-arithmetic>` check by
   fixing false positives when calling indexing operators that do not perform
@@ -342,6 +342,11 @@ Changes in existing checks
   false negatives where math expressions are the operand of assignment operators
   or comparison operators.
 
+- Improved :doc:`readability-named-parameter
+  <clang-tidy/checks/readability/named-parameter>` check by adding the option
+  `InsertPlainNamesInForwardDecls` to insert parameter names without comments
+  for forward declarations only.
+
 - Improved :doc:`readability-qualified-auto
   <clang-tidy/checks/readability/qualified-auto>` check by adding the option
   `AllowedTypes`, that excludes specified types from adding qualifiers.

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

@@ -23,3 +23,12 @@ If a parameter is not utilized, its name can be commented out in a function defi
     }
 
 Corresponding cpplint.py check name: `readability/function`.
+
+Options
+-------
+
+.. option:: InsertPlainNamesInForwardDecls
+
+   If set to `true`, the check will insert parameter names without comments for
+   forward declarations only. Otherwise, the check will insert parameter names
+   as comments (e.g., ``/*param*/``). Default is `false`.

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

@@ -1,29 +1,47 @@
 // RUN: %check_clang_tidy %s readability-named-parameter %t
+// RUN: %check_clang_tidy -check-suffix=PLAIN-NAMES %s readability-named-parameter %t -- \
+// RUN:   -config="{CheckOptions: [{key: readability-named-parameter.InsertPlainNamesInForwardDecls, value: true}]}"
 
 void Method(char *) { /* */ }
 // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: all parameters should be named in a function
 // CHECK-FIXES: void Method(char * /*unused*/) { /* */ }
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:19: warning: all parameters should be named in a function
+// CHECK-FIXES-PLAIN-NAMES: void Method(char * /*unused*/) { /* */ }
 void Method2(char *) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: all parameters should be named in a function
 // CHECK-FIXES: void Method2(char * /*unused*/) {}
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:20: warning: all parameters should be named in a function
+// CHECK-FIXES-PLAIN-NAMES: void Method2(char * /*unused*/) {}
 void Method3(char *, void *) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: all parameters should be named in a function
 // CHECK-FIXES: void Method3(char * /*unused*/, void * /*unused*/) {}
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:20: warning: all parameters should be named in a function
+// CHECK-FIXES-PLAIN-NAMES: void Method3(char * /*unused*/, void * /*unused*/) {}
 void Method4(char *, int /*unused*/) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: all parameters should be named in a function
 // CHECK-FIXES: void Method4(char * /*unused*/, int /*unused*/) {}
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:20: warning: all parameters should be named in a function
+// CHECK-FIXES-PLAIN-NAMES: void Method4(char * /*unused*/, int /*unused*/) {}
 void operator delete[](void *) throw() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: all parameters should be named in a function
 // CHECK-FIXES: void operator delete[](void * /*unused*/) throw() {}
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:30: warning: all parameters should be named in a function
+// CHECK-FIXES-PLAIN-NAMES: void operator delete[](void * /*unused*/) throw() {}
 int Method5(int) { return 0; }
 // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: all parameters should be named in a function
 // CHECK-FIXES: int Method5(int /*unused*/) { return 0; }
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:16: warning: all parameters should be named in a function
+// CHECK-FIXES-PLAIN-NAMES: int Method5(int /*unused*/) { return 0; }
 void Method6(void (*)(void *)) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: all parameters should be named in a function
 // CHECK-FIXES: void Method6(void (* /*unused*/)(void *)) {}
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:21: warning: all parameters should be named in a function
+// CHECK-FIXES-PLAIN-NAMES: void Method6(void (* /*unused*/)(void *)) {}
 template <typename T> void Method7(T) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: all parameters should be named in a function
 // CHECK-FIXES: template <typename T> void Method7(T /*unused*/) {}
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:37: warning: all parameters should be named in a function
+// CHECK-FIXES-PLAIN-NAMES: template <typename T> void Method7(T /*unused*/) {}
 
 // Don't warn in macros.
 #define M void MethodM(int) {}
@@ -55,6 +73,8 @@ struct Y {
   void foo(T) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: all parameters should be named in a function
 // CHECK-FIXES: void foo(T /*unused*/) {}
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:13: warning: all parameters should be named in a function
+// CHECK-FIXES-PLAIN-NAMES: void foo(T /*unused*/) {}
 };
 
 Y<int> y;
@@ -69,19 +89,27 @@ struct Derived : public Base {
   void foo(int);
 // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: all parameters should be named in a function
 // CHECK-FIXES: void foo(int /*argname*/);
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:15: warning: all parameters should be named in a function
+// CHECK-FIXES-PLAIN-NAMES: void foo(int argname);
 };
 
 void FDef(int);
 // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: all parameters should be named in a function
 // CHECK-FIXES: void FDef(int /*n*/);
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:14: warning: all parameters should be named in a function
+// CHECK-FIXES-PLAIN-NAMES: void FDef(int n);
 void FDef(int n) {}
 
 void FDef2(int, int);
 // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: all parameters should be named in a function
 // CHECK-FIXES: void FDef2(int /*n*/, int /*unused*/);
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:15: warning: all parameters should be named in a function
+// CHECK-FIXES-PLAIN-NAMES: void FDef2(int n, int /*unused*/);
 void FDef2(int n, int) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: all parameters should be named in a function
 // CHECK-FIXES: void FDef2(int n, int /*unused*/) {}
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:22: warning: all parameters should be named in a function
+// CHECK-FIXES-PLAIN-NAMES: void FDef2(int n, int /*unused*/) {}
 
 void FNoDef(int);
 
@@ -91,18 +119,26 @@ Z the_z;
 Z &operator++(Z&) { return the_z; }
 // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: all parameters should be named in a function
 // CHECK-FIXES: Z &operator++(Z& /*unused*/) { return the_z; }
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:17: warning: all parameters should be named in a function
+// CHECK-FIXES-PLAIN-NAMES: Z &operator++(Z& /*unused*/) { return the_z; }
 
 Z &operator++(Z&, int) { return the_z; }
 // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: all parameters should be named in a function
 // CHECK-FIXES: Z &operator++(Z& /*unused*/, int) { return the_z; }
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:17: warning: all parameters should be named in a function
+// CHECK-FIXES-PLAIN-NAMES: Z &operator++(Z& /*unused*/, int) { return the_z; }
 
 Z &operator--(Z&) { return the_z; }
 // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: all parameters should be named in a function
 // CHECK-FIXES: Z &operator--(Z& /*unused*/) { return the_z; }
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:17: warning: all parameters should be named in a function
+// CHECK-FIXES-PLAIN-NAMES: Z &operator--(Z& /*unused*/) { return the_z; }
 
 Z &operator--(Z&, int) { return the_z; }
 // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: all parameters should be named in a function
 // CHECK-FIXES: Z &operator--(Z& /*unused*/, int) { return the_z; }
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:17: warning: all parameters should be named in a function
+// CHECK-FIXES-PLAIN-NAMES: Z &operator--(Z& /*unused*/, int) { return the_z; }
 
 namespace testing {
 namespace internal {