fixed #13087 - store unmatchedSuppression in AnalyzerInformation

Author: firewave

cli/cppcheckexecutor.cpp

@@ -299,9 +299,10 @@ int CppCheckExecutor::check_wrapper(const Settings& settings, Suppressions& supp
  * @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)
+static std::vector<::ErrorMessage> reportUnmatchedSuppressions(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
@@ -329,10 +330,11 @@ static bool reportUnmatchedSuppressions(const std::list<SuppressionList::Suppres
         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;
+        auto errmsg = ::ErrorMessage(std::move(callStack), "", Severity::information, "Unmatched suppression: " + s.errorId, "unmatchedSuppression", Certainty::normal);
+        errors.emplace_back(std::move(errmsg));
     }
-    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) {
@@ -358,21 +360,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 read the XML.
+        // re-ordering 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 = ::reportUnmatchedSuppressions(supprlist.getUnmatchedLocalSuppressions(*i), settings.unmatchedSuppressionFilters);
+        err |= reportErrorsFn(i->spath(), 0, errors);
     }
 
     for (auto i = fileSettings.cbegin(); i != fileSettings.cend(); ++i) {
-        err |= ::reportUnmatchedSuppressions(supprlist.getUnmatchedLocalSuppressions(i->file), errorLogger, settings.unmatchedSuppressionFilters);
+        const std::vector<ErrorMessage> errors = ::reportUnmatchedSuppressions(supprlist.getUnmatchedLocalSuppressions(i->file), settings.unmatchedSuppressionFilters);
+        err |= reportErrorsFn(i->file.spath(), i->fileIndex, errors);
     }
 
     if (settings.inlineSuppressions) {
-        err |= ::reportUnmatchedSuppressions(supprlist.getUnmatchedInlineSuppressions(), errorLogger, settings.unmatchedSuppressionFilters);
+        const std::vector<ErrorMessage> errors = ::reportUnmatchedSuppressions(supprlist.getUnmatchedInlineSuppressions(), settings.unmatchedSuppressionFilters);
+        for (const auto& errmsg : errors) {
+            std::string sourcefile;
+            if (!errmsg.callStack.empty())
+                sourcefile = errmsg.callStack.cbegin()->getfile();
+            err |= reportErrorsFn(sourcefile, 0, {errmsg});
+        }
     }
 
-    err |= ::reportUnmatchedSuppressions(supprlist.getUnmatchedGlobalSuppressions(), errorLogger, settings.unmatchedSuppressionFilters);
+    const std::vector<ErrorMessage> errors = ::reportUnmatchedSuppressions(supprlist.getUnmatchedGlobalSuppressions(), settings.unmatchedSuppressionFilters);
+    for (const auto& errmsg : errors) {
+        std::string sourcefile;
+        if (!errmsg.callStack.empty())
+            sourcefile = errmsg.callStack.cbegin()->getfile();
+        err |= reportErrorsFn(sourcefile, 0, {errmsg});
+    }
     return err;
 }
 
@@ -425,9 +461,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

@@ -149,7 +149,6 @@ bool AnalyzerInformation::analyzeFile(const std::string &buildDir, const std::st
 {
     if (buildDir.empty() || sourcefile.empty())
         return true;
-    close();
 
     const std::string analyzerInfoFile = AnalyzerInformation::getAnalyzerInfoFile(buildDir,sourcefile,cfg,fileIndex);
 
@@ -162,11 +161,16 @@ bool AnalyzerInformation::analyzeFile(const std::string &buildDir, const std::st
     if (mOutputStream.is_open()) {
         mOutputStream << "<?xml version=\"1.0\"?>\n";
         mOutputStream << "<analyzerinfo hash=\"" << hash << "\">\n";
-    }
+    } else {
+         // TODO: report error
+         // TODO: return false?
+     }
 
     return true;
 }
 
+#include <iostream>
+
 void AnalyzerInformation::reportErr(const ErrorMessage &msg)
 {
     if (mOutputStream.is_open())
@@ -240,3 +244,23 @@ void AnalyzerInformation::processFilesTxt(const std::string& buildDir, const std
     }
 }
 
+void AnalyzerInformation::reopen(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, int fileIndex)
+{
+    if (buildDir.empty() || sourcefile.empty())
+        return;
+
+    const std::string analyzerInfoFile = AnalyzerInformation::getAnalyzerInfoFile(buildDir,sourcefile,cfg,fileIndex);
+    std::ifstream ifs(analyzerInfoFile);
+    if (!ifs.is_open())
+        return;
+
+    std::ostringstream iss;
+    iss << ifs.rdbuf();
+    ifs.close();
+
+    std::string content = iss.str();
+    content = content.substr(0, content.find("</analyzerinfo>"));
+
+    mOutputStream.open(analyzerInfoFile, std::ios::trunc);
+    mOutputStream << content;
+}

lib/analyzerinfo.h

@@ -66,6 +66,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, int fileIndex);
 
+    void reopen(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, int fileIndex);
+
     static const char sep = ':';
 
     class CPPCHECKLIB Info {

test/cli/other_test.py

@@ -2353,12 +2353,12 @@ 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)
     __test_inline_suppr(tmp_path, ['--cppcheck-build-dir={}'.format(build_dir), '-j1'])
+    with open(build_dir / 'test.a1' , 'r') as f:
+        print(f.readlines())
     __test_inline_suppr(tmp_path, ['--cppcheck-build-dir={}'.format(build_dir), '-j1'])
 
 
@@ -2368,8 +2368,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)