[clang-tidy] Handle template instantiations in container size check
authorStephen Kelly <steveire@gmail.com>
Tue, 27 Oct 2020 23:24:20 +0000 (23:24 +0000)
committerStephen Kelly <steveire@gmail.com>
Tue, 22 Dec 2020 18:44:45 +0000 (18:44 +0000)
readability-container-size-empty currently modifies source code based on
AST nodes in template instantiations, which means that it makes
transformations based on substituted types.  This can lead to
transforming code to be broken.

Change the matcher implementation to ignore template instantiations
explicitly, and add a matcher to explicitly handle template declarations
instead of instantiations.

Differential Revision: https://reviews.llvm.org/D91302

clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/readability-container-size-empty.cpp

index 14a38c0..e7c5f0a 100644 (file)
 using namespace clang::ast_matchers;
 
 namespace clang {
+namespace ast_matchers {
+AST_POLYMORPHIC_MATCHER_P2(hasAnyArgumentWithParam,
+                           AST_POLYMORPHIC_SUPPORTED_TYPES(CallExpr,
+                                                           CXXConstructExpr),
+                           internal::Matcher<Expr>, ArgMatcher,
+                           internal::Matcher<ParmVarDecl>, ParamMatcher) {
+  BoundNodesTreeBuilder Result;
+  // The first argument of an overloaded member operator is the implicit object
+  // argument of the method which should not be matched against a parameter, so
+  // we skip over it here.
+  BoundNodesTreeBuilder Matches;
+  unsigned ArgIndex = cxxOperatorCallExpr(callee(cxxMethodDecl()))
+                              .matches(Node, Finder, &Matches)
+                          ? 1
+                          : 0;
+  int ParamIndex = 0;
+  for (; ArgIndex < Node.getNumArgs(); ++ArgIndex) {
+    BoundNodesTreeBuilder ArgMatches(*Builder);
+    if (ArgMatcher.matches(*(Node.getArg(ArgIndex)->IgnoreParenCasts()), Finder,
+                           &ArgMatches)) {
+      BoundNodesTreeBuilder ParamMatches(ArgMatches);
+      if (expr(anyOf(cxxConstructExpr(hasDeclaration(cxxConstructorDecl(
+                         hasParameter(ParamIndex, ParamMatcher)))),
+                     callExpr(callee(functionDecl(
+                         hasParameter(ParamIndex, ParamMatcher))))))
+              .matches(Node, Finder, &ParamMatches)) {
+        Result.addMatch(ParamMatches);
+        *Builder = std::move(Result);
+        return true;
+      }
+    }
+    ++ParamIndex;
+  }
+  return false;
+}
+
+AST_MATCHER(Expr, usedInBooleanContext) {
+  const char *ExprName = "__booleanContextExpr";
+  auto Result =
+      expr(expr().bind(ExprName),
+           anyOf(hasParent(varDecl(hasType(booleanType()))),
+                 hasParent(cxxConstructorDecl(
+                     hasAnyConstructorInitializer(cxxCtorInitializer(
+                         withInitializer(expr(equalsBoundNode(ExprName))),
+                         forField(hasType(booleanType())))))),
+                 hasParent(fieldDecl(hasType(booleanType()))),
+                 hasParent(stmt(anyOf(
+                     explicitCastExpr(hasDestinationType(booleanType())),
+                     ifStmt(hasCondition(expr(equalsBoundNode(ExprName)))),
+                     doStmt(hasCondition(expr(equalsBoundNode(ExprName)))),
+                     whileStmt(hasCondition(expr(equalsBoundNode(ExprName)))),
+                     forStmt(hasCondition(expr(equalsBoundNode(ExprName)))),
+                     conditionalOperator(
+                         hasCondition(expr(equalsBoundNode(ExprName)))),
+                     parenListExpr(hasParent(varDecl(hasType(booleanType())))),
+                     parenExpr(hasParent(
+                         explicitCastExpr(hasDestinationType(booleanType())))),
+                     returnStmt(forFunction(returns(booleanType()))),
+                     cxxUnresolvedConstructExpr(hasType(booleanType())),
+                     callExpr(hasAnyArgumentWithParam(
+                         expr(equalsBoundNode(ExprName)),
+                         parmVarDecl(hasType(booleanType())))),
+                     binaryOperator(hasAnyOperatorName("&&", "||")),
+                     unaryOperator(hasOperatorName("!")).bind("NegOnSize"))))))
+          .matches(Node, Finder, Builder);
+  Builder->removeBindings([ExprName](const BoundNodesMap &Nodes) {
+    return Nodes.getNode(ExprName).getNodeKind().isNone();
+  });
+  return Result;
+}
+} // namespace ast_matchers
 namespace tidy {
 namespace readability {
 
@@ -26,18 +97,27 @@ ContainerSizeEmptyCheck::ContainerSizeEmptyCheck(StringRef Name,
     : ClangTidyCheck(Name, Context) {}
 
 void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) {
-  const auto ValidContainer = qualType(hasUnqualifiedDesugaredType(
-      recordType(hasDeclaration(cxxRecordDecl(isSameOrDerivedFrom(
-          namedDecl(
-              has(cxxMethodDecl(
-                      isConst(), parameterCountIs(0), isPublic(),
-                      hasName("size"),
-                      returns(qualType(isInteger(), unless(booleanType()))))
-                      .bind("size")),
-              has(cxxMethodDecl(isConst(), parameterCountIs(0), isPublic(),
-                                hasName("empty"), returns(booleanType()))
-                      .bind("empty")))
-              .bind("container")))))));
+  const auto ValidContainerRecord = cxxRecordDecl(isSameOrDerivedFrom(
+      namedDecl(
+          has(cxxMethodDecl(isConst(), parameterCountIs(0), isPublic(),
+                            hasName("size"),
+                            returns(qualType(isInteger(), unless(booleanType()),
+                                             unless(elaboratedType()))))
+                  .bind("size")),
+          has(cxxMethodDecl(isConst(), parameterCountIs(0), isPublic(),
+                            hasName("empty"), returns(booleanType()))
+                  .bind("empty")))
+          .bind("container")));
+
+  const auto ValidContainerNonTemplateType =
+      qualType(hasUnqualifiedDesugaredType(
+          recordType(hasDeclaration(ValidContainerRecord))));
+  const auto ValidContainerTemplateType =
+      qualType(hasUnqualifiedDesugaredType(templateSpecializationType(
+          hasDeclaration(classTemplateDecl(has(ValidContainerRecord))))));
+
+  const auto ValidContainer = qualType(
+      anyOf(ValidContainerNonTemplateType, ValidContainerTemplateType));
 
   const auto WrongUse = traverse(
       TK_AsIs,
@@ -52,18 +132,34 @@ void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) {
               anyOf(hasParent(
                         unaryOperator(hasOperatorName("!")).bind("NegOnSize")),
                     anything()))),
-          hasParent(explicitCastExpr(hasDestinationType(booleanType())))));
+          usedInBooleanContext()));
 
   Finder->addMatcher(
-      cxxMemberCallExpr(on(expr(anyOf(hasType(ValidContainer),
+      cxxMemberCallExpr(unless(isInTemplateInstantiation()),
+                        on(expr(anyOf(hasType(ValidContainer),
                                       hasType(pointsTo(ValidContainer)),
-                                      hasType(references(ValidContainer))))),
+                                      hasType(references(ValidContainer))))
+                               .bind("MemberCallObject")),
                         callee(cxxMethodDecl(hasName("size"))), 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"))),
+               WrongUse,
+               unless(hasAncestor(
+                   cxxMethodDecl(ofClass(equalsBoundNode("container"))))))
+          .bind("SizeCallExpr"),
+      this);
+
   // Empty constructor matcher.
   const auto DefaultConstructor = cxxConstructExpr(
           hasDeclaration(cxxConstructorDecl(isDefaultConstructor())));
@@ -72,12 +168,11 @@ void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) {
       ignoringImpCasts(stringLiteral(hasSize(0))),
       ignoringImpCasts(cxxBindTemporaryExpr(has(DefaultConstructor))),
       ignoringImplicit(DefaultConstructor),
-      cxxConstructExpr(
-          hasDeclaration(cxxConstructorDecl(isCopyConstructor())),
-          has(expr(ignoringImpCasts(DefaultConstructor)))),
-      cxxConstructExpr(
-          hasDeclaration(cxxConstructorDecl(isMoveConstructor())),
-          has(expr(ignoringImpCasts(DefaultConstructor)))));
+      cxxConstructExpr(hasDeclaration(cxxConstructorDecl(isCopyConstructor())),
+                       has(expr(ignoringImpCasts(DefaultConstructor)))),
+      cxxConstructExpr(hasDeclaration(cxxConstructorDecl(isMoveConstructor())),
+                       has(expr(ignoringImpCasts(DefaultConstructor)))),
+      cxxUnresolvedConstructExpr(argumentCountIs(0)));
   // Match the object being compared.
   const auto STLArg =
       anyOf(unaryOperator(
@@ -87,6 +182,7 @@ void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) {
             expr(hasType(ValidContainer)).bind("STLObject"));
   Finder->addMatcher(
       cxxOperatorCallExpr(
+          unless(isInTemplateInstantiation()),
           hasAnyOverloadedOperatorName("==", "!="),
           anyOf(allOf(hasArgument(0, WrongComparend), hasArgument(1, STLArg)),
                 allOf(hasArgument(0, STLArg), hasArgument(1, WrongComparend))),
@@ -94,24 +190,33 @@ void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) {
               cxxMethodDecl(ofClass(equalsBoundNode("container"))))))
           .bind("BinCmp"),
       this);
