moved suppression-specific code out of `ErrorLogger` (#5329)

Makefile

@@ -455,7 +455,7 @@ validateRules:
 
 ###### Build
 
-$(libcppdir)/analyzerinfo.o: lib/analyzerinfo.cpp externals/tinyxml2/tinyxml2.h lib/analyzerinfo.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/path.h lib/platform.h lib/suppressions.h lib/utils.h
+$(libcppdir)/analyzerinfo.o: lib/analyzerinfo.cpp externals/tinyxml2/tinyxml2.h lib/analyzerinfo.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/path.h lib/platform.h lib/utils.h
 	$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/analyzerinfo.cpp
 
 $(libcppdir)/astutils.o: lib/astutils.cpp lib/astutils.h lib/check.h lib/checkclass.h lib/config.h lib/errortypes.h lib/importproject.h lib/infer.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/valueflow.h lib/valueptr.h lib/vfvalue.h
@@ -641,10 +641,10 @@ cli/cmdlineparser.o: cli/cmdlineparser.cpp cli/cmdlineparser.h cli/cppcheckexecu
 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
+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/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/suppressions.h
+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
 	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cppcheckexecutorsig.cpp
 
 cli/executor.o: cli/executor.cpp cli/executor.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/utils.h
@@ -653,7 +653,7 @@ cli/executor.o: cli/executor.cpp cli/executor.h lib/color.h lib/config.h lib/err
 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/suppressions.h
+cli/main.o: cli/main.cpp cli/cppcheckexecutor.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h
 	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/main.cpp
 
 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

cli/cppcheckexecutor.cpp

@@ -261,11 +261,11 @@ bool CppCheckExecutor::reportSuppressions(const Settings &settings, bool unusedF
     bool err = false;
     if (settings.useSingleJob()) {
         for (std::map<std::string, std::size_t>::const_iterator i = files.cbegin(); i != files.cend(); ++i) {
-            err |= errorLogger.reportUnmatchedSuppressions(
+            err |= Suppressions::reportUnmatchedSuppressions(
-                settings.nomsg.getUnmatchedLocalSuppressions(i->first, unusedFunctionCheckEnabled));
+                settings.nomsg.getUnmatchedLocalSuppressions(i->first, unusedFunctionCheckEnabled), errorLogger);
         }
     }
-    err |= errorLogger.reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions(unusedFunctionCheckEnabled));
+    err |= Suppressions::reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions(unusedFunctionCheckEnabled), errorLogger);
     return err;
 }
 

cli/cppcheckexecutor.h

