[clang-tools-extra] r301185 - Extend readability-container-size-empty to add comparisons to empty-state objects.
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 24 07:57:10 PDT 2017
Author: aaronballman
Date: Mon Apr 24 09:57:09 2017
New Revision: 301185
URL: http://llvm.org/viewvc/llvm-project?rev=301185&view=rev
Log:
Extend readability-container-size-empty to add comparisons to empty-state objects.
Patch by Josh Zimmerman.
Modified:
clang-tools-extra/trunk/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp
clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h
clang-tools-extra/trunk/test/clang-tidy/readability-container-size-empty.cpp
Modified: clang-tools-extra/trunk/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/ContainerSizeEmptyCheck.cpp?rev=301185&r1=301184&r2=301185&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/ContainerSizeEmptyCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/ContainerSizeEmptyCheck.cpp Mon Apr 24 09:57:09 2017
@@ -7,6 +7,7 @@
//
//===----------------------------------------------------------------------===//
#include "ContainerSizeEmptyCheck.h"
+#include "../utils/ASTUtils.h"
#include "../utils/Matchers.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchers.h"
@@ -19,6 +20,8 @@ namespace clang {
namespace tidy {
namespace readability {
+using utils::IsBinaryOrTernary;
+
ContainerSizeEmptyCheck::ContainerSizeEmptyCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
@@ -62,23 +65,70 @@ void ContainerSizeEmptyCheck::registerMa
ofClass(equalsBoundNode("container"))))))
.bind("SizeCallExpr"),
this);
+
+ // Empty constructor matcher.
+ const auto DefaultConstructor = cxxConstructExpr(
+ hasDeclaration(cxxConstructorDecl(isDefaultConstructor())));
+ // Comparison to empty string or empty constructor.
+ const auto WrongComparend = anyOf(
+ 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)))));
+ // Match the object being compared.
+ const auto STLArg =
+ anyOf(unaryOperator(
+ hasOperatorName("*"),
+ hasUnaryOperand(
+ expr(hasType(pointsTo(ValidContainer))).bind("Pointee"))),
+ expr(hasType(ValidContainer)).bind("STLObject"));
+ Finder->addMatcher(
+ cxxOperatorCallExpr(
+ anyOf(hasOverloadedOperatorName("=="),
+ hasOverloadedOperatorName("!=")),
+ anyOf(allOf(hasArgument(0, WrongComparend), hasArgument(1, STLArg)),
+ allOf(hasArgument(0, STLArg), hasArgument(1, 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 *BinCmp = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("BinCmp");
const auto *BinaryOp = Result.Nodes.getNodeAs<BinaryOperator>("SizeBinaryOp");
- const auto *E = MemberCall->getImplicitObjectArgument();
+ const auto *Pointee = Result.Nodes.getNodeAs<Expr>("Pointee");
+ const auto *E =
+ MemberCall
+ ? MemberCall->getImplicitObjectArgument()
+ : (Pointee ? Pointee : Result.Nodes.getNodeAs<Expr>("STLObject"));
FixItHint Hint;
std::string ReplacementText =
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.
+ ReplacementText = "(" + ReplacementText + ")";
+ }
if (E->getType()->isPointerType())
ReplacementText += "->empty()";
else
ReplacementText += ".empty()";
- if (BinaryOp) { // Determine the correct transformation.
+ if (BinCmp) {
+ if (BinCmp->getOperator() == OO_ExclaimEqual) {
+ ReplacementText = "!" + ReplacementText;
+ }
+ Hint =
+ FixItHint::CreateReplacement(BinCmp->getSourceRange(), ReplacementText);
+ } else if (BinaryOp) { // Determine the correct transformation.
bool Negation = false;
const bool ContainerIsLHS =
!llvm::isa<IntegerLiteral>(BinaryOp->getLHS()->IgnoreImpCasts());
@@ -148,9 +198,17 @@ void ContainerSizeEmptyCheck::check(cons
"!" + ReplacementText);
}
- diag(MemberCall->getLocStart(), "the 'empty' method should be used to check "
- "for emptiness instead of 'size'")
- << Hint;
+ if (MemberCall) {
+ diag(MemberCall->getLocStart(),
+ "the 'empty' method should be used to check "
+ "for emptiness instead of 'size'")
+ << Hint;
+ } else {
+ diag(BinCmp->getLocStart(),
+ "the 'empty' method should be used to check "
+ "for emptiness instead of comparing to an empty object")
+ << Hint;
+ }
const auto *Container = Result.Nodes.getNodeAs<NamedDecl>("container");
const auto *Empty = Result.Nodes.getNodeAs<FunctionDecl>("empty");
Modified: clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp?rev=301185&r1=301184&r2=301185&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp Mon Apr 24 09:57:09 2017
@@ -23,6 +23,22 @@ const FunctionDecl *getSurroundingFuncti
"function", match(stmt(hasAncestor(functionDecl().bind("function"))),
Statement, Context));
}
+
+bool IsBinaryOrTernary(const Expr *E) {
+ const Expr *E_base = E->IgnoreImpCasts();
+ if (clang::isa<clang::BinaryOperator>(E_base) ||
+ clang::isa<clang::ConditionalOperator>(E_base)) {
+ return true;
+ }
+
+ if (const auto *Operator =
+ clang::dyn_cast<clang::CXXOperatorCallExpr>(E_base)) {
+ return Operator->isInfixBinaryOp();
+ }
+
+ return false;
+}
+
} // namespace utils
} // namespace tidy
} // namespace clang
Modified: clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h?rev=301185&r1=301184&r2=301185&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h (original)
+++ clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h Mon Apr 24 09:57:09 2017
@@ -18,6 +18,8 @@ namespace utils {
// Returns the (closest) Function declaration surrounding |Statement| or NULL.
const FunctionDecl *getSurroundingFunction(ASTContext &Context,
const Stmt &Statement);
+// Determine whether Expr is a Binary or Ternary expression.
+bool IsBinaryOrTernary(const Expr *E);
} // namespace utils
} // namespace tidy
} // namespace clang
Modified: clang-tools-extra/trunk/test/clang-tidy/readability-container-size-empty.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-container-size-empty.cpp?rev=301185&r1=301184&r2=301185&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/readability-container-size-empty.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/readability-container-size-empty.cpp Mon Apr 24 09:57:09 2017
@@ -3,12 +3,19 @@
namespace std {
template <typename T> struct vector {
vector();
+ bool operator==(const vector<T>& other) const;
+ bool operator!=(const vector<T>& other) const;
unsigned long size() const;
bool empty() const;
};
template <typename T> struct basic_string {
basic_string();
+ bool operator==(const basic_string<T>& other) const;
+ bool operator!=(const basic_string<T>& other) const;
+ bool operator==(const char *) const;
+ bool operator!=(const char *) const;
+ basic_string<T> operator+(const basic_string<T>& other) const;
unsigned long size() const;
bool empty() const;
};
@@ -19,6 +26,8 @@ typedef basic_string<wchar_t> wstring;
inline namespace __v2 {
template <typename T> struct set {
set();
+ bool operator==(const set<T>& other) const;
+ bool operator!=(const set<T>& other) const;
unsigned long size() const;
bool empty() const;
};
@@ -29,6 +38,8 @@ template <typename T> struct set {
template <typename T>
class TemplatedContainer {
public:
+ bool operator==(const TemplatedContainer<T>& other) const;
+ bool operator!=(const TemplatedContainer<T>& other) const;
int size() const;
bool empty() const;
};
@@ -36,6 +47,8 @@ public:
template <typename T>
class PrivateEmpty {
public:
+ bool operator==(const PrivateEmpty<T>& other) const;
+ bool operator!=(const PrivateEmpty<T>& other) const;
int size() const;
private:
bool empty() const;
@@ -54,6 +67,7 @@ struct EnumSize {
class Container {
public:
+ bool operator==(const Container& other) const;
int size() const;
bool empty() const;
};
@@ -75,9 +89,21 @@ public:
bool Container3::empty() const { return this->size() == 0; }
+class Container4 {
+public:
+ bool operator==(const Container4& rhs) const;
+ int size() const;
+ bool empty() const { return *this == Container4(); }
+};
+
+std::string s_func() {
+ return std::string();
+}
+
int main() {
std::set<int> intSet;
std::string str;
+ std::string str2;
std::wstring wstr;
str.size() + 0;
str.size() - 0;
@@ -87,24 +113,57 @@ int main() {
;
// 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
+ // CHECK-MESSAGES: :32:8: note: method 'set<int>'::empty() defined here
+ if (intSet == std::set<int>())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness
+ // CHECK-FIXES: {{^ }}if (intSet.empty()){{$}}
+ // CHECK-MESSAGES: :32:8: note: method 'set<int>'::empty() defined here
+ if (s_func() == "")
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (s_func().empty()){{$}}
if (str.size() == 0)
;
// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
// CHECK-FIXES: {{^ }}if (str.empty()){{$}}
+ if ((str + str2).size() == 0)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if ((str + str2).empty()){{$}}
+ if (str == "")
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (str.empty()){{$}}
+ if (str + str2 == "")
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if ((str + str2).empty()){{$}}
if (wstr.size() == 0)
;
// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
// CHECK-FIXES: {{^ }}if (wstr.empty()){{$}}
+ if (wstr == "")
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+ // 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 == std::vector<int>())
+ ;
+ // 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
// CHECK-FIXES: {{^ }}if (!vect.empty()){{$}}
+ if (vect != std::vector<int>())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (!vect.empty()){{$}}
if (0 == vect.size())
;
// CHECK-MESSAGES: :[[@LINE-2]]:12: warning: the 'empty' method should be used
@@ -113,6 +172,14 @@ int main() {
;
// CHECK-MESSAGES: :[[@LINE-2]]:12: warning: the 'empty' method should be used
// CHECK-FIXES: {{^ }}if (!vect.empty()){{$}}
+ if (std::vector<int>() == vect)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (vect.empty()){{$}}
+ if (std::vector<int>() != vect)
+ ;
+ // 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
@@ -172,6 +239,14 @@ int main() {
;
// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
// CHECK-FIXES: {{^ }}if ((*vect3).empty()){{$}}
+ if ((*vect3) == std::vector<int>())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (vect3->empty()){{$}}
+ if (*vect3 == std::vector<int>())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (vect3->empty()){{$}}
delete vect3;
@@ -180,24 +255,44 @@ int main() {
;
// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
// CHECK-FIXES: {{^ }}if (vect4.empty()){{$}}
+ if (vect4 == std::vector<int>())
+ ;
+ // 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 == TemplatedContainer<void>())
+ ;
+ // 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 (templated_container != TemplatedContainer<void>())
+ ;
+ // 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 (TemplatedContainer<void>() == templated_container)
+ ;
+ // 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 (TemplatedContainer<void>() != templated_container)
+ ;
+ // 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
@@ -246,12 +341,20 @@ int main() {
PrivateEmpty<void> private_empty;
if (private_empty.size() == 0)
;
+ if (private_empty == PrivateEmpty<void>())
+ ;
if (private_empty.size() != 0)
;
+ if (private_empty != PrivateEmpty<void>())
+ ;
if (0 == private_empty.size())
;
+ if (PrivateEmpty<void>() == private_empty)
+ ;
if (0 != private_empty.size())
;
+ if (PrivateEmpty<void>() != private_empty)
+ ;
if (private_empty.size() > 0)
;
if (0 < private_empty.size())
@@ -290,6 +393,10 @@ int main() {
;
// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
// CHECK-FIXES: {{^ }}if (derived.empty()){{$}}
+ if (derived == Derived())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (derived.empty()){{$}}
}
#define CHECKSIZE(x) if (x.size())
@@ -301,6 +408,10 @@ template <typename T> void f() {
;
// 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()){{$}}
+ 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: ;
CHECKSIZE(v);
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: the 'empty' method should be used
@@ -311,6 +422,10 @@ template <typename T> void f() {
;
// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
// CHECK-FIXES: {{^ }}if (!templated_container.empty()){{$}}
+ if (templated_container != TemplatedContainer<T>())
+ ;
+ // 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
More information about the cfe-commits
mailing list