robert reif: Fixed #1118 (new check: use empty() instead of checking size() against zero)
This commit is contained in:
parent
69a98d81fc
commit
95f652a2b4
|
@ -530,3 +530,75 @@ void CheckStl::findError(const Token *tok)
|
|||
{
|
||||
reportError(tok, Severity::error, "stlfind", "dangerous usage of find result");
|
||||
}
|
||||
|
||||
bool CheckStl::isStlContainer(const Token *tok)
|
||||
{
|
||||
// check if this token is defined
|
||||
if (tok->varId())
|
||||
{
|
||||
// find where this token is defined
|
||||
const Token *type = Token::findmatch(_tokenizer->tokens(), "%varid%", tok->varId());
|
||||
|
||||
// find where this tokens type starts
|
||||
while (type->previous() && !Token::Match(type->previous(), "[;{]"))
|
||||
type = type->previous();
|
||||
|
||||
// discard namespace if supplied
|
||||
if (Token::Match(type, "std ::"))
|
||||
type = type->next()->next();
|
||||
|
||||
// all possible stl containers
|
||||
static const char STL_CONTAINER_LIST[] = "bitset|deque|list|map|multimap|multiset|priority_queue|queue|set|stack|hash_map|hash_multimap|hash_set|vector";
|
||||
|
||||
// container template string
|
||||
const std::string checkStr = (std::string(STL_CONTAINER_LIST) + " <");
|
||||
|
||||
// check if it's an stl template
|
||||
if (Token::Match(type, checkStr.c_str()))
|
||||
return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
void CheckStl::size()
|
||||
{
|
||||
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
|
||||
{
|
||||
if (Token::Match(tok, "%var% . size ( )"))
|
||||
{
|
||||
if (Token::Match(tok->tokAt(5), "==|!=|> 0"))
|
||||
{
|
||||
if (isStlContainer(tok))
|
||||
sizeError(tok);
|
||||
}
|
||||
else if ((tok->tokAt(5)->str() == ")" ||
|
||||
tok->tokAt(5)->str() == "&&" ||
|
||||
tok->tokAt(5)->str() == "||" ||
|
||||
tok->tokAt(5)->str() == "!" ) &&
|
||||
(tok->tokAt(-1)->str() == "(" ||
|
||||
tok->tokAt(-1)->str() == "&&" ||
|
||||
tok->tokAt(-1)->str() == "||" ||
|
||||
tok->tokAt(-1)->str() == "!"))
|
||||
{
|
||||
if (tok->tokAt(-1)->str() == "(" &&
|
||||
tok->tokAt(5)->str() == ")")
|
||||
{
|
||||
// check for passing size to function call
|
||||
if (Token::Match(tok->tokAt(-2), "if|while"))
|
||||
{
|
||||
if (isStlContainer(tok))
|
||||
sizeError(tok);
|
||||
}
|
||||
}
|
||||
else if (isStlContainer(tok))
|
||||
sizeError(tok);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void CheckStl::sizeError(const Token *tok)
|
||||
{
|
||||
reportError(tok, Severity::possibleStyle, "stlSize", "Replace size() check against 0 with empty(): " + tok->str());
|
||||
}
|
||||
|
|
|
@ -54,6 +54,7 @@ public:
|
|||
checkStl.pushback();
|
||||
checkStl.stlBoundries();
|
||||
checkStl.find();
|
||||
checkStl.size();
|
||||
}
|
||||
|
||||
|
||||
|
@ -96,6 +97,12 @@ public:
|
|||
/** usage of std::find */
|
||||
void find();
|
||||
|
||||
/**
|
||||
* Suggest using empty() instead of checking size() against zero for containers.
|
||||
* Item 4 from Scott Meyers book "Effective STL".
|
||||
*/
|
||||
void size();
|
||||
|
||||
private:
|
||||
|
||||
/**
|
||||
|
@ -113,6 +120,7 @@ private:
|
|||
void invalidPointerError(const Token *tok, const std::string &pointer_name);
|
||||
void stlBoundriesError(const Token *tok, const std::string &container_name);
|
||||
void findError(const Token *tok);
|
||||
void sizeError(const Token *tok);
|
||||
|
||||
void getErrorMessages()
|
||||
{
|
||||
|
@ -125,6 +133,7 @@ private:
|
|||
invalidPointerError(0, "pointer");
|
||||
stlBoundriesError(0, "container");
|
||||
findError(0);
|
||||
sizeError(0);
|
||||
}
|
||||
|
||||
std::string name() const
|
||||
|
@ -142,6 +151,8 @@ private:
|
|||
" * for vectors: using iterator/pointer after push_back has been used\n"
|
||||
" * dangerous usage of find";
|
||||
}
|
||||
|
||||
bool isStlContainer(const Token *tok);
|
||||
};
|
||||
/// @}
|
||||
//---------------------------------------------------------------------------
|
||||
|
|
|
@ -73,6 +73,8 @@ private:
|
|||
|
||||
// find
|
||||
TEST_CASE(find1);
|
||||
|
||||
TEST_CASE(size1);
|
||||
}
|
||||
|
||||
void check(const std::string &code)
|
||||
|
@ -633,6 +635,44 @@ private:
|
|||
ASSERT_EQUALS("", errout.str());
|
||||
}
|
||||
|
||||
void size1()
|
||||
{
|
||||
check("void f()\n"
|
||||
"{\n"
|
||||
" std::list<int> x;\n"
|
||||
" if (x.size() == 0) {}\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS("[test.cpp:4]: (possible style) Replace size() check against 0 with empty(): x\n", errout.str());
|
||||
|
||||
check("void f()\n"
|
||||
"{\n"
|
||||
" std::list<int> x;\n"
|
||||
" if (x.size() != 0) {}\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS("[test.cpp:4]: (possible style) Replace size() check against 0 with empty(): x\n", errout.str());
|
||||
|
||||
check("void f()\n"
|
||||
"{\n"
|
||||
" std::list<int> x;\n"
|
||||
" if (x.size() > 0) {}\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS("[test.cpp:4]: (possible style) Replace size() check against 0 with empty(): x\n", errout.str());
|
||||
|
||||
check("void f()\n"
|
||||
"{\n"
|
||||
" std::list<int> x;\n"
|
||||
" if (x.size()) {}\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS("[test.cpp:4]: (possible style) Replace size() check against 0 with empty(): x\n", errout.str());
|
||||
|
||||
check("void f()\n"
|
||||
"{\n"
|
||||
" std::list<int> x;\n"
|
||||
" fun(x.size());\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
}
|
||||
|
||||
};
|
||||
|
||||
REGISTER_TEST(TestStl)
|
||||
|
|
Loading…
Reference in New Issue