diff --git a/lib/checkinternal.cpp b/lib/checkinternal.cpp index 5945f86f0..f9c1e0c7d 100644 --- a/lib/checkinternal.cpp +++ b/lib/checkinternal.cpp @@ -88,19 +88,37 @@ void CheckInternal::checkTokenMatchPatterns() void CheckInternal::checkRedundantTokCheck() { for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { - if (!Token::Match(tok, "&& Token :: simpleMatch|Match|findsimplematch|findmatch (")) - continue; - // in code like - // if (tok->previous() && Token::match(tok->previous(), "bla")) {} - // the first tok->previous() check is redundant - const Token *astOp1 = tok->astOperand1(); - const Token *astOp2 = getArguments(tok->tokAt(3))[0]; + if (Token::Match(tok, "&& Token :: simpleMatch|Match|findsimplematch|findmatch (")) { + // in code like + // if (tok->previous() && Token::match(tok->previous(), "bla")) {} + // the first tok->previous() check is redundant + const Token *astOp1 = tok->astOperand1(); + const Token *astOp2 = getArguments(tok->tokAt(3))[0]; - if (astOp1->expressionString() == astOp2->expressionString()) { - checkRedundantTokCheckError(astOp2); + // std::cout << "\n astOp1: " << tok->astOperand1()->str() << ""; // tok + // std::cout << "\n astOp2: " << getArguments(tok->tokAt(3))[0]->str() << "\n "; // tok + // std::cout << "\n"; + + + if (astOp1->expressionString() == astOp2->expressionString()) { + checkRedundantTokCheckError(astOp2); + } + // if (!tok || !Token::match(tok, "foo")) + } else if (Token::Match(tok, "%oror% ! Token :: simpleMatch|Match|findsimplematch|findmatch (")) { + // the first tok condition is negated + const Token *negTok = tok->next()->astParent()->astOperand1(); + if (Token::simpleMatch(negTok, "!")) { + const Token *astOp1 = negTok->astOperand1(); + const Token *astOp2 = getArguments(tok->tokAt(4))[0]; + + if (astOp1->expressionString() == astOp2->expressionString()) { + checkRedundantTokCheckError(astOp2); + } + } } + } } -} + void CheckInternal::checkRedundantTokCheckError(const Token* tok) { diff --git a/test/testinternal.cpp b/test/testinternal.cpp index 3f8e4fd93..922e3a696 100644 --- a/test/testinternal.cpp +++ b/test/testinternal.cpp @@ -467,6 +467,34 @@ private: " if(tok->previous() && Token::Match(tok->previous()->previous(), \"5str% foobar\")) {};\n" "}"); ASSERT_EQUALS("", errout.str()); + + // if a && fn(a) triggers, make sure !a || !fn(a) triggers as well! + check("void f() {\n" + " const Token *tok;\n" + " if(!tok || !Token::simpleMatch(tok, \"foobar\")) {};\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (style) Unnecessary check of \"tok\", match-function already checks if it is null.\n", errout.str()); + + // if tok || !Token::simpleMatch... + check("void f() {\n" + " const Token *tok;\n" + " if(tok || !Token::simpleMatch(tok, \"foobar\")) {};\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + // if !tok || Token::simpleMatch... + check("void f() {\n" + " const Token *tok;\n" + " if(!tok || Token::simpleMatch(tok, \"foobar\")) {};\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + // something more complex + check("void f() {\n" + " const Token *tok;\n" + " if(!tok->previous()->previous() || !Token::simpleMatch(tok->previous()->previous(), \"foobar\")) {};\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (style) Unnecessary check of \"tok->previous()->previous()\", match-function already checks if it is null.\n", errout.str()); } };