[clang-tools-extra] efebb4e - [clang-tidy] readability-container-size-empty handle std::string length()
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 31 12:57:10 PDT 2023
Author: Piotr Zegar
Date: 2023-08-31T19:56:07Z
New Revision: efebb4e0fabe91b70eaf58c049e516e50c726893
URL: https://github.com/llvm/llvm-project/commit/efebb4e0fabe91b70eaf58c049e516e50c726893
DIFF: https://github.com/llvm/llvm-project/commit/efebb4e0fabe91b70eaf58c049e516e50c726893.diff
LOG: [clang-tidy] readability-container-size-empty handle std::string length()
Extends readability-container-size-empty to check std::string length() similar to size().
Fixes: #37603
Co-authored-by: Dmitry Venikov <quolyk at gmail.com>
Reviewed By: carlosgalvezp
Differential Revision: https://reviews.llvm.org/D56644
Added:
Modified:
clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.rst
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
index 0ef0389dcc99b9..f0357fb49eff33 100644
--- a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
@@ -120,14 +120,14 @@ void ContainerSizeEmptyCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) {
const auto ValidContainerRecord = cxxRecordDecl(isSameOrDerivedFrom(
- namedDecl(
- has(cxxMethodDecl(
- isConst(), parameterCountIs(0), isPublic(), hasName("size"),
- returns(qualType(isIntegralType(), unless(booleanType()))))
- .bind("size")),
- has(cxxMethodDecl(isConst(), parameterCountIs(0), isPublic(),
- hasName("empty"), returns(booleanType()))
- .bind("empty")))
+ namedDecl(has(cxxMethodDecl(isConst(), parameterCountIs(0), isPublic(),
+ hasAnyName("size", "length"),
+ returns(qualType(isIntegralType(),
+ unless(booleanType()))))
+ .bind("size")),
+ has(cxxMethodDecl(isConst(), parameterCountIs(0), isPublic(),
+ hasName("empty"), returns(booleanType()))
+ .bind("empty")))
.bind("container")));
const auto ValidContainerNonTemplateType =
@@ -149,24 +149,28 @@ void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) {
usedInBooleanContext());
Finder->addMatcher(
- cxxMemberCallExpr(on(expr(anyOf(hasType(ValidContainer),
- hasType(pointsTo(ValidContainer)),
- hasType(references(ValidContainer))))
- .bind("MemberCallObject")),
- callee(cxxMethodDecl(hasName("size"))), WrongUse,
- unless(hasAncestor(cxxMethodDecl(
- ofClass(equalsBoundNode("container"))))))
+ cxxMemberCallExpr(
+ on(expr(anyOf(hasType(ValidContainer),
+ hasType(pointsTo(ValidContainer)),
+ hasType(references(ValidContainer))))
+ .bind("MemberCallObject")),
+ callee(
+ cxxMethodDecl(hasAnyName("size", "length")).bind("SizeMethod")),
+ WrongUse,
+ unless(hasAncestor(
+ cxxMethodDecl(ofClass(equalsBoundNode("container"))))))
.bind("SizeCallExpr"),
this);
Finder->addMatcher(
callExpr(has(cxxDependentScopeMemberExpr(
- hasObjectExpression(
- expr(anyOf(hasType(ValidContainer),
- hasType(pointsTo(ValidContainer)),
- hasType(references(ValidContainer))))
- .bind("MemberCallObject")),
- hasMemberName("size"))),
+ hasObjectExpression(
+ expr(anyOf(hasType(ValidContainer),
+ hasType(pointsTo(ValidContainer)),
+ hasType(references(ValidContainer))))
+ .bind("MemberCallObject")),
+ anyOf(hasMemberName("size"), hasMemberName("length")))
+ .bind("DependentExpr")),
WrongUse,
unless(hasAncestor(
cxxMethodDecl(ofClass(equalsBoundNode("container"))))))
@@ -333,9 +337,18 @@ void ContainerSizeEmptyCheck::check(const MatchFinder::MatchResult &Result) {
auto WarnLoc = MemberCall ? MemberCall->getBeginLoc() : SourceLocation{};
if (WarnLoc.isValid()) {
- diag(WarnLoc, "the 'empty' method should be used to check "
- "for emptiness instead of 'size'")
- << Hint;
+ auto Diag = diag(WarnLoc, "the 'empty' method should be used to check "
+ "for emptiness instead of %0");
+ if (const auto *SizeMethod =
+ Result.Nodes.getNodeAs<NamedDecl>("SizeMethod"))
+ Diag << SizeMethod;
+ else if (const auto *DependentExpr =
+ Result.Nodes.getNodeAs<CXXDependentScopeMemberExpr>(
+ "DependentExpr"))
+ Diag << DependentExpr->getMember();
+ else
+ Diag << "unknown method";
+ Diag << Hint;
} else {
WarnLoc = BinCmpTempl
? BinCmpTempl->getBeginLoc()
diff --git a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h
index a9a65d0185393b..617dadce76bd3e 100644
--- a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h
@@ -14,8 +14,8 @@
namespace clang::tidy::readability {
-/// Checks whether a call to the `size()` method can be replaced with a call to
-/// `empty()`.
+/// Checks whether a call to the `size()`/`length()` method can be replaced with
+/// a call to `empty()`.
///
/// The emptiness of a container should be checked using the `empty()` method
/// instead of the `size()` method. It is not guaranteed that `size()` is a
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 95ee6daf7209e5..69bc981a7931ea 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -248,7 +248,8 @@ Changes in existing checks
- Improved :doc:`readability-container-size-empty
<clang-tidy/checks/readability/container-size-empty>` check to
- detect comparison between string and empty string literals.
+ detect comparison between string and empty string literals and support
+ ``length()`` method as an alternative to ``size()``.
- Improved :doc:`readability-identifier-naming
<clang-tidy/checks/readability/identifier-naming>` check to emit proper
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.rst
index 2efed226ead39a..b5abd340b9b2b3 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.rst
@@ -4,23 +4,24 @@ readability-container-size-empty
================================
-Checks whether a call to the ``size()`` method can be replaced with a call to
-``empty()``.
+Checks whether a call to the ``size()``/``length()`` method can be replaced
+with a call to ``empty()``.
The emptiness of a container should be checked using the ``empty()`` method
-instead of the ``size()`` method. It is not guaranteed that ``size()`` is a
-constant-time function, and it is generally more efficient and also shows
-clearer intent to use ``empty()``. Furthermore some containers may implement
-the ``empty()`` method but not implement the ``size()`` method. Using
-``empty()`` whenever possible makes it easier to switch to another container in
-the future.
+instead of the ``size()``/``length()`` method. It is not guaranteed that
+``size()``/``length()`` is a constant-time function, and it is generally more
+efficient and also shows clearer intent to use ``empty()``. Furthermore some
+containers may implement the ``empty()`` method but not implement the ``size()``
+or ``length()`` method. Using ``empty()`` whenever possible makes it easier to
+switch to another container in the future.
-The check issues warning if a container has ``size()`` and ``empty()`` methods
-matching following signatures:
+The check issues warning if a container has ``empty()`` and ``size()`` or
+``length()`` methods matching following signatures:
.. code-block:: c++
size_type size() const;
+ size_type length() const;
bool empty() const;
`size_type` can be any kind of integer type.
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 80b41da61c2e36..6f569655e6762a 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
@@ -27,6 +27,7 @@ struct basic_string {
bool empty() const;
size_type size() const;
+ size_type length() const;
_Type& append(const C *s);
_Type& append(const C *s, size_type n);
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
index 6f1c014feccb58..29ac86cf1b369c 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
@@ -109,9 +109,13 @@ bool returnsBool() {
std::string str2;
std::wstring wstr;
(void)(str.size() + 0);
+ (void)(str.length() + 0);
(void)(str.size() - 0);
+ (void)(str.length() - 0);
(void)(0 + str.size());
+ (void)(0 + str.length());
(void)(0 - str.size());
+ (void)(0 - str.length());
if (intSet.size() == 0)
;
// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
@@ -128,11 +132,19 @@ bool returnsBool() {
// CHECK-FIXES: {{^ }}if (s_func().empty()){{$}}
if (str.size() == 0)
;
- // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size'
+ // CHECK-FIXES: {{^ }}if (str.empty()){{$}}
+ if (str.length() == 0)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'length'
// CHECK-FIXES: {{^ }}if (str.empty()){{$}}
if ((str + str2).size() == 0)
;
- // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size'
+ // CHECK-FIXES: {{^ }}if ((str + str2).empty()){{$}}
+ if ((str + str2).length() == 0)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'length'
// CHECK-FIXES: {{^ }}if ((str + str2).empty()){{$}}
if (str == "")
;
@@ -144,7 +156,11 @@ bool returnsBool() {
// CHECK-FIXES: {{^ }}if ((str + str2).empty()){{$}}
if (wstr.size() == 0)
;
- // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size'
+ // CHECK-FIXES: {{^ }}if (wstr.empty()){{$}}
+ if (wstr.length() == 0)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'length'
// CHECK-FIXES: {{^ }}if (wstr.empty()){{$}}
if (wstr == L"")
;
@@ -153,7 +169,7 @@ bool returnsBool() {
std::vector<int> vect;
if (vect.size() == 0)
;
- // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size'
// CHECK-FIXES: {{^ }}if (vect.empty()){{$}}
if (vect == std::vector<int>())
;
@@ -508,6 +524,17 @@ template <typename T> void f() {
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: the 'empty' method should be used
// CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here
// CHECK-FIXES: CHECKSIZE(templated_container);
+ std::basic_string<T> s;
+ if (s.size())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
+ // CHECK-MESSAGES: string:28:8: note: method 'basic_string'::empty() defined here
+ // CHECK-FIXES: {{^ }}if (!s.empty()){{$}}
+ if (s.length())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'length' [readability-container-size-empty]
+ // CHECK-MESSAGES: string:28:8: note: method 'basic_string'::empty() defined here
+ // CHECK-FIXES: {{^ }}if (!s.empty()){{$}}
}
void g() {
@@ -757,7 +784,7 @@ bool testArraySize(const Array& value) {
return value.size() == 0U;
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
// CHECK-FIXES: {{^ }}return value.empty();{{$}}
-// CHECK-MESSAGES: :735:8: note: method 'array'::empty() defined here
+// CHECK-MESSAGES: :[[@LINE-25]]:8: note: method 'array'::empty() defined here
}
bool testArrayCompareToEmpty(const Array& value) {
@@ -768,7 +795,7 @@ bool testDummyType(const DummyType& value) {
return value == DummyType();
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used to check for emptiness instead of comparing to an empty object [readability-container-size-empty]
// CHECK-FIXES: {{^ }}return value.empty();{{$}}
-// CHECK-MESSAGES: :745:8: note: method 'DummyType'::empty() defined here
+// CHECK-MESSAGES: :[[@LINE-26]]:8: note: method 'DummyType'::empty() defined here
}
bool testIgnoredDummyType(const IgnoredDummyType& value) {
More information about the cfe-commits
mailing list