fix #2968 (new check: testing if unsigned variable is less than 0)

lib/checkother.cpp

@@ -3866,3 +3866,63 @@ void CheckOther::assignBoolToPointerError(const Token *tok)
     reportError(tok, Severity::error, "assignBoolToPointer",
                 "Assigning bool value to pointer (converting bool value to address)");
 }
+
+//---------------------------------------------------------------------------
+// Check testing sign of unsigned variables.
+//---------------------------------------------------------------------------
+
+void CheckOther::checkSignOfUnsignedVariable()
+{
+    if (!_settings->_checkCodingStyle)
+        return;
+
+    const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
+
+    std::list<Scope>::const_iterator scope;
+
+    for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope)
+    {
+        // only check functions
+        if (scope->type != Scope::eFunction)
+            continue;
+
+        // check all the code in the function
+        for (const Token *tok = scope->classStart; tok && tok != scope->classStart->link(); tok = tok->next())
+        {
+            if (Token::Match(tok, "( %var% <|<= 0 )") && tok->next()->varId())
+            {
+                const Variable * var = symbolDatabase->getVariableFromVarId(tok->next()->varId());
+                if (var && var->typeEndToken()->isUnsigned())
+                    unsignedLessThanZero(tok->next(), tok->next()->str());
+            }
+            else if (Token::Match(tok, "( 0 > %var% )") && tok->tokAt(3)->varId())
+            {
+                const Variable * var = symbolDatabase->getVariableFromVarId(tok->tokAt(3)->varId());
+                if (var && var->typeEndToken()->isUnsigned())
+                    unsignedLessThanZero(tok->tokAt(3), tok->strAt(3));
+            }
+            else if (Token::Match(tok, "( 0 <= %var% )") && tok->tokAt(3)->varId())
+            {
+                const Variable * var = symbolDatabase->getVariableFromVarId(tok->tokAt(3)->varId());
+                if (var && var->typeEndToken()->isUnsigned())
+                    unsignedPositive(tok->tokAt(3), tok->strAt(3));
+            }
+        }
+    }
+}
+
+void CheckOther::unsignedLessThanZero(const Token *tok, const std::string &varname)
+{
+    reportError(tok, Severity::style, "unsignedLessThanZero",
+                "Checking if unsigned variable '" + varname + "' is less than zero.\n"
+                "An unsigned variable will never be negative so it is either pointless or "
+                "an error to check if it is.");
+}
+
+void CheckOther::unsignedPositive(const Token *tok, const std::string &varname)
+{
+    reportError(tok, Severity::style, "unsignedPositive",
+                "Checking if unsigned variable '" + varname + "' is positive is always true.\n"
+                "An unsigned variable will never alwayw be positive so it is either pointless or "
+                "an error to check if it is.");
+}

lib/checkother.h

@@ -103,6 +103,7 @@ public:
         checkOther.checkAlwaysTrueOrFalseStringCompare();
 
         checkOther.checkAssignBoolToPointer();
+        checkOther.checkSignOfUnsignedVariable();
     }
 
     /** @brief Clarify calculation for ".. a * b ? .." */
@@ -233,10 +234,12 @@ public:
     /** @brief check if token is a record type without side effects */
     bool isRecordTypeWithoutSideEffects(const Token *tok);
 
-
     /** @brief assigning bool to pointer */
     void checkAssignBoolToPointer();
 
+    /** @brief %Check for testing sign of unsigned variable */
+    void checkSignOfUnsignedVariable();
+
     // Error messages..
     void cstyleCastError(const Token *tok);
     void dangerousUsageStrtolError(const Token *tok);
@@ -272,6 +275,8 @@ public:
     void alwaysTrueFalseStringCompare(const Token *tok, const std::string& str1, const std::string& str2);
     void duplicateBreakError(const Token *tok);
     void assignBoolToPointerError(const Token *tok);
+    void unsignedLessThanZero(const Token *tok, const std::string &varname);
+    void unsignedPositive(const Token *tok, const std::string &varname);
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings)
     {
@@ -323,6 +328,8 @@ public:
         c.duplicateExpressionError(0, 0, "&&");
         c.alwaysTrueFalseStringCompare(0, "str1", "str2");
         c.duplicateBreakError(0);
+        c.unsignedLessThanZero(0, "varname");
+        c.unsignedPositive(0, "varname");
     }
 
     std::string myName() const
@@ -370,6 +377,8 @@ public:
                "* suspicious condition (assignment+comparison)\n"
                "* suspicious condition (runtime comparison of string literals)\n"
                "* duplicate break statement\n"
+               "* testing if unsigned variable is negative\n"
+               "* testing is unsigned variable is positive\n"
 
                // optimisations
                "* optimisation: detect post increment/decrement\n";

test/testother.cpp

@@ -127,6 +127,7 @@ private:
         TEST_CASE(duplicateExpression2); // ticket #2730
 
         TEST_CASE(alwaysTrueFalseStringCompare);
+        TEST_CASE(checkSignOfUnsignedVariable);
     }
 
     void check(const char code[], const char *filename = NULL)
@@ -2888,6 +2889,75 @@ private:
             "}");
         ASSERT_EQUALS("", errout.str());
     }
+
+    void check_signOfUnsignedVariable(const char code[])
+    {
+        // Clear the error buffer..
+        errout.str("");
+
+        Settings settings;
+        settings._checkCodingStyle = true;
+
+        // Tokenize..
+        Tokenizer tokenizer(&settings, this);
+        std::istringstream istr(code);
+        tokenizer.tokenize(istr, "test.cpp");
+
+        // Check for redundant code..
+        CheckOther checkOther(&tokenizer, &settings, this);
+        checkOther.checkSignOfUnsignedVariable();
+    }
+
+    void checkSignOfUnsignedVariable()
+    {
+        check_signOfUnsignedVariable(
+            "bool foo(unsigned int x) {\n"
+            "  if (x < 0)"
+            "    return true;\n"
+            "  return false;\n"
+            "}");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Checking if unsigned variable 'x' is less than zero.\n", errout.str());
+
+        check_signOfUnsignedVariable(
+            "bool foo(int x) {\n"
+            "  if (x < 0)"
+            "    return true;\n"
+            "  return false;\n"
+            "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check_signOfUnsignedVariable(
+            "bool foo(unsigned int x) {\n"
+            "  if (0 > x)"
+            "    return true;\n"
+            "  return false;\n"
+            "}");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Checking if unsigned variable 'x' is less than zero.\n", errout.str());
+
+        check_signOfUnsignedVariable(
+            "bool foo(int x) {\n"
+            "  if (0 > x)"
+            "    return true;\n"
+            "  return false;\n"
+            "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check_signOfUnsignedVariable(
+            "bool foo(unsigned int x) {\n"
+            "  if (x >= 0)"
+            "    return true;\n"
+            "  return false;\n"
+            "}");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Checking if unsigned variable 'x' is positive is always true.\n", errout.str());
+
+        check_signOfUnsignedVariable(
+            "bool foo(int x) {\n"
+            "  if (x >= 0)"
+            "    return true;\n"
+            "  return false;\n"
+            "}");
+        ASSERT_EQUALS("", errout.str());
+    }
 };
 
 REGISTER_TEST(TestOther)