Index: clang-tidy/google/CMakeLists.txt =================================================================== --- clang-tidy/google/CMakeLists.txt +++ clang-tidy/google/CMakeLists.txt @@ -8,7 +8,6 @@ GoogleTidyModule.cpp IntegerTypesCheck.cpp MemsetZeroLengthCheck.cpp - NamedParameterCheck.cpp OverloadedUnaryAndCheck.cpp StringReferenceMemberCheck.cpp TodoCommentCheck.cpp Index: clang-tidy/google/GoogleTidyModule.cpp =================================================================== --- clang-tidy/google/GoogleTidyModule.cpp +++ clang-tidy/google/GoogleTidyModule.cpp @@ -20,7 +20,6 @@ #include "GlobalNamesInHeadersCheck.h" #include "IntegerTypesCheck.h" #include "MemsetZeroLengthCheck.h" -#include "NamedParameterCheck.h" #include "OverloadedUnaryAndCheck.h" #include "StringReferenceMemberCheck.h" #include "TodoCommentCheck.h" @@ -54,8 +53,6 @@ "google-runtime-memset"); CheckFactories.registerCheck( "google-readability-casting"); - CheckFactories.registerCheck( - "google-readability-function"); CheckFactories.registerCheck( "google-readability-todo"); CheckFactories Index: clang-tidy/google/NamedParameterCheck.h =================================================================== --- clang-tidy/google/NamedParameterCheck.h +++ clang-tidy/google/NamedParameterCheck.h @@ -1,37 +0,0 @@ -//===--- 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_NAMEDPARAMETERCHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_NAMEDPARAMETERCHECK_H - -#include "../ClangTidy.h" - -namespace clang { -namespace tidy { -namespace google { -namespace readability { - -/// \brief Find functions with unnamed arguments. -/// -/// http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Function_Declarations_and_Definitions#Function_Declarations_and_Definitions -/// Corresponding cpplint.py check name: 'readability/function'. -class NamedParameterCheck : public ClangTidyCheck { -public: - NamedParameterCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} - void registerMatchers(ast_matchers::MatchFinder *Finder) override; - void check(const ast_matchers::MatchFinder::MatchResult &Result) override; -}; - -} // namespace readability -} // namespace google -} // namespace tidy -} // namespace clang - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_NAMEDPARAMETERCHECK_H Index: clang-tidy/google/NamedParameterCheck.cpp =================================================================== --- clang-tidy/google/NamedParameterCheck.cpp +++ clang-tidy/google/NamedParameterCheck.cpp @@ -1,127 +0,0 @@ -//===--- 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/AST/ASTContext.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/ASTMatchers/ASTMatchers.h" - -using namespace clang::ast_matchers; - -namespace clang { -namespace tidy { -namespace google { -namespace readability { - -void NamedParameterCheck::registerMatchers(ast_matchers::MatchFinder *Finder) { - Finder->addMatcher(functionDecl(unless(isInstantiated())).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; - - // Ignore declarations without a definition if we're not dealing with an - // overriden method. - const FunctionDecl *Definition = nullptr; - if ((!Function->isDefined(Definition) || Function->isDefaulted() || - Function->isDeleted()) && - (!isa(Function) || - cast(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. - 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; - - // Don't warn on the dummy argument on post-inc and post-dec operators. - if ((Function->getOverloadedOperator() == OO_PlusPlus || - Function->getOverloadedOperator() == OO_MinusMinus) && - Parm->getType()->isSpecificBuiltinType(BuiltinType::Int)) - continue; - - // Sanity check the source locations. - if (!Parm->getLocation().isValid() || Parm->getLocation().isMacroID() || - !SM.isWrittenInSameFile(Parm->getLocStart(), Parm->getLocation())) - continue; - - // Skip gmock testing::Unused parameters. - if (auto Typedef = Parm->getType()->getAs()) - if (Typedef->getDecl()->getQualifiedNameAsString() == "testing::Unused") - continue; - - // Skip std::nullptr_t. - if (Parm->getType().getCanonicalType()->isNullPtrType()) - 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) { - // Fallback to an unused marker. - StringRef NewName = "unused"; - - // If the method is overridden, try to copy the name from the base method - // into the overrider. - const auto *M = dyn_cast(P.first); - if (M && M->size_overridden_methods() > 0) { - const ParmVarDecl *OtherParm = - (*M->begin_overridden_methods())->getParamDecl(P.second); - StringRef Name = OtherParm->getName(); - if (!Name.empty()) - NewName = Name; - } - - // 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() + "*/"); - } - } -} - -} // namespace readability -} // namespace google -} // namespace tidy -} // namespace clang Index: clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tidy/readability/CMakeLists.txt +++ clang-tidy/readability/CMakeLists.txt @@ -5,6 +5,7 @@ ContainerSizeEmptyCheck.cpp ElseAfterReturnCheck.cpp FunctionSizeCheck.cpp + NamedParameterCheck.cpp NamespaceCommentCheck.cpp ReadabilityTidyModule.cpp RedundantStringCStrCheck.cpp Index: clang-tidy/readability/NamedParameterCheck.h =================================================================== --- clang-tidy/readability/NamedParameterCheck.h +++ clang-tidy/readability/NamedParameterCheck.h @@ -7,19 +7,24 @@ // //===----------------------------------------------------------------------===// -#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_NAMEDPARAMETERCHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_NAMEDPARAMETERCHECK_H +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_NAMEDPARAMETERCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_NAMEDPARAMETERCHECK_H #include "../ClangTidy.h" namespace clang { namespace tidy { -namespace google { namespace readability { /// \brief Find functions with unnamed arguments. /// +/// The check implements the following rule originating in the Google C++ Style +/// Guide: /// http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Function_Declarations_and_Definitions#Function_Declarations_and_Definitions +/// +/// All parameters should be named, with identical names in the declaration and +/// implementation. +/// /// Corresponding cpplint.py check name: 'readability/function'. class NamedParameterCheck : public ClangTidyCheck { public: @@ -30,8 +35,7 @@ }; } // namespace readability -} // namespace google } // namespace tidy } // namespace clang -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_NAMEDPARAMETERCHECK_H +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_NAMEDPARAMETERCHECK_H Index: clang-tidy/readability/NamedParameterCheck.cpp =================================================================== --- clang-tidy/readability/NamedParameterCheck.cpp +++ clang-tidy/readability/NamedParameterCheck.cpp @@ -16,7 +16,6 @@ namespace clang { namespace tidy { -namespace google { namespace readability { void NamedParameterCheck::registerMatchers(ast_matchers::MatchFinder *Finder) { @@ -122,6 +121,5 @@ } } // namespace readability -} // namespace google } // namespace tidy } // namespace clang Index: clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tidy/readability/ReadabilityTidyModule.cpp +++ clang-tidy/readability/ReadabilityTidyModule.cpp @@ -14,6 +14,7 @@ #include "ContainerSizeEmptyCheck.h" #include "ElseAfterReturnCheck.h" #include "FunctionSizeCheck.h" +#include "NamedParameterCheck.h" #include "RedundantSmartptrGetCheck.h" #include "RedundantStringCStrCheck.h" #include "ShrinkToFitCheck.h" @@ -33,6 +34,8 @@ "readability-else-after-return"); CheckFactories.registerCheck( "readability-function-size"); + CheckFactories.registerCheck( + "readability-named-parameter"); CheckFactories.registerCheck( "readability-redundant-smartptr-get"); CheckFactories.registerCheck( Index: test/clang-tidy/google-readability-function.cpp =================================================================== --- test/clang-tidy/google-readability-function.cpp +++ test/clang-tidy/google-readability-function.cpp @@ -1,130 +0,0 @@ -// RUN: $(dirname %s)/check_clang_tidy.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) { return 0; } -// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: all parameters should be named in a function -// 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 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) {} - X operator--(int) {} - - X(X&) = delete; - X &operator=(X&) = default; - - 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*/); -}; - -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 FDef2(int, int); -// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: all parameters should be named in a function -// CHECK-FIXES: void FDef2(int /*n*/, int /*unused*/); -void FDef2(int n, int) {} -// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: all parameters should be named in a function -// CHECK-FIXES: void FDef2(int n, int /*unused*/) {} - -void FNoDef(int); - -class Z {}; - -Z &operator++(Z&) {} -// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: all parameters should be named in a function -// CHECK-FIXES: Z &operator++(Z& /*unused*/) {} - -Z &operator++(Z&, int) {} -// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: all parameters should be named in a function -// CHECK-FIXES: Z &operator++(Z& /*unused*/, int) {} - -Z &operator--(Z&) {} -// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: all parameters should be named in a function -// CHECK-FIXES: Z &operator--(Z& /*unused*/) {} - -Z &operator--(Z&, int) {} -// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: all parameters should be named in a function -// CHECK-FIXES: Z &operator--(Z& /*unused*/, int) {} - -namespace testing { -namespace internal { -class IgnoredValue { - public: - template - IgnoredValue(const T& /* ignored */) {} -}; -} -typedef internal::IgnoredValue Unused; -} - -using ::testing::Unused; - -void MockFunction(Unused, int q, Unused) { - ++q; - ++q; - ++q; -} - -namespace std { -typedef decltype(nullptr) nullptr_t; -} - -void f(std::nullptr_t) {} Index: test/clang-tidy/readability-named-parameter.cpp =================================================================== --- test/clang-tidy/readability-named-parameter.cpp +++ test/clang-tidy/readability-named-parameter.cpp @@ -1,4 +1,4 @@ -// RUN: $(dirname %s)/check_clang_tidy.sh %s google-readability-function %t +// RUN: $(dirname %s)/check_clang_tidy.sh %s readability-named-parameter %t // REQUIRES: shell void Method(char *) { /* */ }