diff --git a/lib/checkassignif.cpp b/lib/checkassignif.cpp index 06b87c302..e34d2dc03 100644 --- a/lib/checkassignif.cpp +++ b/lib/checkassignif.cpp @@ -132,3 +132,63 @@ void CheckAssignIf::comparisonError(const Token *tok, bool result) reportError(tok, Severity::style, "comparisonError", errmsg); } + + + + + + +void CheckAssignIf::multicompare() +{ + if (!_settings->_checkCodingStyle) + return; + + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) + { + if (Token::Match(tok, "if ( %var% & %num% ) {")) + { + const unsigned int varid(tok->tokAt(2)->varId()); + if (varid == 0) + continue; + + const MathLib::bigint num1 = MathLib::toLongNumber(tok->strAt(4)); + if (num1 < 0) + continue; + + const Token *tok2 = tok->tokAt(6)->link(); + while (Token::simpleMatch(tok2, "} else { if (")) + { + // Goto '(' + const Token * const opar = tok2->tokAt(4); + + // tok2: skip if-block + tok2 = opar->link(); + if (Token::simpleMatch(tok2, ") {")) + tok2 = tok2->next()->link(); + + // check condition.. + if (Token::Match(opar, "( %varid% == %num% &&|%oror%|)", varid)) + { + const MathLib::bigint num2 = MathLib::toLongNumber(opar->strAt(3)); + if (num2 < 0) + continue; + + if ((num1 & num2) == num2) + { + multicompareError(opar, tok->linenr()); + } + } + } + } + } +} + +void CheckAssignIf::multicompareError(const Token *tok, unsigned int line1) +{ + std::ostringstream errmsg; + errmsg << "Comparison is always false because otherwise the condition at line " + << line1 + << " is not false"; + + reportError(tok, Severity::style, "multicompare", errmsg.str()); +} diff --git a/lib/checkassignif.h b/lib/checkassignif.h index 6ec4c26b2..3b7d47c9c 100644 --- a/lib/checkassignif.h +++ b/lib/checkassignif.h @@ -52,6 +52,7 @@ public: CheckAssignIf checkAssignIf(tokenizer, settings, errorLogger); checkAssignIf.assignIf(); checkAssignIf.comparison(); + checkAssignIf.multicompare(); } /** mismatching assignment / comparison */ @@ -60,17 +61,21 @@ public: /** mismatching lhs and rhs in comparison */ void comparison(); + /** match 'if' and 'else if' conditions */ + void multicompare(); + private: void assignIfError(const Token *tok, bool result); - void comparisonError(const Token *tok, bool result); + void multicompareError(const Token *tok, unsigned int line1); void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) { CheckAssignIf c(0, settings, errorLogger); c.assignIfError(0, false); c.comparisonError(0, false); + c.multicompareError(0,1); } std::string myName() const @@ -82,7 +87,8 @@ private: { return "Match assignments and conditions:\n" " * Mismatching assignment and comparison => comparison is always true/false\n" - " * Mismatching lhs and rhs in comparison => comparison is always true/false"; + " * Mismatching lhs and rhs in comparison => comparison is always true/false\n" + " * Match 'if' and 'else if' conditions"; } }; /// @} diff --git a/test/testassignif.cpp b/test/testassignif.cpp index cfbfab7b1..28810b187 100644 --- a/test/testassignif.cpp +++ b/test/testassignif.cpp @@ -35,8 +35,9 @@ private: void run() { - TEST_CASE(assignAndCompare); - TEST_CASE(compare); + TEST_CASE(assignAndCompare); // assignment and comparison don't match + TEST_CASE(compare); // mismatching LHS/RHS in comparison + TEST_CASE(multicompare); // mismatching comparisons } void check(const char code[]) @@ -58,6 +59,7 @@ private: CheckAssignIf checkAssignIf(&tokenizer, &settings, this); checkAssignIf.assignIf(); checkAssignIf.comparison(); + checkAssignIf.multicompare(); } void assignAndCompare() @@ -97,6 +99,17 @@ private: "}\n"); ASSERT_EQUALS("[test.cpp:3]: (style) Mismatching comparison, the result is always true\n", errout.str()); } + + void multicompare() + { + check("void foo(int x)\n" + "{\n" + " if (x & 7);\n" + " else if (x == 1);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (style) Comparison is always false because otherwise the condition at line 3 is not false\n", errout.str()); + + } }; REGISTER_TEST(TestAssignIf)