Details
Diff Detail
Event Timeline
I just tried this out on a somewhat larger scale - I built all of CMake with this flag enabled, and it found a number of clang-tidy violations and added NOLINT comments to silence them. Worked like a charm.
Though I did find that it inserted more than one NOLINT comment for a templated function, probably due to multiple warnings being issued for the same violation.
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp | ||
---|---|---|
58 | Note that if the NOLINT is being inserted into a macro definition, we also need a backslash before the newline. | |
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h | ||
245 | Generally in Clang and adjacent projects we reserve the word "Type" to refer to C++ types. For other uses, we use a synonym - like "Kind" or "Mode". I think "FixMode" will work well here, WDYT? | |
251 | I think it would be better to use an enum class and avoid prefixing. (There are a lot of plain enums in Clang because Clang was originally developed in C++03.) | |
clang-tools-extra/test/clang-tidy/infrastructure/fix-mode-nolint.cpp | ||
2 ↗ | (On Diff #494031) | Could you add a second clang-tidy invocation over the fixed file to show that no warnings are produced? That would prove that the comments are inserted in the right place. (here and in the other test) |
7 ↗ | (On Diff #494031) | WDYT about making the comment style configurable? I think many people working in C++ will prefer //-style comments. |
18 ↗ | (On Diff #494031) | Could you add more tests, specifically tests with macros?
The SourceLocation computations are quite subtle with macros so these cases are worth covering. Note that when inserting NOLINTs into a macro definition, one has to use /* */-style comments. |
Generally in Clang and adjacent projects we reserve the word "Type" to refer to C++ types. For other uses, we use a synonym - like "Kind" or "Mode".
I think "FixMode" will work well here, WDYT?