fixed #13663/#14232 - reworked unmatchedSuppression exclusion logic / do not report misra-* or premium-* suppressions when they are not enabled (#7925)

Author: firewave

cli/cmdlineparser.cpp

@@ -1609,11 +1609,22 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
     if (mSettings.jobs > 1 && mSettings.buildDir.empty()) {
         // TODO: bail out instead?
         if (mSettings.checks.isEnabled(Checks::unusedFunction))
+        {
             mLogger.printMessage("unusedFunction check requires --cppcheck-build-dir to be active with -j.");
+            mSettings.checks.disable(Checks::unusedFunction);
+            // TODO: is there some later logic to remove?
+        }
         // TODO: enable
         //mLogger.printMessage("whole program analysis requires --cppcheck-build-dir to be active with -j.");
     }
 
+    if (!mSettings.checks.isEnabled(Checks::unusedFunction))
+        mSettings.unmatchedSuppressionFilters.emplace_back("unusedFunction");
+    if (!mSettings.addons.count("misra"))
+        mSettings.unmatchedSuppressionFilters.emplace_back("misra-*");
+    if (!mSettings.premium)
+        mSettings.unmatchedSuppressionFilters.emplace_back("premium-*");
+
     if (inputAsFilter) {
         mSettings.fileFilters.insert(mSettings.fileFilters.end(), mPathNames.cbegin(), mPathNames.cend());
         mPathNames.clear();

cli/cppcheckexecutor.cpp

@@ -293,35 +293,73 @@ int CppCheckExecutor::check_wrapper(const Settings& settings, Suppressions& supp
     return check_internal(settings, supprs);
 }
 
-bool CppCheckExecutor::reportSuppressions(const Settings &settings, const SuppressionList& suppressions, bool unusedFunctionCheckEnabled, const std::list<FileWithDetails> &files, const std::list<FileSettings>& fileSettings, ErrorLogger& errorLogger) {
-    const auto& suppr = suppressions.getSuppressions();
-    if (std::any_of(suppr.begin(), suppr.end(), [](const SuppressionList::Suppression& s) {
-        return s.errorId == "unmatchedSuppression" && s.fileName.empty() && s.lineNumber == SuppressionList::Suppression::NO_LINE;
+/**
+ * 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<SuppressionList::Suppression> &unmatched, ErrorLogger &errorLogger, const std::vector<std::string>& filters)
+{
+    bool err = false;
+    // Report unmatched suppressions
+    for (const SuppressionList::Suppression &s : unmatched) {
+        // check if this unmatched suppression is suppressed
+        bool suppressed = false;
+        for (const SuppressionList::Suppression &s2 : unmatched) {
+            if (s2.errorId == "unmatchedSuppression") {
+                if ((s2.fileName.empty() || s2.fileName == "*" || s2.fileName == s.fileName) &&
+                    (s2.lineNumber == SuppressionList::Suppression::NO_LINE || s2.lineNumber == s.lineNumber)) {
+                    suppressed = true;
+                    break;
+                }
+            }
+        }
+
+        if (suppressed)
+            continue;
+
+        const bool skip = std::any_of(filters.cbegin(), filters.cend(), [&s](const std::string& filter) {
+            return matchglob(filter, s.errorId);
+        });
+        if (skip)
+            continue;
+
+        std::list<::ErrorMessage::FileLocation> callStack;
+        if (!s.fileName.empty()) {
+            callStack.emplace_back(s.fileName, s.lineNumber, 0);
+        }
+        errorLogger.reportErr(::ErrorMessage(std::move(callStack), "", Severity::information, "Unmatched suppression: " + s.errorId, "unmatchedSuppression", Certainty::normal));
+        err = true;
+    }
+    return err;
+}
+
+bool CppCheckExecutor::reportUnmatchedSuppressions(const Settings &settings, const SuppressionList& suppressions, const std::list<FileWithDetails> &files, const std::list<FileSettings>& fileSettings, ErrorLogger& errorLogger) {
+    // the two inputs may only be used exclusively
+    assert(!(!files.empty() && !fileSettings.empty()));
+
+    // bail out if there is a suppression of unmatchedSuppression which matches any file
+    const auto suppr = suppressions.getSuppressions();
+    if (std::any_of(suppr.cbegin(), suppr.cend(), [](const SuppressionList::Suppression& s) {
+        return s.errorId == "unmatchedSuppression" && (s.fileName.empty() || s.fileName == "*") && s.lineNumber == SuppressionList::Suppression::NO_LINE;
     }))
         return false;
 
     bool err = false;
-    if (settings.useSingleJob()) {
-        // the two inputs may only be used exclusively
-        assert(!(!files.empty() && !fileSettings.empty()));
 
-        for (auto i = files.cbegin(); i != files.cend(); ++i) {
-            err |= SuppressionList::reportUnmatchedSuppressions(
-                suppressions.getUnmatchedLocalSuppressions(*i, unusedFunctionCheckEnabled), errorLogger);
-        }
+    for (auto i = files.cbegin(); i != files.cend(); ++i) {
+        err |= ::reportUnmatchedSuppressions(suppressions.getUnmatchedLocalSuppressions(*i), errorLogger, settings.unmatchedSuppressionFilters);
+    }
 
-        for (auto i = fileSettings.cbegin(); i != fileSettings.cend(); ++i) {
-            err |= SuppressionList::reportUnmatchedSuppressions(
-                suppressions.getUnmatchedLocalSuppressions(i->file, unusedFunctionCheckEnabled), errorLogger);
-        }
+    for (auto i = fileSettings.cbegin(); i != fileSettings.cend(); ++i) {
+        err |= ::reportUnmatchedSuppressions(suppressions.getUnmatchedLocalSuppressions(i->file), errorLogger, settings.unmatchedSuppressionFilters);
     }
+
     if (settings.inlineSuppressions) {
-        // report unmatched unusedFunction suppressions
-        err |= SuppressionList::reportUnmatchedSuppressions(
-            suppressions.getUnmatchedInlineSuppressions(unusedFunctionCheckEnabled), errorLogger);
+        err |= ::reportUnmatchedSuppressions(suppressions.getUnmatchedInlineSuppressions(), errorLogger, settings.unmatchedSuppressionFilters);
     }
 
-    err |= SuppressionList::reportUnmatchedSuppressions(suppressions.getUnmatchedGlobalSuppressions(unusedFunctionCheckEnabled), errorLogger);
+    err |= ::reportUnmatchedSuppressions(suppressions.getUnmatchedGlobalSuppressions(), errorLogger, settings.unmatchedSuppressionFilters);
     return err;
 }
 
@@ -376,7 +414,7 @@ int CppCheckExecutor::check_internal(const Settings& settings, Suppressions& sup
     returnValue |= cppcheck.analyseWholeProgram(settings.buildDir, mFiles, mFileSettings, stdLogger.getCtuInfo());
 
     if (settings.severity.isEnabled(Severity::information) || settings.checkConfiguration) {
-        const bool err = reportSuppressions(settings, supprs.nomsg, settings.checks.isEnabled(Checks::unusedFunction), mFiles, mFileSettings, stdLogger);
+        const bool err = reportUnmatchedSuppressions(settings, supprs.nomsg, mFiles, mFileSettings, stdLogger);
         if (err && returnValue == 0)
             returnValue = settings.exitCode;
     }

cli/cppcheckexecutor.h

@@ -70,7 +70,7 @@ class CppCheckExecutor {
 
 protected:
 
-    static bool reportSuppressions(const Settings &settings, const SuppressionList& suppressions, bool unusedFunctionCheckEnabled, const std::list<FileWithDetails> &files, const std::list<FileSettings>& fileSettings, ErrorLogger& errorLogger);
+    static bool reportUnmatchedSuppressions(const Settings &settings, const SuppressionList& suppressions, const std::list<FileWithDetails> &files, const std::list<FileSettings>& fileSettings, ErrorLogger& errorLogger);
 
     /**
      * Wrapper around check_internal

lib/cppcheck.cpp

@@ -1299,16 +1299,6 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
         mErrorLogger.reportErr(errmsg);
     }
 
-    // TODO: this is done too early causing the whole program analysis suppressions to be reported as unmatched
-    if (mSettings.severity.isEnabled(Severity::information) || mSettings.checkConfiguration) {
-        // In jointSuppressionReport mode, unmatched suppressions are
-        // collected after all files are processed
-        if (!mSettings.useSingleJob()) {
-            // TODO: check result?
-            SuppressionList::reportUnmatchedSuppressions(mSuppressions.nomsg.getUnmatchedLocalSuppressions(file, static_cast<bool>(mUnusedFunctionsCheck)), mErrorLogger);
-        }
-    }
-
     if (analyzerInformation) {
         mLogger->setAnalyzerInfo(nullptr);
         analyzerInformation.reset();

lib/settings.h

@@ -435,6 +435,9 @@ class CPPCHECKLIB WARN_UNUSED Settings {
     /** @brief The maximum time in seconds for the typedef simplification */
     std::size_t typedefMaxTime{};
 
+    /** @brief Error IDs which should not be reported as unmatchedSuppression */
+    std::vector<std::string> unmatchedSuppressionFilters;
+
     /** @brief defines given by the user */
     std::string userDefines;
 

lib/suppressions.cpp

@@ -37,7 +37,6 @@
 
 #include "xml.h"
 
-static const char ID_UNUSEDFUNCTION[] = "unusedFunction";
 static const char ID_CHECKERSREPORT[] = "checkersReport";
 
 SuppressionList::ErrorMessage SuppressionList::ErrorMessage::fromErrorMessage(const ::ErrorMessage &msg, const std::set<std::string> &macroNames)
@@ -312,13 +311,10 @@ bool SuppressionList::updateSuppressionState(const SuppressionList::Suppression&
     auto foundSuppression = std::find_if(mSuppressions.begin(), mSuppressions.end(),
                                          std::bind(&Suppression::isSameParameters, &suppression, std::placeholders::_1));
     if (foundSuppression != mSuppressions.end()) {
-        // Update state of existing global suppression
-        if (!suppression.isLocal()) {
-            if (suppression.checked)
-                foundSuppression->checked = true;
-            if (suppression.matched)
-                foundSuppression->matched = true;
-        }
+        if (suppression.checked)
+            foundSuppression->checked = true;
+        if (suppression.matched)
+            foundSuppression->matched = true;
         return true;
     }
 
@@ -555,7 +551,7 @@ void SuppressionList::dump(std::ostream & out, const std::string& filePath) cons
     out << "  </suppressions>" << std::endl;
 }
 
-std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppressions(const FileWithDetails &file, const bool includeUnusedFunction) const
+std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppressions(const FileWithDetails &file) const
 {
     std::lock_guard<std::mutex> lg(mSuppressionsSync);
 
@@ -573,16 +569,14 @@ std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppre
             continue;
         if (s.errorId == ID_CHECKERSREPORT)
             continue;
-        if (!includeUnusedFunction && s.errorId == ID_UNUSEDFUNCTION)
-            continue;
         if (!s.isLocal() || s.fileName != file.spath())
             continue;
         result.push_back(s);
     }
     return result;
 }
 
-std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppressions(const bool includeUnusedFunction) const
+std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppressions() const
 {
     std::lock_guard<std::mutex> lg(mSuppressionsSync);
 
@@ -596,8 +590,6 @@ std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppr
             continue;
         if (s.hash > 0)
             continue;
-        if (!includeUnusedFunction && s.errorId == ID_UNUSEDFUNCTION)
-            continue;
         if (s.errorId == ID_CHECKERSREPORT)
             continue;
         if (s.isLocal())
@@ -607,7 +599,7 @@ std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppr
     return result;
 }
 
-std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedInlineSuppressions(const bool includeUnusedFunction) const
+std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedInlineSuppressions() const
 {
     std::list<SuppressionList::Suppression> result;
     for (const SuppressionList::Suppression &s : SuppressionList::mSuppressions) {
@@ -620,8 +612,6 @@ std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedInlineSuppr
             continue;
         if (s.hash > 0)
             continue;
-        if (!includeUnusedFunction && s.errorId == ID_UNUSEDFUNCTION)
-            continue;
         result.push_back(s);
     }
     return result;
@@ -660,39 +650,6 @@ void SuppressionList::markUnmatchedInlineSuppressionsAsChecked(const Tokenizer &
     }
 }
 
-bool SuppressionList::reportUnmatchedSuppressions(const std::list<SuppressionList::Suppression> &unmatched, ErrorLogger &errorLogger)
-{
-    bool err = false;
-    // Report unmatched suppressions
-    for (const SuppressionList::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 SuppressionList::Suppression &s2 : unmatched) {
-            if (s2.errorId == "unmatchedSuppression") {
-                if ((s2.fileName.empty() || s2.fileName == "*" || s2.fileName == s.fileName) &&
-                    (s2.lineNumber == SuppressionList::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(std::move(callStack), "", Severity::information, "Unmatched suppression: " + s.errorId, "unmatchedSuppression", Certainty::normal));
-        err = true;
-    }
-    return err;
-}
-
 std::string SuppressionList::Suppression::toString() const
 {
     std::string s;

lib/suppressions.h

@@ -34,7 +34,6 @@
 
 class Tokenizer;
 class ErrorMessage;
-class ErrorLogger;
 enum class Certainty : std::uint8_t;
 class FileWithDetails;
 
@@ -258,19 +257,19 @@ class CPPCHECKLIB SuppressionList {
      * @brief Returns list of unmatched local (per-file) suppressions.
      * @return list of unmatched suppressions
      */
-    std::list<Suppression> getUnmatchedLocalSuppressions(const FileWithDetails &file, bool includeUnusedFunction) const;
+    std::list<Suppression> getUnmatchedLocalSuppressions(const FileWithDetails &file) const;
 
     /**
      * @brief Returns list of unmatched global (glob pattern) suppressions.
      * @return list of unmatched suppressions
      */
-    std::list<Suppression> getUnmatchedGlobalSuppressions(bool includeUnusedFunction) const;
+    std::list<Suppression> getUnmatchedGlobalSuppressions() const;
 
     /**
      * @brief Returns list of unmatched inline suppressions.
      * @return list of unmatched suppressions
      */
-    std::list<Suppression> getUnmatchedInlineSuppressions(bool includeUnusedFunction) const;
+    std::list<Suppression> getUnmatchedInlineSuppressions() const;
 
     /**
      * @brief Returns list of all suppressions.
@@ -283,13 +282,6 @@ class CPPCHECKLIB SuppressionList {
      */
     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<SuppressionList::Suppression> &unmatched, ErrorLogger &errorLogger);
-
 private:
     mutable std::mutex mSuppressionsSync;
     /** @brief List of error which the user doesn't want to see. */

test/cli/inline-suppress_test.py

@@ -501,3 +501,60 @@ def test_unmatched_cfg():
     ]
     assert stdout == ''
     assert ret == 0, stdout
+
+
+# do not report unmatched unusedFunction inline suppressions when unusedFunction check is disabled
+# unusedFunction is disabled when -j2 is specified without a builddir
+def test_unused_function_disabled_unmatched_j():
+    args = [
+        '-q',
+        '--template=simple',
+        '--enable=warning,information',
+        '--inline-suppr',
+        '-j2',
+        '--no-cppcheck-build-dir',
+        'proj-inline-suppress/unusedFunctionUnmatched.cpp'
+    ]
+
+    ret, stdout, stderr = cppcheck(args, cwd=__script_dir)
+    assert stderr.splitlines() == [
+        '{}unusedFunctionUnmatched.cpp:5:0: information: Unmatched suppression: uninitvar [unmatchedSuppression]'.format(__proj_inline_suppres_path)
+    ]
+    assert stdout == ''
+    assert ret == 0, stdout
+
+
+# do not report unmatched misra-* inline suppressions when misra is not provided
+def test_misra_disabled_unmatched():  #14232
+    args = [
+        '-q',
+        '--template=simple',
+        '--enable=warning,information',
+        '--inline-suppr',
+        'proj-inline-suppress/misraUnmatched.c'
+    ]
+
+    ret, stdout, stderr = cppcheck(args, cwd=__script_dir)
+    assert stderr.splitlines() == [
+        '{}misraUnmatched.c:5:0: information: Unmatched suppression: uninitvar [unmatchedSuppression]'.format(__proj_inline_suppres_path)
+    ]
+    assert stdout == ''
+    assert ret == 0, stdout
+
+
+# do not report unmatched premium-* inline suppressions when application is not premium
+def test_premium_disabled_unmatched():  #13663
+    args = [
+        '-q',
+        '--template=simple',
+        '--enable=warning,information',
+        '--inline-suppr',
+        'proj-inline-suppress/premiumUnmatched.cpp'
+    ]
+
+    ret, stdout, stderr = cppcheck(args, cwd=__script_dir)
+    assert stderr.splitlines() == [
+        '{}premiumUnmatched.cpp:5:0: information: Unmatched suppression: uninitvar [unmatchedSuppression]'.format(__proj_inline_suppres_path)
+    ]
+    assert stdout == ''
+    assert ret == 0, stdout