diff --git a/lib/checkother.cpp b/lib/checkother.cpp index d8e065610..76af4e0a5 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -35,6 +35,60 @@ CheckOther instance; //--------------------------------------------------------------------------- +void CheckOther::clarifyCalculation() +{ + if (!_settings->_checkCodingStyle) + return; + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) + { + if (tok->str() == "?") + { + // condition + const Token *cond = tok->previous(); + if (cond->isName() || cond->isNumber()) + cond = cond->previous(); + else if (cond->str() == ")") + cond = cond->link()->previous(); + else + continue; + + // multiplication + if (cond->str() == "*") + cond = cond->previous(); + else + continue; + + // skip previous multiplications.. + while (cond && cond->strAt(-1) == "*" && (cond->isName() || cond->isNumber())) + cond = cond->tokAt(-2); + + if (!cond) + continue; + + // first multiplication operand + if (cond->str() == ")") + { + clarifyCalculationError(cond); + } + else if (cond->isName() || cond->isNumber()) + { + if (Token::Match(cond->previous(),"return|+|-|,|(")) + clarifyCalculationError(cond); + } + } + } +} + +void CheckOther::clarifyCalculationError(const Token *tok) +{ + reportError(tok, + Severity::information, + "clarifyCalculation", + "Please clarify precedence: 'a*b?..'\n" + "Found a suspicious multiplication of condition. Please use parantheses to clarify the code. " + "The code 'a*b?1:2' should be written as either '(a*b)?1:2' or 'a*(b?1:2)'."); +} + void CheckOther::warningOldStylePointerCast() { diff --git a/lib/checkother.h b/lib/checkother.h index 0d0798dd7..bace5a82e 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -70,6 +70,8 @@ public: { CheckOther checkOther(tokenizer, settings, errorLogger); + checkOther.clarifyCalculation(); + // Coding style checks checkOther.checkConstantFunctionParameter(); checkOther.checkIncompleteStatement(); @@ -87,6 +89,10 @@ public: checkOther.checkMemsetZeroBytes(); } + /** @brief Clarify calculation for ".. a * b ? .." */ + void clarifyCalculation(); + void clarifyCalculationError(const Token *tok); + /** @brief Are there C-style pointer casts in a c++ file? */ void warningOldStylePointerCast(); @@ -236,6 +242,7 @@ public: c.unassignedVariableError(0, "varname"); c.catchExceptionByValueError(0); c.memsetZeroBytesError(0, "varname"); + c.clarifyCalculationError(0); } std::string name() const @@ -274,6 +281,7 @@ public: "* assignment of a variable to itself\n" "* mutual exclusion over || always evaluating to true\n" "* exception caught by value instead of by reference\n" + "* Clarify calculation with parantheses\n" // optimisations "* optimisation: detect post increment/decrement\n"; diff --git a/test/testother.cpp b/test/testother.cpp index f71d7eb97..8546c5d83 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -99,6 +99,8 @@ private: TEST_CASE(memsetZeroBytes); TEST_CASE(sizeofForArrayParameter); + + TEST_CASE(clarifyCalculation); } void check(const char code[], const char *filename = NULL) @@ -134,6 +136,7 @@ private: checkOther.checkIncorrectLogicOperator(); checkOther.checkCatchExceptionByValue(); checkOther.checkMemsetZeroBytes(); + checkOther.clarifyCalculation(); } @@ -1748,8 +1751,19 @@ private: "}\n" ); ASSERT_EQUALS("", errout.str()); + } + void clarifyCalculation() + { + 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()); + 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()); } };