Skip to content

Commit ffd9b4c

Browse files
firewavefirewave
authored
refs #12171 - report error when adding a suppression more than once / added Suppressions::updateSuppressionState() (danmar#7122)
1 parent 047b725 commit ffd9b4c

File tree

10 files changed

+322
-11
lines changed

10 files changed

+322
-11
lines changed

‎gui/mainwindow.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1097,7 +1097,7 @@ QPair<bool,Settings> MainWindow::getCppcheckSettings()
10971097
}
10981098

10991099
for (const SuppressionList::Suppression &suppression : mProjectFile->getCheckingSuppressions()) {
1100-
result.supprs.nomsg.addSuppression(suppression);
1100+
result.supprs.nomsg.addSuppression(suppression); // TODO: check result
11011101
}
11021102

11031103
// Only check the given -D configuration

‎lib/cppcheck.cpp‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -815,7 +815,17 @@ unsigned int CppCheck::check(const FileSettings &fs)
815815
return returnValue;
816816
}
817817
const unsigned int returnValue = temp.checkFile(fs.file, fs.cfg);
818-
mSettings.supprs.nomsg.addSuppressions(temp.mSettings.supprs.nomsg.getSuppressions());
818+
for (const auto& suppr : temp.mSettings.supprs.nomsg.getSuppressions())
819+
{
820+
const bool res = mSettings.supprs.nomsg.updateSuppressionState(suppr);
821+
if (!res)
822+
{
823+
// TODO: remove fallback
824+
const std::string err = mSettings.supprs.nomsg.addSuppression(suppr);
825+
if (!err.empty())
826+
throw InternalError(nullptr, "could not update suppression '" + suppr.errorId + "'");
827+
}
828+
}
819829
if (mUnusedFunctionsCheck)
820830
mUnusedFunctionsCheck->updateFunctionData(*temp.mUnusedFunctionsCheck);
821831
while (!temp.mFileInfo.empty()) {

‎lib/importproject.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1404,7 +1404,7 @@ bool ImportProject::importCppcheckGuiProject(std::istream &istr, Settings *setti
14041404

14051405
for (const std::string &p : paths)
14061406
guiProject.pathNames.push_back(Path::fromNativeSeparators(p));
1407-
settings->supprs.nomsg.addSuppressions(std::move(suppressions));
1407+
settings->supprs.nomsg.addSuppressions(std::move(suppressions)); // TODO: check result
14081408
settings->checkHeaders = temp.checkHeaders;
14091409
settings->checkUnusedTemplates = temp.checkUnusedTemplates;
14101410
settings->maxCtuDepth = temp.maxCtuDepth;

‎lib/preprocessor.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
256256
suppr.lineNumber = supprBegin->lineNumber;
257257
suppr.type = SuppressionList::Type::block;
258258
inlineSuppressionsBlockBegin.erase(supprBegin);
259-
suppressions.addSuppression(std::move(suppr));
259+
suppressions.addSuppression(std::move(suppr)); // TODO: check result
260260
throwError = false;
261261
break;
262262
}
@@ -281,10 +281,10 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
281281
suppr.thisAndNextLine = thisAndNextLine;
282282
suppr.lineNumber = tok->location.line;
283283
suppr.macroName = macroName;
284-
suppressions.addSuppression(std::move(suppr));
284+
suppressions.addSuppression(std::move(suppr)); // TODO: check result
285285
} else if (SuppressionList::Type::file == suppr.type) {
286286
if (onlyComments)
287-
suppressions.addSuppression(std::move(suppr));
287+
suppressions.addSuppression(std::move(suppr)); // TODO: check result
288288
else
289289
bad.emplace_back(suppr.fileName, suppr.lineNumber, "File suppression should be at the top of the file");
290290
}

