[PATCH] D158346: [clang-tidy] [readability-container-size-empty] improved check to detect missing usage of .empty() on string_literals Fixes #64547

Félix-Antoine Constantin via Phabricator via cfe-commits Sat, 19 Aug 2023 13:31:30 -0700

felix642 updated this revision to Diff 551785.
felix642 added a comment.

Fixed tests and addressed comments.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158346/new/

https://reviews.llvm.org/D158346

Files:
  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

Index: clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
@@ -22,6 +22,10 @@
 };
 }
 
+namespace string_literals{
+string operator""s(const char *, size_t);
+}
+
 }
 
 template <typename T>
@@ -461,7 +465,7 @@
 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 @@
   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 @@
   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 @@
 
   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 @@
   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 @@
   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;
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -222,6 +222,10 @@
   <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.
Index: clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
@@ -94,6 +94,13 @@
   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 +173,11 @@
       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(

_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158346: [clang-tidy] [readability-container-size-empty] improved check to detect missing usage of .empty() on string_literals Fixes #64547

Reply via email to