diff --git a/Makefile b/Makefile index 5d7931f9a..aec9288fe 100644 --- a/Makefile +++ b/Makefile @@ -455,7 +455,7 @@ validateRules: ###### Build -$(libcppdir)/analyzerinfo.o: lib/analyzerinfo.cpp externals/tinyxml2/tinyxml2.h lib/analyzerinfo.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/path.h lib/platform.h lib/suppressions.h lib/utils.h +$(libcppdir)/analyzerinfo.o: lib/analyzerinfo.cpp externals/tinyxml2/tinyxml2.h lib/analyzerinfo.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/path.h lib/platform.h lib/utils.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/analyzerinfo.cpp $(libcppdir)/astutils.o: lib/astutils.cpp lib/astutils.h lib/check.h lib/checkclass.h lib/config.h lib/errortypes.h lib/importproject.h lib/infer.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/valueptr.h lib/vfvalue.h @@ -641,10 +641,10 @@ cli/cmdlineparser.o: cli/cmdlineparser.cpp cli/cmdlineparser.h cli/cppcheckexecu cli/cppcheckexecutor.o: cli/cppcheckexecutor.cpp cli/cmdlineparser.h cli/cppcheckexecutor.h cli/cppcheckexecutorseh.h cli/cppcheckexecutorsig.h cli/executor.h cli/filelister.h cli/processexecutor.h cli/singleexecutor.h cli/threadexecutor.h lib/analyzerinfo.h lib/check.h lib/checkunusedfunctions.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/pathmatch.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h $(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cppcheckexecutor.cpp -cli/cppcheckexecutorseh.o: cli/cppcheckexecutorseh.cpp cli/cppcheckexecutor.h cli/cppcheckexecutorseh.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/suppressions.h lib/utils.h +cli/cppcheckexecutorseh.o: cli/cppcheckexecutorseh.cpp cli/cppcheckexecutor.h cli/cppcheckexecutorseh.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/utils.h $(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cppcheckexecutorseh.cpp -cli/cppcheckexecutorsig.o: cli/cppcheckexecutorsig.cpp cli/cppcheckexecutor.h cli/cppcheckexecutorsig.h cli/stacktrace.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/suppressions.h +cli/cppcheckexecutorsig.o: cli/cppcheckexecutorsig.cpp cli/cppcheckexecutor.h cli/cppcheckexecutorsig.h cli/stacktrace.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h $(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cppcheckexecutorsig.cpp cli/executor.o: cli/executor.cpp cli/executor.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h @@ -653,7 +653,7 @@ cli/executor.o: cli/executor.cpp cli/executor.h lib/color.h lib/config.h lib/err cli/filelister.o: cli/filelister.cpp cli/filelister.h lib/config.h lib/path.h lib/pathmatch.h lib/utils.h $(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/filelister.cpp -cli/main.o: cli/main.cpp cli/cppcheckexecutor.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/suppressions.h +cli/main.o: cli/main.cpp cli/cppcheckexecutor.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h $(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/main.cpp cli/processexecutor.o: cli/processexecutor.cpp cli/cppcheckexecutor.h cli/executor.h cli/processexecutor.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index eabf05820..540619939 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -261,11 +261,11 @@ bool CppCheckExecutor::reportSuppressions(const Settings &settings, bool unusedF bool err = false; if (settings.useSingleJob()) { for (std::map::const_iterator i = files.cbegin(); i != files.cend(); ++i) { - err |= errorLogger.reportUnmatchedSuppressions( - settings.nomsg.getUnmatchedLocalSuppressions(i->first, unusedFunctionCheckEnabled)); + err |= Suppressions::reportUnmatchedSuppressions( + settings.nomsg.getUnmatchedLocalSuppressions(i->first, unusedFunctionCheckEnabled), errorLogger); } } - err |= errorLogger.reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions(unusedFunctionCheckEnabled)); + err |= Suppressions::reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions(unusedFunctionCheckEnabled), errorLogger); return err; } diff --git a/cli/cppcheckexecutor.h b/cli/cppcheckexecutor.h index 23e845e0e..e5577396d 100644 --- a/cli/cppcheckexecutor.h +++ b/cli/cppcheckexecutor.h @@ -43,6 +43,8 @@ class Settings; */ class CppCheckExecutor : public ErrorLogger { public: + friend class TestSuppressions; + /** * Constructor */ @@ -101,8 +103,6 @@ public: */ static bool executeCommand(std::string exe, std::vector args, std::string redirect, std::string &output_); - static bool reportSuppressions(const Settings &settings, bool unusedFunctionCheckEnabled, const std::map &files, ErrorLogger& errorLogger); - protected: /** @@ -124,6 +124,8 @@ protected: private: + static bool reportSuppressions(const Settings &settings, bool unusedFunctionCheckEnabled, const std::map &files, ErrorLogger& errorLogger); + /** * Wrapper around check_internal * - installs optional platform dependent signal handling diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index f336630bc..11b4db07c 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -1021,7 +1021,7 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string // In jointSuppressionReport mode, unmatched suppressions are // collected after all files are processed if (!mSettings.useSingleJob() && (mSettings.severity.isEnabled(Severity::information) || mSettings.checkConfiguration)) { - reportUnmatchedSuppressions(mSettings.nomsg.getUnmatchedLocalSuppressions(filename, isUnusedFunctionCheckEnabled())); + Suppressions::reportUnmatchedSuppressions(mSettings.nomsg.getUnmatchedLocalSuppressions(filename, isUnusedFunctionCheckEnabled()), *this); } mErrorList.clear(); @@ -1597,7 +1597,7 @@ void CppCheck::reportErr(const ErrorMessage &msg) mAnalyzerInformation.reportErr(msg); // TODO: only convert if necessary - const Suppressions::ErrorMessage errorMessage = msg.toSuppressionsErrorMessage(); + const auto errorMessage = Suppressions::ErrorMessage::fromErrorMessage(msg); if (mSettings.nomsg.isSuppressed(errorMessage, mUseGlobalSuppressions)) { return; diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp index f6454e1d5..16010663e 100644 --- a/lib/errorlogger.cpp +++ b/lib/errorlogger.cpp @@ -211,22 +211,6 @@ void ErrorMessage::setmsg(const std::string &msg) } } -Suppressions::ErrorMessage ErrorMessage::toSuppressionsErrorMessage() const -{ - Suppressions::ErrorMessage ret; - ret.hash = hash; - ret.errorId = id; - if (!callStack.empty()) { - ret.setFileName(callStack.back().getfile(false)); - ret.lineNumber = callStack.back().line; - } else { - ret.lineNumber = Suppressions::Suppression::NO_LINE; - } - ret.certainty = certainty; - ret.symbolNames = mSymbolNames; - return ret; -} - static void serializeString(std::string &oss, const std::string & str) { oss += std::to_string(str.length()); @@ -681,39 +665,6 @@ std::string ErrorMessage::toString(bool verbose, const std::string &templateForm return result; } -bool ErrorLogger::reportUnmatchedSuppressions(const std::list &unmatched) -{ - bool err = false; - // Report unmatched suppressions - for (const Suppressions::Suppression &s : unmatched) { - // don't report "unmatchedSuppression" as unmatched - if (s.errorId == "unmatchedSuppression") - continue; - - // check if this unmatched suppression is suppressed - bool suppressed = false; - for (const Suppressions::Suppression &s2 : unmatched) { - if (s2.errorId == "unmatchedSuppression") { - if ((s2.fileName.empty() || s2.fileName == "*" || s2.fileName == s.fileName) && - (s2.lineNumber == Suppressions::Suppression::NO_LINE || s2.lineNumber == s.lineNumber)) { - suppressed = true; - break; - } - } - } - - if (suppressed) - continue; - - std::list callStack; - if (!s.fileName.empty()) - callStack.emplace_back(s.fileName, s.lineNumber, 0); - reportErr(ErrorMessage(callStack, emptyString, Severity::information, "Unmatched suppression: " + s.errorId, "unmatchedSuppression", Certainty::normal)); - err = true; - } - return err; -} - std::string ErrorLogger::callStackToString(const std::list &callStack) { std::string str; diff --git a/lib/errorlogger.h b/lib/errorlogger.h index 4cb1352c2..d9983df1b 100644 --- a/lib/errorlogger.h +++ b/lib/errorlogger.h @@ -23,7 +23,6 @@ #include "config.h" #include "errortypes.h" -#include "suppressions.h" #include "color.h" #include @@ -198,8 +197,6 @@ public: return mSymbolNames; } - Suppressions::ErrorMessage toSuppressionsErrorMessage() const; - private: static std::string fixInvalidChars(const std::string& raw); @@ -250,13 +247,6 @@ public: (void)value; } - /** - * Report unmatched suppressions - * @param unmatched list of unmatched suppressions (from Settings::Suppressions::getUnmatched(Local|Global)Suppressions) - * @return true is returned if errors are reported - */ - bool reportUnmatchedSuppressions(const std::list &unmatched); - static std::string callStackToString(const std::list &callStack); /** diff --git a/lib/suppressions.cpp b/lib/suppressions.cpp index ca5676e04..eac6ae6cf 100644 --- a/lib/suppressions.cpp +++ b/lib/suppressions.cpp @@ -35,6 +35,22 @@ #include +Suppressions::ErrorMessage Suppressions::ErrorMessage::fromErrorMessage(const ::ErrorMessage &msg) +{ + Suppressions::ErrorMessage ret; + ret.hash = msg.hash; + ret.errorId = msg.id; + if (!msg.callStack.empty()) { + ret.setFileName(msg.callStack.back().getfile(false)); + ret.lineNumber = msg.callStack.back().line; + } else { + ret.lineNumber = Suppressions::Suppression::NO_LINE; + } + ret.certainty = msg.certainty; + ret.symbolNames = msg.symbolNames(); + return ret; +} + static bool isAcceptedErrorIdChar(char c) { switch (c) { @@ -384,7 +400,7 @@ bool Suppressions::isSuppressed(const ::ErrorMessage &errmsg) { if (mSuppressions.empty()) return false; - return isSuppressed(errmsg.toSuppressionsErrorMessage()); + return isSuppressed(Suppressions::ErrorMessage::fromErrorMessage(errmsg)); } void Suppressions::dump(std::ostream & out) const @@ -461,3 +477,36 @@ void Suppressions::markUnmatchedInlineSuppressionsAsChecked(const Tokenizer &tok } } } + +bool Suppressions::reportUnmatchedSuppressions(const std::list &unmatched, ErrorLogger &errorLogger) +{ + bool err = false; + // Report unmatched suppressions + for (const Suppressions::Suppression &s : unmatched) { + // don't report "unmatchedSuppression" as unmatched + if (s.errorId == "unmatchedSuppression") + continue; + + // check if this unmatched suppression is suppressed + bool suppressed = false; + for (const Suppressions::Suppression &s2 : unmatched) { + if (s2.errorId == "unmatchedSuppression") { + if ((s2.fileName.empty() || s2.fileName == "*" || s2.fileName == s.fileName) && + (s2.lineNumber == Suppressions::Suppression::NO_LINE || s2.lineNumber == s.lineNumber)) { + suppressed = true; + break; + } + } + } + + if (suppressed) + continue; + + std::list<::ErrorMessage::FileLocation> callStack; + if (!s.fileName.empty()) + callStack.emplace_back(s.fileName, s.lineNumber, 0); + errorLogger.reportErr(::ErrorMessage(callStack, emptyString, Severity::information, "Unmatched suppression: " + s.errorId, "unmatchedSuppression", Certainty::normal)); + err = true; + } + return err; +} diff --git a/lib/suppressions.h b/lib/suppressions.h index cc64ee51c..6e1fd9c9e 100644 --- a/lib/suppressions.h +++ b/lib/suppressions.h @@ -34,6 +34,7 @@ class Tokenizer; class ErrorMessage; +class ErrorLogger; enum class Certainty; /** @brief class for handling suppressions */ @@ -50,6 +51,8 @@ public: int lineNumber; Certainty certainty; std::string symbolNames; + + static Suppressions::ErrorMessage fromErrorMessage(const ::ErrorMessage &msg); private: std::string mFileName; }; @@ -201,6 +204,13 @@ public: */ void markUnmatchedInlineSuppressionsAsChecked(const Tokenizer &tokenizer); + /** + * Report unmatched suppressions + * @param unmatched list of unmatched suppressions (from Settings::Suppressions::getUnmatched(Local|Global)Suppressions) + * @return true is returned if errors are reported + */ + static bool reportUnmatchedSuppressions(const std::list &unmatched, ErrorLogger &errorLogger); + private: /** @brief List of error which the user doesn't want to see. */ std::list mSuppressions; diff --git a/test/testerrorlogger.cpp b/test/testerrorlogger.cpp index 481f67bb7..14de55390 100644 --- a/test/testerrorlogger.cpp +++ b/test/testerrorlogger.cpp @@ -61,7 +61,6 @@ private: TEST_CASE(SerializeSanitize); TEST_CASE(SerializeFileLocation); - TEST_CASE(suppressUnmatchedSuppressions); TEST_CASE(substituteTemplateFormatStatic); TEST_CASE(substituteTemplateLocationStatic); } @@ -425,64 +424,6 @@ private: ASSERT_EQUALS("abcd:/,", msg2.callStack.front().getinfo()); } - void suppressUnmatchedSuppressions() { - std::list suppressions; - - // No unmatched suppression - errout.str(""); - suppressions.clear(); - reportUnmatchedSuppressions(suppressions); - ASSERT_EQUALS("", errout.str()); - - // suppress all unmatchedSuppression - errout.str(""); - suppressions.clear(); - suppressions.emplace_back("abc", "a.c", 10U); - suppressions.emplace_back("unmatchedSuppression", "*", Suppressions::Suppression::NO_LINE); - reportUnmatchedSuppressions(suppressions); - ASSERT_EQUALS("", errout.str()); - - // suppress all unmatchedSuppression (corresponds to "--suppress=unmatchedSuppression") - errout.str(""); - suppressions.clear(); - suppressions.emplace_back("abc", "a.c", 10U); - suppressions.emplace_back("unmatchedSuppression", "", Suppressions::Suppression::NO_LINE); - reportUnmatchedSuppressions(suppressions); - ASSERT_EQUALS("", errout.str()); - - // suppress all unmatchedSuppression in a.c - errout.str(""); - suppressions.clear(); - suppressions.emplace_back("abc", "a.c", 10U); - suppressions.emplace_back("unmatchedSuppression", "a.c", Suppressions::Suppression::NO_LINE); - reportUnmatchedSuppressions(suppressions); - ASSERT_EQUALS("", errout.str()); - - // suppress unmatchedSuppression in a.c at line 10 - errout.str(""); - suppressions.clear(); - suppressions.emplace_back("abc", "a.c", 10U); - suppressions.emplace_back("unmatchedSuppression", "a.c", 10U); - reportUnmatchedSuppressions(suppressions); - ASSERT_EQUALS("", errout.str()); - - // don't suppress unmatchedSuppression when file is mismatching - errout.str(""); - suppressions.clear(); - suppressions.emplace_back("abc", "a.c", 10U); - suppressions.emplace_back("unmatchedSuppression", "b.c", Suppressions::Suppression::NO_LINE); - reportUnmatchedSuppressions(suppressions); - ASSERT_EQUALS("[a.c:10]: (information) Unmatched suppression: abc\n", errout.str()); - - // don't suppress unmatchedSuppression when line is mismatching - errout.str(""); - suppressions.clear(); - suppressions.emplace_back("abc", "a.c", 10U); - suppressions.emplace_back("unmatchedSuppression", "a.c", 1U); - reportUnmatchedSuppressions(suppressions); - ASSERT_EQUALS("[a.c:10]: (information) Unmatched suppression: abc\n", errout.str()); - } - void substituteTemplateFormatStatic() const { { diff --git a/test/testsuppressions.cpp b/test/testsuppressions.cpp index f1c6a7d8a..19fdf307c 100644 --- a/test/testsuppressions.cpp +++ b/test/testsuppressions.cpp @@ -83,6 +83,8 @@ private: TEST_CASE(suppressingSyntaxErrorAndExitCode); TEST_CASE(suppressLocal); + + TEST_CASE(suppressUnmatchedSuppressions); } void suppressionsBadId1() const { @@ -859,6 +861,64 @@ private: ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid2", "test2.cpp", 1))); ASSERT_EQUALS(false, suppressions.isSuppressed(errorMessage("errorid2", "test2.cpp", 1), false)); } + + void suppressUnmatchedSuppressions() { + std::list suppressions; + + // No unmatched suppression + errout.str(""); + suppressions.clear(); + Suppressions::reportUnmatchedSuppressions(suppressions, *this); + ASSERT_EQUALS("", errout.str()); + + // suppress all unmatchedSuppression + errout.str(""); + suppressions.clear(); + suppressions.emplace_back("abc", "a.c", 10U); + suppressions.emplace_back("unmatchedSuppression", "*", Suppressions::Suppression::NO_LINE); + Suppressions::reportUnmatchedSuppressions(suppressions, *this); + ASSERT_EQUALS("", errout.str()); + + // suppress all unmatchedSuppression (corresponds to "--suppress=unmatchedSuppression") + errout.str(""); + suppressions.clear(); + suppressions.emplace_back("abc", "a.c", 10U); + suppressions.emplace_back("unmatchedSuppression", "", Suppressions::Suppression::NO_LINE); + Suppressions::reportUnmatchedSuppressions(suppressions, *this); + ASSERT_EQUALS("", errout.str()); + + // suppress all unmatchedSuppression in a.c + errout.str(""); + suppressions.clear(); + suppressions.emplace_back("abc", "a.c", 10U); + suppressions.emplace_back("unmatchedSuppression", "a.c", Suppressions::Suppression::NO_LINE); + Suppressions::reportUnmatchedSuppressions(suppressions, *this); + ASSERT_EQUALS("", errout.str()); + + // suppress unmatchedSuppression in a.c at line 10 + errout.str(""); + suppressions.clear(); + suppressions.emplace_back("abc", "a.c", 10U); + suppressions.emplace_back("unmatchedSuppression", "a.c", 10U); + Suppressions::reportUnmatchedSuppressions(suppressions, *this); + ASSERT_EQUALS("", errout.str()); + + // don't suppress unmatchedSuppression when file is mismatching + errout.str(""); + suppressions.clear(); + suppressions.emplace_back("abc", "a.c", 10U); + suppressions.emplace_back("unmatchedSuppression", "b.c", Suppressions::Suppression::NO_LINE); + Suppressions::reportUnmatchedSuppressions(suppressions, *this); + ASSERT_EQUALS("[a.c:10]: (information) Unmatched suppression: abc\n", errout.str()); + + // don't suppress unmatchedSuppression when line is mismatching + errout.str(""); + suppressions.clear(); + suppressions.emplace_back("abc", "a.c", 10U); + suppressions.emplace_back("unmatchedSuppression", "a.c", 1U); + Suppressions::reportUnmatchedSuppressions(suppressions, *this); + ASSERT_EQUALS("[a.c:10]: (information) Unmatched suppression: abc\n", errout.str()); + } }; REGISTER_TEST(TestSuppressions)