llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Nicolas van Kempen (nicovank) <details> <summary>Changes</summary> Simplifies two cases and matches two new cases previously missing, `container.end() [!=]= container.find(...)`. I had split this change from #<!-- -->107521 for easier review. --- Full diff: https://github.com/llvm/llvm-project/pull/109178.diff 3 Files Affected: - (modified) clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp (+4-10) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+2-1) - (modified) clang-tools-extra/test/clang-tidy/checkers/readability/container-contains.cpp (+27) ``````````diff diff --git a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp index 698231d777d2d4..05d4c87bc73cef 100644 --- a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp @@ -62,10 +62,7 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) { .bind("positiveComparison"), this); AddSimpleMatcher( - binaryOperator(hasLHS(CountCall), hasOperatorName("!="), hasRHS(Literal0)) - .bind("positiveComparison")); - AddSimpleMatcher( - binaryOperator(hasLHS(Literal0), hasOperatorName("!="), hasRHS(CountCall)) + binaryOperator(hasOperatorName("!="), hasOperands(CountCall, Literal0)) .bind("positiveComparison")); AddSimpleMatcher( binaryOperator(hasLHS(CountCall), hasOperatorName(">"), hasRHS(Literal0)) @@ -82,10 +79,7 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) { // Find inverted membership tests which use `count()`. AddSimpleMatcher( - binaryOperator(hasLHS(CountCall), hasOperatorName("=="), hasRHS(Literal0)) - .bind("negativeComparison")); - AddSimpleMatcher( - binaryOperator(hasLHS(Literal0), hasOperatorName("=="), hasRHS(CountCall)) + binaryOperator(hasOperatorName("=="), hasOperands(CountCall, Literal0)) .bind("negativeComparison")); AddSimpleMatcher( binaryOperator(hasLHS(CountCall), hasOperatorName("<="), hasRHS(Literal0)) @@ -102,10 +96,10 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) { // Find membership tests based on `find() == end()`. AddSimpleMatcher( - binaryOperator(hasLHS(FindCall), hasOperatorName("!="), hasRHS(EndCall)) + binaryOperator(hasOperatorName("!="), hasOperands(FindCall, EndCall)) .bind("positiveComparison")); AddSimpleMatcher( - binaryOperator(hasLHS(FindCall), hasOperatorName("=="), hasRHS(EndCall)) + binaryOperator(hasOperatorName("=="), hasOperands(FindCall, EndCall)) .bind("negativeComparison")); } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 82a761bd7f40ab..c125223adcdf64 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -167,7 +167,8 @@ Changes in existing checks - Improved :doc:`readability-container-contains <clang-tidy/checks/readability/container-contains>` check to let it work on - any class that has a ``contains`` method. + any class that has a ``contains`` method. Also now match previously missing + cases with uncommon operand ordering. - Improved :doc:`readability-implicit-bool-conversion <clang-tidy/checks/readability/implicit-bool-conversion>` check 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 906515b075d4de..9a9b233e07229b 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 @@ -426,3 +426,30 @@ void testBox() { // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] // CHECK-FIXES: if (Set.contains(0)) {}; } + +void testOperandPermutations(std::map<int, int>& Map) { + if (Map.count(0) != 0) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] + // CHECK-FIXES: if (Map.contains(0)) {}; + if (0 != Map.count(0)) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] + // CHECK-FIXES: if (Map.contains(0)) {}; + if (Map.count(0) == 0) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] + // CHECK-FIXES: if (!Map.contains(0)) {}; + if (0 == Map.count(0)) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] + // CHECK-FIXES: if (!Map.contains(0)) {}; + if (Map.find(0) != Map.end()) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] + // CHECK-FIXES: if (Map.contains(0)) {}; + if (Map.end() != Map.find(0)) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] + // CHECK-FIXES: if (Map.contains(0)) {}; + if (Map.find(0) == Map.end()) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] + // CHECK-FIXES: if (!Map.contains(0)) {}; + if (Map.end() == Map.find(0)) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] + // CHECK-FIXES: if (!Map.contains(0)) {}; +} `````````` </details> https://github.com/llvm/llvm-project/pull/109178 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