+  Finder->addMatcher(
+      binaryOperator(hasAnyOperatorName("==", "!="),
+                     anyOf(allOf(hasLHS(WrongComparend), hasRHS(STLArg)),
+                           allOf(hasLHS(STLArg), hasRHS(WrongComparend))),
+                     unless(hasAncestor(
+                         cxxMethodDecl(ofClass(equalsBoundNode("container"))))))
+          .bind("BinCmp"),
+      this);
 }
 
 void ContainerSizeEmptyCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *MemberCall =
-      Result.Nodes.getNodeAs<CXXMemberCallExpr>("SizeCallExpr");
+  const auto *MemberCall = Result.Nodes.getNodeAs<Expr>("SizeCallExpr");
+  const auto *MemberCallObject =
+      Result.Nodes.getNodeAs<Expr>("MemberCallObject");
   const auto *BinCmp = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("BinCmp");
+  const auto *BinCmpTempl = Result.Nodes.getNodeAs<BinaryOperator>("BinCmp");
   const auto *BinaryOp = Result.Nodes.getNodeAs<BinaryOperator>("SizeBinaryOp");
   const auto *Pointee = Result.Nodes.getNodeAs<Expr>("Pointee");
   const auto *E =
-      MemberCall
-          ? MemberCall->getImplicitObjectArgument()
+      MemberCallObject
+          ? MemberCallObject
           : (Pointee ? Pointee : Result.Nodes.getNodeAs<Expr>("STLObject"));
   FixItHint Hint;
   std::string ReplacementText = std::string(
       Lexer::getSourceText(CharSourceRange::getTokenRange(E->getSourceRange()),
                            *Result.SourceManager, getLangOpts()));
-  if (BinCmp && IsBinaryOrTernary(E)) {
-    // Not just a DeclRefExpr, so parenthesize to be on the safe side.
+  if (IsBinaryOrTernary(E) || isa<UnaryOperator>(E)) {
     ReplacementText = "(" + ReplacementText + ")";
   }
   if (E->getType()->isPointerType())
@@ -125,7 +230,13 @@ void ContainerSizeEmptyCheck::check(const MatchFinder::MatchResult &Result) {
     }
     Hint =
         FixItHint::CreateReplacement(BinCmp->getSourceRange(), ReplacementText);
-  } else if (BinaryOp) {  // Determine the correct transformation.
+  } else if (BinCmpTempl) {
+    if (BinCmpTempl->getOpcode() == BinaryOperatorKind::BO_NE) {
+      ReplacementText = "!" + ReplacementText;
+    }
+    Hint = FixItHint::CreateReplacement(BinCmpTempl->getSourceRange(),
+                                        ReplacementText);
+  } else if (BinaryOp) { // Determine the correct transformation.
     bool Negation = false;
     const bool ContainerIsLHS =
         !llvm::isa<IntegerLiteral>(BinaryOp->getLHS()->IgnoreImpCasts());
@@ -195,15 +306,17 @@ void ContainerSizeEmptyCheck::check(const MatchFinder::MatchResult &Result) {
                                           "!" + ReplacementText);
   }
 
-  if (MemberCall) {
-    diag(MemberCall->getBeginLoc(),
-         "the 'empty' method should be used to check "
-         "for emptiness instead of 'size'")
+  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;
   } else {
-    diag(BinCmp->getBeginLoc(),
-         "the 'empty' method should be used to check "
-         "for emptiness instead of comparing to an empty object")
+    WarnLoc = BinCmpTempl ? BinCmpTempl->getBeginLoc()
+                          : (BinCmp ? BinCmp->getBeginLoc() : SourceLocation{});
+    diag(WarnLoc, "the 'empty' method should be used to check "
+                  "for emptiness instead of comparing to an empty object")
         << Hint;
   }
 
index 9100559..e730b1b 100644 (file)
@@ -96,6 +96,11 @@ public:
   bool empty() const { return *this == Container4(); }
 };
 
+struct Lazy {
+  constexpr unsigned size() const { return 0; }
+  constexpr bool empty() const { return true; }
+};
+
 std::string s_func() {
   return std::string();
 }
@@ -440,6 +445,43 @@ bool returnsBool() {
   // CHECK-FIXES: {{^  }}return !derived.empty();
 }
 
+class ConstructWithBoolField {
+  bool B;
+public:
+  ConstructWithBoolField(const std::vector<int> &C) : B(C.size()) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:57: warning: the 'empty' method should be used
+// CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+// CHECK-FIXES: {{^  }}ConstructWithBoolField(const std::vector<int> &C) : B(!C.empty()) {}
+};
+
+struct StructWithNSDMI {
+  std::vector<int> C;
+  bool B = C.size();
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: the 'empty' method should be used
+// CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+// CHECK-FIXES: {{^  }}bool B = !C.empty();
+};
+
+int func(const std::vector<int> &C) {
+  return C.size() ? 0 : 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used
+// CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+// CHECK-FIXES: {{^  }}return !C.empty() ? 0 : 1;
+}
+
+constexpr Lazy L;
+static_assert(!L.size(), "");
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: the 'empty' method should be used
+// CHECK-MESSAGES: :101:18: note: method 'Lazy'::empty() defined here
+// CHECK-FIXES: {{^}}static_assert(L.empty(), "");
+
+struct StructWithLazyNoexcept {
+  void func() noexcept(L.size());
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: the 'empty' method should be used
+// CHECK-MESSAGES: :101:18: note: method 'Lazy'::empty() defined here
+// CHECK-FIXES: {{^  }}void func() noexcept(!L.empty());
+};
+
 #define CHECKSIZE(x) if (x.size()) {}
 // CHECK-FIXES: #define CHECKSIZE(x) if (x.size()) {}
 
@@ -483,3 +525,177 @@ void g() {
   f<double>();
   f<char *>();
 }
+
+template <typename T>
+bool neverInstantiatedTemplate() {
+  std::vector<T> v;
+  if (v.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: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-FIXES: {{^  }}if (!v.empty()){{$}}
+
+  if (v == std::vector<T>())
+    ;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of comparing to an empty object [readability-container-size-empty]
+  // CHECK-FIXES: {{^  }}if (v.empty()){{$}}
+  // CHECK-FIXES-NEXT: ;
+  if (v.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-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-FIXES: {{^  }}if (v.empty()){{$}}
+  if (v.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-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-FIXES: {{^  }}if (!v.empty()){{$}}
+  if (v.size() < 1)
+    ;
+  // 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: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-FIXES: {{^  }}if (v.empty()){{$}}
+  if (v.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-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-FIXES: {{^  }}if (!v.empty()){{$}}
+  if (v.size() == 1)
+    ;
+  if (v.size() != 1)
+    ;
+  if (v.size() == 2)
+    ;
+  if (v.size() != 2)
+    ;
+
+  if (static_cast<bool>(v.size()))
+    ;
+  // CHECK-MESSAGES: :[[@LINE-2]]:25: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
+  // CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-FIXES: {{^  }}if (static_cast<bool>(!v.empty())){{$}}
+  if (v.size() && false)
+    ;
+  // 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: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-FIXES: {{^  }}if (!v.empty() && false){{$}}
+  if (!v.size())
+    ;
+  // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
+  // CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-FIXES: {{^  }}if (v.empty()){{$}}
+
+  TemplatedContainer<T> templated_container;
+  if (templated_container.size())
+    ;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-MESSAGES: :44: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: :44: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: :44: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: :44: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: :44: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: :44: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: :44: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: :44: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: :44: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: :44: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: :44: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: :44: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: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-FIXES: {{^  }}auto b5 = bool(!templated_container.empty());
+
+  takesBool(templated_container.size());
+  // We don't detect this one because we don't know the parameter of takesBool
+  // until the type of templated_container.size() is known
+
+  return templated_container.size();
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used
+  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-FIXES: {{^  }}return !templated_container.empty();
+}
+
+template <typename TypeRequiresSize>
+void instantiatedTemplateWithSizeCall() {
+  TypeRequiresSize t;
+  // The instantiation of the template with std::vector<int> should not
+  // result in changing the template, because we don't know that
+  // TypeRequiresSize generally has `.empty()`
+  if (t.size())
+    ;
+
+  if (t == TypeRequiresSize{})
+    ;
+
+  if (t != TypeRequiresSize{})
+    ;
+}
+
+class TypeWithSize {
+public:
+  TypeWithSize();
+  bool operator==(const TypeWithSize &other) const;
+  bool operator!=(const TypeWithSize &other) const;
+
+  unsigned size() const { return 0; }
+  // Does not have `.empty()`
+};
+
+void instantiator() {
+  instantiatedTemplateWithSizeCall<TypeWithSize>();
+  instantiatedTemplateWithSizeCall<std::vector<int>>();
+}