Fixed cppcheck-opensource#6960 (New check: enum variable is assigned mismatching value)

danmar · danmar · commit fb5e6d8a7dc5 · 2016-05-10T20:22:35.000+02:00
diff --git a/lib/checktype.cpp b/lib/checktype.cpp
@@ -300,3 +300,71 @@ void CheckType::longCastReturnError(const Token *tok)
                 "int result is returned as long value. If the return value is long to avoid loss of information, then you have loss of information.\n"
                 "int result is returned as long value. If the return value is long to avoid loss of information, then there is loss of information. To avoid loss of information you must cast a calculation operand to long, for example 'return a*b;' => 'return (long)a*b'.");
 }
+
+static const ValueFlow::Value *mismatchingValue(const ValueType *enumType, const std::list<ValueFlow::Value> &values)
+{
+    if (!enumType || !enumType->typeScope || enumType->typeScope->type != Scope::eEnum)
+        return nullptr;
+    const Scope * const enumScope = enumType->typeScope;
+    for (std::list<ValueFlow::Value>::const_iterator it = values.begin(); it != values.end(); ++it) {
+        if (it->tokvalue)
+            continue;
+        bool found = false;
+        for (unsigned int i = 0; i < enumScope->enumeratorList.size(); ++i) {
+            if (enumScope->enumeratorList[i].value_known && enumScope->enumeratorList[i].value == it->intvalue) {
+                found = true;
+                break;
+            }
+        }
+        if (!found)
+            return &(*it);
+    }
+    return nullptr;
+}
+
+void CheckType::checkEnumMismatch()
+{
+    if (!_settings->isEnabled("style"))
+        return;
+    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
+        // Assigning mismatching value to enum variable
+        if (tok->str() == "=") {
+            if (!tok->astOperand1() || !tok->astOperand2())
+                continue;
+
+            const ValueFlow::Value *v = mismatchingValue(tok->astOperand1()->valueType(), tok->astOperand2()->values);
+            if (v)
+                enumMismatchAssignError(tok, *v);
+        }
+
+        // Comparing enum variable against mismatching value
+        else if (tok->isComparisonOp()) {
+            if (!tok->astOperand1() || !tok->astOperand2())
+                continue;
+
+            const ValueFlow::Value * const v1 = mismatchingValue(tok->astOperand1()->valueType(), tok->astOperand2()->values);
+            if (v1)
+                enumMismatchCompareError(tok, *v1);
+
+            const ValueFlow::Value * const v2 = mismatchingValue(tok->astOperand2()->valueType(), tok->astOperand1()->values);
+            if (v2)
+                enumMismatchCompareError(tok, *v2);
+        }
+    }
+}
+
+void CheckType::enumMismatchAssignError(const Token *tok, const ValueFlow::Value &value)
+{
+    reportError(tok,
+                Severity::style,
+                "enumMismatch",
+                "Assigning mismatching value " + MathLib::toString(value.intvalue) + " to enum variable.");
+}
+
+void CheckType::enumMismatchCompareError(const Token *tok, const ValueFlow::Value &value)
+{
+    reportError(tok,
+                Severity::style,
+                "enumMismatch",
+                "Comparing mismatching value " + MathLib::toString(value.intvalue) + " with enum variable.");
+}
diff --git a/lib/checktype.h b/lib/checktype.h
@@ -50,6 +50,7 @@ class CPPCHECKLIB CheckType : public Check {
         checkType.checkIntegerOverflow();
         checkType.checkSignConversion();
         checkType.checkLongCast();
+        checkType.checkEnumMismatch();
     }
 
     /** @brief Run checks against the simplified token list */
@@ -70,6 +71,9 @@ class CPPCHECKLIB CheckType : public Check {
 
     /** @brief %Check for implicit long cast of int result */
     void checkLongCast();
+
+    /** @brief %Check for mismatching enum usage */
+    void checkEnumMismatch();
 private:
 
     // Error messages..
@@ -78,6 +82,8 @@ class CPPCHECKLIB CheckType : public Check {
     void signConversionError(const Token *tok, const bool constvalue);
     void longCastAssignError(const Token *tok);
     void longCastReturnError(const Token *tok);
+    void enumMismatchAssignError(const Token *tok, const ValueFlow::Value &value);
+    void enumMismatchCompareError(const Token *tok, const ValueFlow::Value &value);
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
         CheckType c(nullptr, settings, errorLogger);
@@ -86,6 +92,7 @@ class CPPCHECKLIB CheckType : public Check {
         c.signConversionError(nullptr, false);
         c.longCastAssignError(nullptr);
         c.longCastReturnError(nullptr);
+        c.enumMismatchAssignError(nullptr, ValueFlow::Value(1000));
     }
 
     static std::string myName() {
@@ -98,7 +105,8 @@ class CPPCHECKLIB CheckType : public Check {
                "- signed integer overflow (only enabled when --platform is used)\n"
                "- dangerous sign conversion, when signed value can be negative\n"
                "- possible loss of information when assigning int result to long variable\n"
-               "- possible loss of information when returning int result as long return value\n";
+               "- possible loss of information when returning int result as long return value\n"
+               "- enum usage with mismatching values\n";
     }
 };
 /// @}
diff --git a/test/testtype.cpp b/test/testtype.cpp
@@ -39,6 +39,8 @@ class TestType : public TestFixture {
         TEST_CASE(signConversion);
         TEST_CASE(longCastAssign);
         TEST_CASE(longCastReturn);
+        TEST_CASE(enumMismatchAssign);
+        TEST_CASE(enumMismatchCompare);
     }
 
     void check(const char code[], Settings* settings = 0) {
@@ -199,6 +201,35 @@ class TestType : public TestFixture {
               "}\n", &settings);
         ASSERT_EQUALS("", errout.str());
     }
+
+    void enumMismatchAssign() {
+        Settings settings;
+        settings.addEnabled("style");
+
+        check("enum ABC {A,B,C};\n"
+              "void f() {\n"
+              "  enum ABC abc = 5;\n"
+              "}", &settings);
+        ASSERT_EQUALS("[test.cpp:3]: (style) Assigning mismatching value 5 to enum variable.\n", errout.str());
+    }
+
+    void enumMismatchCompare() {
+        Settings settings;
+        settings.addEnabled("style");
+
+        check("enum ABC {A,B,C};\n"
+              "void f(enum ABC abc) {\n"
+              "  if (abc==5) {}\n"
+              "}", &settings);
+        ASSERT_EQUALS("[test.cpp:3]: (style) Comparing mismatching value 5 with enum variable.\n", errout.str());
+
+        check("enum ABC {A,B,C};\n"
+              "void f(enum ABC abc) {\n"
+              "  if (5==abc) {}\n"
+              "}", &settings);
+        ASSERT_EQUALS("[test.cpp:3]: (style) Comparing mismatching value 5 with enum variable.\n", errout.str());
+    }
+
 };
 
 REGISTER_TEST(TestType)
