Skip to content

fixed #14276 - report inline suppressions with invalid error IDs #7970

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
+102 −5
Draft

fixed #14276 - report inline suppressions with invalid error IDs #7970

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions lib/preprocessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<BadInlineSuppression> &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<BadInlineSuppression> &bad)
{
std::list<SuppressionList::Suppression> inlineSuppressionsBlockBegin;
Expand Down Expand Up @@ -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;
}
Expand All @@ -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
}
Expand Down
59 changes: 57 additions & 2 deletions test/cli/inline-suppress_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
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)
]
33 changes: 33 additions & 0 deletions test/testsuppressions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down