[clang-tools-extra] [clang-tidy] 'modernize-use-starts-ends-with': fixed false positives on `find` and `rfind` (PR #129564)
Baranov Victor via cfe-commits
cfe-commits at lists.llvm.org
Sun Mar 9 05:48:25 PDT 2025
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.2002 at gmail.com>
Date: Mon, 3 Mar 2025 21:00:32 +0300
Subject: [PATCH 1/6] [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.2002 at gmail.com>
Date: Thu, 6 Mar 2025 00:21:16 +0300
Subject: [PATCH 2/6] [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.2002 at gmail.com>
Date: Thu, 6 Mar 2025 00:29:36 +0300
Subject: [PATCH 3/6] 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
>From 3df70a395b75114cdb85deb39dbfe20679957893 Mon Sep 17 00:00:00 2001
From: Victor Baranov <bar.victor.2002 at gmail.com>
Date: Thu, 6 Mar 2025 00:33:07 +0300
Subject: [PATCH 4/6] fixed / to - in release notes
---
clang-tools-extra/docs/ReleaseNotes.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 798ff55ccb56c..adade4b523326 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -133,7 +133,7 @@ Changes in existing checks
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
+- 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
to avoid matching usage of functions within the current compilation unit.
>From c8d9108aefa06b60a3623623fff9de7a1b063a14 Mon Sep 17 00:00:00 2001
From: Victor Baranov <bar.victor.2002 at gmail.com>
Date: Fri, 7 Mar 2025 08:40:46 +0300
Subject: [PATCH 5/6] Fixed ordering in ReleaseNotes
---
clang-tools-extra/docs/ReleaseNotes.rst | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index adade4b523326..7fbba64c72d4c 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -133,15 +133,15 @@ Changes in existing checks
matched scenarios of ``find`` and ``rfind`` methods and fixing false
positives when those methods were called with 3 arguments.
+- Improved :doc:`performance-move-const-arg
+ <clang-tidy/checks/performance/move-const-arg>` check by fixing false negatives
+ on ternary operators calling ``std::move``.
+
- 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
to avoid matching usage of functions within the current compilation unit.
-- Improved :doc:`performance-move-const-arg
- <clang-tidy/checks/performance/move-const-arg>` check by fixing false negatives
- on ternary operators calling ``std::move``.
-
Removed checks
^^^^^^^^^^^^^^
>From 1e54abfa985955c139eb79d886c8c7959a65d93f Mon Sep 17 00:00:00 2001
From: Victor Baranov <bar.victor.2002 at gmail.com>
Date: Sun, 9 Mar 2025 15:48:09 +0300
Subject: [PATCH 6/6] fix issues in release notes
---
clang-tools-extra/docs/ReleaseNotes.rst | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6b94f52077566..d75afaa0d510f 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -132,11 +132,7 @@ 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-ranges
- <clang-tidy/checks/modernize/use-ranges>` check by updating suppress
- warnings logic for ``nullptr`` in ``std::find``.
-
- - Improved :doc:`misc-unused-using-decls
+- Improved :doc:`misc-unused-using-decls
<clang-tidy/checks/misc/unused-using-decls>` check by fixing false positives
on ``operator""`` with template parameters.
@@ -144,6 +140,10 @@ Changes in existing checks
<clang-tidy/checks/misc/use-internal-linkage>` check by fix false positives
for function or variable in header file which contains macro expansion.
+- Improved :doc:`modernize-use-ranges
+ <clang-tidy/checks/modernize/use-ranges>` check by updating suppress
+ warnings logic for ``nullptr`` in ``std::find``.
+
- Improved :doc:`modernize-use-starts-ends-with
<clang-tidy/checks/modernize/use-starts-ends-with>` check by adding more
matched scenarios of ``find`` and ``rfind`` methods and fixing false
More information about the cfe-commits
mailing list