[clang-tidy] fixed misc-unused-parameters omitting parameters default value

[PriMee]

Summary

Bug: https://bugs.llvm.org/show_bug.cgi?id=34450

Problem:

Clang-tidy check misc-unused-parameters omits parameter default value what results in its complete removal. Compilation errors might occur after clang-tidy fix.

Patch description:

Changed removal range. The range should end after parameter declarator, not after whole parameter declaration (which might contain a default value).

[alexfh]

Thank you for the fix!

LG with one nit.

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

73 ↗

(On Diff #114165)

Please add an invocation of staticFunctionF without parameters.

[PriMee]

Done :) Could you please commit this for me?

[alexfh]

In D37566#866096, @PriMee wrote:

Done :) Could you please commit this for me?

Sure, just committed the patch.

clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp

if (!Indexer) {
     Indexer = llvm::make_unique<IndexerVisitor>(
         Result.Context->getTranslationUnitDecl());
   }
 
   // Comment out parameter name for non-local functions.
   if (Function->isExternallyVisible() ||
       !Result.SourceManager->isInMainFile(Function->getLocation()) ||
       !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) {
-    SourceRange RemovalRange(Param->getLocation(), Param->getLocEnd());
+    SourceRange RemovalRange(Param->getLocation(),
+                             Param->DeclaratorDecl::getSourceRange().getEnd());
     // Note: We always add a space before the '/*' to not accidentally create a
     // '*/*' for pointer types, which doesn't start a comment. clang-format will
     // clean this up afterwards.
     MyDiag << FixItHint::CreateReplacement(
         RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
     return;
   }
 

clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp

 // RUN: echo "static void staticFunctionHeader(int i) {}" > %T/header.h
 // RUN: echo "static void staticFunctionHeader(int  /*i*/) {}" > %T/header-fixed.h
 // RUN: %check_clang_tidy %s misc-unused-parameters %t -- -header-filter='.*' -- -std=c++11 -fno-delayed-template-parsing
 // RUN: diff %T/header.h %T/header-fixed.h
 
 #include "header.h"
 // CHECK-MESSAGES: header.h:1:38: warning
 
 // Basic removal
 // =============
 void a(int i) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: parameter 'i' is unused [misc-unused-parameters]
 // CHECK-FIXES: {{^}}void a(int  /*i*/) {}{{$}}
 
 void b(int i = 1) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: parameter 'i' is unused [misc-unused-parameters]
-// CHECK-FIXES: {{^}}void b(int  /*i*/) {}{{$}}
+// CHECK-FIXES: {{^}}void b(int  /*i*/ = 1) {}{{$}}
 
 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 f(int i); // Don't remove stuff in declarations
+void g(int i = 1);
 void h(int i) { (void)i; } // Don't remove used parameters
 
 bool useLambda(int (*fn)(int));
 static bool static_var = useLambda([] (int a) { return a; });
 
 // Remove parameters of local functions
 // ====================================
 static void staticFunctionA(int i);

 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 staticFunctionF(int i = 4);
+// CHECK-FIXES: {{^}}static void staticFunctionF();
+static void staticFunctionF(int i) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:33: warning
+// CHECK-FIXES: {{^}}static void staticFunctionF()
 
 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();
+  staticFunctionE(1);
+// CHECK-FIXES: staticFunctionE();
+  staticFunctionF(1);
+// CHECK-FIXES: staticFunctionF();
+  staticFunctionF();
+// CHECK-FIXES: staticFunctionF();
 }
 
 /*
  * FIXME: This fails because the removals overlap and ClangTidy doesn't apply
  *        them.
  * static void bothVarsUnused(int a, int b) {}
  */
 
// CHECK-FIXES: variableWithLongName1(LongName1);
   variableWithLongName2(LongName1, LongName2);
 // CHECK-FIXES: variableWithLongName2(LongName2);
 }
 
 class SomeClass {
   static void f(int i) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:21: warning
 // CHECK-FIXES: static void f(int  /*i*/) {}
+  static void g(int i = 1) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning
+// CHECK-FIXES: static void g(int  /*i*/ = 1) {}
 };
 
 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 s(int i = 1) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning
+// CHECK-FIXES: void s(int  /*i*/ = 1) {}
 };
 
 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);
+  useFunction(&C::s);;
 }
 
 class Base {
   virtual void f(int i);
 };
 
 class Derived : public Base {
   void f(int i) override {}