Fixed #2079 (detect side effects in assert)

lib/checkother.cpp

@@ -229,6 +229,33 @@ void CheckOther::checkSelfAssignment()
     }
 }
 
+//---------------------------------------------------------------------------
+//    int a = 1;
+//    assert(a = 2);            // <- assert should not have a side-effect
+//---------------------------------------------------------------------------
+void CheckOther::checkAssignmentInAssert()
+{
+    const char assertPattern[] = "assert ( %any%";
+    const Token *tok = Token::findmatch(_tokenizer->tokens(), assertPattern);
+    const Token *endTok = tok ? tok->next()->link() : NULL;
+
+    while (tok && endTok)
+    {
+        const Token* varTok = Token::findmatch(tok->tokAt(2), "%var% --|++|+=|-=|*=|/=|&=|^=|=", endTok);
+        if (varTok)
+        {
+            assignmentInAssertError(tok, varTok->str());
+        }
+        else if ((varTok = Token::findmatch(tok->tokAt(2), "--|++ %var%", endTok)))
+        {
+            assignmentInAssertError(tok, varTok->strAt(1));
+        }
+
+        tok = Token::findmatch(endTok->next(), assertPattern);
+        endTok = tok ? tok->next()->link() : NULL;
+    }
+}
+
 //---------------------------------------------------------------------------
 // strtol(str, 0, radix)  <- radix must be 0 or 2-36
 //---------------------------------------------------------------------------
@@ -4156,6 +4183,12 @@ void CheckOther::selfAssignmentError(const Token *tok, const std::string &varnam
                 "selfAssignment", "Redundant assignment of \"" + varname + "\" to itself");
 }
 
+void CheckOther::assignmentInAssertError(const Token *tok, const std::string &varname)
+{
+    reportError(tok, Severity::error,
+                "assignmentInAssert", "Assert statement modifies '" + varname + "' instead of just testing it");
+}
+
 void CheckOther::misusedScopeObjectError(const Token *tok, const std::string& varname)
 {
     reportError(tok, Severity::error,

lib/checkother.h

@@ -63,6 +63,7 @@ public:
         checkOther.sizeofsizeof();
         checkOther.sizeofCalculation();
         checkOther.checkRedundantAssignmentInSwitch();
+        checkOther.checkAssignmentInAssert();
     }
 
     /** @brief Run checks against the simplified token list */
@@ -184,6 +185,9 @@ public:
     /** @brief %Check for assigning a variable to itself*/
     void checkSelfAssignment();
 
+    /** @brief %Check for assignment to a variable in an assert test*/
+    void checkAssignmentInAssert();
+
     /** @brief %Check for objects that are destroyed immediately */
     void checkMisusedScopedObject();
 
@@ -213,6 +217,7 @@ public:
     void fflushOnInputStreamError(const Token *tok, const std::string &varname);
     void redundantAssignmentInSwitchError(const Token *tok, const std::string &varname);
     void selfAssignmentError(const Token *tok, const std::string &varname);
+    void assignmentInAssertError(const Token *tok, const std::string &varname);
     void misusedScopeObjectError(const Token *tok, const std::string &varname);
 
     void getErrorMessages()
@@ -244,6 +249,7 @@ public:
         sizeofCalculationError(0);
         redundantAssignmentInSwitchError(0, "varname");
         selfAssignmentError(0, "varname");
+        assignmentInAssertError(0, "varname");
         invalidScanfError(0);
 
         // optimisations
@@ -267,6 +273,7 @@ public:
                "* using uninitialized variables and data\n"
                "* using fflush() on an input stream\n"
                "* scoped object destroyed immediately after construction\n"
+               "* assignment in an assert statement\n"
 
                // style
                "* C-style pointer cast in cpp file\n"

lib/tokenize.cpp

@@ -8151,7 +8151,11 @@ void Tokenizer::simplifyAssignmentInFunctionCall()
     {
         if (tok->str() == "(")
             tok = tok->link();
-        else if (Token::Match(tok, "[;{}] %var% ( %var% =") && Token::simpleMatch(tok->tokAt(2)->link(), ") ;"))
+
+        // Find 'foo(var='. Exclude 'assert(var=' to allow tests to check that assert(...) does not contain side-effects
+        else if (Token::Match(tok, "[;{}] %var% ( %var% =") &&
+                 Token::simpleMatch(tok->tokAt(2)->link(), ") ;") &&
+                 tok->strAt(1) != "assert")
         {
             const std::string funcname(tok->strAt(1));
             const Token * const vartok = tok->tokAt(3);

test/testother.cpp

@@ -115,6 +115,8 @@ private:
         TEST_CASE(testMisusedScopeObjectDoesNotPickLocalClassConstructors);
         TEST_CASE(testMisusedScopeObjectDoesNotPickUsedObject);
         TEST_CASE(trac2071);
+
+        TEST_CASE(assignmentInAssert);
     }
 
     void check(const char code[])
@@ -135,6 +137,7 @@ private:
         checkOther.sizeofsizeof();
         checkOther.sizeofCalculation();
         checkOther.checkRedundantAssignmentInSwitch();
+        checkOther.checkAssignmentInAssert();
 
         // Simplify token list..
         tokenizer.simplifyTokenList();
@@ -3169,6 +3172,66 @@ private:
              );
         ASSERT_EQUALS("", errout.str());
     }
+
+    void assignmentInAssert()
+    {
+        check("void f() {\n"
+              "    int a = 0;\n"
+              "    assert(a = 2);\n"
+              "    return a;\n"
+              "}\n"
+             );
+        ASSERT_EQUALS("[test.cpp:3]: (error) Assert statement modifies 'a' instead of just testing it\n", errout.str());
+
+        check("void f() {\n"
+              "    int a = 0;\n"
+              "    assert(a == 2);\n"
+              "    return a;\n"
+              "}\n"
+             );
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"
+              "    int a = 0;\n"
+              "    int b = 0;\n"
+              "    assert(a == 2 && b = 1);\n"
+              "    return a;\n"
+              "}\n"
+             );
+        ASSERT_EQUALS("[test.cpp:4]: (error) Assert statement modifies 'b' instead of just testing it\n", errout.str());
+
+        check("void f() {\n"
+              "    int a = 0;\n"
+              "    assert(a += 2);\n"
+              "    return a;\n"
+              "}\n"
+             );
+        ASSERT_EQUALS("[test.cpp:3]: (error) Assert statement modifies 'a' instead of just testing it\n", errout.str());
+
+        check("void f() {\n"
+              "    int a = 0;\n"
+              "    assert(a *= 2);\n"
+              "    return a;\n"
+              "}\n"
+             );
+        ASSERT_EQUALS("[test.cpp:3]: (error) Assert statement modifies 'a' instead of just testing it\n", errout.str());
+
+        check("void f() {\n"
+              "    int a = 0;\n"
+              "    assert(a -= 2);\n"
+              "    return a;\n"
+              "}\n"
+             );
+        ASSERT_EQUALS("[test.cpp:3]: (error) Assert statement modifies 'a' instead of just testing it\n", errout.str());
+
+        check("void f() {\n"
+              "    int a = 0;\n"
+              "    assert(a --);\n"
+              "    return a;\n"
+              "}\n"
+             );
+        ASSERT_EQUALS("[test.cpp:3]: (error) Assert statement modifies 'a' instead of just testing it\n", errout.str());
+    }
 };
 
 REGISTER_TEST(TestOther)