Skip to content

Commit a8b8ce0

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 llvm#107521 for easier review.
1 parent 1be4c97 commit a8b8ce0

File tree

3 files changed

+33
-11
lines changed

3 files changed

+33
-11
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/docs/ReleaseNotes.rst

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,8 @@ Changes in existing checks
167167

168168
- Improved :doc:`readability-container-contains
169169
<clang-tidy/checks/readability/container-contains>` check to let it work on
170-
any class that has a ``contains`` method.
170+
any class that has a ``contains`` method. Also now match previously missing
171+
cases with uncommon operand ordering.
171172

172173
- Improved :doc:`readability-implicit-bool-conversion
173174
<clang-tidy/checks/readability/implicit-bool-conversion>` check

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)