diff --git a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp index 0ef0389dcc99b..f0357fb49eff3 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("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/clang-tidy/readability/ContainerSizeEmptyCheck.h b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h index a9a65d0185393..617dadce76bd3 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 95ee6daf7209e..69bc981a7931e 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 ` 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 index 2efed226ead39..b5abd340b9b2b 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 80b41da61c2e3..6f569655e6762 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 6f1c014feccb5..29ac86cf1b369 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 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 @@ template 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 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) {