fixed #13087 - store unmatchedSuppression in AnalyzerInformation (#8173)

Author: firewave

cli/cppcheckexecutor.cpp

@@ -295,13 +295,15 @@ int CppCheckExecutor::check_wrapper(const Settings& settings, Suppressions& supp
 }
 
 /**
- * Report unmatched suppressions
- * @param unmatched list of unmatched suppressions (from Settings::Suppressions::getUnmatched(Local|Global)Suppressions)
- * @return true is returned if errors are reported
+ * Get list of unmatchedSuppression errors
+ * @param unmatched list of unmatched suppressions
+ * @param filters a list of (globbed) IDs to filter out
+ * @return vector of unmatchedSuppression errors
  */
-static bool reportUnmatchedSuppressions(const std::list<SuppressionList::Suppression> &unmatched, ErrorLogger &errorLogger, const std::vector<std::string>& filters)
+static std::vector<ErrorMessage> getUnmatchedSuppressions(const std::list<SuppressionList::Suppression> &unmatched, const std::vector<std::string>& filters)
 {
-    bool err = false;
+    std::vector<ErrorMessage> errors;
+
     // Report unmatched suppressions
     for (const SuppressionList::Suppression &s : unmatched) {
         // check if this unmatched suppression is suppressed
@@ -325,15 +327,15 @@ static bool reportUnmatchedSuppressions(const std::list<SuppressionList::Suppres
         if (skip)
             continue;
 
-        std::list<::ErrorMessage::FileLocation> callStack;
+        std::list<ErrorMessage::FileLocation> callStack;
         if (!s.fileName.empty()) {
             callStack.emplace_back(s.fileName, s.lineNumber, 0);
         }
         const std::string unmatchedSuppressionId = s.isPolyspace ? "unmatchedPolyspaceSuppression" : "unmatchedSuppression";
-        errorLogger.reportErr(::ErrorMessage(std::move(callStack), "", Severity::information, "Unmatched suppression: " + s.errorId, unmatchedSuppressionId, Certainty::normal));
-        err = true;
+        errors.emplace_back(std::move(callStack), "", Severity::information, "Unmatched suppression: " + s.errorId, unmatchedSuppressionId, Certainty::normal);
     }
-    return err;
+
+    return errors;
 }
 
 bool CppCheckExecutor::reportUnmatchedSuppressions(const Settings &settings, const SuppressionList& suppressions, const std::list<FileWithDetails> &files, const std::list<FileSettings>& fileSettings, ErrorLogger& errorLogger) {
@@ -359,21 +361,55 @@ bool CppCheckExecutor::reportUnmatchedSuppressions(const Settings &settings, con
         supprlist.addSuppression(std::move(s));
     }
 
+    const auto reportErrorsFn = [&](const std::string& sourcefile, std::size_t fsFileId, const std::vector<ErrorMessage>& errors) -> bool {
+        if (errors.empty())
+            return false;
+
+        // TODO: what if sourcefile is empty?
+
+        AnalyzerInformation analyzerInfo;
+        // FIXME: this is a horrible hack
+        // we need to "re-open" the file so we can add the unmatchedSuppression findings.
+        // we cannot keep it open conditionally because the whole program analysis reads the XML.
+        // re-ordering the code is also not an option because the unmatched suppression reporting needs to be run after all other checks.
+        analyzerInfo.reopen(settings.buildDir, sourcefile, /*cfgname*/ "", fsFileId);
+
+        for (const auto& errmsg : errors) {
+            analyzerInfo.reportErr(errmsg);
+            errorLogger.reportErr(errmsg);
+        }
+        return true;
+    };
+
     bool err = false;
 
     for (auto i = files.cbegin(); i != files.cend(); ++i) {
-        err |= ::reportUnmatchedSuppressions(supprlist.getUnmatchedLocalSuppressions(*i), errorLogger, settings.unmatchedSuppressionFilters);
+        const std::vector<ErrorMessage> errors = getUnmatchedSuppressions(supprlist.getUnmatchedLocalSuppressions(*i), settings.unmatchedSuppressionFilters);
+        err |= reportErrorsFn(i->spath(), i->fsFileId(), errors);
     }
 
     for (auto i = fileSettings.cbegin(); i != fileSettings.cend(); ++i) {
-        err |= ::reportUnmatchedSuppressions(supprlist.getUnmatchedLocalSuppressions(i->file), errorLogger, settings.unmatchedSuppressionFilters);
+        const std::vector<ErrorMessage> errors = getUnmatchedSuppressions(supprlist.getUnmatchedLocalSuppressions(i->file), settings.unmatchedSuppressionFilters);
+        err |= reportErrorsFn(i->file.spath(), i->file.fsFileId(), errors);
     }
 
     if (settings.inlineSuppressions) {
-        err |= ::reportUnmatchedSuppressions(supprlist.getUnmatchedInlineSuppressions(), errorLogger, settings.unmatchedSuppressionFilters);
+        const std::vector<ErrorMessage> errors = getUnmatchedSuppressions(supprlist.getUnmatchedInlineSuppressions(), settings.unmatchedSuppressionFilters);
+        for (const auto& errmsg : errors) {
+            std::string sourcefile;
+            if (!errmsg.callStack.empty())
+                sourcefile = errmsg.callStack.cbegin()->getfile(false); // TODO: simplify path?
+            err |= reportErrorsFn(sourcefile, 0, {errmsg});
+        }
     }
 
-    err |= ::reportUnmatchedSuppressions(supprlist.getUnmatchedGlobalSuppressions(), errorLogger, settings.unmatchedSuppressionFilters);
+    const std::vector<ErrorMessage> errors = getUnmatchedSuppressions(supprlist.getUnmatchedGlobalSuppressions(), settings.unmatchedSuppressionFilters);
+    for (const auto& errmsg : errors) {
+        std::string sourcefile;
+        if (!errmsg.callStack.empty())
+            sourcefile = errmsg.callStack.cbegin()->getfile(false); // TODO: simplify path?
+        err |= reportErrorsFn(sourcefile, 0, {errmsg});
+    }
     return err;
 }
 
@@ -426,9 +462,10 @@ int CppCheckExecutor::check_internal(const Settings& settings, Suppressions& sup
 #endif
     }
 
+    // TODO: is this run again instead of using previously cached results?
     returnValue |= cppcheck.analyseWholeProgram(settings.buildDir, mFiles, mFileSettings, stdLogger.getCtuInfo());
 
-    if (settings.severity.isEnabled(Severity::information) || settings.checkConfiguration) {
+    if ((settings.severity.isEnabled(Severity::information) || settings.checkConfiguration) && !supprs.nomsg.getSuppressions().empty()) {
         const bool err = reportUnmatchedSuppressions(settings, supprs.nomsg, mFiles, mFileSettings, stdLogger);
         if (err && returnValue == 0)
             returnValue = settings.exitCode;

lib/analyzerinfo.cpp

@@ -292,3 +292,23 @@ std::string AnalyzerInformation::processFilesTxt(const std::string& buildDir, co
     return "";
 }
 
+void AnalyzerInformation::reopen(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, std::size_t fsFileId)
+{
+    if (buildDir.empty() || sourcefile.empty())
+        return;
+
+    const std::string analyzerInfoFile = AnalyzerInformation::getAnalyzerInfoFile(buildDir,sourcefile,cfg,fsFileId);
+    std::ifstream ifs(analyzerInfoFile);
+    if (!ifs.is_open())
+        return;
+
+    std::ostringstream iss;
+    iss << ifs.rdbuf();
+    ifs.close();
+
+    std::string content = iss.str();
+    content.resize(content.find("</analyzerinfo>"));
+
+    mOutputStream.open(analyzerInfoFile, std::ios::trunc);
+    mOutputStream << content;
+}

lib/analyzerinfo.h

@@ -69,6 +69,8 @@ class CPPCHECKLIB AnalyzerInformation {
     void setFileInfo(const std::string &check, const std::string &fileInfo);
     static std::string getAnalyzerInfoFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, std::size_t fsFileId);
 
+    void reopen(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, std::size_t fsFileId);
+
     static const char sep = ':';
 
     class CPPCHECKLIB Info {

test/cli/other_test.py

@@ -2373,8 +2373,6 @@ def test_inline_suppr_builddir(tmp_path):
     __test_inline_suppr(tmp_path, ['--cppcheck-build-dir={}'.format(build_dir), '-j1'])
 
 
-# TODO: the suppressions are generated outside of the scope which captures the analysis information
-@pytest.mark.xfail(strict=True)
 def test_inline_suppr_builddir_cached(tmp_path):
     build_dir = tmp_path / 'b1'
     os.mkdir(build_dir)
@@ -2388,8 +2386,6 @@ def test_inline_suppr_builddir_j(tmp_path):
     __test_inline_suppr(tmp_path, ['--cppcheck-build-dir={}'.format(build_dir), '-j2'])
 
 
-# TODO: the suppressions are generated outside of the scope which captures the analysis information
-@pytest.mark.xfail(strict=True)
 def test_inline_suppr_builddir_j_cached(tmp_path):
     build_dir = tmp_path / 'b1'
     os.mkdir(build_dir)