From 40552e233991b6941dafeb5759c1ead852ea3b0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20St=C3=B6neberg?= Date: Mon, 12 Feb 2024 14:40:49 +0100 Subject: [PATCH] stop pretending `CheckUnusedFunctions` is a real `Check` (#5884) --- Makefile | 2 +- lib/checkunusedfunctions.cpp | 64 ++++++++++++------------- lib/checkunusedfunctions.h | 54 ++++++--------------- lib/cppcheck.cpp | 92 ++++++++++++++++++++---------------- test/testunusedfunctions.cpp | 12 ++--- 5 files changed, 102 insertions(+), 122 deletions(-) diff --git a/Makefile b/Makefile index a4d33f18c67..fa42518dda1 100644 --- a/Makefile +++ b/Makefile @@ -551,7 +551,7 @@ $(libcppdir)/checktype.o: lib/checktype.cpp lib/addoninfo.h lib/check.h lib/chec $(libcppdir)/checkuninitvar.o: lib/checkuninitvar.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checknullpointer.h lib/checkuninitvar.h lib/color.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkuninitvar.cpp -$(libcppdir)/checkunusedfunctions.o: lib/checkunusedfunctions.cpp externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/astutils.h lib/check.h lib/checkunusedfunctions.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h lib/xml.h +$(libcppdir)/checkunusedfunctions.o: lib/checkunusedfunctions.cpp externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/astutils.h lib/checkunusedfunctions.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h lib/xml.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkunusedfunctions.cpp $(libcppdir)/checkunusedvar.o: lib/checkunusedvar.cpp externals/simplecpp/simplecpp.h lib/addoninfo.h lib/astutils.h lib/check.h lib/checkunusedvar.h lib/config.h lib/errortypes.h lib/fwdanalysis.h lib/library.h lib/mathlib.h lib/platform.h lib/preprocessor.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/vfvalue.h diff --git a/lib/checkunusedfunctions.cpp b/lib/checkunusedfunctions.cpp index f4072d70f5d..3c707f4dcb7 100644 --- a/lib/checkunusedfunctions.cpp +++ b/lib/checkunusedfunctions.cpp @@ -74,9 +74,9 @@ void CheckUnusedFunctions::clear() instance.mFunctionCalls.clear(); } -void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char FileName[], const Settings *settings) +void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char FileName[], const Settings &settings) { - const bool doMarkup = settings->library.markupFile(FileName); + const bool doMarkup = settings.library.markupFile(FileName); // Function declarations.. if (!doMarkup) { @@ -125,8 +125,8 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi lambdaEndToken = findLambdaEndToken(tok); // parsing of library code to find called functions - if (settings->library.isexecutableblock(FileName, tok->str())) { - const Token * markupVarToken = tok->tokAt(settings->library.blockstartoffset(FileName)); + if (settings.library.isexecutableblock(FileName, tok->str())) { + const Token * markupVarToken = tok->tokAt(settings.library.blockstartoffset(FileName)); // not found if (!markupVarToken) continue; @@ -134,12 +134,12 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi bool start = true; // find all function calls in library code (starts with '(', not if or while etc) while ((scope || start) && markupVarToken) { - if (markupVarToken->str() == settings->library.blockstart(FileName)) { + if (markupVarToken->str() == settings.library.blockstart(FileName)) { scope++; start = false; - } else if (markupVarToken->str() == settings->library.blockend(FileName)) + } else if (markupVarToken->str() == settings.library.blockend(FileName)) scope--; - else if (!settings->library.iskeyword(FileName, markupVarToken->str())) { + else if (!settings.library.iskeyword(FileName, markupVarToken->str())) { mFunctionCalls.insert(markupVarToken->str()); if (mFunctions.find(markupVarToken->str()) != mFunctions.end()) mFunctions[markupVarToken->str()].usedOtherFile = true; @@ -157,10 +157,10 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi } if (!doMarkup // only check source files - && settings->library.isexporter(tok->str()) && tok->next() != nullptr) { + && settings.library.isexporter(tok->str()) && tok->next() != nullptr) { const Token * propToken = tok->next(); while (propToken && propToken->str() != ")") { - if (settings->library.isexportedprefix(tok->str(), propToken->str())) { + if (settings.library.isexportedprefix(tok->str(), propToken->str())) { const Token* nextPropToken = propToken->next(); const std::string& value = nextPropToken->str(); if (mFunctions.find(value) != mFunctions.end()) { @@ -168,7 +168,7 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi } mFunctionCalls.insert(value); } - if (settings->library.isexportedsuffix(tok->str(), propToken->str())) { + if (settings.library.isexportedsuffix(tok->str(), propToken->str())) { const Token* prevPropToken = propToken->previous(); const std::string& value = prevPropToken->str(); if (value != ")" && mFunctions.find(value) != mFunctions.end()) { @@ -180,7 +180,7 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi } } - if (doMarkup && settings->library.isimporter(FileName, tok->str()) && tok->next()) { + if (doMarkup && settings.library.isimporter(FileName, tok->str()) && tok->next()) { const Token * propToken = tok->next(); if (propToken->next()) { propToken = propToken->next(); @@ -196,8 +196,8 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi } } - if (settings->library.isreflection(tok->str())) { - const int argIndex = settings->library.reflectionArgument(tok->str()); + if (settings.library.isreflection(tok->str())) { + const int argIndex = settings.library.reflectionArgument(tok->str()); if (argIndex >= 0) { const Token * funcToken = tok->next(); int index = 0; @@ -319,7 +319,7 @@ static bool isOperatorFunction(const std::string & funcName) return std::find(additionalOperators.cbegin(), additionalOperators.cend(), funcName.substr(operatorPrefix.length())) != additionalOperators.cend(); } -bool CheckUnusedFunctions::check(ErrorLogger * const errorLogger, const Settings& settings) const +bool CheckUnusedFunctions::check(ErrorLogger& errorLogger, const Settings& settings) const { using ErrorParams = std::tuple; std::vector errors; // ensure well-defined order @@ -353,7 +353,7 @@ bool CheckUnusedFunctions::check(ErrorLogger * const errorLogger, const Settings return !errors.empty(); } -void CheckUnusedFunctions::unusedFunctionError(ErrorLogger * const errorLogger, +void CheckUnusedFunctions::unusedFunctionError(ErrorLogger& errorLogger, const std::string &filename, unsigned int fileIndex, unsigned int lineNumber, const std::string &funcname) { @@ -364,29 +364,27 @@ void CheckUnusedFunctions::unusedFunctionError(ErrorLogger * const errorLogger, } const ErrorMessage errmsg(std::move(locationList), emptyString, Severity::style, "$symbol:" + funcname + "\nThe function '$symbol' is never used.", "unusedFunction", CWE561, Certainty::normal); - if (errorLogger) - errorLogger->reportErr(errmsg); - else - Check::writeToErrorList(errmsg); + errorLogger.reportErr(errmsg); } -Check::FileInfo *CheckUnusedFunctions::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const +void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const Settings &settings) { - if (!settings->checks.isEnabled(Checks::unusedFunction)) - return nullptr; - if (settings->useSingleJob() && settings->buildDir.empty()) - instance.parseTokens(*tokenizer, tokenizer->list.getFiles().front().c_str(), settings); - return nullptr; + if (!settings.checks.isEnabled(Checks::unusedFunction)) + return; + if (settings.useSingleJob() && settings.buildDir.empty()) + instance.parseTokens(tokenizer, tokenizer.list.getFiles().front().c_str(), settings); } -bool CheckUnusedFunctions::analyseWholeProgram(const CTU::FileInfo *ctu, const std::list &fileInfo, const Settings& settings, ErrorLogger &errorLogger) +#define logChecker(id) \ + do { \ + const ErrorMessage errmsg({}, nullptr, Severity::internal, "logChecker", (id), CWE(0U), Certainty::normal); \ + errorLogger.reportErr(errmsg); \ + } while (false) + +bool CheckUnusedFunctions::check(const Settings& settings, ErrorLogger &errorLogger) { - (void)ctu; - (void)fileInfo; - CheckUnusedFunctions dummy(nullptr, &settings, &errorLogger); - dummy. - logChecker("CheckUnusedFunctions::analyseWholeProgram"); // unusedFunctions - return check(&errorLogger, settings); + logChecker("CheckUnusedFunctions::check"); // unusedFunction + return instance.check(errorLogger, settings); } CheckUnusedFunctions::FunctionDecl::FunctionDecl(const Function *f) @@ -416,7 +414,7 @@ namespace { }; } -void CheckUnusedFunctions::analyseWholeProgram(const Settings &settings, ErrorLogger * const errorLogger, const std::string &buildDir) +void CheckUnusedFunctions::analyseWholeProgram(const Settings &settings, ErrorLogger &errorLogger, const std::string &buildDir) { std::map decls; std::set calls; diff --git a/lib/checkunusedfunctions.h b/lib/checkunusedfunctions.h index b7bdee62700..6db37f95aeb 100644 --- a/lib/checkunusedfunctions.h +++ b/lib/checkunusedfunctions.h @@ -22,7 +22,6 @@ #define checkunusedfunctionsH //--------------------------------------------------------------------------- -#include "check.h" #include "config.h" #include @@ -35,71 +34,46 @@ class Function; class Settings; class Tokenizer; -namespace CTU { - class FileInfo; -} - -/// @addtogroup Checks /** @brief Check for functions never called */ /// @{ -class CPPCHECKLIB CheckUnusedFunctions : public Check { +class CPPCHECKLIB CheckUnusedFunctions { friend class TestSuppressions; friend class TestSingleExecutorBase; friend class TestProcessExecutorBase; friend class TestThreadExecutorBase; + friend class TestUnusedFunctions; public: - /** @brief This constructor is used when registering the CheckUnusedFunctions */ - CheckUnusedFunctions() : Check(myName()) {} - - /** @brief This constructor is used when running checks. */ - CheckUnusedFunctions(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) - : Check(myName(), tokenizer, settings, errorLogger) {} + CheckUnusedFunctions() = default; // Parse current tokens and determine.. // * Check what functions are used // * What functions are declared - void parseTokens(const Tokenizer &tokenizer, const char FileName[], const Settings *settings); + void parseTokens(const Tokenizer &tokenizer, const char FileName[], const Settings &settings); - // Return true if an error is reported. - bool check(ErrorLogger * const errorLogger, const Settings& settings) const; + static void parseTokens(const Tokenizer &tokenizer, const Settings &settings); - /** @brief Parse current TU and extract file info */ - Check::FileInfo *getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const override; + std::string analyzerInfo() const; - /** @brief Analyse all file infos for all TU */ - bool analyseWholeProgram(const CTU::FileInfo *ctu, const std::list &fileInfo, const Settings& settings, ErrorLogger &errorLogger) override; + static void analyseWholeProgram(const Settings &settings, ErrorLogger& errorLogger, const std::string &buildDir); - std::string analyzerInfo() const; + static void getErrorMessages(ErrorLogger &errorLogger) { + unusedFunctionError(errorLogger, emptyString, 0, 0, "funcName"); + } - /** @brief Combine and analyze all analyzerInfos for all TUs */ - static void analyseWholeProgram(const Settings &settings, ErrorLogger * const errorLogger, const std::string &buildDir); + static bool check(const Settings& settings, ErrorLogger &errorLogger); private: static void clear(); - void getErrorMessages(ErrorLogger *errorLogger, const Settings * /*settings*/) const override { - CheckUnusedFunctions::unusedFunctionError(errorLogger, emptyString, 0, 0, "funcName"); - } - - void runChecks(const Tokenizer & /*tokenizer*/, ErrorLogger * /*errorLogger*/) override {} + // Return true if an error is reported. + bool check(ErrorLogger& errorLogger, const Settings& settings) const; - /** - * Dummy implementation, just to provide error for --errorlist - */ - static void unusedFunctionError(ErrorLogger * const errorLogger, + static void unusedFunctionError(ErrorLogger& errorLogger, const std::string &filename, unsigned int fileIndex, unsigned int lineNumber, const std::string &funcname); - static std::string myName() { - return "Unused functions"; - } - - std::string classInfo() const override { - return "Check for functions that are never called\n"; - } - struct CPPCHECKLIB FunctionUsage { std::string filename; unsigned int lineNumber{}; diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 29a62661314..ddf9f4c49d0 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -631,7 +631,7 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string mPlistFile.close(); } - CheckUnusedFunctions checkUnusedFunctions(nullptr, nullptr, nullptr); + CheckUnusedFunctions checkUnusedFunctions; try { if (mSettings.library.markupFile(filename)) { @@ -642,7 +642,7 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string std::ifstream in(filename); tokenizer.list.createTokens(in, filename); } - checkUnusedFunctions.getFileInfo(&tokenizer, &mSettings); + CheckUnusedFunctions::parseTokens(tokenizer, mSettings); return EXIT_SUCCESS; } @@ -935,7 +935,7 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string // Analyze info.. if (!mSettings.buildDir.empty()) - checkUnusedFunctions.parseTokens(tokenizer, filename.c_str(), &mSettings); + checkUnusedFunctions.parseTokens(tokenizer, filename.c_str(), mSettings); #ifdef HAVE_RULES // handling of "simple" rules has been removed. @@ -1076,39 +1076,39 @@ void CppCheck::checkNormalTokens(const Tokenizer &tokenizer) const char* unusedFunctionOnly = std::getenv("UNUSEDFUNCTION_ONLY"); const bool doUnusedFunctionOnly = unusedFunctionOnly && (std::strcmp(unusedFunctionOnly, "1") == 0); - const std::time_t maxTime = mSettings.checksMaxTime > 0 ? std::time(nullptr) + mSettings.checksMaxTime : 0; + if (!doUnusedFunctionOnly) { + const std::time_t maxTime = mSettings.checksMaxTime > 0 ? std::time(nullptr) + mSettings.checksMaxTime : 0; - // call all "runChecks" in all registered Check classes - // cppcheck-suppress shadowFunction - TODO: fix this - for (Check *check : Check::instances()) { - if (Settings::terminated()) - return; - - if (maxTime > 0 && std::time(nullptr) > maxTime) { - if (mSettings.debugwarnings) { - ErrorMessage::FileLocation loc; - loc.setfile(tokenizer.list.getFiles()[0]); - ErrorMessage errmsg({std::move(loc)}, - emptyString, - Severity::debug, - "Checks maximum time exceeded", - "checksMaxTime", - Certainty::normal); - reportErr(errmsg); + // call all "runChecks" in all registered Check classes + // cppcheck-suppress shadowFunction - TODO: fix this + for (Check *check : Check::instances()) { + if (Settings::terminated()) + return; + + if (maxTime > 0 && std::time(nullptr) > maxTime) { + if (mSettings.debugwarnings) { + ErrorMessage::FileLocation loc; + loc.setfile(tokenizer.list.getFiles()[0]); + ErrorMessage errmsg({std::move(loc)}, + emptyString, + Severity::debug, + "Checks maximum time exceeded", + "checksMaxTime", + Certainty::normal); + reportErr(errmsg); + } + return; } - return; - } - - if (doUnusedFunctionOnly && dynamic_cast(check) == nullptr) - continue; - Timer timerRunChecks(check->name() + "::runChecks", mSettings.showtime, &s_timerResults); - check->runChecks(tokenizer, this); + Timer timerRunChecks(check->name() + "::runChecks", mSettings.showtime, &s_timerResults); + check->runChecks(tokenizer, this); + } } - if (mSettings.clang) + if (mSettings.clang) { // TODO: Use CTU for Clang analysis return; + } if (mSettings.useSingleJob() || !mSettings.buildDir.empty()) { @@ -1123,20 +1123,21 @@ void CppCheck::checkNormalTokens(const Tokenizer &tokenizer) delete fi1; } - // cppcheck-suppress shadowFunction - TODO: fix this - for (const Check *check : Check::instances()) { - if (doUnusedFunctionOnly && dynamic_cast(check) == nullptr) - continue; - - if (Check::FileInfo * const fi = check->getFileInfo(&tokenizer, &mSettings)) { - if (!mSettings.buildDir.empty()) - mAnalyzerInformation.setFileInfo(check->name(), fi->toString()); - if (mSettings.useSingleJob()) - mFileInfo.push_back(fi); - else - delete fi; + if (!doUnusedFunctionOnly) { + // cppcheck-suppress shadowFunction - TODO: fix this + for (const Check *check : Check::instances()) { + if (Check::FileInfo * const fi = check->getFileInfo(&tokenizer, &mSettings)) { + if (!mSettings.buildDir.empty()) + mAnalyzerInformation.setFileInfo(check->name(), fi->toString()); + if (mSettings.useSingleJob()) + mFileInfo.push_back(fi); + else + delete fi; + } } } + + CheckUnusedFunctions::parseTokens(tokenizer, mSettings); } #ifdef HAVE_RULES @@ -1683,6 +1684,7 @@ void CppCheck::getErrorMessages(ErrorLogger &errorlogger) for (std::list::const_iterator it = Check::instances().cbegin(); it != Check::instances().cend(); ++it) (*it)->getErrorMessages(&errorlogger, &s); + CheckUnusedFunctions::getErrorMessages(errorlogger); Preprocessor::getErrorMessages(&errorlogger, s); } @@ -1777,9 +1779,13 @@ bool CppCheck::analyseWholeProgram() ctu.nestedCalls.insert(ctu.nestedCalls.end(), fi2->nestedCalls.cbegin(), fi2->nestedCalls.cend()); } } + // cppcheck-suppress shadowFunction - TODO: fix this for (Check *check : Check::instances()) errors |= check->analyseWholeProgram(&ctu, mFileInfo, mSettings, *this); // TODO: ctu + + errors |= CheckUnusedFunctions::check(mSettings, *this); + return errors && (mExitCode > 0); } @@ -1791,7 +1797,7 @@ void CppCheck::analyseWholeProgram(const std::string &buildDir, const std::list< return; } if (mSettings.checks.isEnabled(Checks::unusedFunction)) - CheckUnusedFunctions::analyseWholeProgram(mSettings, this, buildDir); + CheckUnusedFunctions::analyseWholeProgram(mSettings, *this, buildDir); std::list fileInfoList; CTU::FileInfo ctuFileInfo; @@ -1844,6 +1850,8 @@ void CppCheck::analyseWholeProgram(const std::string &buildDir, const std::list< for (Check *check : Check::instances()) check->analyseWholeProgram(&ctuFileInfo, fileInfoList, mSettings, *this); + CheckUnusedFunctions::check(mSettings, *this); + for (Check::FileInfo *fi : fileInfoList) delete fi; } diff --git a/test/testunusedfunctions.cpp b/test/testunusedfunctions.cpp index 04f88c39dac..d5235a7be93 100644 --- a/test/testunusedfunctions.cpp +++ b/test/testunusedfunctions.cpp @@ -95,10 +95,10 @@ class TestUnusedFunctions : public TestFixture { ASSERT_LOC(tokenizer.tokenize(istr, "test.cpp"), file, line); // Check for unused functions.. - CheckUnusedFunctions checkUnusedFunctions(&tokenizer, &settings1, this); - checkUnusedFunctions.parseTokens(tokenizer, "someFile.c", &settings1); + CheckUnusedFunctions checkUnusedFunctions; + checkUnusedFunctions.parseTokens(tokenizer, "someFile.c", settings1); // check() returns error if and only if errout is not empty. - if ((checkUnusedFunctions.check)(this, settings1)) { + if ((checkUnusedFunctions.check)(*this, settings1)) { ASSERT(!errout.str().empty()); } else { ASSERT_EQUALS("", errout.str()); @@ -538,7 +538,7 @@ class TestUnusedFunctions : public TestFixture { void multipleFiles() { Tokenizer tokenizer(settings, this); - CheckUnusedFunctions c(&tokenizer, &settings, nullptr); + CheckUnusedFunctions c; // Clear the error buffer.. errout.str(""); @@ -556,11 +556,11 @@ class TestUnusedFunctions : public TestFixture { std::istringstream istr(code); ASSERT(tokenizer2.tokenize(istr, fname.str().c_str())); - c.parseTokens(tokenizer2, "someFile.c", &settings); + c.parseTokens(tokenizer2, "someFile.c", settings); } // Check for unused functions.. - (c.check)(this, settings); + (c.check)(*this, settings); ASSERT_EQUALS("[test1.cpp:1]: (style) The function 'f' is never used.\n", errout.str()); }