@@ -43,6 +43,8 @@ class Settings;
  */
 class CppCheckExecutor : public ErrorLogger {
 public:
+    friend class TestSuppressions;
+
     /**
      * Constructor
      */
@@ -101,8 +103,6 @@ public:
      */
     static bool executeCommand(std::string exe, std::vector<std::string> args, std::string redirect, std::string &output_);
 
-    static bool reportSuppressions(const Settings &settings, bool unusedFunctionCheckEnabled, const std::map<std::string, std::size_t> &files, ErrorLogger& errorLogger);
-
 protected:
 
     /**
@@ -124,6 +124,8 @@ protected:
 
 private:
 
+    static bool reportSuppressions(const Settings &settings, bool unusedFunctionCheckEnabled, const std::map<std::string, std::size_t> &files, ErrorLogger& errorLogger);
+
     /**
      * Wrapper around check_internal
      *   - installs optional platform dependent signal handling

lib/cppcheck.cpp

@@ -1021,7 +1021,7 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string
     // In jointSuppressionReport mode, unmatched suppressions are
     // collected after all files are processed
     if (!mSettings.useSingleJob() && (mSettings.severity.isEnabled(Severity::information) || mSettings.checkConfiguration)) {
-        reportUnmatchedSuppressions(mSettings.nomsg.getUnmatchedLocalSuppressions(filename, isUnusedFunctionCheckEnabled()));
+        Suppressions::reportUnmatchedSuppressions(mSettings.nomsg.getUnmatchedLocalSuppressions(filename, isUnusedFunctionCheckEnabled()), *this);
     }
 
     mErrorList.clear();
@@ -1597,7 +1597,7 @@ void CppCheck::reportErr(const ErrorMessage &msg)
         mAnalyzerInformation.reportErr(msg);
 
     // TODO: only convert if necessary
-    const Suppressions::ErrorMessage errorMessage = msg.toSuppressionsErrorMessage();
+    const auto errorMessage = Suppressions::ErrorMessage::fromErrorMessage(msg);
 
     if (mSettings.nomsg.isSuppressed(errorMessage, mUseGlobalSuppressions)) {
         return;

lib/errorlogger.cpp

@@ -211,22 +211,6 @@ void ErrorMessage::setmsg(const std::string &msg)
     }
 }
 
-Suppressions::ErrorMessage ErrorMessage::toSuppressionsErrorMessage() const
-{
-    Suppressions::ErrorMessage ret;
-    ret.hash = hash;
-    ret.errorId = id;
-    if (!callStack.empty()) {
-        ret.setFileName(callStack.back().getfile(false));
-        ret.lineNumber = callStack.back().line;
-    } else {
-        ret.lineNumber = Suppressions::Suppression::NO_LINE;
-    }
-    ret.certainty = certainty;
-    ret.symbolNames = mSymbolNames;
-    return ret;
-}
-
 static void serializeString(std::string &oss, const std::string & str)
 {
     oss += std::to_string(str.length());
@@ -681,39 +665,6 @@ std::string ErrorMessage::toString(bool verbose, const std::string &templateForm
     return result;
 }
 
-bool ErrorLogger::reportUnmatchedSuppressions(const std::list<Suppressions::Suppression> &unmatched)
-{
-    bool err = false;
-    // Report unmatched suppressions
-    for (const Suppressions::Suppression &s : unmatched) {
-        // don't report "unmatchedSuppression" as unmatched
-        if (s.errorId == "unmatchedSuppression")
-            continue;
-
-        // check if this unmatched suppression is suppressed
-        bool suppressed = false;
-        for (const Suppressions::Suppression &s2 : unmatched) {
-            if (s2.errorId == "unmatchedSuppression") {
-                if ((s2.fileName.empty() || s2.fileName == "*" || s2.fileName == s.fileName) &&
-                    (s2.lineNumber == Suppressions::Suppression::NO_LINE || s2.lineNumber == s.lineNumber)) {
-                    suppressed = true;
-                    break;
-                }
-            }
-        }
-
-        if (suppressed)
-            continue;
-
-        std::list<ErrorMessage::FileLocation> callStack;
-        if (!s.fileName.empty())
-            callStack.emplace_back(s.fileName, s.lineNumber, 0);
-        reportErr(ErrorMessage(callStack, emptyString, Severity::information, "Unmatched suppression: " + s.errorId, "unmatchedSuppression", Certainty::normal));
-        err = true;
-    }
-    return err;
-}
-
 std::string ErrorLogger::callStackToString(const std::list<ErrorMessage::FileLocation> &callStack)
 {
     std::string str;

lib/errorlogger.h

@@ -23,7 +23,6 @@
 
 #include "config.h"
 #include "errortypes.h"
-#include "suppressions.h"
 #include "color.h"
 
 #include <cstddef>
@@ -198,8 +197,6 @@ public:
         return mSymbolNames;
     }
 
-    Suppressions::ErrorMessage toSuppressionsErrorMessage() const;
-
 private:
     static std::string fixInvalidChars(const std::string& raw);
 
@@ -250,13 +247,6 @@ public:
         (void)value;
     }
 
-    /**
-     * Report unmatched suppressions
-     * @param unmatched list of unmatched suppressions (from Settings::Suppressions::getUnmatched(Local|Global)Suppressions)
-     * @return true is returned if errors are reported
-     */
-    bool reportUnmatchedSuppressions(const std::list<Suppressions::Suppression> &unmatched);
-
     static std::string callStackToString(const std::list<ErrorMessage::FileLocation> &callStack);
 
     /**

lib/suppressions.cpp

@@ -35,6 +35,22 @@
 
 #include <tinyxml2.h>
 
+Suppressions::ErrorMessage Suppressions::ErrorMessage::fromErrorMessage(const ::ErrorMessage &msg)
+{
+    Suppressions::ErrorMessage ret;
+    ret.hash = msg.hash;
+    ret.errorId = msg.id;
+    if (!msg.callStack.empty()) {
+        ret.setFileName(msg.callStack.back().getfile(false));
+        ret.lineNumber = msg.callStack.back().line;
+    } else {
+        ret.lineNumber = Suppressions::Suppression::NO_LINE;
+    }
+    ret.certainty = msg.certainty;
+    ret.symbolNames = msg.symbolNames();
+    return ret;
+}
+
 static bool isAcceptedErrorIdChar(char c)
 {
     switch (c) {
@@ -384,7 +400,7 @@ bool Suppressions::isSuppressed(const ::ErrorMessage &errmsg)
 {
     if (mSuppressions.empty())
         return false;
-    return isSuppressed(errmsg.toSuppressionsErrorMessage());
+    return isSuppressed(Suppressions::ErrorMessage::fromErrorMessage(errmsg));
 }
 
 void Suppressions::dump(std::ostream & out) const
@@ -461,3 +477,36 @@ void Suppressions::markUnmatchedInlineSuppressionsAsChecked(const Tokenizer &tok
         }
     }
 }
+
+bool Suppressions::reportUnmatchedSuppressions(const std::list<Suppressions::Suppression> &unmatched, ErrorLogger &errorLogger)
+{
+    bool err = false;
+    // Report unmatched suppressions
+    for (const Suppressions::Suppression &s : unmatched) {
+        // don't report "unmatchedSuppression" as unmatched
+        if (s.errorId == "unmatchedSuppression")
+            continue;
+
+        // check if this unmatched suppression is suppressed
+        bool suppressed = false;
+        for (const Suppressions::Suppression &s2 : unmatched) {
+            if (s2.errorId == "unmatchedSuppression") {
+                if ((s2.fileName.empty() || s2.fileName == "*" || s2.fileName == s.fileName) &&
+                    (s2.lineNumber == Suppressions::Suppression::NO_LINE || s2.lineNumber == s.lineNumber)) {
+                    suppressed = true;
+                    break;
+                }
+            }
+        }
+
+        if (suppressed)
+            continue;
+
+        std::list<::ErrorMessage::FileLocation> callStack;
+        if (!s.fileName.empty())
+            callStack.emplace_back(s.fileName, s.lineNumber, 0);
+        errorLogger.reportErr(::ErrorMessage(callStack, emptyString, Severity::information, "Unmatched suppression: " + s.errorId, "unmatchedSuppression", Certainty::normal));
+        err = true;
+    }
+    return err;
+}

lib/suppressions.h

@@ -34,6 +34,7 @@
 
 class Tokenizer;
 class ErrorMessage;
+class ErrorLogger;
 enum class Certainty;
 
 /** @brief class for handling suppressions */
@@ -50,6 +51,8 @@ public:
         int lineNumber;
         Certainty certainty;
         std::string symbolNames;
+
+        static Suppressions::ErrorMessage fromErrorMessage(const ::ErrorMessage &msg);
     private:
         std::string mFileName;
     };
