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

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz added a comment.

In D108560#3012057 <https://reviews.llvm.org/D108560#3012057>, @aaron.ballman wrote:

> Thanks, I think this is getting close! There are two more test cases that I think are interesting (and should cause any issues, hopefully):
>
>   // NOLINTEND
>   // CHECK-MESSAGES: for diagnostic on the previous line
>
> and
>
>   // CHECK-MESSAGES: for diagnostic on the subsequent line
>   // NOLINTBEGIN
>
> as the only contents in the file. The idea is that we want to check that searching for a "begin" when seeing an "end" at the top of the file doesn't do anything silly like try to read off the start of the file, and similar for "end" when seeing a "begin" at the end of a file.

The good news is that the program does not do anything silly like read off the boundaries of the file. :-)
What is noteworthy, however, is that in `test/clang-tidy/infrastructure/nolintbeginend-begin-at-eof.cpp` only the original clang-tidy check diag is shown, not the diag about the unmatched NOLINTBEGIN. This is because of the early exit in `lineIsWithinNolintBegin()`:

  // Check if there's an open NOLINT(BEGIN...END) block on the previous lines.
  // ...
  auto Error = tallyNolintBegins(Context, SM, CheckName, PrevLines,
                                 FileStartLoc, NolintBegins);
  // ...
  if (NolintBegins.empty())
    return false;
  
  // Check that every block is terminated correctly on the following lines.
  // ...

This has been fixed in the patch I just posted.

As for your latest message about NOLINTBEGIN(check) ... NOLINTEND(other-check), I'll have a think about it and get back to you in a day or two.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108560