Add internal check that checks for redundant non-nullness checks in Token::(simple)Match checks.
In code like if (tok && Token::simpleMatch(tok, "bla")) {} or if (tok->previous() && Token::Match(tok->previous(), "foo")) {}, the first check is redundant because Token::(simple)Match already checks if the argument token is null.
This commit is contained in:
parent
9d08cbf4d2
commit
aa3f1db33c
2
Makefile
2
Makefile
|
@ -339,7 +339,7 @@ $(SRCDIR)/checkexceptionsafety.o: lib/checkexceptionsafety.cpp lib/cxx11emu.h li
|
||||||
$(SRCDIR)/checkfunctions.o: lib/checkfunctions.cpp lib/cxx11emu.h lib/checkfunctions.h lib/config.h lib/check.h lib/token.h lib/valueflow.h lib/mathlib.h lib/tokenize.h lib/errorlogger.h lib/suppressions.h lib/tokenlist.h lib/settings.h lib/library.h lib/standards.h lib/platform.h lib/importproject.h lib/timer.h lib/astutils.h lib/symboldatabase.h
|
$(SRCDIR)/checkfunctions.o: lib/checkfunctions.cpp lib/cxx11emu.h lib/checkfunctions.h lib/config.h lib/check.h lib/token.h lib/valueflow.h lib/mathlib.h lib/tokenize.h lib/errorlogger.h lib/suppressions.h lib/tokenlist.h lib/settings.h lib/library.h lib/standards.h lib/platform.h lib/importproject.h lib/timer.h lib/astutils.h lib/symboldatabase.h
|
||||||
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CFG) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(SRCDIR)/checkfunctions.o $(SRCDIR)/checkfunctions.cpp
|
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CFG) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(SRCDIR)/checkfunctions.o $(SRCDIR)/checkfunctions.cpp
|
||||||
|
|
||||||
$(SRCDIR)/checkinternal.o: lib/checkinternal.cpp lib/cxx11emu.h lib/checkinternal.h lib/check.h lib/config.h lib/token.h lib/valueflow.h lib/mathlib.h lib/tokenize.h lib/errorlogger.h lib/suppressions.h lib/tokenlist.h lib/settings.h lib/library.h lib/standards.h lib/platform.h lib/importproject.h lib/timer.h lib/symboldatabase.h lib/utils.h
|
$(SRCDIR)/checkinternal.o: lib/checkinternal.cpp lib/cxx11emu.h lib/checkinternal.h lib/check.h lib/config.h lib/token.h lib/valueflow.h lib/mathlib.h lib/tokenize.h lib/errorlogger.h lib/suppressions.h lib/tokenlist.h lib/settings.h lib/library.h lib/standards.h lib/platform.h lib/importproject.h lib/timer.h lib/symboldatabase.h lib/utils.h lib/astutils.h
|
||||||
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CFG) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(SRCDIR)/checkinternal.o $(SRCDIR)/checkinternal.cpp
|
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CFG) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(SRCDIR)/checkinternal.o $(SRCDIR)/checkinternal.cpp
|
||||||
|
|
||||||
$(SRCDIR)/checkio.o: lib/checkio.cpp lib/cxx11emu.h lib/checkio.h lib/check.h lib/config.h lib/token.h lib/valueflow.h lib/mathlib.h lib/tokenize.h lib/errorlogger.h lib/suppressions.h lib/tokenlist.h lib/settings.h lib/library.h lib/standards.h lib/platform.h lib/importproject.h lib/timer.h lib/symboldatabase.h lib/utils.h
|
$(SRCDIR)/checkio.o: lib/checkio.cpp lib/cxx11emu.h lib/checkio.h lib/check.h lib/config.h lib/token.h lib/valueflow.h lib/mathlib.h lib/tokenize.h lib/errorlogger.h lib/suppressions.h lib/tokenlist.h lib/settings.h lib/library.h lib/standards.h lib/platform.h lib/importproject.h lib/timer.h lib/symboldatabase.h lib/utils.h
|
||||||
|
|
|
@ -21,6 +21,7 @@
|
||||||
#include "checkinternal.h"
|
#include "checkinternal.h"
|
||||||
#include "symboldatabase.h"
|
#include "symboldatabase.h"
|
||||||
#include "utils.h"
|
#include "utils.h"
|
||||||
|
#include "astutils.h"
|
||||||
#include <string>
|
#include <string>
|
||||||
#include <set>
|
#include <set>
|
||||||
#include <cstring>
|
#include <cstring>
|
||||||
|
@ -83,6 +84,29 @@ 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 (astOp1->expressionString() == astOp2->expressionString()) {
|
||||||
|
checkRedundantTokCheckError(astOp2);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
void CheckInternal::checkRedundantTokCheckError(const Token* tok)
|
||||||
|
{
|
||||||
|
reportError(tok, Severity::style, "redundantTokCheck",
|
||||||
|
"Unneccessary check of \"" + (tok? tok->expressionString(): emptyString) + "\", match-function already checks if it is null.");
|
||||||
|
}
|
||||||
|
|
||||||
void CheckInternal::checkTokenSimpleMatchPatterns()
|
void CheckInternal::checkTokenSimpleMatchPatterns()
|
||||||
{
|
{
|
||||||
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
|
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
|
||||||
|
|
|
@ -54,6 +54,7 @@ public:
|
||||||
checkInternal.checkUnknownPattern();
|
checkInternal.checkUnknownPattern();
|
||||||
checkInternal.checkRedundantNextPrevious();
|
checkInternal.checkRedundantNextPrevious();
|
||||||
checkInternal.checkExtraWhitespace();
|
checkInternal.checkExtraWhitespace();
|
||||||
|
checkInternal.checkRedundantTokCheck();
|
||||||
}
|
}
|
||||||
|
|
||||||
/** @brief %Check if a simple pattern is used inside Token::Match or Token::findmatch */
|
/** @brief %Check if a simple pattern is used inside Token::Match or Token::findmatch */
|
||||||
|
@ -74,6 +75,8 @@ public:
|
||||||
/** @brief %Check if there is whitespace at the beginning or at the end of a pattern */
|
/** @brief %Check if there is whitespace at the beginning or at the end of a pattern */
|
||||||
void checkExtraWhitespace();
|
void checkExtraWhitespace();
|
||||||
|
|
||||||
|
/** @brief %Check if there is a redundant check for none-nullness of parameter before Match functions, such as (tok && Token::Match(tok, "foo")) */
|
||||||
|
void checkRedundantTokCheck();
|
||||||
private:
|
private:
|
||||||
void multiComparePatternError(const Token *tok, const std::string &pattern, const std::string &funcname);
|
void multiComparePatternError(const Token *tok, const std::string &pattern, const std::string &funcname);
|
||||||
void simplePatternError(const Token *tok, const std::string &pattern, const std::string &funcname);
|
void simplePatternError(const Token *tok, const std::string &pattern, const std::string &funcname);
|
||||||
|
@ -83,6 +86,7 @@ private:
|
||||||
void redundantNextPreviousError(const Token* tok, const std::string& func1, const std::string& func2);
|
void redundantNextPreviousError(const Token* tok, const std::string& func1, const std::string& func2);
|
||||||
void orInComplexPattern(const Token *tok, const std::string &pattern, const std::string &funcname);
|
void orInComplexPattern(const Token *tok, const std::string &pattern, const std::string &funcname);
|
||||||
void extraWhitespaceError(const Token *tok, const std::string &pattern, const std::string &funcname);
|
void extraWhitespaceError(const Token *tok, const std::string &pattern, const std::string &funcname);
|
||||||
|
void checkRedundantTokCheckError(const Token *tok);
|
||||||
|
|
||||||
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
|
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
|
||||||
CheckInternal c(nullptr, settings, errorLogger);
|
CheckInternal c(nullptr, settings, errorLogger);
|
||||||
|
@ -94,6 +98,7 @@ private:
|
||||||
c.redundantNextPreviousError(nullptr, "previous", "next");
|
c.redundantNextPreviousError(nullptr, "previous", "next");
|
||||||
c.orInComplexPattern(nullptr, "||", "Match");
|
c.orInComplexPattern(nullptr, "||", "Match");
|
||||||
c.extraWhitespaceError(nullptr, "%str% ", "Match");
|
c.extraWhitespaceError(nullptr, "%str% ", "Match");
|
||||||
|
c.checkRedundantTokCheckError(nullptr);
|
||||||
}
|
}
|
||||||
|
|
||||||
static std::string myName() {
|
static std::string myName() {
|
||||||
|
|
|
@ -44,6 +44,7 @@ private:
|
||||||
TEST_CASE(internalError);
|
TEST_CASE(internalError);
|
||||||
TEST_CASE(orInComplexPattern);
|
TEST_CASE(orInComplexPattern);
|
||||||
TEST_CASE(extraWhitespace);
|
TEST_CASE(extraWhitespace);
|
||||||
|
TEST_CASE(checkRedundantTokCheck);
|
||||||
}
|
}
|
||||||
|
|
||||||
void check(const char code[]) {
|
void check(const char code[]) {
|
||||||
|
@ -408,6 +409,65 @@ private:
|
||||||
"}");
|
"}");
|
||||||
ASSERT_EQUALS("[test.cpp:3]: (warning) Found extra whitespace inside Token::findsimplematch() call: \"foobar \"\n", errout.str());
|
ASSERT_EQUALS("[test.cpp:3]: (warning) Found extra whitespace inside Token::findsimplematch() call: \"foobar \"\n", errout.str());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void checkRedundantTokCheck() {
|
||||||
|
// findsimplematch
|
||||||
|
check("void f() {\n"
|
||||||
|
" const Token *tok;\n"
|
||||||
|
" if(tok && Token::findsimplematch(tok, \"foobar\")) {};\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("[test.cpp:3]: (style) Unneccessary check of \"tok\", match-function already checks if it is null.\n", errout.str());
|
||||||
|
|
||||||
|
// findmatch
|
||||||
|
check("void f() {\n"
|
||||||
|
" const Token *tok;\n"
|
||||||
|
" if(tok && Token::findmatch(tok, \"%str% foobar\")) {};\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("[test.cpp:3]: (style) Unneccessary check of \"tok\", match-function already checks if it is null.\n", errout.str());
|
||||||
|
|
||||||
|
// Match
|
||||||
|
check("void f() {\n"
|
||||||
|
" const Token *tok;\n"
|
||||||
|
" if(tok && Token::Match(tok, \"5str% foobar\")) {};\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("[test.cpp:3]: (style) Unneccessary check of \"tok\", match-function already checks if it is null.\n", errout.str());
|
||||||
|
|
||||||
|
// simpleMatch
|
||||||
|
check("void f() {\n"
|
||||||
|
" const Token *tok;\n"
|
||||||
|
" if(tok && Token::simpleMatch(tok, \"foobar\")) {};\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("[test.cpp:3]: (style) Unneccessary check of \"tok\", match-function already checks if it is null.\n", errout.str());
|
||||||
|
|
||||||
|
// Match
|
||||||
|
check("void f() {\n"
|
||||||
|
" const Token *tok;\n"
|
||||||
|
" if(tok->previous() && Token::Match(tok->previous(), \"5str% foobar\")) {};\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("[test.cpp:3]: (style) Unneccessary check of \"tok->previous()\", match-function already checks if it is null.\n", errout.str());
|
||||||
|
|
||||||
|
// don't report:
|
||||||
|
// tok->previous() vs tok
|
||||||
|
check("void f() {\n"
|
||||||
|
" const Token *tok;\n"
|
||||||
|
" if(tok->previous() && Token::Match(tok, \"5str% foobar\")) {};\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
|
||||||
|
// tok vs tok->previous())
|
||||||
|
check("void f() {\n"
|
||||||
|
" const Token *tok;\n"
|
||||||
|
" if(tok && Token::Match(tok->previous(), \"5str% foobar\")) {};\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
|
||||||
|
// tok->previous() vs tok->previous()->previous())
|
||||||
|
check("void f() {\n"
|
||||||
|
" const Token *tok;\n"
|
||||||
|
" if(tok->previous() && Token::Match(tok->previous()->previous(), \"5str% foobar\")) {};\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
REGISTER_TEST(TestInternal)
|
REGISTER_TEST(TestInternal)
|
||||||
|
|
Loading…
Reference in New Issue