New check: warn about such suspicious conditions: '(a & b == c)'

This commit is contained in:
Daniel Marjamäki 2011-08-19 00:14:15 +02:00
parent 8bc7b5c5b9
commit a7728fef48
3 changed files with 26 additions and 8 deletions

View File

@ -137,13 +137,14 @@ void CheckOther::clarifyCalculationError(const Token *tok, const std::string &op
// Clarify condition '(x = a < 0)' into '((x = a) < 0)' or '(x = (a < 0))' // Clarify condition '(x = a < 0)' into '((x = a) < 0)' or '(x = (a < 0))'
// Clarify condition '(a & b == c)' into '((a & b) == c)' or '(a & (b == c))'
void CheckOther::clarifyCondition() void CheckOther::clarifyCondition()
{ {
if (!_settings->isEnabled("style")) if (!_settings->isEnabled("style"))
return; return;
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
{ {
if (Token::Match(tok, "( %var% =")) if (Token::Match(tok, "( %var% [=&|^]"))
{ {
for (const Token *tok2 = tok->tokAt(2); tok2; tok2 = tok2->next()) for (const Token *tok2 = tok->tokAt(2); tok2; tok2 = tok2->next())
{ {
@ -153,7 +154,7 @@ void CheckOther::clarifyCondition()
break; break;
else if (Token::Match(tok2, "<|<=|==|!=|>|>= %num% )")) else if (Token::Match(tok2, "<|<=|==|!=|>|>= %num% )"))
{ {
clarifyConditionError(tok); clarifyConditionError(tok, tok->strAt(2) == "=");
break; break;
} }
} }
@ -161,12 +162,19 @@ void CheckOther::clarifyCondition()
} }
} }
void CheckOther::clarifyConditionError(const Token *tok) void CheckOther::clarifyConditionError(const Token *tok, bool assign)
{ {
std::string errmsg;
if (assign)
errmsg = "Suspicious condition (assignment+comparison), it can be clarified with parentheses";
else
errmsg = "Suspicious condition (bitwise operator + comparison), it can be clarified with parentheses\n"
"Suspicious condition. Comparison operators have higher precedence than bitwise operators. Please clarify the condition with parentheses.";
reportError(tok, reportError(tok,
Severity::style, Severity::style,
"clarifyCondition", "clarifyCondition",
"Suspicious condition (assignment+comparison), it can be clarified with parentheses"); errmsg);
} }

View File

@ -112,7 +112,7 @@ public:
/** @brief Suspicious condition (assignment+comparison) */ /** @brief Suspicious condition (assignment+comparison) */
void clarifyCondition(); void clarifyCondition();
void clarifyConditionError(const Token *tok); void clarifyConditionError(const Token *tok, bool assign);
/** @brief Are there C-style pointer casts in a c++ file? */ /** @brief Are there C-style pointer casts in a c++ file? */
void warningOldStylePointerCast(); void warningOldStylePointerCast();
@ -319,7 +319,7 @@ public:
c.catchExceptionByValueError(0); c.catchExceptionByValueError(0);
c.memsetZeroBytesError(0, "varname"); c.memsetZeroBytesError(0, "varname");
c.clarifyCalculationError(0, "+"); c.clarifyCalculationError(0, "+");
c.clarifyConditionError(0); c.clarifyConditionError(0, true);
c.incorrectStringCompareError(0, "substr", "\"Hello World\"", "12"); c.incorrectStringCompareError(0, "substr", "\"Hello World\"", "12");
c.incrementBooleanError(0); c.incrementBooleanError(0);
c.comparisonOfBoolWithIntError(0, "varname"); c.comparisonOfBoolWithIntError(0, "varname");

View File

@ -114,7 +114,8 @@ private:
TEST_CASE(clarifyCalculation); TEST_CASE(clarifyCalculation);
TEST_CASE(clarifyCondition); // if (a = b() < 0) TEST_CASE(clarifyCondition1); // if (a = b() < 0)
TEST_CASE(clarifyCondition2); // if (a & b == c)
TEST_CASE(incorrectStringCompare); TEST_CASE(incorrectStringCompare);
@ -2623,7 +2624,7 @@ private:
} }
// clarify conditions with = and comparison // clarify conditions with = and comparison
void clarifyCondition() void clarifyCondition1()
{ {
check("void f() {\n" check("void f() {\n"
" if (x = b() < 0) {}\n" " if (x = b() < 0) {}\n"
@ -2631,6 +2632,15 @@ private:
ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious condition (assignment+comparison), it can be clarified with parentheses\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious condition (assignment+comparison), it can be clarified with parentheses\n", errout.str());
} }
// clarify conditions with bitwise operator and comparison
void clarifyCondition2()
{
check("void f() {\n"
" if (x & 2 == 2) {}\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious condition (bitwise operator + comparison), it can be clarified with parentheses\n", errout.str());
}
void incorrectStringCompare() void incorrectStringCompare()
{ {
check("int f() {\n" check("int f() {\n"