Fixed #432 (New check: wrong usage of ! operator in conditions)

lib/checkother.cpp

@@ -608,6 +608,35 @@ void CheckOther::invalidScanf()
     }
 }
 
+//---------------------------------------------------------------------------
+//    if (!x==3) <- Probably meant to be "x!=3"
+//---------------------------------------------------------------------------
+void CheckOther::checkComparisonOfBoolWithInt()
+{
+    if (!_settings->_checkCodingStyle)
+        return;
+
+    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
+    {
+        if (Token::Match(tok, "( ! %var% ==|!= %num% )"))
+        {
+            const Token *numTok = tok->tokAt(4);
+            if (numTok && numTok->str() != "0")
+            {
+                comparisonOfBoolWithIntError(numTok, tok->strAt(2));
+            }
+        }
+        else if (Token::Match(tok, "( %num% ==|!= ! %var% )"))
+        {
+            const Token *numTok = tok->tokAt(1);
+            if (numTok && numTok->str() != "0")
+            {
+                comparisonOfBoolWithIntError(numTok, tok->strAt(4));
+            }
+        }
+    }
+}
+
 void CheckOther::sizeofForArrayParameterError(const Token *tok)
 {
     reportError(tok, Severity::error,
@@ -3016,3 +3045,10 @@ void CheckOther::incorrectStringCompareError(const Token *tok, const std::string
 {
     reportError(tok, Severity::warning, "incorrectStringCompare", "String literal " + string + " doesn't match length argument for " + func + "(" + len + ").");
 }
+
+void CheckOther::comparisonOfBoolWithIntError(const Token *tok, const std::string &varname)
+{
+    reportError(tok, Severity::warning, "comparisonOfBoolWithInt",
+                "Comparison of a boolean with a non-zero integer\n"
+                "The expression \"!" + varname + "\" is of type 'bool' but is compared against a non-zero 'int'.");
+}

lib/checkother.h

@@ -89,6 +89,7 @@ public:
         checkOther.checkMemsetZeroBytes();
         checkOther.checkIncorrectStringCompare();
         checkOther.checkIncrementBoolean();
+        checkOther.checkComparisonOfBoolWithInt();
     }
 
     /** @brief Clarify calculation for ".. a * b ? .." */
@@ -188,6 +189,9 @@ public:
     /** @brief %Check for using postfix increment on bool */
     void checkIncrementBoolean();
 
+    /** @brief %Check for suspicious comparison of a bool and a non-zero (and non-one) value (e.g. "if (!x==4)") */
+    void checkComparisonOfBoolWithInt();
+
     // Error messages..
     void cstyleCastError(const Token *tok);
     void dangerousUsageStrtolError(const Token *tok);
@@ -214,6 +218,7 @@ public:
     void sizeofForArrayParameterError(const Token *tok);
     void incorrectStringCompareError(const Token *tok, const std::string& func, const std::string &string, const std::string &len);
     void incrementBooleanError(const Token *tok);
+    void comparisonOfBoolWithIntError(const Token *tok, const std::string &varname);
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings)
     {
@@ -255,6 +260,7 @@ public:
         c.clarifyCalculationError(0);
         c.incorrectStringCompareError(0, "substr", "\"Hello World\"", "12");
         c.incrementBooleanError(0);
+        c.comparisonOfBoolWithIntError(0, "varname");
     }
 
     std::string myName() const
@@ -296,6 +302,7 @@ public:
                "* exception caught by value instead of by reference\n"
                "* Clarify calculation with parantheses\n"
                "* using increment on boolean\n"
+               "* comparison of a boolean with a non-zero integer\n"
 
                // optimisations
                "* optimisation: detect post increment/decrement\n";

test/testother.cpp

@@ -106,6 +106,7 @@ private:
         TEST_CASE(incorrectStringCompare);
 
         TEST_CASE(incrementBoolean);
+        TEST_CASE(comparisonOfBoolWithInt);
     }
 
     void check(const char code[], const char *filename = NULL)
@@ -144,6 +145,7 @@ private:
         checkOther.clarifyCalculation();
         checkOther.checkIncorrectStringCompare();
         checkOther.checkIncrementBoolean();
+        checkOther.checkComparisonOfBoolWithInt();
     }
 
 
@@ -1923,6 +1925,51 @@ private:
               "}");
         ASSERT_EQUALS("", errout.str());
     }
+
+    void comparisonOfBoolWithInt()
+    {
+        check("void f(int x) {\n"
+              "    if (!x == 10) {\n"
+              "        printf(\"x not equal to 10\");\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with a non-zero integer\n", errout.str());
+
+        check("void f(int x) {\n"
+              "    if (!x != 10) {\n"
+              "        printf(\"x not equal to 10\");\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with a non-zero integer\n", errout.str());
+
+        check("void f(int x) {\n"
+              "    if (x != 10) {\n"
+              "        printf(\"x not equal to 10\");\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f(int x) {\n"
+              "    if (10 == !x) {\n"
+              "        printf(\"x not equal to 10\");\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with a non-zero integer\n", errout.str());
+
+        check("void f(int x) {\n"
+              "    if (10 != !x) {\n"
+              "        printf(\"x not equal to 10\");\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean with a non-zero integer\n", errout.str());
+
+        check("void f(int x) {\n"
+              "    if (10 != x) {\n"
+              "        printf(\"x not equal to 10\");\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+    }
 };
 
 REGISTER_TEST(TestOther)