Skip to content

Commit ed47ac2

Browse files
firewavefirewave
committed
fixed #14276 - report inline suppressions with invalid error IDs [skip ci]
1 parent a9f88d4 commit ed47ac2

File tree

3 files changed

+102
-5
lines changed

3 files changed

+102
-5
lines changed

‎lib/preprocessor.cpp‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,15 @@ static std::string getRelativeFilename(const simplecpp::Token* tok, const Settin
179179
return Path::simplifyPath(std::move(relativeFilename));
180180
}
181181

182+
static void addInlineSuppression(SuppressionList::Suppression suppr, SuppressionList &suppressions, std::list<BadInlineSuppression> &bad)
183+
{
184+
const std::string file = suppr.fileName;
185+
const int line = suppr.lineNumber;
186+
const std::string errmsg = suppressions.addSuppression(std::move(suppr));
187+
if (!errmsg.empty())
188+
bad.emplace_back(file, line, 0, errmsg); // TODO: set column
189+
}
190+
182191
static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Settings &settings, SuppressionList &suppressions, std::list<BadInlineSuppression> &bad)
183192
{
184193
std::list<SuppressionList::Suppression> inlineSuppressionsBlockBegin;
@@ -257,7 +266,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
257266
suppr.lineNumber = supprBegin->lineNumber;
258267
suppr.type = SuppressionList::Type::block;
259268
inlineSuppressionsBlockBegin.erase(supprBegin);
260-
suppressions.addSuppression(std::move(suppr)); // TODO: check result
269+
addInlineSuppression(std::move(suppr), suppressions, bad);
261270
throwError = false;
262271
break;
263272
}
@@ -282,10 +291,10 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
282291
suppr.thisAndNextLine = thisAndNextLine;
283292
suppr.lineNumber = tok->location.line;
284293
suppr.macroName = macroName;
285-
suppressions.addSuppression(std::move(suppr)); // TODO: check result
294+
addInlineSuppression(std::move(suppr), suppressions, bad);
286295
} else if (SuppressionList::Type::file == suppr.type) {
287296
if (onlyComments)
288-
suppressions.addSuppression(std::move(suppr)); // TODO: check result
297+
addInlineSuppression(std::move(suppr), suppressions, bad);
289298
else
290299
bad.emplace_back(suppr.fileName, suppr.lineNumber, 0, "File suppression should be at the top of the file"); // TODO: set column
291300
}

‎test/cli/inline-suppress_test.py‎

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,9 @@ def test_1():
3636
'proj-inline-suppress'
3737
]
3838
ret, stdout, stderr = cppcheck(args, cwd=__script_dir)
39-
assert stderr == ''
39+
assert stderr.splitlines() == [
40+
"{}duplicate.cpp:3:0: error: suppression 'unreadVariable:proj-inline-suppress/duplicate.cpp:3' already exists [invalidSuppression]".format(__proj_inline_suppres_path)
41+
]
4042
assert stdout == ''
4143
assert ret == 0, stdout
4244

@@ -557,4 +559,57 @@ def test_premium_disabled_unmatched(): #13663
557559
'{}premiumUnmatched.cpp:5:0: information: Unmatched suppression: uninitvar [unmatchedSuppression]'.format(__proj_inline_suppres_path)
558560
]
559561
assert stdout == ''
560-
assert ret == 0, stdout
562+
assert ret == 0, stdout
563+
564+
@pytest.mark.xfail(strict=True)
565+
def test_duplicate_include(tmp_path):
566+
test_file_1 = tmp_path / 'test_1.c'
567+
with open(test_file_1, "w") as f:
568+
f.write('#include "test.h"')
569+
570+
test_file_2 = tmp_path / 'test_2.c'
571+
with open(test_file_2, "w") as f:
572+
f.write('#include "test.h"')
573+
574+
test_header = tmp_path / 'test.h'
575+
with open(test_header, "w") as f:
576+
f.write('// cppcheck-suppress id')
577+
578+
args = [
579+
'-q',
580+
'--template=simple',
581+
'--emit-duplicates',
582+
'--inline-suppr',
583+
str(test_file_1),
584+
str(test_file_2)
585+
]
586+
587+
exitcode, stdout, stderr = cppcheck(args)
588+
assert exitcode == 0, stdout
589+
assert stdout.splitlines() == []
590+
assert stderr.splitlines() == []
591+
592+
593+
def test_duplicate_header(tmp_path):
594+
test_file = tmp_path / 'test.c'
595+
with open(test_file, "w") as f:
596+
f.write('#include "test.h"')
597+
598+
test_header = tmp_path / 'test.h'
599+
with open(test_header, "w") as f:
600+
f.write('// cppcheck-suppress [id,id]')
601+
602+
args = [
603+
'-q',
604+
'--template=simple',
605+
'--emit-duplicates',
606+
'--inline-suppr',
607+
str(test_file)
608+
]
609+
610+
exitcode, stdout, stderr = cppcheck(args)
611+
assert exitcode == 0, stdout
612+
assert stdout.splitlines() == []
613+
assert stderr.splitlines() == [
614+
"{}:1:0: error: suppression 'id:{}:1' already exists [invalidSuppression]".format(test_header,test_header)
615+
]

