diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 2b9674056..97744c092 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -115,7 +115,7 @@ void CheckOther::clarifyCalculation() void CheckOther::clarifyCalculationError(const Token *tok) { reportError(tok, - Severity::information, + Severity::style, "clarifyCalculation", "Please clarify precedence: 'a*b?..'\n" "Found a suspicious multiplication of condition. Please use parantheses to clarify the code. " @@ -123,6 +123,41 @@ void CheckOther::clarifyCalculationError(const Token *tok) } +// Clarify condition '(x = a < 0)' into '((x = a) < 0)' or '(x = (a < 0))' +void CheckOther::clarifyCondition() +{ + if (!_settings->_checkCodingStyle) + return; + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) + { + if (Token::Match(tok, "( %var% =")) + { + for (const Token *tok2 = tok->tokAt(2); tok2; tok2 = tok2->next()) + { + if (tok2->str() == "(" || tok2->str() == "[") + tok2 = tok2->link(); + else if (tok2->str() == "||" || tok2->str() == "&&" || tok2->str() == "?") + break; + else if (Token::Match(tok2, "<|<=|==|!=|>|>= %num% )")) + { + clarifyConditionError(tok); + break; + } + } + } + } +} + +void CheckOther::clarifyConditionError(const Token *tok) +{ + reportError(tok, + Severity::style, + "clarifyCondition", + "Suspicious condition (assignment+comparison), it can be clarified with parantheses"); +} + + + void CheckOther::warningOldStylePointerCast() { if (!_settings->_checkCodingStyle || diff --git a/lib/checkother.h b/lib/checkother.h index 80605333b..2855c5bb1 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -64,6 +64,7 @@ public: checkOther.checkAssignmentInAssert(); checkOther.checkSizeofForArrayParameter(); checkOther.checkSelfAssignment(); + checkOther.clarifyCondition(); // not simplified because ifAssign } /** @brief Run checks against the simplified token list */ @@ -97,6 +98,10 @@ public: void clarifyCalculation(); void clarifyCalculationError(const Token *tok); + /** @brief Suspicious condition (assignment+comparison) */ + void clarifyCondition(); + void clarifyConditionError(const Token *tok); + /** @brief Are there C-style pointer casts in a c++ file? */ void warningOldStylePointerCast(); @@ -264,6 +269,7 @@ public: c.catchExceptionByValueError(0); c.memsetZeroBytesError(0, "varname"); c.clarifyCalculationError(0); + c.clarifyConditionError(0); c.incorrectStringCompareError(0, "substr", "\"Hello World\"", "12"); c.incrementBooleanError(0); c.comparisonOfBoolWithIntError(0, "varname"); @@ -309,6 +315,7 @@ public: "* Clarify calculation with parantheses\n" "* using increment on boolean\n" "* comparison of a boolean with a non-zero integer\n" + "* suspicious condition (assignment+comparison)" // optimisations "* optimisation: detect post increment/decrement\n"; diff --git a/test/testother.cpp b/test/testother.cpp index 8366b4d68..d6fb26305 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -105,6 +105,8 @@ private: TEST_CASE(clarifyCalculation); + TEST_CASE(clarifyCondition); // if (a = b() < 0) + TEST_CASE(incorrectStringCompare); TEST_CASE(incrementBoolean); @@ -131,6 +133,7 @@ private: checkOther.checkRedundantAssignmentInSwitch(); checkOther.checkAssignmentInAssert(); checkOther.checkSizeofForArrayParameter(); + checkOther.clarifyCondition(); // Simplify token list.. tokenizer.simplifyTokenList(); @@ -2206,12 +2209,12 @@ private: check("int f(char c) {\n" " return 10 * (c == 0) ? 1 : 2;\n" "}"); - ASSERT_EQUALS("[test.cpp:2]: (information) Please clarify precedence: 'a*b?..'\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style) Please clarify precedence: 'a*b?..'\n", errout.str()); check("void f(char c) {\n" " printf(\"%i\", 10 * (c == 0) ? 1 : 2);\n" "}"); - ASSERT_EQUALS("[test.cpp:2]: (information) Please clarify precedence: 'a*b?..'\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style) Please clarify precedence: 'a*b?..'\n", errout.str()); // Ticket #2585 - segmentation fault for invalid code check("abcdef?" "?<" @@ -2220,6 +2223,15 @@ private: ASSERT_EQUALS("", errout.str()); } + // clarify conditions with = and comparison + void clarifyCondition() + { + check("void f() {\n" + " if (x = b() < 0) {}\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious condition (assignment+comparison), it can be clarified with parantheses\n", errout.str()); + } + void incorrectStringCompare() { check("int f() {\n"