adjustments for unmatched unusedFunction inline suppressions

danmar · Apr 7, 2024 · d83b6a6 · d83b6a6
1 parent a70b32f
commit d83b6a6
Show file tree

Hide file tree

Showing 6 changed files with 108 additions and 7 deletions.
diff --git 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(unusedFunctionCheckEnabled), errorLogger);
+    }
+
     err |= SuppressionList::reportUnmatchedSuppressions(suppressions.getUnmatchedGlobalSuppressions(unusedFunctionCheckEnabled), errorLogger);
     return err;
 }

diff --git 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
@@ -81,8 +81,8 @@ class SyncLogForwarder : public ErrorLogger
 class ThreadData
 {
 public:
-    ThreadData(ThreadExecutor &threadExecutor, ErrorLogger &errorLogger, const Settings &settings, const std::list<std::pair<std::string, std::size_t>> &files, const std::list<FileSettings> &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<std::pair<std::string, std::size_t>> &files, const std::list<FileSettings> &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<std::future<unsigned int>> 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
@@ -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,13 +1050,14 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string
 
     if (mSettings.severity.isEnabled(Severity::information) || mSettings.checkConfiguration) {
         // TODO: check result?
+        // defer reporting of unusedFunction to later
         SuppressionList::reportUnmatchedSuppressions(mSettings.supprs.nomsg.getUnmatchedInlineSuppressions(false), *this);
 
         // In jointSuppressionReport mode, unmatched suppressions are
         // collected after all files are processed
         if (!mSettings.useSingleJob()) {
             // TODO: check result?
-            SuppressionList::reportUnmatchedSuppressions(mSettings.supprs.nomsg.getUnmatchedLocalSuppressions(filename, (bool)mUnusedFunctionsCheck)), *this);
+            SuppressionList::reportUnmatchedSuppressions(mSettings.supprs.nomsg.getUnmatchedLocalSuppressions(filename, (bool)mUnusedFunctionsCheck), *this);
         }
     }
 

diff --git 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,73 @@ def test_duplicate_file(tmpdir):
     lines = stderr.splitlines()
     assert lines == []
     assert stdout == ''
-    assert ret == 0, stdout
+    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'])
+
+
+def test_unused_function_unmatched_build_dir_j_process(tmpdir):
+    __test_unused_function_unmatched_build_dir(tmpdir, ['-j2', '--executor=process'])
diff --git a/test/cli/proj-inline-suppress/unusedFunctionUnmatched.cpp b/test/cli/proj-inline-suppress/unusedFunctionUnmatched.cpp
@@ -0,0 +1,5 @@
+// cppcheck-suppress unusedFunction
+void f() {
+    // cppcheck-suppress unusedFunction
+    // cppcheck-suppress uninitvar
+}