diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 01ea707cc..41a1a6774 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -85,12 +85,13 @@ void CheckOther::clarifyCalculation() else continue; - // multiplication - if (cond && cond->str() == "*") - cond = cond->previous(); - else + // calculation + if (!Token::Match(cond, "[+-*/]")) continue; + const char op = cond->str()[0]; + cond = cond->previous(); + // skip previous multiplications.. while (cond && cond->strAt(-1) == "*" && (cond->isName() || cond->isNumber())) cond = cond->tokAt(-2); @@ -101,25 +102,34 @@ void CheckOther::clarifyCalculation() // first multiplication operand if (cond->str() == ")") { - clarifyCalculationError(cond); + clarifyCalculationError(cond, op); } else if (cond->isName() || cond->isNumber()) { if (Token::Match(cond->previous(),"return|+|-|,|(")) - clarifyCalculationError(cond); + clarifyCalculationError(cond, op); } } } } -void CheckOther::clarifyCalculationError(const Token *tok) +void CheckOther::clarifyCalculationError(const Token *tok, char op) { + // suspicious calculation + const std::string calc(std::string("'a") + op + "b?c:d'"); + + // recommended calculation #1 + const std::string s1(std::string("'(a") + op + "b)?c:d'"); + + // recommended calculation #2 + const std::string s2(std::string("'a") + op + "(b?c:d)'"); + reportError(tok, Severity::style, "clarifyCalculation", - "Please clarify precedence: 'a*b?..'\n" - "Found a suspicious multiplication of condition. Please use parentheses to clarify the code. " - "The code 'a*b?1:2' should be written as either '(a*b)?1:2' or 'a*(b?1:2)'."); + "Suspicious calculation, " + calc + " can be clarified as either " + s1 + " or " + s2 + "\n" + + "Suspicious calculation. Please use parentheses to clarify the code. " + "The code " + calc + " should be written as either " + s1 + " or " + s2 + "."); } diff --git a/lib/checkother.h b/lib/checkother.h index 1e65b7e6e..ad2b8c5ee 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -97,7 +97,7 @@ public: /** @brief Clarify calculation for ".. a * b ? .." */ void clarifyCalculation(); - void clarifyCalculationError(const Token *tok); + void clarifyCalculationError(const Token *tok, char op); /** @brief Suspicious condition (assignment+comparison) */ void clarifyCondition(); @@ -269,7 +269,7 @@ public: c.unassignedVariableError(0, "varname"); c.catchExceptionByValueError(0); c.memsetZeroBytesError(0, "varname"); - c.clarifyCalculationError(0); + c.clarifyCalculationError(0, '+'); c.clarifyConditionError(0); c.incorrectStringCompareError(0, "substr", "\"Hello World\"", "12"); c.incrementBooleanError(0); diff --git a/test/testother.cpp b/test/testother.cpp index 123632126..8126ad0a4 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -2274,18 +2274,23 @@ private: check("int f(char c) {\n" " return 10 * (c == 0) ? 1 : 2;\n" "}"); - ASSERT_EQUALS("[test.cpp:2]: (style) Please clarify precedence: 'a*b?..'\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious calculation, 'a*b?c:d' can be clarified as either '(a*b)?c:d' or 'a*(b?c:d)'\n", errout.str()); check("void f(char c) {\n" " printf(\"%i\", 10 * (c == 0) ? 1 : 2);\n" "}"); - ASSERT_EQUALS("[test.cpp:2]: (style) Please clarify precedence: 'a*b?..'\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious calculation, 'a*b?c:d' can be clarified as either '(a*b)?c:d' or 'a*(b?c:d)'\n", errout.str()); // Ticket #2585 - segmentation fault for invalid code check("abcdef?" "?<" "123456?" "?>" "+?" "?="); ASSERT_EQUALS("", errout.str()); + + check("void f(char c) {\n" + " printf(\"%i\", 1 + 1 ? 1 : 2);\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious calculation, 'a+b?c:d' can be clarified as either '(a+b)?c:d' or 'a+(b?c:d)'\n", errout.str()); } // clarify conditions with = and comparison