salman-javed-nz updated this revision to Diff 395606.
salman-javed-nz added a comment.
Remove unnecessary `llvm::` qualification.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116085/new/
https://reviews.llvm.org/D116085
Files:
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-LIFO.cpp
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-all-end-glob.cpp
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-all-end-specific.cpp
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-glob-end-all.cpp
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-glob-end-specific.cpp
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-global-end-specific.cpp
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-multiple-end-single.cpp
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-single-end-multiple.cpp
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-all.cpp
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-glob.cpp
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-global.cpp
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-mismatched-check-names.cpp
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-mismatched-delims.cpp
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-typo-in-check-name.cpp
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
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
@@ -58,30 +58,24 @@
// NOLINTEND(some-other-check)
// NOLINTEND(google-explicit-constructor)
-// NOLINTBEGIN(google-explicit-constructor)
-// NOLINTBEGIN(some-other-check)
-class C7 { C7(int i); };
-// NOLINTEND(google-explicit-constructor)
-// NOLINTEND(some-other-check)
-
// NOLINTBEGIN(google-explicit-constructor)
// NOLINTBEGIN
-class C8 { C8(int i); };
+class C7 { C7(int i); };
// NOLINTEND
// NOLINTEND(google-explicit-constructor)
// NOLINTBEGIN
// NOLINTBEGIN(google-explicit-constructor)
-class C9 { C9(int i); };
+class C8 { C8(int i); };
// NOLINTEND(google-explicit-constructor)
// NOLINTEND
// NOLINTBEGIN(not-closed-bracket-is-treated-as-skip-all
-class C10 { C10(int i); };
+class C9 { C9(int i); };
// NOLINTEND(not-closed-bracket-is-treated-as-skip-all
// NOLINTBEGIN without-brackets-skip-all, another-check
-class C11 { C11(int i); };
+class C10 { C10(int i); };
// NOLINTEND without-brackets-skip-all, another-check
#define MACRO(X) class X { X(int i); };
@@ -114,28 +108,28 @@
MACRO_NO_LINT_INSIDE_MACRO
// NOLINTBEGIN(google*)
-class C12 { C12(int i); };
+class C11 { C11(int i); };
// NOLINTEND(google*)
// NOLINTBEGIN(*explicit-constructor)
-class C15 { C15(int i); };
+class C12 { C12(int i); };
// NOLINTEND(*explicit-constructor)
// NOLINTBEGIN(*explicit*)
-class C16 { C16(int i); };
+class C13 { C13(int i); };
// NOLINTEND(*explicit*)
// NOLINTBEGIN(-explicit-constructor)
-class C17 { C17(int x); };
+class C14 { C14(int x); };
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: single-argument constructors must be marked explicit
// NOLINTEND(-explicit-constructor)
// NOLINTBEGIN(google*,-google*)
-class C18 { C18(int x); };
+class C15 { C15(int x); };
// NOLINTEND(google*,-google*)
// NOLINTBEGIN(*,-google*)
-class C19 { C19(int x); };
+class C16 { C16(int x); };
// NOLINTEND(*,-google*)
int array1[10];
@@ -154,4 +148,4 @@
int array4[10];
// NOLINTEND(*-avoid-c-arrays)
-// CHECK-MESSAGES: Suppressed 27 warnings (27 NOLINT).
+// CHECK-MESSAGES: Suppressed 26 warnings (26 NOLINT).
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-typo-in-check-name.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-typo-in-check-name.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-typo-in-check-name.cpp
@@ -11,3 +11,6 @@
// CHECK: TBEGIN' comment without a subsequent 'NOLIN
// CHECK: TEND' comment [clang-tidy-nolint]
// CHECK: :[[@LINE-9]]:11: warning: single-argument constructors must be marked explicit
+// CHECK: :[[@LINE-9]]:4: error: unmatched 'NOLIN
+// CHECK: TEND' comment without a previous 'NOLIN
+// CHECK: TBEGIN' comment [clang-tidy-nolint]
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-mismatched-delims.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-mismatched-delims.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-mismatched-delims.cpp
@@ -11,4 +11,3 @@
// CHECK: :[[@LINE-8]]:4: error: unmatched 'NOLIN
// CHECK: TBEGIN' comment without a subsequent 'NOLIN
// CHECK: TEND' comment [clang-tidy-nolint]
-// CHECK: :[[@LINE-9]]:11: warning: single-argument constructors must be marked explicit
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-mismatched-check-names.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-mismatched-check-names.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-mismatched-check-names.cpp
@@ -16,24 +16,3 @@
// CHECK: :[[@LINE-10]]:4: error: unmatched 'NOLIN
// CHECK: TEND' comment without a previous 'NOLIN
// CHECK: TBEGIN' comment [clang-tidy-nolint]
-
-// NOLINTBEGIN(google-explicit-constructor,google-readability-casting)
-class B { B(int i); };
-// NOLINTEND(google-explicit-constructor)
-auto Num2 = (unsigned int)(-1);
-// NOLINTEND(google-readability-casting)
-
-// Note: the expected output has been split over several lines so that clang-tidy
-// does not see the "no lint" suppression comment and mistakenly assume it
-// is meant for itself.
-// CHECK: :[[@LINE-9]]:4: error: unmatched 'NOLIN
-// CHECK: TBEGIN' comment without a subsequent 'NOLIN
-// CHECK: TEND' comment [clang-tidy-nolint]
-// CHECK: :[[@LINE-11]]:11: warning: single-argument constructors must be marked explicit
-// CHECK: :[[@LINE-11]]:4: error: unmatched 'NOLIN
-// CHECK: TEND' comment without a previous 'NOLIN
-// CHECK: TBEGIN' comment [clang-tidy-nolint]
-// CHECK: :[[@LINE-13]]:13: warning: C-style casts are discouraged; use static_cast
-// CHECK: :[[@LINE-13]]:4: error: unmatched 'NOLIN
-// CHECK: TEND' comment without a previous 'NOLIN
-// CHECK: TBEGIN' comment [clang-tidy-nolint]
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-global.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-global.cpp
+++ /dev/null
@@ -1,25 +0,0 @@
-// RUN: not clang-tidy %s --checks="-*,google-explicit-constructor" 2>&1 | FileCheck %s
-
-// NOLINTBEGIN(google-explicit-constructor)
-class A { A(int i); };
-// NOLINTEND
-
-// Note: the expected output has been split over several lines so that clang-tidy
-// does not see the "no lint" suppression comment and mistakenly assume it
-// is meant for itself.
-// CHECK: :[[@LINE-6]]:11: warning: single-argument constructors must be marked explicit
-// CHECK: :[[@LINE-6]]:4: error: unmatched 'NOLIN
-// CHECK: TEND' comment without a previous 'NOLIN
-// CHECK: TBEGIN' comment [clang-tidy-nolint]
-
-// NOLINTBEGIN(*)
-class B { B(int i); };
-// NOLINTEND
-
-// Note: the expected output has been split over several lines so that clang-tidy
-// does not see the "no lint" suppression comment and mistakenly assume it
-// is meant for itself.
-// CHECK: :[[@LINE-6]]:11: warning: single-argument constructors must be marked explicit
-// CHECK: :[[@LINE-6]]:4: error: unmatched 'NOLIN
-// CHECK: TEND' comment without a previous 'NOLIN
-// CHECK: TBEGIN' comment [clang-tidy-nolint]
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-glob.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-glob.cpp
@@ -0,0 +1,16 @@
+// RUN: not clang-tidy %s --checks="-*,google-explicit-constructor" 2>&1 | FileCheck %s
+
+// NOLINTBEGIN(google-explicit-constructor)
+class A { A(int i); };
+// NOLINTEND(*)
+
+// Note: the expected output has been split over several lines so that clang-tidy
+// does not see the "no lint" suppression comment and mistakenly assume it
+// is meant for itself.
+// CHECK: :[[@LINE-7]]:4: error: unmatched 'NOLIN
+// CHECK: TBEGIN' comment without a subsequent 'NOLIN
+// CHECK: TEND' comment [clang-tidy-nolint]
+// CHECK: :[[@LINE-9]]:11: warning: single-argument constructors must be marked explicit
+// CHECK: :[[@LINE-9]]:4: error: unmatched 'NOLIN
+// CHECK: TEND' comment without a previous 'NOLIN
+// CHECK: TBEGIN' comment [clang-tidy-nolint]
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-all.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-all.cpp
@@ -0,0 +1,16 @@
+// RUN: not clang-tidy %s --checks="-*,google-explicit-constructor" 2>&1 | FileCheck %s
+
+// NOLINTBEGIN(google-explicit-constructor)
+class A { A(int i); };
+// NOLINTEND
+
+// Note: the expected output has been split over several lines so that clang-tidy
+// does not see the "no lint" suppression comment and mistakenly assume it
+// is meant for itself.
+// CHECK: :[[@LINE-7]]:4: error: unmatched 'NOLIN
+// CHECK: TBEGIN' comment without a subsequent 'NOLIN
+// CHECK: TEND' comment [clang-tidy-nolint]
+// CHECK: :[[@LINE-9]]:11: warning: single-argument constructors must be marked explicit
+// CHECK: :[[@LINE-9]]:4: error: unmatched 'NOLIN
+// CHECK: TEND' comment without a previous 'NOLIN
+// CHECK: TBEGIN' comment [clang-tidy-nolint]
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-single-end-multiple.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-single-end-multiple.cpp
@@ -0,0 +1,22 @@
+// RUN: not clang-tidy %s --checks='-*,google-explicit-constructor,google-readability-casting' 2>&1 | FileCheck %s
+
+// NOLINTBEGIN(google-explicit-constructor)
+// NOLINTBEGIN(google-readability-casting)
+class B { B(int i); };
+auto Num2 = (unsigned int)(-1);
+// NOLINTEND(google-explicit-constructor,google-readability-casting)
+
+// Note: the expected output has been split over several lines so that clang-tidy
+// does not see the "no lint" suppression comment and mistakenly assume it
+// is meant for itself.
+// CHECK: :[[@LINE-9]]:4: error: unmatched 'NOLIN
+// CHECK: TBEGIN' comment without a subsequent 'NOLIN
+// CHECK: TEND' comment [clang-tidy-nolint]
+// CHECK: :[[@LINE-11]]:4: error: unmatched 'NOLIN
+// CHECK: TBEGIN' comment without a subsequent 'NOLIN
+// CHECK: TEND' comment [clang-tidy-nolint]
+// CHECK: :[[@LINE-13]]:11: warning: single-argument constructors must be marked explicit
+// CHECK: :[[@LINE-13]]:13: warning: C-style casts are discouraged; use static_cast
+// CHECK: :[[@LINE-13]]:4: error: unmatched 'NOLIN
+// CHECK: TEND' comment without a previous 'NOLIN
+// CHECK: TBEGIN' comment [clang-tidy-nolint]
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-multiple-end-single.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-multiple-end-single.cpp
@@ -0,0 +1,22 @@
+// RUN: not clang-tidy %s --checks='-*,google-explicit-constructor,google-readability-casting' 2>&1 | FileCheck %s
+
+// NOLINTBEGIN(google-explicit-constructor,google-readability-casting)
+class B { B(int i); };
+// NOLINTEND(google-explicit-constructor)
+auto Num2 = (unsigned int)(-1);
+// NOLINTEND(google-readability-casting)
+
+// Note: the expected output has been split over several lines so that clang-tidy
+// does not see the "no lint" suppression comment and mistakenly assume it
+// is meant for itself.
+// CHECK: :[[@LINE-9]]:4: error: unmatched 'NOLIN
+// CHECK: TBEGIN' comment without a subsequent 'NOLIN
+// CHECK: TEND' comment [clang-tidy-nolint]
+// CHECK: :[[@LINE-11]]:11: warning: single-argument constructors must be marked explicit
+// CHECK: :[[@LINE-11]]:4: error: unmatched 'NOLIN
+// CHECK: TEND' comment without a previous 'NOLIN
+// CHECK: TBEGIN' comment [clang-tidy-nolint]
+// CHECK: :[[@LINE-13]]:13: warning: C-style casts are discouraged; use static_cast
+// CHECK: :[[@LINE-13]]:4: error: unmatched 'NOLIN
+// CHECK: TEND' comment without a previous 'NOLIN
+// CHECK: TBEGIN' comment [clang-tidy-nolint]
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-global-end-specific.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-global-end-specific.cpp
+++ /dev/null
@@ -1,25 +0,0 @@
-// RUN: not clang-tidy %s --checks='-*,google-explicit-constructor' 2>&1 | FileCheck %s
-
-// NOLINTBEGIN
-class A { A(int i); };
-// NOLINTEND(google-explicit-constructor)
-
-// Note: the expected output has been split over several lines so that clang-tidy
-// does not see the "no lint" suppression comment and mistakenly assume it
-// is meant for itself.
-// CHECK: :[[@LINE-6]]:11: warning: single-argument constructors must be marked explicit
-// CHECK: :[[@LINE-6]]:4: error: unmatched 'NOLIN
-// CHECK: TEND' comment without a previous 'NOLIN
-// CHECK: TBEGIN' comment [clang-tidy-nolint]
-
-// NOLINTBEGIN
-class B { B(int i); };
-// NOLINTEND(*)
-
-// Note: the expected output has been split over several lines so that clang-tidy
-// does not see the "no lint" suppression comment and mistakenly assume it
-// is meant for itself.
-// CHECK: :[[@LINE-6]]:11: warning: single-argument constructors must be marked explicit
-// CHECK: :[[@LINE-6]]:4: error: unmatched 'NOLIN
-// CHECK: TEND' comment without a previous 'NOLIN
-// CHECK: TBEGIN' comment [clang-tidy-nolint]
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-glob-end-specific.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-glob-end-specific.cpp
@@ -0,0 +1,16 @@
+// RUN: not clang-tidy %s --checks="-*,google-explicit-constructor" 2>&1 | FileCheck %s
+
+// NOLINTBEGIN(*)
+class B { B(int i); };
+// NOLINTEND(google-explicit-constructor)
+
+// Note: the expected output has been split over several lines so that clang-tidy
+// does not see the "no lint" suppression comment and mistakenly assume it
+// is meant for itself.
+// CHECK: :[[@LINE-7]]:4: error: unmatched 'NOLIN
+// CHECK: TBEGIN' comment without a subsequent 'NOLIN
+// CHECK: TEND' comment [clang-tidy-nolint]
+// CHECK: :[[@LINE-9]]:11: warning: single-argument constructors must be marked explicit
+// CHECK: :[[@LINE-9]]:4: error: unmatched 'NOLIN
+// CHECK: TEND' comment without a previous 'NOLIN
+// CHECK: TBEGIN' comment [clang-tidy-nolint]
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-glob-end-all.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-glob-end-all.cpp
@@ -0,0 +1,16 @@
+// RUN: not clang-tidy %s --checks="-*,google-explicit-constructor" 2>&1 | FileCheck %s
+
+// NOLINTBEGIN(*)
+class B { B(int i); };
+// NOLINTEND
+
+// Note: the expected output has been split over several lines so that clang-tidy
+// does not see the "no lint" suppression comment and mistakenly assume it
+// is meant for itself.
+// CHECK: :[[@LINE-7]]:4: error: unmatched 'NOLIN
+// CHECK: TBEGIN' comment without a subsequent 'NOLIN
+// CHECK: TEND' comment [clang-tidy-nolint]
+// CHECK: :[[@LINE-9]]:11: warning: single-argument constructors must be marked explicit
+// CHECK: :[[@LINE-9]]:4: error: unmatched 'NOLIN
+// CHECK: TEND' comment without a previous 'NOLIN
+// CHECK: TBEGIN' comment [clang-tidy-nolint]
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-all-end-specific.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-all-end-specific.cpp
@@ -0,0 +1,16 @@
+// RUN: not clang-tidy %s --checks='-*,google-explicit-constructor' 2>&1 | FileCheck %s
+
+// NOLINTBEGIN
+class A { A(int i); };
+// NOLINTEND(google-explicit-constructor)
+
+// Note: the expected output has been split over several lines so that clang-tidy
+// does not see the "no lint" suppression comment and mistakenly assume it
+// is meant for itself.
+// CHECK: :[[@LINE-7]]:4: error: unmatched 'NOLIN
+// CHECK: TBEGIN' comment without a subsequent 'NOLIN
+// CHECK: TEND' comment [clang-tidy-nolint]
+// CHECK: :[[@LINE-9]]:11: warning: single-argument constructors must be marked explicit
+// CHECK: :[[@LINE-9]]:4: error: unmatched 'NOLIN
+// CHECK: TEND' comment without a previous 'NOLIN
+// CHECK: TBEGIN' comment [clang-tidy-nolint]
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-all-end-glob.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-all-end-glob.cpp
@@ -0,0 +1,16 @@
+// RUN: not clang-tidy %s --checks='-*,google-explicit-constructor' 2>&1 | FileCheck %s
+
+// NOLINTBEGIN
+class B { B(int i); };
+// NOLINTEND(*)
+
+// Note: the expected output has been split over several lines so that clang-tidy
+// does not see the "no lint" suppression comment and mistakenly assume it
+// is meant for itself.
+// CHECK: :[[@LINE-7]]:4: error: unmatched 'NOLIN
+// CHECK: TBEGIN' comment without a subsequent 'NOLIN
+// CHECK: TEND' comment [clang-tidy-nolint]
+// CHECK: :[[@LINE-9]]:11: warning: single-argument constructors must be marked explicit
+// CHECK: :[[@LINE-9]]:4: error: unmatched 'NOLIN
+// CHECK: TEND' comment without a previous 'NOLIN
+// CHECK: TBEGIN' comment [clang-tidy-nolint]
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-LIFO.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-LIFO.cpp
@@ -0,0 +1,19 @@
+// RUN: not clang-tidy %s --checks='-*,google-explicit-constructor,google-readability-casting' 2>&1 | FileCheck %s
+
+// NOLINTBEGIN(google-explicit-constructor)
+// NOLINTBEGIN(google-readability-casting)
+class A { A(int i); };
+auto Num = (unsigned int)(-1);
+// NOLINTEND(google-explicit-constructor)
+// NOLINTEND(google-readability-casting)
+
+// Note: the expected output has been split over several lines so that clang-tidy
+// does not see the "no lint" suppression comment and mistakenly assume it
+// is meant for itself.
+// CHECK: :[[@LINE-10]]:4: error: unmatched 'NOLIN
+// CHECK: TBEGIN' comment without a subsequent 'NOLIN
+// CHECK: TEND' comment [clang-tidy-nolint]
+// CHECK: :[[@LINE-11]]:11: warning: single-argument constructors must be marked explicit
+// CHECK: :[[@LINE-10]]:4: error: unmatched 'NOLIN
+// CHECK: TEND' comment without a previous 'NOLIN
+// CHECK: TBEGIN' comment [clang-tidy-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
@@ -30,9 +30,31 @@
class C6 { C6(int i); }; // NOLINT without-brackets-skip-all, another-check
-class C7 { C7(int i); }; // NOLINTNEXTLINE doesn't get misconstrued as a NOLINT
+// Other NOLINT* types (e.g. NEXTLINE) should not be misconstrued as a NOLINT:
+class C7 { C7(int i); }; // NOLINTNEXTLINE
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+// NOLINT must be UPPERCASE:
+class C8 { C8(int i); }; // nolint
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+
+// Unrecognized marker:
+class C9 { C9(int i); }; // NOLINTXYZ
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+
+// C-style comments are supported:
+class C10 { C10(int i); }; /* NOLINT */
+/* NOLINT */ class C11 { C11(int i); };
+
+// Multiple NOLINTs in the same comment:
+class C12 { C12(int i); }; // NOLINT(some-other-check) NOLINT(google-explicit-constructor)
+class C13 { C13(int i); }; // NOLINT(google-explicit-constructor) NOLINT(some-other-check)
+class C14 { C14(int i); }; // NOLINTNEXTLINE(some-other-check) NOLINT(google-explicit-constructor)
+
+// Any text after a NOLINT expression is treated as a comment:
+class C15 { C15(int i); }; // NOLINT: suppress check because <reason>
+class C16 { C16(int i); }; // NOLINT(google-explicit-constructor): suppress check because <reason>
+
void f() {
int i;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unused variable 'i' [clang-diagnostic-unused-variable]
@@ -71,4 +93,4 @@
int array3[10]; // NOLINT(cppcoreguidelines-avoid-c-arrays,modernize-avoid-c-arrays)
int array4[10]; // NOLINT(*-avoid-c-arrays)
-// CHECK-MESSAGES: Suppressed 23 warnings (23 NOLINT)
+// CHECK-MESSAGES: Suppressed 30 warnings (30 NOLINT)
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -60,6 +60,52 @@
}
};
+/// Records the presence of a NOLINT comment - its type, location, checks -
+/// as parsed from the file's character contents.
+struct NoLintToken {
+ enum class Type { Unknown, NoLint, NoLintNextLine, NoLintBegin, NoLintEnd };
+ Type NoLintType = Type::Unknown;
+
+ /// The location of the first character, "N", in "NOLINT".
+ SourceLocation Loc;
+
+ /// Whether this NOLINT comment provides an optional list of checks in
+ /// brackets, i.e. "NOLINT(check1, check2, etc)".
+ bool SpecifiesChecks = false;
+ // NOTE: at first glance, *it may seem* that this bool is redundant when the
+ // `Checks.empty()` method already exists.
+ // However, `Checks.empty()` cannot differentiate "NOLINT" from "NOLINT()".
+ // "NOLINT" should suppress all checks, while "NOLINT()" should suppress zero
+ // checks. In both cases, `Checks.empty()` returns true.
+
+ /// The optional list of checks.
+ StringRef Checks;
+
+ /// Convert a string like "NOLINTNEXTLINE" to its enum `Type::NoLintNextLine`.
+ static Type strToType(StringRef S);
+
+ /// Whether this NOLINT comment applies to the provided `CheckName`.
+ bool suppresses(StringRef CheckName) const;
+};
+
+/// Represents a source range within a pair of NOLINT(BEGIN/END) comments.
+struct NoLintBlockToken {
+ NoLintToken Begin;
+ NoLintToken End;
+
+ /// Whether the two tokens can be paired together to form a block.
+ static bool isValidPair(const NoLintToken &Begin, const NoLintToken &End,
+ const SourceManager &SM);
+
+ /// Whether the provided `Loc` lies between the NOLINT(BEGIN/END) comments.
+ bool contains(SourceLocation Loc, const SourceManager &SM) const;
+
+ /// Whether the provided diagnostic is within and is suppressible by this
+ /// block of NOLINT(BEGIN/END) comments.
+ bool suppresses(SourceLocation DiagLoc, StringRef DiagName,
+ const SourceManager &SM) const;
+};
+
/// Every \c ClangTidyCheck reports errors through a \c DiagnosticsEngine
/// provided by this context.
///
@@ -183,6 +229,13 @@
DiagEngine->getDiagnosticIDs()->getDescription(DiagnosticID)));
}
+ /// Get a list of all NOLINT(BEGIN/END) blocks in the file.
+ /// NOTE: this is a heavy operation, therefore this method returns a cached
+ /// result on subsequent calls.
+ const SmallVector<NoLintBlockToken> &
+ getNoLintBlocks(FileID File, SmallVectorImpl<ClangTidyError> &Errors,
+ bool AllowIO = true) const;
+
private:
// Writes to Stats.
friend class ClangTidyDiagnosticConsumer;
@@ -208,6 +261,8 @@
std::string ProfilePrefix;
bool AllowEnablingAnalyzerAlphaCheckers;
+
+ mutable llvm::DenseMap<FileID, SmallVector<NoLintBlockToken>> NoLintCache;
};
/// Check whether a given diagnostic should be suppressed due to the presence
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -193,7 +193,8 @@
SM.getFileManager().getFile(Error.Message.FilePath);
FileID ID = SM.getOrCreateFileID(*File, SrcMgr::C_User);
SourceLocation FileStartLoc = SM.getLocForStartOfFile(ID);
- SourceLocation Loc = FileStartLoc.getLocWithOffset(Error.Message.FileOffset);
+ SourceLocation Loc = FileStartLoc.getLocWithOffset(
+ static_cast<SourceLocation::IntTy>(Error.Message.FileOffset));
return diag(Error.DiagnosticName, Loc, Error.Message.Message,
static_cast<DiagnosticIDs::Level>(Error.DiagLevel));
}
@@ -305,47 +306,61 @@
LastErrorPassesLineFilter = false;
}
-static bool isNOLINTFound(StringRef NolintDirectiveText, StringRef CheckName,
- StringRef Line, size_t *FoundNolintIndex = nullptr,
- StringRef *FoundNolintChecksStr = nullptr) {
- if (FoundNolintIndex)
- *FoundNolintIndex = StringRef::npos;
- if (FoundNolintChecksStr)
- *FoundNolintChecksStr = StringRef();
-
- size_t NolintIndex = Line.find(NolintDirectiveText);
- if (NolintIndex == StringRef::npos)
- return false;
-
- size_t BracketIndex = NolintIndex + NolintDirectiveText.size();
- if (BracketIndex < Line.size() && isalnum(Line[BracketIndex])) {
- // Reject this search result, otherwise it will cause false positives when
- // NOLINT is found as a substring of NOLINT(NEXTLINE/BEGIN/END).
- return false;
- }
-
- // Check if specific checks are specified in brackets.
- if (BracketIndex < Line.size() && Line[BracketIndex] == '(') {
- ++BracketIndex;
- const size_t BracketEndIndex = Line.find(')', BracketIndex);
- if (BracketEndIndex != StringRef::npos) {
- StringRef ChecksStr =
- Line.substr(BracketIndex, BracketEndIndex - BracketIndex);
- if (FoundNolintChecksStr)
- *FoundNolintChecksStr = ChecksStr;
- // Allow specifying a few checks with a glob expression, ignoring
- // negative globs (which would effectively disable the suppression).
- GlobList Globs(ChecksStr, /*KeepNegativeGlobs=*/false);
- if (!Globs.contains(CheckName))
- return false;
- }
- }
-
- if (FoundNolintIndex)
- *FoundNolintIndex = NolintIndex;
-
- return true;
-}
+/// Return a list of all NOLINT comments in the provided `Buffer`.
+/// `Buffer` can be a line or the entire file contents.
+static SmallVector<NoLintToken> findNoLints(StringRef Buffer,
+ SourceLocation BufferLoc) {
+ static constexpr StringRef NoLintStr = "NOLINT";
+ SmallVector<NoLintToken> NoLints;
+ size_t Pos = 0;
+
+ // Keep checking for NOLINTs until all of them are found, or the buffer is
+ // exhausted.
+ while (Pos < Buffer.size()) {
+ // Look for the next "NOLINT".
+ const size_t NoLintPos = Buffer.find(NoLintStr, Pos);
+ if (NoLintPos == StringRef::npos)
+ break;
+ NoLintToken NoLint;
+ NoLint.Loc = BufferLoc.getLocWithOffset(
+ static_cast<SourceLocation::IntTy>(NoLintPos));
+
+ // Look for characters immediately after the "NOLINT" (e.g. the "NEXTLINE"
+ // in "NOLINTNEXTLINE").
+ Pos = NoLintPos + NoLintStr.size();
+ while (Pos < Buffer.size() && isalpha(Buffer[Pos]) != 0)
+ ++Pos;
+ const StringRef NoLintType = Buffer.slice(NoLintPos, Pos);
+ NoLint.NoLintType = NoLintToken::strToType(NoLintType);
+ if (NoLint.NoLintType == NoLintToken::Type::Unknown)
+ continue;
+
+ // Look for "(checks-list)" immediately after.
+ if (Pos < Buffer.size() && Buffer[Pos] == '(') {
+ const size_t ChecksPos = ++Pos;
+ // Look for closing bracket (or end of the line - whichever comes first).
+ while (Pos < Buffer.size() && Buffer[Pos] != ')' && Buffer[Pos] != '\n')
+ ++Pos;
+ if (Pos < Buffer.size() && Buffer[Pos] == ')') {
+ NoLint.Checks = Buffer.slice(ChecksPos, Pos);
+ NoLint.SpecifiesChecks = true;
+ }
+ }
+
+ NoLints.emplace_back(NoLint);
+ }
+
+ return NoLints;
+}
+
+static bool isNoLintFound(StringRef Line, NoLintToken::Type NoLintType,
+ StringRef CheckName) {
+ SmallVector<NoLintToken> NoLints = findNoLints(Line, SourceLocation());
+ return std::any_of(
+ NoLints.begin(), NoLints.end(), [&](const NoLintToken &NoLint) {
+ return NoLint.NoLintType == NoLintType && NoLint.suppresses(CheckName);
+ });
+}
static llvm::Optional<StringRef> getBuffer(const SourceManager &SM, FileID File,
bool AllowIO) {
@@ -355,94 +370,86 @@
static ClangTidyError createNolintError(const ClangTidyContext &Context,
const SourceManager &SM,
- SourceLocation Loc,
- bool IsNolintBegin) {
+ const NoLintToken &NoLint) {
ClangTidyError Error("clang-tidy-nolint", ClangTidyError::Error,
Context.getCurrentBuildDirectory(), false);
StringRef Message =
- IsNolintBegin
+ NoLint.NoLintType == NoLintToken::Type::NoLintBegin
? ("unmatched 'NOLINTBEGIN' comment without a subsequent 'NOLINT"
"END' comment")
: ("unmatched 'NOLINTEND' comment without a previous 'NOLINT"
"BEGIN' comment");
- Error.Message = tooling::DiagnosticMessage(Message, SM, Loc);
+ Error.Message = tooling::DiagnosticMessage(Message, SM, NoLint.Loc);
return Error;
}
-static Optional<ClangTidyError> tallyNolintBegins(
- const ClangTidyContext &Context, const SourceManager &SM,
- StringRef CheckName, SmallVector<StringRef> Lines, SourceLocation LinesLoc,
- SmallVector<std::pair<SourceLocation, StringRef>> &NolintBegins) {
- // Keep a running total of how many NOLINT(BEGIN...END) blocks are active, as
- // well as the bracket expression (if any) that was used in the NOLINT
- // expression.
- size_t NolintIndex;
- StringRef NolintChecksStr;
- for (const auto &Line : Lines) {
- if (isNOLINTFound("NOLINTBEGIN", CheckName, Line, &NolintIndex,
- &NolintChecksStr)) {
- // Check if a new block is being started.
- NolintBegins.emplace_back(std::make_pair(
- LinesLoc.getLocWithOffset(NolintIndex), NolintChecksStr));
- } else if (isNOLINTFound("NOLINTEND", CheckName, Line, &NolintIndex,
- &NolintChecksStr)) {
- // Check if the previous block is being closed.
- if (!NolintBegins.empty() &&
- NolintBegins.back().second == NolintChecksStr) {
- NolintBegins.pop_back();
- } else {
- // Trying to close a nonexistent block. Return a diagnostic about this
- // misuse that can be displayed along with the original clang-tidy check
- // that the user was attempting to suppress.
- return createNolintError(Context, SM,
- LinesLoc.getLocWithOffset(NolintIndex), false);
- }
- }
- // Advance source location to the next line.
- LinesLoc = LinesLoc.getLocWithOffset(Line.size() + sizeof('\n'));
- }
- return None; // All NOLINT(BEGIN/END) use has been consistent so far.
-}
-
-static bool
-lineIsWithinNolintBegin(const ClangTidyContext &Context,
- SmallVectorImpl<ClangTidyError> &SuppressionErrors,
- const SourceManager &SM, SourceLocation Loc,
- StringRef CheckName, StringRef TextBeforeDiag,
- StringRef TextAfterDiag) {
- Loc = SM.getExpansionRange(Loc).getBegin();
- SourceLocation FileStartLoc = SM.getLocForStartOfFile(SM.getFileID(Loc));
- SmallVector<std::pair<SourceLocation, StringRef>> NolintBegins;
-
- // Check if there's an open NOLINT(BEGIN...END) block on the previous lines.
- SmallVector<StringRef> PrevLines;
- TextBeforeDiag.split(PrevLines, '\n');
- auto Error = tallyNolintBegins(Context, SM, CheckName, PrevLines,
- FileStartLoc, NolintBegins);
- if (Error) {
- SuppressionErrors.emplace_back(Error.getValue());
- }
- bool WithinNolintBegin = !NolintBegins.empty();
-
- // Check that every block is terminated correctly on the following lines.
- SmallVector<StringRef> FollowingLines;
- TextAfterDiag.split(FollowingLines, '\n');
- Error = tallyNolintBegins(Context, SM, CheckName, FollowingLines, Loc,
- NolintBegins);
- if (Error) {
- SuppressionErrors.emplace_back(Error.getValue());
- }
-
- // The following blocks were never closed. Return diagnostics for each
- // instance that can be displayed along with the original clang-tidy check
- // that the user was attempting to suppress.
- for (const auto &NolintBegin : NolintBegins) {
- SuppressionErrors.emplace_back(
- createNolintError(Context, SM, NolintBegin.first, true));
- }
-
- return WithinNolintBegin && SuppressionErrors.empty();
-}
+/// Look through the entire `Buffer` and keep track of all NOLINT(BEGIN/END)
+/// blocks. If any invalid blocks are found, e.g. a `BEGIN` comment without a
+/// corresponding `END`, return a diagnostic about it in `Errors`.
+static SmallVector<NoLintBlockToken>
+collectNoLintBlocks(const ClangTidyContext &Context, const SourceManager &SM,
+ StringRef Buffer, SourceLocation BufferLoc,
+ SmallVectorImpl<ClangTidyError> &Errors) {
+ const SmallVector<NoLintToken> NoLints = findNoLints(Buffer, BufferLoc);
+ SmallVector<const NoLintToken *> NoLintBegins;
+ SmallVector<NoLintBlockToken> CompletedBlocks;
+
+ for (const NoLintToken &NoLint : NoLints) {
+ // Check if a new block is being started. Add it to the stack.
+ if (NoLint.NoLintType == NoLintToken::Type::NoLintBegin)
+ NoLintBegins.emplace_back(&NoLint);
+ else if (NoLint.NoLintType == NoLintToken::Type::NoLintEnd) {
+ // The previous block is being closed. Pop one element off the stack.
+ if (NoLintBegins.empty() ||
+ !NoLintBlockToken::isValidPair(*NoLintBegins.back(), NoLint, SM)) {
+ // Trying to close the wrong block. Return a diagnostic about this
+ // misuse that can be displayed along with the original clang-tidy check
+ // that the user was attempting to suppress.
+ ClangTidyError Error = createNolintError(Context, SM, NoLint);
+ Errors.emplace_back(Error);
+ continue;
+ }
+ NoLintBlockToken Block{*NoLintBegins.back(), NoLint};
+ CompletedBlocks.emplace_back(Block);
+ NoLintBegins.pop_back();
+ }
+ }
+
+ // Check that every block has been closed by the end of the file.
+ for (const NoLintToken *NoLintBegin : NoLintBegins) {
+ ClangTidyError Error = createNolintError(Context, SM, *NoLintBegin);
+ Errors.emplace_back(Error);
+ }
+
+ return CompletedBlocks;
+}
+
+const SmallVector<NoLintBlockToken> &ClangTidyContext::getNoLintBlocks(
+ FileID File, SmallVectorImpl<ClangTidyError> &Errors, bool AllowIO) const {
+ // Check cache first.
+ const auto Cache = NoLintCache.find(File);
+ if (Cache != NoLintCache.end())
+ return Cache->second;
+
+ // This file has not been searched yet. Warning: heavy operation below. Need
+ // to read the whole file.
+ auto &NewCache = NoLintCache.FindAndConstruct(File);
+ const SourceManager &SM = DiagEngine->getSourceManager();
+ Optional<StringRef> Buffer = getBuffer(SM, File, AllowIO);
+ if (Buffer) {
+ NewCache.second = collectNoLintBlocks(
+ *this, SM, *Buffer, SM.getLocForStartOfFile(File), Errors);
+ }
+ return NewCache.second;
+}
+
+/// Look behind and ahead for '\n' characters. These mark the start and end of
+/// this line.
+static std::pair<size_t, size_t> getLineStartAndEnd(StringRef S, size_t From) {
+ size_t StartPos = S.find_last_of('\n', From) + 1;
+ size_t EndPos = std::min(S.find('\n', From), S.size());
+ return std::make_pair(StartPos, EndPos);
+}
static bool
lineIsMarkedWithNOLINT(const ClangTidyContext &Context,
@@ -451,37 +458,37 @@
SourceLocation Loc, StringRef CheckName,
bool EnableNolintBlocks) {
// Get source code for this location.
+ Loc = SM.getSpellingLoc(Loc);
FileID File;
unsigned Offset;
- std::tie(File, Offset) = SM.getDecomposedSpellingLoc(Loc);
+ std::tie(File, Offset) = SM.getDecomposedLoc(Loc);
Optional<StringRef> Buffer = getBuffer(SM, File, AllowIO);
if (!Buffer)
return false;
// Check if there's a NOLINT on this line.
- StringRef TextAfterDiag = Buffer->substr(Offset);
- StringRef RestOfThisLine, FollowingLines;
- std::tie(RestOfThisLine, FollowingLines) = TextAfterDiag.split('\n');
- if (isNOLINTFound("NOLINT", CheckName, RestOfThisLine))
- return true;
+ auto ThisLinePos = getLineStartAndEnd(*Buffer, Offset);
+ StringRef ThisLine = Buffer->slice(ThisLinePos.first, ThisLinePos.second);
+ if (isNoLintFound(ThisLine, NoLintToken::Type::NoLint, CheckName))
+ return true;
// Check if there's a NOLINTNEXTLINE on the previous line.
- StringRef TextBeforeDiag = Buffer->substr(0, Offset);
- size_t LastNewLinePos = TextBeforeDiag.rfind('\n');
- StringRef PrevLines = (LastNewLinePos == StringRef::npos)
- ? StringRef()
- : TextBeforeDiag.slice(0, LastNewLinePos);
- LastNewLinePos = PrevLines.rfind('\n');
- StringRef PrevLine = (LastNewLinePos == StringRef::npos)
- ? PrevLines
- : PrevLines.substr(LastNewLinePos + 1);
- if (isNOLINTFound("NOLINTNEXTLINE", CheckName, PrevLine))
- return true;
+ if (ThisLinePos.first > 0) {
+ auto PrevLinePos = getLineStartAndEnd(*Buffer, ThisLinePos.first - 1);
+ StringRef PrevLine = Buffer->slice(PrevLinePos.first, PrevLinePos.second);
+ if (isNoLintFound(PrevLine, NoLintToken::Type::NoLintNextLine, CheckName))
+ return true;
+ }
// Check if this line is within a NOLINT(BEGIN...END) block.
- return EnableNolintBlocks &&
- lineIsWithinNolintBegin(Context, SuppressionErrors, SM, Loc, CheckName,
- TextBeforeDiag, TextAfterDiag);
+ if (!EnableNolintBlocks)
+ return false;
+ const SmallVector<NoLintBlockToken> &NoLintBlocks =
+ Context.getNoLintBlocks(File, SuppressionErrors, AllowIO);
+ return std::any_of(NoLintBlocks.begin(), NoLintBlocks.end(),
+ [&](const NoLintBlockToken &NoLintBlock) {
+ return NoLintBlock.suppresses(Loc, CheckName, SM);
+ });
}
static bool lineIsMarkedWithNOLINTinMacro(
@@ -535,6 +542,52 @@
return Result;
}
+NoLintToken::Type NoLintToken::strToType(StringRef S) {
+ static const llvm::StringMap<Type> StrMapping = {
+ {"NOLINT", Type::NoLint},
+ {"NOLINTNEXTLINE", Type::NoLintNextLine},
+ {"NOLINTBEGIN", Type::NoLintBegin},
+ {"NOLINTEND", Type::NoLintEnd}};
+ auto It = StrMapping.find(S);
+ if (It == StrMapping.end())
+ return Type::Unknown;
+ return It->second;
+}
+
+bool NoLintToken::suppresses(StringRef CheckName) const {
+ // NOLINT supports an optional list of checks.
+ // If the list is omitted, the default behavior is to suppresses all checks.
+ assert(SpecifiesChecks || Checks.empty());
+ if (!SpecifiesChecks)
+ return true;
+ GlobList Globs(Checks, /*KeepNegativeGlobs=*/false);
+ return Globs.contains(CheckName);
+}
+
+bool NoLintBlockToken::isValidPair(const NoLintToken &Begin,
+ const NoLintToken &End,
+ const SourceManager &SM) {
+ return (Begin.NoLintType == NoLintToken::Type::NoLintBegin) &&
+ (End.NoLintType == NoLintToken::Type::NoLintEnd) &&
+ (SM.getFileID(Begin.Loc) == SM.getFileID(End.Loc)) &&
+ (Begin.Loc < End.Loc) &&
+ (Begin.SpecifiesChecks == End.SpecifiesChecks) &&
+ (Begin.Checks == End.Checks);
+}
+
+bool NoLintBlockToken::contains(SourceLocation Loc,
+ const SourceManager &SM) const {
+ if (!isValidPair(Begin, End, SM))
+ return false;
+ bool InSameFile = SM.getFileID(Loc) == SM.getFileID(Begin.Loc);
+ return InSameFile && (Begin.Loc < Loc) && (Loc < End.Loc);
+}
+
+bool NoLintBlockToken::suppresses(SourceLocation DiagLoc, StringRef DiagName,
+ const SourceManager &SM) const {
+ return contains(DiagLoc, SM) && Begin.suppresses(DiagName);
+}
+
} // namespace tidy
} // namespace clang
@@ -546,8 +599,13 @@
SmallVector<ClangTidyError, 1> SuppressionErrors;
if (shouldSuppressDiagnostic(DiagLevel, Info, Context, SuppressionErrors,
EnableNolintBlocks)) {
- ++Context.Stats.ErrorsIgnoredNOLINT;
+ // Even though this diagnostic is suppressed, there may be other issues with
+ // the file that need raising, such as unclosed NOLINT(BEGIN/END) blocks.
+ Context.DiagEngine->Clear();
+ for (const auto &Error : SuppressionErrors)
+ Context.diag(Error);
// Ignored a warning, should ignore related notes as well
+ ++Context.Stats.ErrorsIgnoredNOLINT;
LastErrorWasIgnored = true;
return;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
