Skip to content

Commit 3f5df5d

Browse files
committed
[clang-tidy][readability-container-contains] Use hasOperands when possible
Simplifies two cases and matches two new cases previously missing, `container.end() [!=]= container.find(...)`. I had split this change from #107521 for easier review.
1 parent 1be4c97 commit 3f5df5d

File tree

2 files changed

+31
-10
lines changed

2 files changed

+31
-10
lines changed

clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,7 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) {
6262
.bind("positiveComparison"),
6363
this);
6464
AddSimpleMatcher(
65-
binaryOperator(hasLHS(CountCall), hasOperatorName("!="), hasRHS(Literal0))
66-
.bind("positiveComparison"));
67-
AddSimpleMatcher(
68-
binaryOperator(hasLHS(Literal0), hasOperatorName("!="), hasRHS(CountCall))
65+
binaryOperator(hasOperatorName("!="), hasOperands(CountCall, Literal0))
6966
.bind("positiveComparison"));
7067
AddSimpleMatcher(
7168
binaryOperator(hasLHS(CountCall), hasOperatorName(">"), hasRHS(Literal0))
@@ -82,10 +79,7 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) {
8279

8380
// Find inverted membership tests which use `count()`.
8481
AddSimpleMatcher(
85-
binaryOperator(hasLHS(CountCall), hasOperatorName("=="), hasRHS(Literal0))
86-
.bind("negativeComparison"));
87-
AddSimpleMatcher(
88-
binaryOperator(hasLHS(Literal0), hasOperatorName("=="), hasRHS(CountCall))
82+
binaryOperator(hasOperatorName("=="), hasOperands(CountCall, Literal0))
8983
.bind("negativeComparison"));
9084
AddSimpleMatcher(
9185
binaryOperator(hasLHS(CountCall), hasOperatorName("<="), hasRHS(Literal0))
@@ -102,10 +96,10 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) {
10296

10397
// Find membership tests based on `find() == end()`.
10498
AddSimpleMatcher(
105-
binaryOperator(hasLHS(FindCall), hasOperatorName("!="), hasRHS(EndCall))
99+
binaryOperator(hasOperatorName("!="), hasOperands(FindCall, EndCall))
106100
.bind("positiveComparison"));
107101
AddSimpleMatcher(
108-
binaryOperator(hasLHS(FindCall), hasOperatorName("=="), hasRHS(EndCall))
102+
binaryOperator(hasOperatorName("=="), hasOperands(FindCall, EndCall))
109103
.bind("negativeComparison"));
110104
}
111105

clang-tools-extra/test/clang-tidy/checkers/readability/container-contains.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,3 +426,30 @@ void testBox() {
426426
// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
427427
// CHECK-FIXES: if (Set.contains(0)) {};
428428
}
429+
430+
void testOperandPermutations(std::map<int, int>& Map) {
431+
if (Map.count(0) != 0) {};
432+
// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
433+
// CHECK-FIXES: if (Map.contains(0)) {};
434+
if (0 != Map.count(0)) {};
435+
// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
436+
// CHECK-FIXES: if (Map.contains(0)) {};
437+
if (Map.count(0) == 0) {};
438+
// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
439+
// CHECK-FIXES: if (!Map.contains(0)) {};
440+
if (0 == Map.count(0)) {};
441+
// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
442+
// CHECK-FIXES: if (!Map.contains(0)) {};
443+
if (Map.find(0) != Map.end()) {};
444+
// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
445+
// CHECK-FIXES: if (Map.contains(0)) {};
446+
if (Map.end() != Map.find(0)) {};
447+
// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
448+
// CHECK-FIXES: if (Map.contains(0)) {};
449+
if (Map.find(0) == Map.end()) {};
450+
// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
451+
// CHECK-FIXES: if (!Map.contains(0)) {};
452+
if (Map.end() == Map.find(0)) {};
453+
// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains]
454+
// CHECK-FIXES: if (!Map.contains(0)) {};
455+
}

0 commit comments

Comments
 (0)