//
//===----------------------------------------------------------------------===//
#include "ContainerSizeEmptyCheck.h"
+#include "../utils/Matchers.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Lex/Lexer.h"
#include "llvm/ADT/StringRef.h"
-#include "../utils/Matchers.h"
using namespace clang::ast_matchers;
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 = 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 WrongUse = anyOf(
hasParent(binaryOperator(
hasParent(explicitCastExpr(hasDestinationType(booleanType()))));
Finder->addMatcher(
- cxxMemberCallExpr(
- on(expr(anyOf(hasType(namedDecl(stlContainer)),
- hasType(pointsTo(namedDecl(stlContainer))),
- hasType(references(namedDecl(stlContainer)))))
- .bind("STLObject")),
- callee(cxxMethodDecl(hasName("size"))), WrongUse)
+ cxxMemberCallExpr(on(expr(anyOf(hasType(validContainer),
+ hasType(pointsTo(validContainer)),
+ hasType(references(validContainer))))
+ .bind("STLObject")),
+ callee(cxxMethodDecl(hasName("size"))), WrongUse)
.bind("SizeCallExpr"),
this);
}
Hint = FixItHint::CreateReplacement(MemberCall->getSourceRange(),
"!" + ReplacementText);
}
+
diag(MemberCall->getLocStart(), "the 'empty' method should be used to check "
"for emptiness instead of 'size'")
<< Hint;
+
+ const auto *Container = Result.Nodes.getNodeAs<NamedDecl>("container");
+ const auto *Empty = Result.Nodes.getNodeAs<FunctionDecl>("empty");
+
+ diag(Empty->getLocation(), "method %0::empty() defined here",
+ DiagnosticIDs::Note)
+ << Container;
}
} // namespace readability
};
}
-
}
+template <typename T>
+class TemplatedContainer {
+public:
+ int size() const;
+ bool empty() const;
+};
+
+template <typename T>
+class PrivateEmpty {
+public:
+ int size() const;
+private:
+ bool empty() const;
+};
+
+struct BoolSize {
+ bool size() const;
+ bool empty() const;
+};
+
+struct EnumSize {
+ enum E { one };
+ enum E size() const;
+ bool empty() const;
+};
+
+class Container {
+public:
+ int size() const;
+ bool empty() const;
+};
+
+class Derived : public Container {
+};
+
int main() {
std::set<int> intSet;
std::string str;
;
// 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()){{$}}
+ // CHECK-MESSAGES: :23:8: note: method 'set<int>'::empty() defined here
if (str.size() == 0)
;
// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
;
// 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)
+ ;
+
+ Derived derived;
+ if (derived.size() == 0)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (derived.empty()){{$}}
}
#define CHECKSIZE(x) if (x.size())
std::vector<T> v;
if (v.size())
;
- // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+ // 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 (!v.empty()){{$}}
// CHECK-FIXES-NEXT: ;
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() {