llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.14k stars 11.11k forks source link

[clang-tidy] Add support for check comments in config file #64343

Open PiotrZSL opened 11 months ago

PiotrZSL commented 11 months ago

Example:

Checks: |
    -*,
    clang-diagnostic-*,

    boost-*,
    bugprone-*,
    -bugprone-narrowing-conversions, #cppcoreguidelines-narrowing-conversions
    -bugprone-easily-swappable-parameters,
    -bugprone-unchecked-optional-access,

    cert-*,
    -cert-con36-c,   #bugprone-spuriously-wake-up-functions
    -cert-con54-cpp, #misc-new-delete-overloads
    -cert-dcl21-cpp,
    -cert-dcl37-c,   #bugprone-reserved-identifier
    -cert-dcl51-cpp, #bugprone-reserved-identifier
    -cert-err09-cpp, #misc-throw-by-value-catch-by-reference
    -cert-err33-c,   #bugprone-unused-return-value
    -cert-err58-cpp,
    -cert-err61-cpp, #misc-throw-by-value-catch-by-reference
    -cert-msc30-c,   #cert-msc50-cpp
    -cert-msc32-c,   #cert-msc51-cpp

This work, but --verify-config prints:

command-line option '-config': warning: unknown check '#bugprone-spuriously-wake-up-functions
-cert-con54-cpp' [-verify-config]
command-line option '-config': warning: unknown check '#misc-new-delete-overloads
-cert-dcl21-cpp' [-verify-config]
command-line option '-config': warning: unknown check '#bugprone-reserved-identifier
-cert-dcl51-cpp' [-verify-config]
command-line option '-config': warning: unknown check '#bugprone-reserved-identifier
-cert-err09-cpp' [-verify-config]
command-line option '-config': warning: unknown check '#misc-throw-by-value-catch-by-reference
-cert-err33-c' [-verify-config]
command-line option '-config': warning: unknown check '#bugprone-unused-return-value
-cert-err58-cpp' [-verify-config]

Would be nice to have some native support for comments, like anything after # character is ignored till the end of line.

llvmbot commented 11 months ago

@llvm/issue-subscribers-clang-tidy

carlosgalvezp commented 11 months ago

You can achieve that moving to the native YAML list syntax, without changes to clang-tidy:

Checks: [
    -*,
    bugprone-*,
    -bugprone-narrowing-conversions, #cppcoreguidelines-narrowing-conversions
]

This way, the comment is no longer part of the string and not counted as a check. I'm not sure we should support this functionality on the legacy format.

PiotrZSL commented 11 months ago
.clang-tidy:4:5: error: unexpected scalar
    -*,
    ^
Error: invalid configuration specified.

Does not work.

carlosgalvezp commented 11 months ago

Did you add the brackets? Works for me...

$ cat .clang-tidy 
Checks: [
  -*,
  bugprone-*,
  -bugprone-narrowing-conversions, #cppcoreguidelines-narrowing-conversions
]
$ cat foo.cpp 
int main()
{
  int i = 0;
  while(i < 10){}
}
$ clang-tidy --version
LLVM (http://llvm.org/):
  LLVM version 18.0.0git
  Optimized build.
$ clang-tidy foo.cpp --
1 warning generated.
/tmp/foo.cpp:4:3: warning: this loop is infinite; none of its condition variables (i) are updated in the loop body [bugprone-infinite-loop]
    4 |   while(i < 10){}
      |   ^
$ clang-tidy --verify-config
No config errors detected.
PiotrZSL commented 11 months ago

Yes, I added, looks like this syntax isn't supported in Clang-tidy 16 (the one I'm using).

carlosgalvezp commented 11 months ago

Ah yes, this was added on clang-tidy 17. But anyway the proposed fix in the OP would be available from clang-tidy 18, at which point it's better to just move to the new format instead?

PiotrZSL commented 11 months ago

Yes it could be, I only mention this issue because it is a little bit problematic currently, when I looked into function that does parsing, it shouldn't be hard to add such comment exclude.

Unless we deprecate "string syntax", this improvement would be nice, but if we deprecate it, then probably we need to change also --dump-config to dump a list of checks, instead of string.

zjyhjqs commented 10 months ago

You can achieve that moving to the native YAML list syntax, without changes to clang-tidy:

Checks: [
    -*,
    bugprone-*,
    -bugprone-narrowing-conversions, #cppcoreguidelines-narrowing-conversions
]

This way, the comment is no longer part of the string and not counted as a check. I'm not sure we should support this functionality on the legacy format.

Checks works, but WarningsAsErrors not.

% clang-tidy --version
Homebrew LLVM version 17.0.1
  Optimized build.
% cat .clang-tidy
WarningsAsErrors: [
  bugprone-*
]%                                     
% clang-tidy --verify-config
/Users/zjyhjqs/Codes/MyConfigurations/clang-tools-extra/.clang-tidy:2:3: error: unexpected scalar
  bugprone-*
  ^
Error parsing /Users/zjyhjqs/Codes/MyConfigurations/clang-tools-extra/.clang-tidy: Invalid argument
/Users/zjyhjqs/Codes/MyConfigurations/clang-tools-extra/.clang-tidy:2:3: error: unexpected scalar
  bugprone-*
  ^
Error parsing /Users/zjyhjqs/Codes/MyConfigurations/clang-tools-extra/.clang-tidy: Invalid argument
No config errors detected.
carlosgalvezp commented 10 months ago

We can add support for that too, could you open a new ticket for the and close this one?

zjyhjqs commented 9 months ago

We can add support for that too, could you open a new ticket for the and close this one?

I am not the creator of this issue so I don't have the permission to directly close it.

And the title is Add support for check comments in config file. I think this one is included too?

alexolog commented 4 months ago

I am trying to allow all checks except for those that are explicitly disabled. It works with the string format, but does not work with the YAML format.

Checks: [
  *, # <-- Include all checks (this does not seem to work)
  -altera-*,
  -android-*,
  -fuchsia-*,
  -llvm-*,
  -llvmlibc-*,
  -objc-*,

  # more exclusions here...
]

Please advise.

PiotrZSL commented 4 months ago

@alexolog

Checks: [
  "*",
  -altera-*,
  -android-*,
  -fuchsia-*,
  -llvm-*,
  -llvmlibc-*,
  -objc-*,

  # more exclusions here...
]
alexolog commented 4 months ago

Thanks @PiotrZSL !