diff --git a/lib/checktype.cpp b/lib/checktype.cpp index a52e74095..0b7eb84b9 100644 --- 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 &values) +{ + if (!enumType || !enumType->typeScope || enumType->typeScope->type != Scope::eEnum) + return nullptr; + const Scope * const enumScope = enumType->typeScope; + for (std::list::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 index fa5c0b629..d43bd6095 100644 --- a/lib/checktype.h +++ b/lib/checktype.h @@ -50,6 +50,7 @@ public: checkType.checkIntegerOverflow(); checkType.checkSignConversion(); checkType.checkLongCast(); + checkType.checkEnumMismatch(); } /** @brief Run checks against the simplified token list */ @@ -70,6 +71,9 @@ public: /** @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 @@ private: 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 @@ private: c.signConversionError(nullptr, false); c.longCastAssignError(nullptr); c.longCastReturnError(nullptr); + c.enumMismatchAssignError(nullptr, ValueFlow::Value(1000)); } static std::string myName() { @@ -98,7 +105,8 @@ private: "- 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 index 13c37917c..767e8460d 100644 --- a/test/testtype.cpp +++ b/test/testtype.cpp @@ -39,6 +39,8 @@ private: 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 @@ private: "}\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)