diff --git a/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h b/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h --- a/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h @@ -38,6 +38,12 @@ /// The parameter typename suffixes (as written in the source code) to be /// ignored. const std::vector IgnoredParameterTypeSuffixes; + + /// The number of characters two parameter names might be dissimilar at + /// either end for the report about the parameters to be silenced. + /// E.g. the names "LHS" and "RHS" are 1-dissimilar suffixes of each other, + /// while "Text1" and "Text2" are 1-dissimilar prefixes of each other. + const std::size_t NamePrefixSuffixSilenceDissimilarityTreshold; }; } // namespace bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp --- a/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp @@ -18,7 +18,7 @@ namespace optutils = clang::tidy::utils::options; /// The default value for the MinimumLength check option. -static constexpr unsigned DefaultMinimumLength = 2; +static constexpr std::size_t DefaultMinimumLength = 2; /// The default value for ignored parameter names. static const std::string DefaultIgnoredParameterNames = @@ -53,6 +53,11 @@ "reverse_iterator", "reverse_const_iterator"}); +/// The default value for the NamePrefixSuffixSilenceDissimilarityTreshold +/// check option. +static constexpr std::size_t + DefaultNamePrefixSuffixSilenceDissimilarityTreshold = 1; + #ifndef NDEBUG template @@ -93,6 +98,8 @@ namespace filter { static bool isIgnoredParameter(const TheCheck &Check, const ParmVarDecl *Node); +static bool prefixSuffixCoverUnderThreshold(std::size_t Threshold, + StringRef Str1, StringRef Str2); } // namespace filter namespace model { @@ -222,13 +229,25 @@ for (std::size_t I = StartIndex + 1; I < NumParams; ++I) { const ParmVarDecl *Ith = FD->getParamDecl(I); - LLVM_DEBUG(llvm::dbgs() << "Check param #" << I << "...\n"); - + StringRef ParamName = Ith->getName(); + LLVM_DEBUG(llvm::dbgs() + << "Check param #" << I << " '" << ParamName << "'...\n"); if (filter::isIgnoredParameter(Check, Ith)) { LLVM_DEBUG(llvm::dbgs() << "Param #" << I << " is ignored. Break!\n"); break; } + StringRef PrevParamName = FD->getParamDecl(I - 1)->getName(); + if (!ParamName.empty() && !PrevParamName.empty() && + filter::prefixSuffixCoverUnderThreshold( + Check.NamePrefixSuffixSilenceDissimilarityTreshold, PrevParamName, + ParamName)) { + LLVM_DEBUG(llvm::dbgs() << "Parameter '" << ParamName + << "' follows a pattern with previous parameter '" + << PrevParamName << "'. Break!\n"); + break; + } + // Now try to go forward and build the range of [Start, ..., I, I + 1, ...] // parameters that can be messed up at a call site. MixableParameterRange::MixVector MixesOfIth; @@ -329,6 +348,60 @@ return false; } + +static void padStringAtEnd(SmallVectorImpl &Str, std::size_t ToLen) { + while (Str.size() < ToLen) + Str.emplace_back('\0'); +} + +static void padStringAtBegin(SmallVectorImpl &Str, std::size_t ToLen) { + while (Str.size() < ToLen) + Str.insert(Str.begin(), '\0'); +} + +static bool isCommonPrefixWithoutSomeCharacters(std::size_t N, StringRef S1, + StringRef S2) { + StringRef S1Prefix = S1.take_front(S1.size() - N), + S2Prefix = S2.take_front(S2.size() - N); + return S1Prefix == S2Prefix && !S1Prefix.empty(); +} + +static bool isCommonSuffixWithoutSomeCharacters(std::size_t N, StringRef S1, + StringRef S2) { + StringRef S1Suffix = S1.take_back(S1.size() - N), + S2Suffix = S2.take_back(S2.size() - N); + return S1Suffix == S2Suffix && !S1Suffix.empty(); +} + +/// Returns whether the two strings are prefixes or suffixes of each other with +/// at most Threshold characters differing on the non-common end. +static bool prefixSuffixCoverUnderThreshold(std::size_t Threshold, + StringRef Str1, StringRef Str2) { + if (Threshold == 0) + return false; + + // Pad the two strings to the longer length. + std::size_t BiggerLength = std::max(Str1.size(), Str2.size()); + SmallString<32> S1PadE{Str1}, S2PadE{Str2}; + padStringAtEnd(S1PadE, BiggerLength); + padStringAtEnd(S2PadE, BiggerLength); + + if (isCommonPrefixWithoutSomeCharacters( + Threshold, StringRef{S1PadE.begin(), BiggerLength}, + StringRef{S2PadE.begin(), BiggerLength})) + return true; + + SmallString<32> S1PadB{Str1}, S2PadB{Str2}; + padStringAtBegin(S1PadB, BiggerLength); + padStringAtBegin(S2PadB, BiggerLength); + + if (isCommonSuffixWithoutSomeCharacters( + Threshold, StringRef{S1PadB.begin(), BiggerLength}, + StringRef{S2PadB.begin(), BiggerLength})) + return true; + + return false; +} } // namespace filter /// Matches functions that have at least the specified amount of parameters. @@ -384,7 +457,10 @@ Options.get("IgnoredParameterNames", DefaultIgnoredParameterNames))), IgnoredParameterTypeSuffixes(optutils::parseStringList( Options.get("IgnoredParameterTypeSuffixes", - DefaultIgnoredParameterTypeSuffixes))) {} + DefaultIgnoredParameterTypeSuffixes))), + NamePrefixSuffixSilenceDissimilarityTreshold( + Options.get("NamePrefixSuffixSilenceDissimilarityTreshold", + DefaultNamePrefixSuffixSilenceDissimilarityTreshold)) {} void EasilySwappableParametersCheck::storeOptions( ClangTidyOptions::OptionMap &Opts) { @@ -393,6 +469,8 @@ optutils::serializeStringList(IgnoredParameterNames)); Options.store(Opts, "IgnoredParameterTypeSuffixes", optutils::serializeStringList(IgnoredParameterTypeSuffixes)); + Options.store(Opts, "NamePrefixSuffixSilenceDissimilarityTreshold", + NamePrefixSuffixSilenceDissimilarityTreshold); } void EasilySwappableParametersCheck::registerMatchers(MatchFinder *Finder) { diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst @@ -81,6 +81,27 @@ In addition, `_Bool` (but not `_bool`) is also part of the default value. +.. option:: NamePrefixSuffixSilenceDissimilarityTreshold + + The number of characters two parameter names might be different on *either* + the head or the tail end with the rest of the name the same so that the + warning about the two parameters are silenced. + Defaults to `1`. + Might be any positive integer. + If `0`, the filtering heuristic based on the parameters' names is turned + off. + + This option can be used to silence warnings about parameters where the + naming scheme indicates that the order of those parameters do not matter. + + For example, the parameters ``LHS`` and ``RHS`` are 1-dissimilar suffixes + of each other: ``L`` and ``R`` is the different character, while ``HS`` + is the common suffix. + Similarly, parameters ``text1, text2, text3`` are 1-dissimilar prefixes + of each other, with the numbers at the end being the dissimilar part. + If the value is at least `1`, such cases will not be reported. + + Limitations ----------- diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp @@ -2,7 +2,8 @@ // RUN: -config='{CheckOptions: [ \ // RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \ // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: "\"\";Foo;Bar"}, \ -// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "T"} \ +// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "T"}, \ +// RUN: {key: bugprone-easily-swappable-parameters.NamePrefixSuffixSilenceDissimilarityTreshold, value: 0} \ // RUN: ]}' -- void ignoredUnnamed(int I, int, int) {} // NO-WARN: No >= 2 length of non-unnamed. diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp @@ -2,7 +2,8 @@ // RUN: -config='{CheckOptions: [ \ // RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \ // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \ -// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""} \ +// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \ +// RUN: {key: bugprone-easily-swappable-parameters.NamePrefixSuffixSilenceDissimilarityTreshold, value: 0} \ // RUN: ]}' -- #define assert(X) ((void)(X)) diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp @@ -2,7 +2,8 @@ // RUN: -config='{CheckOptions: [ \ // RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 3}, \ // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \ -// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""} \ +// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \ +// RUN: {key: bugprone-easily-swappable-parameters.NamePrefixSuffixSilenceDissimilarityTreshold, value: 0} \ // RUN: ]}' -- int add(int Left, int Right) { return Left + Right; } // NO-WARN: Only 2 parameters. diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-prefixsuffixname.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-prefixsuffixname.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-prefixsuffixname.cpp @@ -0,0 +1,53 @@ +// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \ +// RUN: -config='{CheckOptions: [ \ +// RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \ +// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \ +// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \ +// RUN: {key: bugprone-easily-swappable-parameters.NamePrefixSuffixSilenceDissimilarityTreshold, value: 1} \ +// RUN: ]}' -- + +namespace std { +struct string {}; +} // namespace std +class Matrix {}; + +void test1(int Foo, int Bar) {} +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 2 adjacent parameters of 'test1' of similar type ('int') are easily swapped by mistake [bugprone-easily-swappable-parameters] +// CHECK-MESSAGES: :[[@LINE-2]]:16: note: the first parameter in the range is 'Foo' +// CHECK-MESSAGES: :[[@LINE-3]]:25: note: the last parameter in the range is 'Bar' + +void test2(int A, int B) {} +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 2 adjacent parameters of 'test2' of similar type ('int') +// CHECK-MESSAGES: :[[@LINE-2]]:16: note: the first parameter in the range is 'A' +// CHECK-MESSAGES: :[[@LINE-3]]:23: note: the last parameter in the range is 'B' + +void test3(int Val1, int Val2) {} // NO-WARN. + +void test4(int ValA, int Valb) {} // NO-WARN. + +void test5(int Val1, int ValZ) {} // NO-WARN. + +void test6(int PValue, int QValue) {} // NO-WARN. + +void test7(std::string Astr, std::string Bstr) {} // NO-WARN. + +void test8(int Aladdin, int Alabaster) {} +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 2 adjacent parameters of 'test8' of similar type ('int') +// CHECK-MESSAGES: :[[@LINE-2]]:16: note: the first parameter in the range is 'Aladdin' +// CHECK-MESSAGES: :[[@LINE-3]]:29: note: the last parameter in the range is 'Alabaster' + +void test9(Matrix Qmat, Matrix Rmat, Matrix Tmat) {} // NO-WARN. + +void test10(int Something, int Other, int Foo, int Bar1, int Bar2, int Baz, int Qux) {} +// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: 4 adjacent parameters of 'test10' of similar type ('int') are +// CHECK-MESSAGES: :[[@LINE-2]]:17: note: the first parameter in the range is 'Something' +// CHECK-MESSAGES: :[[@LINE-3]]:52: note: the last parameter in the range is 'Bar1' +// +// CHECK-MESSAGES: :[[@LINE-5]]:58: warning: 3 adjacent parameters of 'test10' of similar type ('int') are +// CHECK-MESSAGES: :[[@LINE-6]]:62: note: the first parameter in the range is 'Bar2' +// CHECK-MESSAGES: :[[@LINE-7]]:81: note: the last parameter in the range is 'Qux' + +void test11(int Foobar, int Foo) {} +// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: 2 adjacent parameters of 'test11' of similar type ('int') +// CHECK-MESSAGES: :[[@LINE-2]]:17: note: the first parameter in the range is 'Foobar' +// CHECK-MESSAGES: :[[@LINE-3]]:29: note: the last parameter in the range is 'Foo' diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c @@ -2,7 +2,8 @@ // RUN: -config='{CheckOptions: [ \ // RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \ // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \ -// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"} \ +// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"}, \ +// RUN: {key: bugprone-easily-swappable-parameters.NamePrefixSuffixSilenceDissimilarityTreshold, value: 0} \ // RUN: ]}' -- -x c #define bool _Bool