[clang-tidy] Support specific checks for NOLINT directive

[xgsa]

Summary

The NOLINT directive was extended to support the "NOLINT(check)" and "NOLINT(*)" syntax. Also it is possible to specify a few checks separated with comma "NOLINT(check1, check2)"

[lebedev.ri]

Please upload patches with full context (-U999999)
Also, you probably want to add some reviewers, see CODE_OWNERS.txt e.g.

[xgsa]

Update the diff to contain the full context

[JonasToth]

Could you please explain what category means? Could i disable all of cppcoreguidelines with something like // NOLINT (cppcoreguidelines-*)?

[JonasToth]

clang-tidy/ClangTidyDiagnosticConsumer.cpp

298

Maybe some comments whats happening might be helpfull. It would simplify later modifications.

test/clang-tidy/nolintnextline.cpp

14

missing )

[xgsa]

In D40671#941676, @JonasToth wrote:

Could you please explain what category means? Could i disable all of cppcoreguidelines with something like // NOLINT (cppcoreguidelines-*)?

No, only individual checks can be disabled. You are right, by "categories" I meant just "checks". Sorry for confusion. I'll update the description.

test/clang-tidy/nolintnextline.cpp

14

No, it's intentionally: it's a test for case when ) is missing

[JonasToth]

test/clang-tidy/nolintnextline.cpp

14

It was early, i didn't read properly. Sorry for that.

A testcase for where all parens are missing would be nice, too. I assume that everything will be ignored, is that correct?

[xgsa]

test/clang-tidy/nolintnextline.cpp

14

There is such test case (for class "B") or did you mean something else?

[xgsa]

Add comments to code as it was recommended.

[JonasToth]

test/clang-tidy/nolintnextline.cpp

14

No i mean an errornous NOLINT.

Like // NOLINT some_check, othercheck, blaa. One might expect it to work, but the missing parens would inhibit that expected behaviour.

[xgsa]

A few additional test cases were added.

[aaron.ballman]

clang-tidy/ClangTidyDiagnosticConsumer.cpp

296

Only use auto when the type is spelled out explicitly in the initialization (usually through a cast or constructor). Same comment applies elsewhere.

312–314

llvm::transform(Checks, Checks.begin(), [](StringRef S) { return S.trim(); });

315

Can use llvm::find(Checks, CheckName)

[Eugene.Zelenko]

clang-tidy/ClangTidyDiagnosticConsumer.cpp

294–325

Please separate with empty line.

324

Please separate with empty line.

[aaron.ballman]

This feature should probably be mentioned in the release notes.

[xgsa]

An item to release notes was added.

Also I have added a paragraph about NOLINT to the main documentation page, because I suppose it's useful information and it's related to the feature. Please, let me know if it should be added with a separate commit or shouldn't be added at all.

[xgsa]

Minor change: update default value of SmallVector of check names.

[lebedev.ri]

docs/ReleaseNotes.rst

259

This reads strange, maybe it can be reworded somehow?

[xgsa]

Release note item was reworded

[JonasToth]

Its good that you added code examples to the clang-tidy documentation page. I think that feature was not documented well before.

test/clang-tidy/nolintnextline.cpp

23

Ian confused now. The NOLINTNEXTLINE with incorrect parents should not silence the diagnostic, should it?

In my understanding the following line should cause the explicit constructor check to warn. Is that check message missing or did I get something wrong?

[xgsa]

test/clang-tidy/nolintnextline.cpp

23

Without parentheses, it works just as NOLINTNEXTLINE (i.e. suppresses all the diagnostics for line), because it's impossible to distinguish check names from user comments after NOLINTNEXTLINE:

// NOLINTNEXTLINE check-name, another-check
// NOLINTNEXTLINE Some description, why the suppression is added

[JonasToth]

test/clang-tidy/nolintnextline.cpp

23

Ah sure, that makes sense.

[JonasToth]

I am happy with everything now. But one of the reviewers must accept.

[aaron.ballman]

clang-tidy/ClangTidyDiagnosticConsumer.cpp

299

Comments should be complete sentences, including punctuation (here and elsewhere).

