From edf3e71e6491824c1e344f37c53bfc5087f65412 Mon Sep 17 00:00:00 2001 From: firewave Date: Fri, 29 Mar 2024 14:06:16 +0100 Subject: [PATCH] adjustments for unmatched `unusedFunction` inline suppressions --- cli/cppcheckexecutor.cpp | 6 ++ cli/processexecutor.cpp | 3 + cli/threadexecutor.cpp | 20 ++++- lib/cppcheck.cpp | 9 ++- lib/suppressions.cpp | 14 ++-- lib/suppressions.h | 11 ++- test/cli/inline-suppress_test.py | 73 ++++++++++++++++++- .../unusedFunctionUnmatched.cpp | 5 ++ 8 files changed, 125 insertions(+), 16 deletions(-) create mode 100644 test/cli/proj-inline-suppress/unusedFunctionUnmatched.cpp diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index 82673af9c14d..8b3e3790000c 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -240,6 +240,12 @@ bool CppCheckExecutor::reportSuppressions(const Settings &settings, const Suppre suppressions.getUnmatchedLocalSuppressions(i->filename, unusedFunctionCheckEnabled), errorLogger); } } + if (settings.inlineSuppressions) { + // report unmatched unusedFunction suppressions + err |= SuppressionList::reportUnmatchedSuppressions( + suppressions.getUnmatchedInlineSuppressions(SuppressionList::UnusedFunction::Only), errorLogger); + } + err |= SuppressionList::reportUnmatchedSuppressions(suppressions.getUnmatchedGlobalSuppressions(unusedFunctionCheckEnabled), errorLogger); return err; } diff --git a/cli/processexecutor.cpp b/cli/processexecutor.cpp index de847015ded1..a29252afe1ac 100644 --- a/cli/processexecutor.cpp +++ b/cli/processexecutor.cpp @@ -289,6 +289,9 @@ unsigned int ProcessExecutor::check() // TODO: call analyseClangTidy()? } + // TODO: need to transfer inline unusedFunction suppressions + // TODO: need to update suppressions states + pipewriter.writeEnd(std::to_string(resultOfCheck)); std::exit(EXIT_SUCCESS); } diff --git a/cli/threadexecutor.cpp b/cli/threadexecutor.cpp index 9f44d5b1553b..2b536767e4b8 100644 --- a/cli/threadexecutor.cpp +++ b/cli/threadexecutor.cpp @@ -81,8 +81,8 @@ class SyncLogForwarder : public ErrorLogger class ThreadData { public: - ThreadData(ThreadExecutor &threadExecutor, ErrorLogger &errorLogger, const Settings &settings, const std::list> &files, const std::list &fileSettings, CppCheck::ExecuteCmdFn executeCommand) - : mFiles(files), mFileSettings(fileSettings), mSettings(settings), mExecuteCommand(std::move(executeCommand)), logForwarder(threadExecutor, errorLogger) + ThreadData(ThreadExecutor &threadExecutor, ErrorLogger &errorLogger, const Settings &settings, SuppressionList& supprs, const std::list> &files, const std::list &fileSettings, CppCheck::ExecuteCmdFn executeCommand) + : mFiles(files), mFileSettings(fileSettings), mSettings(settings), mSuppressions(supprs), mExecuteCommand(std::move(executeCommand)), logForwarder(threadExecutor, errorLogger) { mItNextFile = mFiles.begin(); mItNextFileSettings = mFileSettings.begin(); @@ -128,6 +128,19 @@ class ThreadData result = fileChecker.check(*file); // TODO: call analyseClangTidy()? } + for (const auto& suppr : fileChecker.settings().supprs.nomsg.getSuppressions()) { + // need to transfer unusedFunction suppressions because these are handled later on + if (suppr.isInline && suppr.errorId == "unusedFunction") { + mSuppressions.addSuppression(suppr); // TODO: check result + continue; + } + + // propagate state of global suppressions + if (!suppr.isLocal()) { + mSuppressions.updateSuppressionState(suppr); // TODO: check result + continue; + } + } return result; } @@ -152,6 +165,7 @@ class ThreadData std::mutex mFileSync; const Settings &mSettings; + SuppressionList &mSuppressions; CppCheck::ExecuteCmdFn mExecuteCommand; public: @@ -180,7 +194,7 @@ unsigned int ThreadExecutor::check() std::vector> threadFutures; threadFutures.reserve(mSettings.jobs); - ThreadData data(*this, mErrorLogger, mSettings, mFiles, mFileSettings, mExecuteCommand); + ThreadData data(*this, mErrorLogger, mSettings, mSuppressions, mFiles, mFileSettings, mExecuteCommand); for (unsigned int i = 0; i < mSettings.jobs; ++i) { try { diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 5c3081743107..8783e987d68c 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -603,8 +603,12 @@ unsigned int CppCheck::check(const FileSettings &fs) for (const auto& suppr : temp.mSettings.supprs.nomsg.getSuppressions()) { // skip inline suppressions - are handled in checkFile() - if (suppr.isInline) + if (suppr.isInline) { + // need to transfer unusedFunction suppressions because these are handled later on + if (suppr.errorId == "unusedFunction") + mSettings.supprs.nomsg.addSuppression(suppr); // TODO: check result continue; + } const bool res = mSettings.supprs.nomsg.updateSuppressionState(suppr); if (!res) @@ -1046,7 +1050,8 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string if (mSettings.severity.isEnabled(Severity::information) || mSettings.checkConfiguration) { // TODO: check result? - SuppressionList::reportUnmatchedSuppressions(mSettings.supprs.nomsg.getUnmatchedInlineSuppressions(false), *this); + // defer reporting of unusedFunction to later + SuppressionList::reportUnmatchedSuppressions(mSettings.supprs.nomsg.getUnmatchedInlineSuppressions(SuppressionList::UnusedFunction::Exclude), *this); // In jointSuppressionReport mode, unmatched suppressions are // collected after all files are processed diff --git a/lib/suppressions.cpp b/lib/suppressions.cpp index 551a59318798..e1ce24374eae 100644 --- a/lib/suppressions.cpp +++ b/lib/suppressions.cpp @@ -487,7 +487,7 @@ void SuppressionList::dump(std::ostream & out) const out << " " << std::endl; } -std::list SuppressionList::getUnmatchedLocalSuppressions(const std::string &file, const bool unusedFunctionChecking) const +std::list SuppressionList::getUnmatchedLocalSuppressions(const std::string &file, const bool includeUnusedFunction) const { std::string tmpFile = Path::simplifyPath(file); std::list result; @@ -502,7 +502,7 @@ std::list SuppressionList::getUnmatchedLocalSuppre continue; if (s.errorId == ID_CHECKERSREPORT) continue; - if (!unusedFunctionChecking && s.errorId == ID_UNUSEDFUNCTION) + if (!includeUnusedFunction && s.errorId == ID_UNUSEDFUNCTION) continue; if (tmpFile.empty() || !s.isLocal() || s.fileName != tmpFile) continue; @@ -511,7 +511,7 @@ std::list SuppressionList::getUnmatchedLocalSuppre return result; } -std::list SuppressionList::getUnmatchedGlobalSuppressions(const bool unusedFunctionChecking) const +std::list SuppressionList::getUnmatchedGlobalSuppressions(const bool includeUnusedFunction) const { std::list result; for (const Suppression &s : mSuppressions) { @@ -521,7 +521,7 @@ std::list SuppressionList::getUnmatchedGlobalSuppr continue; if (s.hash > 0) continue; - if (!unusedFunctionChecking && s.errorId == ID_UNUSEDFUNCTION) + if (!includeUnusedFunction && s.errorId == ID_UNUSEDFUNCTION) continue; if (s.errorId == ID_CHECKERSREPORT) continue; @@ -532,7 +532,7 @@ std::list SuppressionList::getUnmatchedGlobalSuppr return result; } -std::list SuppressionList::getUnmatchedInlineSuppressions(const bool unusedFunctionChecking) const +std::list SuppressionList::getUnmatchedInlineSuppressions(const UnusedFunction unusedFunction) const { std::list result; for (const SuppressionList::Suppression &s : SuppressionList::mSuppressions) { @@ -544,7 +544,9 @@ std::list SuppressionList::getUnmatchedInlineSuppr continue; if (s.hash > 0) continue; - if (!unusedFunctionChecking && s.errorId == ID_UNUSEDFUNCTION) + if (unusedFunction == UnusedFunction::Only && s.errorId != ID_UNUSEDFUNCTION) + continue; + if (unusedFunction == UnusedFunction::Exclude && s.errorId == ID_UNUSEDFUNCTION) continue; result.push_back(s); } diff --git a/lib/suppressions.h b/lib/suppressions.h index 0e227e6002a5..41dce1defafc 100644 --- a/lib/suppressions.h +++ b/lib/suppressions.h @@ -237,19 +237,24 @@ class CPPCHECKLIB SuppressionList { * @brief Returns list of unmatched local (per-file) suppressions. * @return list of unmatched suppressions */ - std::list getUnmatchedLocalSuppressions(const std::string &file, const bool unusedFunctionChecking) const; + std::list getUnmatchedLocalSuppressions(const std::string &file, const bool includeUnusedFunction) const; /** * @brief Returns list of unmatched global (glob pattern) suppressions. * @return list of unmatched suppressions */ - std::list getUnmatchedGlobalSuppressions(const bool unusedFunctionChecking) const; + std::list getUnmatchedGlobalSuppressions(const bool includeUnusedFunction) const; + + enum UnusedFunction { + Exclude, + Only + }; /** * @brief Returns list of unmatched inline suppressions. * @return list of unmatched suppressions */ - std::list getUnmatchedInlineSuppressions(const bool unusedFunctionChecking) const; + std::list getUnmatchedInlineSuppressions(const UnusedFunction unusedFunction) const; /** * @brief Returns list of all suppressions. diff --git a/test/cli/inline-suppress_test.py b/test/cli/inline-suppress_test.py index ac9e451cd7c8..b3d92a25c8d8 100644 --- a/test/cli/inline-suppress_test.py +++ b/test/cli/inline-suppress_test.py @@ -172,7 +172,6 @@ def __test_compile_commands_unused_function_suppression(tmpdir, use_j): assert ret == 0, stdout -@pytest.mark.xfail(strict=True) # TODO: need to propagate back inline suppressions def test_compile_commands_unused_function_suppression(tmpdir): __test_compile_commands_unused_function_suppression(tmpdir, False) @@ -342,4 +341,74 @@ def test_duplicate_file(tmpdir): lines = stderr.splitlines() assert lines == [] assert stdout == '' - assert ret == 0, stdout \ No newline at end of file + assert ret == 0, stdout + + +# reporting of inline unusedFunction is deferred +def __test_unused_function_unmatched(tmpdir, use_j): + args = [ + '-q', + '--template=simple', + '--enable=all', + '--inline-suppr', + 'proj-inline-suppress/unusedFunctionUnmatched.cpp' + ] + + if use_j: + args.append('-j2') + else: + args.append('-j1') + + ret, stdout, stderr = cppcheck(args, cwd=__script_dir) + lines = stderr.splitlines() + assert lines == [ + '{}unusedFunctionUnmatched.cpp:5:0: information: Unmatched suppression: uninitvar [unmatchedSuppression]'.format(__proj_inline_suppres_path), + '{}unusedFunctionUnmatched.cpp:5:0: information: Unmatched suppression: unusedFunction [unmatchedSuppression]'.format(__proj_inline_suppres_path) + ] + assert stdout == '' + assert ret == 0, stdout + + +def test_unused_function_unmatched(tmpdir): + __test_unused_function_unmatched(tmpdir, False) + + +@pytest.mark.skip # unusedFunction does not work with -j +def test_unused_function_unmatched_j(tmpdir): + __test_unused_function_unmatched(tmpdir, True) + + +# reporting of inline unusedFunction is deferred +def __test_unused_function_unmatched_build_dir(tmpdir, extra_args): + args = [ + '-q', + '--template=simple', + '--cppcheck-build-dir={}'.format(tmpdir), + '--enable=all', + '--inline-suppr', + 'proj-inline-suppress/unusedFunctionUnmatched.cpp' + ] + + args = args + extra_args + + ret, stdout, stderr = cppcheck(args, cwd=__script_dir) + lines = stderr.splitlines() + assert lines == [ + '{}unusedFunctionUnmatched.cpp:5:0: information: Unmatched suppression: uninitvar [unmatchedSuppression]'.format(__proj_inline_suppres_path), + '{}unusedFunctionUnmatched.cpp:5:0: information: Unmatched suppression: unusedFunction [unmatchedSuppression]'.format(__proj_inline_suppres_path) + ] + assert stdout == '' + assert ret == 0, stdout + + +def test_unused_function_unmatched_build_dir(tmpdir): + __test_unused_function_unmatched_build_dir(tmpdir, ['-j1']) + + +def test_unused_function_unmatched_build_dir_j_thread(tmpdir): + __test_unused_function_unmatched_build_dir(tmpdir, ['-j2', '--executor=thread']) + + +@pytest.mark.xfail(strict=True) +def test_unused_function_unmatched_build_dir_j_process(tmpdir): + __test_unused_function_unmatched_build_dir(tmpdir, ['-j2', '--executor=process']) \ No newline at end of file diff --git a/test/cli/proj-inline-suppress/unusedFunctionUnmatched.cpp b/test/cli/proj-inline-suppress/unusedFunctionUnmatched.cpp new file mode 100644 index 000000000000..8bd895e99bb1 --- /dev/null +++ b/test/cli/proj-inline-suppress/unusedFunctionUnmatched.cpp @@ -0,0 +1,5 @@ +// cppcheck-suppress unusedFunction +void f() { + // cppcheck-suppress unusedFunction + // cppcheck-suppress uninitvar +}