[clang-tools-extra] [clang-tidy] readability-container-contains literal suffixes (PR #74215)
Thomas Schenker via cfe-commits
cfe-commits at lists.llvm.org
Sat Dec 2 15:28:09 PST 2023
https://github.com/schenker updated https://github.com/llvm/llvm-project/pull/74215
>From f93185fe2346e7184c17ff35fb82ba0873e040a1 Mon Sep 17 00:00:00 2001
From: Thomas Schenker <thomas.schenker at protonmail.com>
Date: Sat, 2 Dec 2023 11:10:26 +0100
Subject: [PATCH] [clang-tidy] readability-container-contains literal suffixes
Before this PR, readability-container-contains fixits did not handle
integer literal suffixes correctly. It e.g. changed
```
MyMap.count(2) != 0U;
```
into
```
MyMap.contains(2)U;
```
With this PR, it correctly changes it to
```
MyMap.contains(2);
```
---
.../readability/ContainerContainsCheck.cpp | 4 +--
clang-tools-extra/docs/ReleaseNotes.rst | 4 +++
.../readability/container-contains.cpp | 32 +++++++++++++++++++
3 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
index 970ed8b83e0a7..dbb50a060e596 100644
--- a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
@@ -131,8 +131,8 @@ void ContainerContainsCheck::check(const MatchFinder::MatchResult &Result) {
Diag << FixItHint::CreateReplacement(
CharSourceRange::getCharRange(ComparisonBegin, CallBegin),
Negated ? "!" : "");
- Diag << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
- CallEnd.getLocWithOffset(1), ComparisonEnd.getLocWithOffset(1)));
+ Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+ CallEnd.getLocWithOffset(1), ComparisonEnd));
}
} // namespace clang::tidy::readability
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6d5f49dc06254..bdc44d6e4c14e 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -399,6 +399,10 @@ Changes in existing checks
<clang-tidy/checks/readability/avoid-const-params-in-decls>` diagnositics to
highlight the const location
+- Improved :doc:`readability-container-contains
+ <clang-tidy/checks/readability/container-contains>` to correctly handle
+ interger literals with suffixes in fixits.
+
- Improved :doc:`readability-container-size-empty
<clang-tidy/checks/readability/container-size-empty>` check to
detect comparison between string and empty string literals and support
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/container-contains.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/container-contains.cpp
index 05a4ebc9c8a17..cecedbb2b3ea1 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/container-contains.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/container-contains.cpp
@@ -55,6 +55,7 @@ int testDifferentCheckTypes(std::map<int, int> &MyMap) {
auto C6 = MyMap.find(5) != MyMap.end();
// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
// CHECK-FIXES: auto C6 = MyMap.contains(5);
+
return C1 + C2 + C3 + C4 + C5 + C6;
}
@@ -175,6 +176,37 @@ int nonRewrittenCount(std::multimap<int, int> &MyMap) {
return C1 + C2 + C3 + C4;
}
+// Check different integer literal suffixes
+int testDifferentIntegerLiteralSuffixes(std::map<int, int> &MyMap) {
+
+ auto C1 = MyMap.count(2) != 0U;
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: auto C1 = MyMap.contains(2);
+ auto C2 = MyMap.count(2) != 0UL;
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: auto C2 = MyMap.contains(2);
+ auto C3 = 0U != MyMap.count(2);
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: auto C3 = MyMap.contains(2);
+ auto C4 = 0UL != MyMap.count(2);
+ // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: auto C4 = MyMap.contains(2);
+ auto C5 = MyMap.count(2) < 1U;
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: auto C5 = !MyMap.contains(2);
+ auto C6 = MyMap.count(2) < 1UL;
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: auto C6 = !MyMap.contains(2);
+ auto C7 = 1U > MyMap.count(2);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: auto C7 = !MyMap.contains(2);
+ auto C8 = 1UL > MyMap.count(2);
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use 'contains' to check for membership [readability-container-contains]
+ // CHECK-FIXES: auto C8 = !MyMap.contains(2);
+
+ return C1 + C2 + C3 + C4 + C5 + C6 + C7 + C8;
+}
+
// We don't want to rewrite if the `contains` call is from a macro expansion
int testMacroExpansion(std::unordered_set<int> &MySet) {
#define COUNT_ONES(SET) SET.count(1)
More information about the cfe-commits
mailing list