Suspicious condition: Added new check for conditions that contains assignment+comparison

lib/checkother.cpp

@@ -115,7 +115,7 @@ void CheckOther::clarifyCalculation()
 void CheckOther::clarifyCalculationError(const Token *tok)
 {
     reportError(tok,
-                Severity::information,
+                Severity::style,
                 "clarifyCalculation",
                 "Please clarify precedence: 'a*b?..'\n"
                 "Found a suspicious multiplication of condition. Please use parantheses to clarify the code. "
@@ -123,6 +123,41 @@ void CheckOther::clarifyCalculationError(const Token *tok)
 }
 
 
+// Clarify condition '(x = a < 0)' into '((x = a) < 0)' or '(x = (a < 0))'
+void CheckOther::clarifyCondition()
+{
+    if (!_settings->_checkCodingStyle)
+        return;
+    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
+    {
+        if (Token::Match(tok, "( %var% ="))
+        {
+            for (const Token *tok2 = tok->tokAt(2); tok2; tok2 = tok2->next())
+            {
+                if (tok2->str() == "(" || tok2->str() == "[")
+                    tok2 = tok2->link();
+                else if (tok2->str() == "||" || tok2->str() == "&&" || tok2->str() == "?")
+                    break;
+                else if (Token::Match(tok2, "<|<=|==|!=|>|>= %num% )"))
+                {
+                    clarifyConditionError(tok);
+                    break;
+                }
+            }
+        }
+    }
+}
+
+void CheckOther::clarifyConditionError(const Token *tok)
+{
+    reportError(tok,
+                Severity::style,
+                "clarifyCondition",
+                "Suspicious condition (assignment+comparison), it can be clarified with parantheses");
+}
+
+
+
 void CheckOther::warningOldStylePointerCast()
 {
     if (!_settings->_checkCodingStyle ||

lib/checkother.h

@@ -64,6 +64,7 @@ public:
         checkOther.checkAssignmentInAssert();
         checkOther.checkSizeofForArrayParameter();
         checkOther.checkSelfAssignment();
+        checkOther.clarifyCondition();   // not simplified because ifAssign
     }
 
     /** @brief Run checks against the simplified token list */
@@ -97,6 +98,10 @@ public:
     void clarifyCalculation();
     void clarifyCalculationError(const Token *tok);
 
+    /** @brief Suspicious condition (assignment+comparison) */
+    void clarifyCondition();
+    void clarifyConditionError(const Token *tok);
+
     /** @brief Are there C-style pointer casts in a c++ file? */
     void warningOldStylePointerCast();
 
@@ -264,6 +269,7 @@ public:
         c.catchExceptionByValueError(0);
         c.memsetZeroBytesError(0, "varname");
         c.clarifyCalculationError(0);
+        c.clarifyConditionError(0);
         c.incorrectStringCompareError(0, "substr", "\"Hello World\"", "12");
         c.incrementBooleanError(0);
         c.comparisonOfBoolWithIntError(0, "varname");
@@ -309,6 +315,7 @@ public:
                "* Clarify calculation with parantheses\n"
                "* using increment on boolean\n"
                "* comparison of a boolean with a non-zero integer\n"
+               "* suspicious condition (assignment+comparison)"
 
                // optimisations
                "* optimisation: detect post increment/decrement\n";

test/testother.cpp

@@ -105,6 +105,8 @@ private:
 
         TEST_CASE(clarifyCalculation);
 
+        TEST_CASE(clarifyCondition);     // if (a = b() < 0)
+
         TEST_CASE(incorrectStringCompare);
 
         TEST_CASE(incrementBoolean);
@@ -131,6 +133,7 @@ private:
         checkOther.checkRedundantAssignmentInSwitch();
         checkOther.checkAssignmentInAssert();
         checkOther.checkSizeofForArrayParameter();
+        checkOther.clarifyCondition();
 
         // Simplify token list..
         tokenizer.simplifyTokenList();
@@ -2206,12 +2209,12 @@ private:
         check("int f(char c) {\n"
               "    return 10 * (c == 0) ? 1 : 2;\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:2]: (information) Please clarify precedence: 'a*b?..'\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:2]: (style) Please clarify precedence: 'a*b?..'\n", errout.str());
 
         check("void f(char c) {\n"
               "    printf(\"%i\", 10 * (c == 0) ? 1 : 2);\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:2]: (information) Please clarify precedence: 'a*b?..'\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:2]: (style) Please clarify precedence: 'a*b?..'\n", errout.str());
 
         // Ticket #2585 - segmentation fault for invalid code
         check("abcdef?" "?<"
@@ -2220,6 +2223,15 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    // clarify conditions with = and comparison
+    void clarifyCondition()
+    {
+        check("void f() {\n"
+              "    if (x = b() < 0) {}\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious condition (assignment+comparison), it can be clarified with parantheses\n", errout.str());
+    }
+
     void incorrectStringCompare()
     {
         check("int f() {\n"