Author: sammccall Date: Fri Jul 13 04:41:56 2018 New Revision: 336992 URL: http://llvm.org/viewvc/llvm-project?rev=336992&view=rev Log: [clang-tidy] readability-inconsistent-declaration-parameter-name: accept approximate name matches.
Summary: The goal is to reduce false positives when the difference is intentional, like: foo(StringRef name); foo(StringRef name_ref) { string name = cleanup(name_ref); ... } Or semantically unimportant, like: foo(StringRef full_name); foo(StringRef name) { ... } There are other matching names we won't recognise (e.g. syns vs synonyms) but this catches many that we see in practice, and gives people a systematic workaround. The old behavior is available as a 'Strict' option. Subscribers: xazax.hun, cfe-commits Differential Revision: https://reviews.llvm.org/D49285 Added: clang-tools-extra/trunk/test/clang-tidy/readability-inconsistent-declaration-parameter-name-strict.cpp Modified: clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h clang-tools-extra/trunk/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst clang-tools-extra/trunk/test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp Modified: clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp?rev=336992&r1=336991&r2=336992&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp Fri Jul 13 04:41:56 2018 @@ -90,10 +90,20 @@ bool checkIfFixItHintIsApplicable( return true; } +bool nameMatch(StringRef L, StringRef R, bool Strict) { + if (Strict) + return L.empty() || R.empty() || L == R; + // We allow two names if one is a prefix/suffix of the other, ignoring case. + // Important special case: this is true if either parameter has no name! + return L.startswith_lower(R) || R.startswith_lower(L) || + L.endswith_lower(R) || R.endswith_lower(L); +} + DifferingParamsContainer findDifferingParamsInDeclaration(const FunctionDecl *ParameterSourceDeclaration, const FunctionDecl *OtherDeclaration, - const FunctionDecl *OriginalDeclaration) { + const FunctionDecl *OriginalDeclaration, + bool Strict) { DifferingParamsContainer DifferingParams; auto SourceParamIt = ParameterSourceDeclaration->param_begin(); @@ -106,8 +116,7 @@ findDifferingParamsInDeclaration(const F // FIXME: Provide a way to extract commented out parameter name from comment // next to it. - if (!SourceParamName.empty() && !OtherParamName.empty() && - SourceParamName != OtherParamName) { + if (!nameMatch(SourceParamName, OtherParamName, Strict)) { SourceRange OtherParamNameRange = DeclarationNameInfo((*OtherParamIt)->getDeclName(), (*OtherParamIt)->getLocation()) @@ -128,9 +137,9 @@ findDifferingParamsInDeclaration(const F } InconsistentDeclarationsContainer -findInconsitentDeclarations(const FunctionDecl *OriginalDeclaration, +findInconsistentDeclarations(const FunctionDecl *OriginalDeclaration, const FunctionDecl *ParameterSourceDeclaration, - SourceManager &SM) { + SourceManager &SM, bool Strict) { InconsistentDeclarationsContainer InconsistentDeclarations; SourceLocation ParameterSourceLocation = ParameterSourceDeclaration->getLocation(); @@ -141,7 +150,7 @@ findInconsitentDeclarations(const Functi DifferingParamsContainer DifferingParams = findDifferingParamsInDeclaration(ParameterSourceDeclaration, OtherDeclaration, - OriginalDeclaration); + OriginalDeclaration, Strict); if (!DifferingParams.empty()) { InconsistentDeclarations.emplace_back(OtherDeclaration->getLocation(), std::move(DifferingParams)); @@ -284,6 +293,7 @@ void formatDiagnostics( void InconsistentDeclarationParameterNameCheck::storeOptions( ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "IgnoreMacros", IgnoreMacros); + Options.store(Opts, "Strict", Strict); } void InconsistentDeclarationParameterNameCheck::registerMatchers( @@ -305,9 +315,9 @@ void InconsistentDeclarationParameterNam getParameterSourceDeclaration(OriginalDeclaration); InconsistentDeclarationsContainer InconsistentDeclarations = - findInconsitentDeclarations(OriginalDeclaration, - ParameterSourceDeclaration, - *Result.SourceManager); + findInconsistentDeclarations(OriginalDeclaration, + ParameterSourceDeclaration, + *Result.SourceManager, Strict); if (InconsistentDeclarations.empty()) { // Avoid unnecessary further visits. markRedeclarationsAsVisited(OriginalDeclaration); Modified: clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h?rev=336992&r1=336991&r2=336992&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h (original) +++ clang-tools-extra/trunk/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h Fri Jul 13 04:41:56 2018 @@ -28,7 +28,8 @@ public: InconsistentDeclarationParameterNameCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), - IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {} + IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0), + Strict(Options.getLocalOrGlobal("Strict", 0) != 0) {} void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; @@ -39,6 +40,7 @@ private: llvm::DenseSet<const FunctionDecl *> VisitedDeclarations; const bool IgnoreMacros; + const bool Strict; }; } // namespace readability Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst?rev=336992&r1=336991&r2=336992&view=diff ============================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst (original) +++ clang-tools-extra/trunk/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst Fri Jul 13 04:41:56 2018 @@ -29,6 +29,15 @@ function declarations, for example: void foo(int a); void foo(int); // no warning +One name is also allowed to be a case-insensitive prefix/suffix of the other: + +.. code-block:: c++ + + void foo(int count); + void foo(int count_input) { // no warning + int count = adjustCount(count_input); + } + To help with refactoring, in some cases fix-it hints are generated to align parameter names to a single naming convention. This works with the assumption that the function definition is the most up-to-date version, as it directly @@ -47,3 +56,8 @@ the definition or the first declaration If this option is set to non-zero (default is `1`), the check will not warn about names declared inside macros. + +.. option:: Strict + + If this option is set to non-zero (default is `0`), then names must match + exactly (or be absent). Added: clang-tools-extra/trunk/test/clang-tidy/readability-inconsistent-declaration-parameter-name-strict.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-inconsistent-declaration-parameter-name-strict.cpp?rev=336992&view=auto ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/readability-inconsistent-declaration-parameter-name-strict.cpp (added) +++ clang-tools-extra/trunk/test/clang-tidy/readability-inconsistent-declaration-parameter-name-strict.cpp Fri Jul 13 04:41:56 2018 @@ -0,0 +1,11 @@ +// RUN: %check_clang_tidy %s readability-inconsistent-declaration-parameter-name %t -- \ +// RUN: -config="{CheckOptions: [{key: readability-inconsistent-declaration-parameter-name.Strict, value: 1}]}" \ +// RUN: -- -std=c++11 + +void inconsistentFunction(int a, int b, int c); +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'inconsistentFunction' has 1 other declaration with different parameter names +void inconsistentFunction(int prefixA, int b, int cSuffix); +// CHECK-MESSAGES: :[[@LINE-1]]:6: note: the 1st inconsistent declaration seen here +// CHECK-MESSAGES: :[[@LINE-2]]:6: note: differing parameters are named here: ('prefixA', 'cSuffix'), in the other declaration: ('a', 'c') +void inconsistentFunction(int a, int b, int c); +void inconsistentFunction(int /*c*/, int /*c*/, int /*c*/); Modified: clang-tools-extra/trunk/test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp?rev=336992&r1=336991&r2=336992&view=diff ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp Fri Jul 13 04:41:56 2018 @@ -2,6 +2,8 @@ void consistentFunction(int a, int b, int c); void consistentFunction(int a, int b, int c); +void consistentFunction(int prefixA, int b, int cSuffix); +void consistentFunction(int a, int b, int c); void consistentFunction(int a, int b, int /*c*/); void consistentFunction(int /*c*/, int /*c*/, int /*c*/); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits