Merge branch 'master' of github.com:danmar/cppcheck

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());
+}

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);
 };
 /// @}
 //---------------------------------------------------------------------------

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<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)