Fix #476 STL Container checks.

Fix #473 Add post increment check for STL objects and Classes.

src/checkother.cpp

@@ -1141,6 +1141,36 @@ void CheckOther::checkZeroDivision()
 
 
 
+void CheckOther::postIncrement()
+{
+    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
+    {
+        if (!Token::simpleMatch(tok, "for ("))
+            continue;
+
+        for (const Token *tok2 = tok; tok2; tok2 = tok2->next())
+        {
+            if (Token::Match(tok2, "; %var% ++|-- )"))
+            {
+                // Take a look at the variable declaration
+                const Token *decltok = Token::findmatch(_tokenizer->tokens(), "%varid%", tok2->tokAt(1)->varId());
+                const std::string classDef = std::string("class ") + std::string(decltok->previous()->strAt(0));
+
+                // Is the variable an iterator?
+                if (decltok && Token::Match(decltok->previous(), "iterator|const_iterator"))
+                    postIncrementError(tok2, tok2->strAt(1), (std::string("++") == tok2->strAt(2)));
+                // Is the variable a class?
+                else if (Token::findmatch( _tokenizer->tokens(), classDef.c_str() ))
+                    postIncrementError(tok2, tok2->strAt(1), (std::string("++") == tok2->strAt(2)));
+
+                break;
+            }
+        }
+    }
+}
+
+
+
 
 void CheckOther::cstyleCastError(const Token *tok)
 {
@@ -1231,3 +1261,9 @@ void CheckOther::zerodivError(const Token *tok)
 {
     reportError(tok, Severity::error, "zerodiv", "Division by zero");
 }
+
+void CheckOther::postIncrementError(const Token *tok, const std::string &var_name, const bool isIncrement)
+{
+    std::string type = ( isIncrement ? "Incrementing" : "Decrementing" );
+    reportError(tok, Severity::possibleStyle, "postIncrementDecrement", ("Pre-" + type + " variable '" + var_name + "' is preferred to Post-" + type) );
+}

src/checkother.h

@@ -67,6 +67,7 @@ public:
             checkOther.checkConstantFunctionParameter();
             checkOther.checkStructMemberUsage();
             checkOther.checkIncompleteStatement();
+            checkOther.postIncrement();
         }
 
         checkOther.strPlusChar();
@@ -114,6 +115,9 @@ public:
     /** Check zero division*/
     void checkZeroDivision();
 
+    /** Check for post increment/decrement in for loop*/
+    void postIncrement();
+
 protected:
     void lookupVar(const Token *tok1, const char varname[]);
 
@@ -142,6 +146,7 @@ private:
     void strPlusChar(const Token *tok);
     void nullPointerError(const Token *tok);
     void zerodivError(const Token *tok);
+    void postIncrementError(const Token *tok, const std::string &var_name, const bool isIncrement);
 
     void getErrorMessages()
     {
@@ -164,6 +169,7 @@ private:
         strPlusChar(0);
         nullPointerError(0);
         zerodivError(0);
+        postIncrementError(0, "varname", true);
     }
 
     std::string name() const

src/checkstl.cpp

@@ -20,6 +20,9 @@
 #include "tokenize.h"
 #include "token.h"
 
+// All STL Containers
+#define STL_CONTAINER_LIST "bitset|deque|list|map|multimap|multiset|priority_queue|queue|set|stack|vector|hash_map|hash_multimap|hash_set"
+
 
 // Register this check class (by creating a static instance of it)
 namespace
@@ -366,8 +369,10 @@ void CheckStl::stlBoundries()
     for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
     {
         // Declaring iterator..
-        if (Token::simpleMatch(tok, "list <"))
+        const std::string checkStr = (std::string(STL_CONTAINER_LIST) + " <");
+        if (Token::Match( tok, checkStr.c_str() ))
         {
+            const std::string container_name( tok->strAt(0) );
             while (tok && tok->str() != ">")
                 tok = tok->next();
             if (!tok)
@@ -393,7 +398,7 @@ void CheckStl::stlBoundries()
                     }
                     else if (tok2->varId() == iteratorid && tok2->next() && tok2->next()->str() == "<")
                     {
-                        stlBoundriesError(tok2);
+                        stlBoundriesError(tok2, container_name);
                         break;
                     }
                 }
@@ -403,7 +408,7 @@ void CheckStl::stlBoundries()
 }
 
 // Error message for bad boundry usage..
-void CheckStl::stlBoundriesError(const Token *tok)
+void CheckStl::stlBoundriesError(const Token *tok, const std::string &container_name)
 {
-    reportError(tok, Severity::error, "stlBoundries", "STL range check should be using != and not < since the order of the pointers isn't guaranteed");
+    reportError(tok, Severity::error, "stlBoundries", container_name + " range check should use != and not < since the order of the pointers isn't guaranteed");
 }

src/checkstl.h

@@ -99,7 +99,7 @@ private:
     void eraseError(const Token *tok);
     void pushbackError(const Token *tok, const std::string &iterator_name);
     void invalidPointerError(const Token *tok, const std::string &pointer_name);
