[clang-tools-extra] [clang-tidy] support string::contains (PR #110351)
Tommy Chen via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 4 21:27:25 PDT 2024
https://github.com/dl8sd11 updated https://github.com/llvm/llvm-project/pull/110351
>From b98e9a4d50d74c298096d2bd2d70ff4c0ef5c4a4 Mon Sep 17 00:00:00 2001
From: dl8sd11 <gcchen at google.com>
Date: Sat, 28 Sep 2024 07:37:50 +0000
Subject: [PATCH 1/4] [clang-tidy] support string::contains
---
.../readability/ContainerContainsCheck.cpp | 18 +++++--
.../readability/container-contains.cpp | 49 +++++++++++++++++++
2 files changed, 63 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
index 05d4c87bc73cef..a5e5960eaa6a3c 100644
--- a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
@@ -9,6 +9,7 @@
#include "ContainerContainsCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
using namespace clang::ast_matchers;
@@ -32,7 +33,8 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) {
const auto FindCall =
cxxMemberCallExpr(
- argumentCountIs(1),
+ anyOf(argumentCountIs(1),
+ allOf(argumentCountIs(2), hasArgument(1, cxxDefaultArgExpr()))),
callee(cxxMethodDecl(
hasName("find"),
hasParameter(0, hasType(hasUnqualifiedDesugaredType(
@@ -51,6 +53,12 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) {
const auto Literal0 = integerLiteral(equals(0));
const auto Literal1 = integerLiteral(equals(1));
+ const auto StringLikeClass = cxxRecordDecl(
+ hasAnyName("::std::basic_string", "::std::basic_string_view",
+ "::absl::string_view"));
+ const auto StringNpos = declRefExpr(
+ to(varDecl(hasName("npos"), hasDeclContext(StringLikeClass))));
+
auto AddSimpleMatcher = [&](auto Matcher) {
Finder->addMatcher(
traverse(TK_IgnoreUnlessSpelledInSource, std::move(Matcher)), this);
@@ -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/readability/container-contains.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/container-contains.cpp
index 9a9b233e07229b..69cc5c88479040 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
@@ -29,6 +29,43 @@ struct multimap {
bool contains(const Key &K) const;
};
+using size_t = decltype(sizeof(int));
+
+// Lightweight standin for std::string_view.
+template <typename C>
+class basic_string_view {
+public:
+ basic_string_view();
+ basic_string_view(const basic_string_view &);
+ basic_string_view(const C *);
+ ~basic_string_view();
+ int find(basic_string_view s, int pos = 0);
+ int find(const C *s, int pos = 0);
+ int find(const C *s, int pos, int n);
+ int find(char c, int pos = 0);
+ static constexpr size_t npos = -1;
+};
+typedef basic_string_view<char> string_view;
+
+// Lightweight standin for std::string.
+template <typename C>
+class basic_string {
+public:
+ basic_string();
+ basic_string(const basic_string &);
+ basic_string(const C *);
+ ~basic_string();
+ int find(basic_string s, int pos = 0);
+ int find(const C *s, int pos = 0);
+ int find(const C *s, int pos, int n);
+ int find(char c, int pos = 0);
+ bool contains(const C *s) const;
+ bool contains(C s) const;
+ bool contains(basic_string_view<C> s) const;
+ static constexpr size_t npos = -1;
+};
+typedef basic_string<char> string;
+
} // namespace std
// Check that we detect various common ways to check for membership
@@ -453,3 +490,15 @@ void testOperandPermutations(std::map<int, int>& 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) {
+ 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(SubStr) != std::string::npos) {};
+}
>From e42c356bdd679beaf1ee9199afeee566726e9fc9 Mon Sep 17 00:00:00 2001
From: dl8sd11 <gcchen at google.com>
Date: Sat, 5 Oct 2024 04:24:41 +0000
Subject: [PATCH 2/4] [clang-tidy] fix string::find(s, 0)
---
.../readability/ContainerContainsCheck.cpp | 20 +++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
index a5e5960eaa6a3c..fc05498b47e2e1 100644
--- a/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
@@ -8,6 +8,7 @@
#include "ContainerContainsCheck.h"
#include "clang/AST/ASTContext.h"
+#include "clang/AST/Expr.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
@@ -31,10 +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(
anyOf(argumentCountIs(1),
- allOf(argumentCountIs(2), hasArgument(1, cxxDefaultArgExpr()))),
+ allOf(argumentCountIs(2),
+ hasArgument(1, anyOf(cxxDefaultArgExpr(),
+ ignoringParenImpCasts(Literal0))))),
callee(cxxMethodDecl(
hasName("find"),
hasParameter(0, hasType(hasUnqualifiedDesugaredType(
@@ -50,14 +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 StringLikeClass = cxxRecordDecl(
- hasAnyName("::std::basic_string", "::std::basic_string_view",
- "::absl::string_view"));
- const auto StringNpos = declRefExpr(
- to(varDecl(hasName("npos"), hasDeclContext(StringLikeClass))));
+ const auto StringNpos = anyOf(declRefExpr(to(varDecl(hasName("npos")))),
+ memberExpr(member(hasName("npos"))));
auto AddSimpleMatcher = [&](auto Matcher) {
Finder->addMatcher(
@@ -102,7 +102,7 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) {
binaryOperator(hasLHS(Literal1), hasOperatorName(">"), hasRHS(CountCall))
.bind("negativeComparison"));
- // Find membership tests based on `find() == end() or find() == npos`.
+ // Find membership tests based on `find() == end()` or `find() == npos`.
AddSimpleMatcher(
binaryOperator(hasOperatorName("!="),
hasOperands(FindCall, anyOf(EndCall, StringNpos)))
>From 9d44ee040204e106ecaea4694949693d5f6dce15 Mon Sep 17 00:00:00 2001
From: dl8sd11 <gcchen at google.com>
Date: Sat, 5 Oct 2024 04:25:55 +0000
Subject: [PATCH 3/4] [clang-tidy] add string::contains
---
.../test/clang-tidy/checkers/Inputs/Headers/string | 7 +++++++
1 file changed, 7 insertions(+)
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 0c160bc182b6eb..b1a3f40c10f185 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;
>From 36252210a2cd6ce53ab3f25d8c7b863807f00b0d Mon Sep 17 00:00:00 2001
From: dl8sd11 <gcchen at google.com>
Date: Sat, 5 Oct 2024 04:26:25 +0000
Subject: [PATCH 4/4] [clang-tidy] adopt mock headers and add string_view tests
---
.../readability/container-contains.cpp | 62 +++++++------------
1 file changed, 22 insertions(+), 40 deletions(-)
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 69cc5c88479040..4fa2a378cff2c0 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 <string>
// Some *very* simplified versions of `map` etc.
namespace std {
@@ -29,42 +32,7 @@ struct multimap {
bool contains(const Key &K) const;
};
-using size_t = decltype(sizeof(int));
-
-// Lightweight standin for std::string_view.
-template <typename C>
-class basic_string_view {
-public:
- basic_string_view();
- basic_string_view(const basic_string_view &);
- basic_string_view(const C *);
- ~basic_string_view();
- int find(basic_string_view s, int pos = 0);
- int find(const C *s, int pos = 0);
- int find(const C *s, int pos, int n);
- int find(char c, int pos = 0);
- static constexpr size_t npos = -1;
-};
-typedef basic_string_view<char> string_view;
-
-// Lightweight standin for std::string.
-template <typename C>
-class basic_string {
-public:
- basic_string();
- basic_string(const basic_string &);
- basic_string(const C *);
- ~basic_string();
- int find(basic_string s, int pos = 0);
- int find(const C *s, int pos = 0);
- int find(const C *s, int pos, int n);
- int find(char c, int pos = 0);
- bool contains(const C *s) const;
- bool contains(C s) const;
- bool contains(basic_string_view<C> s) const;
- static constexpr size_t npos = -1;
-};
-typedef basic_string<char> string;
+
} // namespace std
@@ -491,7 +459,7 @@ void testOperandPermutations(std::map<int, int>& Map) {
// CHECK-FIXES: if (!Map.contains(0)) {};
}
-void testStringNops(std::string Str, std::string SubStr) {
+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")) {};
@@ -499,6 +467,20 @@ void testStringNops(std::string Str, std::string SubStr) {
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(SubStr) != std::string::npos) {};
+
+ 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')) {};
}
More information about the cfe-commits
mailing list