diff --git a/src/checkother.cpp b/src/checkother.cpp index 44719e0dd..242729786 100644 --- a/src/checkother.cpp +++ b/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) ); +} diff --git a/src/checkother.h b/src/checkother.h index a684d3158..ca34f56ce 100644 --- a/src/checkother.h +++ b/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 diff --git a/src/checkstl.cpp b/src/checkstl.cpp index 54cf47e04..a8a9bb572 100644 --- a/src/checkstl.cpp +++ b/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"); } diff --git a/src/checkstl.h b/src/checkstl.h index 05b74932b..1883a2c40 100644 --- a/src/checkstl.h +++ b/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 diff --git a/test/testother.cpp b/test/testother.cpp index 0280b0de8..73c454bd0 100644 --- a/test/testother.cpp +++ b/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::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::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) diff --git a/test/teststl.cpp b/test/teststl.cpp index 3faa450b7..0d068e536 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -375,23 +375,27 @@ private: void stlBoundries1() { - check("void f()\n" - "{\n" - " std::list::iterator it;\n" - " for (it = ab.begin(); it < ab.end(); ++it)\n" - " ;\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()); + const int STL_CONTAINER_LIST = 10; + const std::string stlCont[STL_CONTAINER_LIST] = + {"vector", "deque", "list", "set", "multiset", "map", + "multimap", "hash_map", "hash_multimap", "hash_set"}; - check("void f()\n" - "{\n" - " std::vector::iterator it;\n" - " for (it = ab.begin(); it < ab.end(); ++it)\n" - " ;\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); + for(int i = 0; i < STL_CONTAINER_LIST; ++i) + { + const std::string checkStr( "void f()\n" + "{\n" + " std::" + stlCont[i] + "::iterator it;\n" + " for (it = ab.begin(); it < ab.end(); ++it)\n" + " ;\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)