Report opposite expressions (#1182)

* Report opposite expressions

* Skip assignment operator
This commit is contained in:
Paul Fultz II 2018-04-21 04:28:21 -05:00 committed by Daniel Marjamäki
parent bad66594d6
commit d939c6015a
3 changed files with 48 additions and 1 deletions

View File

@ -1974,7 +1974,12 @@ void CheckOther::checkDuplicateExpression()
} }
duplicateExpressionError(tok, tok, tok->str()); duplicateExpressionError(tok, tok, tok->str());
} }
} }
} else if (styleEnabled &&
isOppositeCond(true, _tokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), _settings->library, false) &&
!Token::simpleMatch(tok, "=") &&
isWithoutSideEffects(_tokenizer->isCPP(), tok->astOperand1())) {
oppositeExpressionError(tok, tok, tok->str());
} else if (!Token::Match(tok, "[-/%]")) { // These operators are not associative } else if (!Token::Match(tok, "[-/%]")) { // These operators are not associative
if (styleEnabled && tok->astOperand2() && tok->str() == tok->astOperand1()->str() && isSameExpression(_tokenizer->isCPP(), true, tok->astOperand2(), tok->astOperand1()->astOperand2(), _settings->library, true) && isWithoutSideEffects(_tokenizer->isCPP(), tok->astOperand2())) if (styleEnabled && tok->astOperand2() && tok->str() == tok->astOperand1()->str() && isSameExpression(_tokenizer->isCPP(), true, tok->astOperand2(), tok->astOperand1()->astOperand2(), _settings->library, true) && isWithoutSideEffects(_tokenizer->isCPP(), tok->astOperand2()))
duplicateExpressionError(tok->astOperand2(), tok->astOperand2(), tok->str()); duplicateExpressionError(tok->astOperand2(), tok->astOperand2(), tok->str());
@ -2003,6 +2008,16 @@ void CheckOther::checkDuplicateExpression()
} }
} }
void CheckOther::oppositeExpressionError(const Token *tok1, const Token *tok2, const std::string &op)
{
const std::list<const Token *> toks = { tok2, tok1 };
reportError(toks, Severity::style, "oppositeExpression", "Opposite expression on both sides of \'" + op + "\'.\n"
"Finding the opposite expression on both sides of an operator is suspicious and might "
"indicate a cut and paste or logic error. Please examine this code carefully to "
"determine if it is correct.", CWE398, false);
}
void CheckOther::duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op) void CheckOther::duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op)
{ {
const std::list<const Token *> toks = { tok2, tok1 }; const std::list<const Token *> toks = { tok2, tok1 };

View File

@ -239,6 +239,7 @@ private:
void misusedScopeObjectError(const Token *tok, const std::string &varname); void misusedScopeObjectError(const Token *tok, const std::string &varname);
void duplicateBranchError(const Token *tok1, const Token *tok2); void duplicateBranchError(const Token *tok1, const Token *tok2);
void duplicateAssignExpressionError(const Token *tok1, const Token *tok2); void duplicateAssignExpressionError(const Token *tok1, const Token *tok2);
void oppositeExpressionError(const Token *tok1, const Token *tok2, const std::string &op);
void duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op); void duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op);
void duplicateValueTernaryError(const Token *tok); void duplicateValueTernaryError(const Token *tok);
void duplicateExpressionTernaryError(const Token *tok); void duplicateExpressionTernaryError(const Token *tok);
@ -298,6 +299,7 @@ private:
c.clarifyCalculationError(nullptr, "+"); c.clarifyCalculationError(nullptr, "+");
c.clarifyStatementError(nullptr); c.clarifyStatementError(nullptr);
c.duplicateBranchError(nullptr, nullptr); c.duplicateBranchError(nullptr, nullptr);
c.oppositeExpressionError(nullptr, nullptr, "&&");
c.duplicateExpressionError(nullptr, nullptr, "&&"); c.duplicateExpressionError(nullptr, nullptr, "&&");
c.duplicateValueTernaryError(nullptr); c.duplicateValueTernaryError(nullptr);
c.duplicateExpressionTernaryError(nullptr); c.duplicateExpressionTernaryError(nullptr);

View File

@ -131,6 +131,7 @@ private:
TEST_CASE(duplicateValueTernary); TEST_CASE(duplicateValueTernary);
TEST_CASE(duplicateExpressionTernary); // #6391 TEST_CASE(duplicateExpressionTernary); // #6391
TEST_CASE(duplicateExpressionTemplate); // #6930 TEST_CASE(duplicateExpressionTemplate); // #6930
TEST_CASE(oppositeExpression);
TEST_CASE(duplicateVarExpression); TEST_CASE(duplicateVarExpression);
TEST_CASE(checkSignOfUnsignedVariable); TEST_CASE(checkSignOfUnsignedVariable);
@ -3918,6 +3919,35 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void oppositeExpression() {
check("void f(bool a) { if(a && !a) {} }");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '&&'.\n", errout.str());
check("void f(bool a) { if(a != !a) {} }");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str());
check("void f(bool a) { if( a == !(a) ) {}}");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '=='.\n", errout.str());
check("void f(bool a) { if( a != !(a) ) {}}");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str());
check("void f(bool a) { if( !(a) == a ) {}}");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '=='.\n", errout.str());
check("void f(bool a) { if( !(a) != a ) {}}");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str());
check("void f(bool a) { if( !(!a) == !(a) ) {}}");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '=='.\n", errout.str());
check("void f(bool a) { if( !(!a) != !(a) ) {}}");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str());
check("void f(bool a) { a = !a; }");
ASSERT_EQUALS("", errout.str());
}
void duplicateVarExpression() { void duplicateVarExpression() {
check("int f() __attribute__((pure));\n" check("int f() __attribute__((pure));\n"
"void test() {\n" "void test() {\n"