-    void stlBoundriesError(const Token *tok);
+    void stlBoundriesError(const Token *tok, const std::string &container_name);
 
     void getErrorMessages()
     {
@@ -110,7 +110,7 @@ private:
         eraseError(0);
         pushbackError(0, "iterator");
         invalidPointerError(0, "pointer");
-        stlBoundriesError(0);
+        stlBoundriesError(0, "container");
     }
 
     std::string name() const

test/testother.cpp

@@ -62,6 +62,9 @@ private:
         TEST_CASE(nullpointer4);
 
         TEST_CASE(oldStylePointerCast);
+
+        TEST_CASE(postIncrementDecrementStl);
+        TEST_CASE(postIncrementDecrementClass);
     }
 
     void check(const char code[])
@@ -616,6 +619,64 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void checkpostIncrementDecrement(const char code[])
+    {
+        // Tokenize..
+        Tokenizer tokenizer;
+        std::istringstream istr(code);
+        tokenizer.tokenize(istr, "test.cpp");
+        tokenizer.setVarId();
+
+        // Clear the error buffer..
+        errout.str("");
+
+        // Check for redundant code..
+        Settings settings;
+        settings._checkCodingStyle = true;
+        CheckOther checkOther(&tokenizer, &settings, this);
+        checkOther.postIncrement();
+    }
+
+    void postIncrementDecrementStl()
+    {
+      checkpostIncrementDecrement("void f1()\n"
+            "{\n"
+            "    std::list<int>::iterator it;\n"
+            "    for (it = ab.begin(); it != ab.end(); it++)\n"
+            "        ;\n"
+            "}\n");
+      ASSERT_EQUALS("[test.cpp:4]: (possible style) Pre-Incrementing variable 'it' is preferred to Post-Incrementing\n", errout.str());
+
+      checkpostIncrementDecrement("void f2()\n"
+            "{\n"
+            "    std::list<int>::iterator it;\n"
+            "    for (it = ab.end(); it != ab.begin(); it--)\n"
+            "        ;\n"
+            "}\n");
+      ASSERT_EQUALS("[test.cpp:4]: (possible style) Pre-Decrementing variable 'it' is preferred to Post-Decrementing\n", errout.str());
+    }
+
+   void postIncrementDecrementClass()
+   {
+      checkpostIncrementDecrement("class TestClass;\n"
+            "void f1()\n"
+            "{\n"
+            "    TestClass tClass;\n"
+            "    for (tClass = TestClass.begin(); tClass != TestClass.end(); tClass++)\n"
+            "        ;\n"
+            "}\n");
+      ASSERT_EQUALS("[test.cpp:5]: (possible style) Pre-Incrementing variable 'tClass' is preferred to Post-Incrementing\n", errout.str());
+
+      checkpostIncrementDecrement("class TestClass;\n"
+            "void f1()\n"
+            "{\n"
+            "    TestClass tClass;\n"
+            "    for (tClass = TestClass.end(); tClass != TestClass.begin(); tClass--)\n"
+            "        ;\n"
+            "}\n");
+      ASSERT_EQUALS("[test.cpp:5]: (possible style) Pre-Decrementing variable 'tClass' is preferred to Post-Decrementing\n", errout.str());
+   }
+
 };
 
 REGISTER_TEST(TestOther)

test/teststl.cpp

@@ -375,23 +375,27 @@ private:
 
     void stlBoundries1()
     {
-        check("void f()\n"
+        const int STL_CONTAINER_LIST = 10;
-              "{\n"
+        const std::string stlCont[STL_CONTAINER_LIST] =
-              "    std::list<int>::iterator it;\n"
+    	    {"vector", "deque", "list", "set", "multiset", "map",
-              "    for (it = ab.begin(); it < ab.end(); ++it)\n"
+             "multimap", "hash_map", "hash_multimap", "hash_set"};
-              "        ;\n"
-              "}\n");
-        ASSERT_EQUALS("[test.cpp:4]: (error) STL range check should be using != and not < since the order of the pointers isn't guaranteed\n", errout.str());
 
-        check("void f()\n"
+        for(int i = 0; i < STL_CONTAINER_LIST; ++i)
-              "{\n"
+        {
-              "    std::vector<int>::iterator it;\n"
+            const std::string checkStr( "void f()\n"
-              "    for (it = ab.begin(); it < ab.end(); ++it)\n"
+                  "{\n"
-              "        ;\n"
+		  "    std::" + stlCont[i] + "<int>::iterator it;\n"
-              "}\n");
+		  "    for (it = ab.begin(); it < ab.end(); ++it)\n"
-        ASSERT_EQUALS("", errout.str());
+		  "        ;\n"
+		  "}\n" );
+
+            check( checkStr.c_str() );
+
+            ASSERT_EQUALS("[test.cpp:4]: (error) " + stlCont[i]  + " range check should use != and not < since the order of the pointers isn't guaranteed\n", errout.str());
+        }
     }
 
+
 };
 
 REGISTER_TEST(TestStl)