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

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 {