diff --git a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h --- 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/clang-tidy/readability/ContainerSizeEmptyCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp --- a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp @@ -120,14 +120,14 @@ 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 @@ 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 @@ 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("SizeMethod")) + Diag << SizeMethod; + else if (const auto *DependentExpr = + Result.Nodes.getNodeAs( + "DependentExpr")) + Diag << DependentExpr->getMember(); + else + Diag << "unknown method"; + Diag << Hint; } else { WarnLoc = BinCmpTempl ? BinCmpTempl->getBeginLoc() diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -248,7 +248,8 @@ - Improved :doc:`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 ` 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 --- 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 @@ ================================ -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 --- 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 @@ 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 --- 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 @@ 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 @@ // 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 @@ // 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 @@ std::vector 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()) ; @@ -508,6 +524,17 @@ // 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 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 @@ 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 @@ 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) {