docs/ReleaseNotes.rst

259–260

How about: "Added the ability to suppress specific checks (or all checks) in a NOLINT or NOLINTNEXTLINE comment."?

docs/clang-tidy/index.rst

253

I don't agree with that initial statement's use of "generally" -- checks that are chatty live in clang-tidy, as are checks for various coding standards (which commonly have a deviation mechanism). Also, I don't think we should encourage users to unconditionally report false positives as bugs; many of the coding standard related checks provide true positives that are annoying and users may want to deviate in certain circumstances (like CERT's rule banning use of rand() or system()). I would reword this to:

While clang-tidy diagnostics are intended to call out code that does not adhere to a coding standard, or is otherwise problematic in some way, there are times when it is more appropriate to silence the diagnostic instead of changing the semantics of the code. In such circumstances, the NOLINT or NOLINTNEXTLINE comments can be used to silence the diagnostic. For example:

I would also describe the comment syntax more formally as (my markdown may be incorrect, you should ensure this renders sensibly), with surrounding prose:

*lint-comment:*
  *lint-command* *lint-args~opt~*
  
*lint-args:*
  `(` *check-name-list* `)`

*check-name-list:*
  *check-name*
  *check-name-list* `,` *check-name*

*lint-command:*
  `NOLINT`
  `NOLINTNEXTLINE`

Specific to the prose mentioned above, you should document where the feature is tolerant to whitespace (can there be a space between NOLINT and the parens, what about inside the parens, how about after or before commas, etc).

270

