extracted single job execution into SingleExecutor / improved testing / do not unconditionally apply colors to output

Makefile

@@ -261,6 +261,7 @@ CLIOBJ =      cli/cmdlineparser.o \
               cli/filelister.o \
               cli/main.o \
               cli/processexecutor.o \
+              cli/singleexecutor.o \
               cli/stacktrace.o \
               cli/threadexecutor.o
 
@@ -310,6 +311,7 @@ TESTOBJ =     test/fixture.o \
               test/testsimplifytokens.o \
               test/testsimplifytypedef.o \
               test/testsimplifyusing.o \
+              test/testsingleexecutor.o \
               test/testsizeof.o \
               test/teststl.o \
               test/teststring.o \
@@ -342,7 +344,7 @@ cppcheck: $(LIBOBJ) $(CLIOBJ) $(EXTOBJ)
 
 all:	cppcheck testrunner
 
-testrunner: $(TESTOBJ) $(LIBOBJ) $(EXTOBJ) cli/executor.o cli/processexecutor.o cli/threadexecutor.o cli/cmdlineparser.o cli/cppcheckexecutor.o cli/cppcheckexecutorseh.o cli/cppcheckexecutorsig.o cli/stacktrace.o cli/filelister.o
+testrunner: $(TESTOBJ) $(LIBOBJ) $(EXTOBJ) cli/executor.o cli/processexecutor.o cli/singleexecutor.o cli/threadexecutor.o cli/cmdlineparser.o cli/cppcheckexecutor.o cli/cppcheckexecutorseh.o cli/cppcheckexecutorsig.o cli/stacktrace.o cli/filelister.o
 	$(CXX) $(CPPFLAGS) $(CXXFLAGS) -o $@ $^ $(LIBS) $(LDFLAGS) $(RDYNAMIC)
 
 test:	all
@@ -634,7 +636,7 @@ $(libcppdir)/vfvalue.o: lib/vfvalue.cpp lib/config.h lib/errortypes.h lib/mathli
 cli/cmdlineparser.o: cli/cmdlineparser.cpp cli/cmdlineparser.h cli/cppcheckexecutor.h cli/filelister.h externals/tinyxml2/tinyxml2.h lib/check.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h
 	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cmdlineparser.cpp
 
-cli/cppcheckexecutor.o: cli/cppcheckexecutor.cpp cli/cmdlineparser.h cli/cppcheckexecutor.h cli/cppcheckexecutorseh.h cli/cppcheckexecutorsig.h cli/executor.h cli/filelister.h cli/processexecutor.h cli/threadexecutor.h lib/analyzerinfo.h lib/check.h lib/checkunusedfunctions.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.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
+cli/cppcheckexecutor.o: cli/cppcheckexecutor.cpp 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/analyzerinfo.h lib/check.h lib/checkunusedfunctions.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.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/suppressions.h lib/utils.h
@@ -655,6 +657,9 @@ cli/main.o: cli/main.cpp cli/cppcheckexecutor.h lib/color.h lib/config.h lib/err
 cli/processexecutor.o: cli/processexecutor.cpp cli/cppcheckexecutor.h cli/executor.h cli/processexecutor.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.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/processexecutor.cpp
 
+cli/singleexecutor.o: cli/singleexecutor.cpp cli/executor.h cli/singleexecutor.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.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/singleexecutor.cpp
+
 cli/stacktrace.o: cli/stacktrace.cpp cli/stacktrace.h lib/config.h lib/utils.h
 	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/stacktrace.cpp
 
@@ -799,6 +804,9 @@ test/testsimplifytypedef.o: test/testsimplifytypedef.cpp externals/simplecpp/sim
 test/testsimplifyusing.o: test/testsimplifyusing.cpp lib/check.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h
 	$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testsimplifyusing.cpp
 
+test/testsingleexecutor.o: test/testsingleexecutor.cpp cli/executor.h cli/singleexecutor.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h test/redirect.h
+	$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testsingleexecutor.cpp
+
 test/testsizeof.o: test/testsizeof.cpp externals/simplecpp/simplecpp.h lib/check.h lib/checksizeof.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h
 	$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testsizeof.cpp
 

cli/cli.vcxproj

@@ -435,6 +435,7 @@
     <ClInclude Include="executor.h" />
     <ClInclude Include="filelister.h" />
     <ClInclude Include="processexecutor.h" />
