Skip to content

[clang-tidy][modernize-use-starts-ends-with] Fix minor mistake in error message #116132

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

nicovank
Copy link
Contributor

In one of the cases recently added to this check in #110448, the error message is no longer accurate as the comparison is not with zero. #116033 brought my attention to this as it may add another comparison without zero.

Remove the [!=] 0 part of the diagnostic. This is in line with some other checks e.g. modernize-use-emplace.

> cat tmp.cpp
#include <string>
bool f(std::string u, std::string v) {
  return u.rfind(v) == u.size() - v.size();
}

# Before.
> ./build/bin/clang-tidy -checks="-*,modernize-use-starts-ends-with" tmp.cpp -- -std=c++20
tmp.cpp:3:12: warning: use ends_with instead of rfind() == 0 [modernize-use-starts-ends-with]
    3 |   return u.rfind(v) == u.size() - v.size();
      |            ^~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~
      |            ends_with( )

# After.
> ./build/bin/clang-tidy -checks="-*,modernize-use-starts-ends-with" tmp.cpp -- -std=c++20
tmp.cpp:3:12: warning: use ends_with instead of rfind [modernize-use-starts-ends-with]
    3 |   return u.rfind(v) == u.size() - v.size();
      |            ^~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~
      |            ends_with( )

@llvmbot
Copy link
Member

llvmbot commented Nov 14, 2024

@llvm/pr-subscribers-clang-tidy

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

Author: Nicolas van Kempen (nicovank)

Changes

In one of the cases recently added to this check in #110448, the error message is no longer accurate as the comparison is not with zero. #116033 brought my attention to this as it may add another comparison without zero.

Remove the [!=] 0 part of the diagnostic. This is in line with some other checks e.g. modernize-use-emplace.

&gt; cat tmp.cpp
#include &lt;string&gt;
bool f(std::string u, std::string v) {
  return u.rfind(v) == u.size() - v.size();
}

# Before.
&gt; ./build/bin/clang-tidy -checks="-*,modernize-use-starts-ends-with" tmp.cpp -- -std=c++20
tmp.cpp:3:12: warning: use ends_with instead of rfind() == 0 [modernize-use-starts-ends-with]
    3 |   return u.rfind(v) == u.size() - v.size();
      |            ^~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~
      |            ends_with( )

# After.
&gt; ./build/bin/clang-tidy -checks="-*,modernize-use-starts-ends-with" tmp.cpp -- -std=c++20
tmp.cpp:3:12: warning: use ends_with instead of rfind [modernize-use-starts-ends-with]
    3 |   return u.rfind(v) == u.size() - v.size();
      |            ^~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~
      |            ends_with( )

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

3 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp (+3-6)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+1-1)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp (+4-4)
diff --git a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
index 1231f954298adc..4d8f6dbc2cc11b 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp
@@ -189,11 +189,8 @@ void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) {
   if (ComparisonExpr->getBeginLoc().isMacroID())
     return;
 
-  const bool Neg = ComparisonExpr->getOpcode() == BO_NE;
-
-  auto Diagnostic =
-      diag(FindExpr->getExprLoc(), "use %0 instead of %1() %select{==|!=}2 0")
-      << ReplacementFunction->getName() << FindFun->getName() << Neg;
+  auto Diagnostic = diag(FindExpr->getExprLoc(), "use %0 instead of %1")
+                    << ReplacementFunction->getName() << FindFun->getName();
 
   // Remove possible arguments after search expression and ' [!=]= .+' suffix.
   Diagnostic << FixItHint::CreateReplacement(
@@ -215,7 +212,7 @@ void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) {
       (ReplacementFunction->getName() + "(").str());
 
   // Add possible negation '!'.
-  if (Neg)
+  if (ComparisonExpr->getOpcode() == BO_NE)
     Diagnostic << FixItHint::CreateInsertion(FindExpr->getBeginLoc(), "!");
 }
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index db971f08ca3dbc..a6c2ced1a56783 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -244,7 +244,7 @@ Changes in existing checks
 
 - Improved :doc:`modernize-use-starts-ends-with
   <clang-tidy/checks/modernize/use-starts-ends-with>` check to handle two cases
-  that can be replaced with ``ends_with``
+  that can be replaced with ``ends_with``. Minor change to error message.
 
 - Improved :doc:`modernize-use-std-format
   <clang-tidy/checks/modernize/use-std-format>` check to support replacing
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 91477241e82e54..788de297a01c01 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
@@ -36,7 +36,7 @@ void test(std::string s, std::string_view sv, sub_string ss, sub_sub_string sss,
           string_like sl, string_like_camel slc, prefer_underscore_version puv,
           prefer_underscore_version_flip puvf) {
   s.find("a") == 0;
-  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead of find() == 0
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead of find
   // CHECK-FIXES: s.starts_with("a");
 
   (((((s)).find("a")))) == ((0));
@@ -68,7 +68,7 @@ void test(std::string s, std::string_view sv, sub_string ss, sub_sub_string sss,
   // CHECK-FIXES: !s.starts_with("a");
 
   s.rfind("a", 0) == 0;
-  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead of rfind() == 0
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead of rfind
   // CHECK-FIXES: s.starts_with("a");
 
   s.rfind(s, 0) == 0;
@@ -149,11 +149,11 @@ void test(std::string s, std::string_view sv, sub_string ss, sub_sub_string sss,
   // CHECK-FIXES: puvf.starts_with("a");
 
   s.compare(0, 1, "a") == 0;
-  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead of compare() == 0
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead of compare
   // CHECK-FIXES: s.starts_with("a");
 
   s.compare(0, 1, "a") != 0;
-  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead of compare() != 0
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use starts_with instead of compare
   // CHECK-FIXES: !s.starts_with("a");
 
   s.compare(0, strlen("a"), "a") == 0;

…or message

In one of the cases recently added to this check in llvm#110448, the error message is no longer accurate as the comparison is not with zero. llvm#116033 brought my attention to this as it may add another comparison without zero.

Remove the `[!=] 0` part of the diagnostic. This is in line with some other checks e.g. modernize-use-emplace.

```
> cat tmp.cpp
#include <string>
bool f(std::string u, std::string v) {
  return u.rfind(v) == u.size() - v.size();
}

# Before.
> ./build/bin/clang-tidy -checks="-*,modernize-use-starts-ends-with" tmp.cpp -- -std=c++20
tmp.cpp:3:12: warning: use ends_with instead of rfind() == 0 [modernize-use-starts-ends-with]
    3 |   return u.rfind(v) == u.size() - v.size();
      |            ^~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~
      |            ends_with( )

# After.
> ./build/bin/clang-tidy -checks="-*,modernize-use-starts-ends-with" tmp.cpp -- -std=c++20
tmp.cpp:3:12: warning: use ends_with instead of rfind [modernize-use-starts-ends-with]
    3 |   return u.rfind(v) == u.size() - v.size();
      |            ^~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~
      |            ends_with( )
```
@nicovank
Copy link
Contributor Author

Incorporated in #116033.

@nicovank nicovank closed this Nov 27, 2024
@nicovank nicovank deleted the pr116132 branch November 27, 2024 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants