diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 8bb595fa8..a8a08b0e3 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -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()); +} diff --git a/lib/checkstl.h b/lib/checkstl.h index 30882b0f9..131c77af2 100644 --- a/lib/checkstl.h +++ b/lib/checkstl.h @@ -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); }; /// @} //--------------------------------------------------------------------------- diff --git a/test/teststl.cpp b/test/teststl.cpp index 5ee14dae1..093221d64 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -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 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 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 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 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 x;\n" + " fun(x.size());\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + }; REGISTER_TEST(TestStl)