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

Add check for duplicate condition and assignment (#1799) 

* Add check duplicate condition and expression

* Format

* Add assign token

* Add to classInfo

* Change note messages
This commit is contained in:
Paul Fultz II 2019-04-18 13:20:24 -05:00 committed by Daniel Marjamäki
parent 206488c0ea
commit 103002578d
3 changed files with 96 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
lib/checkcondition.cpp
View File

@ -1537,3 +1537,53 @@ void CheckCondition::pointerAdditionResultNotNullError(const Token *tok, const T
const std::string s = calc ? calc->expressionString() : "ptr+1";
reportError(tok, Severity::warning, "pointerAdditionResultNotNull", "Comparison is wrong. Result of '" + s + "' can't be 0 unless there is pointer overflow, and pointer overflow is undefined behaviour.");
}
void CheckCondition::checkDuplicateConditionalAssign()
{
if (!mSettings->isEnabled(Settings::STYLE))
return;
const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
for (const Scope *scope : symbolDatabase->functionScopes) {
for (const Token *tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) {
if (!Token::simpleMatch(tok, "if ("))
continue;
if (!Token::simpleMatch(tok->next()->link(), ") {"))
continue;
const Token *blockTok = tok->next()->link()->next();
const Token *condTok = tok->next()->astOperand2();
if (!Token::Match(condTok, "==|!="))
continue;
if (condTok->str() == "!=" && Token::simpleMatch(blockTok->link(), "} else {"))
continue;
if (!blockTok->next())
continue;
const Token *assignTok = blockTok->next()->astTop();
if (!Token::simpleMatch(assignTok, "="))
continue;
if (!isSameExpression(
mTokenizer->isCPP(), true, condTok->astOperand1(), assignTok->astOperand1(), mSettings->library, true, true))
continue;
if (!isSameExpression(
mTokenizer->isCPP(), true, condTok->astOperand2(), assignTok->astOperand2(), mSettings->library, true, true))
continue;
duplicateConditionalAssignError(condTok, assignTok);
}
}
}
void CheckCondition::duplicateConditionalAssignError(const Token *condTok, const Token* assignTok)
{
ErrorPath errors;
if (condTok && assignTok) {
if (condTok->str() == "==") {
errors.emplace_back(condTok, "Condition '" + condTok->expressionString() + "'");
errors.emplace_back(assignTok, "Assignment '" + assignTok->expressionString() + "' is redundant");
} else {
errors.emplace_back(assignTok, "Assignment '" + assignTok->expressionString() + "'");
errors.emplace_back(condTok, "Condition '" + condTok->expressionString() + "' is redundant");
}
}
reportError(
errors, Severity::style, "duplicateConditionalAssign", "Duplicate expression for the condition and assignment.", CWE398, false);
}

9
lib/checkcondition.h
View File

@ -61,6 +61,7 @@ public:
checkCondition.alwaysTrueFalse();
checkCondition.duplicateCondition();
checkCondition.checkPointerAdditionResultNotNull();
checkCondition.checkDuplicateConditionalAssign();
checkCondition.assignIf();
checkCondition.checkBadBitmaskCheck();
checkCondition.comparison();
@ -115,7 +116,9 @@ public:
/** @brief Check if pointer addition result is NULL '(ptr + 1) == NULL' */
void checkPointerAdditionResultNotNull();
private:
void checkDuplicateConditionalAssign();
private:
// The conditions that have been diagnosed
std::set<const Token*> mCondDiags;
bool diag(const Token* tok, bool insert=true);
@ -151,6 +154,8 @@ private:
void invalidTestForOverflow(const Token* tok, bool result);
void pointerAdditionResultNotNullError(const Token *tok, const Token *calc);
void duplicateConditionalAssignError(const Token *condTok, const Token* assignTok);
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const OVERRIDE {
CheckCondition c(nullptr, settings, errorLogger);
@ -172,6 +177,7 @@ private:
c.alwaysTrueFalseError(nullptr, nullptr);
c.invalidTestForOverflow(nullptr, false);
c.pointerAdditionResultNotNullError(nullptr, nullptr);
c.duplicateConditionalAssignError(nullptr, nullptr);
}
static std::string myName() {
@ -183,6 +189,7 @@ private:
"- Mismatching assignment and comparison => comparison is always true/false\n"
"- Mismatching lhs and rhs in comparison => comparison is always true/false\n"
"- Detect usage of | where & should be used\n"
"- Duplicate condition and assignment\n"
"- Detect matching 'if' and 'else if' conditions\n"
"- Mismatching bitand (a &= 0xf0; a &= 1; => a = 0)\n"
"- Opposite inner condition is always false\n"

38
test/testcondition.cpp
View File

@ -114,6 +114,7 @@ private:
TEST_CASE(checkConditionIsAlwaysTrueOrFalseInsideIfWhile);
TEST_CASE(alwaysTrueFalseInLogicalOperators);
TEST_CASE(pointerAdditionResultNotNull);
TEST_CASE(duplicateConditionalAssign);
}
void check(const char code[], const char* filename = "test.cpp", bool inconclusive = false) {
@ -3068,6 +3069,43 @@ private:
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison is wrong. Result of 'ptr+1' can't be 0 unless there is pointer overflow, and pointer overflow is undefined behaviour.\n", errout.str());
}
void duplicateConditionalAssign()
{
check("void f(int& x, int y) {\n"
" if (x == y)\n"
" x = y;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Duplicate expression for the condition and assignment.\n", errout.str());
check("void f(int& x, int y) {\n"
" if (x != y)\n"
" x = y;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (style) Duplicate expression for the condition and assignment.\n", errout.str());
check("void f(int& x, int y) {\n"
" if (x == y)\n"
" x = y;\n"
" else\n"
" x = 1;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Duplicate expression for the condition and assignment.\n", errout.str());
check("void f(int& x, int y) {\n"
" if (x != y)\n"
" x = y;\n"
" else\n"
" x = 1;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void f(int& x, int y) {\n"
" if (x == y)\n"
" x = y + 1;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}
};
REGISTER_TEST(TestCondition)