‎test/testsuppressions.cpp‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -703,6 +703,39 @@ class TestSuppressions : public TestFixture {
703703
"[test.cpp:3:5]: (error) Uninitialized variable: a [uninitvar]\n"
704704
"[test.cpp:5:5]: (error) Uninitialized variable: b [uninitvar]\n", errout_str());
705705

706+
ASSERT_EQUALS(1, (this->*check)("// cppcheck-suppress-file :id0\n"
707+
"// cppcheck-suppress :id\n"
708+
"// cppcheck-suppress [:id1,id2]\n"
709+
"// cppcheck-suppress-begin :id3\n"
710+
"void f() {}\n"
711+
"// cppcheck-suppress-end :id3\n",
712+
""));
713+
ASSERT_EQUALS("[test.cpp:1:0]: (error) Failed to add suppression. Invalid id \":id0\" [invalidSuppression]\n"
714+
"[test.cpp:5:0]: (error) Failed to add suppression. Invalid id \":id\" [invalidSuppression]\n" // TODO: should we report the location of the suppression instead?
715+
"[test.cpp:5:0]: (error) Failed to add suppression. Invalid id \":id1\" [invalidSuppression]\n" // TODO: should we report the location of the suppression instead?
716+
"[test.cpp:4:0]: (error) Failed to add suppression. Invalid id \":id3\" [invalidSuppression]\n"
717+
"[test.cpp:5:0]: (information) Unmatched suppression: id2 [unmatchedSuppression]\n", errout_str()); // TODO: should we report the location of the suppression instead?
718+
719+
ASSERT_EQUALS(1, (this->*check)("// cppcheck-suppress-file id\n"
720+
"// cppcheck-suppress-file [id,id0,id0]\n" // TODO: duplicated suppression "id" not detected
721+
"// cppcheck-suppress id1\n"
722+
"// cppcheck-suppress id1\n"
723+
"// cppcheck-suppress [id1,id2,id2]\n"
724+
"// cppcheck-suppress-begin [id3,id3]\n"
725+
"void f() {}\n"
726+
"// cppcheck-suppress-end [id3,id3]\n",
727+
""));
728+
ASSERT_EQUALS("[test.cpp:2:0]: (error) suppression 'id0:test.cpp:2' already exists [invalidSuppression]\n"
729+
"[test.cpp:7:0]: (error) suppression 'id1:test.cpp:7' already exists [invalidSuppression]\n" // TODO: should we report the location of the suppression instead?
730+
"[test.cpp:7:0]: (error) suppression 'id2:test.cpp:7' already exists [invalidSuppression]\n" // TODO: should we report the location of the suppression instead?
731+
"[test.cpp:6:0]: (error) suppression 'id3:test.cpp:6' already exists [invalidSuppression]\n"
732+
"[test.cpp:1:0]: (information) Unmatched suppression: id [unmatchedSuppression]\n"
733+
"[test.cpp:2:0]: (information) Unmatched suppression: id [unmatchedSuppression]\n" // TODO: should not be reported
734+
"[test.cpp:2:0]: (information) Unmatched suppression: id0 [unmatchedSuppression]\n" // TODO: should we report the location of the suppression instead?
735+
"[test.cpp:7:0]: (information) Unmatched suppression: id1 [unmatchedSuppression]\n" // TODO: should we report the location of the suppression instead?
736+
"[test.cpp:7:0]: (information) Unmatched suppression: id2 [unmatchedSuppression]\n" // TODO: should we report the location of the suppression instead?
737+
"[test.cpp:6:0]: (information) Unmatched suppression: id3 [unmatchedSuppression]\n", errout_str());
738+
706739
ASSERT_EQUALS(1, (this->*check)("void f() {\n"
707740
" int a;\n"
708741
" // cppcheck-suppress-begin uninitvar\n"

0 commit comments

Comments
 (0)