[clang-tools-extra] [clang-tidy][modernize-use-starts-ends-with] Fix operator rewriting false negative (PR #117837)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tools-extra

Author: Nicolas van Kempen (nicovank)

<details>
<summary>Changes</summary>


In C++20, `operator!=` can be rewritten by negating `operator==`. This is the 
case for `std::string`, where `operator!=` is not provided hence relying on 
this rewriting.

Cover this case by matching `binaryOperation` and adding one case to 
`isNegativeComparison`.

This is only relevant in the newly added `substr` case, previous cases used a 
simple BinaryOperator. Release notes already mention adding this new case so I 
don't think further comments there are necessary.

Testing on non-mock:
```
&gt; cat tmp.cpp
#include &lt;string&gt;
void f(std::string u, std::string v) { u.substr(0, v.size()) != v; }

# Before change, no warning.
&gt; ./build/bin/clang-tidy -checks="-*,modernize-use-starts-ends-with" tmp.cpp 
-- -std=c++20

# After change.
&gt; ./build/bin/clang-tidy -checks="-*,modernize-use-starts-ends-with" tmp.cpp 
-- -std=c++20
tmp.cpp:2:42: warning: use starts_with instead of substr 
[modernize-use-starts-ends-with]
    2 | void f(std::string u, std::string v) { u.substr(0, v.size()) != v; }
      |                                          ^~~~~~ ~~~~~~~~~~~~~~~~~
      |                                        ! starts_with v)
```


---
Full diff: https://github.com/llvm/llvm-project/pull/117837.diff


3 Files Affected:

- (modified) clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp 
(+7-3) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string 
(+6-4) 
- (modified) 
clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp 
(+10) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
index 234f6790a7ed9b..ab72c6796bb4cd 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
@@ -20,12 +20,16 @@ using namespace clang::ast_matchers;
 namespace clang::tidy::modernize {
 
 static bool isNegativeComparison(const Expr *ComparisonExpr) {
-  if (const auto *BO = llvm::dyn_cast<BinaryOperator>(ComparisonExpr))
-    return BO->getOpcode() == BO_NE;
+  if (const auto *Op = llvm::dyn_cast<BinaryOperator>(ComparisonExpr))
+    return Op->getOpcode() == BO_NE;
 
   if (const auto *Op = llvm::dyn_cast<CXXOperatorCallExpr>(ComparisonExpr))
     return Op->getOperator() == OO_ExclaimEqual;
 
+  if (const auto *Op =
+          llvm::dyn_cast<CXXRewrittenBinaryOperator>(ComparisonExpr))
+    return Op->getOperator() == BO_NE;
+
   return false;
 }
 
@@ -185,7 +189,7 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder 
*Finder) {
 
   // Case 6: X.substr(0, LEN(Y)) [!=]= Y -> starts_with.
   Finder->addMatcher(
-      cxxOperatorCallExpr(
+      binaryOperation(
           hasAnyOperatorName("==", "!="),
           hasOperands(
               expr().bind("needle"),
diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string 
b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
index 51f68174169067..7e579e5319752a 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
@@ -136,10 +136,6 @@ bool operator==(const std::string&, const std::string&);
 bool operator==(const std::string&, const char*);
 bool operator==(const char*, const std::string&);
 
-bool operator!=(const std::string&, const std::string&);
-bool operator!=(const std::string&, const char*);
-bool operator!=(const char*, const std::string&);
-
 bool operator==(const std::wstring&, const std::wstring&);
 bool operator==(const std::wstring&, const wchar_t*);
 bool operator==(const wchar_t*, const std::wstring&);
@@ -148,9 +144,15 @@ bool operator==(const std::string_view&, const 
std::string_view&);
 bool operator==(const std::string_view&, const char*);
 bool operator==(const char*, const std::string_view&);
 
+#if __cplusplus < 202002L
+bool operator!=(const std::string&, const std::string&);
+bool operator!=(const std::string&, const char*);
+bool operator!=(const char*, const std::string&);
+
 bool operator!=(const std::string_view&, const std::string_view&);
 bool operator!=(const std::string_view&, const char*);
 bool operator!=(const char*, const std::string_view&);
+#endif
 
 size_t strlen(const char* str);
 }
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 18a65f7f1cf260..8699ca03ba3313 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
@@ -320,3 +320,13 @@ void test_substr() {
 
     str.substr(0, strlen("hello123")) == "hello";
 }
+
+void test_operator_rewriting(std::string str, std::string prefix) {
+  str.substr(0, prefix.size()) == prefix;
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead 
of substr
+  // CHECK-FIXES: str.starts_with(prefix);
+
+  str.substr(0, prefix.size()) != prefix;
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead 
of substr
+  // CHECK-FIXES: !str.starts_with(prefix);
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/117837
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits