[clang-tidy] readability-container-size-empty fixes

[xazax.hun]

Summary

This patch fixes PR27410 and adds std::basic_string support.

[hokein]

Thanks! LGTM with one nit.

test/clang-tidy/readability-container-size-empty.cpp

44

You don't need to check the whole message (we only check the whole message at first place), just keep CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used.

48

The same.

clang-tidy/readability/ContainerSizeEmptyCheck.cpp

 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/StringRef.h"
 
 using namespace clang::ast_matchers;
 
 static bool isContainerName(llvm::StringRef ClassName) {
   static const char *const ContainerNames[] = {"array",
+                                               "basic_string",
                                                "deque",
                                                "forward_list",
                                                "list",
                                                "map",
                                                "multimap",
                                                "multiset",
                                                "priority_queue",
                                                "queue",

 
 void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matchers for C++; the functionality currently does not
   // provide any benefit to other languages, despite being benign.
   if (!getLangOpts().CPlusPlus)
     return;
 
   const auto WrongUse = anyOf(
-      hasParent(
-          binaryOperator(
-              anyOf(has(integerLiteral(equals(0))),
-                    allOf(anyOf(hasOperatorName("<"), hasOperatorName(">="),
-                                hasOperatorName(">"), hasOperatorName("<=")),
-                          hasEitherOperand(
-                              ignoringImpCasts(integerLiteral(equals(1)))))))
+      hasParent(binaryOperator(
+                    anyOf(hasOperatorName("<"), hasOperatorName(">="),
+                          hasOperatorName(">"), hasOperatorName("<="),
+                          hasOperatorName("=="), hasOperatorName("!=")),
+                    hasEitherOperand(ignoringImpCasts(anyOf(
+                        integerLiteral(equals(1)), integerLiteral(equals(0))))))
                     .bind("SizeBinaryOp")),
       hasParent(implicitCastExpr(
           hasImplicitDestinationType(booleanType()),
           anyOf(
               hasParent(unaryOperator(hasOperatorName("!")).bind("NegOnSize")),
               anything()))),
       hasParent(explicitCastExpr(hasDestinationType(booleanType()))));
 
   Finder->addMatcher(
if (ContainerIsLHS) {
               ->getValue()
               .getLimitedValue();
     }
 
     // Constant that is not handled.
     if (Value > 1)
       return;
 
+    if (Value == 1 && (OpCode == BinaryOperatorKind::BO_EQ ||
+                       OpCode == BinaryOperatorKind::BO_NE))
+      return;
+
     // Always true, no warnings for that.
     if ((OpCode == BinaryOperatorKind::BO_GE && Value == 0 && ContainerIsLHS) ||
         (OpCode == BinaryOperatorKind::BO_LE && Value == 0 && !ContainerIsLHS))
       return;
 
     // Do not warn for size > 1, 1 < size, size <= 1, 1 >= size.
     if (Value == 1) {
       if ((OpCode == BinaryOperatorKind::BO_GT && ContainerIsLHS) ||

test/clang-tidy/readability-container-size-empty.cpp

 // RUN: %check_clang_tidy %s readability-container-size-empty %t
 
 namespace std {
 template <typename T> struct vector {
   vector();
   unsigned long size() const;
   bool empty() const;
 };
 
+template <typename T> struct basic_string {
+  basic_string();
+  unsigned long size() const;
+  bool empty() const;
+};
+
+typedef basic_string<char> string;
+typedef basic_string<wchar_t> wstring;
+
 inline namespace __v2 {
 template <typename T> struct set {
   set();
   unsigned long size() const;
   bool empty() const;
 };
 }
 
 
 }
 
 int main() {
   std::set<int> intSet;
+  std::string str;
+  std::wstring wstr;
+  str.size() + 0;
+  str.size() - 0;
+  0 + str.size();
+  0 - str.size();
   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]
   // CHECK-FIXES: {{^  }}if (intSet.empty()){{$}}
+  if (str.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]
+  // CHECK-FIXES: {{^  }}if (str.empty()){{$}}
+  if (wstr.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]
+  // CHECK-FIXES: {{^  }}if (wstr.empty()){{$}}
   std::vector<int> vect;
   if (vect.size() == 0)
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}if (vect.empty()){{$}}
   if (vect.size() != 0)
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used