Extend misc-unused-parameters to delete parameters of static functions

[djasper]

Summary

Also see: llvm.org/PR24180.

[chandlerc]

Feel free to address the comments below in a follow-up patch. This seems a fine next step, LGTM

clang-tidy/misc/UnusedParametersCheck.cpp

76

Hmm, I thought of a reason to not delete parameters from local functions -- if they're used in some way other than calling them such as using them as an argument to a template.

Maybe check for non-CallExpr DeclRefExprs of the function, and fall back to the commenting strategy?

test/clang-tidy/misc-unused-parameters.cpp

23–24

Does this already handle function in anonymous namespaces? Want to add those as a follow-up? The next obvious chunk is to handle non-virtual methods of classes in anonymous namespaces. Perhaps those will be handled automatically though.

[djasper]

Addressed both comments and submitted as r242659.

clang-tidy/misc/UnusedParametersCheck.cpp

 namespace tidy {
 
 void UnusedParametersCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
       parmVarDecl(hasAncestor(functionDecl().bind("function"))).bind("x"),
       this);
 }
 
+static FixItHint removeParameter(const FunctionDecl *Function, unsigned Index) {
+  const ParmVarDecl *Param = Function->getParamDecl(Index);
+  unsigned ParamCount = Function->getNumParams();
+  SourceRange RemovalRange = Param->getSourceRange();
+  if (ParamCount == 1)
+    return FixItHint::CreateRemoval(RemovalRange);
+
+  if (Index == 0)
+    RemovalRange.setEnd(
+        Function->getParamDecl(Index + 1)->getLocStart().getLocWithOffset(-1));
+  else
+    RemovalRange.setBegin(
+        Function->getParamDecl(Index - 1)->getLocEnd().getLocWithOffset(1));
+
+  return FixItHint::CreateRemoval(RemovalRange);
+}
+
+static FixItHint removeArgument(const CallExpr *Call, unsigned Index) {
+  unsigned ArgCount = Call->getNumArgs();
+  const Expr *Arg = Call->getArg(Index);
+  SourceRange RemovalRange = Arg->getSourceRange();
+  if (ArgCount == 1)
+    return FixItHint::CreateRemoval(RemovalRange);
+  if (Index == 0)
+    RemovalRange.setEnd(
+        Call->getArg(Index + 1)->getLocStart().getLocWithOffset(-1));
+  else
+    RemovalRange.setBegin(
+        Call->getArg(Index - 1)->getLocEnd().getLocWithOffset(1));
+  return FixItHint::CreateRemoval(RemovalRange);
+}
+
 void UnusedParametersCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *Function = Result.Nodes.getNodeAs<FunctionDecl>("function");
   if (!Function->doesThisDeclarationHaveABody())
     return;
   const auto *Param = Result.Nodes.getNodeAs<ParmVarDecl>("x");
   if (Param->isUsed())
     return;
 
   auto MyDiag = diag(Param->getLocation(), "parameter '%0' is unused")
                 << Param->getName();
 
+  // Comment out parameter name for non-local functions.
+  if (Function->getStorageClass() != StorageClass::SC_Static) {
     SourceRange RemovalRange(Param->getLocation(), Param->getLocEnd());
     MyDiag << FixItHint::CreateReplacement(
         RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
+    return;
+  }
+
+  // Handle local functions by deleting the parameters.
+  unsigned ParamIndex = Param->getFunctionScopeIndex();
+  // Fix all redeclarations.
+  for (const FunctionDecl *FD : Function->redecls())
+    MyDiag << removeParameter(FD, ParamIndex);
+
+  // Fix all call sites.
+  auto CallMatches = ast_matchers::match(
+      decl(forEachDescendant(
+          callExpr(callee(functionDecl(equalsNode(Function)))).bind("x"))),
+      *Result.Context->getTranslationUnitDecl(), *Result.Context);
+  for (const auto &Match : CallMatches)
+    MyDiag << removeArgument(Match.getNodeAs<CallExpr>("x"), ParamIndex);
 }
 
 } // namespace tidy
 } // namespace clang
 

test/clang-tidy/misc-unused-parameters.cpp

 void c(int *i) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: parameter 'i' is unused [misc-unused-parameters]
 // CHECK-FIXES: {{^}}void c(int * /*i*/) {}{{$}}
 
 // Unchanged cases
 // ===============
 void g(int i);             // Don't remove stuff in declarations
 void h(int i) { (void)i; } // Don't remove used parameters
+
+// Remove parameters of local functions
+// ====================================
+static void staticFunctionA(int i);
+// CHECK-FIXES: {{^}}static void staticFunctionA();
+static void staticFunctionA(int i) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:33: warning
+// CHECK-FIXES: {{^}}static void staticFunctionA()
+
+static void staticFunctionB(int i, int j) { (void)i; }
+// CHECK-MESSAGES: :[[@LINE-1]]:40: warning
+// CHECK-FIXES: {{^}}static void staticFunctionB(int i)
+
+static void staticFunctionC(int i, int j) { (void)j; }
+// CHECK-MESSAGES: :[[@LINE-1]]:33: warning
+// CHECK-FIXES: {{^}}static void staticFunctionC( int j)
+
+static void staticFunctionD(int i, int j, int k) { (void)i; (void)k; }
+// CHECK-MESSAGES: :[[@LINE-1]]:40: warning
+// CHECK-FIXES: {{^}}static void staticFunctionD(int i, int k)
+
+static void staticFunctionE(int i = 4) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:33: warning
+// CHECK-FIXES: {{^}}static void staticFunctionE()
+
+
+static void someCallSites() {
+  staticFunctionA(1);
+// CHECK-FIXES: {{^}}  staticFunctionA();
+  staticFunctionB(1, 2);
+// CHECK-FIXES: {{^}}  staticFunctionB(1);
+  staticFunctionC(1, 2);
+// CHECK-FIXES: {{^}}  staticFunctionC( 2);
+  staticFunctionD(1, 2, 3);
+// CHECK-FIXES: {{^}}  staticFunctionD(1, 3);
+  staticFunctionE();
+}