From cc8904fe665ecabe1d2c93ca3c8c1de06f0b62e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 18 Dec 2023 14:03:51 +0100 Subject: [PATCH 1/7] Fixed #12281 (IDE plugin integration is broken by checkers report) --- cli/cppcheckexecutor.cpp | 47 ++++++++++++++++++++++++---------------- lib/suppressions.cpp | 11 ++++++++-- test/cli/testutils.py | 2 -- 3 files changed, 37 insertions(+), 23 deletions(-) diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index 364beef5508..a9dd3b884ce 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -116,7 +116,7 @@ class CppCheckExecutor::StdLogger : public ErrorLogger /** * @brief Write the checkers report */ - void writeCheckersReport() const; + void writeCheckersReport(); bool hasCriticalErrors() const { return !mCriticalErrors.empty(); @@ -286,12 +286,13 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck) const cppcheck.tooManyConfigsError(emptyString,0U); } + if (false && settings.severity.isEnabled(Severity::information)) // <- TODO: settings.safety + mStdLogger->writeCheckersReport(); + if (settings.xml) { mStdLogger->reportErr(ErrorMessage::getXMLFooter()); } - mStdLogger->writeCheckersReport(); - if (settings.safety && mStdLogger->hasCriticalErrors()) return EXIT_FAILURE; @@ -300,28 +301,36 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck) const return EXIT_SUCCESS; } -void CppCheckExecutor::StdLogger::writeCheckersReport() const +void CppCheckExecutor::StdLogger::writeCheckersReport() { - CheckersReport checkersReport(mSettings, mActiveCheckers); + for (const Suppressions::Suppression& s : mSettings.nomsg.getSuppressions()) { + if (s.errorId == "checkersReport") + return; + } - if (!mSettings.quiet) { - const int activeCheckers = checkersReport.getActiveCheckersCount(); - const int totalCheckers = checkersReport.getAllCheckersCount(); + ErrorMessage msg; + msg.severity = Severity::information; + msg.id = "checkersReport"; - const std::string extra = mSettings.verbose ? " (use --checkers-report= to see details)" : ""; - if (mCriticalErrors.empty()) - std::cout << "Active checkers: " << activeCheckers << "/" << totalCheckers << extra << std::endl; - else - std::cout << "Active checkers: There was critical errors" << extra << std::endl; - } + CheckersReport checkersReport(mSettings, mActiveCheckers); - if (mSettings.checkersReportFilename.empty()) - return; + const int activeCheckers = checkersReport.getActiveCheckersCount(); + const int totalCheckers = checkersReport.getAllCheckersCount(); - std::ofstream fout(mSettings.checkersReportFilename); - if (fout.is_open()) - fout << checkersReport.getReport(mCriticalErrors); + std::string what; + if (mCriticalErrors.empty()) + what = std::to_string(activeCheckers) + "/" + std::to_string(totalCheckers); + else + what = "There was critical errors"; + msg.setmsg("Active checkers: " + what + " (use --checkers-report= to see details)"); + reportErr(msg); + + if (!mSettings.checkersReportFilename.empty()) { + std::ofstream fout(mSettings.checkersReportFilename); + if (fout.is_open()) + fout << checkersReport.getReport(mCriticalErrors); + } } #ifdef _WIN32 diff --git a/lib/suppressions.cpp b/lib/suppressions.cpp index e5f4e7b473a..d13ba5705fe 100644 --- a/lib/suppressions.cpp +++ b/lib/suppressions.cpp @@ -35,6 +35,9 @@ #include "xml.h" +static const char ID_UNUSEDFUNCTION[] = "unusedFunction"; +static const char ID_CHECKERSREPORT[] = "checkersReport"; + Suppressions::ErrorMessage Suppressions::ErrorMessage::fromErrorMessage(const ::ErrorMessage &msg, const std::set ¯oNames) { Suppressions::ErrorMessage ret; @@ -468,7 +471,9 @@ std::list Suppressions::getUnmatchedLocalSuppressions continue; if (s.hash > 0) continue; - if (!unusedFunctionChecking && s.errorId == "unusedFunction") + if (s.errorId == ID_CHECKERSREPORT) + continue; + if (!unusedFunctionChecking && s.errorId == ID_UNUSEDFUNCTION) continue; if (tmpFile.empty() || !s.isLocal() || s.fileName != tmpFile) continue; @@ -485,7 +490,9 @@ std::list Suppressions::getUnmatchedGlobalSuppression continue; if (s.hash > 0) continue; - if (!unusedFunctionChecking && s.errorId == "unusedFunction") + if (!unusedFunctionChecking && s.errorId == ID_UNUSEDFUNCTION) + continue; + if (s.errorId == ID_CHECKERSREPORT) continue; if (s.isLocal()) continue; diff --git a/test/cli/testutils.py b/test/cli/testutils.py index dd529e1b64f..699c04b669d 100644 --- a/test/cli/testutils.py +++ b/test/cli/testutils.py @@ -80,8 +80,6 @@ def cppcheck(args, env=None, remove_active_checkers=True): comm = p.communicate() stdout = comm[0].decode(encoding='utf-8', errors='ignore').replace('\r\n', '\n') stderr = comm[1].decode(encoding='utf-8', errors='ignore').replace('\r\n', '\n') - if remove_active_checkers and stdout.find('\nActive checkers:') > 0: - stdout = stdout[:1 + stdout.find('\nActive checkers:')] return p.returncode, stdout, stderr From 936e43f70a789e86165880b9105e296df2841736 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 18 Dec 2023 18:56:01 +0100 Subject: [PATCH 2/7] fix --- cli/cppcheckexecutor.cpp | 2 +- test/cli/testutils.py | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index a9dd3b884ce..933429a8ead 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -286,7 +286,7 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck) const cppcheck.tooManyConfigsError(emptyString,0U); } - if (false && settings.severity.isEnabled(Severity::information)) // <- TODO: settings.safety + if (settings.safety || settings.severity.isEnabled(Severity::information)) mStdLogger->writeCheckersReport(); if (settings.xml) { diff --git a/test/cli/testutils.py b/test/cli/testutils.py index 699c04b669d..4ed71d15c25 100644 --- a/test/cli/testutils.py +++ b/test/cli/testutils.py @@ -71,7 +71,7 @@ def __lookup_cppcheck_exe(): # Run Cppcheck with args -def cppcheck(args, env=None, remove_active_checkers=True): +def cppcheck(args, env=None, remove_checkers_report=True): exe = __lookup_cppcheck_exe() assert exe is not None, 'no cppcheck binary found' @@ -80,6 +80,13 @@ def cppcheck(args, env=None, remove_active_checkers=True): comm = p.communicate() stdout = comm[0].decode(encoding='utf-8', errors='ignore').replace('\r\n', '\n') stderr = comm[1].decode(encoding='utf-8', errors='ignore').replace('\r\n', '\n') + if remove_checkers_report and stderr.find('[checkersReport]\n') > 0: + start_id = stderr.find('[checkersReport]\n') + start_line = stderr.rfind('\n', 0, start_id) + if start_line <= 0: + stderr = '' + else: + stderr = stderr[:start_line + 1] return p.returncode, stdout, stderr From dab570c2d98680ec63b3485bca96dd4703f488fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 18 Dec 2023 20:18:40 +0100 Subject: [PATCH 3/7] --checkers-report --- cli/cppcheckexecutor.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index 933429a8ead..8a67fd4bca4 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -286,7 +286,7 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck) const cppcheck.tooManyConfigsError(emptyString,0U); } - if (settings.safety || settings.severity.isEnabled(Severity::information)) + if (settings.safety || settings.severity.isEnabled(Severity::information) || !settings.checkersReportFilename.empty()) mStdLogger->writeCheckersReport(); if (settings.xml) { @@ -303,11 +303,6 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck) const void CppCheckExecutor::StdLogger::writeCheckersReport() { - for (const Suppressions::Suppression& s : mSettings.nomsg.getSuppressions()) { - if (s.errorId == "checkersReport") - return; - } - ErrorMessage msg; msg.severity = Severity::information; msg.id = "checkersReport"; @@ -324,7 +319,14 @@ void CppCheckExecutor::StdLogger::writeCheckersReport() what = "There was critical errors"; msg.setmsg("Active checkers: " + what + " (use --checkers-report= to see details)"); - reportErr(msg); + bool suppressed = false; + for (const Suppressions::Suppression& s : mSettings.nomsg.getSuppressions()) { + if (s.errorId == "checkersReport") + suppressed = true; + } + + if (!suppressed) + reportErr(msg); if (!mSettings.checkersReportFilename.empty()) { std::ofstream fout(mSettings.checkersReportFilename); From 8ee1aa0d6404e5d70376dd98cdd9dacecbd97f4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 18 Dec 2023 20:24:03 +0100 Subject: [PATCH 4/7] refactor --- cli/cppcheckexecutor.cpp | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index 8a67fd4bca4..0340d7277e0 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -303,30 +303,31 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck) const void CppCheckExecutor::StdLogger::writeCheckersReport() { - ErrorMessage msg; - msg.severity = Severity::information; - msg.id = "checkersReport"; - CheckersReport checkersReport(mSettings, mActiveCheckers); - const int activeCheckers = checkersReport.getActiveCheckersCount(); - const int totalCheckers = checkersReport.getAllCheckersCount(); - - std::string what; - if (mCriticalErrors.empty()) - what = std::to_string(activeCheckers) + "/" + std::to_string(totalCheckers); - else - what = "There was critical errors"; - msg.setmsg("Active checkers: " + what + " (use --checkers-report= to see details)"); - bool suppressed = false; for (const Suppressions::Suppression& s : mSettings.nomsg.getSuppressions()) { if (s.errorId == "checkersReport") suppressed = true; } - if (!suppressed) + if (!suppressed) { + ErrorMessage msg; + msg.severity = Severity::information; + msg.id = "checkersReport"; + + const int activeCheckers = checkersReport.getActiveCheckersCount(); + const int totalCheckers = checkersReport.getAllCheckersCount(); + + std::string what; + if (mCriticalErrors.empty()) + what = std::to_string(activeCheckers) + "/" + std::to_string(totalCheckers); + else + what = "There was critical errors"; + msg.setmsg("Active checkers: " + what + " (use --checkers-report= to see details)"); + reportErr(msg); + } if (!mSettings.checkersReportFilename.empty()) { std::ofstream fout(mSettings.checkersReportFilename); From e9bb7ae8b9969d18e78cdaafe9c35f357c277ccf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 18 Dec 2023 21:20:52 +0100 Subject: [PATCH 5/7] test cppcheck1 format --- test/cli/testutils.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/test/cli/testutils.py b/test/cli/testutils.py index 4ed71d15c25..a27900a0e87 100644 --- a/test/cli/testutils.py +++ b/test/cli/testutils.py @@ -80,13 +80,20 @@ def cppcheck(args, env=None, remove_checkers_report=True): comm = p.communicate() stdout = comm[0].decode(encoding='utf-8', errors='ignore').replace('\r\n', '\n') stderr = comm[1].decode(encoding='utf-8', errors='ignore').replace('\r\n', '\n') - if remove_checkers_report and stderr.find('[checkersReport]\n') > 0: - start_id = stderr.find('[checkersReport]\n') - start_line = stderr.rfind('\n', 0, start_id) - if start_line <= 0: - stderr = '' - else: - stderr = stderr[:start_line + 1] + if remove_checkers_report: + if stderr.find('[checkersReport]\n') > 0: + start_id = stderr.find('[checkersReport]\n') + start_line = stderr.rfind('\n', 0, start_id) + if start_line <= 0: + stderr = '' + else: + stderr = stderr[:start_line + 1] + elif stderr.find(': (information) Active checkers: ') >= 0: + pos = stderr.find(': (information) Active checkers: ') + if pos == 0: + stderr = '' + elif stderr[pos - 1] == '\n': + stderr = stderr[:pos] return p.returncode, stdout, stderr From 7e54e49fad8aa5e0b21eb4cfed67e7a3e4877402 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 18 Dec 2023 22:54:56 +0100 Subject: [PATCH 6/7] fix ci --- test/cli/test-suppress-syntaxError.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/cli/test-suppress-syntaxError.py b/test/cli/test-suppress-syntaxError.py index b2293bc84da..3c77ba5b808 100644 --- a/test/cli/test-suppress-syntaxError.py +++ b/test/cli/test-suppress-syntaxError.py @@ -14,14 +14,14 @@ def test_j2_suppress(): assert len(stderr) == 0 def test_safety_suppress_syntax_error_implicitly(tmpdir): - ret, stdout, stderr = cppcheck(['--safety', '--suppress=*', 'proj-suppress-syntaxError'], remove_active_checkers=False) + ret, stdout, stderr = cppcheck(['--safety', '--suppress=*', 'proj-suppress-syntaxError'], remove_checkers_report=False) assert ret == 1 assert '[syntaxError]' in stderr - assert 'Active checkers: There was critical errors' in stdout + assert 'Active checkers: There was critical errors' in stderr def test_safety_suppress_syntax_error_explicitly(): - ret, stdout, stderr = cppcheck(['--safety', '--suppress=syntaxError', 'proj-suppress-syntaxError'], remove_active_checkers=False) + ret, stdout, stderr = cppcheck(['--safety', '--suppress=syntaxError', 'proj-suppress-syntaxError'], remove_checkers_report=False) assert ret == 1 assert '[syntaxError]' not in stderr - assert 'Active checkers: There was critical errors' in stdout + assert 'Active checkers: There was critical errors' in stderr From eef798ba1312ce1363a862fac70accee333ae4dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 18 Dec 2023 22:59:16 +0100 Subject: [PATCH 7/7] releasenotes [ci skip] --- releasenotes.txt | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/releasenotes.txt b/releasenotes.txt index 41295d3251b..593f61498c8 100644 --- a/releasenotes.txt +++ b/releasenotes.txt @@ -12,7 +12,7 @@ GUI: - Changed interface: -- +- Final report of active checkers is reported as a normal information message instead. Deprecations: - "--showtime=top5" has been deprecated and will be removed in Cppcheck 2.14. Please use --showtime=top5_file or --showtime=top5_summary instead. @@ -44,6 +44,3 @@ Other: - Markup files will now be processed after the regular source files when using multiple threads/processes (some issues remain - see Trac #12167 for details). - Added file name to ValueFlow "--debug" output. - Fixed build when using "clang-cl" in CMake. - -Safety critical fixes: -- Added "--safety" option. It makes Cppcheck more strict about critical errors.