[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

A high-level comment: NOLINT category parsing and warning on incorrect NOLINT categories makes it more difficult for code to comply both with clang-tidy and cpplint (https://github.com/cpplint/cpplint), since:

1. the category names in these tools are different,
2. cpplint currently doesn't support more than one category with the `// NOLINT(category)` syntax,
3. cpplint complains on `// NOLINT` directives with an unknown category.

So when suppressing a warning that is present in both clang-tidy and cpplint (e.g. google-runtime-int) one has to use `// NOLINT` without categories, which kind of undermines the effort of allowing category names explicitly specified in suppression directives.

There are multiple solutions possible. One would be to add a clang-tidy-specific suppression directive and only support specifying check names in it (`// NOCLANGTIDY` seems verbose, but I don't have a better name. Suggestions are welcome.). Leaving support for `// NOLINT` without category names would be nice for suppressing warnings common with cpplint.

Another option would be changing cpplint to somehow recognize and ignore clang-tidy check names in the suppressed categories list.



================
Comment at: clang-tidy/ClangTidy.cpp:402
+  if (Context.isCheckEnabled(NolintCheckName))
+        CheckNames.push_back(NolintCheckName);
+
----------------
clang-format, please


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41326