[PATCH] D108560: [clang-tidy] Add support for NOLINTBEGIN ... NOLINTEND comments to suppress clang-tidy warnings over multiple lines

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Sorry for not thinking of this sooner, but there is another diagnostic we might want to consider.

  // NOLINTBEGIN(check)
  // NOLINTEND(other-check)

where the file does not contain a `NOLINTEND(check)` comment anywhere. In this case, the markers are not actually matched, so it's a `NOLINTBEGIN(check)` comment with no end and a `NOLINTEND(other-check)` comment with no begin. That seems like user confusion we'd also want to diagnose, but it also could be tricky because you really want to add diagnostic arguments for the check name. My concern here is mostly with catching typos where the user types `check` in the first and `chekc` in the second, so if we wanted to use the edit distance between what was provided and the list of check names to provide a fix-it, that would be very handy (and also probably difficult to implement so I definitely don't insist on a fix-it).

Relatedly, I think this construct is perhaps fine:

  // NOLINTBEGIN(check)
  // NOLINTEND(*)

because the end "covers" the begin. I'm a bit less clear on this though:

  // NOLINTBEGIN(*)
  // NOLINTEND(check)

because the begin is not fully covered by the end. However, I'm a bit less clear on what should or should not be diagnosed here, so if we wanted to leave that for follow-up work, that'd be fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108560