Consider pre{inc,dec}rements on assert checks (cppcheck-opensource#2605)

michaelgruner · web-flow · commit 7c93f51885a9 · 2020-04-18T09:26:24.000+02:00
* Consider pre{inc,dec}rements on assert checks

* Simplify code by using new AST APIs

* Fix assert test with invalid syntax
diff --git a/lib/checkassert.cpp b/lib/checkassert.cpp
@@ -120,13 +120,16 @@ void CheckAssert::assignmentInAssertError(const Token *tok, const std::string& v
 // checks if side effects happen on the variable prior to tmp
 void CheckAssert::checkVariableAssignment(const Token* assignTok, const Scope *assertionScope)
 {
-    const Variable* prevVar = assignTok->previous()->variable();
-    if (!prevVar)
+    if (!assignTok->isAssignmentOp() && assignTok->tokType() != Token::eIncDecOp)
+        return;
+
+    const Variable* var = assignTok->astOperand1()->variable();
+    if (!var)
         return;
 
     // Variable declared in inner scope in assert => don't warn
-    if (assertionScope != prevVar->scope()) {
-        const Scope *s = prevVar->scope();
+    if (assertionScope != var->scope()) {
+        const Scope *s = var->scope();
         while (s && s != assertionScope)
             s = s->nestedIn;
         if (s == assertionScope)
@@ -135,12 +138,12 @@ void CheckAssert::checkVariableAssignment(const Token* assignTok, const Scope *a
 
     // assignment
     if (assignTok->isAssignmentOp() || assignTok->tokType() == Token::eIncDecOp) {
-        if (prevVar->isConst())
+        if (var->isConst()) {
             return;
-
-        assignmentInAssertError(assignTok, prevVar->name());
+        }
+        assignmentInAssertError(assignTok, var->name());
     }
-    // TODO: function calls on prevVar
+    // TODO: function calls on var
 }
 
 bool CheckAssert::inSameScope(const Token* returnTok, const Token* assignTok)
diff --git a/test/testassert.cpp b/test/testassert.cpp
@@ -183,7 +183,7 @@ class TestAssert : public TestFixture {
         ASSERT_EQUALS("", errout.str());
 
         check("void f(int a, int b) {\n"
-              "    assert(a == 2 && b = 1);\n"
+              "    assert(a == 2 && (b = 1));\n"
               "    return a;\n"
               "}\n"
              );
@@ -220,6 +220,13 @@ class TestAssert : public TestFixture {
               "}\n");
         ASSERT_EQUALS("[test.cpp:3]: (warning) Assert statement modifies 'a'.\n", errout.str());
 
+	check("void f() {\n"
+              "    int a = 0;\n"
+              "    assert(--a);\n"
+              "    return a;\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (warning) Assert statement modifies 'a'.\n", errout.str());
+
         check("void f() {\n"
               "  assert(std::all_of(first, last, []() {\n"
               "                  auto tmp = x.someValue();\n"
