diff --git a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp index 05d4c87bc73ce..61f6a703257d6 100644 --- a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp @@ -8,7 +8,9 @@ #include "ContainerContainsCheck.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/Expr.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" using namespace clang::ast_matchers; @@ -30,9 +32,15 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) { ofClass(cxxRecordDecl(HasContainsMatchingParamType))))) .bind("call"); + const auto Literal0 = integerLiteral(equals(0)); + const auto Literal1 = integerLiteral(equals(1)); + const auto FindCall = cxxMemberCallExpr( - argumentCountIs(1), + anyOf(argumentCountIs(1), + allOf(argumentCountIs(2), // string::find takes two arguments + hasArgument(1, anyOf(cxxDefaultArgExpr(), + ignoringParenImpCasts(Literal0))))), callee(cxxMethodDecl( hasName("find"), hasParameter(0, hasType(hasUnqualifiedDesugaredType( @@ -48,8 +56,8 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) { // before EndCall so 'parameterType' is properly bound. ofClass(cxxRecordDecl(HasContainsMatchingParamType))))); - const auto Literal0 = integerLiteral(equals(0)); - const auto Literal1 = integerLiteral(equals(1)); + const auto StringNpos = anyOf(declRefExpr(to(varDecl(hasName("npos")))), + memberExpr(member(hasName("npos")))); auto AddSimpleMatcher = [&](auto Matcher) { Finder->addMatcher( @@ -94,12 +102,14 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) { binaryOperator(hasLHS(Literal1), hasOperatorName(">"), hasRHS(CountCall)) .bind("negativeComparison")); - // Find membership tests based on `find() == end()`. + // Find membership tests based on `find() == end()` or `find() == npos`. AddSimpleMatcher( - binaryOperator(hasOperatorName("!="), hasOperands(FindCall, EndCall)) + binaryOperator(hasOperatorName("!="), + hasOperands(FindCall, anyOf(EndCall, StringNpos))) .bind("positiveComparison")); AddSimpleMatcher( - binaryOperator(hasOperatorName("=="), hasOperands(FindCall, EndCall)) + binaryOperator(hasOperatorName("=="), + hasOperands(FindCall, anyOf(EndCall, StringNpos))) .bind("negativeComparison")); } diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string index 0c160bc182b6e..b1a3f40c10f18 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string @@ -50,12 +50,16 @@ struct basic_string { size_type find(const _Type& str, size_type pos = 0) const; size_type find(const C* s, size_type pos = 0) const; size_type find(const C* s, size_type pos, size_type n) const; + size_type find(C ch, size_type pos = 0) const; size_type rfind(const _Type& str, size_type pos = npos) const; size_type rfind(const C* s, size_type pos, size_type count) const; size_type rfind(const C* s, size_type pos = npos) const; size_type rfind(C ch, size_type pos = npos) const; + bool contains(const C *s) const; + bool contains(C s) const; + _Type& insert(size_type pos, const _Type& str); _Type& insert(size_type pos, const C* s); _Type& insert(size_type pos, const C* s, size_type n); @@ -104,6 +108,9 @@ struct basic_string_view { size_type rfind(const C* s, size_type pos, size_type count) const; size_type rfind(const C* s, size_type pos = npos) const; + bool contains(const C *s) const; + bool contains(C s) const; + constexpr bool starts_with(basic_string_view sv) const noexcept; constexpr bool starts_with(C ch) const noexcept; constexpr bool starts_with(const C* s) const; 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 9a9b233e07229..4fa2a378cff2c 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 @@ -1,4 +1,7 @@ -// RUN: %check_clang_tidy -std=c++20-or-later %s readability-container-contains %t +// RUN: %check_clang_tidy -std=c++20-or-later %s readability-container-contains %t -- \ +// RUN: -- -isystem %clang_tidy_headers + +#include // Some *very* simplified versions of `map` etc. namespace std { @@ -29,6 +32,8 @@ struct multimap { bool contains(const Key &K) const; }; + + } // namespace std // Check that we detect various common ways to check for membership @@ -453,3 +458,29 @@ void testOperandPermutations(std::map& Map) { // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] // CHECK-FIXES: if (!Map.contains(0)) {}; } + +void testStringNops(std::string Str, std::string SubStr, std::string_view StrView) { + if (Str.find("test") == std::string::npos) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] + // CHECK-FIXES: if (!Str.contains("test")) {}; + + if (Str.find('c') != std::string::npos) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] + // CHECK-FIXES: if (Str.contains('c')) {}; + + if (Str.find('c') != Str.npos) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] + // CHECK-FIXES: if (Str.contains('c')) {}; + + if (StrView.find("test") == std::string::npos) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] + // CHECK-FIXES: if (!StrView.contains("test")) {}; + + if (StrView.find('c') != std::string::npos) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] + // CHECK-FIXES: if (StrView.contains('c')) {}; + + if (StrView.find('c') != StrView.npos) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'contains' to check for membership [readability-container-contains] + // CHECK-FIXES: if (StrView.contains('c')) {}; +}