salman-javed-nz updated this revision to Diff 371308.
salman-javed-nz added a comment.
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp:
- Added test to check what happens when NOLINTEND marker is used before
NOLINTBEGIN marker (class B1 <https://reviews.llvm.org/B1>, line 7).
- Warning is still displayed (NOLINTEND ends suppression but this is
redundant as there was no suppression in the first place).
- Added test to check what happens when NOLINTBEGIN marker is used but no
NOLINTEND marker is used afterwards (classes H1 <https://reviews.llvm.org/H1>,
H2 <https://reviews.llvm.org/H2>; lines 87, 88).
- Warning is suppressed for the remainder of the file.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108560/new/
https://reviews.llvm.org/D108560
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/Inputs/nolintbeginend/error_in_include.inc
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/nolint_in_include.inc
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
@@ -0,0 +1,90 @@
+// RUN: %check_clang_tidy %s google-explicit-constructor %t -- --header-filter=.* -system-headers -- -isystem %S/Inputs/nolintbeginend
+
+class A { A(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTEND
+class B1 { B1(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+
+// NOLINTBEGIN
+class B2 { B2(int i); };
+// NOLINTEND
+
+// NOLINTBEGIN
+// NOLINTEND
+// NOLINTBEGIN
+class B3 { B3(int i); };
+// NOLINTEND
+
+// NOLINTBEGIN
+// NOLINTEND
+class B4 { B4(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+
+// NOLINTBEGIN(google-explicit-constructor)
+class C1 { C1(int i); };
+// NOLINTEND(google-explicit-constructor)
+
+// NOLINTBEGIN(*)
+class C2 { C2(int i); };
+// NOLINTEND(*)
+
+// NOLINTBEGIN(some-other-check)
+class C3 { C3(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); };
+// NOLINTEND(some-other-check, google-explicit-constructor)
+
+// NOLINTBEGIN(some-other-check, google-explicit-constructor)
+// NOLINTEND(some-other-check)
+class C5 { C5(int i); };
+// NOLINTEND(google-explicit-constructor)
+
+// NOLINTBEGIN(some-other-check, google-explicit-constructor)
+// NOLINTEND(google-explicit-constructor)
+class C6 { C6(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); };
+// NOLINTEND(some-other-check)
+// NOLINTEND(google-explicit-constructor)
+
+// NOLINTBEGIN(not-closed-bracket-is-treated-as-skip-all
+class C8 { C8(int i); };
+// NOLINTEND(not-closed-bracket-is-treated-as-skip-all
+
+// NOLINTBEGIN without-brackets-skip-all, another-check
+class C9 { C9(int i); };
+// NOLINTEND without-brackets-skip-all, another-check
+
+#define MACRO(X) class X { X(int i); };
+
+MACRO(D1)
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
+
+// NOLINTBEGIN
+MACRO(D2)
+// NOLINTEND
+
+#define MACRO_NOARG class E { E(int i); };
+// NOLINTBEGIN
+MACRO_NOARG
+// NOLINTEND
+
+#include "error_in_include.inc"
+// CHECK-MESSAGES: warning: single-argument constructors must be marked explicit
+
+#include "nolint_in_include.inc"
+
+// NOLINTBEGIN
+class H1 { H1(int i); };
+class H2 { H2(int i); };
+
+// CHECK-MESSAGES: Suppressed 14 warnings (14 NOLINT)
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/nolint_in_include.inc
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/nolint_in_include.inc
@@ -0,0 +1,3 @@
+// NOLINTBEGIN
+class G { G(int i); };
+// NOLINTEND
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/error_in_include.inc
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/error_in_include.inc
@@ -0,0 +1 @@
+class F { F(int i); };
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
@@ -295,17 +295,19 @@
If a specific suppression mechanism is not available for a certain warning, or
its use is not desired for some reason, :program:`clang-tidy` has a generic
-mechanism to suppress diagnostics using ``NOLINT`` or ``NOLINTNEXTLINE``
-comments.
+mechanism to suppress diagnostics using ``NOLINT``, ``NOLINTNEXTLINE``, and
+``NOLINTBEGIN`` ... ``NOLINTEND`` comments.
The ``NOLINT`` comment instructs :program:`clang-tidy` to ignore warnings on the
*same line* (it doesn't apply to a function, a block of code or any other
language construct, it applies to the line of code it is on). If introducing the
comment in the same line would change the formatting in undesired way, the
``NOLINTNEXTLINE`` comment allows to suppress clang-tidy warnings on the *next
-line*.
+line*. The ``NOLINTBEGIN`` and ``NOLINTEND`` comments allow suppressing
+clang-tidy warnings on *multiple lines* (affecting all lines between the two
+comments).
-Both comments can be followed by an optional list of check names in parentheses
+All comments can be followed by an optional list of check names in parentheses
(see below for the formal syntax).
For example:
@@ -325,9 +327,16 @@
// Silence only the specified diagnostics for the next line
// NOLINTNEXTLINE(google-explicit-constructor, google-runtime-int)
Foo(bool param);
+
+ // Silence only the specified diagnostics for all lines between NOLINTBEGIN/END.
+ // NOLINTBEGIN(google-explicit-constructor, google-runtime-int)
+ Foo(short param);
+ Foo(long param);
+ // NOLINTEND(google-explicit-constructor, google-runtime-int)
};
-The formal syntax of ``NOLINT``/``NOLINTNEXTLINE`` is the following:
+The formal syntax of ``NOLINT``, ``NOLINTNEXTLINE``, and ``NOLINTBEGIN`` ...
+``NOLINTEND`` is the following:
.. parsed-literal::
@@ -345,11 +354,14 @@
lint-command:
**NOLINT**
**NOLINTNEXTLINE**
-
-Note that whitespaces between ``NOLINT``/``NOLINTNEXTLINE`` and the opening
-parenthesis are not allowed (in this case the comment will be treated just as
-``NOLINT``/``NOLINTNEXTLINE``), whereas in check names list (inside the
-parenthesis) whitespaces can be used and will be ignored.
+ **NOLINTBEGIN**
+ **NOLINTEND**
+
+Note that whitespaces between ``NOLINT``/``NOLINTNEXTLINE``/``NOLINTBEGIN``/``NOLINTEND``
+and the opening parenthesis are not allowed (in this case the comment will be
+treated just as ``NOLINT``/``NOLINTNEXTLINE``/``NOLINTBEGIN``/``NOLINTEND``),
+whereas in check names list (inside the parenthesis) whitespaces can be used and
+will be ignored.
.. _LibTooling: https://clang.llvm.org/docs/LibTooling.html
.. _How To Setup Tooling For LLVM: https://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,7 +67,7 @@
Improvements to clang-tidy
--------------------------
-The improvements are...
+- Added support for `NOLINTBEGIN` ... `NOLINTEND` comments to suppress ClangTidy warnings over multiple lines.
New checks
^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -361,7 +361,21 @@
// Check if there's a NOLINTNEXTLINE on the previous line.
StringRef PrevLine =
Buffer->substr(0, Offset).rsplit('\n').first.rsplit('\n').second;
- return IsNOLINTFound("NOLINTNEXTLINE", PrevLine, DiagID, Context);
+ if (IsNOLINTFound("NOLINTNEXTLINE", PrevLine, DiagID, Context))
+ return true;
+
+ // Check if there's an open NOLINTBEGIN ... NOLINTEND block on the previous
+ // lines.
+ SmallVector<StringRef> PrevLines;
+ Buffer->substr(0, Offset).rsplit('\n').first.split(PrevLines, '\n');
+ for (auto I = PrevLines.rbegin(), E = PrevLines.rend(); I != E; ++I) {
+ if (IsNOLINTFound("NOLINTEND", *I, DiagID, Context))
+ return false;
+ if (IsNOLINTFound("NOLINTBEGIN", *I, DiagID, Context))
+ return true;
+ }
+
+ return false;
}
static bool LineIsMarkedWithNOLINTinMacro(const SourceManager &SM,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
