omtcyfz updated this revision to Diff 70717.
omtcyfz added a comment.
Blacklist `enum` and `bool` return types for `size()`.
https://reviews.llvm.org/D24349
Files:
clang-tidy/readability/ContainerSizeEmptyCheck.cpp
test/clang-tidy/readability-container-size-empty.cpp
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 <typename T>
+class TemplatedContainer {
+public:
+ int size();
+ bool empty();
+};
+
+template <typename T>
+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<int> 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<void> 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<void> 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<T> 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() {
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"),
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
