carlosgalvezp updated this revision to Diff 378427.
carlosgalvezp marked 5 inline comments as done.
carlosgalvezp added a comment.
-Fixed formatting comments.
-Added test for NOLINT(), NOLINTNEXTLINE() and NOLINTBEGIN()
-Changed last test to more effectively test NOLINT(*,-google*)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111208/new/
https://reviews.llvm.org/D111208
Files:
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/index.rst
clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp
@@ -11,21 +11,25 @@
class D { D(int i); };
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
-// NOLINTNEXTLINE(*)
+// NOLINTNEXTLINE()
class D1 { D1(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
-// NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all
+// NOLINTNEXTLINE(*)
class D2 { D2(int i); };
-// NOLINTNEXTLINE(google-explicit-constructor)
+// NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all
class D3 { D3(int i); };
-// NOLINTNEXTLINE(some-check, google-explicit-constructor)
+// NOLINTNEXTLINE(google-explicit-constructor)
class D4 { D4(int i); };
-// NOLINTNEXTLINE without-brackets-skip-all, another-check
+// NOLINTNEXTLINE(some-check, google-explicit-constructor)
class D5 { D5(int i); };
+// NOLINTNEXTLINE without-brackets-skip-all, another-check
+class D6 { D6(int i); };
+
// NOLINTNEXTLINE
class E { E(int i); };
@@ -46,6 +50,21 @@
// NOLINTNEXTLINE
MACRO_NOARG
-// CHECK-MESSAGES: Suppressed 9 warnings (9 NOLINT)
+// NOLINTNEXTLINE(google*)
+class I1 { I1(int i); };
+
+// NOLINTNEXTLINE(google*,-google*)
+class I2 { I2(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+
+class I3 { I3(int x); void f() { int i; } };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+// CHECK-MESSAGES: :[[@LINE-2]]:38: warning: unused variable 'i' [clang-diagnostic-unused-variable]
+
+// NOLINTNEXTLINE(*,-google*)
+class I4 { I4(int x); void f() { int i; } };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+
+// CHECK-MESSAGES: Suppressed 11 warnings (11 NOLINT)
-// RUN: %check_clang_tidy %s google-explicit-constructor %t --
+// RUN: %check_clang_tidy %s google-explicit-constructor,clang-diagnostic-unused-variable %t -- -extra-arg=-Wunused-variable
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s google-explicit-constructor %t
+// RUN: %check_clang_tidy %s google-explicit-constructor,clang-diagnostic-unused-variable %t -- -extra-arg=-Wunused-variable
class A { A(int i); };
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
@@ -34,60 +34,65 @@
class C1 { C1(int i); };
// NOLINTEND(google-explicit-constructor)
-// NOLINTBEGIN(*)
+// NOLINTBEGIN()
class C2 { C2(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+// NOLINTEND()
+
+// NOLINTBEGIN(*)
+class C3 { C3(int i); };
// NOLINTEND(*)
// NOLINTBEGIN(some-other-check)
-class C3 { C3(int i); };
+class C4 { C4(int i); };
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
// NOLINTEND(some-other-check)
// NOLINTBEGIN(some-other-check, google-explicit-constructor)
-class C4 { C4(int i); };
+class C5 { C5(int i); };
// NOLINTEND(some-other-check, google-explicit-constructor)
// NOLINTBEGIN(some-other-check, google-explicit-constructor)
// NOLINTEND(some-other-check)
-class C5 { C5(int i); };
+class C6 { C6(int i); };
// NOLINTEND(google-explicit-constructor)
// NOLINTBEGIN(some-other-check, google-explicit-constructor)
// NOLINTEND(google-explicit-constructor)
-class C6 { C6(int i); };
+class C7 { C7(int i); };
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
// NOLINTEND(some-other-check)
// NOLINTBEGIN(google-explicit-constructor)
// NOLINTBEGIN(some-other-check)
-class C7 { C7(int i); };
+class C8 { C8(int i); };
// NOLINTEND(some-other-check)
// NOLINTEND(google-explicit-constructor)
// NOLINTBEGIN(google-explicit-constructor)
// NOLINTBEGIN(some-other-check)
-class C8 { C8(int i); };
+class C9 { C9(int i); };
// NOLINTEND(google-explicit-constructor)
// NOLINTEND(some-other-check)
// NOLINTBEGIN(google-explicit-constructor)
// NOLINTBEGIN
-class C9 { C9(int i); };
+class C10 { C10(int i); };
// NOLINTEND
// NOLINTEND(google-explicit-constructor)
// NOLINTBEGIN
// NOLINTBEGIN(google-explicit-constructor)
-class C10 { C10(int i); };
+class C11 { C11(int i); };
// NOLINTEND(google-explicit-constructor)
// NOLINTEND
// NOLINTBEGIN(not-closed-bracket-is-treated-as-skip-all
-class C11 { C11(int i); };
+class C12 { C12(int i); };
// NOLINTEND(not-closed-bracket-is-treated-as-skip-all
// NOLINTBEGIN without-brackets-skip-all, another-check
-class C12 { C12(int i); };
+class C13 { C13(int i); };
// NOLINTEND without-brackets-skip-all, another-check
#define MACRO(X) class X { X(int i); };
@@ -119,4 +124,22 @@
MACRO_NO_LINT_INSIDE_MACRO
-// CHECK-MESSAGES: Suppressed 18 warnings (18 NOLINT).
+// NOLINTBEGIN(google*)
+class C14 { C14(int i); };
+// NOLINTEND(google*)
+
+// NOLINTBEGIN(google*,-google*)
+class C15 { C15(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: single-argument constructors must be marked explicit
+// NOLINTEND(google*,-google*)
+
+class C16 { C16(int x); void f() { int i; } };
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: single-argument constructors must be marked explicit
+// CHECK-MESSAGES: :[[@LINE-2]]:40: warning: unused variable 'i' [clang-diagnostic-unused-variable]
+
+// NOLINTBEGIN(*,-google*)
+class C17 { C17(int x); void f() { int i; } };
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: single-argument constructors must be marked explicit
+// NOLINTEND(*,-google*)
+
+// CHECK-MESSAGES: Suppressed 20 warnings (20 NOLINT).
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp
@@ -17,17 +17,20 @@
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 C1 { C1(int i); }; // NOLINT()
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+
+class C2 { C2(int i); }; // NOLINT(*)
-class C2 { C2(int i); }; // NOLINT(not-closed-bracket-is-treated-as-skip-all
+class C3 { C3(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(google-explicit-constructor)
-class C4 { C4(int i); }; // NOLINT(some-check, google-explicit-constructor)
+class C5 { C5(int i); }; // NOLINT(some-check, google-explicit-constructor)
-class C5 { C5(int i); }; // NOLINT without-brackets-skip-all, another-check
+class C6 { C6(int i); }; // NOLINT without-brackets-skip-all, another-check
-class C6 { C6(int i); }; // NOLINTNEXTLINE doesn't get misconstrued as a NOLINT
+class C7 { C7(int i); }; // NOLINTNEXTLINE doesn't get misconstrued as a NOLINT
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
void f() {
@@ -51,4 +54,20 @@
#define DOUBLE_MACRO MACRO(H) // NOLINT
DOUBLE_MACRO
-// CHECK-MESSAGES: Suppressed 13 warnings (13 NOLINT)
+class D1 { D1(int x); }; // NOLINT(google*)
+class D2 { D2(int x); }; // NOLINT(*explicit-constructor)
+class D3 { D3(int x); }; // NOLINT(*explicit*)
+
+class D4 { D4(int x); }; // NOLINT(google*,-google*)
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+
+class D5 { D5(int x); }; // NOLINT(google*,-google*,google*)
+
+class D6 { D6(int x); void f() { int i; } };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+// CHECK-MESSAGES: :[[@LINE-2]]:38: warning: unused variable 'i' [clang-diagnostic-unused-variable]
+
+class D7 { D7(int x); void f() { int i; } }; // NOLINT(*,-google*)
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+
+// CHECK-MESSAGES: Suppressed 18 warnings (18 NOLINT)
Index: clang-tools-extra/docs/clang-tidy/index.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -173,11 +173,11 @@
errors were found. If compiler errors have
attached fix-its, clang-tidy will apply them as
well.
- --fix-notes -
- If a warning has no fix, but a single fix can
- be found through an associated diagnostic note,
- apply the fix.
- Specifying this flag will implicitly enable the
+ --fix-notes -
+ If a warning has no fix, but a single fix can
+ be found through an associated diagnostic note,
+ apply the fix.
+ Specifying this flag will implicitly enable the
'--fix' flag.
--format-style=<string> -
Style for formatting code around applied fixes:
@@ -308,7 +308,10 @@
comments).
All comments can be followed by an optional list of check names in parentheses
-(see below for the formal syntax).
+(see below for the formal syntax). The list of check names supports globbing,
+with the same format and semantics as for enabling checks. Note that adding
+a dash, e.g. `NOLINT(-check-name)`, is a double negation ("do *not* suppress
+`check-name`"), so the warning is not expected to be suppressed.
For example:
@@ -333,6 +336,15 @@
Foo(short param);
Foo(long param);
// NOLINTEND(google-explicit-constructor, google-runtime-int)
+
+ // Silence all warnings from the "google" module
+ Foo(bool param); // NOLINT(google*)
+
+ // Silence all warnings, *except* the ones from the "google" module
+ Foo(bool param); // NOLINT(*,-google*)
+
+ // No warnings are suppressed, due to double negation
+ Foo(bool param); // NOLINT(-google*)
};
The formal syntax of ``NOLINT``, ``NOLINTNEXTLINE``, and ``NOLINTBEGIN`` ...
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,6 +67,9 @@
Improvements to clang-tidy
--------------------------
+- Added support for globbing in `NOLINT*` expressions, to simplify suppressing
+ multiple warnings in the same line.
+
- Added support for `NOLINTBEGIN` ... `NOLINTEND` comments to suppress
Clang-Tidy warnings over multiple lines.
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -345,18 +345,12 @@
if (BracketEndIndex != StringRef::npos) {
StringRef ChecksStr =
Line.substr(BracketIndex, BracketEndIndex - BracketIndex);
- // Allow disabling all the checks with "*".
- if (ChecksStr != "*") {
- // 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(); });
- if (llvm::find(Checks, CheckName) == Checks.end())
- return false;
- if (SuppressionIsSpecific)
- *SuppressionIsSpecific = true;
- }
+ // Allow specifying a few checks with a glob expression.
+ GlobList Globs(ChecksStr);
+ if (!Globs.contains(CheckName))
+ return false;
+ if (SuppressionIsSpecific)
+ *SuppressionIsSpecific = true;
}
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
