Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow suppressing a checker on a source file level #2339

Open
whisperity opened this issue Sep 27, 2019 · 1 comment
Open

Allow suppressing a checker on a source file level #2339

whisperity opened this issue Sep 27, 2019 · 1 comment
Labels
analyzer 📈 Related to the analyze commands (analysis driver) enhancement 🌟 usability 👍 Usability-related features

Comments

@whisperity
Copy link
Contributor

Currently, the // codechecker_suppress mechanics only allow suppressing a report at a particular location. Sometimes, multiple reports of the almost exact same kind, for the exact same reason, can be present multiple times in the same file.

One striking example might be GoogleTest's way of initialising the test type and code via a macro.

See this case snippet:
cert-err58-cpp result due to GoogleTest macro doing something weird

I'm not sure if suppressing all these reports via a line above TEST(...) is possible, but even if it is, it'd be really annoying to keep copying these lines.

A better solution seems to be to allow suppressing a report for the entire file. Granular scripts which call different CodeChecker analyze calls for parts of the project could be a solution here, but that would be immensely hard to maintain.

The above example is from JetBrains' CLion which has a built-in Clang-Tidy invoker and report shower. Clicking on More options... shows a button where a lot of configuration shows up, one of which is labelled Disable "cert-err58-cpp" for file. CLion automatically reformats the file to add a few #pragma lines and configuration which are not ever picked up by the compiler during compilation, but is either "parsed" via the IDE, or by Clang-Tidy itself to not show this particular warning.

(I'm leaning more on the "parsed by IDE" side, as calling Clang-Tidy on a file with these extra pragmas in still results in the report shown when viewed via CodeChecker.)

Note TEST not being underlined anymore:

The particular issue is not reported anymore.

Perhaps // codechecker_suppress could be extended so that it may appear somewhere in a file, marking that reports that end in the said file should all be considered ignored (with optional comment, etc. as the usual feature set.)
For example, I'd be happy to preamble all my test files (could even set an IDE template so when I create a new C++ Test source file it is inserted automatically) with this:

#pragma clang diagnostic push
#pragma ide diagnostic ignored "cert-err58-cpp"

// codechecker ignored [cert-err58-cpp] {sourcefile}
// Test code, if there's not enough space to allocate the
// strings needed to test, just bail out.

/* ... TEMPLATE: Put real test code here. ... */

#pragma clang diagnostic pop

Another idea that could be considered for suppressions is how Clang Format works: you can put // clang-format off // clang-format on markers in the code, and between the off and on part, the formatting does not touch your code, even if it is badly formatted.
Similar could be done with suppressions, if one might wants to ignore an entire code block – however, this is not appropriate here: I only want to not care about the static initialisation yadda for the test class bodies, I'd still be happy to see other kinds of issues that might be present in the test codes I've written.

@whisperity whisperity added enhancement 🌟 analyzer 📈 Related to the analyze commands (analysis driver) usability 👍 Usability-related features labels Sep 27, 2019
@ebarcsa
Copy link

ebarcsa commented Aug 25, 2021

Hi,
I support this request as it would fix use cases when legacy / 3rd pty library is used and even if there was chance to refactor,
it would take too much time, compared to original commit, it would be easier to whitelist for time, a single or multiple checks.
So the outcome now usually to disable that check for entire project which is opposite of what we actually want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer 📈 Related to the analyze commands (analysis driver) enhancement 🌟 usability 👍 Usability-related features
Projects
None yet
Development

No branches or pull requests

2 participants