‎lib/suppressions.cpp‎

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -258,10 +258,7 @@ std::string SuppressionList::addSuppression(SuppressionList::Suppression suppres
258258
auto foundSuppression = std::find_if(mSuppressions.begin(), mSuppressions.end(),
259259
std::bind(&Suppression::isSameParameters, &suppression, std::placeholders::_1));
260260
if (foundSuppression != mSuppressions.end()) {
261-
// Update matched state of existing global suppression
262-
if (!suppression.isLocal() && suppression.matched)
263-
foundSuppression->matched = suppression.matched;
264-
return "";
261+
return "suppression '" + suppression.toString() + "' already exists";
265262
}
266263

267264
// Check that errorId is valid..
@@ -297,6 +294,21 @@ std::string SuppressionList::addSuppressions(std::list<Suppression> suppressions
297294
return "";
298295
}
299296

297+
bool SuppressionList::updateSuppressionState(const SuppressionList::Suppression& suppression)
298+
{
299+
// Check if suppression is already in list
300+
auto foundSuppression = std::find_if(mSuppressions.begin(), mSuppressions.end(),
301+
std::bind(&Suppression::isSameParameters, &suppression, std::placeholders::_1));
302+
if (foundSuppression != mSuppressions.end()) {
303+
// Update matched state of existing global suppression
304+
if (!suppression.isLocal() && suppression.matched)
305+
foundSuppression->matched = suppression.matched;
306+
return true;
307+
}
308+
309+
return false;
310+
}
311+
300312
void SuppressionList::ErrorMessage::setFileName(std::string s)
301313
{
302314
mFileName = Path::simplifyPath(std::move(s));
@@ -585,3 +597,22 @@ bool SuppressionList::reportUnmatchedSuppressions(const std::list<SuppressionLis
585597
}
586598
return err;
587599
}
600+
601+
std::string SuppressionList::Suppression::toString() const
602+
{
603+
std::string s;
604+
s+= errorId;
605+
if (!fileName.empty()) {
606+
s += ':';
607+
s += fileName;
608+
if (lineNumber != -1) {
609+
s += ':';
610+
s += std::to_string(lineNumber);
611+
}
612+
}
613+
if (!symbolName.empty()) {
614+
s += ':';
615+
s += symbolName;
616+
}
617+
return s;
618+
}

‎lib/suppressions.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,8 @@ class CPPCHECKLIB SuppressionList {
136136
thisAndNextLine == other.thisAndNextLine;
137137
}
138138

139+
std::string toString() const;
140+
139141
std::string errorId;
140142
std::string fileName;
141143
int lineNumber = NO_LINE;
@@ -196,6 +198,13 @@ class CPPCHECKLIB SuppressionList {
196198
*/
197199
std::string addSuppressions(std::list<Suppression> suppressions);
198200

201+
/**
202+
* @brief Updates the state of the given suppression.
203+
* @param suppression the suppression to update
204+
* @return true if suppression to update was found
205+
*/
206+
bool updateSuppressionState(const SuppressionList::Suppression& suppression);
207+
199208
/**
200209
* @brief Returns true if this message should not be shown to the user.
201210
* @param errmsg error message

‎test/cli/inline-suppress_test.py‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,3 +346,63 @@ def test_unused_function_unmatched_build_dir(tmpdir):
346346
@pytest.mark.xfail(strict=True)
347347
def test_unused_function_unmatched_build_dir_j(tmpdir):
348348
__test_unused_function_unmatched_build_dir(tmpdir, ['-j2'])
349+
350+
351+
@pytest.mark.xfail(strict=True) # no error as inline suppressions are currently not being propagated back
352+
def test_duplicate():
353+
args = [
354+
'-q',
355+
'--template=simple',
356+
'--enable=all',
357+
'--inline-suppr',
358+
'proj-inline-suppress/duplicate.cpp'
359+
]
360+
361+
ret, stdout, stderr = cppcheck(args, cwd=__script_dir)
362+
assert stderr.splitlines() == []
363+
assert stdout.splitlines() == [
364+
"cppcheck: error: suppression 'unreadVariable' already exists"
365+
]
366+
assert ret == 0, stdout
367+
368+
369+
@pytest.mark.xfail(strict=True) # no error as inline suppressions are currently not being propagated back
370+
def test_duplicate_cmd():
371+
args = [
372+
'-q',
373+
'--template=simple',
374+
'--enable=all',
375+
'--inline-suppr',
376+
'--suppress=unreadVariable',
377+
'proj-inline-suppress/4.c'
378+
]
379+
380+
ret, stdout, stderr = cppcheck(args, cwd=__script_dir)
381+
assert stderr.splitlines() == []
382+
assert stdout.splitlines() == [
383+
"cppcheck: error: suppression 'unreadVariable' already exists"
384+
]
385+
assert ret == 0, stdout
386+
387+
388+
@pytest.mark.xfail(strict=True) # no error as inline suppressions are currently not being propagated back
389+
def test_duplicate_file(tmp_path):
390+
suppr_file = tmp_path / 'suppressions'
391+
with open(suppr_file, 'wt') as f:
392+
f.write('unreadVariable')
393+
394+
args = [
395+
'-q',
396+
'--template=simple',
397+
'--enable=all',
398+
'--inline-suppr',
399+
'--suppressions-list={}'.format(suppr_file),
400+
'proj-inline-suppress/4.c'
401+
]
402+
403+
ret, stdout, stderr = cppcheck(args, cwd=__script_dir)
404+
assert stderr.splitlines() == []
405+
assert stdout.splitlines() == [
406+
"cppcheck: error: suppression 'unreadVariable' already exists"
407+
]
408+
assert ret == 0, stdout

‎test/cli/other_test.py‎

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2602,4 +2602,74 @@ def test_inline_suppr_builddir_j_cached(tmp_path):
26022602
build_dir = tmp_path / 'b1'
26032603
os.mkdir(build_dir)
26042604
__test_inline_suppr(tmp_path, ['--cppcheck-build-dir={}'.format(build_dir), '-j2'])
2605-
__test_inline_suppr(tmp_path, ['--cppcheck-build-dir={}'.format(build_dir), '-j2'])
2605+
__test_inline_suppr(tmp_path, ['--cppcheck-build-dir={}'.format(build_dir), '-j2'])
2606+
2607+
2608+
def test_duplicate_suppression(tmp_path):
2609+
test_file = tmp_path / 'file.cpp'
2610+
with open(test_file, 'wt'):
2611+
pass
2612+
2613+
args = [
2614+
'-q',
2615+
'--suppress=uninitvar',
2616+
'--suppress=uninitvar',
2617+
str(test_file)
2618+
]
2619+
2620+
exitcode, stdout, stderr = cppcheck(args)
2621+
assert exitcode == 1, stdout
2622+
assert stdout.splitlines() == [
2623+
"cppcheck: error: suppression 'uninitvar' already exists"
2624+
]
2625+
assert stderr == ''
2626+
2627+
2628+
def test_duplicate_suppressions_list(tmp_path):
2629+
suppr_file = tmp_path / 'suppressions'
2630+
with open(suppr_file, 'wt') as f:
2631+
f.write('''
2632+
uninitvar
2633+
uninitvar
2634+
''')
2635+
2636+
test_file = tmp_path / 'file.cpp'
2637+
with open(test_file, 'wt'):
2638+
pass
2639+
2640+
args = [
2641+
'-q',
2642+
'--suppressions-list={}'.format(suppr_file),
2643+
str(test_file)
2644+
]
2645+
2646+
exitcode, stdout, stderr = cppcheck(args)
2647+
assert exitcode == 1, stdout
2648+
assert stdout.splitlines() == [
2649+
"cppcheck: error: suppression 'uninitvar' already exists"
2650+
]
2651+
assert stderr == ''
2652+
2653+
2654+
def test_duplicate_suppressions_mixed(tmp_path):
2655+
suppr_file = tmp_path / 'suppressions'
2656+
with open(suppr_file, 'wt') as f:
2657+
f.write('uninitvar')
2658+
2659+
test_file = tmp_path / 'file.cpp'
2660+
with open(test_file, 'wt'):
2661+
pass
2662+
2663+
args = [
2664+
'-q',
2665+
'--suppress=uninitvar',
2666+
'--suppressions-list={}'.format(suppr_file),
2667+
str(test_file)
2668+
]
2669+
2670+
exitcode, stdout, stderr = cppcheck(args)
2671+
assert exitcode == 1, stdout
2672+
assert stdout.splitlines() == [
2673+
"cppcheck: error: suppression 'uninitvar' already exists"
2674+
]
2675+
assert stderr == ''

‎test/cli/proj-inline-suppress/duplicate.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
int main() {
2+
// cppcheck-suppress [unreadVariable,unreadVariable]
3+
int i = 0;
4+
return 0;
5+
}

0 commit comments

Comments
 (0)