Extend misc-unused-parameters to delete parameters of local functions.

Also see: llvm.org/PR24180.

llvm-svn: 242659

2 files changed, 134 insertions, 3 deletions

diff --git a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
index 170a274d70f..bfc213efcca 100644
--- a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -22,6 +22,38 @@ void UnusedParametersCheck::registerMatchers(MatchFinder *Finder) {
       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())
@@ -33,9 +65,39 @@ void UnusedParametersCheck::check(const MatchFinder::MatchResult &Result) {
   auto MyDiag = diag(Param->getLocation(), "parameter '%0' is unused")
                 << Param->getName();
 
-  SourceRange RemovalRange(Param->getLocation(), Param->getLocEnd());
-  MyDiag << FixItHint::CreateReplacement(
-      RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
+  auto UsedByRef = [&] {
+    return !ast_matchers::match(
+                decl(hasDescendant(
+                    declRefExpr(to(equalsNode(Function)),
+                                unless(hasAncestor(
+                                    callExpr(callee(equalsNode(Function)))))))),
+                *Result.Context->getTranslationUnitDecl(), *Result.Context)
+                .empty();
+  };
+
+  // Comment out parameter name for non-local functions.
+  if ((Function->isExternallyVisible() &&
+       Function->getStorageClass() != StorageClass::SC_Static) ||
+      UsedByRef()) {
+    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
diff --git a/clang-tools-extra/test/clang-tidy/misc-unused-parameters.cpp b/clang-tools-extra/test/clang-tidy/misc-unused-parameters.cpp
index 70d1c8c0913..3b54cc5a3bf 100644
--- a/clang-tools-extra/test/clang-tidy/misc-unused-parameters.cpp
+++ b/clang-tools-extra/test/clang-tidy/misc-unused-parameters.cpp
@@ -19,3 +19,72 @@ void c(int *i) {}
 // ===============
 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();
+}
+
+namespace {
+class C {
+public:
+  void f(int i);
+// CHECK-FIXES: void f();
+  void g(int i) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning
+// CHECK-FIXES: void g() {}
+  void h(int i) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning
+// CHECK-FIXES: void h(int  /*i*/) {}
+};
+
+void C::f(int i) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning
+// CHECK-FIXES: void C::f() {}
+
+template <typename T>
+void useFunction(T t);
+
+void someMoreCallSites() {
+  C c;
+  c.f(1);
+// CHECK-FIXES: c.f();
+  c.g(1);
+// CHECK-FIXES: c.g();
+
+  useFunction(&C::h);
+}
+
+} // end namespace