From 2c96167a082a407258c5ae44e0f312495ab970da Mon Sep 17 00:00:00 2001 From: firewave Date: Sun, 16 Apr 2023 18:29:59 +0200 Subject: [PATCH 1/8] SingleExecutor: added TODOs --- cli/singleexecutor.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cli/singleexecutor.cpp b/cli/singleexecutor.cpp index e1854234f38..639f03f28bf 100644 --- a/cli/singleexecutor.cpp +++ b/cli/singleexecutor.cpp @@ -51,6 +51,8 @@ unsigned int SingleExecutor::check() std::size_t processedsize = 0; unsigned int c = 0; + // TODO: processes either mSettings.project.fileSettings or mFiles - process/thread implementations process both + // TODO: thread/process implementations process fileSettings first if (mSettings.project.fileSettings.empty()) { for (std::map::const_iterator i = mFiles.cbegin(); i != mFiles.cend(); ++i) { if (!mSettings.library.markupFile(i->first) @@ -59,6 +61,7 @@ unsigned int SingleExecutor::check() processedsize += i->second; if (!mSettings.quiet) reportStatus(c + 1, mFiles.size(), processedsize, totalfilesize); + // TODO: call analyseClangTidy() c++; } } @@ -66,6 +69,7 @@ unsigned int SingleExecutor::check() // filesettings // check all files of the project for (const ImportProject::FileSettings &fs : mSettings.project.fileSettings) { + // TODO: handle markup files result += mCppcheck.check(fs); ++c; if (!mSettings.quiet) From ac544c946d2098d8c253adbcb292929a937cb434 Mon Sep 17 00:00:00 2001 From: firewave Date: Sun, 16 Apr 2023 18:37:40 +0200 Subject: [PATCH 2/8] test `SingleExecutor` with files and project --- lib/library.h | 2 +- test/testsingleexecutor.cpp | 35 +++++++++++++++++++++++++++++++---- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/lib/library.h b/lib/library.h index 5fac8a73308..0cce94533a0 100644 --- a/lib/library.h +++ b/lib/library.h @@ -53,7 +53,7 @@ namespace tinyxml2 { class CPPCHECKLIB Library { // TODO: get rid of this friend class TestSymbolDatabase; // For testing only - friend class TestSingleExecutor; // For testing only + friend class TestSingleExecutorBase; // For testing only friend class TestThreadExecutor; // For testing only friend class TestProcessExecutor; // For testing only diff --git a/test/testsingleexecutor.cpp b/test/testsingleexecutor.cpp index c3d647f7400..1867b7bc972 100644 --- a/test/testsingleexecutor.cpp +++ b/test/testsingleexecutor.cpp @@ -36,12 +36,13 @@ #include #include -class TestSingleExecutor : public TestFixture { -public: - TestSingleExecutor() : TestFixture("TestSingleExecutor") {} +class TestSingleExecutorBase : public TestFixture { +protected: + TestSingleExecutorBase(const char * const name, bool useFS) : TestFixture(name), useFS(useFS) {} private: Settings settings = settingsBuilder().library("std.cfg").build(); + bool useFS; static std::string zpad3(int i) { @@ -55,18 +56,29 @@ class TestSingleExecutor : public TestFixture { void check(int files, int result, const std::string &data, SHOWTIME_MODES showtime = SHOWTIME_MODES::SHOWTIME_NONE, const char* const plistOutput = nullptr, const std::vector& filesList = {}) { errout.str(""); output.str(""); + settings.project.fileSettings.clear(); std::map filemap; if (filesList.empty()) { for (int i = 1; i <= files; ++i) { const std::string s = "file_" + zpad3(i) + ".cpp"; filemap[s] = data.size(); + if (useFS) { + ImportProject::FileSettings fs; + fs.filename = s; + settings.project.fileSettings.emplace_back(std::move(fs)); + } } } else { for (const auto& f : filesList) { filemap[f] = data.size(); + if (useFS) { + ImportProject::FileSettings fs; + fs.filename = f; + settings.project.fileSettings.emplace_back(std::move(fs)); + } } } @@ -85,6 +97,10 @@ class TestSingleExecutor : public TestFixture { for (std::map::const_iterator i = filemap.cbegin(); i != filemap.cend(); ++i) scopedfiles.emplace_back(new ScopedFile(i->first, data)); + // clear files list so only fileSettings ware used + if (useFS) + filemap.clear(); + ASSERT_EQUALS(result, executor.check()); } @@ -217,4 +233,15 @@ class TestSingleExecutor : public TestFixture { // TODO: test whole program analysis }; -REGISTER_TEST(TestSingleExecutor) +class TestSingleExecutorFiles : public TestSingleExecutorBase { +public: + TestSingleExecutorFiles() : TestSingleExecutorBase("TestSingleExecutorFiles", false) {} +}; + +class TestSingleExecutorFS : public TestSingleExecutorBase { +public: + TestSingleExecutorFS() : TestSingleExecutorBase("TestSingleExecutorFS", true) {} +}; + +REGISTER_TEST(TestSingleExecutorFiles) +REGISTER_TEST(TestSingleExecutorFS) From 34b92a395b66db8a9d9dd0e25aaf6f74f9dc157d Mon Sep 17 00:00:00 2001 From: firewave Date: Sun, 16 Apr 2023 18:39:35 +0200 Subject: [PATCH 3/8] SingleExecutor: process markup files after code when scanning project --- cli/singleexecutor.cpp | 48 ++++++++++++++++++++++++++++++------------ releasenotes.txt | 1 + 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/cli/singleexecutor.cpp b/cli/singleexecutor.cpp index 639f03f28bf..ce7476f3d4c 100644 --- a/cli/singleexecutor.cpp +++ b/cli/singleexecutor.cpp @@ -69,25 +69,45 @@ unsigned int SingleExecutor::check() // filesettings // check all files of the project for (const ImportProject::FileSettings &fs : mSettings.project.fileSettings) { - // TODO: handle markup files - result += mCppcheck.check(fs); - ++c; - if (!mSettings.quiet) - reportStatus(c, mSettings.project.fileSettings.size(), c, mSettings.project.fileSettings.size()); - if (mSettings.clangTidy) - mCppcheck.analyseClangTidy(fs); + if (!mSettings.library.markupFile(fs.filename) + || !mSettings.library.processMarkupAfterCode(fs.filename)) { + result += mCppcheck.check(fs); + ++c; + if (!mSettings.quiet) + reportStatus(c, mSettings.project.fileSettings.size(), c, mSettings.project.fileSettings.size()); + if (mSettings.clangTidy) + mCppcheck.analyseClangTidy(fs); + } } } // second loop to parse all markup files which may not work until all // c/cpp files have been parsed and checked - for (std::map::const_iterator i = mFiles.cbegin(); i != mFiles.cend(); ++i) { - if (mSettings.library.markupFile(i->first) && mSettings.library.processMarkupAfterCode(i->first)) { - result += mCppcheck.check(i->first); - processedsize += i->second; - if (!mSettings.quiet) - reportStatus(c + 1, mFiles.size(), processedsize, totalfilesize); - c++; + // TODO: get rid of duplicated code + if (mSettings.project.fileSettings.empty()) { + for (std::map::const_iterator i = mFiles.cbegin(); i != mFiles.cend(); ++i) { + if (mSettings.library.markupFile(i->first) + && mSettings.library.processMarkupAfterCode(i->first)) { + result += mCppcheck.check(i->first); + processedsize += i->second; + if (!mSettings.quiet) + reportStatus(c + 1, mFiles.size(), processedsize, totalfilesize); + // TODO: call analyseClangTidy() + c++; + } + } + } + else { + for (const ImportProject::FileSettings &fs : mSettings.project.fileSettings) { + if (mSettings.library.markupFile(fs.filename) + && mSettings.library.processMarkupAfterCode(fs.filename)) { + result += mCppcheck.check(fs); + ++c; + if (!mSettings.quiet) + reportStatus(c, mSettings.project.fileSettings.size(), c, mSettings.project.fileSettings.size()); + if (mSettings.clangTidy) + mCppcheck.analyseClangTidy(fs); + } } } if (mCppcheck.analyseWholeProgram()) diff --git a/releasenotes.txt b/releasenotes.txt index fda9b05f3b8..afb3e40bbde 100644 --- a/releasenotes.txt +++ b/releasenotes.txt @@ -14,3 +14,4 @@ release notes for cppcheck-2.11 - `constVariableReference` - `constVariablePointer` - More command-line parameters will now check if the given integer argument is actually valid. Several other internal string-to-integer conversions will not be error checked. +- scanning projects (with -j1) will now defer the analysis of markup files until the whole code was processed From f1eaa9a2a2525df7d36e580f6cfb48e9f1ad0ae0 Mon Sep 17 00:00:00 2001 From: firewave Date: Tue, 18 Apr 2023 11:51:15 +0200 Subject: [PATCH 4/8] TestSingleExecutor: generate scoped files before calling executor --- test/testsingleexecutor.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/testsingleexecutor.cpp b/test/testsingleexecutor.cpp index 1867b7bc972..57c1699ac4a 100644 --- a/test/testsingleexecutor.cpp +++ b/test/testsingleexecutor.cpp @@ -90,17 +90,18 @@ class TestSingleExecutorBase : public TestFixture { return false; }); cppcheck.settings() = settings; - // TODO: test with settings.project.fileSettings; - SingleExecutor executor(cppcheck, filemap, settings, *this); + std::vector> scopedfiles; scopedfiles.reserve(filemap.size()); for (std::map::const_iterator i = filemap.cbegin(); i != filemap.cend(); ++i) scopedfiles.emplace_back(new ScopedFile(i->first, data)); - // clear files list so only fileSettings ware used + // clear files list so only fileSettings are used if (useFS) filemap.clear(); + // TODO: test with settings.project.fileSettings; + SingleExecutor executor(cppcheck, filemap, settings, *this); ASSERT_EQUALS(result, executor.check()); } From 0989fc4e2bb2d8e6efeb973df7da0ade1277711b Mon Sep 17 00:00:00 2001 From: firewave Date: Tue, 18 Apr 2023 12:14:34 +0200 Subject: [PATCH 5/8] CI-unixish.yml: added `--output-on-failure` to CTest call --- .github/workflows/CI-unixish.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/CI-unixish.yml b/.github/workflows/CI-unixish.yml index e941de7e043..08094cbfdd9 100644 --- a/.github/workflows/CI-unixish.yml +++ b/.github/workflows/CI-unixish.yml @@ -111,7 +111,7 @@ jobs: - name: Run CTest run: | pushd cmake.output - ctest -j$(nproc) + ctest --output-on-failure -j$(nproc) build_uchar: From a37bc6eaf9e7fe918433e40fce7d4a86c27f33f9 Mon Sep 17 00:00:00 2001 From: firewave Date: Tue, 18 Apr 2023 12:22:58 +0200 Subject: [PATCH 6/8] helpers.cpp: improved error reporting in `~ScopedFile()` --- test/helpers.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/test/helpers.cpp b/test/helpers.cpp index d18ef288940..8d818f6446e 100644 --- a/test/helpers.cpp +++ b/test/helpers.cpp @@ -22,6 +22,7 @@ #include "preprocessor.h" #include +#include #include #include #include @@ -60,12 +61,21 @@ ScopedFile::ScopedFile(std::string name, const std::string &content, std::string } ScopedFile::~ScopedFile() { - std::remove(mFullPath.c_str()); + const int remove_res = std::remove(mFullPath.c_str()); + if (remove_res != 0) { + std::cout << "ScopedFile(" << mFullPath + ") - could not delete file (" << remove_res << ")"; + } if (!mPath.empty() && mPath != Path::getCurrentPath()) { #ifdef _WIN32 - RemoveDirectoryA(mPath.c_str()); + if (!RemoveDirectoryA(mPath.c_str())) { + std::cout << "ScopedFile(" << mFullPath + ") - could not delete folder (" << GetLastError() << ")"; + } #else - rmdir(mPath.c_str()); + const int rmdir_res = rmdir(mPath.c_str()); + if (rmdir_res == -1) { + const int err = errno; + std::cout << "ScopedFile(" << mFullPath + ") - could not delete folder (" << err << ")"; + } #endif } } From 82dbf7064b0bc1bf7a421b1e6d5dcda7b3ffab06 Mon Sep 17 00:00:00 2001 From: firewave Date: Tue, 18 Apr 2023 12:50:29 +0200 Subject: [PATCH 7/8] use unique filenames in executor tests to avoid collisions --- test/testprocessexecutor.cpp | 25 +++++++++++++++---------- test/testsingleexecutor.cpp | 21 ++++++++++++++------- test/testthreadexecutor.cpp | 25 +++++++++++++++---------- 3 files changed, 44 insertions(+), 27 deletions(-) diff --git a/test/testprocessexecutor.cpp b/test/testprocessexecutor.cpp index c81402a1cb3..eb3d7a12ff0 100644 --- a/test/testprocessexecutor.cpp +++ b/test/testprocessexecutor.cpp @@ -41,6 +41,11 @@ class TestProcessExecutor : public TestFixture { private: Settings settings = settingsBuilder().library("std.cfg").build(); + std::string fprefix() const + { + return "process"; + } + /** * Execute check using n jobs for y files which are have * identical data, given within data. @@ -53,7 +58,7 @@ class TestProcessExecutor : public TestFixture { if (filesList.empty()) { for (int i = 1; i <= files; ++i) { std::ostringstream oss; - oss << "file_" << i << ".cpp"; + oss << fprefix() << "_" << i << ".cpp"; filemap[oss.str()] = data.size(); } } @@ -186,7 +191,7 @@ class TestProcessExecutor : public TestFixture { settings.library.mProcessAfterCode.emplace(".cp1", true); const std::vector files = { - "file_1.cp1", "file_2.cpp", "file_3.cp1", "file_4.cpp" + fprefix() + "_1.cp1", fprefix() + "_2.cpp", fprefix() + "_3.cp1", fprefix() + "_4.cpp" }; // the checks are not executed on the markup files => expected result is 2 @@ -198,21 +203,21 @@ class TestProcessExecutor : public TestFixture { "}", SHOWTIME_MODES::SHOWTIME_NONE, nullptr, files); // TODO: order of "Checking" and "checked" is affected by thread - /*TODO_ASSERT_EQUALS("Checking file_2.cpp ...\n" + /*TODO_ASSERT_EQUALS("Checking " + fprefix() + "_2.cpp ...\n" "1/4 files checked 25% done\n" - "Checking file_4.cpp ...\n" + "Checking " + fprefix() + "_4.cpp ...\n" "2/4 files checked 50% done\n" - "Checking file_1.cp1 ...\n" + "Checking " + fprefix() + "_1.cp1 ...\n" "3/4 files checked 75% done\n" - "Checking file_3.cp1 ...\n" + "Checking " + fprefix() + "_3.cp1 ...\n" "4/4 files checked 100% done\n", - "Checking file_1.cp1 ...\n" + "Checking " + fprefix() + "_1.cp1 ...\n" "1/4 files checked 25% done\n" - "Checking file_2.cpp ...\n" + "Checking " + fprefix() + "_2.cpp ...\n" "2/4 files checked 50% done\n" - "Checking file_3.cp1 ...\n" + "Checking " + fprefix() + "_3.cp1 ...\n" "3/4 files checked 75% done\n" - "Checking file_4.cpp ...\n" + "Checking " + fprefix() + "_4.cpp ...\n" "4/4 files checked 100% done\n", output.str());*/ settings = settingsOld; diff --git a/test/testsingleexecutor.cpp b/test/testsingleexecutor.cpp index 57c1699ac4a..1ebf9a83c58 100644 --- a/test/testsingleexecutor.cpp +++ b/test/testsingleexecutor.cpp @@ -44,6 +44,13 @@ class TestSingleExecutorBase : public TestFixture { Settings settings = settingsBuilder().library("std.cfg").build(); bool useFS; + std::string fprefix() const + { + if (useFS) + return "singlefs"; + return "single"; + } + static std::string zpad3(int i) { if (i < 10) @@ -61,7 +68,7 @@ class TestSingleExecutorBase : public TestFixture { std::map filemap; if (filesList.empty()) { for (int i = 1; i <= files; ++i) { - const std::string s = "file_" + zpad3(i) + ".cpp"; + const std::string s = fprefix() + "_" + zpad3(i) + ".cpp"; filemap[s] = data.size(); if (useFS) { ImportProject::FileSettings fs; @@ -129,7 +136,7 @@ class TestSingleExecutorBase : public TestFixture { "}"); std::string expected; for (int i = 1; i <= 100; ++i) { - expected += "Checking file_" + zpad3(i) + ".cpp ...\n"; + expected += "Checking " + fprefix() + "_" + zpad3(i) + ".cpp ...\n"; expected += std::to_string(i) + "/100 files checked " + std::to_string(i) + "% done\n"; } ASSERT_EQUALS(expected, output.str()); @@ -207,7 +214,7 @@ class TestSingleExecutorBase : public TestFixture { settings.library.mProcessAfterCode.emplace(".cp1", true); const std::vector files = { - "file_1.cp1", "file_2.cpp", "file_3.cp1", "file_4.cpp" + fprefix() + "_1.cp1", fprefix() + "_2.cpp", fprefix() + "_3.cp1", fprefix() + "_4.cpp" }; // checks are not executed on markup files => expected result is 2 @@ -219,13 +226,13 @@ class TestSingleExecutorBase : public TestFixture { "}", SHOWTIME_MODES::SHOWTIME_NONE, nullptr, files); // TODO: filter out the "files checked" messages - ASSERT_EQUALS("Checking file_2.cpp ...\n" + ASSERT_EQUALS("Checking " + fprefix() + "_2.cpp ...\n" "1/4 files checked 25% done\n" - "Checking file_4.cpp ...\n" + "Checking " + fprefix() + "_4.cpp ...\n" "2/4 files checked 50% done\n" - "Checking file_1.cp1 ...\n" + "Checking " + fprefix() + "_1.cp1 ...\n" "3/4 files checked 75% done\n" - "Checking file_3.cp1 ...\n" + "Checking " + fprefix() + "_3.cp1 ...\n" "4/4 files checked 100% done\n", output.str()); settings = settingsOld; } diff --git a/test/testthreadexecutor.cpp b/test/testthreadexecutor.cpp index 3e92b320038..e2cb9e0039e 100644 --- a/test/testthreadexecutor.cpp +++ b/test/testthreadexecutor.cpp @@ -41,6 +41,11 @@ class TestThreadExecutor : public TestFixture { private: Settings settings = settingsBuilder().library("std.cfg").build(); + std::string fprefix() const + { + return "thread"; + } + /** * Execute check using n jobs for y files which are have * identical data, given within data. @@ -53,7 +58,7 @@ class TestThreadExecutor : public TestFixture { if (filesList.empty()) { for (int i = 1; i <= files; ++i) { std::ostringstream oss; - oss << "file_" << i << ".cpp"; + oss << fprefix() << "_" << i << ".cpp"; filemap[oss.str()] = data.size(); } } @@ -184,7 +189,7 @@ class TestThreadExecutor : public TestFixture { settings.library.mProcessAfterCode.emplace(".cp1", true); const std::vector files = { - "file_1.cp1", "file_2.cpp", "file_3.cp1", "file_4.cpp" + fprefix() + "_1.cp1", fprefix() + "_2.cpp", fprefix() + "_3.cp1", fprefix() + "_4.cpp" }; // checks are not executed on markup files => expected result is 2 @@ -196,21 +201,21 @@ class TestThreadExecutor : public TestFixture { "}", SHOWTIME_MODES::SHOWTIME_NONE, nullptr, files); // TODO: order of "Checking" and "checked" is affected by thread - /*TODO_ASSERT_EQUALS("Checking file_2.cpp ...\n" + /*TODO_ASSERT_EQUALS("Checking " + fprefix() + "_2.cpp ...\n" "1/4 files checked 25% done\n" - "Checking file_4.cpp ...\n" + "Checking " + fprefix() + "_4.cpp ...\n" "2/4 files checked 50% done\n" - "Checking file_1.cp1 ...\n" + "Checking " + fprefix() + "_1.cp1 ...\n" "3/4 files checked 75% done\n" - "Checking file_3.cp1 ...\n" + "Checking " + fprefix() + "_3.cp1 ...\n" "4/4 files checked 100% done\n", - "Checking file_1.cp1 ...\n" + "Checking " + fprefix() + "_1.cp1 ...\n" "1/4 files checked 25% done\n" - "Checking file_2.cpp ...\n" + "Checking " + fprefix() + "_2.cpp ...\n" "2/4 files checked 50% done\n" - "Checking file_3.cp1 ...\n" + "Checking " + fprefix() + "_3.cp1 ...\n" "3/4 files checked 75% done\n" - "Checking file_4.cpp ...\n" + "Checking " + fprefix() + "_4.cpp ...\n" "4/4 files checked 100% done\n", output.str());*/ settings = settingsOld; From d34a95d850d38813bc62a58ae20b411a3e0a00b1 Mon Sep 17 00:00:00 2001 From: firewave Date: Tue, 18 Apr 2023 13:27:50 +0200 Subject: [PATCH 8/8] fixed `functionStatic` selfcheck warnings --- test/testprocessexecutor.cpp | 2 +- test/testthreadexecutor.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/testprocessexecutor.cpp b/test/testprocessexecutor.cpp index eb3d7a12ff0..cef576b5e66 100644 --- a/test/testprocessexecutor.cpp +++ b/test/testprocessexecutor.cpp @@ -41,7 +41,7 @@ class TestProcessExecutor : public TestFixture { private: Settings settings = settingsBuilder().library("std.cfg").build(); - std::string fprefix() const + static std::string fprefix() { return "process"; } diff --git a/test/testthreadexecutor.cpp b/test/testthreadexecutor.cpp index e2cb9e0039e..69b529a26e6 100644 --- a/test/testthreadexecutor.cpp +++ b/test/testthreadexecutor.cpp @@ -41,7 +41,7 @@ class TestThreadExecutor : public TestFixture { private: Settings settings = settingsBuilder().library("std.cfg").build(); - std::string fprefix() const + static std::string fprefix() { return "thread"; }