[clang-tidy] Make the named parameter check only warn on declarations if a definition is visible.

Summary:
This allows us to copy the parameter name from the definition (as a comment)
or insert /*unused*/ in both places.

Reviewers: alexfh, klimek

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D4772

llvm-svn: 214701
This commit is contained in:
Benjamin Kramer 2014-08-04 09:33:58 +00:00
parent 2abde4f9d7
commit 610ba533d0
2 changed files with 56 additions and 32 deletions

View File

@ -37,6 +37,14 @@ void NamedParameterCheck::check(const MatchFinder::MatchResult &Result) {
if (Function->isImplicit())
return;
// Ignore declarations without a definition if we're not dealing with an
// overriden method.
const FunctionDecl *Definition = nullptr;
if (!Function->isDefined(Definition) &&
(!isa<CXXMethodDecl>(Function) ||
cast<CXXMethodDecl>(Function)->size_overridden_methods() == 0))
return;
// TODO: Handle overloads.
// TODO: We could check that all redeclarations use the same name for
// arguments in the same position.
@ -69,26 +77,35 @@ void NamedParameterCheck::check(const MatchFinder::MatchResult &Result) {
auto D = diag(FirstParm->getLocation(),
"all parameters should be named in a function");
// Fallback to an unused marker.
StringRef NewName = "unused";
for (auto P : UnnamedParams) {
// If the method is overridden, try to copy the name from the base method
// into the overrider.
const ParmVarDecl *Parm = P.first->getParamDecl(P.second);
const auto *M = dyn_cast<CXXMethodDecl>(P.first);
if (M && M->size_overridden_methods() > 0) {
const ParmVarDecl *OtherParm =
(*M->begin_overridden_methods())->getParamDecl(P.second);
std::string Name = OtherParm->getNameAsString();
if (!Name.empty()) {
D << FixItHint::CreateInsertion(Parm->getLocation(),
" /*" + Name + "*/");
continue;
}
StringRef Name = OtherParm->getName();
if (!Name.empty())
NewName = Name;
}
// Otherwise just insert an unused marker. Note that getLocation() points
// to the place where the name would be, this allows us to also get
// complex cases like function pointers right.
D << FixItHint::CreateInsertion(Parm->getLocation(), " /*unused*/");
// If the definition has a named parameter use that name.
if (Definition) {
const ParmVarDecl *DefParm = Definition->getParamDecl(P.second);
StringRef Name = DefParm->getName();
if (!Name.empty())
NewName = Name;
}
// Now insert the comment. 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() + "*/");
}
}
}

View File

@ -4,57 +4,57 @@
void Method(char *) { /* */ }
// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: all parameters should be named in a function
// CHECK-FIXES: void Method(char * /*unused*/) { /* */ }
void Method2(char *);
void Method2(char *) {}
// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: all parameters should be named in a function
// CHECK-FIXES: void Method2(char * /*unused*/);
void Method3(char *, void *);
// CHECK-FIXES: 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*/);
void Method4(char *, int /*unused*/);
// CHECK-FIXES: 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*/);
void operator delete[](void *) throw();
// CHECK-FIXES: 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();
int Method5(int);
// CHECK-FIXES: 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*/);
// CHECK-FIXES: 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 *)) {}
template <typename T> void Method7(T);
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-FIXES: template <typename T> void Method7(T /*unused*/) {}
// Don't warn in macros.
#define M void MethodM(int);
#define M void MethodM(int) {}
M
void operator delete(void *x) throw();
void operator delete(void *x) throw() {}
void Method7(char * /*x*/) {}
void Method8(char *x);
void Method8(char *x) {}
typedef void (*TypeM)(int x);
void operator delete[](void *x) throw();
void operator delete[](void * /*x*/) throw();
struct X {
X operator++(int);
X operator++(int) {}
// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: all parameters should be named in a function
// CHECK-FIXES: X operator++(int /*unused*/);
X operator--(int /*unused*/);
// CHECK-FIXES: X operator++(int /*unused*/) {}
X operator--(int /*unused*/) {}
const int &i;
};
void (*Func1)(void *);
void Func2(void (*func)(void *)) {}
template <void Func(void *)> void Func3();
template <void Func(void *)> void Func3() {}
template <typename T>
struct Y {
void foo(T);
void foo(T) {}
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: all parameters should be named in a function
// CHECK-FIXES: void foo(T /*unused*/);
// CHECK-FIXES: void foo(T /*unused*/) {}
};
Y<int> y;
@ -70,3 +70,10 @@ struct Derived : public Base {
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: all parameters should be named in a function
// CHECK-FIXES: 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*/);
void FDef(int n) {};
void FNoDef(int);