Fix #14974: [regression] inline supression does not work with build dir anymore

Makefile

@@ -792,7 +792,7 @@ test/testcondition.o: test/testcondition.cpp lib/check.h lib/checkcondition.h li
 test/testconstructors.o: test/testconstructors.cpp lib/check.h lib/checkclass.h lib/checkers.h lib/checkimpl.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h
 	$(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testconstructors.cpp
 
-test/testcppcheck.o: test/testcppcheck.cpp externals/simplecpp/simplecpp.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/suppressions.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h test/redirect.h
+test/testcppcheck.o: test/testcppcheck.cpp externals/simplecpp/simplecpp.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/suppressions.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h test/redirect.h
 	$(CXX) ${INCLUDE_FOR_TEST} ${CFLAGS_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testcppcheck.cpp
 
 test/testerrorlogger.o: test/testerrorlogger.cpp externals/tinyxml2/tinyxml2.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/xml.h test/fixture.h test/helpers.h

lib/analyzerinfo.cpp

@@ -58,6 +58,68 @@ void AnalyzerInformation::writeFilesTxt(const std::string &buildDir, const std::
     fout << getFilesTxt(sourcefiles, fileSettings);
 }
 
+void AnalyzerInformation::writeIncludes(const std::set<std::string> &files)
+{
+    if (!files.empty() && mOutputStream.is_open()) {
+        mOutputStream << "  <includes>\n";
+        for (const std::string &file : files) {
+            mOutputStream << "    <filename>" << file << "</filename>\n";
+        }
+        mOutputStream << "  </includes>\n";
+    }
+}
+
+std::set<std::string> AnalyzerInformation::getIncludes(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, std::size_t fsFileId) const
+{
+    if (mOutputStream.is_open())
+        throw std::runtime_error("analyzer information file is already open");
+
+    std::set<std::string> files;
+
+    if (buildDir.empty() || sourcefile.empty())
+        return files;
+
+    const std::string analyzerInfoFile = AnalyzerInformation::getAnalyzerInfoFile(buildDir, sourcefile, cfg, fsFileId);
+
+    tinyxml2::XMLDocument analyzerInfoDoc;
+    if (analyzerInfoDoc.LoadFile(analyzerInfoFile.c_str()) != tinyxml2::XML_SUCCESS)
+        return files;
+
+    const tinyxml2::XMLElement *const rootNode = analyzerInfoDoc.FirstChildElement();
+    if (rootNode == nullptr)
+        return files;
+
+    if (strcmp(rootNode->Name(), "analyzerinfo") != 0)
+        return files;
+
+    const tinyxml2::XMLElement *includesNode = nullptr;
+    for (const tinyxml2::XMLElement *e = rootNode->FirstChildElement(); e; e = e->NextSiblingElement()) {
+        if (strcmp(e->Name(), "includes") == 0) {
+            includesNode = e;
+            break;
+        }
+    }
+
+    if (includesNode == nullptr)
+        return files;
+
+    for (const tinyxml2::XMLElement *e = includesNode->FirstChildElement(); e; e = e->NextSiblingElement()) {
+        if (strcmp(e->Name(), "filename") != 0)
+            continue;
+
+        files.insert(e->GetText());
+    }
+
+    return files;
+}
+
+void AnalyzerInformation::writeHash(std::size_t hash)
+{
+    if (mOutputStream.is_open()) {
+        mOutputStream << "  <hash>" << hash << "</hash>\n";
+    }
+}
+
 std::string AnalyzerInformation::getFilesTxt(const std::list<std::string> &sourcefiles, const std::list<FileSettings> &fileSettings) {
     std::ostringstream ret;
 
@@ -94,10 +156,17 @@ std::string AnalyzerInformation::skipAnalysis(const tinyxml2::XMLDocument &analy
     if (strcmp(rootNode->Name(), "analyzerinfo") != 0)
         return "unexpected root node";
 
-    const char * const attr = rootNode->Attribute("hash");
-    if (!attr)
-        return "no 'hash' attribute found";
-    if (attr != std::to_string(hash))
+    const tinyxml2::XMLElement *hashNode = nullptr;
+    for (const tinyxml2::XMLElement *e = rootNode->FirstChildElement(); e; e = e->NextSiblingElement()) {
+        if (strcmp(e->Name(), "hash") == 0) {
+            hashNode = e;
+            break;
+        }
+    }
+
+    if (!hashNode)
+        return "no 'hash' node found";
+    if (hashNode->GetText() != std::to_string(hash))
         return "hash mismatch";
 
     for (const tinyxml2::XMLElement *e = rootNode->FirstChildElement(); e; e = e->NextSiblingElement()) {
@@ -194,7 +263,7 @@ bool AnalyzerInformation::analyzeFile(const std::string &buildDir, const std::st
     if (!mOutputStream.is_open())
         throw std::runtime_error("failed to open '" + analyzerInfoFile + "'");
     mOutputStream << "<?xml version=\"1.0\"?>\n";
-    mOutputStream << "<analyzerinfo hash=\"" << hash << "\">\n";
+    mOutputStream << "<analyzerinfo>\n";
 
     return true;
 }

lib/analyzerinfo.h

@@ -27,6 +27,7 @@
 #include <fstream>
 #include <functional>
 #include <list>
+#include <set>
 #include <string>
 
 class ErrorMessage;
@@ -67,6 +68,9 @@ class CPPCHECKLIB AnalyzerInformation {
     bool analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, std::size_t fsFileId, std::size_t hash, std::list<ErrorMessage> &errors, bool debug = false);
     void reportErr(const ErrorMessage &msg);
     void setFileInfo(const std::string &check, const std::string &fileInfo);
+    void writeIncludes(const std::set<std::string> &files);
+    std::set<std::string> getIncludes(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, std::size_t fsFileId) const;
+    void writeHash(std::size_t hash);
     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);

lib/cppcheck.cpp

@@ -1026,25 +1026,6 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
         preprocessor.inlineSuppressions(mSuppressions.nomsg);
         preprocessor.removeComments();
 
-        if (!mSettings.buildDir.empty()) {
-            analyzerInformation.reset(new AnalyzerInformation);
-            mLogger->setAnalyzerInfo(analyzerInformation.get());
-        }
-
-        if (analyzerInformation) {
-            // Calculate hash so it can be compared with old hash / future hashes
-            const std::size_t hash = calculateHash(preprocessor, file.spath());
-            std::list<ErrorMessage> errors;
-            if (!analyzerInformation->analyzeFile(mSettings.buildDir, file.spath(), cfgname, file.fsFileId(), hash, errors, mSettings.debugainfo)) {
-                while (!errors.empty()) {
-                    mErrorLogger.reportErr(errors.front());
-                    errors.pop_front();
-                }
-                mLogger->setAnalyzerInfo(nullptr);
-                return mLogger->exitcode();  // known results => no need to reanalyze file
-            }
-        }
-
         // Get directives
         std::list<Directive> directives;
         preprocessor.createDirectives(directives);
@@ -1062,7 +1043,13 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
                        std::inserter(configDefines, configDefines.end()),
                        getDefineName);
 
+        // Keep track of all included files when using build dir
+        std::set<std::string> includedFiles;
+
         preprocessor.setLoadCallback([&](simplecpp::FileData &data, bool loaded) {
+            if (analyzerInformation) {
+                includedFiles.insert(data.filename);
+            }
             if (loaded) {
                 // Do preprocessing on included file
                 mLogger->addRemarkComments(preprocessor.getRemarkComments(data.tokens));
@@ -1078,12 +1065,37 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
 
         preprocessor.setPlatformInfo();
 
+        if (!mSettings.buildDir.empty()) {
+            analyzerInformation.reset(new AnalyzerInformation);
+            mLogger->setAnalyzerInfo(analyzerInformation.get());
+        }
+
+        if (analyzerInformation) {
+            // Load all included files to get correct hashes and suppressions
+            for (const std::string &filename : analyzerInformation->getIncludes(mSettings.buildDir, file.spath(), cfgname, file.fsFileId()))
+                preprocessor.loadFile(files, filename);
+            // Calculate hash so it can be compared with old hash / future hashes
+            const std::size_t hash = calculateHash(preprocessor, file.spath());
+            std::list<ErrorMessage> errors;
+            if (!analyzerInformation->analyzeFile(mSettings.buildDir, file.spath(), cfgname, file.fsFileId(), hash, errors, mSettings.debugainfo)) {
+                while (!errors.empty()) {
+                    mErrorLogger.reportErr(errors.front());
+                    errors.pop_front();
+                }
+                mLogger->setAnalyzerInfo(nullptr);
+                return mLogger->exitcode();  // known results => no need to reanalyze file
+            }
+            // Clear included file list; we don't want to keep includes that have been removed from the source
+            // Any includes that are still present will be readded
+            includedFiles.clear();
+        }
+
         // Get configurations..
         if (maxConfigs > 1) {
             Timer::run("Preprocessor::getConfigs", mTimerResults, [&]() {
                 configurations = { "" };
                 preprocessor.getConfigs(configDefines, configurations);
-                preprocessor.loadFiles(files);
+                preprocessor.loadAllIncludes(files);
             });
         } else {
             configurations = { mSettings.userDefines };
@@ -1306,6 +1318,11 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
             mLogger->setPlistFilenames(std::move(files));
         }
 
+        if (analyzerInformation) {
+            analyzerInformation->writeIncludes(includedFiles);
+            analyzerInformation->writeHash(calculateHash(preprocessor, file.spath()));
+        }
+
         executeAddons(dumpFile, file);
     } catch (const TerminateException &) {
         // Analysis is terminated

lib/preprocessor.cpp

@@ -833,7 +833,7 @@ const simplecpp::Output* Preprocessor::handleErrors(const simplecpp::OutputList&
     return reportOutput(outputList, showerror);
 }
 
-bool Preprocessor::loadFiles(std::vector<std::string> &files)
+bool Preprocessor::loadAllIncludes(std::vector<std::string> &files)
 {
     const simplecpp::DUI dui = createDUI(mSettings, "", mLang);
 
@@ -842,6 +842,13 @@ bool Preprocessor::loadFiles(std::vector<std::string> &files)
     return !handleErrors(outputList);
 }
 
+simplecpp::FileData *Preprocessor::loadFile(std::vector<std::string> &files, const std::string &file)
+{
+    const simplecpp::DUI dui = createDUI(mSettings, "", mLang);
+
+    return mFileCache.get("", file, dui, false, files, nullptr).first;
+}
+
 void Preprocessor::removeComments()
 {
     removeComments(mTokens);

lib/preprocessor.h

@@ -122,7 +122,9 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor {
 
     std::vector<RemarkComment> getRemarkComments(const simplecpp::TokenList &tokens) const;
 
-    bool loadFiles(std::vector<std::string> &files);
+    bool loadAllIncludes(std::vector<std::string> &files);
+
+    simplecpp::FileData *loadFile(std::vector<std::string> &files, const std::string &file);
 
     void removeComments();
 

test/cli/inline-suppress_test.py

@@ -248,6 +248,30 @@ def test_build_dir(tmpdir):
     assert stdout == ''
     assert ret == 0, stdout
 
+
+def test_build_dir_include(tmpdir):
+    args = [
+        '-q',
+        '--template=simple',
+        '--cppcheck-build-dir={}'.format(tmpdir),
+        '--enable=all',
+        '--inline-suppr',
+        '{}5.cpp'.format(__proj_inline_suppres_path)
+    ]
+
+    ret, stdout, stderr = cppcheck(args, cwd=__script_dir)
+    lines = stderr.splitlines()
+    assert lines == []
+    assert stdout == ''
+    assert ret == 0, stdout
+
+    ret, stdout, stderr = cppcheck(args, cwd=__script_dir)
+    lines = stderr.splitlines()
+    assert lines == []
+    assert stdout == ''
+    assert ret == 0, stdout
+
+
 def test_build_dir_jobs_suppressions(tmpdir): #14064
     args = [
         '-q',

test/cli/other_test.py

@@ -2320,7 +2320,7 @@ def test_builddir_hash_check_level(tmp_path):  # #13376
     cache_file = (build_dir / 'test.a1')
 
     root = ElementTree.fromstring(cache_file.read_text())
-    hash_1 = root.get('hash')
+    hash_1 = root.findtext('hash')
 
     args += ['--check-level=exhaustive']
 
@@ -2329,7 +2329,7 @@ def test_builddir_hash_check_level(tmp_path):  # #13376
     assert stderr == ''
 
     root = ElementTree.fromstring(cache_file.read_text())
-    hash_2 = root.get('hash')
+    hash_2 = root.findtext('hash')
 
     assert hash_1 != hash_2
 
@@ -4531,17 +4531,17 @@ def run_and_assert_cppcheck(stdout_exp):
         "discarding cached result from '{}' for '{}' - unexpected root node".format(test_a1_file_s, test_file_s)
     ])
 
-    # missing 'hash' attribute
+    # missing 'hash' node
     with open(test_a1_file, 'w') as f:
         f.write('<?xml version="1.0"?><analyzerinfo/>')
 
     run_and_assert_cppcheck([
-        "discarding cached result from '{}' for '{}' - no 'hash' attribute found".format(test_a1_file_s, test_file_s)
+        "discarding cached result from '{}' for '{}' - no 'hash' node found".format(test_a1_file_s, test_file_s)
     ])
 
-    # invalid 'hash' attribute
+    # invalid 'hash' node
     with open(test_a1_file, 'w') as f:
-        f.write('<?xml version="1.0"?><analyzerinfo hash="hash"/>')
+        f.write('<?xml version="1.0"?><analyzerinfo><hash>hash</hash></analyzerinfo>')
 
     run_and_assert_cppcheck([
         "discarding cached result from '{}' for '{}' - hash mismatch".format(test_a1_file_s, test_file_s)

test/cli/premium_test.py

@@ -88,11 +88,11 @@ def test_build_dir_hash_cppcheck_product(tmpdir):
     assert exitcode == 0
 
     def _get_hash(s:str):
-        i = s.find(' hash="')
+        i = s.find('<hash>')
         if i <= -1:
             return ''
         i += 7
-        return s[i:s.find('"', i)]
+        return s[i:s.find('</hash>', i)]
 
     with open(build_dir.join('test.a1'), 'rt') as f:
         f1 = f.read()

test/cli/proj-inline-suppress/5.cpp

@@ -0,0 +1 @@
+#include "5.h"

test/cli/proj-inline-suppress/5.h

@@ -0,0 +1,5 @@
+struct expected
+{
+    // cppcheck-suppress noExplicitConstructor
+    expected(int){}
+};

test/helpers.cpp

@@ -121,7 +121,7 @@ void SimpleTokenizer2::preprocess(const char* code, std::size_t size, std::vecto
     simplecpp::TokenList tokens1({code, size}, files, file0, dui, &outputList);
 
     Preprocessor preprocessor(tokens1, tokenizer.getSettings(), errorlogger, Path::identify(tokens1.getFiles()[0], false));
-    (void)preprocessor.loadFiles(files); // TODO: check result
+    (void)preprocessor.loadAllIncludes(files); // TODO: check result
     simplecpp::TokenList tokens2 = preprocessor.preprocess("", files, outputList);
     (void)preprocessor.reportOutput(outputList, true);
 

test/helpers.h

@@ -27,6 +27,7 @@
 #include "tokenlist.h"
 
 #include <cstddef>
+#include <cstdio>
 #include <stdexcept>
 #include <sstream>
 #include <string>
@@ -165,6 +166,24 @@ class ScopedFile {
     const std::string mFullPath;
 };
 
+// File deleter
+class FilesDeleter {
+public:
+    ~FilesDeleter()
+    {
+        for (const std::string &fileName: mFilenames)
+            std::remove(fileName.c_str());
+    }
+
+    void addFile(const std::string &fileName)
+    {
+        mFilenames.push_back(fileName);
+    }
+
+private:
+    std::vector<std::string> mFilenames;
+};
+
 namespace cppcheck {
     template<typename T>
     std::size_t count_all_of(const std::string& str, T sub) {