xgsa created this revision.
xgsa added a project: clang-tools-extra.

The NOLINT directive was extended to support the "NOLINT(category)" and 
"NOLINT(*)" syntax. Also it is possible to specify a few categories separated 
with comma "NOLINT(cat1, cat2)"


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40671

Files:
  clang-tidy/ClangTidyDiagnosticConsumer.cpp
  test/clang-tidy/nolint.cpp
  test/clang-tidy/nolintnextline.cpp

Index: test/clang-tidy/nolint.cpp
===================================================================
--- test/clang-tidy/nolint.cpp
+++ test/clang-tidy/nolint.cpp
@@ -13,8 +13,17 @@
 
 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-category)
+// 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-category, google-explicit-constructor)
+
 void f() {
   int i;
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unused variable 'i' [clang-diagnostic-unused-variable]
@@ -35,4 +44,4 @@
 #define DOUBLE_MACRO MACRO(H) // NOLINT
 DOUBLE_MACRO
 
-// CHECK-MESSAGES: Suppressed 8 warnings (8 NOLINT)
+// CHECK-MESSAGES: Suppressed 11 warnings (11 NOLINT)
Index: test/clang-tidy/nolintnextline.cpp
===================================================================
--- test/clang-tidy/nolintnextline.cpp
+++ test/clang-tidy/nolintnextline.cpp
@@ -4,10 +4,23 @@
 // NOLINTNEXTLINE
 class B { B(int i); };
 
-// NOLINTNEXTLINE(we-dont-care-about-categories-yet)
+// NOLINTNEXTLINE(for-some-other-category)
 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-category, google-explicit-constructor)
+class C4 { C4(int i); };
+
+
 // NOLINTNEXTLINE
 
 class D { D(int i); };
@@ -28,6 +41,6 @@
 // NOLINTNEXTLINE
 MACRO_NOARG
 
-// CHECK-MESSAGES: Suppressed 4 warnings (4 NOLINT)
+// CHECK-MESSAGES: Suppressed 7 warnings (7 NOLINT)
 
 // RUN: %check_clang_tidy %s google-explicit-constructor %t --
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -22,6 +22,7 @@
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Frontend/DiagnosticRenderer.h"
 #include "llvm/ADT/SmallString.h"
+#include <algorithm>
 #include <tuple>
 #include <vector>
 using namespace clang;
@@ -289,8 +290,37 @@
   LastErrorRelatesToUserCode = false;
   LastErrorPassesLineFilter = false;
 }
-
-static bool LineIsMarkedWithNOLINT(SourceManager &SM, SourceLocation Loc) {
+static bool IsNOLINTFound(StringRef NolintMacro, StringRef Line,
+                          unsigned DiagID, const ClangTidyContext &Context) {
+  const auto NolintIndex = Line.find(NolintMacro);
+  if (NolintIndex != StringRef::npos) {
+    auto BracketIndex = NolintIndex + NolintMacro.size();
+    if (BracketIndex < Line.size() && Line[BracketIndex] == '(') {
+      ++BracketIndex;
+      const auto BracketEndIndex = Line.find(')', BracketIndex);
+      if (BracketEndIndex != StringRef::npos) {
+        auto ChecksStr =
+            Line.substr(BracketIndex, BracketEndIndex - BracketIndex);
+        if (ChecksStr != "*") {
+          auto CheckName = Context.getCheckName(DiagID);
+          // Allow specifying a few check names, delimited with comma
+          SmallVector<StringRef, 2> Checks;
+          ChecksStr.split(Checks, ',', -1, false);
+          for (auto &Check : Checks) {
+            Check = Check.trim();
+          }
+          return std::find(Checks.begin(), Checks.end(), 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)
@@ -301,8 +331,7 @@
   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.
@@ -329,16 +358,17 @@
     --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;
@@ -355,7 +385,8 @@
   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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to