+    <ClInclude Include="singleexecutor.h" />
     <ClInclude Include="stacktrace.h" />
     <ClInclude Include="threadexecutor.h" />
   </ItemGroup>
@@ -461,6 +462,7 @@
     <ClCompile Include="filelister.cpp" />
     <ClCompile Include="main.cpp" />
     <ClCompile Include="processexecutor.cpp" />
+    <ClCompile Include="singleexecutor.cpp" />
     <ClCompile Include="stacktrace.cpp" />
     <ClCompile Include="threadexecutor.cpp" />
   </ItemGroup>

cli/cppcheckexecutor.cpp

@@ -30,8 +30,10 @@
 #include "path.h"
 #include "pathmatch.h"
 #include "settings.h"
+#include "singleexecutor.h"
 #include "suppressions.h"
 #include "utils.h"
+
 #include "checkunusedfunctions.h"
 
 #if defined(THREADING_MODEL_THREAD)
@@ -51,7 +53,6 @@
 #include <sstream> // IWYU pragma: keep
 #include <utility>
 #include <vector>
-#include <numeric>
 
 #ifdef USE_UNIX_SIGNAL_HANDLING
 #include "cppcheckexecutorsig.h"
@@ -242,7 +243,7 @@ bool CppCheckExecutor::reportSuppressions(const Settings &settings, bool unusedF
         return false;
 
     bool err = false;
-    if (settings.jointSuppressionReport) {
+    if (settings.useSingleJob()) {
         for (std::map<std::string, std::size_t>::const_iterator i = files.cbegin(); i != files.cend(); ++i) {
             err |= errorLogger.reportUnmatchedSuppressions(
                 settings.nomsg.getUnmatchedLocalSuppressions(i->first, unusedFunctionCheckEnabled));
@@ -310,54 +311,10 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck)
     }
 
     unsigned int returnValue = 0;
-    if (settings.jobs == 1) {
+    if (settings.useSingleJob()) {
         // Single process
-        settings.jointSuppressionReport = true;
-
-        const std::size_t totalfilesize = std::accumulate(mFiles.cbegin(), mFiles.cend(), std::size_t(0), [](std::size_t v, const std::pair<std::string, std::size_t>& f) {
-            return v + f.second;
-        });
-
-        std::size_t processedsize = 0;
-        unsigned int c = 0;
-        if (settings.project.fileSettings.empty()) {
-            for (std::map<std::string, std::size_t>::const_iterator i = mFiles.cbegin(); i != mFiles.cend(); ++i) {
-                if (!settings.library.markupFile(i->first)
-                    || !settings.library.processMarkupAfterCode(i->first)) {
-                    returnValue += cppcheck.check(i->first);
-                    processedsize += i->second;
-                    if (!settings.quiet)
-                        reportStatus(c + 1, mFiles.size(), processedsize, totalfilesize);
-                    c++;
-                }
-            }
-        } else {
-            // filesettings
-            // check all files of the project
-            for (const ImportProject::FileSettings &fs : settings.project.fileSettings) {
-                returnValue += cppcheck.check(fs);
-                ++c;
-                if (!settings.quiet)
-                    reportStatus(c, settings.project.fileSettings.size(), c, settings.project.fileSettings.size());
-                if (settings.clangTidy)
-                    cppcheck.analyseClangTidy(fs);
-            }
-        }
-
-        // TODO: not performed when multiple jobs are being used
-        // second loop to parse all markup files which may not work until all
-        // c/cpp files have been parsed and checked
-        for (std::map<std::string, std::size_t>::const_iterator i = mFiles.cbegin(); i != mFiles.cend(); ++i) {
-            if (settings.library.markupFile(i->first) && settings.library.processMarkupAfterCode(i->first)) {
-                returnValue += cppcheck.check(i->first);
-                processedsize += i->second;
-                if (!settings.quiet)
-                    reportStatus(c + 1, mFiles.size(), processedsize, totalfilesize);
-                c++;
-            }
-        }
-        if (cppcheck.analyseWholeProgram())
-            returnValue++;
+        SingleExecutor executor(cppcheck, mFiles, settings, *this);
+        returnValue = executor.check();
     } else {
 #if defined(THREADING_MODEL_THREAD)
         ThreadExecutor executor(mFiles, settings, *this);
@@ -423,8 +380,10 @@ void CppCheckExecutor::reportErr(const std::string &errmsg)
 
 void CppCheckExecutor::reportOut(const std::string &outmsg, Color c)
 {
-    // TODO: do not unconditionally apply colors
-    std::cout << c << ansiToOEM(outmsg, true) << Color::Reset << std::endl;
+    if (c == Color::Reset)
+        std::cout << ansiToOEM(outmsg, true) << std::endl;
+    else
+        std::cout << toString(c) << ansiToOEM(outmsg, true) << toString(Color::Reset) << std::endl;
 }
 
 void CppCheckExecutor::reportProgress(const std::string &filename, const char stage[], const std::size_t value)
@@ -450,19 +409,6 @@ void CppCheckExecutor::reportProgress(const std::string &filename, const char st
     }
 }
 
-void CppCheckExecutor::reportStatus(std::size_t fileindex, std::size_t filecount, std::size_t sizedone, std::size_t sizetotal)
-{
-    if (filecount > 1) {
-        std::ostringstream oss;
-        const long percentDone = (sizetotal > 0) ? static_cast<long>(static_cast<long double>(sizedone) / sizetotal * 100) : 0;
-        oss << fileindex << '/' << filecount
-            << " files checked " << percentDone
-            << "% done";
-        // TODO: do not unconditionally print in color
-        std::cout << Color::FgBlue << oss.str() << Color::Reset << std::endl;
-    }
-}
-
 void CppCheckExecutor::reportErr(const ErrorMessage &msg)
 {
     if (mShowAllErrors) {

cli/cppcheckexecutor.h

@@ -81,16 +81,6 @@ class CppCheckExecutor : public ErrorLogger {
 
     void reportProgress(const std::string &filename, const char stage[], const std::size_t value) override;
 
-    /**
-     * Information about how many files have been checked
-     *
-     * @param fileindex This many files have been checked.
-     * @param filecount This many files there are in total.
-     * @param sizedone The sum of sizes of the files checked.
-     * @param sizetotal The total sizes of the files.
-     */
-    static void reportStatus(std::size_t fileindex, std::size_t filecount, std::size_t sizedone, std::size_t sizetotal);
-
     /**
      * @param exceptionOutput Output file
      */

cli/executor.cpp

@@ -18,11 +18,14 @@
 
 #include "executor.h"
 
+#include "color.h"
 #include "errorlogger.h"
 #include "settings.h"
 #include "suppressions.h"
 
 #include <algorithm>
+#include <iostream>
+#include <sstream> // IWYU pragma: keep
 #include <utility>
 
 Executor::Executor(const std::map<std::string, std::size_t> &files, Settings &settings, ErrorLogger &errorLogger)
@@ -47,3 +50,15 @@ bool Executor::hasToLog(const ErrorMessage &msg)
     return false;
 }
 
+void Executor::reportStatus(std::size_t fileindex, std::size_t filecount, std::size_t sizedone, std::size_t sizetotal)
+{
+    if (filecount > 1) {
+        std::ostringstream oss;
+        const unsigned long percentDone = (sizetotal > 0) ? (100 * sizedone) / sizetotal : 0;
+        oss << fileindex << '/' << filecount
+            << " files checked " << percentDone
+            << "% done";
+        mErrorLogger.reportOut(oss.str(), Color::FgBlue);
+    }
+}
+

cli/executor.h

@@ -46,6 +46,16 @@ class Executor {
 
     virtual unsigned int check() = 0;
 
+    /**
+     * Information about how many files have been checked
+     *
+     * @param fileindex This many files have been checked.
+     * @param filecount This many files there are in total.
+     * @param sizedone The sum of sizes of the files checked.
+     * @param sizetotal The total sizes of the files.
+     */
+    void reportStatus(std::size_t fileindex, std::size_t filecount, std::size_t sizedone, std::size_t sizetotal);
+
 protected:
     /**
      * @brief Check if message is being suppressed and unique.

cli/processexecutor.cpp

@@ -73,8 +73,7 @@ class PipeWriter : public ErrorLogger {
     explicit PipeWriter(int pipe) : mWpipe(pipe) {}
 
     void reportOut(const std::string &outmsg, Color c) override {
-        // TODO: do not unconditionally apply colors
-        writeToPipe(REPORT_OUT, ::toString(c) + outmsg + ::toString(Color::Reset));
+        writeToPipe(REPORT_OUT, static_cast<char>(c) + outmsg);
     }
 
     void reportErr(const ErrorMessage &msg) override {
@@ -178,7 +177,9 @@ bool ProcessExecutor::handleRead(int rpipe, unsigned int &result, const std::str
 
     bool res = true;
     if (type == PipeWriter::REPORT_OUT) {
-        mErrorLogger.reportOut(buf);
+        // the first charcater is the color
+        const Color c = static_cast<Color>(buf[0]);
+        mErrorLogger.reportOut(buf + 1, c);
     } else if (type == PipeWriter::REPORT_ERROR) {
         ErrorMessage msg;
         try {
@@ -328,7 +329,7 @@ unsigned int ProcessExecutor::check()
                             fileCount++;
                             processedsize += size;
                             if (!mSettings.quiet)
-                                CppCheckExecutor::reportStatus(fileCount, mFiles.size() + mSettings.project.fileSettings.size(), processedsize, totalfilesize);
+                                Executor::reportStatus(fileCount, mFiles.size() + mSettings.project.fileSettings.size(), processedsize, totalfilesize);
 
                             close(*rp);
                             rp = rpipes.erase(rp);

cli/singleexecutor.cpp

@@ -0,0 +1,90 @@
+/*
+ * Cppcheck - A tool for static C/C++ code analysis
+ * Copyright (C) 2007-2023 Cppcheck team.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "singleexecutor.h"
+
+#include "cppcheck.h"
+#include "importproject.h"
+#include "library.h"
+#include "settings.h"
+
+#include <list>
+#include <numeric>
+#include <utility>
+
+class ErrorLogger;
+
+SingleExecutor::SingleExecutor(CppCheck &cppcheck, const std::map<std::string, std::size_t> &files, Settings &settings, ErrorLogger &errorLogger)
+    : Executor(files, settings, errorLogger)
+    , mCppcheck(cppcheck)
+{}
+
+SingleExecutor::~SingleExecutor()
+{}
+
+// TODO: markup handling is not performed with multiple jobs
+unsigned int SingleExecutor::check()
+{
+    unsigned int result = 0;
+
+    const std::size_t totalfilesize = std::accumulate(mFiles.cbegin(), mFiles.cend(), std::size_t(0), [](std::size_t v, const std::pair<std::string, std::size_t>& f) {
+        return v + f.second;
+    });
+
+    std::size_t processedsize = 0;
+    unsigned int c = 0;
+    if (mSettings.project.fileSettings.empty()) {
+        for (std::map<std::string, std::size_t>::const_iterator i = mFiles.cbegin(); i != mFiles.cend(); ++i) {
+            if (!mSettings.library.markupFile(i->first)
+                || !mSettings.library.processMarkupAfterCode(i->first)) {
+                result += mCppcheck.check(i->first);
+                processedsize += i->second;
+                if (!mSettings.quiet)
+                    reportStatus(c + 1, mFiles.size(), processedsize, totalfilesize);
+                c++;
+            }
+        }
+    } else {
+        // filesettings
+        // check all files of the project
+        for (const ImportProject::FileSettings &fs : mSettings.project.fileSettings) {
+            result += mCppcheck.check(fs);
+            ++c;
+            if (!mSettings.quiet)
+                reportStatus(c, mSettings.project.fileSettings.size(), c, mSettings.project.fileSettings.size());
+            if (mSettings.clangTidy)
+                mCppcheck.analyseClangTidy(fs);
+        }
+    }
+
+    // second loop to parse all markup files which may not work until all
+    // c/cpp files have been parsed and checked
+    for (std::map<std::string, std::size_t>::const_iterator i = mFiles.cbegin(); i != mFiles.cend(); ++i) {
+        if (mSettings.library.markupFile(i->first) && mSettings.library.processMarkupAfterCode(i->first)) {
+            result += mCppcheck.check(i->first);
+            processedsize += i->second;
+            if (!mSettings.quiet)
+                reportStatus(c + 1, mFiles.size(), processedsize, totalfilesize);
+            c++;
+        }
+    }
+    if (mCppcheck.analyseWholeProgram())
+        result++;
+
+    return result;
+}