Skip to content

Commit 9e30245

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

File tree

12 files changed

+244
-133
lines changed

12 files changed

+244
-133
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 & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -293,35 +293,64 @@ 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) {
319-
// report unmatched unusedFunction suppressions
320-
err |= SuppressionList::reportUnmatchedSuppressions(
321-
suppressions.getUnmatchedInlineSuppressions(unusedFunctionCheckEnabled), errorLogger);
350+
err |= ::reportUnmatchedSuppressions(suppressions.getUnmatchedInlineSuppressions(), errorLogger, settings.unmatchedSuppressionFilters);
322351
}
323352

324-
err |= SuppressionList::reportUnmatchedSuppressions(suppressions.getUnmatchedGlobalSuppressions(unusedFunctionCheckEnabled), errorLogger);
353+
err |= ::reportUnmatchedSuppressions(suppressions.getUnmatchedGlobalSuppressions(), errorLogger, settings.unmatchedSuppressionFilters);
325354
return err;
326355
}
327356

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

378407
if (settings.severity.isEnabled(Severity::information) || settings.checkConfiguration) {
379-
const bool err = reportSuppressions(settings, supprs.nomsg, settings.checks.isEnabled(Checks::unusedFunction), mFiles, mFileSettings, stdLogger);
408+
const bool err = reportUnmatchedSuppressions(settings, supprs.nomsg, mFiles, mFileSettings, stdLogger);
380409
if (err && returnValue == 0)
381410
returnValue = settings.exitCode;
382411
}

‎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: 6 additions & 45 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)
@@ -312,8 +311,9 @@ bool SuppressionList::updateSuppressionState(const SuppressionList::Suppression&
312311
auto foundSuppression = std::find_if(mSuppressions.begin(), mSuppressions.end(),
313312
std::bind(&Suppression::isSameParameters, &suppression, std::placeholders::_1));
314313
if (foundSuppression != mSuppressions.end()) {
315-
// Update state of existing global suppression
316-
if (!suppression.isLocal()) {
314+
// Update state of existing suppressions
315+
//if (!suppression.isLocal())
316+
{
317317
if (suppression.checked)
318318
foundSuppression->checked = true;
319319
if (suppression.matched)
@@ -555,7 +555,7 @@ void SuppressionList::dump(std::ostream & out, const std::string& filePath) cons
555555
out << " </suppressions>" << std::endl;
556556
}
557557

558-
std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppressions(const FileWithDetails &file, const bool includeUnusedFunction) const
558+
std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppressions(const FileWithDetails &file) const
559559
{
560560
std::lock_guard<std::mutex> lg(mSuppressionsSync);
561561

@@ -573,16 +573,14 @@ std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppre
573573
continue;
574574
if (s.errorId == ID_CHECKERSREPORT)
575575
continue;
576-
if (!includeUnusedFunction && s.errorId == ID_UNUSEDFUNCTION)
577-
continue;
578576
if (!s.isLocal() || s.fileName != file.spath())
579577
continue;
580578
result.push_back(s);
581579
}
582580
return result;
583581
}
584582

585-
std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppressions(const bool includeUnusedFunction) const
583+
std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppressions() const
586584
{
587585
std::lock_guard<std::mutex> lg(mSuppressionsSync);
588586

@@ -596,8 +594,6 @@ std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppr
596594
continue;
597595
if (s.hash > 0)
598596
continue;
599-
if (!includeUnusedFunction && s.errorId == ID_UNUSEDFUNCTION)
600-
continue;
601597
if (s.errorId == ID_CHECKERSREPORT)
602598
continue;
603599
if (s.isLocal())
@@ -607,7 +603,7 @@ std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppr
607603
return result;
608604
}
609605

610-
std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedInlineSuppressions(const bool includeUnusedFunction) const
606+
std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedInlineSuppressions() const
611607
{
612608
std::list<SuppressionList::Suppression> result;
613609
for (const SuppressionList::Suppression &s : SuppressionList::mSuppressions) {
@@ -620,8 +616,6 @@ std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedInlineSuppr
620616
continue;
621617
if (s.hash > 0)
622618
continue;
623-
if (!includeUnusedFunction && s.errorId == ID_UNUSEDFUNCTION)
624-
continue;
625619
result.push_back(s);
626620
}
627621
return result;
@@ -660,39 +654,6 @@ void SuppressionList::markUnmatchedInlineSuppressionsAsChecked(const Tokenizer &
660654
}
661655
}
662656

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-
696657
std::string SuppressionList::Suppression::toString() const
697658
{
698659
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)