@@ -201,6 +204,13 @@ public:
      */
     void markUnmatchedInlineSuppressionsAsChecked(const Tokenizer &tokenizer);
 
+    /**
+     * Report unmatched suppressions
+     * @param unmatched list of unmatched suppressions (from Settings::Suppressions::getUnmatched(Local|Global)Suppressions)
+     * @return true is returned if errors are reported
+     */
+    static bool reportUnmatchedSuppressions(const std::list<Suppressions::Suppression> &unmatched, ErrorLogger &errorLogger);
+
 private:
     /** @brief List of error which the user doesn't want to see. */
     std::list<Suppression> mSuppressions;

test/testerrorlogger.cpp

@@ -61,7 +61,6 @@ private:
         TEST_CASE(SerializeSanitize);
         TEST_CASE(SerializeFileLocation);
 
-        TEST_CASE(suppressUnmatchedSuppressions);
         TEST_CASE(substituteTemplateFormatStatic);
         TEST_CASE(substituteTemplateLocationStatic);
     }
@@ -425,64 +424,6 @@ private:
         ASSERT_EQUALS("abcd:/,", msg2.callStack.front().getinfo());
     }
 
-    void suppressUnmatchedSuppressions() {
-        std::list<Suppressions::Suppression> suppressions;
-
-        // No unmatched suppression
-        errout.str("");
-        suppressions.clear();
-        reportUnmatchedSuppressions(suppressions);
-        ASSERT_EQUALS("", errout.str());
-
-        // suppress all unmatchedSuppression
-        errout.str("");
-        suppressions.clear();
-        suppressions.emplace_back("abc", "a.c", 10U);
-        suppressions.emplace_back("unmatchedSuppression", "*", Suppressions::Suppression::NO_LINE);
-        reportUnmatchedSuppressions(suppressions);
-        ASSERT_EQUALS("", errout.str());
-
-        // suppress all unmatchedSuppression (corresponds to "--suppress=unmatchedSuppression")
-        errout.str("");
-        suppressions.clear();
-        suppressions.emplace_back("abc", "a.c", 10U);
-        suppressions.emplace_back("unmatchedSuppression", "", Suppressions::Suppression::NO_LINE);
-        reportUnmatchedSuppressions(suppressions);
-        ASSERT_EQUALS("", errout.str());
-
-        // suppress all unmatchedSuppression in a.c
-        errout.str("");
-        suppressions.clear();
-        suppressions.emplace_back("abc", "a.c", 10U);
-        suppressions.emplace_back("unmatchedSuppression", "a.c", Suppressions::Suppression::NO_LINE);
-        reportUnmatchedSuppressions(suppressions);
-        ASSERT_EQUALS("", errout.str());
-
-        // suppress unmatchedSuppression in a.c at line 10
-        errout.str("");
-        suppressions.clear();
-        suppressions.emplace_back("abc", "a.c", 10U);
-        suppressions.emplace_back("unmatchedSuppression", "a.c", 10U);
-        reportUnmatchedSuppressions(suppressions);
-        ASSERT_EQUALS("", errout.str());
-
-        // don't suppress unmatchedSuppression when file is mismatching
-        errout.str("");
-        suppressions.clear();
-        suppressions.emplace_back("abc", "a.c", 10U);
-        suppressions.emplace_back("unmatchedSuppression", "b.c", Suppressions::Suppression::NO_LINE);
-        reportUnmatchedSuppressions(suppressions);
-        ASSERT_EQUALS("[a.c:10]: (information) Unmatched suppression: abc\n", errout.str());
-
-        // don't suppress unmatchedSuppression when line is mismatching
-        errout.str("");
-        suppressions.clear();
-        suppressions.emplace_back("abc", "a.c", 10U);
-        suppressions.emplace_back("unmatchedSuppression", "a.c", 1U);
-        reportUnmatchedSuppressions(suppressions);
-        ASSERT_EQUALS("[a.c:10]: (information) Unmatched suppression: abc\n", errout.str());
-    }
-
     void substituteTemplateFormatStatic() const
     {
         {

test/testsuppressions.cpp

@@ -83,6 +83,8 @@ private:
 
         TEST_CASE(suppressingSyntaxErrorAndExitCode);
         TEST_CASE(suppressLocal);
+
+        TEST_CASE(suppressUnmatchedSuppressions);
     }
 
     void suppressionsBadId1() const {
@@ -859,6 +861,64 @@ private:
         ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid2", "test2.cpp", 1)));
         ASSERT_EQUALS(false, suppressions.isSuppressed(errorMessage("errorid2", "test2.cpp", 1), false));
     }
+
+    void suppressUnmatchedSuppressions() {
+        std::list<Suppressions::Suppression> suppressions;
+
+        // No unmatched suppression
+        errout.str("");
+        suppressions.clear();
+        Suppressions::reportUnmatchedSuppressions(suppressions, *this);
+        ASSERT_EQUALS("", errout.str());
+
+        // suppress all unmatchedSuppression
+        errout.str("");
+        suppressions.clear();
+        suppressions.emplace_back("abc", "a.c", 10U);
+        suppressions.emplace_back("unmatchedSuppression", "*", Suppressions::Suppression::NO_LINE);
+        Suppressions::reportUnmatchedSuppressions(suppressions, *this);
+        ASSERT_EQUALS("", errout.str());
+
+        // suppress all unmatchedSuppression (corresponds to "--suppress=unmatchedSuppression")
+        errout.str("");
+        suppressions.clear();
+        suppressions.emplace_back("abc", "a.c", 10U);
+        suppressions.emplace_back("unmatchedSuppression", "", Suppressions::Suppression::NO_LINE);
+        Suppressions::reportUnmatchedSuppressions(suppressions, *this);
+        ASSERT_EQUALS("", errout.str());
+
+        // suppress all unmatchedSuppression in a.c
+        errout.str("");
+        suppressions.clear();
+        suppressions.emplace_back("abc", "a.c", 10U);
+        suppressions.emplace_back("unmatchedSuppression", "a.c", Suppressions::Suppression::NO_LINE);
+        Suppressions::reportUnmatchedSuppressions(suppressions, *this);
+        ASSERT_EQUALS("", errout.str());
+
+        // suppress unmatchedSuppression in a.c at line 10
+        errout.str("");
+        suppressions.clear();
+        suppressions.emplace_back("abc", "a.c", 10U);
+        suppressions.emplace_back("unmatchedSuppression", "a.c", 10U);
+        Suppressions::reportUnmatchedSuppressions(suppressions, *this);
+        ASSERT_EQUALS("", errout.str());
+
+        // don't suppress unmatchedSuppression when file is mismatching
+        errout.str("");
+        suppressions.clear();
+        suppressions.emplace_back("abc", "a.c", 10U);
+        suppressions.emplace_back("unmatchedSuppression", "b.c", Suppressions::Suppression::NO_LINE);
+        Suppressions::reportUnmatchedSuppressions(suppressions, *this);
+        ASSERT_EQUALS("[a.c:10]: (information) Unmatched suppression: abc\n", errout.str());
+
+        // don't suppress unmatchedSuppression when line is mismatching
+        errout.str("");
+        suppressions.clear();
+        suppressions.emplace_back("abc", "a.c", 10U);
+        suppressions.emplace_back("unmatchedSuppression", "a.c", 1U);
+        Suppressions::reportUnmatchedSuppressions(suppressions, *this);
+        ASSERT_EQUALS("[a.c:10]: (information) Unmatched suppression: abc\n", errout.str());
+    }
 };
 
 REGISTER_TEST(TestSuppressions)