Index: clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt +++ clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt @@ -5,6 +5,7 @@ ExplicitConstructorCheck.cpp ExplicitMakePairCheck.cpp GoogleTidyModule.cpp + NamedParameterCheck.cpp OverloadedUnaryAndCheck.cpp LINK_LIBS Index: clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp +++ clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp @@ -13,6 +13,7 @@ #include "AvoidCStyleCastsCheck.h" #include "ExplicitConstructorCheck.h" #include "ExplicitMakePairCheck.h" +#include "NamedParameterCheck.h" #include "OverloadedUnaryAndCheck.h" using namespace clang::ast_matchers; @@ -35,6 +36,9 @@ CheckFactories.addCheckFactory( "google-readability-casting", new ClangTidyCheckFactory()); + CheckFactories.addCheckFactory( + "google-readability-function", + new ClangTidyCheckFactory()); } }; Index: clang-tools-extra/trunk/clang-tidy/google/NamedParameterCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/google/NamedParameterCheck.h +++ clang-tools-extra/trunk/clang-tidy/google/NamedParameterCheck.h @@ -0,0 +1,32 @@ +//===--- NamedParameterCheck.h - clang-tidy ---------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_NAMED_PARAMETER_CHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_NAMED_PARAMETER_CHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace readability { + +/// \brief Find functions with unnamed arguments. +/// +/// Corresponding cpplint.py check name: 'readability/function'. +class NamedParameterCheck : public ClangTidyCheck { +public: + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace readability +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_NAMED_PARAMETER_CHECK_H Index: clang-tools-extra/trunk/clang-tidy/google/NamedParameterCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/google/NamedParameterCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/google/NamedParameterCheck.cpp @@ -0,0 +1,98 @@ +//===--- NamedParameterCheck.cpp - clang-tidy -------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "NamedParameterCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/AST/ASTContext.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace readability { + +void NamedParameterCheck::registerMatchers(ast_matchers::MatchFinder *Finder) { + Finder->addMatcher( + functionDecl( + unless(hasAncestor(decl( + anyOf(recordDecl(ast_matchers::isTemplateInstantiation()), + functionDecl(ast_matchers::isTemplateInstantiation())))))) + .bind("decl"), + this); +} + +void NamedParameterCheck::check(const MatchFinder::MatchResult &Result) { + const SourceManager &SM = *Result.SourceManager; + const auto *Function = Result.Nodes.getNodeAs("decl"); + SmallVector, 4> UnnamedParams; + + // Ignore implicitly generated members. + if (Function->isImplicit()) + return; + + // TODO: Handle overloads. + // TODO: We could check that all redeclarations use the same name for + // arguments in the same position. + for (unsigned I = 0, E = Function->getNumParams(); I != E; ++I) { + const ParmVarDecl *Parm = Function->getParamDecl(I); + // Look for unnamed parameters. + if (!Parm->getName().empty()) + continue; + + // Sanity check the source locations. + if (!Parm->getLocation().isValid() || Parm->getLocation().isMacroID() || + !SM.isWrittenInSameFile(Parm->getLocStart(), Parm->getLocation())) + continue; + + // Look for comments. We explicitly want to allow idioms like + // void foo(int /*unused*/) + const char *Begin = SM.getCharacterData(Parm->getLocStart()); + const char *End = SM.getCharacterData(Parm->getLocation()); + StringRef Data(Begin, End - Begin); + if (Data.find("/*") != StringRef::npos) + continue; + + UnnamedParams.push_back(std::make_pair(Function, I)); + } + + // Emit only one warning per function but fixits for all unnamed parameters. + if (!UnnamedParams.empty()) { + const ParmVarDecl *FirstParm = + UnnamedParams.front().first->getParamDecl(UnnamedParams.front().second); + auto D = diag(FirstParm->getLocation(), + "all parameters should be named in a function"); + + 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(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; + } + } + + // 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*/"); + } + } +} + +} // namespace readability +} // namespace tidy +} // namespace clang Index: clang-tools-extra/trunk/test/clang-tidy/google-readability-function.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/google-readability-function.cpp +++ clang-tools-extra/trunk/test/clang-tidy/google-readability-function.cpp @@ -0,0 +1,72 @@ +// RUN: $(dirname %s)/check_clang_tidy_fix.sh %s google-readability-function %t +// REQUIRES: shell + +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 *); +// 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-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-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-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-MESSAGES: :[[@LINE-1]]:16: warning: all parameters should be named in a function +// CHECK-FIXES: int Method5(int /*unused*/); +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 void Method7(T); +// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: all parameters should be named in a function +// CHECK-FIXES: template void Method7(T /*unused*/); + +// Don't warn in macros. +#define M void MethodM(int); +M + +void operator delete(void *x) throw(); +void Method7(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); +// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: all parameters should be named in a function +// CHECK-FIXES: X operator++(int /*unused*/); + X operator--(int /*unused*/); + + const int &i; +}; + +void (*Func1)(void *); +void Func2(void (*func)(void *)) {} +template void Func3(); + +template +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*/); +}; + +Y y; +Y z; + +struct Base { + virtual void foo(bool notThisOne); + virtual void foo(int argname); +}; + +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*/); +};