Author: Felix Date: 2023-08-21T17:47:14Z New Revision: 4001ae175cbe351d496a6cd5481a554b346f706d
URL: https://github.com/llvm/llvm-project/commit/4001ae175cbe351d496a6cd5481a554b346f706d DIFF: https://github.com/llvm/llvm-project/commit/4001ae175cbe351d496a6cd5481a554b346f706d.diff LOG: [clang-tidy] readability-container-size-empty - detect missing usage of .empty() on string_literals Detect comparison between string and empty string_literals. Fixes #64547 Reviewed By: PiotrZSL Differential Revision: https://reviews.llvm.org/D158346 Added: Modified: clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst 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 e24cb195e70cb5..0ef0389dcc99b9 100644 --- a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp @@ -94,6 +94,14 @@ AST_MATCHER(QualType, isIntegralType) { return Node->isIntegralType(Finder->getASTContext()); } +AST_MATCHER_P(UserDefinedLiteral, hasLiteral, + clang::ast_matchers::internal::Matcher<Expr>, InnerMatcher) { + if (const Expr *CookedLiteral = Node.getCookedLiteral()) { + return InnerMatcher.matches(*CookedLiteral, Finder, Builder); + } + return false; +} + } // namespace ast_matchers namespace tidy::readability { @@ -166,9 +174,11 @@ void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) { this); // Comparison to empty string or empty constructor. - const auto WrongComparend = anyOf( - stringLiteral(hasSize(0)), cxxConstructExpr(isDefaultConstruction()), - cxxUnresolvedConstructExpr(argumentCountIs(0))); + const auto WrongComparend = + anyOf(stringLiteral(hasSize(0)), + userDefinedLiteral(hasLiteral(stringLiteral(hasSize(0)))), + cxxConstructExpr(isDefaultConstruction()), + cxxUnresolvedConstructExpr(argumentCountIs(0))); // Match the object being compared. const auto STLArg = anyOf(unaryOperator( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 6b910920f6b7b6..470f06d82bf1d5 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -126,7 +126,7 @@ New checks - New :doc:`bugprone-incorrect-enable-if <clang-tidy/checks/bugprone/incorrect-enable-if>` check. - Detects incorrect usages of ``std::enable_if`` that don't name the nested + Detects incorrect usages of ``std::enable_if`` that don't name the nested ``type`` type. - New :doc:`bugprone-multi-level-implicit-pointer-conversion @@ -228,6 +228,10 @@ Changes in existing checks <clang-tidy/checks/performance/noexcept-swap>` check to enforce a stricter match with the swap function signature, eliminating false-positives. +- Improved :doc:`readability-container-size-empty + <clang-tidy/checks/readability/container-size-empty>` check to + detect comparison between string and empty string literals. + - Improved :doc:`readability-identifier-naming <clang-tidy/checks/readability/identifier-naming>` check to emit proper warnings when a type forward declaration precedes its definition. 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 ae67bf2e92eb0d..592ffc21f41a4f 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 @@ -22,6 +22,10 @@ template <typename T> struct set { }; } +namespace string_literals{ +string operator""s(const char *, size_t); +} + } template <typename T> @@ -461,7 +465,7 @@ int func(const std::vector<int> &C) { constexpr Lazy L; static_assert(!L.size(), ""); // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: the 'empty' method should be used -// CHECK-MESSAGES: :90:18: note: method 'Lazy'::empty() defined here +// CHECK-MESSAGES: :94:18: note: method 'Lazy'::empty() defined here // CHECK-FIXES: {{^}}static_assert(L.empty(), ""); struct StructWithLazyNoexcept { @@ -492,17 +496,17 @@ template <typename T> void f() { if (templated_container.size()) ; // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used - // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here + // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here // CHECK-FIXES: {{^ }}if (!templated_container.empty()){{$}} if (templated_container != TemplatedContainer<T>()) ; // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used - // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here + // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here // CHECK-FIXES: {{^ }}if (!templated_container.empty()){{$}} // CHECK-FIXES-NEXT: ; CHECKSIZE(templated_container); // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: the 'empty' method should be used - // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here + // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here // CHECK-FIXES: CHECKSIZE(templated_container); } @@ -575,74 +579,74 @@ bool neverInstantiatedTemplate() { if (templated_container.size()) ; // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used - // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here + // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here // CHECK-FIXES: {{^ }}if (!templated_container.empty()){{$}} if (templated_container != TemplatedContainer<T>()) ; // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used - // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here + // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here // CHECK-FIXES: {{^ }}if (!templated_container.empty()){{$}} // CHECK-FIXES-NEXT: ; while (templated_container.size()) ; // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: the 'empty' method should be used - // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here + // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here // CHECK-FIXES: {{^ }}while (!templated_container.empty()){{$}} do { } while (templated_container.size()); // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used - // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here + // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here // CHECK-FIXES: {{^ }}while (!templated_container.empty()); for (; templated_container.size();) ; // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: the 'empty' method should be used - // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here + // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here // CHECK-FIXES: {{^ }}for (; !templated_container.empty();){{$}} if (true && templated_container.size()) ; // CHECK-MESSAGES: :[[@LINE-2]]:15: warning: the 'empty' method should be used - // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here + // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here // CHECK-FIXES: {{^ }}if (true && !templated_container.empty()){{$}} if (true || templated_container.size()) ; // CHECK-MESSAGES: :[[@LINE-2]]:15: warning: the 'empty' method should be used - // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here + // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here // CHECK-FIXES: {{^ }}if (true || !templated_container.empty()){{$}} if (!templated_container.size()) ; // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: the 'empty' method should be used - // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here + // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here // CHECK-FIXES: {{^ }}if (templated_container.empty()){{$}} bool b1 = templated_container.size(); // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: the 'empty' method should be used - // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here + // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here // CHECK-FIXES: {{^ }}bool b1 = !templated_container.empty(); bool b2(templated_container.size()); // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the 'empty' method should be used - // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here + // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here // CHECK-FIXES: {{^ }}bool b2(!templated_container.empty()); auto b3 = static_cast<bool>(templated_container.size()); // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: the 'empty' method should be used - // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here + // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here // CHECK-FIXES: {{^ }}auto b3 = static_cast<bool>(!templated_container.empty()); auto b4 = (bool)templated_container.size(); // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: the 'empty' method should be used - // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here + // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here // CHECK-FIXES: {{^ }}auto b4 = (bool)!templated_container.empty(); auto b5 = bool(templated_container.size()); // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: the 'empty' method should be used - // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here + // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here // CHECK-FIXES: {{^ }}auto b5 = bool(!templated_container.empty()); takesBool(templated_container.size()); @@ -651,7 +655,7 @@ bool neverInstantiatedTemplate() { return templated_container.size(); // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used - // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here + // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here // CHECK-FIXES: {{^ }}return !templated_container.empty(); } @@ -753,7 +757,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: :731:8: note: method 'array'::empty() defined here +// CHECK-MESSAGES: :735:8: note: method 'array'::empty() defined here } bool testArrayCompareToEmpty(const Array& value) { @@ -764,9 +768,23 @@ 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: :741:8: note: method 'DummyType'::empty() defined here +// CHECK-MESSAGES: :745:8: note: method 'DummyType'::empty() defined here } bool testIgnoredDummyType(const IgnoredDummyType& value) { return value == IgnoredDummyType(); } + +bool testStringLiterals(const std::string& s) +{ + using namespace std::string_literals; + return s == ""s; + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used + // CHECK-FIXES: {{^ }}return s.empty() +} + +bool testNotEmptyStringLiterals(const std::string& s) +{ + using namespace std::string_literals; + return s == "foo"s; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits