Index: clang-tidy/readability/ContainerSizeEmptyCheck.cpp =================================================================== --- clang-tidy/readability/ContainerSizeEmptyCheck.cpp +++ clang-tidy/readability/ContainerSizeEmptyCheck.cpp @@ -29,11 +29,11 @@ if (!getLangOpts().CPlusPlus) return; - const auto stlContainer = hasAnyName( - "array", "basic_string", "deque", "forward_list", "list", "map", - "multimap", "multiset", "priority_queue", "queue", "set", "stack", - "unordered_map", "unordered_multimap", "unordered_multiset", - "unordered_set", "vector"); + const auto validContainer = namedDecl( + has(functionDecl( + isPublic(), hasName("size"), returns(isInteger()), + unless(anyOf(returns(isAnyCharacter()), returns(booleanType()))))), + has(functionDecl(isPublic(), hasName("empty"), returns(booleanType())))); const auto WrongUse = anyOf( hasParent(binaryOperator( @@ -50,9 +50,9 @@ Finder->addMatcher( cxxMemberCallExpr( - on(expr(anyOf(hasType(namedDecl(stlContainer)), - hasType(pointsTo(namedDecl(stlContainer))), - hasType(references(namedDecl(stlContainer))))) + on(expr(anyOf(hasType(namedDecl(validContainer)), + hasType(pointsTo(namedDecl(validContainer))), + hasType(references(namedDecl(validContainer))))) .bind("STLObject")), callee(cxxMethodDecl(hasName("size"))), WrongUse) .bind("SizeCallExpr"), Index: test/clang-tidy/readability-container-size-empty.cpp =================================================================== --- test/clang-tidy/readability-container-size-empty.cpp +++ test/clang-tidy/readability-container-size-empty.cpp @@ -24,9 +24,34 @@ }; } - } +template +class TemplatedContainer { +public: + int size(); + bool empty(); +}; + +template +class PrivateEmpty { +public: + int size(); +private: + bool empty(); +}; + +struct BoolSize { + bool size(); + bool empty(); +}; + +struct EnumSize { + enum E { one }; + enum E size() const; + bool empty(); +}; + int main() { std::set intSet; std::string str; @@ -127,6 +152,110 @@ ; // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used // CHECK-FIXES: {{^ }}if (vect4.empty()){{$}} + + TemplatedContainer templated_container; + if (templated_container.size() == 0) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used + // CHECK-FIXES: {{^ }}if (templated_container.empty()){{$}} + if (templated_container.size() != 0) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used + // CHECK-FIXES: {{^ }}if (!templated_container.empty()){{$}} + if (0 == templated_container.size()) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: the 'empty' method should be used + // CHECK-FIXES: {{^ }}if (templated_container.empty()){{$}} + if (0 != templated_container.size()) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: the 'empty' method should be used + // CHECK-FIXES: {{^ }}if (!templated_container.empty()){{$}} + if (templated_container.size() > 0) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used + // CHECK-FIXES: {{^ }}if (!templated_container.empty()){{$}} + if (0 < templated_container.size()) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: the 'empty' method should be used + // CHECK-FIXES: {{^ }}if (!templated_container.empty()){{$}} + if (templated_container.size() < 1) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used + // CHECK-FIXES: {{^ }}if (templated_container.empty()){{$}} + if (1 > templated_container.size()) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: the 'empty' method should be used + // CHECK-FIXES: {{^ }}if (templated_container.empty()){{$}} + if (templated_container.size() >= 1) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used + // CHECK-FIXES: {{^ }}if (!templated_container.empty()){{$}} + if (1 <= templated_container.size()) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: the 'empty' method should be used + // CHECK-FIXES: {{^ }}if (!templated_container.empty()){{$}} + if (templated_container.size() > 1) // no warning + ; + if (1 < templated_container.size()) // no warning + ; + if (templated_container.size() <= 1) // no warning + ; + if (1 >= templated_container.size()) // no warning + ; + if (!templated_container.size()) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: the 'empty' method should be used + // CHECK-FIXES: {{^ }}if (templated_container.empty()){{$}} + if (templated_container.size()) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used + // CHECK-FIXES: {{^ }}if (!templated_container.empty()){{$}} + + if (templated_container.empty()) + ; + + // No warnings expected. + PrivateEmpty private_empty; + if (private_empty.size() == 0) + ; + if (private_empty.size() != 0) + ; + if (0 == private_empty.size()) + ; + if (0 != private_empty.size()) + ; + if (private_empty.size() > 0) + ; + if (0 < private_empty.size()) + ; + if (private_empty.size() < 1) + ; + if (1 > private_empty.size()) + ; + if (private_empty.size() >= 1) + ; + if (1 <= private_empty.size()) + ; + if (private_empty.size() > 1) + ; + if (1 < private_empty.size()) + ; + if (private_empty.size() <= 1) + ; + if (1 >= private_empty.size()) + ; + if (!private_empty.size()) + ; + if (private_empty.size()) + ; + + // Types with weird size() return type. + BoolSize bs; + if (bs.size() == 0) + ; + EnumSize es; + if (es.size() == 0) + ; } #define CHECKSIZE(x) if (x.size()) @@ -142,6 +271,16 @@ CHECKSIZE(v); // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: the 'empty' method should be used // CHECK-MESSAGES: CHECKSIZE(v); + + TemplatedContainer templated_container; + if (templated_container.size()) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used + // 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: CHECKSIZE(templated_container); } void g() {