https://github.com/vbvictor updated https://github.com/llvm/llvm-project/pull/129564
>From c3f21bb654d2980955f2c37a5dc7a02a1ced7891 Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Mon, 3 Mar 2025 21:00:32 +0300 Subject: [PATCH 1/3] [clang-tidy] fixed false positives on find and rfind --- .../modernize/UseStartsEndsWithCheck.cpp | 7 ++++--- clang-tools-extra/docs/ReleaseNotes.rst | 4 ++++ .../checks/modernize/use-starts-ends-with.rst | 1 + .../checkers/modernize/use-starts-ends-with.cpp | 15 +++++++++++++++ 4 files changed, 24 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp index ab72c6796bb4c..02a537cccc430 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp @@ -113,9 +113,10 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { const auto OnClassWithEndsWithFunction = ClassTypeWithMethod( "ends_with_fun", "ends_with", "endsWith", "endswith", "EndsWith"); - // Case 1: X.find(Y) [!=]= 0 -> starts_with. + // Case 1: X.find(Y, [0]) [!=]= 0 -> starts_with. const auto FindExpr = cxxMemberCallExpr( - anyOf(argumentCountIs(1), hasArgument(1, ZeroLiteral)), + anyOf(argumentCountIs(1), + allOf(argumentCountIs(2), hasArgument(1, ZeroLiteral))), callee( cxxMethodDecl(hasName("find"), ofClass(OnClassWithStartsWithFunction)) .bind("find_fun")), @@ -123,7 +124,7 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { // Case 2: X.rfind(Y, 0) [!=]= 0 -> starts_with. const auto RFindExpr = cxxMemberCallExpr( - hasArgument(1, ZeroLiteral), + argumentCountIs(2), hasArgument(1, ZeroLiteral), callee(cxxMethodDecl(hasName("rfind"), ofClass(OnClassWithStartsWithFunction)) .bind("find_fun")), diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 07a79d6bbe807..e29598ccf0b43 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -137,6 +137,10 @@ Changes in existing checks <clang-tidy/checks/performance/move-const-arg>` check by fixing false negatives on ternary operators calling ``std::move``. +- Improved :doc:`modernize-use-starts-ends-with + <clang-tidy/checks/modernize/use-starts-ends-with>` check by fixing false + positives on methods ``find`` and ``rfind`` called with three arguments. + Removed checks ^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst index 78cd900885ac3..bad1952db22f9 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst @@ -13,6 +13,7 @@ Covered scenarios: Expression Replacement ---------------------------------------------------- --------------------- ``u.find(v) == 0`` ``u.starts_with(v)`` +``u.find(v, 0) == 0`` ``u.starts_with(v)`` ``u.rfind(v, 0) != 0`` ``!u.starts_with(v)`` ``u.compare(0, v.size(), v) == 0`` ``u.starts_with(v)`` ``u.substr(0, v.size()) == v`` ``u.starts_with(v)`` diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp index 8699ca03ba331..4d61709eff463 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp @@ -236,6 +236,18 @@ void test(std::string s, std::string_view sv, sub_string ss, sub_sub_string sss, // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use ends_with // CHECK-FIXES: s.ends_with(suffix); + s.find("a", 0) == 0; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with + // CHECK-FIXES: s.starts_with("a"); + + s.find(s, ZERO) == 0; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with + // CHECK-FIXES: s.starts_with(s); + + s.find(s, 0) == ZERO; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with + // CHECK-FIXES: s.starts_with(s); + struct S { std::string s; } t; @@ -261,6 +273,9 @@ void test(std::string s, std::string_view sv, sub_string ss, sub_sub_string sss, #define STRING s if (0 == STRING.find("ala")) { /* do something */} + + s.find("aaa", 0, 3) == 0; + s.rfind("aaa", std::string::npos, 3) == 0; } void test_substr() { >From e379faf582f3dea5072f6a7c86a44ea6aeed6749 Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Thu, 6 Mar 2025 00:21:16 +0300 Subject: [PATCH 2/3] [clang-tidy] add more matched-cases --- .../modernize/UseStartsEndsWithCheck.cpp | 27 +++++++++++----- clang-tools-extra/docs/ReleaseNotes.rst | 8 ++--- .../checks/modernize/use-starts-ends-with.rst | 2 ++ .../modernize/use-starts-ends-with.cpp | 31 +++++++++++++++++-- 4 files changed, 54 insertions(+), 14 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp index 02a537cccc430..2e059f24d47b6 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp @@ -113,22 +113,33 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) { const auto OnClassWithEndsWithFunction = ClassTypeWithMethod( "ends_with_fun", "ends_with", "endsWith", "endswith", "EndsWith"); - // Case 1: X.find(Y, [0]) [!=]= 0 -> starts_with. + // Case 1: X.find(Y, [0], [LEN(Y)]) [!=]= 0 -> starts_with. const auto FindExpr = cxxMemberCallExpr( - anyOf(argumentCountIs(1), - allOf(argumentCountIs(2), hasArgument(1, ZeroLiteral))), callee( cxxMethodDecl(hasName("find"), ofClass(OnClassWithStartsWithFunction)) .bind("find_fun")), - hasArgument(0, expr().bind("needle"))); - - // Case 2: X.rfind(Y, 0) [!=]= 0 -> starts_with. + hasArgument(0, expr().bind("needle")), + anyOf( + // Detect the expression: X.find(Y); + argumentCountIs(1), + // Detect the expression: X.find(Y, 0); + allOf(argumentCountIs(2), hasArgument(1, ZeroLiteral)), + // Detect the expression: X.find(Y, 0, LEN(Y)); + allOf(argumentCountIs(3), hasArgument(1, ZeroLiteral), + hasArgument(2, lengthExprForStringNode("needle"))))); + + // Case 2: X.rfind(Y, 0, [LEN(Y)]) [!=]= 0 -> starts_with. const auto RFindExpr = cxxMemberCallExpr( - argumentCountIs(2), hasArgument(1, ZeroLiteral), callee(cxxMethodDecl(hasName("rfind"), ofClass(OnClassWithStartsWithFunction)) .bind("find_fun")), - hasArgument(0, expr().bind("needle"))); + hasArgument(0, expr().bind("needle")), + anyOf( + // Detect the expression: X.rfind(Y, 0); + allOf(argumentCountIs(2), hasArgument(1, ZeroLiteral)), + // Detect the expression: X.rfind(Y, 0, LEN(Y)); + allOf(argumentCountIs(3), hasArgument(1, ZeroLiteral), + hasArgument(2, lengthExprForStringNode("needle"))))); // Case 3: X.compare(0, LEN(Y), Y) [!=]= 0 -> starts_with. const auto CompareExpr = cxxMemberCallExpr( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index e29598ccf0b43..36b4ad860a8a4 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -128,6 +128,10 @@ Changes in existing checks <clang-tidy/checks/misc/redundant-expression>` check by providing additional examples and fixing some macro related false positives. +- Improved :doc:`modernize-use-starts-ends-with + <clang-tidy/checks/modernize/use-starts-ends-with>` check by fixing false + positives on methods ``find`` and ``rfind`` called with three arguments. + - Improved :doc:`performance/unnecessary-value-param <clang-tidy/checks/performance/unnecessary-value-param>` check performance by tolerating fix-it breaking compilation when functions is used as pointers @@ -137,10 +141,6 @@ Changes in existing checks <clang-tidy/checks/performance/move-const-arg>` check by fixing false negatives on ternary operators calling ``std::move``. -- Improved :doc:`modernize-use-starts-ends-with - <clang-tidy/checks/modernize/use-starts-ends-with>` check by fixing false - positives on methods ``find`` and ``rfind`` called with three arguments. - Removed checks ^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst index bad1952db22f9..1babc2d1660ec 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst @@ -14,7 +14,9 @@ Expression Replacement ---------------------------------------------------- --------------------- ``u.find(v) == 0`` ``u.starts_with(v)`` ``u.find(v, 0) == 0`` ``u.starts_with(v)`` +``u.find(v, 0, v.size()) == 0`` ``u.starts_with(v)`` ``u.rfind(v, 0) != 0`` ``!u.starts_with(v)`` +``u.rfind(v, 0, v.size()) != 0`` ``!u.starts_with(v)`` ``u.compare(0, v.size(), v) == 0`` ``u.starts_with(v)`` ``u.substr(0, v.size()) == v`` ``u.starts_with(v)`` ``v != u.substr(0, v.size())`` ``!u.starts_with(v)`` diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp index 4d61709eff463..376c916eb5035 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp @@ -248,6 +248,30 @@ void test(std::string s, std::string_view sv, sub_string ss, sub_sub_string sss, // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with // CHECK-FIXES: s.starts_with(s); + s.find("aaa", 0, 3) == 0; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with + // CHECK-FIXES: s.starts_with("aaa"); + + s.find("aaa", ZERO, 3) == 0; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with + // CHECK-FIXES: s.starts_with("aaa"); + + s.find("aaa", ZERO, strlen(("aaa"))) == ZERO; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with + // CHECK-FIXES: s.starts_with("aaa"); + + s.rfind("aaa", 0, 3) != 0; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with + // CHECK-FIXES: !s.starts_with("aaa"); + + s.rfind("aaa", ZERO, 3) != 0; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with + // CHECK-FIXES: !s.starts_with("aaa"); + + s.rfind("aaa", ZERO, strlen(("aaa"))) == ZERO; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with + // CHECK-FIXES: s.starts_with("aaa"); + struct S { std::string s; } t; @@ -274,8 +298,11 @@ void test(std::string s, std::string_view sv, sub_string ss, sub_sub_string sss, #define STRING s if (0 == STRING.find("ala")) { /* do something */} - s.find("aaa", 0, 3) == 0; - s.rfind("aaa", std::string::npos, 3) == 0; + // Cases when literal-size and size parameters are different are not being matched. + s.find("aaa", 0, 2) == 0; + s.find("aaa", 0, strlen("aa")) == 0; + s.rfind("aaa", 0, 2) == 0; + s.rfind("aaa", 0, strlen("aa")) == 0; } void test_substr() { >From c011e2389557f441c02ae364b74a2264a1748637 Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Thu, 6 Mar 2025 00:29:36 +0300 Subject: [PATCH 3/3] enchanced release notes --- clang-tools-extra/docs/ReleaseNotes.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 36b4ad860a8a4..798ff55ccb56c 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -129,8 +129,9 @@ Changes in existing checks examples and fixing some macro related false positives. - Improved :doc:`modernize-use-starts-ends-with - <clang-tidy/checks/modernize/use-starts-ends-with>` check by fixing false - positives on methods ``find`` and ``rfind`` called with three arguments. + <clang-tidy/checks/modernize/use-starts-ends-with>` check by adding more + matched scenarios of ``find`` and ``rfind`` methods and fixing false + positives when those methods were called with 3 arguments. - Improved :doc:`performance/unnecessary-value-param <clang-tidy/checks/performance/unnecessary-value-param>` check performance by _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits