-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[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
Conversation
@llvm/pr-subscribers-clang-tidy @llvm/pr-subscribers-clang-tools-extra Author: Nicolas van Kempen (nicovank) ChangesIn 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
Full diff: https://github.com/llvm/llvm-project/pull/116132.diff 3 Files Affected:
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;
|
clang-tools-extra/test/clang-tidy/checkers/modernize/use-starts-ends-with.cpp
Outdated
Show resolved
Hide resolved
…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( ) ```
Incorporated in #116033. |
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.