Skip to content

Fix/filter out duplicate suppressions - #96

Closed
davidramnero wants to merge 7 commits into
mainfrom
fix/filter-out-duplicate-suppressions
Closed

Fix/filter out duplicate suppressions#96
davidramnero wants to merge 7 commits into
mainfrom
fix/filter-out-duplicate-suppressions

Conversation

@davidramnero

Copy link
Copy Markdown
Collaborator

No description provided.

Comment thread src/extension.ts
// Duplicate suppressions returns an error from cppcheck, so for ease of use we filter these out
const projectFileSuppressions = await parseSuppressionsFromProjectFile(cppcheckProjectFileUri);
for (const projectFileSuppression of projectFileSuppressions) {
const duplicateSuppressionIndex = args.findIndex((a) => a === `--suppress=${projectFileSuppression}`);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unfortunately it's not enough to look at the id, here is the code to detect duplicates in cppcheck:

        bool isSameParameters(const Suppression &other) const {
            return errorId == other.errorId &&
                   fileName == other.fileName &&
                   lineNumber == other.lineNumber &&
                   symbolName == other.symbolName &&
                   hash == other.hash &&
                   thisAndNextLine == other.thisAndNextLine;
        }

Only id, file and line number can be supplied on the command line though, so should be enough to check those.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These are the possible formats for --suppress=:

[error id]:[filename]:[line]
[error id]:[filename2]
[error id]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It's maybe a bit unnecessary to have this logic in both places though, I think it would be good to have a --ignore-duplicate-suppressions option instead. I can create a ticket.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

okay makes sense, I'll close this PR for now then

@davidramnero

Copy link
Copy Markdown
Collaborator Author

we'll sort duplicates handling in cppcheck instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants