CppCheckExecutor: extracted logging instance related code into separate implementation class (#5658)

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

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 <cassert>
 #include <cstdio>
 #include <cstdlib> // EXIT_SUCCESS and EXIT_FAILURE
+#include <ctime>
 #include <iostream>
 #include <iterator>
 #include <list>
+#include <set>
 #include <sstream> // IWYU pragma: keep
 #include <unordered_set>
 #include <utility>
@@ -106,17 +109,90 @@ public:
     }
 };
 
+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<std::string> mShownErrors;
+
+    /**
+     * Report progress time
+     */
+    std::time_t mLatestProgressOutputTime{};
+
+    /**
+     * Error output
+     */
+    std::ofstream* mErrorOutput{};
+
+    /**
+     * Checkers that has been executed
+     */
+    std::set<std::string> 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);
+        mStdLogger->resetLatestProgressOutputTime();
-
-    if (!settings.outputFile.empty()) {
-        mErrorOutput = new std::ofstream(settings.outputFile);
-    }
 
     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=<filename> to see details)" : "";
+        const std::string extra = mSettings.verbose ? " (use --checkers-report=<filename> 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)

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 <cstdio>
-#include <ctime>
-#include <iosfwd>
 #include <list>
-#include <set>
 #include <string>
 #include <utility>
 #include <vector>
@@ -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 @@ public:
     CppCheckExecutor(const CppCheckExecutor &) = delete;
     void operator=(const CppCheckExecutor&) = delete;
 
-    /**
-     * Destructor
-     */
-    ~CppCheckExecutor() override;
-
     /**
      * Starts the checking.
      *
@@ -73,23 +64,6 @@ public:
      */
     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 @@ private:
      */
     static int executeCommand(std::string exe, std::vector<std::string> args, std::string redirect, std::string &output_);
 
-    /**
+protected:
-     * Helper function to print out errors. Appends a line change.
-     * @param errmsg String printed to error stream
-     */
-    void reportErr(const std::string &errmsg);
 
     /**
      * @brief Parse command line args and get settings and file lists
@@ -156,7 +126,7 @@ private:
      * @param settings Settings
      * @return Returns true if successful
      */
-    bool loadLibraries(Settings& settings);
+    static bool loadLibraries(Settings& settings);
 
     /**
      * @brief Load addons
@@ -165,21 +135,6 @@ private:
      */
     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<std::string> mShownErrors;
-
     /**
      * Filename associated with size of file
      */
@@ -187,11 +142,6 @@ private:
 
     std::list<FileSettings> 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 @@ private:
     static FILE* mExceptionOutput;
 #endif
 
-    /**
+    class StdLogger;
-     * Error output
+    StdLogger* mStdLogger{};
-     */
-    std::ofstream* mErrorOutput{};
-
-    /**
-     * Checkers that has been executed
-     */
-    std::set<std::string> mActiveCheckers;
-
-    /**
-     * True if there are critical errors
-     */
-    std::string mCriticalErrors;
 };
 
 #endif // CPPCHECKEXECUTOR_H