adjustments for unmatched unusedFunction inline suppressions

danmar · Apr 9, 2024 · edf3e71 · edf3e71
1 parent c8b4667
commit edf3e71
Show file tree

Hide file tree

Showing 8 changed files with 125 additions and 16 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(SuppressionList::UnusedFunction::Only), 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,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
@@ -487,7 +487,7 @@ void SuppressionList::dump(std::ostream & out) const
     out << "  </suppressions>" << std::endl;
 }
 
-std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppressions(const std::string &file, const bool unusedFunctionChecking) const
+std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppressions(const std::string &file, const bool includeUnusedFunction) const
 {
     std::string tmpFile = Path::simplifyPath(file);
     std::list<Suppression> result;
@@ -502,7 +502,7 @@ std::list<SuppressionList::Suppression> 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::Suppression> SuppressionList::getUnmatchedLocalSuppre
     return result;
 }
 
-std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppressions(const bool unusedFunctionChecking) const
+std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppressions(const bool includeUnusedFunction) const
 {
     std::list<Suppression> result;
     for (const Suppression &s : mSuppressions) {
@@ -521,7 +521,7 @@ std::list<SuppressionList::Suppression> 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::Suppression> SuppressionList::getUnmatchedGlobalSuppr
     return result;
 }
 
-std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedInlineSuppressions(const bool unusedFunctionChecking) const
+std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedInlineSuppressions(const UnusedFunction unusedFunction) const
 {
     std::list<SuppressionList::Suppression> result;
     for (const SuppressionList::Suppression &s : SuppressionList::mSuppressions) {
@@ -544,7 +544,9 @@ std::list<SuppressionList::Suppression> 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
@@ -237,19 +237,24 @@ class CPPCHECKLIB SuppressionList {
      * @brief Returns list of unmatched local (per-file) suppressions.
      * @return list of unmatched suppressions
      */
-    std::list<Suppression> getUnmatchedLocalSuppressions(const std::string &file, const bool unusedFunctionChecking) const;
+    std::list<Suppression> 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<Suppression> getUnmatchedGlobalSuppressions(const bool unusedFunctionChecking) const;
+    std::list<Suppression> getUnmatchedGlobalSuppressions(const bool includeUnusedFunction) const;
+
+    enum UnusedFunction {
+        Exclude,
+        Only
+    };
 
     /**
      * @brief Returns list of unmatched inline suppressions.
      * @return list of unmatched suppressions
      */
-    std::list<Suppression> getUnmatchedInlineSuppressions(const bool unusedFunctionChecking) const;
+    std::list<Suppression> 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
@@ -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
+    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'])
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
+}