diff --git a/lib/preprocessor.cpp b/lib/preprocessor.cpp index 04df86312e3..cd120e35d18 100644 --- a/lib/preprocessor.cpp +++ b/lib/preprocessor.cpp @@ -179,6 +179,15 @@ static std::string getRelativeFilename(const simplecpp::Token* tok, const Settin return Path::simplifyPath(std::move(relativeFilename)); } +static void addInlineSuppression(SuppressionList::Suppression suppr, SuppressionList &suppressions, std::list &bad) +{ + const std::string file = suppr.fileName; + const int line = suppr.lineNumber; + const std::string errmsg = suppressions.addSuppression(std::move(suppr)); + if (!errmsg.empty()) + bad.emplace_back(file, line, 0, errmsg); // TODO: set column +} + static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Settings &settings, SuppressionList &suppressions, std::list &bad) { std::list inlineSuppressionsBlockBegin; @@ -257,7 +266,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett suppr.lineNumber = supprBegin->lineNumber; suppr.type = SuppressionList::Type::block; inlineSuppressionsBlockBegin.erase(supprBegin); - suppressions.addSuppression(std::move(suppr)); // TODO: check result + addInlineSuppression(std::move(suppr), suppressions, bad); throwError = false; break; } @@ -282,10 +291,10 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett suppr.thisAndNextLine = thisAndNextLine; suppr.lineNumber = tok->location.line; suppr.macroName = macroName; - suppressions.addSuppression(std::move(suppr)); // TODO: check result + addInlineSuppression(std::move(suppr), suppressions, bad); } else if (SuppressionList::Type::file == suppr.type) { if (onlyComments) - suppressions.addSuppression(std::move(suppr)); // TODO: check result + addInlineSuppression(std::move(suppr), suppressions, bad); else bad.emplace_back(suppr.fileName, suppr.lineNumber, 0, "File suppression should be at the top of the file"); // TODO: set column } diff --git a/test/cli/inline-suppress_test.py b/test/cli/inline-suppress_test.py index 7b8be839df4..2aabf22ed3e 100644 --- a/test/cli/inline-suppress_test.py +++ b/test/cli/inline-suppress_test.py @@ -36,7 +36,9 @@ def test_1(): 'proj-inline-suppress' ] ret, stdout, stderr = cppcheck(args, cwd=__script_dir) - assert stderr == '' + assert stderr.splitlines() == [ + "{}duplicate.cpp:3:0: error: suppression 'unreadVariable:proj-inline-suppress/duplicate.cpp:3' already exists [invalidSuppression]".format(__proj_inline_suppres_path) + ] assert stdout == '' assert ret == 0, stdout @@ -557,4 +559,57 @@ def test_premium_disabled_unmatched(): #13663 '{}premiumUnmatched.cpp:5:0: information: Unmatched suppression: uninitvar [unmatchedSuppression]'.format(__proj_inline_suppres_path) ] assert stdout == '' - assert ret == 0, stdout \ No newline at end of file + assert ret == 0, stdout + +@pytest.mark.xfail(strict=True) +def test_duplicate_include(tmp_path): + test_file_1 = tmp_path / 'test_1.c' + with open(test_file_1, "w") as f: + f.write('#include "test.h"') + + test_file_2 = tmp_path / 'test_2.c' + with open(test_file_2, "w") as f: + f.write('#include "test.h"') + + test_header = tmp_path / 'test.h' + with open(test_header, "w") as f: + f.write('// cppcheck-suppress id') + + args = [ + '-q', + '--template=simple', + '--emit-duplicates', + '--inline-suppr', + str(test_file_1), + str(test_file_2) + ] + + exitcode, stdout, stderr = cppcheck(args) + assert exitcode == 0, stdout + assert stdout.splitlines() == [] + assert stderr.splitlines() == [] + + +def test_duplicate_header(tmp_path): + test_file = tmp_path / 'test.c' + with open(test_file, "w") as f: + f.write('#include "test.h"') + + test_header = tmp_path / 'test.h' + with open(test_header, "w") as f: + f.write('// cppcheck-suppress [id,id]') + + args = [ + '-q', + '--template=simple', + '--emit-duplicates', + '--inline-suppr', + str(test_file) + ] + + exitcode, stdout, stderr = cppcheck(args) + assert exitcode == 0, stdout + assert stdout.splitlines() == [] + assert stderr.splitlines() == [ + "{}:1:0: error: suppression 'id:{}:1' already exists [invalidSuppression]".format(test_header,test_header) + ] \ No newline at end of file diff --git a/test/testsuppressions.cpp b/test/testsuppressions.cpp index 6c6b06be8e7..d4312e22611 100644 --- a/test/testsuppressions.cpp +++ b/test/testsuppressions.cpp @@ -703,6 +703,39 @@ class TestSuppressions : public TestFixture { "[test.cpp:3:5]: (error) Uninitialized variable: a [uninitvar]\n" "[test.cpp:5:5]: (error) Uninitialized variable: b [uninitvar]\n", errout_str()); + ASSERT_EQUALS(1, (this->*check)("// cppcheck-suppress-file :id0\n" + "// cppcheck-suppress :id\n" + "// cppcheck-suppress [:id1,id2]\n" + "// cppcheck-suppress-begin :id3\n" + "void f() {}\n" + "// cppcheck-suppress-end :id3\n", + "")); + ASSERT_EQUALS("[test.cpp:1:0]: (error) Failed to add suppression. Invalid id \":id0\" [invalidSuppression]\n" + "[test.cpp:5:0]: (error) Failed to add suppression. Invalid id \":id\" [invalidSuppression]\n" // TODO: should we report the location of the suppression instead? + "[test.cpp:5:0]: (error) Failed to add suppression. Invalid id \":id1\" [invalidSuppression]\n" // TODO: should we report the location of the suppression instead? + "[test.cpp:4:0]: (error) Failed to add suppression. Invalid id \":id3\" [invalidSuppression]\n" + "[test.cpp:5:0]: (information) Unmatched suppression: id2 [unmatchedSuppression]\n", errout_str()); // TODO: should we report the location of the suppression instead? + + ASSERT_EQUALS(1, (this->*check)("// cppcheck-suppress-file id\n" + "// cppcheck-suppress-file [id,id0,id0]\n" // TODO: duplicated suppression "id" not detected + "// cppcheck-suppress id1\n" + "// cppcheck-suppress id1\n" + "// cppcheck-suppress [id1,id2,id2]\n" + "// cppcheck-suppress-begin [id3,id3]\n" + "void f() {}\n" + "// cppcheck-suppress-end [id3,id3]\n", + "")); + ASSERT_EQUALS("[test.cpp:2:0]: (error) suppression 'id0:test.cpp:2' already exists [invalidSuppression]\n" + "[test.cpp:7:0]: (error) suppression 'id1:test.cpp:7' already exists [invalidSuppression]\n" // TODO: should we report the location of the suppression instead? + "[test.cpp:7:0]: (error) suppression 'id2:test.cpp:7' already exists [invalidSuppression]\n" // TODO: should we report the location of the suppression instead? + "[test.cpp:6:0]: (error) suppression 'id3:test.cpp:6' already exists [invalidSuppression]\n" + "[test.cpp:1:0]: (information) Unmatched suppression: id [unmatchedSuppression]\n" + "[test.cpp:2:0]: (information) Unmatched suppression: id [unmatchedSuppression]\n" // TODO: should not be reported + "[test.cpp:2:0]: (information) Unmatched suppression: id0 [unmatchedSuppression]\n" // TODO: should we report the location of the suppression instead? + "[test.cpp:7:0]: (information) Unmatched suppression: id1 [unmatchedSuppression]\n" // TODO: should we report the location of the suppression instead? + "[test.cpp:7:0]: (information) Unmatched suppression: id2 [unmatchedSuppression]\n" // TODO: should we report the location of the suppression instead? + "[test.cpp:6:0]: (information) Unmatched suppression: id3 [unmatchedSuppression]\n", errout_str()); + ASSERT_EQUALS(1, (this->*check)("void f() {\n" " int a;\n" " // cppcheck-suppress-begin uninitvar\n"