From 1811182e5282c8d903b4214a38859ba7d2c2ac1a Mon Sep 17 00:00:00 2001 From: firewave Date: Wed, 3 Dec 2025 22:39:33 +0100 Subject: [PATCH] refs #14280 - report inline suppressions without an error ID --- lib/preprocessor.cpp | 15 ++++++++++++--- test/testsuppressions.cpp | 38 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/lib/preprocessor.cpp b/lib/preprocessor.cpp index 0e02e3bb488..636e5a848a1 100644 --- a/lib/preprocessor.cpp +++ b/lib/preprocessor.cpp @@ -88,18 +88,22 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std: const std::string::size_type pos1 = comment.find_first_not_of("/* \t"); if (pos1 == std::string::npos) return false; - if (pos1 + cppchecksuppress.size() >= comment.size()) - return false; if (comment.substr(pos1, cppchecksuppress.size()) != cppchecksuppress) return false; + if (pos1 + cppchecksuppress.size() >= comment.size()) { + bad.emplace_back(tok->location.file(), tok->location.line, 0, "suppression without error ID"); + return false; + } // check if it has a prefix const std::string::size_type posEndComment = comment.find_first_of(" [", pos1+cppchecksuppress.size()); // skip spaces after "cppcheck-suppress" and its possible prefix const std::string::size_type pos2 = comment.find_first_not_of(' ', posEndComment); - if (pos2 == std::string::npos) + if (pos2 == std::string::npos) { + bad.emplace_back(tok->location.file(), tok->location.line, 0, "suppression without error ID"); return false; + } SuppressionList::Type errorType = SuppressionList::Type::unique; @@ -142,9 +146,11 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std: s.lineNumber = tok->location.line; } + // TODO: return false? if (!errmsg.empty()) bad.emplace_back(tok->location.file(), tok->location.line, tok->location.col, std::move(errmsg)); + // TODO: report ones without ID - return false? std::copy_if(suppressions.cbegin(), suppressions.cend(), std::back_inserter(inlineSuppressions), [](const SuppressionList::Suppression& s) { return !s.errorId.empty(); }); @@ -159,9 +165,12 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std: s.type = errorType; s.lineNumber = tok->location.line; + // TODO: report when no ID - unreachable? if (!s.errorId.empty()) inlineSuppressions.push_back(std::move(s)); + // TODO: unreachable? + // TODO: return false? if (!errmsg.empty()) bad.emplace_back(tok->location.file(), tok->location.line, tok->location.col, std::move(errmsg)); } diff --git a/test/testsuppressions.cpp b/test/testsuppressions.cpp index 205c6a2f33c..f946461ff89 100644 --- a/test/testsuppressions.cpp +++ b/test/testsuppressions.cpp @@ -396,7 +396,6 @@ class TestSuppressions : public TestFixture { } #endif // HAS_THREADING_MODEL_FORK - // TODO: check all results void runChecks(unsigned int (TestSuppressions::*check)(const char[], const std::string &)) { // check to make sure the appropriate errors are present ASSERT_EQUALS(1, (this->*check)("void f() {\n" @@ -720,6 +719,40 @@ class TestSuppressions : public TestFixture { "[test.cpp:6:0]: (error) unknown suppression type 'cppcheck-suppress-end-unknown' [invalidSuppression]\n" "[test.cpp:4:0]: (error) Suppress Begin: No matching end [invalidSuppression]\n", errout_str()); + ASSERT_EQUALS(1, (this->*check)("// cppcheck-suppress-file\n" + "// cppcheck-suppress\n" + "// cppcheck-suppress \n" + "// cppcheck-suppress\t\n" + "// cppcheck-suppress []\n" // TODO + "// cppcheck-suppress-macro\n" + "// cppcheck-suppress-begin\n" + "// cppcheck-suppress-begin id0\n" + "void f() {}\n" + "// cppcheck-suppress-end\n", + "")); + ASSERT_EQUALS("[test.cpp:1:0]: (error) suppression without error ID [invalidSuppression]\n" + "[test.cpp:2:0]: (error) suppression without error ID [invalidSuppression]\n" + "[test.cpp:3:0]: (error) suppression without error ID [invalidSuppression]\n" + "[test.cpp:4:0]: (error) suppression without error ID [invalidSuppression]\n" + "[test.cpp:6:0]: (error) suppression without error ID [invalidSuppression]\n" + "[test.cpp:7:0]: (error) suppression without error ID [invalidSuppression]\n" + "[test.cpp:10:0]: (error) suppression without error ID [invalidSuppression]\n" + "[test.cpp:8:0]: (error) Suppress Begin: No matching end [invalidSuppression]\n", errout_str()); + + ASSERT_EQUALS(1, (this->*check)("// cppcheck-suppress:\n" + "// cppcheck-suppress-unknown\n" + "// cppcheck-suppress-begin-unknown\n" + "// cppcheck-suppress-begin\n" + "void f() {}\n" + "// cppcheck-suppress-end-unknown\n", + "")); + // TODO: actually these are all invalid types + ASSERT_EQUALS("[test.cpp:1:0]: (error) suppression without error ID [invalidSuppression]\n" + "[test.cpp:2:0]: (error) suppression without error ID [invalidSuppression]\n" + "[test.cpp:3:0]: (error) suppression without error ID [invalidSuppression]\n" + "[test.cpp:4:0]: (error) suppression without error ID [invalidSuppression]\n" + "[test.cpp:6:0]: (error) suppression without error ID [invalidSuppression]\n", errout_str()); + ASSERT_EQUALS(1, (this->*check)("void f() {\n" " int a;\n" " // cppcheck-suppress-begin uninitvar\n" @@ -915,13 +948,14 @@ class TestSuppressions : public TestFixture { "uninitvar")); ASSERT_EQUALS("", errout_str()); - // cppcheck-suppress-macro + // TODO: check result (this->*check)("// cppcheck-suppress-macro zerodiv\n" "#define DIV(A,B) A/B\n" "a = DIV(10,0);\n", ""); ASSERT_EQUALS("", errout_str()); + // TODO: check result (this->*check)("// cppcheck-suppress-macro abc\n" "#define DIV(A,B) A/B\n" "a = DIV(10,1);\n",