[PATCH] D61487: [clang-tidy] Make the plugin honor NOLINT

[Nikolai Kosjar via Phabricator via cfe-commits]

nik marked 4 inline comments as done.
nik added a comment.

As I've commented on, this change is not finished. However, I've addressed the inline comments nevertheless.

There is one TODO left for which I would like to have an opinion.



================
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:472
   if (Info.hasSourceManager())
     checkFilters(Info.getLocation(), Info.getSourceManager());
 }
----------------
gribozavr wrote:
> It seems like the `checkFilters` call should not be skipped even if we have another diagnostic engine.
checkFilters() sets some state so that later finalizeLastError() can remove diagnostics/errors from ClangTidyDiagnosticConsumer::Errors and also track some statistics.

Statistics are not relevant for the pluginc case as they should not be printed out for that case.
The filtering happening in finalizeLastError() does not look relevant as the plugin currently only supports the "-checks=..." argument but not the e.g. header and line filter. When checks are created in ClangTidyCheckFactories::createChecks, the "-checks=..." argument is honored, so that the !Context.isCheckEnabled(...) in finalizeLastError() is also not relevant.

I agree that checkFilters()/finalizeLastError() will be needed once the plugin case should support the other filtering options (header + line filter), but note that this goes beyond the scope of this change here, which is NOLINT filtering.

This is all new to me, so if I miss something regarding the checkFilters()/finalizeLastError() thingy, please tell me, preferably with an example if possible :)


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61487/new/

https://reviews.llvm.org/D61487