Index: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h =================================================================== --- clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h +++ 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 Index: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp +++ clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp @@ -58,6 +58,11 @@ "Constreverseiterator", "constreverseiterator"}); +/// The default value for the NamePrefixSuffixSilenceDissimilarityTreshold +/// check option. +static constexpr std::size_t + DefaultNamePrefixSuffixSilenceDissimilarityTreshold = 1; + #ifndef NDEBUG // The check's modelling logic is complex, and potentially hard to understand @@ -102,6 +107,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 { @@ -231,13 +238,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; @@ -339,6 +358,70 @@ 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) { + assert(S1.size() >= N && S2.size() >= N); + 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) { + assert(S1.size() >= N && S2.size() >= N); + 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()); + + if (BiggerLength <= Threshold) + // If the length of the strings is still smaller than the threshold, they + // would be covered by an empty prefix/suffix with the rest differing. + // (E.g. "A" and "X" with Threshold = 1 would mean we think they are + // similar and do not warn about them, which is a too eager assumption.) + return false; + + 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. @@ -396,7 +479,10 @@ Options.get("IgnoredParameterNames", DefaultIgnoredParameterNames))), IgnoredParameterTypeSuffixes(optutils::parseStringList( Options.get("IgnoredParameterTypeSuffixes", - DefaultIgnoredParameterTypeSuffixes))) {} + DefaultIgnoredParameterTypeSuffixes))), + NamePrefixSuffixSilenceDissimilarityTreshold( + Options.get("NamePrefixSuffixSilenceDissimilarityTreshold", + DefaultNamePrefixSuffixSilenceDissimilarityTreshold)) {} void EasilySwappableParametersCheck::storeOptions( ClangTidyOptions::OptionMap &Opts) { @@ -405,6 +491,8 @@ optutils::serializeStringList(IgnoredParameterNames)); Options.store(Opts, "IgnoredParameterTypeSuffixes", optutils::serializeStringList(IgnoredParameterTypeSuffixes)); + Options.store(Opts, "NamePrefixSuffixSilenceDissimilarityTreshold", + NamePrefixSuffixSilenceDissimilarityTreshold); } void EasilySwappableParametersCheck::registerMatchers(MatchFinder *Finder) { Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst =================================================================== --- clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst +++ 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 ----------- Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp +++ 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. Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp +++ 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)) Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp +++ 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. Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-prefixsuffixname.cpp =================================================================== --- /dev/null +++ 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' Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c +++ 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