[cfe-dev] [RFC][clang-tidy] Register warnings as check aliases

[Aaron Ballman via cfe-dev]

On Mon, Oct 3, 2016 at 4:47 AM, Gábor Horváth <xazax.hun at gmail.com> wrote:
>
>
> On 30 September 2016 at 16:14, Aaron Ballman <aaron at aaronballman.com> wrote:
>>
>> On Wed, Sep 28, 2016 at 3:44 AM, Gábor Horváth <xazax.hun at gmail.com>
>> wrote:
>> > Hi!
>> >
>> > I would like to propose that it should be possible to register compiler
>> > warnings as clang-tidy check aliases.
>>
>> I think this is an excellent idea!
>>
>> > As a motivating example, there is a CERT C++ secure coding rule:
>> > ERR54-CPP
>> > [1]
>> >
>> > This rule is covered by the clang warning: -Wexceptions
>> >
>> > So turning on this check in clang tidy would have two effects: turning
>> > on
>> > -Wexceptions and display the result of -Wexceptions as ERR54-CPP hits.
>>
>> How do you envision the diagnostics being reported? For instance,
>> would it be [cert-err54-cpp, -Wexceptions], [cert-err54-cpp], or
>> [-Wexceptions]?
>
>
> I think it should be either [cert-err54-cpp, -Wexceptions] or
> [cert-err54-cpp]. In the warning it should be clear that there is a CERT
> violation.

I've been thinking on this, and I think that we should print whatever
identifier is used to enable the diagnostic, because that will help
inform the user how to disable it. So, for instance, if the user runs:

clang-tidy -checks=-*,cert-* SomeFile.cpp -- -Wall

then the diagnostic should show [cert-err54-cpp, -Wexceptions] because
the user would need to disable *both* cert-err54-cpp and -Wexceptions
in order to avoid the diagnostic.

>> Also, do you envision this overriding a flag if it's disabled? e.g.,
>> would this diagnose, or silence the diagnostic?
>>
>> clang-tidy E:\SomeFile.cpp -checks=-*,cert-err54-cpp -- -std=c++14
>> -Wno-exceptions
>
>
> I would except the tidy flags to be "stronger" and overwrite the compilation
> flags. The compilations flags most of the time reflect the requirements of
> the builds and not the requirements of the additional static analysis. What
> do you think?

I agree. I think that if the user runs:

clang-tidy -checks=-*,cert-* SomeFile.cpp -- -Wall -Wno-exceptions

then the diagnostic should still be triggered, and show
[cert-err54-cpp, -Wexceptions]. I think the diagnostic should still
list -Wexceptions so that users know all the way the diagnostic can be
enabled, but maybe in an ideal world with color diagnostics, we could
print the -Wexceptions in grey (not even remotely a high priority;
possibly not even a good idea).

If they run:

clang-tidy -checks=-*,cert-dcl* SomeFile.cpp -- -Wall -Wno-exceptions

the diagnostic will be silenced entirely.

>>
>> > In my opinion aliases like this would be a great usability improvement:
>> >  - it would be easier to check the code against some coding guidelines.
>> >  - it would be easier to check what rules are already covered.
>> >  - it would be easier to find uncovered rules to implement.
>> >
>> > What do you think? Would you support a feature like that?
>>
>> I would love to see a feature like this, especially if it's something
>> users can configure themselves with some sort of file-based
>> configuration. This degree of flexibility would allow us to more
>> easily maintain common rulesets like CERT, MISRA, JSF++, C++ Core
>> Guidelines, etc while still giving users the ability to support custom
>> rulesets without modifying the Clang source.
>
> Do you mean registering an alias using a configuration file?

Yes, essentially. It would be nice if I could register a file that
defines all of the mappings for a check category (possibly even a
category that does not currently exist in clang-tidy, such as MISRA).
I'm not quite certain how you would associate this file with
clang-tidy, but I think we should have the ability to have default
mappings that require no user intervention to enable (for instance,
for CERT checks, since we already have a CERT category) that we can
provide, and ad hoc mappings that the user can pass to clang-tidy (for
instance, to enforce some local coding standards on a per-instance
basis).

~Aaron