From 8b69ccadd4d197bedf793688ae2b03140563a554 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20St=C3=B6neberg?= Date: Thu, 16 Nov 2023 10:35:43 +0100 Subject: [PATCH] CppCheckExecutor: extracted logging instance related code into separate implementation class (#5658) --- Makefile | 6 +- cli/cppcheckexecutor.cpp | 148 +++++++++++++++++++++++++++++---------- cli/cppcheckexecutor.h | 74 ++------------------ 3 files changed, 119 insertions(+), 109 deletions(-) diff --git a/Makefile b/Makefile index 0b6bf4d7581..0590db67eb3 100644 --- a/Makefile +++ b/Makefile @@ -653,10 +653,10 @@ cli/cmdlineparser.o: cli/cmdlineparser.cpp cli/cmdlinelogger.h cli/cmdlineparser cli/cppcheckexecutor.o: cli/cppcheckexecutor.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/cppcheckexecutor.h cli/cppcheckexecutorseh.h cli/cppcheckexecutorsig.h cli/executor.h cli/filelister.h cli/processexecutor.h cli/singleexecutor.h cli/threadexecutor.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/checkersreport.h lib/checkunusedfunctions.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/path.h lib/pathmatch.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h $(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cppcheckexecutor.cpp -cli/cppcheckexecutorseh.o: cli/cppcheckexecutorseh.cpp cli/cppcheckexecutor.h cli/cppcheckexecutorseh.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/platform.h lib/standards.h lib/utils.h +cli/cppcheckexecutorseh.o: cli/cppcheckexecutorseh.cpp cli/cppcheckexecutor.h cli/cppcheckexecutorseh.h lib/config.h lib/filesettings.h lib/platform.h lib/standards.h lib/utils.h $(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cppcheckexecutorseh.cpp -cli/cppcheckexecutorsig.o: cli/cppcheckexecutorsig.cpp cli/cppcheckexecutor.h cli/cppcheckexecutorsig.h cli/stacktrace.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/platform.h lib/standards.h lib/utils.h +cli/cppcheckexecutorsig.o: cli/cppcheckexecutorsig.cpp cli/cppcheckexecutor.h cli/cppcheckexecutorsig.h cli/stacktrace.h lib/config.h lib/filesettings.h lib/platform.h lib/standards.h lib/utils.h $(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cppcheckexecutorsig.cpp cli/executor.o: cli/executor.cpp cli/executor.h lib/addoninfo.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/standards.h lib/suppressions.h lib/utils.h @@ -665,7 +665,7 @@ cli/executor.o: cli/executor.cpp cli/executor.h lib/addoninfo.h lib/color.h lib/ cli/filelister.o: cli/filelister.cpp cli/filelister.h lib/config.h lib/path.h lib/pathmatch.h lib/utils.h $(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/filelister.cpp -cli/main.o: cli/main.cpp cli/cppcheckexecutor.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/platform.h lib/standards.h lib/utils.h +cli/main.o: cli/main.cpp cli/cppcheckexecutor.h lib/config.h lib/errortypes.h lib/filesettings.h lib/platform.h lib/standards.h lib/utils.h $(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/main.cpp cli/processexecutor.o: cli/processexecutor.cpp cli/executor.h cli/processexecutor.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index ff40ac04b08..14a928e5007 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -26,6 +26,7 @@ #include "color.h" #include "config.h" #include "cppcheck.h" +#include "errorlogger.h" #include "errortypes.h" #include "filelister.h" #include "filesettings.h" @@ -49,9 +50,11 @@ #include #include #include // EXIT_SUCCESS and EXIT_FAILURE +#include #include #include #include +#include #include // IWYU pragma: keep #include #include @@ -106,17 +109,90 @@ class CmdLineLoggerStd : public CmdLineLogger } }; +class CppCheckExecutor::StdLogger : public ErrorLogger +{ +public: + explicit StdLogger(const Settings& settings) + : mSettings(settings) + { + if (!mSettings.outputFile.empty()) { + mErrorOutput = new std::ofstream(settings.outputFile); + } + } + + ~StdLogger() override { + delete mErrorOutput; + } + + StdLogger(const StdLogger&) = delete; + StdLogger& operator=(const SingleExecutor &) = delete; + + void resetLatestProgressOutputTime() { + mLatestProgressOutputTime = std::time(nullptr); + } + + /** + * Helper function to print out errors. Appends a line change. + * @param errmsg String printed to error stream + */ + void reportErr(const std::string &errmsg); + + /** + * @brief Write the checkers report + */ + void writeCheckersReport() const; + +private: + /** + * Information about progress is directed here. This should be + * called by the CppCheck class only. + * + * @param outmsg Progress message e.g. "Checking main.cpp..." + */ + void reportOut(const std::string &outmsg, Color c = Color::Reset) override; + + /** xml output of errors */ + void reportErr(const ErrorMessage &msg) override; + + void reportProgress(const std::string &filename, const char stage[], const std::size_t value) override; + + /** + * Pointer to current settings; set while check() is running for reportError(). + */ + const Settings& mSettings; + + /** + * Used to filter out duplicate error messages. + */ + std::set mShownErrors; + + /** + * Report progress time + */ + std::time_t mLatestProgressOutputTime{}; + + /** + * Error output + */ + std::ofstream* mErrorOutput{}; + + /** + * Checkers that has been executed + */ + std::set mActiveCheckers; + + /** + * True if there are critical errors + */ + std::string mCriticalErrors; +}; + // TODO: do not directly write to stdout #if defined(USE_WINDOWS_SEH) || defined(USE_UNIX_SIGNAL_HANDLING) /*static*/ FILE* CppCheckExecutor::mExceptionOutput = stdout; #endif -CppCheckExecutor::~CppCheckExecutor() -{ - delete mErrorOutput; -} - bool CppCheckExecutor::parseFromArgs(Settings &settings, int argc, const char* const argv[]) { CmdLineLoggerStd logger; @@ -168,6 +244,7 @@ bool CppCheckExecutor::parseFromArgs(Settings &settings, int argc, const char* c if (Path::isDirectory(path)) ++iter; else { + // TODO: this bypasses the template format and other settings // If the include path is not found, warn user and remove the non-existing path from the list. if (settings.severity.isEnabled(Severity::information)) std::cout << "(information) Couldn't find path given by -I '" << path << '\'' << std::endl; @@ -283,13 +360,14 @@ int CppCheckExecutor::check(int argc, const char* const argv[]) return EXIT_SUCCESS; } - CppCheck cppCheck(*this, true, executeCommand); + mStdLogger = new StdLogger(settings); + CppCheck cppCheck(*mStdLogger, true, executeCommand); cppCheck.settings() = settings; - mSettings = &settings; const int ret = check_wrapper(cppCheck); - mSettings = nullptr; + delete mStdLogger; + mStdLogger = nullptr; return ret; } @@ -331,14 +409,10 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck) Settings& settings = cppcheck.settings(); if (settings.reportProgress >= 0) - mLatestProgressOutputTime = std::time(nullptr); - - if (!settings.outputFile.empty()) { - mErrorOutput = new std::ofstream(settings.outputFile); - } + mStdLogger->resetLatestProgressOutputTime(); if (settings.xml) { - reportErr(ErrorMessage::getXMLHeader(settings.cppcheckCfgProductName)); + mStdLogger->reportErr(ErrorMessage::getXMLHeader(settings.cppcheckCfgProductName)); } if (!settings.buildDir.empty()) { @@ -353,16 +427,16 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck) if (!settings.checkersReportFilename.empty()) std::remove(settings.checkersReportFilename.c_str()); - unsigned int returnValue = 0; + unsigned int returnValue; if (settings.useSingleJob()) { // Single process - SingleExecutor executor(cppcheck, mFiles, mFileSettings, settings, settings.nomsg, *this); + SingleExecutor executor(cppcheck, mFiles, mFileSettings, settings, settings.nomsg, *mStdLogger); returnValue = executor.check(); } else { #if defined(THREADING_MODEL_THREAD) - ThreadExecutor executor(mFiles, mFileSettings, settings, settings.nomsg, *this, CppCheckExecutor::executeCommand); + ThreadExecutor executor(mFiles, mFileSettings, settings, settings.nomsg, *mStdLogger, CppCheckExecutor::executeCommand); #elif defined(THREADING_MODEL_FORK) - ProcessExecutor executor(mFiles, mFileSettings, settings, settings.nomsg, *this, CppCheckExecutor::executeCommand); + ProcessExecutor executor(mFiles, mFileSettings, settings, settings.nomsg, *mStdLogger, CppCheckExecutor::executeCommand); #endif returnValue = executor.check(); } @@ -370,7 +444,7 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck) cppcheck.analyseWholeProgram(settings.buildDir, mFiles, mFileSettings); if (settings.severity.isEnabled(Severity::information) || settings.checkConfiguration) { - const bool err = reportSuppressions(settings, cppcheck.isUnusedFunctionCheckEnabled(), mFiles, *this); + const bool err = reportSuppressions(settings, cppcheck.isUnusedFunctionCheckEnabled(), mFiles, *mStdLogger); if (err && returnValue == 0) returnValue = settings.exitCode; } @@ -380,35 +454,35 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck) } if (settings.xml) { - reportErr(ErrorMessage::getXMLFooter()); + mStdLogger->reportErr(ErrorMessage::getXMLFooter()); } - writeCheckersReport(settings); + mStdLogger->writeCheckersReport(); if (returnValue) return settings.exitCode; return 0; } -void CppCheckExecutor::writeCheckersReport(const Settings& settings) const +void CppCheckExecutor::StdLogger::writeCheckersReport() const { - CheckersReport checkersReport(settings, mActiveCheckers); + CheckersReport checkersReport(mSettings, mActiveCheckers); - if (!settings.quiet) { + if (!mSettings.quiet) { const int activeCheckers = checkersReport.getActiveCheckersCount(); const int totalCheckers = checkersReport.getAllCheckersCount(); - const std::string extra = settings.verbose ? " (use --checkers-report= to see details)" : ""; + const std::string extra = mSettings.verbose ? " (use --checkers-report= to see details)" : ""; if (mCriticalErrors.empty()) std::cout << "Active checkers: " << activeCheckers << "/" << totalCheckers << extra << std::endl; else std::cout << "Active checkers: There was critical errors" << extra << std::endl; } - if (settings.checkersReportFilename.empty()) + if (mSettings.checkersReportFilename.empty()) return; - std::ofstream fout(settings.checkersReportFilename); + std::ofstream fout(mSettings.checkersReportFilename); if (fout.is_open()) fout << checkersReport.getReport(mCriticalErrors); @@ -481,16 +555,16 @@ static inline std::string ansiToOEM(const std::string &msg, bool doConvert) #define ansiToOEM(msg, doConvert) (msg) #endif -void CppCheckExecutor::reportErr(const std::string &errmsg) +void CppCheckExecutor::StdLogger::reportErr(const std::string &errmsg) { if (mErrorOutput) *mErrorOutput << errmsg << std::endl; else { - std::cerr << ansiToOEM(errmsg, (mSettings == nullptr) ? true : !mSettings->xml) << std::endl; + std::cerr << ansiToOEM(errmsg, !mSettings.xml) << std::endl; } } -void CppCheckExecutor::reportOut(const std::string &outmsg, Color c) +void CppCheckExecutor::StdLogger::reportOut(const std::string &outmsg, Color c) { if (c == Color::Reset) std::cout << ansiToOEM(outmsg, true) << std::endl; @@ -499,7 +573,7 @@ void CppCheckExecutor::reportOut(const std::string &outmsg, Color c) } // TODO: remove filename parameter? -void CppCheckExecutor::reportProgress(const std::string &filename, const char stage[], const std::size_t value) +void CppCheckExecutor::StdLogger::reportProgress(const std::string &filename, const char stage[], const std::size_t value) { (void)filename; @@ -508,7 +582,7 @@ void CppCheckExecutor::reportProgress(const std::string &filename, const char st // Report progress messages every x seconds const std::time_t currentTime = std::time(nullptr); - if (currentTime >= (mLatestProgressOutputTime + mSettings->reportProgress)) + if (currentTime >= (mLatestProgressOutputTime + mSettings.reportProgress)) { mLatestProgressOutputTime = currentTime; @@ -523,10 +597,8 @@ void CppCheckExecutor::reportProgress(const std::string &filename, const char st } } -void CppCheckExecutor::reportErr(const ErrorMessage &msg) +void CppCheckExecutor::StdLogger::reportErr(const ErrorMessage &msg) { - assert(mSettings != nullptr); - if (msg.severity == Severity::none && (msg.id == "logChecker" || endsWith(msg.id, "-logChecker"))) { const std::string& checker = msg.shortMessage(); mActiveCheckers.emplace(checker); @@ -534,7 +606,7 @@ void CppCheckExecutor::reportErr(const ErrorMessage &msg) } // Alert only about unique errors - if (!mShownErrors.insert(msg.toString(mSettings->verbose)).second) + if (!mShownErrors.insert(msg.toString(mSettings.verbose)).second) return; if (ErrorLogger::isCriticalErrorId(msg.id) && mCriticalErrors.find(msg.id) == std::string::npos) { @@ -543,10 +615,10 @@ void CppCheckExecutor::reportErr(const ErrorMessage &msg) mCriticalErrors += msg.id; } - if (mSettings->xml) + if (mSettings.xml) reportErr(msg.toXML()); else - reportErr(msg.toString(mSettings->verbose, mSettings->templateFormat, mSettings->templateLocation)); + reportErr(msg.toString(mSettings.verbose, mSettings.templateFormat, mSettings.templateLocation)); } #if defined(USE_WINDOWS_SEH) || defined(USE_UNIX_SIGNAL_HANDLING) diff --git a/cli/cppcheckexecutor.h b/cli/cppcheckexecutor.h index 93c094bee9b..48e13b75247 100644 --- a/cli/cppcheckexecutor.h +++ b/cli/cppcheckexecutor.h @@ -19,16 +19,11 @@ #ifndef CPPCHECKEXECUTOR_H #define CPPCHECKEXECUTOR_H -#include "color.h" #include "config.h" -#include "errorlogger.h" #include "filesettings.h" #include -#include -#include #include -#include #include #include #include @@ -36,6 +31,7 @@ class CppCheck; class Library; class Settings; +class ErrorLogger; /** * This class works as an example of how CppCheck can be used in external @@ -44,7 +40,7 @@ class Settings; * just rewrite this class for your needs and possibly use other methods * from CppCheck class instead the ones used here. */ -class CppCheckExecutor : public ErrorLogger { +class CppCheckExecutor { public: friend class TestSuppressions; @@ -55,11 +51,6 @@ class CppCheckExecutor : public ErrorLogger { CppCheckExecutor(const CppCheckExecutor &) = delete; void operator=(const CppCheckExecutor&) = delete; - /** - * Destructor - */ - ~CppCheckExecutor() override; - /** * Starts the checking. * @@ -73,23 +64,6 @@ class CppCheckExecutor : public ErrorLogger { */ int check(int argc, const char* const argv[]); -private: - - /** - * Information about progress is directed here. This should be - * called by the CppCheck class only. - * - * @param outmsg Progress message e.g. "Checking main.cpp..." - */ - void reportOut(const std::string &outmsg, Color c = Color::Reset) override; - - /** xml output of errors */ - void reportErr(const ErrorMessage &msg) override; - - void reportProgress(const std::string &filename, const char stage[], const std::size_t value) override; - -public: - /** * @param exceptionOutput Output file */ @@ -112,11 +86,7 @@ class CppCheckExecutor : public ErrorLogger { */ static int executeCommand(std::string exe, std::vector args, std::string redirect, std::string &output_); - /** - * Helper function to print out errors. Appends a line change. - * @param errmsg String printed to error stream - */ - void reportErr(const std::string &errmsg); +protected: /** * @brief Parse command line args and get settings and file lists @@ -156,7 +126,7 @@ class CppCheckExecutor : public ErrorLogger { * @param settings Settings * @return Returns true if successful */ - bool loadLibraries(Settings& settings); + static bool loadLibraries(Settings& settings); /** * @brief Load addons @@ -165,21 +135,6 @@ class CppCheckExecutor : public ErrorLogger { */ static bool loadAddons(Settings& settings); - /** - * @brief Write the checkers report - */ - void writeCheckersReport(const Settings& settings) const; - - /** - * Pointer to current settings; set while check() is running for reportError(). - */ - const Settings* mSettings{}; - - /** - * Used to filter out duplicate error messages. - */ - std::set mShownErrors; - /** * Filename associated with size of file */ @@ -187,11 +142,6 @@ class CppCheckExecutor : public ErrorLogger { std::list mFileSettings; - /** - * Report progress time - */ - std::time_t mLatestProgressOutputTime{}; - #if defined(USE_WINDOWS_SEH) || defined(USE_UNIX_SIGNAL_HANDLING) /** * Output file name for exception handler @@ -199,20 +149,8 @@ class CppCheckExecutor : public ErrorLogger { static FILE* mExceptionOutput; #endif - /** - * Error output - */ - std::ofstream* mErrorOutput{}; - - /** - * Checkers that has been executed - */ - std::set mActiveCheckers; - - /** - * True if there are critical errors - */ - std::string mCriticalErrors; + class StdLogger; + StdLogger* mStdLogger{}; }; #endif // CPPCHECKEXECUTOR_H