Improved check (#6391): Detect identical code in both branches of ternary operator

This commit is contained in:
PKEuS 2015-01-03 18:01:49 +01:00
parent bb9ce68354
commit 267552779d
3 changed files with 32 additions and 1 deletions

View File

@ -87,7 +87,7 @@ bool isSameExpression(const Token *tok1, const Token *tok2, const std::set<std::
tok1 = tok1->astOperand2();
if (tok2->str() == "." && tok2->astOperand1() && tok2->astOperand1()->str() == "this")
tok2 = tok2->astOperand2();
if (tok1->str() != tok2->str() || tok1->varId() != tok2->varId())
if (tok1->varId() != tok2->varId() || tok1->str() != tok2->str())
return false;
if (tok1->str() == "." && tok1->originalName() != tok2->originalName())
return false;
@ -2303,6 +2303,7 @@ void CheckOther::checkDuplicateExpression()
std::list<Scope>::const_iterator scope;
std::list<const Function*> constFunctions;
const std::set<std::string> temp; // Can be used as dummy for isSameExpression()
getConstFunctions(symbolDatabase, constFunctions);
for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
@ -2350,6 +2351,9 @@ void CheckOther::checkDuplicateExpression()
}
}
}
} else if (tok->astOperand1() && tok->astOperand2() && tok->str() == ":" && tok->astParent() && tok->astParent()->str() == "?") {
if (isSameExpression(tok->astOperand1(), tok->astOperand2(), temp))
duplicateExpressionTernaryError(tok);
}
}
}
@ -2367,6 +2371,13 @@ void CheckOther::duplicateExpressionError(const Token *tok1, const Token *tok2,
"determine if it is correct.");
}
void CheckOther::duplicateExpressionTernaryError(const Token *tok)
{
reportError(tok, Severity::style, "duplicateExpressionTernary", "Same expression in both branches of ternary operator.\n"
"Finding the same expression in both branches of ternary operator is suspicious as "
"the same code is executed regardless of the condition.");
}
void CheckOther::selfAssignmentError(const Token *tok, const std::string &varname)
{
reportError(tok, Severity::warning,

View File

@ -271,6 +271,7 @@ private:
void duplicateIfError(const Token *tok1, const Token *tok2);
void duplicateBranchError(const Token *tok1, const Token *tok2);
void duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op);
void duplicateExpressionTernaryError(const Token *tok);
void alwaysTrueFalseStringCompareError(const Token *tok, const std::string& str1, const std::string& str2);
void alwaysTrueStringVariableCompareError(const Token *tok, const std::string& str1, const std::string& str2);
void duplicateBreakError(const Token *tok, bool inconclusive);
@ -331,6 +332,7 @@ private:
c.clarifyStatementError(0);
c.duplicateBranchError(0, 0);
c.duplicateExpressionError(0, 0, "&&");
c.duplicateExpressionTernaryError(0);
c.duplicateBreakError(0, false);
c.unreachableCodeError(0, false);
c.unsignedLessThanZeroError(0, "varname", false);

View File

@ -140,6 +140,7 @@ private:
TEST_CASE(duplicateExpression4); // ticket #3354 (++)
TEST_CASE(duplicateExpression5); // ticket #3749 (macros with same values)
TEST_CASE(duplicateExpression6); // ticket #4639
TEST_CASE(duplicateExpressionTernary); // #6391
TEST_CASE(checkSignOfUnsignedVariable);
TEST_CASE(checkSignOfPointer);
@ -4322,6 +4323,23 @@ private:
ASSERT_EQUALS("", errout.str());
}
void duplicateExpressionTernary() { // #6391
check("void f() {\n"
" return A ? x : x;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Same expression in both branches of ternary operator.\n", errout.str());
check("void f() {\n"
" if( a ? (b ? false:false): false ) ;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Same expression in both branches of ternary operator.\n", errout.str());
check("void f() {\n"
" return A ? x : z;\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void check_signOfUnsignedVariable(const char code[], bool inconclusive=false) {
// Clear the error buffer..
errout.str("");