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,46 @@ +//===--- 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: + void markRedeclarationsAsVisited(const FunctionDecl *FunctionDeclaration); + + llvm::DenseSet VisitedDeclarations; +}; + +} // 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,206 @@ +//===--- 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" + +#include + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace readability { + +namespace { + +AST_MATCHER(FunctionDecl, hasOtherDeclarations) { + auto It = Node.redecls_begin(); + auto EndIt = Node.redecls_end(); + + if (It == EndIt) + return false; + + ++It; + return It != EndIt; +} + +struct DifferingParamInfo { + DifferingParamInfo(int Number, + llvm::StringRef MainName, + llvm::StringRef OtherName, + SourceRange OtherNameRange) + : Number(Number), + MainName(MainName), + OtherName(OtherName), + OtherNameRange(OtherNameRange) {} + + int Number; + llvm::StringRef MainName; + llvm::StringRef OtherName; + SourceRange OtherNameRange; +}; + +using DifferingParamsContainer = llvm::SmallVector; + +struct InconsistentDeclarationInfo { + InconsistentDeclarationInfo(SourceLocation DeclarationLocation, + bool IsTemplateSpecialization, + DifferingParamsContainer&& DifferingParams) + : DeclarationLocation(DeclarationLocation), + IsTemplateSpecialization(IsTemplateSpecialization), + DifferingParams(std::move(DifferingParams)) {} + + SourceLocation DeclarationLocation; + bool IsTemplateSpecialization; + DifferingParamsContainer DifferingParams; +}; + +using InconsistentDeclarationsContainer = llvm::SmallVector; + +InconsistentDeclarationsContainer +findInconsitentDeclarations(const FunctionDecl *FunctionDeclaration, SourceManager& SM) { + InconsistentDeclarationsContainer InconsistentDeclarations; + for (const FunctionDecl *OtherDeclaration : FunctionDeclaration->redecls()) { + if (OtherDeclaration == FunctionDeclaration) // skip self + continue; + + auto MainParamIt = FunctionDeclaration->param_begin(); + auto OtherParamIt = OtherDeclaration->param_begin(); + int Count = 1; + DifferingParamsContainer DifferingParams; + + while (MainParamIt != FunctionDeclaration->param_end() && + OtherParamIt != OtherDeclaration->param_end()) { + auto MainParamName = (*MainParamIt)->getName(); + auto OtherParamName = (*OtherParamIt)->getName(); + + // FIXME: provide a way to extract commented out parameter name from comment + if (!MainParamName.empty() && + !OtherParamName.empty() && + MainParamName != OtherParamName) { + SourceRange OtherParamNameRange = DeclarationNameInfo((*OtherParamIt)->getDeclName(), + (*OtherParamIt)->getLocation()).getSourceRange(); + DifferingParams.emplace_back(Count, MainParamName, OtherParamName, OtherParamNameRange); + } + + ++MainParamIt; + ++OtherParamIt; + ++Count; + } + + if (!DifferingParams.empty()) { + bool IsTemplateSpecialization = + OtherDeclaration->getTemplatedKind() == + FunctionDecl::TemplatedKind::TK_FunctionTemplateSpecialization; + + SourceLocation OtherLocation; + if (IsTemplateSpecialization) { + // special case of template specializations + // what we actually see here is a generated declaration from the main template declaration + // but it's regular getLocation() method confusingly shows the location of this specialization, not main template + OtherLocation = OtherDeclaration->getPrimaryTemplate()->getLocation(); + } else { + OtherLocation = OtherDeclaration->getLocation(); + } + + InconsistentDeclarations.emplace_back(OtherLocation, IsTemplateSpecialization, std::move(DifferingParams)); + } + } + + std::sort(InconsistentDeclarations.begin(), InconsistentDeclarations.end(), + [&SM](const InconsistentDeclarationInfo& Info1, const InconsistentDeclarationInfo& Info2) + { + return SM.isBeforeInTranslationUnit(Info1.DeclarationLocation, Info2.DeclarationLocation); + }); + return InconsistentDeclarations; +} + +const FunctionDecl* chooseDefinitionIfAvailable(const FunctionDecl* FunctionDeclaration) { + if (FunctionDeclaration->isThisDeclarationADefinition()) + return FunctionDeclaration; // already a definition + + for (const FunctionDecl *OtherDeclaration : FunctionDeclaration->redecls()) { + if (OtherDeclaration->isThisDeclarationADefinition()) { + return OtherDeclaration; // change to definition + } + } + + return FunctionDeclaration; // no definition found, so return original declaration +} + +} // anonymous namespace + +void InconsistentDeclarationParameterNameCheck::registerMatchers( + MatchFinder *Finder) { + Finder->addMatcher(functionDecl(unless(isImplicit()), + hasOtherDeclarations()).bind("functionDecl"), + this); +} + +void InconsistentDeclarationParameterNameCheck::check( + const MatchFinder::MatchResult &Result) { + const FunctionDecl *FunctionDeclaration = + Result.Nodes.getNodeAs("functionDecl"); + if (FunctionDeclaration == nullptr) + return; + + if (VisitedDeclarations.count(FunctionDeclaration) > 0) + return; // avoid multiple warnings + + FunctionDeclaration = chooseDefinitionIfAvailable(FunctionDeclaration); + bool HaveFunctionDefinition = FunctionDeclaration->isThisDeclarationADefinition(); + + InconsistentDeclarationsContainer InconsistentDeclarations = + findInconsitentDeclarations(FunctionDeclaration, *Result.SourceManager); + if (InconsistentDeclarations.empty()) { + markRedeclarationsAsVisited(FunctionDeclaration); // to avoid unnecessary further visits + return; + } + + diag(FunctionDeclaration->getLocation(), + "function %q0 has %1 other declaration%s1 with differently named parameters") + << FunctionDeclaration << static_cast(InconsistentDeclarations.size()); + + int Count = 1; + for (const InconsistentDeclarationInfo& InconsistentDeclaration : InconsistentDeclarations) { + diag(InconsistentDeclaration.DeclarationLocation, "%ordinal0 inconsistent declaration seen here", + DiagnosticIDs::Level::Note) + << Count; + + for (const DifferingParamInfo &ParamInfo : InconsistentDeclaration.DifferingParams) { + auto ParamDiag = diag(ParamInfo.OtherNameRange.getBegin(), + "parameter %0 is named '%1' here, but '%2' in compared declaration", + DiagnosticIDs::Level::Note) + << ParamInfo.Number << ParamInfo.OtherName << ParamInfo.MainName; + + if (HaveFunctionDefinition && !InconsistentDeclaration.IsTemplateSpecialization) { + ParamDiag << FixItHint::CreateReplacement(CharSourceRange::getTokenRange(ParamInfo.OtherNameRange), + ParamInfo.MainName); + } + // TODO: decide what to do if we see only declarations and no definition + } + + ++Count; + } + + markRedeclarationsAsVisited(FunctionDeclaration); +} + +void InconsistentDeclarationParameterNameCheck::markRedeclarationsAsVisited(const FunctionDecl *FunctionDeclaration) { + for (const FunctionDecl *Redecl : FunctionDeclaration->redecls()) { + VisitedDeclarations.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,8 +47,9 @@ 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 readability-shrink-to-fit readability-simplify-boolean-expr \ No newline at end of file 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,153 @@ +// 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 2 other declarations with differently named parameters [readability-inconsistent-declaration-parameter-name] +void inconsistentFunction(int a, int b, int c); +// CHECK-MESSAGES: :[[@LINE+4]]:6: note: 1st inconsistent declaration seen here +// CHECK-MESSAGES: :[[@LINE+3]]:31: note: parameter 1 is named 'd' here, but 'a' in compared declaration +// CHECK-MESSAGES: :[[@LINE+2]]:38: note: parameter 2 is named 'e' here, but 'b' in compared declaration +// CHECK-MESSAGES: :[[@LINE+1]]:45: note: parameter 3 is named 'f' here, but 'c' in compared declaration +void inconsistentFunction(int d, int e, int f); +// CHECK-MESSAGES: :[[@LINE+4]]:6: note: 2nd inconsistent declaration seen here +// CHECK-MESSAGES: :[[@LINE+3]]:31: note: parameter 1 is named 'x' here, but 'a' in compared declaration +// CHECK-MESSAGES: :[[@LINE+2]]:38: note: parameter 2 is named 'y' here, but 'b' in compared declaration +// CHECK-MESSAGES: :[[@LINE+1]]:45: note: parameter 3 is named 'z' here, but 'c' in compared declaration +void inconsistentFunction(int x, int y, int z); + +////////////////////////////////////////////////////// + +// CHECK-MESSAGES: :[[@LINE+5]]:6: warning: function 'inconsistentFunctionWithVisibleDefinition' has 1 other declaration with differently named parameters [readability-inconsistent-declaration-parameter-name] +// CHECK-MESSAGES: :[[@LINE+3]]:6: note: 1st inconsistent declaration seen here +// CHECK-MESSAGES: :[[@LINE+2]]:52: note: parameter 1 is named 'a' here, but 'b' in compared declaration +// CHECK-FIXES: void inconsistentFunctionWithVisibleDefinition(int b); +void inconsistentFunctionWithVisibleDefinition(int a); +void inconsistentFunctionWithVisibleDefinition(int b) {} + +////////////////////////////////////////////////////// + +struct Struct { +// CHECK-MESSAGES: :[[@LINE+7]]:14: warning: function 'Struct::inconsistentFunction' has 1 other declaration with differently named parameters [readability-inconsistent-declaration-parameter-name] +// CHECK-MESSAGES: :[[@LINE+3]]:8: note: 1st inconsistent declaration seen here +// CHECK-MESSAGES: :[[@LINE+2]]:33: note: parameter 1 is named 'a' here, but 'b' in compared declaration +// CHECK-FIXES: void inconsistentFunction(int b); + void inconsistentFunction(int a); +}; + +void Struct::inconsistentFunction(int b) {} + +////////////////////////////////////////////////////// + +struct SpecialFunctions { +// CHECK-MESSAGES: :[[@LINE+13]]:19: warning: function 'SpecialFunctions::SpecialFunctions' has 1 other declaration with differently named parameters [readability-inconsistent-declaration-parameter-name] +// CHECK-MESSAGES: :[[@LINE+3]]:3: note: 1st inconsistent declaration seen here +// CHECK-MESSAGES: :[[@LINE+2]]:24: note: parameter 1 is named 'a' here, but 'b' in compared declaration +// CHECK-FIXES: SpecialFunctions(int b); + SpecialFunctions(int a); + +// CHECK-MESSAGES: :[[@LINE+9]]:37: warning: function 'SpecialFunctions::operator=' has 1 other declaration with differently named parameters [readability-inconsistent-declaration-parameter-name] +// CHECK-MESSAGES: :[[@LINE+3]]:21: note: 1st inconsistent declaration seen here +// CHECK-MESSAGES: :[[@LINE+2]]:55: note: parameter 1 is named 'a' here, but 'b' in compared declaration +// CHECK-FIXES: SpecialFunctions& operator=(const SpecialFunctions& b); + SpecialFunctions& operator=(const SpecialFunctions& a); +}; + +SpecialFunctions::SpecialFunctions(int b) {} + +SpecialFunctions& SpecialFunctions::operator=(const SpecialFunctions& b) { return *this; } + +////////////////////////////////////////////////////// + +// CHECK-MESSAGES: :[[@LINE+8]]:6: warning: function 'templateFunctionWithSeparateDeclarationAndDefinition' has 1 other declaration with differently named parameters [readability-inconsistent-declaration-parameter-name] +// CHECK-MESSAGES: :[[@LINE+4]]:6: note: 1st inconsistent declaration seen here +// CHECK-MESSAGES: :[[@LINE+3]]:61: note: parameter 1 is named 'a' here, but 'b' in compared declaration +// CHECK-FIXES: void templateFunctionWithSeparateDeclarationAndDefinition(T b); +template +void templateFunctionWithSeparateDeclarationAndDefinition(T a); + +template +void templateFunctionWithSeparateDeclarationAndDefinition(T b) {} + +////////////////////////////////////////////////////// + +template +void templateFunctionWithSpecializations(T a) {} + +template<> +// CHECK-MESSAGES: :[[@LINE+3]]:6: warning: function 'templateFunctionWithSpecializations' has 1 other declaration with differently named parameters [readability-inconsistent-declaration-parameter-name] +// CHECK-MESSAGES: :[[@LINE-4]]:6: note: 1st inconsistent declaration seen here +// CHECK-MESSAGES: :[[@LINE-5]]:44: note: parameter 1 is named 'a' here, but 'b' in compared declaration +void templateFunctionWithSpecializations(int b) {} + +template<> +// CHECK-MESSAGES: :[[@LINE+3]]:6: warning: function 'templateFunctionWithSpecializations' has 1 other declaration with differently named parameters [readability-inconsistent-declaration-parameter-name] +// CHECK-MESSAGES: :[[@LINE-10]]:6: note: 1st inconsistent declaration seen here +// CHECK-MESSAGES: :[[@LINE-11]]:44: note: parameter 1 is named 'a' here, but 'c' in compared declaration +void templateFunctionWithSpecializations(float c) {} + +////////////////////////////////////////////////////// + +template +class ClassTemplate +{ +public: +// CHECK-MESSAGES: :[[@LINE+8]]:24: warning: function 'ClassTemplate::functionInClassTemplateWithSeparateDeclarationAndDefinition' has 1 other declaration with differently named parameters [readability-inconsistent-declaration-parameter-name] +// CHECK-MESSAGES: :[[@LINE+3]]:10: note: 1st inconsistent declaration seen here +// CHECK-MESSAGES: :[[@LINE+2]]:74: note: parameter 1 is named 'a' here, but 'b' in compared declaration +// CHECK-FIXES: void functionInClassTemplateWithSeparateDeclarationAndDefinition(int b); + void functionInClassTemplateWithSeparateDeclarationAndDefinition(int a); +}; + +template +void ClassTemplate::functionInClassTemplateWithSeparateDeclarationAndDefinition(int b) {} + +////////////////////////////////////////////////////// + +class Class +{ +public: + template +// CHECK-MESSAGES: :[[@LINE+13]]:13: warning: function 'Class::memberFunctionTemplateWithSeparateDeclarationAndDefinition' has 1 other declaration with differently named parameters [readability-inconsistent-declaration-parameter-name] +// CHECK-MESSAGES: :[[@LINE+3]]:8: note: 1st inconsistent declaration seen here +// CHECK-MESSAGES: :[[@LINE+2]]:69: note: parameter 1 is named 'a' here, but 'b' in compared declaration +// CHECK-FIXES: void memberFunctionTemplateWithSeparateDeclarationAndDefinition(T b); + void memberFunctionTemplateWithSeparateDeclarationAndDefinition(T a); + + template + void memberFunctionTemplateWithSpecializations(T a) {} +}; + +////////////////////////////////////////////////////// + +template +void Class::memberFunctionTemplateWithSeparateDeclarationAndDefinition(T b) {} + +////////////////////////////////////////////////////// + +template<> +// CHECK-MESSAGES: :[[@LINE+3]]:13: warning: function 'Class::memberFunctionTemplateWithSpecializations' has 1 other declaration with differently named parameters [readability-inconsistent-declaration-parameter-name] +// CHECK-MESSAGES: :[[@LINE-12]]:8: note: 1st inconsistent declaration seen here +// CHECK-MESSAGES: :[[@LINE-13]]:52: note: parameter 1 is named 'a' here, but 'b' in compared declaration +void Class::memberFunctionTemplateWithSpecializations(int b) {} + +template<> +// CHECK-MESSAGES: :[[@LINE+3]]:13: warning: function 'Class::memberFunctionTemplateWithSpecializations' has 1 other declaration with differently named parameters [readability-inconsistent-declaration-parameter-name] +// CHECK-MESSAGES: :[[@LINE-18]]:8: note: 1st inconsistent declaration seen here +// CHECK-MESSAGES: :[[@LINE-19]]:52: note: parameter 1 is named 'a' here, but 'c' in compared declaration +void Class::memberFunctionTemplateWithSpecializations(float c) {} + +////////////////////////////////////////////////////// + +#define DECLARE_FUNCTION_WITH_PARAM_NAME(function_name, param_name) \ + void function_name(int param_name) + +// CHECK-MESSAGES: :[[@LINE+1]]:34: warning: function 'macroFunction' has 1 other declaration with differently named parameters [readability-inconsistent-declaration-parameter-name] +DECLARE_FUNCTION_WITH_PARAM_NAME(macroFunction, a); +// CHECK-MESSAGES: :[[@LINE+2]]:34: note: 1st inconsistent declaration seen here +// CHECK-MESSAGES: :[[@LINE+1]]:49: note: parameter 1 is named 'b' here, but 'a' in compared declaration +DECLARE_FUNCTION_WITH_PARAM_NAME(macroFunction, b);