From e35329aba334059737407063d8780d82d30232f6 Mon Sep 17 00:00:00 2001 From: PKEuS Date: Mon, 1 Sep 2014 10:13:03 +0200 Subject: [PATCH] Fixed reporting of unmatched suppressions for unusedFunction (#4946) --- cli/cppcheckexecutor.cpp | 2 +- lib/cppcheck.cpp | 2 +- lib/suppressions.cpp | 20 ++++++-------------- lib/suppressions.h | 4 ++-- test/testother.cpp | 2 +- test/testsuppressions.cpp | 24 ++++++++++++++++++------ 6 files changed, 29 insertions(+), 25 deletions(-) diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index 09c533ac8..be66725f2 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -770,7 +770,7 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck, int /*argc*/, const cha } if (settings.isEnabled("information") || settings.checkConfiguration) - reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions()); + reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions(settings._jobs == 1 && settings.isEnabled("unusedFunction"))); if (!settings.checkConfiguration) { cppcheck.tooManyConfigsError("",0U); diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 7d836b3dc..c4fd3d442 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -242,7 +242,7 @@ unsigned int CppCheck::processFile(const std::string& filename, const std::strin } if (_settings.isEnabled("information") || _settings.checkConfiguration) - reportUnmatchedSuppressions(_settings.nomsg.getUnmatchedLocalSuppressions(filename)); + reportUnmatchedSuppressions(_settings.nomsg.getUnmatchedLocalSuppressions(filename, _settings._jobs == 1 && _settings.isEnabled("unusedFunction"))); _errorList.clear(); return exitcode; diff --git a/lib/suppressions.cpp b/lib/suppressions.cpp index 951c9042c..fca35b342 100644 --- a/lib/suppressions.cpp +++ b/lib/suppressions.cpp @@ -265,11 +265,11 @@ bool Suppressions::isSuppressedLocal(const std::string &errorId, const std::stri return _suppressions[errorId].isSuppressedLocal(file, line); } -std::list Suppressions::getUnmatchedLocalSuppressions(const std::string &file) const +std::list Suppressions::getUnmatchedLocalSuppressions(const std::string &file, bool unusedFunctionChecking) const { std::list r; for (std::map::const_iterator i = _suppressions.begin(); i != _suppressions.end(); ++i) { - if (i->first == "unusedFunction") + if (!unusedFunctionChecking && i->first == "unusedFunction") continue; // unusedFunction is not a "local" suppression std::map >::const_iterator f = i->second._files.find(file); @@ -284,10 +284,13 @@ std::list Suppressions::getUnmatchedLocalSuppres return r; } -std::list Suppressions::getUnmatchedGlobalSuppressions() const +std::list Suppressions::getUnmatchedGlobalSuppressions(bool unusedFunctionChecking) const { std::list r; for (std::map::const_iterator i = _suppressions.begin(); i != _suppressions.end(); ++i) { + if (!unusedFunctionChecking && i->first == "unusedFunction") + continue; + // global suppressions.. for (std::map >::const_iterator g = i->second._globs.begin(); g != i->second._globs.end(); ++g) { for (std::map::const_iterator l = g->second.begin(); l != g->second.end(); ++l) { @@ -296,17 +299,6 @@ std::list Suppressions::getUnmatchedGlobalSuppre } } } - - // unusedFunction.. - if (i->first == "unusedFunction") { - for (std::map >::const_iterator f = i->second._files.begin(); f != i->second._files.end(); ++f) { - for (std::map::const_iterator l = f->second.begin(); l != f->second.end(); ++l) { - if (!l->second) { - r.push_back(SuppressionEntry(i->first, f->first, l->first)); - } - } - } - } } return r; } diff --git a/lib/suppressions.h b/lib/suppressions.h index 1d15f2ee3..b356b6a06 100644 --- a/lib/suppressions.h +++ b/lib/suppressions.h @@ -133,13 +133,13 @@ public: * @brief Returns list of unmatched local (per-file) suppressions. * @return list of unmatched suppressions */ - std::list getUnmatchedLocalSuppressions(const std::string &file) const; + std::list getUnmatchedLocalSuppressions(const std::string &file, bool unusedFunctionChecking) const; /** * @brief Returns list of unmatched global (glob pattern) suppressions. * @return list of unmatched suppressions */ - std::list getUnmatchedGlobalSuppressions() const; + std::list getUnmatchedGlobalSuppressions(bool unusedFunctionChecking) const; }; /// @} diff --git a/test/testother.cpp b/test/testother.cpp index 989b16cab..42874ae89 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -244,7 +244,7 @@ private: CheckOther checkOther(&tokenizer, &settings, &logger); checkOther.checkSwitchCaseFallThrough(); - logger.reportUnmatchedSuppressions(settings.nomsg.getUnmatchedLocalSuppressions(filename)); + logger.reportUnmatchedSuppressions(settings.nomsg.getUnmatchedLocalSuppressions(filename, false)); } diff --git a/test/testsuppressions.cpp b/test/testsuppressions.cpp index 935370109..a25f4de00 100644 --- a/test/testsuppressions.cpp +++ b/test/testsuppressions.cpp @@ -44,6 +44,7 @@ private: TEST_CASE(suppressionsPathSeparator); TEST_CASE(inlinesuppress_unusedFunction); // #4210 - unusedFunction + TEST_CASE(globalsuppress_unusedFunction); // #4946 TEST_CASE(suppressionWithRelativePaths); // #4733 } @@ -130,7 +131,7 @@ private: cppCheck.check("test.cpp", code); - reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions()); + reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions(true)); } void checkSuppressionThreads(const char code[], const std::string &suppression = emptyString) { @@ -153,7 +154,7 @@ private: executor.check(); - reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions()); + reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions(false)); } // Check the suppression for multiple files @@ -171,7 +172,7 @@ private: for (int i = 0; names[i] != NULL; ++i) cppCheck.check(names[i], codes[i]); - reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions()); + reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions(true)); } void runChecks(void (TestSuppressions::*check)(const char[], const std::string &)) { @@ -323,11 +324,22 @@ private: ASSERT_EQUALS(true, suppressions.isSuppressed("someid", "test/foo/bar.cpp", 142)); } - void inlinesuppress_unusedFunction() const { // #4210 - wrong report of "unmatchedSuppression" for "unusedFunction" + void inlinesuppress_unusedFunction() const { // #4210, #4946 - wrong report of "unmatchedSuppression" for "unusedFunction" Suppressions suppressions; suppressions.addSuppression("unusedFunction", "test.c", 3U); - ASSERT_EQUALS(true, suppressions.getUnmatchedLocalSuppressions("test.c").empty()); - ASSERT_EQUALS(false, suppressions.getUnmatchedGlobalSuppressions().empty()); + ASSERT_EQUALS(true, !suppressions.getUnmatchedLocalSuppressions("test.c", true).empty()); + ASSERT_EQUALS(false, !suppressions.getUnmatchedGlobalSuppressions(true).empty()); + ASSERT_EQUALS(false, !suppressions.getUnmatchedLocalSuppressions("test.c", false).empty()); + ASSERT_EQUALS(false, !suppressions.getUnmatchedGlobalSuppressions(false).empty()); + } + + void globalsuppress_unusedFunction() const { // #4946 - wrong report of "unmatchedSuppression" for "unusedFunction" + Suppressions suppressions; + suppressions.addSuppressionLine("unusedFunction:*"); + ASSERT_EQUALS(false, !suppressions.getUnmatchedLocalSuppressions("test.c", true).empty()); + ASSERT_EQUALS(true, !suppressions.getUnmatchedGlobalSuppressions(true).empty()); + ASSERT_EQUALS(false, !suppressions.getUnmatchedLocalSuppressions("test.c", false).empty()); + ASSERT_EQUALS(false, !suppressions.getUnmatchedGlobalSuppressions(false).empty()); } void suppressionWithRelativePaths() {