Skip to content

Commit b484351

Browse files
firewavefirewave
committed
fixed #13663 - reworked unmatchedSuppression exclusion logic / do not report misra-* or premium-* suppressions when they are not enabled
1 parent a8b7e2b commit b484351

File tree

12 files changed

+241
-130
lines changed

12 files changed

+241
-130
lines changed

‎cli/cmdlineparser.cpp‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1609,11 +1609,22 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
16091609
if (mSettings.jobs > 1 && mSettings.buildDir.empty()) {
16101610
// TODO: bail out instead?
16111611
if (mSettings.checks.isEnabled(Checks::unusedFunction))
1612+
{
16121613
mLogger.printMessage("unusedFunction check requires --cppcheck-build-dir to be active with -j.");
1614+
mSettings.checks.disable(Checks::unusedFunction);
1615+
// TODO: is there some later logic to remove?
1616+
}
16131617
// TODO: enable
16141618
//mLogger.printMessage("whole program analysis requires --cppcheck-build-dir to be active with -j.");
16151619
}
16161620

1621+
if (!mSettings.checks.isEnabled(Checks::unusedFunction))
1622+
mSettings.unmatchedSuppressionFilters.emplace_back("unusedFunction");
1623+
if (!mSettings.addons.count("misra"))
1624+
mSettings.unmatchedSuppressionFilters.emplace_back("misra-*");
1625+
if (!mSettings.premium)
1626+
mSettings.unmatchedSuppressionFilters.emplace_back("premium-*");
1627+
16171628
if (inputAsFilter) {
16181629
mSettings.fileFilters.insert(mSettings.fileFilters.end(), mPathNames.cbegin(), mPathNames.cend());
16191630
mPathNames.clear();

‎cli/cppcheckexecutor.cpp‎

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -293,35 +293,65 @@ int CppCheckExecutor::check_wrapper(const Settings& settings, Suppressions& supp
293293
return check_internal(settings, supprs);
294294
}
295295

296-
bool CppCheckExecutor::reportSuppressions(const Settings &settings, const SuppressionList& suppressions, bool unusedFunctionCheckEnabled, const std::list<FileWithDetails> &files, const std::list<FileSettings>& fileSettings, ErrorLogger& errorLogger) {
297-
const auto& suppr = suppressions.getSuppressions();
298-
if (std::any_of(suppr.begin(), suppr.end(), [](const SuppressionList::Suppression& s) {
296+
/**
297+
* Report unmatched suppressions
298+
* @param unmatched list of unmatched suppressions (from Settings::Suppressions::getUnmatched(Local|Global)Suppressions)
299+
* @return true is returned if errors are reported
300+
*/
301+
static bool reportUnmatchedSuppressions(const std::list<SuppressionList::Suppression> &unmatched, ErrorLogger &errorLogger, const std::vector<std::string>& filters)
302+
{
303+
bool err = false;
304+
// Report unmatched suppressions
305+
for (const SuppressionList::Suppression &s : unmatched) {
306+
bool skip = false;
307+
for (const auto& filter : filters)
308+
{
309+
if (matchglob(filter, s.errorId))
310+
{
311+
skip = true;
312+
break;
313+
}
314+
}
315+
if (skip)
316+
continue;
317+
318+
std::list<::ErrorMessage::FileLocation> callStack;
319+
if (!s.fileName.empty()) {
320+
callStack.emplace_back(s.fileName, s.lineNumber, 0);
321+
}
322+
errorLogger.reportErr(::ErrorMessage(std::move(callStack), "", Severity::information, "Unmatched suppression: " + s.errorId, "unmatchedSuppression", Certainty::normal));
323+
err = true;
324+
}
325+
return err;
326+
}
327+
328+
bool CppCheckExecutor::reportUnmatchedSuppressions(const Settings &settings, const SuppressionList& suppressions, const std::list<FileWithDetails> &files, const std::list<FileSettings>& fileSettings, ErrorLogger& errorLogger) {
329+
// TODO: document this
330+
const auto suppr = suppressions.getSuppressions();
331+
if (std::any_of(suppr.cbegin(), suppr.cend(), [](const SuppressionList::Suppression& s) {
299332
return s.errorId == "unmatchedSuppression" && s.fileName.empty() && s.lineNumber == SuppressionList::Suppression::NO_LINE;
300333
}))
301334
return false;
302335

303336
bool err = false;
304-
if (settings.useSingleJob()) {
337+
{
305338
// the two inputs may only be used exclusively
306339
assert(!(!files.empty() && !fileSettings.empty()));
307340

308341
for (auto i = files.cbegin(); i != files.cend(); ++i) {
309-
err |= SuppressionList::reportUnmatchedSuppressions(
310-
suppressions.getUnmatchedLocalSuppressions(*i, unusedFunctionCheckEnabled), errorLogger);
342+
err |= ::reportUnmatchedSuppressions(suppressions.getUnmatchedLocalSuppressions(*i), errorLogger, settings.unmatchedSuppressionFilters);
311343
}
312344

313345
for (auto i = fileSettings.cbegin(); i != fileSettings.cend(); ++i) {
314-
err |= SuppressionList::reportUnmatchedSuppressions(
315-
suppressions.getUnmatchedLocalSuppressions(i->file, unusedFunctionCheckEnabled), errorLogger);
346+
err |= ::reportUnmatchedSuppressions(suppressions.getUnmatchedLocalSuppressions(i->file), errorLogger, settings.unmatchedSuppressionFilters);
316347
}
317348
}
318349
if (settings.inlineSuppressions) {
319350
// report unmatched unusedFunction suppressions
320-
err |= SuppressionList::reportUnmatchedSuppressions(
321-
suppressions.getUnmatchedInlineSuppressions(unusedFunctionCheckEnabled), errorLogger);
351+
err |= ::reportUnmatchedSuppressions(suppressions.getUnmatchedInlineSuppressions(), errorLogger, settings.unmatchedSuppressionFilters);
322352
}
323353

324-
err |= SuppressionList::reportUnmatchedSuppressions(suppressions.getUnmatchedGlobalSuppressions(unusedFunctionCheckEnabled), errorLogger);
354+
err |= ::reportUnmatchedSuppressions(suppressions.getUnmatchedGlobalSuppressions(), errorLogger, settings.unmatchedSuppressionFilters);
325355
return err;
326356
}
327357

@@ -376,7 +406,7 @@ int CppCheckExecutor::check_internal(const Settings& settings, Suppressions& sup
376406
returnValue |= cppcheck.analyseWholeProgram(settings.buildDir, mFiles, mFileSettings, stdLogger.getCtuInfo());
377407

378408
if (settings.severity.isEnabled(Severity::information) || settings.checkConfiguration) {
379-
const bool err = reportSuppressions(settings, supprs.nomsg, settings.checks.isEnabled(Checks::unusedFunction), mFiles, mFileSettings, stdLogger);
409+
const bool err = reportUnmatchedSuppressions(settings, supprs.nomsg, mFiles, mFileSettings, stdLogger);
380410
if (err && returnValue == 0)
381411
returnValue = settings.exitCode;
382412
}

‎cli/cppcheckexecutor.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ class CppCheckExecutor {
7070

7171
protected:
7272

73-
static bool reportSuppressions(const Settings &settings, const SuppressionList& suppressions, bool unusedFunctionCheckEnabled, const std::list<FileWithDetails> &files, const std::list<FileSettings>& fileSettings, ErrorLogger& errorLogger);
73+
static bool reportUnmatchedSuppressions(const Settings &settings, const SuppressionList& suppressions, const std::list<FileWithDetails> &files, const std::list<FileSettings>& fileSettings, ErrorLogger& errorLogger);
7474

7575
/**
7676
* Wrapper around check_internal

‎lib/cppcheck.cpp‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1299,16 +1299,6 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
12991299
mErrorLogger.reportErr(errmsg);
13001300
}
13011301

1302-
// TODO: this is done too early causing the whole program analysis suppressions to be reported as unmatched
1303-
if (mSettings.severity.isEnabled(Severity::information) || mSettings.checkConfiguration) {
1304-
// In jointSuppressionReport mode, unmatched suppressions are
1305-
// collected after all files are processed
1306-
if (!mSettings.useSingleJob()) {
1307-
// TODO: check result?
1308-
SuppressionList::reportUnmatchedSuppressions(mSuppressions.nomsg.getUnmatchedLocalSuppressions(file, static_cast<bool>(mUnusedFunctionsCheck)), mErrorLogger);
1309-
}
1310-
}
1311-
13121302
if (analyzerInformation) {
13131303
mLogger->setAnalyzerInfo(nullptr);
13141304
analyzerInformation.reset();

‎lib/settings.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,9 @@ class CPPCHECKLIB WARN_UNUSED Settings {
435435
/** @brief The maximum time in seconds for the typedef simplification */
436436
std::size_t typedefMaxTime{};
437437

438+
/** @brief Error IDs which should not be reported as unmatchedSuppression */
439+
std::vector<std::string> unmatchedSuppressionFilters;
440+
438441
/** @brief defines given by the user */
439442
std::string userDefines;
440443

‎lib/suppressions.cpp‎

Lines changed: 3 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737

3838
#include "xml.h"
3939

40-
static const char ID_UNUSEDFUNCTION[] = "unusedFunction";
4140
static const char ID_CHECKERSREPORT[] = "checkersReport";
4241

4342
SuppressionList::ErrorMessage SuppressionList::ErrorMessage::fromErrorMessage(const ::ErrorMessage &msg, const std::set<std::string> &macroNames)
@@ -555,7 +554,7 @@ void SuppressionList::dump(std::ostream & out, const std::string& filePath) cons
555554
out << " </suppressions>" << std::endl;
556555
}
557556

558-
std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppressions(const FileWithDetails &file, const bool includeUnusedFunction) const
557+
std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppressions(const FileWithDetails &file) const
559558
{
560559
std::lock_guard<std::mutex> lg(mSuppressionsSync);
561560

@@ -573,16 +572,14 @@ std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppre
573572
continue;
574573
if (s.errorId == ID_CHECKERSREPORT)
575574
continue;
576-
if (!includeUnusedFunction && s.errorId == ID_UNUSEDFUNCTION)
577-
continue;
578575
if (!s.isLocal() || s.fileName != file.spath())
579576
continue;
580577
result.push_back(s);
581578
}
582579
return result;
583580
}
584581

585-
std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppressions(const bool includeUnusedFunction) const
582+
std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppressions() const
586583
{
587584
std::lock_guard<std::mutex> lg(mSuppressionsSync);
588585

@@ -596,8 +593,6 @@ std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppr
596593
continue;
597594
if (s.hash > 0)
598595
continue;
599-
if (!includeUnusedFunction && s.errorId == ID_UNUSEDFUNCTION)
600-
continue;
601596
if (s.errorId == ID_CHECKERSREPORT)
602597
continue;
603598
if (s.isLocal())
@@ -607,7 +602,7 @@ std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppr
607602
return result;
608603
}
609604

610-
std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedInlineSuppressions(const bool includeUnusedFunction) const
605+
std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedInlineSuppressions() const
611606
{
612607
std::list<SuppressionList::Suppression> result;
613608
for (const SuppressionList::Suppression &s : SuppressionList::mSuppressions) {
@@ -620,8 +615,6 @@ std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedInlineSuppr
620615
continue;
621616
if (s.hash > 0)
622617
continue;
623-
if (!includeUnusedFunction && s.errorId == ID_UNUSEDFUNCTION)
624-
continue;
625618
result.push_back(s);
626619
}
627620
return result;
@@ -660,39 +653,6 @@ void SuppressionList::markUnmatchedInlineSuppressionsAsChecked(const Tokenizer &
660653
}
661654
}
662655

663-
bool SuppressionList::reportUnmatchedSuppressions(const std::list<SuppressionList::Suppression> &unmatched, ErrorLogger &errorLogger)
664-
{
665-
bool err = false;
666-
// Report unmatched suppressions
667-
for (const SuppressionList::Suppression &s : unmatched) {
668-
// don't report "unmatchedSuppression" as unmatched
669-
if (s.errorId == "unmatchedSuppression")
670-
continue;
671-
672-
// check if this unmatched suppression is suppressed
673-
bool suppressed = false;
674-
for (const SuppressionList::Suppression &s2 : unmatched) {
675-
if (s2.errorId == "unmatchedSuppression") {
676-
if ((s2.fileName.empty() || s2.fileName == "*" || s2.fileName == s.fileName) &&
677-
(s2.lineNumber == SuppressionList::Suppression::NO_LINE || s2.lineNumber == s.lineNumber)) {
678-
suppressed = true;
679-
break;
680-
}
681-
}
682-
}
683-
684-
if (suppressed)
685-
continue;
686-
687-
std::list<::ErrorMessage::FileLocation> callStack;
688-
if (!s.fileName.empty())
689-
callStack.emplace_back(s.fileName, s.lineNumber, 0);
690-
errorLogger.reportErr(::ErrorMessage(std::move(callStack), "", Severity::information, "Unmatched suppression: " + s.errorId, "unmatchedSuppression", Certainty::normal));
691-
err = true;
692-
}
693-
return err;
694-
}
695-
696656
std::string SuppressionList::Suppression::toString() const
697657
{
698658
std::string s;

‎lib/suppressions.h‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434

3535
class Tokenizer;
3636
class ErrorMessage;
37-
class ErrorLogger;
3837
enum class Certainty : std::uint8_t;
3938
class FileWithDetails;
4039

@@ -258,19 +257,19 @@ class CPPCHECKLIB SuppressionList {
258257
* @brief Returns list of unmatched local (per-file) suppressions.
259258
* @return list of unmatched suppressions
260259
*/
261-
std::list<Suppression> getUnmatchedLocalSuppressions(const FileWithDetails &file, bool includeUnusedFunction) const;
260+
std::list<Suppression> getUnmatchedLocalSuppressions(const FileWithDetails &file) const;
262261

263262
/**
264263
* @brief Returns list of unmatched global (glob pattern) suppressions.
265264
* @return list of unmatched suppressions
266265
*/
267-
std::list<Suppression> getUnmatchedGlobalSuppressions(bool includeUnusedFunction) const;
266+
std::list<Suppression> getUnmatchedGlobalSuppressions() const;
268267

269268
/**
270269
* @brief Returns list of unmatched inline suppressions.
271270
* @return list of unmatched suppressions
272271
*/
273-
std::list<Suppression> getUnmatchedInlineSuppressions(bool includeUnusedFunction) const;
272+
std::list<Suppression> getUnmatchedInlineSuppressions() const;
274273

275274
/**
276275
* @brief Returns list of all suppressions.
@@ -283,13 +282,6 @@ class CPPCHECKLIB SuppressionList {
283282
*/
284283
void markUnmatchedInlineSuppressionsAsChecked(const Tokenizer &tokenizer);
285284

286-
/**
287-
* Report unmatched suppressions
288-
* @param unmatched list of unmatched suppressions (from Settings::Suppressions::getUnmatched(Local|Global)Suppressions)
289-
* @return true is returned if errors are reported
290-
*/
291-
static bool reportUnmatchedSuppressions(const std::list<SuppressionList::Suppression> &unmatched, ErrorLogger &errorLogger);
292-
293285
private:
294286
mutable std::mutex mSuppressionsSync;
295287
/** @brief List of error which the user doesn't want to see. */

‎test/cli/inline-suppress_test.py‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,3 +501,60 @@ def test_unmatched_cfg():
501501
]
502502
assert stdout == ''
503503
assert ret == 0, stdout
504+
505+
506+
# do not report unmatched unusedFunction inline suppressions when unusedFunction check is disabled
507+
# unusedFunction is disabled when -j2 is specified without a builddir
508+
def test_unused_function_disabled_unmatched_j():
509+
args = [
510+
'-q',
511+
'--template=simple',
512+
'--enable=warning,information',
513+
'--inline-suppr',
514+
'-j2',
515+
'--no-cppcheck-build-dir',
516+
'proj-inline-suppress/unusedFunctionUnmatched.cpp'
517+
]
518+
519+
ret, stdout, stderr = cppcheck(args, cwd=__script_dir)
520+
assert stderr.splitlines() == [
521+
'{}unusedFunctionUnmatched.cpp:5:0: information: Unmatched suppression: uninitvar [unmatchedSuppression]'.format(__proj_inline_suppres_path)
522+
]
523+
assert stdout == ''
524+
assert ret == 0, stdout
525+
526+
527+
# do not report unmatched misra-* inline suppressions when misra is not provided
528+
def test_misra_disabled_unmatched(): #13663
529+
args = [
530+
'-q',
531+
'--template=simple',
532+
'--enable=warning,information',
533+
'--inline-suppr',
534+
'proj-inline-suppress/misraUnmatched.c'
535+
]
536+
537+
ret, stdout, stderr = cppcheck(args, cwd=__script_dir)
538+
assert stderr.splitlines() == [
539+
'{}misraUnmatched.c:5:0: information: Unmatched suppression: uninitvar [unmatchedSuppression]'.format(__proj_inline_suppres_path)
540+
]
541+
assert stdout == ''
542+
assert ret == 0, stdout
543+
544+
545+
# do not report unmatched premium-* inline suppressions when application is not premium
546+
def test_premium_disabled_unmatched(): #13663
547+
args = [
548+
'-q',
549+
'--template=simple',
550+
'--enable=warning,information',
551+
'--inline-suppr',
552+
'proj-inline-suppress/premiumUnmatched.cpp'
553+
]
554+
555+
ret, stdout, stderr = cppcheck(args, cwd=__script_dir)
556+
assert stderr.splitlines() == [
557+
'{}premiumUnmatched.cpp:5:0: information: Unmatched suppression: uninitvar [unmatchedSuppression]'.format(__proj_inline_suppres_path)
558+
]
559+
assert stdout == ''
560+
assert ret == 0, stdout

0 commit comments

Comments
 (0)