walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Fix issue 9103: False positive duplicateConditionAssign (#1808) 

* Fix issue 9103: False positive duplicateConditionAssign

* Update conditional message
This commit is contained in:
Paul Fultz II 2019-04-26 05:30:41 -05:00 committed by Daniel Marjamäki
parent e856920488
commit c4325bbec3
2 changed files with 19 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
lib/checkcondition.cpp
View File

@ -1543,10 +1543,6 @@ void CheckCondition::pointerAdditionResultNotNullError(const Token *tok, const T
void CheckCondition::checkDuplicateConditionalAssign() void CheckCondition::checkDuplicateConditionalAssign()
{ {
// danmar: this is disabled until #9103 is fixed.
// the output should be clarified somehow.. see #9101
return;
if (!mSettings->isEnabled(Settings::STYLE)) if (!mSettings->isEnabled(Settings::STYLE))
return; return;
@ -1568,6 +1564,8 @@ void CheckCondition::checkDuplicateConditionalAssign()
const Token *assignTok = blockTok->next()->astTop(); const Token *assignTok = blockTok->next()->astTop();
if (!Token::simpleMatch(assignTok, "=")) if (!Token::simpleMatch(assignTok, "="))
continue; continue;
if (nextAfterAstRightmostLeaf(assignTok) != blockTok->link()->previous())
continue;
if (!isSameExpression( if (!isSameExpression(
mTokenizer->isCPP(), true, condTok->astOperand1(), assignTok->astOperand1(), mSettings->library, true, true)) mTokenizer->isCPP(), true, condTok->astOperand1(), assignTok->astOperand1(), mSettings->library, true, true))
continue; continue;
@ -1582,15 +1580,19 @@ void CheckCondition::checkDuplicateConditionalAssign()
void CheckCondition::duplicateConditionalAssignError(const Token *condTok, const Token* assignTok) void CheckCondition::duplicateConditionalAssignError(const Token *condTok, const Token* assignTok)
{ {
ErrorPath errors; ErrorPath errors;
std::string msg = "Duplicate expression for the condition and assignment.";
if (condTok && assignTok) { if (condTok && assignTok) {
if (condTok->str() == "==") { if (condTok->str() == "==") {
msg = "Assignment '" + assignTok->expressionString() + "' is redundant with condition '" + condTok->expressionString() + "'.";
errors.emplace_back(condTok, "Condition '" + condTok->expressionString() + "'"); errors.emplace_back(condTok, "Condition '" + condTok->expressionString() + "'");
errors.emplace_back(assignTok, "Assignment '" + assignTok->expressionString() + "' is redundant"); errors.emplace_back(assignTok, "Assignment '" + assignTok->expressionString() + "' is redundant");
} else { } else {
msg = "The statement 'if (" + condTok->expressionString() + ") " + assignTok->expressionString() + "' is logically equivalent to '" + assignTok->expressionString() + "'.";
errors.emplace_back(assignTok, "Assignment '" + assignTok->expressionString() + "'"); errors.emplace_back(assignTok, "Assignment '" + assignTok->expressionString() + "'");
errors.emplace_back(condTok, "Condition '" + condTok->expressionString() + "' is redundant"); errors.emplace_back(condTok, "Condition '" + condTok->expressionString() + "' is redundant");
} }
} }
reportError( reportError(
errors, Severity::style, "duplicateConditionalAssign", "Duplicate expression for the condition and assignment.", CWE398, false); errors, Severity::style, "duplicateConditionalAssign", msg, CWE398, false);
} }

15
test/testcondition.cpp
View File

@ -3084,13 +3084,13 @@ private:
" if (x == y)\n" " if (x == y)\n"
" x = y;\n" " x = y;\n"
"}\n"); "}\n");
TODO_ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Duplicate expression for the condition and assignment.\n", "", errout.str()); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Assignment 'x=y' is redundant with condition 'x==y'.\n", errout.str());
check("void f(int& x, int y) {\n" check("void f(int& x, int y) {\n"
" if (x != y)\n" " if (x != y)\n"
" x = y;\n" " x = y;\n"
"}\n"); "}\n");
TODO_ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (style) Duplicate expression for the condition and assignment.\n", "", errout.str()); ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (style) The statement 'if (x!=y) x=y' is logically equivalent to 'x=y'.\n", errout.str());
check("void f(int& x, int y) {\n" check("void f(int& x, int y) {\n"
" if (x == y)\n" " if (x == y)\n"
@ -3098,7 +3098,7 @@ private:
" else\n" " else\n"
" x = 1;\n" " x = 1;\n"
"}\n"); "}\n");
TODO_ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Duplicate expression for the condition and assignment.\n", "", errout.str()); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Assignment 'x=y' is redundant with condition 'x==y'.\n", errout.str());
check("void f(int& x, int y) {\n" check("void f(int& x, int y) {\n"
" if (x != y)\n" " if (x != y)\n"
@ -3113,6 +3113,15 @@ private:
" x = y + 1;\n" " x = y + 1;\n"
"}\n"); "}\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check("void g();\n"
"void f(int& x, int y) {\n"
" if (x == y) {\n"
" x = y;\n"
" g();\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
} }
}; };