Index: clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tidy/readability/CMakeLists.txt +++ clang-tidy/readability/CMakeLists.txt @@ -6,6 +6,7 @@ ElseAfterReturnCheck.cpp FunctionSizeCheck.cpp IdentifierNamingCheck.cpp + InconsistentDeclarationParameterNameCheck.cpp NamedParameterCheck.cpp NamespaceCommentCheck.cpp ReadabilityTidyModule.cpp Index: clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h =================================================================== --- clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h +++ clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h @@ -0,0 +1,44 @@ +//===--- InconsistentDeclarationParameterNameCheck.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_READABILITY_INCONSISTENT_DECLARATION_PARAMETER_NAME_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_INCONSISTENT_DECLARATION_PARAMETER_NAME_H + +#include "../ClangTidy.h" + +#include "llvm/ADT/DenseSet.h" + +namespace clang { +namespace tidy { +namespace readability { + +/// \brief Checks for declarations of functions which differ in parameter names +/// +/// Detailed documentation is provided in HTML files, see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.html +/// +class InconsistentDeclarationParameterNameCheck : public ClangTidyCheck { +public: + InconsistentDeclarationParameterNameCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + llvm::DenseSet ReportedFunctions; +}; + +} // namespace readability +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_INCONSISTENT_DECLARATION_PARAMETER_NAME_H Index: clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp =================================================================== --- clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp +++ clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp @@ -0,0 +1,126 @@ +//===--- InconsistentDeclarationParameterNameCheck.cpp - +//clang-tidy---------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "InconsistentDeclarationParameterNameCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace readability { + +namespace { + +struct DifferingParamInfo { + DifferingParamInfo(int Number, SourceLocation Location, + llvm::StringRef CurrentDeclarationName, + llvm::StringRef OtherDeclarationName) + : Number(Number), Location(Location), + CurrentDeclarationName(CurrentDeclarationName), + OtherDeclarationName(OtherDeclarationName) {} + + int Number; + SourceLocation Location; + llvm::StringRef CurrentDeclarationName; + llvm::StringRef OtherDeclarationName; +}; + +using DifferingParamsContainer = llvm::SmallVector; + +struct InconsistentDeclarationInfo { + SourceLocation OtherDeclarationLocation; + DifferingParamsContainer DifferingParams; +}; + +InconsistentDeclarationInfo +findOtherInconsitentDeclaration(const FunctionDecl *FunctionDeclaration) { + for (const FunctionDecl *OtherDeclaration : FunctionDeclaration->redecls()) { + if (OtherDeclaration == FunctionDeclaration) // skip self + continue; + + auto MyParamIt = FunctionDeclaration->param_begin(); + auto OtherParamIt = OtherDeclaration->param_begin(); + int Count = 1; + DifferingParamsContainer DifferingParams; + + while (MyParamIt != FunctionDeclaration->param_end() && + OtherParamIt != OtherDeclaration->param_end()) { + auto MyParamName = (*MyParamIt)->getName(); + auto OtherParamName = (*OtherParamIt)->getName(); + + if (!MyParamName.empty() && !OtherParamName.empty() && + MyParamName != OtherParamName) { + DifferingParams.emplace_back(Count, (*MyParamIt)->getLocation(), + MyParamName, OtherParamName); + } + + ++MyParamIt; + ++OtherParamIt; + ++Count; + } + + if (!DifferingParams.empty()) { + return {OtherDeclaration->getLocation(), DifferingParams}; + } + } + + return {SourceLocation(), {}}; +} + +} // anonymous namespace + +void InconsistentDeclarationParameterNameCheck::registerMatchers( + MatchFinder *Finder) { + Finder->addMatcher(functionDecl(unless(isImplicit())).bind("functionDecl"), + this); +} + +void InconsistentDeclarationParameterNameCheck::check( + const MatchFinder::MatchResult &Result) { + const FunctionDecl *FunctionDeclaration = + Result.Nodes.getNodeAs("functionDecl"); + if (FunctionDeclaration == nullptr) + return; + + if (ReportedFunctions.count(FunctionDeclaration) > 0) + return; // avoid multiple warnings + + InconsistentDeclarationInfo OtherInfo = + findOtherInconsitentDeclaration(FunctionDeclaration); + if (OtherInfo.OtherDeclarationLocation.isInvalid()) + return; + + diag(FunctionDeclaration->getLocation(), + "function '%0' has other declaration with different parameter name%s1") + << FunctionDeclaration->getQualifiedNameAsString() + << static_cast(OtherInfo.DifferingParams.size()); + + diag(OtherInfo.OtherDeclarationLocation, "other declaration seen here", + DiagnosticIDs::Level::Note); + + for (const DifferingParamInfo &ParamInfo : OtherInfo.DifferingParams) { + diag(ParamInfo.Location, + "parameter %0 is named '%1' here, but '%2' in other declaration", + DiagnosticIDs::Level::Note) + << ParamInfo.Number << ParamInfo.CurrentDeclarationName + << ParamInfo.OtherDeclarationName; + } + + // mark all redeclarations as visited + for (const FunctionDecl *Redecl : FunctionDeclaration->redecls()) { + ReportedFunctions.insert(Redecl); + } +} + +} // namespace readability +} // namespace tidy +} // namespace clang Index: clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tidy/readability/ReadabilityTidyModule.cpp +++ clang-tidy/readability/ReadabilityTidyModule.cpp @@ -15,6 +15,7 @@ #include "ElseAfterReturnCheck.h" #include "FunctionSizeCheck.h" #include "IdentifierNamingCheck.h" +#include "InconsistentDeclarationParameterNameCheck.h" #include "NamedParameterCheck.h" #include "RedundantSmartptrGetCheck.h" #include "RedundantStringCStrCheck.h" @@ -38,6 +39,8 @@ "readability-function-size"); CheckFactories.registerCheck( "readability-identifier-naming"); + CheckFactories.registerCheck( + "readability-inconsistent-declaration-parameter-name"); CheckFactories.registerCheck( "readability-named-parameter"); CheckFactories.registerCheck( Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -47,6 +47,7 @@ readability-else-after-return readability-function-size readability-identifier-naming + readability-inconsistent-declaration-parameter-name readability-named-parameter readability-redundant-smartptr-get readability-redundant-string-cstr Index: docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst =================================================================== --- docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst +++ docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst @@ -0,0 +1,29 @@ +readability-inconsistent-declaration-parameter-name +=================================================== + + +Find function declarations which differ in parameter names. + +Example: + +.. code:: c++ + + // in foo.hpp: + void foo(int a, int b, int c); + + // in foo.cpp: + void foo(int d, int e, int f); // warning + +This check should help to enforce consistency in large projects, where it often happens +that a definition of function is refactored, changing the parameter names, but its declaration +in header file is not updated. With this check, we can easily find and correct such inconsistencies, +keeping declaration and definition always in sync. + +Unnamed parameters are allowed and are not taken into account when comparing function declarations, for example: + +.. code:: c++ + + void foo(int a); + void foo(int); // no warning + +If there are multiple declarations of same function, only one warning will be generated. \ No newline at end of file Index: test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp =================================================================== --- test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp +++ test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp @@ -0,0 +1,56 @@ +// RUN: %python %S/check_clang_tidy.py %s readability-inconsistent-declaration-parameter-name %t + +void consistentFunction(int a, int b, int c); +void consistentFunction(int a, int b, int c); +void consistentFunction(int a, int b, int /*c*/); +void consistentFunction(int /*c*/, int /*c*/, int /*c*/); + +// CHECK-MESSAGES: :[[@LINE+1]]:6: warning: function 'inconsistentFunction' has other declaration with different parameter names [readability-inconsistent-declaration-parameter-name] +void inconsistentFunction(int a, int b, int c); +void inconsistentFunction(int d, int e, int f); +void inconsistentFunction(int x, int y, int z); + +struct SomeStruct { +// CHECK-MESSAGES: :[[@LINE+1]]:8: warning: function 'SomeStruct::inconsistentFunction' has other declaration with different parameter name [readability-inconsistent-declaration-parameter-name] + void inconsistentFunction(int a); +}; + +void SomeStruct::inconsistentFunction(int b) +{} + + +struct SpecialFunctions { +// CHECK-MESSAGES: :[[@LINE+1]]:3: warning: function 'SpecialFunctions::SpecialFunctions' has other declaration with different parameter name [readability-inconsistent-declaration-parameter-name] + SpecialFunctions(int a); + +// CHECK-MESSAGES: :[[@LINE+1]]:21: warning: function 'SpecialFunctions::operator=' has other declaration with different parameter name [readability-inconsistent-declaration-parameter-name] + SpecialFunctions& operator=(const SpecialFunctions& a); +}; + +SpecialFunctions::SpecialFunctions(int b) +{} + +SpecialFunctions& SpecialFunctions::operator=(const SpecialFunctions& b) { + return *this; +} + +template +void templateFunction(T a); + +template<> +// CHECK-MESSAGES: :[[@LINE+1]]:6: warning: function 'templateFunction' has other declaration with different parameter name [readability-inconsistent-declaration-parameter-name] +// FIXME: note messages are not very clear here +void templateFunction(int b) +{} + +template<> +// CHECK-MESSAGES: :[[@LINE+1]]:6: warning: function 'templateFunction' has other declaration with different parameter name [readability-inconsistent-declaration-parameter-name] +void templateFunction(float b) +{} + +#define DECL_FUNCTION(name, arg_name) \ + void name(int arg_name) + +// CHECK-MESSAGES: :[[@LINE+1]]:15: warning: function 'macroFunction' has other declaration with different parameter name [readability-inconsistent-declaration-parameter-name] +DECL_FUNCTION(macroFunction, a); +DECL_FUNCTION(macroFunction, b);