salman-javed-nz created this revision.
salman-javed-nz added reviewers: alexfh, aaron.ballman, njames93.
salman-javed-nz added a project: clang-tools-extra.
Herald added subscribers: arphaman, xazax.hun.
salman-javed-nz requested review of this revision.
Add support for `NOLINTBEGIN` ... `NOLINTEND` comments to suppress clang-tidy
warnings over multiple lines. All lines between the "begin" and "end" markers
are suppressed.
Example:
// NOLINTBEGIN(some-check)
// <Code with warnings to be suppressed, line 1>
// <Code with warnings to be suppressed, line 2>
// <Code with warnings to be suppressed, line 3>
// NOLINTEND(some-check)
Follows similar syntax as the `NOLINT` and `NOLINTNEXTLINE` comments that are
already implemented, i.e. allows multiple checks to be provided in parentheses;
suppresses all checks if the parentheses are omitted, etc.
Repository:
rG LLVM Github Monorepo
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/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,77 @@
+class A { A(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTBEGIN
+class B { B(int i); };
+// NOLINTEND
+
+// NOLINTBEGIN
+// NOLINTEND
+// NOLINTBEGIN
+class B1 { B1(int i); };
+// NOLINTEND
+
+// NOLINTBEGIN
+// NOLINTEND
+class B2 { B2(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+
+// NOLINTBEGIN(google-explicit-constructor)
+class C { C(int i); };
+// NOLINTEND(google-explicit-constructor)
+
+// NOLINTBEGIN(*)
+class C1 { C1(int i); };
+// NOLINTEND(*)
+
+// NOLINTBEGIN(some-other-check)
+class C2 { C2(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 C3 { C3(int i); };
+// NOLINTEND(some-other-check, google-explicit-constructor)
+
+// NOLINTBEGIN(some-other-check, google-explicit-constructor)
+// NOLINTEND(some-other-check)
+class C4 { C4(int i); };
+// NOLINTEND(google-explicit-constructor)
+
+// NOLINTBEGIN(some-other-check, google-explicit-constructor)
+// NOLINTEND(google-explicit-constructor)
+class C5 { C5(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 C6 { C6(int i); };
+// NOLINTEND(some-other-check)
+// NOLINTEND(google-explicit-constructor)
+
+// NOLINTBEGIN(not-closed-bracket-is-treated-as-skip-all
+class C7 { C7(int i); };
+// NOLINTEND(not-closed-bracket-is-treated-as-skip-all
+
+// NOLINTBEGIN without-brackets-skip-all, another-check
+class C8 { C8(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
+
+// CHECK-MESSAGES: Suppressed 11 warnings (11 NOLINT)
+
+// RUN: %check_clang_tidy %s google-explicit-constructor %t --
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