Is the space before the ( intended?

[xgsa]

docs/clang-tidy/index.rst

270

No, it's a mistake, thank you.

[xgsa]

Updated documentation and comments in code.

[xgsa]

A typo in documentation was fixed.

[aaron.ballman]

Aside from a minor commenting nit, this LGTM. Thanks!

docs/clang-tidy/index.rst

280

This should have a subscript opt following lint-args to denote that the args are optional. I think you can achieve that with:

lint-args\ :sub:`opt`

[xgsa]

docs/clang-tidy/index.rst

280

Sorry, I am not very familiar with Sphinx, but

\ :sub:`opt`

seems doesn't work inside parsed-literal block (I have searched through the llvm docs and found a few similar places, which uses such construction in parsed-literal block, and it is not rendered properly even on official web site).
Here parsed-literal block is used to render a quote-like block with custom formatting, so the whole grammar is shown as a single entity, and I cannot achieve this with the other constructs.
To show that lint-args is optional I split this grammar rule into 2:

lint-command
lint-command lint-args

Isn't it enough or are there any suggestions how to handle this?

[alexfh]

docs/clang-tidy/index.rst

253

One little request from me: the documentation should recommend using check-specific ways to mute diagnostics, if they are available (e.g. bugprone-use-after-move can be silenced by re-initializing the variable after it has been moved out, misc-string-integer-assignment can be suppressed by explicitly casting the integer to char, readability-implicit-bool-conversion can also be suppressed by using explicit casts, etc.). NOLINT(NEXTLINE)? should be treated as the last resort, when fixing the code is infeasible or impractical and there's no diagnostic-specific suppression mechanism available.

[alexfh]

clang-tidy/ClangTidyDiagnosticConsumer.cpp

294–325

"NOLINT" is not a "macro". Maybe NolintDirective, NolintSpelling, NolintWord or something like this?

297

Please use early returns instead of nested ifs. http://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code

[xgsa]

docs/clang-tidy/index.rst

253

Thank you for the examples of the check-specific mute ways, I have looked for that, but haven't found the good ones.

[aaron.ballman]

docs/clang-tidy/index.rst

254–255

I would reword this somewhat now. I would put a period before ", there are times" and then move that whole "there are times" clause below.

256

s/preferable/preferred

257

s/mute/silence

261

At the end of this sentence, I would put "However, there are times". e.g.,

However, there are times when it is more appropriate to silence the diagnostic instead of changing the semantics of the code. In these circumstances, the NOLINT or NOLINTNEXTLINE comments can be used to silence the diagnostic.

280

I think that's sufficient, thanks!

[alexfh]

BTW, how will this feature interact with cpplint.py's way of handling specific NOLINT directives that use different lint rule names, which sometimes refer to the same rule (e.g. // NOLINT(runtime/explicit) suppresses the runtime/explicit cpplint rule that enforces the same style rule as the google-runtime-explicit check)?

[xgsa]

docs/clang-tidy/index.rst

254–255

I tried to move the "there are times"-clause below, but in this case "The preferable way of doing this is using..." becomes unclear, because it tries to explain the way of doing something without naming the purpose that is expected to achieve. So I suppose, it is necessary to place "However, there are times when it is more appropriate to silence the diagnostic instead of changing the semantics of the code" here. Am I missing something?

[xgsa]

Updated documentation

[xgsa]

In D40671#944906, @alexfh wrote:

BTW, how will this feature interact with cpplint.py's way of handling specific NOLINT directives that use different lint rule names, which sometimes refer to the same rule (e.g. // NOLINT(runtime/explicit) suppresses the runtime/explicit cpplint rule that enforces the same style rule as the google-runtime-explicit check)?

This feature accepts only full check names.

[xgsa]

So can this patch be submitted? Should I do something to make it happen?

[aaron.ballman]

In D40671#947762, @xgsa wrote:

So can this patch be submitted? Should I do something to make it happen?

There are still some outstanding concerns around the documentation wording, but once those are resolved it should be ready to commit. If you don't have commit access, I can commit on your behalf -- just let me know.

docs/clang-tidy/index.rst

254–255

The current formatting of this flows strangely. It starts by saying clang-tidy calls out problems, says there are times when it is more appropriate to silence the diagnostic without changing the code, then says the preferred way is to change the code, then says you can NOLINT it if you don't want to change the code.

I'd prefer if the flow was: clang-tidy calls out problems, the preferred way is to change the code, but there are still times when changing the code is not helpful and thus you can use NOLINT.

[xgsa]

In D40671#948826, @aaron.ballman wrote:

There are still some outstanding concerns around the documentation wording, but once those are resolved it should be ready to commit. If you don't have commit access, I can commit on your behalf -- just let me know.

Yes, I don't have commit access, so I need your help with this, Aaron. Thank you!

[xgsa]

Documentation update

[alexfh]

In D40671#945158, @xgsa wrote:

In D40671#944906, @alexfh wrote:

BTW, how will this feature interact with cpplint.py's way of handling specific NOLINT directives that use different lint rule names, which sometimes refer to the same rule (e.g. // NOLINT(runtime/explicit) suppresses the runtime/explicit cpplint rule that enforces the same style rule as the google-runtime-explicit check)?

This feature accepts only full check names.

How are unknown check names handled? More specifically: will the // NOLINT(runtime/explicit) comment disable all clang-tidy checks or none?

[xgsa]

In D40671#949687, @alexfh wrote:

How are unknown check names handled? More specifically: will the // NOLINT(runtime/explicit) comment disable all clang-tidy checks or none?

None. If comment is syntactically correct and contains parenthesis, it works only for the known checks inside.

Still, I don't think it worth mentioning all the corner cases in documentation to keep things simple.

[xgsa]

@aaron.ballman, sorry for my insistence, but it seems all the comments are fixed and the patch is ready for commit or am I missing something? Could you please commit it on my behalf, as I don't have rights to do that?

[aaron.ballman]

In D40671#953291, @xgsa wrote:

@aaron.ballman, sorry for my insistence, but it seems all the comments are fixed and the patch is ready for commit or am I missing something? Could you please commit it on my behalf, as I don't have rights to do that?

The check now LGTM, but I am going to wait to commit in case @alexfh has concerns regarding unknown check names.

FWIW, I think we should do something about unknown check names in NOLINT comments, but that can be done as a follow-up patch. If we're ignoring the comment, we might want to diagnose that fact so users have an idea what's going on.

[alexfh]

In D40671#949732, @xgsa wrote:

In D40671#949687, @alexfh wrote:

How are unknown check names handled? More specifically: will the // NOLINT(runtime/explicit) comment disable all clang-tidy checks or none?

None. If comment is syntactically correct and contains parenthesis, it works only for the known checks inside.

Still, I don't think it worth mentioning all the corner cases in documentation to keep things simple.

Documenting interaction with cpplint-style NOLINT categories would potentially save time to the users and clang-tidy maintainers (at least for codebases using Google style and cpplint). Fine for a follow-up.

In D40671#953888, @aaron.ballman wrote:

In D40671#953291, @xgsa wrote:

@aaron.ballman, sorry for my insistence, but it seems all the comments are fixed and the patch is ready for commit or am I missing something? Could you please commit it on my behalf, as I don't have rights to do that?

The check now LGTM, but I am going to wait to commit in case @alexfh has concerns regarding unknown check names.

FWIW, I think we should do something about unknown check names in NOLINT comments, but that can be done as a follow-up patch. If we're ignoring the comment, we might want to diagnose that fact so users have an idea what's going on.

IIUC, cpplint can output a diagnostic about unknown categories inside NOLINT and about NOLINT directives that happen on lines where no warning is emitted. Both would be useful in clang-tidy, IMO.

[xgsa]

In D40671#954661, @alexfh wrote:

In D40671#953888, @aaron.ballman wrote:

FWIW, I think we should do something about unknown check names in NOLINT comments, but that can be done as a follow-up patch. If we're ignoring the comment, we might want to diagnose that fact so users have an idea what's going on.

IIUC, cpplint can output a diagnostic about unknown categories inside NOLINT and about NOLINT directives that happen on lines where no warning is emitted. Both would be useful in clang-tidy, IMO.

I agree with your statements and I think there should be the following diagnostics about NOLINT usage:

• as you described, using of NOLINT with unknown check names;
• using of NOLINT for the line, on which there is no diagnostics (at all with NOLINT and for the swpecified diagnostics); this should help to detect dangling NOLINT comments, that have no meaning anymore.

Moreover, there should be a way to turn on/off these diagnostics, so possibily they should be a separate checks. What do you think? Is there a way for a check to collect the emitted diagnostics?

[aaron.ballman]

In D40671#954906, @xgsa wrote:

In D40671#954661, @alexfh wrote:

In D40671#953888, @aaron.ballman wrote:

FWIW, I think we should do something about unknown check names in NOLINT comments, but that can be done as a follow-up patch. If we're ignoring the comment, we might want to diagnose that fact so users have an idea what's going on.

IIUC, cpplint can output a diagnostic about unknown categories inside NOLINT and about NOLINT directives that happen on lines where no warning is emitted. Both would be useful in clang-tidy, IMO.

I agree with your statements and I think there should be the following diagnostics about NOLINT usage:

• as you described, using of NOLINT with unknown check names;
• using of NOLINT for the line, on which there is no diagnostics (at all with NOLINT and for the swpecified diagnostics); this should help to detect dangling NOLINT comments, that have no meaning anymore.

Moreover, there should be a way to turn on/off these diagnostics, so possibily they should be a separate checks. What do you think? Is there a way for a check to collect the emitted diagnostics?

I think this is desirable functionality and can be done in follow-up patches. NOLINT of unknown check names should be pretty easy, but detecting NOLINT comments on lines that do not trigger the specified diagnostic may be a bit more tricky.

[aaron.ballman]

Committed in r320713.

[alexfh]

In D40671#954906, @xgsa wrote:

In D40671#954661, @alexfh wrote:

In D40671#953888, @aaron.ballman wrote:

FWIW, I think we should do something about unknown check names in NOLINT comments, but that can be done as a follow-up patch. If we're ignoring the comment, we might want to diagnose that fact so users have an idea what's going on.

IIUC, cpplint can output a diagnostic about unknown categories inside NOLINT and about NOLINT directives that happen on lines where no warning is emitted. Both would be useful in clang-tidy, IMO.

I agree with your statements and I think there should be the following diagnostics about NOLINT usage:

• as you described, using of NOLINT with unknown check names;
• using of NOLINT for the line, on which there is no diagnostics (at all with NOLINT and for the swpecified diagnostics); this should help to detect dangling NOLINT comments, that have no meaning anymore.

Moreover, there should be a way to turn on/off these diagnostics, so possibily they should be a separate checks. What do you think? Is there a way for a check to collect the emitted diagnostics?

The existing clang-tidy infrastructure for checks doesn't provide a way for checks to interact or to collect other checks' diagnostics, and I'm not sure this functionality would be helpful beyond this specific feature. So all of this seems more reasonable to implement as a part of clang-tidy itself.

clang-tidy/ClangTidyDiagnosticConsumer.cpp

 ///
 //===----------------------------------------------------------------------===//
 
 #include "ClangTidyDiagnosticConsumer.h"
 #include "ClangTidyOptions.h"
 #include "clang/AST/ASTDiagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Frontend/DiagnosticRenderer.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include <tuple>
 #include <vector>
 using namespace clang;
 using namespace tidy;
 
 namespace {
 class ClangTidyDiagnosticRenderer : public DiagnosticRenderer {
if (!Errors.empty()) {
     } else {
       ++Context.Stats.ErrorsDisplayed;
     }
   }
   LastErrorRelatesToUserCode = false;
   LastErrorPassesLineFilter = false;
 }
 
-static bool LineIsMarkedWithNOLINT(SourceManager &SM, SourceLocation Loc) {
+static bool IsNOLINTFound(StringRef NolintMacro, StringRef Line,
+                          unsigned DiagID, const ClangTidyContext &Context) {
+  const size_t NolintIndex = Line.find(NolintMacro);
+  if (NolintIndex != StringRef::npos) {
+    size_t BracketIndex = NolintIndex + NolintMacro.size();
+    // Check if the specific checks are specified in brackets
+    if (BracketIndex < Line.size() && Line[BracketIndex] == '(') {
+      ++BracketIndex;
+      const size_t BracketEndIndex = Line.find(')', BracketIndex);
+      if (BracketEndIndex != StringRef::npos) {
+        StringRef ChecksStr =
+            Line.substr(BracketIndex, BracketEndIndex - BracketIndex);
+        // Allow disabling all the checks with "*"
+        if (ChecksStr != "*") {
+          StringRef CheckName = Context.getCheckName(DiagID);
+          // Allow specifying a few check names, delimited with comma
+          SmallVector<StringRef, 1> Checks;
+          ChecksStr.split(Checks, ',', -1, false);
+          llvm::transform(Checks, Checks.begin(),
+                          [](StringRef S) { return S.trim(); });
+          return llvm::find(Checks, CheckName) != Checks.end();
+        }
+      }
+    }
+    return true;
+  }
+  return false;
+}
+
+static bool LineIsMarkedWithNOLINT(SourceManager &SM, SourceLocation Loc,
+                                   unsigned DiagID,
+                                   const ClangTidyContext &Context) {
   bool Invalid;
   const char *CharacterData = SM.getCharacterData(Loc, &Invalid);
   if (Invalid)
     return false;
 
   // Check if there's a NOLINT on this line.
   const char *P = CharacterData;
   while (*P != '\0' && *P != '\r' && *P != '\n')
     ++P;
   StringRef RestOfLine(CharacterData, P - CharacterData + 1);
-  // FIXME: Handle /\bNOLINT\b(\([^)]*\))?/ as cpplint.py does.
-  if (RestOfLine.find("NOLINT") != StringRef::npos)
+  if (IsNOLINTFound("NOLINT", RestOfLine, DiagID, Context))
     return true;
 
   // Check if there's a NOLINTNEXTLINE on the previous line.
   const char *BufBegin =
       SM.getCharacterData(SM.getLocForStartOfFile(SM.getFileID(Loc)), &Invalid);
   if (Invalid || P == BufBegin)
     return false;
 
static bool LineIsMarkedWithNOLINT(SourceManager &SM, SourceLocation Loc,
   --P;
   const char *LineEnd = P;
 
   // Now we're on the previous line. Skip to the beginning of it.
   while (P != BufBegin && *P != '\n')
     --P;
 
   RestOfLine = StringRef(P, LineEnd - P + 1);
-  if (RestOfLine.find("NOLINTNEXTLINE") != StringRef::npos)
+  if (IsNOLINTFound("NOLINTNEXTLINE", RestOfLine, DiagID, Context))
     return true;
 
   return false;
 }
 
-static bool LineIsMarkedWithNOLINTinMacro(SourceManager &SM,
-                                          SourceLocation Loc) {
+static bool LineIsMarkedWithNOLINTinMacro(SourceManager &SM, SourceLocation Loc,
+                                          unsigned DiagID,
+                                          const ClangTidyContext &Context) {
   while (true) {
-    if (LineIsMarkedWithNOLINT(SM, Loc))
+    if (LineIsMarkedWithNOLINT(SM, Loc, DiagID, Context))
       return true;
     if (!Loc.isMacroID())
       return false;
     Loc = SM.getImmediateExpansionRange(Loc).first;
   }
   return false;
 }
 
 void ClangTidyDiagnosticConsumer::HandleDiagnostic(
     DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) {
   if (LastErrorWasIgnored && DiagLevel == DiagnosticsEngine::Note)
     return;
 
   if (Info.getLocation().isValid() && DiagLevel != DiagnosticsEngine::Error &&
       DiagLevel != DiagnosticsEngine::Fatal &&
       LineIsMarkedWithNOLINTinMacro(Diags->getSourceManager(),
-                                    Info.getLocation())) {
+                                    Info.getLocation(), Info.getID(),
+                                    Context)) {
     ++Context.Stats.ErrorsIgnoredNOLINT;
     // Ignored a warning, should ignore related notes as well
     LastErrorWasIgnored = true;
     return;
   }
 
   LastErrorWasIgnored = false;
   // Count warnings/errors.

docs/ReleaseNotes.rst

- Added aliases for the `High Integrity C++ Coding Standard <http://www.codingstandard.com/section/index/>`_
   - `hicpp-no-malloc <http://clang.llvm.org/extra/clang-tidy/checks/hicpp-no-malloc.html>`_
   - `hicpp-static-assert <http://clang.llvm.org/extra/clang-tidy/checks/hicpp-static-assert.html>`_
   - `hicpp-use-auto <http://clang.llvm.org/extra/clang-tidy/checks/hicpp-use-auto.html>`_
   - `hicpp-use-emplace <http://clang.llvm.org/extra/clang-tidy/checks/hicpp-use-emplace.html>`_
   - `hicpp-use-noexcept <http://clang.llvm.org/extra/clang-tidy/checks/hicpp-use-noexcept.html>`_
   - `hicpp-use-nullptr <http://clang.llvm.org/extra/clang-tidy/checks/hicpp-use-nullptr.html>`_
   - `hicpp-vararg <http://clang.llvm.org/extra/clang-tidy/checks/hicpp-vararg.html>`_
 
+- Added an ability to specify in parentheses the list of checks to suppress for the ``NOLINT`` 
+  and ``NOLINTNEXTLINE`` comments.
+
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...
 
 Improvements to modularize
 --------------------------
 
 The improvements are...

docs/clang-tidy/index.rst

configuration can be inspected using -dump-config:
       AnalyzeTemporaryDtors: false
       FormatStyle:     none
       User:            user
       CheckOptions:
         - key:             some-check.SomeOption
           value:           'some value'
       ...
 
+Generally, there is no need to suppress :program:`clang-tidy` diagnostics. If
+there are false positives, either a bug should be reported or the code should be
+updated to be clear for both tool and developer. However, if there is a need to
+silent some diagnostics for a line of code, the ``NOLINT`` or ``NOLINTNEXTLINE``
+comments can be used. For example:
+
+.. code-block:: c++
+
+  class Foo
+  {
+    // Skip all the diagnostics for the line
+    Foo(int param); // NOLINT
+
+    // Skip only the specified checks for the line
+    Foo(double param); // NOLINT(google-explicit-constructor, google-runtime-int)
+
+    // Skip only the specified diagnostics for the next line
+    // NOLINTNEXTLINE (google-explicit-constructor, google-runtime-int)
+    Foo(bool param); 
+  };
+
 .. _LibTooling: http://clang.llvm.org/docs/LibTooling.html
 .. _How To Setup Tooling For LLVM: http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
 
 
 Getting Involved
 ================
 
 :program:`clang-tidy` has several own checks and can run Clang static analyzer
 checks, but its power is in the ability to easily write custom checks.
 
 Checks are organized in modules, which can be linked into :program:`clang-tidy`
 with minimal or no code changes in :program:`clang-tidy`.
 
 Checks can plug into the analysis on the preprocessor level using `PPCallbacks`_
 or on the AST level using `AST Matchers`_. When an error is found, checks can

test/clang-tidy/nolint.cpp

 // RUN: %check_clang_tidy %s google-explicit-constructor,clang-diagnostic-unused-variable,clang-analyzer-core.UndefinedBinaryOperatorResult %t -- -extra-arg=-Wunused-variable -- -I%S/Inputs/nolint
 
 #include "trigger_warning.h"
 void I(int& Out) {
   int In;
   A1(In, Out);
 }
 // CHECK-MESSAGES-NOT: trigger_warning.h:{{.*}} warning
 // CHECK-MESSAGES-NOT: :[[@LINE-4]]:{{.*}} note
 
 class A { A(int i); };
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
 
 class B { B(int i); }; // NOLINT
 
-class C { C(int i); }; // NOLINT(we-dont-care-about-categories-yet)
+class C { C(int i); }; // NOLINT(for-some-other-check)
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+class C1 { C1(int i); }; // NOLINT(*)
+
+class C2 { C2(int i); }; // NOLINT(not-closed-bracket-is-treated-as-skip-all
+
+class C3 { C3(int i); }; // NOLINT(google-explicit-constructor)
+
+class C4 { C4(int i); }; // NOLINT(some-check, google-explicit-constructor)
+
+class C5 { C5(int i); }; // NOLINT without-brackets-skip-all, another-check
 
 void f() {
   int i;
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unused variable 'i' [clang-diagnostic-unused-variable]
   int j; // NOLINT
 }
 
 #define MACRO(X) class X { X(int i); };
 MACRO(D)
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
 MACRO(E) // NOLINT
 
 #define MACRO_NOARG class F { F(int i); };
 MACRO_NOARG // NOLINT
 
 #define MACRO_NOLINT class G { G(int i); }; // NOLINT
 MACRO_NOLINT
 
 #define DOUBLE_MACRO MACRO(H) // NOLINT
 DOUBLE_MACRO
 
-// CHECK-MESSAGES: Suppressed 8 warnings (8 NOLINT)
+// CHECK-MESSAGES: Suppressed 12 warnings (12 NOLINT)

test/clang-tidy/nolintnextline.cpp

 class A { A(int i); };
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
 
 // NOLINTNEXTLINE
 class B { B(int i); };
 
-// NOLINTNEXTLINE(we-dont-care-about-categories-yet)
+// NOLINTNEXTLINE(for-some-other-check)
 class C { C(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE(*)
+class C1 { C1(int i); };
+
+// NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all
+class C2 { C2(int i); };
+
+// NOLINTNEXTLINE(google-explicit-constructor)
+class C3 { C3(int i); };
+
+// NOLINTNEXTLINE(some-check, google-explicit-constructor)
+class C4 { C4(int i); };
+
+// NOLINTNEXTLINE without-brackets-skip-all, another-check
+class C5 { C5(int i); };
 
 
 // NOLINTNEXTLINE
 
 class D { D(int i); };
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
 
 // NOLINTNEXTLINE
 //
 class E { E(int i); };
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
 
 #define MACRO(X) class X { X(int i); };
 MACRO(F)
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
 // NOLINTNEXTLINE
 MACRO(G)
 
 #define MACRO_NOARG class H { H(int i); };
 // NOLINTNEXTLINE
 MACRO_NOARG
 
-// CHECK-MESSAGES: Suppressed 4 warnings (4 NOLINT)
+// CHECK-MESSAGES: Suppressed 8 warnings (8 NOLINT)
 
 // RUN: %check_clang_tidy %s google-explicit-constructor %t --