Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 6 additions & 2 deletions addons/cppcheckdata.py
Original file line number Diff line number Diff line change
Expand Up @@ -997,6 +997,7 @@ class Suppression:
errorId The id string of the error to suppress, can be a wildcard
fileName The name of the file to suppress warnings for, can include wildcards
lineNumber The number of the line to suppress warnings from, can be 0 to represent any line
thisAndNextLine Set if the line after lineNumber is suppressed too
symbolName The name of the symbol to match warnings for, can include wildcards
lineBegin The first line to suppress warnings from
lineEnd The last line to suppress warnings from
Expand All @@ -1006,6 +1007,7 @@ class Suppression:
errorId = None
fileName = None
lineNumber = None
thisAndNextLine = None
symbolName = None
lineBegin = None
lineEnd = None
Expand All @@ -1015,13 +1017,14 @@ def __init__(self, element):
self.errorId = element.get('errorId')
self.fileName = element.get('fileName')
self.lineNumber = element.get('lineNumber')
self.thisAndNextLine = element.get('thisAndNextLine')
self.symbolName = element.get('symbolName')
self.lineBegin = element.get('lineBegin')
self.lineEnd = element.get('lineEnd')
self.suppressionType = element.get('type')

def __repr__(self):
attrs = ["errorId", "fileName", "lineNumber", "symbolName", "lineBegin", "lineEnd","suppressionType"]
attrs = ["errorId", "fileName", "lineNumber", "thisAndNextLine", "symbolName", "lineBegin", "lineEnd","suppressionType"]
return "{}({})".format(
"Suppression",
", ".join(("{}={}".format(a, repr(getattr(self, a))) for a in attrs))
Expand All @@ -1031,7 +1034,7 @@ def isMatch(self, file, line, message, errorId):
# Line Suppression
if ((self.fileName is None or fnmatch(file, self.fileName))
and (self.suppressionType is None) # Verify use of default suppression type (None = unique)
and (self.lineNumber is not None and int(line) == int(self.lineNumber))
and (self.lineNumber is not None and (int(line) == int(self.lineNumber) or (self.thisAndNextLine and int(line) == int(self.lineNumber) + 1)))
and (self.symbolName is None or fnmatch(message, '*'+self.symbolName+'*'))
and fnmatch(errorId, self.errorId)):
return True
Expand All @@ -1052,6 +1055,7 @@ def isMatch(self, file, line, message, errorId):
# Other Suppression (Globally set via suppression file or cli command)
if ((self.fileName is None or fnmatch(file, self.fileName))
and (self.suppressionType is None)
and (self.lineNumber is None)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review

This breaks the { // cppcheck-suppress id form (with the { alone on its line). cppcheck applies that suppression to the comment's line and to the next line (thisAndNextLine in lib/preprocessor.cpp and SuppressionList::Suppression::isSuppressed). SuppressionList::dump() does not write thisAndNextLine, so the dump only has lineNumber = the line of the {. Before this PR, this fallback branch suppressed the warning on the next line by accident. Now the addon reports it when run with Python on the dump, while cppcheck --addon=misra --inline-suppr still suppresses it.

Example:

int g(int x)
{ // cppcheck-suppress misra-c2012-14.4
    if (x) { return 1; }
    return 0;
}

With the PR, misra.py on the dump reports [t2.c:3] [misra-c2012-14.4]; the base version does not.

To fix this, add e.g. thisAndNextLine="true" to the dump and in the line-suppression branch also accept int(line) == int(self.lineNumber) + 1 when that flag is set.

and (self.symbolName is None or fnmatch(message, '*'+self.symbolName+'*'))
and fnmatch(errorId, self.errorId)):
return True
Expand Down
2 changes: 2 additions & 0 deletions lib/suppressions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,8 @@ void SuppressionList::dump(std::ostream & out, const std::string& filePath) cons
out << " fileName=\"" << ErrorLogger::toxml(suppression.fileName) << '"';
if (suppression.lineNumber != Suppression::NO_LINE)
out << " lineNumber=\"" << suppression.lineNumber << '"';
if (suppression.thisAndNextLine)
out << " thisAndNextLine=\"true\"";
if (!suppression.symbolName.empty())
out << " symbolName=\"" << ErrorLogger::toxml(suppression.symbolName) << '\"';
if (suppression.hash > 0)
Expand Down
13 changes: 9 additions & 4 deletions test/addon/test_cppcheckdata.py
Original file line number Diff line number Diff line change
Expand Up @@ -771,13 +771,17 @@ def test_line_suppression(self):
assert supp.isMatch('a.c', '5', 'msg', 'zerodiv')
assert not supp.isMatch('a.c', '5', 'msg', 'nullPointer')

@pytest.mark.xfail(strict=False,
reason='the "other suppression" fallback in Suppression.isMatch() does not check '
'that lineNumber is unset, so a line suppression matches every line')
def test_line_suppression_other_line(self):
supp = self.make_suppression(errorId='zerodiv', lineNumber='5')
assert not supp.isMatch('a.c', '6', 'msg', 'zerodiv')

def test_line_suppression_this_and_next_line(self):
supp = self.make_suppression(errorId='zerodiv', lineNumber='5', thisAndNextLine='true')
assert supp.isMatch('a.c', '5', 'msg', 'zerodiv')
assert supp.isMatch('a.c', '6', 'msg', 'zerodiv')
assert not supp.isMatch('a.c', '4', 'msg', 'zerodiv')
assert not supp.isMatch('a.c', '7', 'msg', 'zerodiv')

def test_wildcard_errorId(self):
supp = self.make_suppression(errorId='*', lineNumber='5')
assert supp.isMatch('a.c', '5', 'msg', 'anything')
Expand Down Expand Up @@ -821,7 +825,8 @@ def test_dumpfile_suppressions(self, dump_factory):
location = cppcheckdata.Location({'file': supp.fileName, 'line': '5', 'column': '5'})
assert cppcheckdata.is_suppressed(location, 'msg', 'uninitvar')
assert not cppcheckdata.is_suppressed(location, 'msg', 'nullPointer')
# NOTE: a non-matching line is not checked here - see test_line_suppression_other_line
location = cppcheckdata.Location({'file': supp.fileName, 'line': '6', 'column': '5'})
assert not cppcheckdata.is_suppressed(location, 'msg', 'uninitvar')


class TestPureHelpers:
Expand Down
19 changes: 19 additions & 0 deletions test/cli/dumpfile_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,3 +146,22 @@ def test_line_macro(tmpdir):
with open(dumpfile, 'rt') as f:
dump = f.read()
assert 'type="number" isInt="true" macroName="__LINE__"' in dump


def test_suppression_this_and_next_line(tmpdir):
test_file = str(tmpdir / 'test.c')
with open(test_file, 'wt') as f:
f.write('void f(void)\n')
f.write('{ // cppcheck-suppress id1\n')
f.write(' // cppcheck-suppress id2\n')
f.write(' x = 0;\n')
f.write('}\n')

exitcode, _, stderr = cppcheck(['--dump', '--inline-suppr', test_file])
assert exitcode == 0, stderr

dumpfile = test_file + '.dump'
with open(dumpfile, 'rt') as f:
dump = f.read()
assert 'lineNumber="2" thisAndNextLine="true" inline="true"' in dump
assert 'lineNumber="4" inline="true"' in dump
Loading