diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h @@ -99,6 +99,8 @@ DiagnosticBuilder diag(StringRef CheckName, StringRef Message, DiagnosticIDs::Level Level = DiagnosticIDs::Warning); + DiagnosticBuilder diag(const ClangTidyError &Error); + /// Report any errors to do with reading the configuration using this method. DiagnosticBuilder configurationDiag(StringRef Message, @@ -165,7 +167,7 @@ } /// Returns build directory of the current translation unit. - const std::string &getCurrentBuildDirectory() { + const std::string &getCurrentBuildDirectory() const { return CurrentBuildDirectory; } @@ -217,15 +219,24 @@ /// This is exposed so that other tools that present clang-tidy diagnostics /// (such as clangd) can respect the same suppression rules as clang-tidy. /// This does not handle suppression of notes following a suppressed diagnostic; -/// that is left to the caller is it requires maintaining state in between calls +/// that is left to the caller as it requires maintaining state in between calls /// to this function. /// If `AllowIO` is false, the function does not attempt to read source files /// from disk which are not already mapped into memory; such files are treated /// as not containing a suppression comment. +/// If suppression is not possible due to improper use of "NOLINT" comments - +/// for example, the use of a "NOLINTBEGIN" comment that is not followed by a +/// "NOLINTEND" comment - a diagnostic regarding the improper use is returned +/// via the output argument `SuppressionErrors`. bool shouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info, ClangTidyContext &Context, bool AllowIO = true); +bool shouldSuppressDiagnostic( + DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info, + ClangTidyContext &Context, + SmallVectorImpl &SuppressionErrors, bool AllowIO = true); + /// Gets the Fix attached to \p Diagnostic. /// If there isn't a Fix attached to the diagnostic and \p AnyFix is true, Check /// to see if exactly one note has a Fix and return it. Otherwise return diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -23,6 +23,7 @@ #include "clang/AST/Attr.h" #include "clang/Basic/Diagnostic.h" #include "clang/Basic/DiagnosticOptions.h" +#include "clang/Basic/FileManager.h" #include "clang/Basic/SourceManager.h" #include "clang/Frontend/DiagnosticRenderer.h" #include "clang/Lex/Lexer.h" @@ -202,6 +203,17 @@ return DiagEngine->Report(ID); } +DiagnosticBuilder ClangTidyContext::diag(const ClangTidyError &Error) { + SourceManager &SM = DiagEngine->getSourceManager(); + llvm::ErrorOr File = + 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); + return diag(Error.DiagnosticName, Loc, Error.Message.Message, + static_cast(Error.DiagLevel)); +} + DiagnosticBuilder ClangTidyContext::configurationDiag( StringRef Message, DiagnosticIDs::Level Level /* = DiagnosticIDs::Warning*/) { @@ -307,14 +319,26 @@ LastErrorPassesLineFilter = false; } -static bool IsNOLINTFound(StringRef NolintDirectiveText, StringRef Line, - unsigned DiagID, const ClangTidyContext &Context) { - const size_t NolintIndex = Line.find(NolintDirectiveText); +static bool isNOLINTFound(StringRef NolintDirectiveText, StringRef CheckName, + StringRef Line, size_t *FoundNolintIndex = nullptr, + bool *SuppressionIsSpecific = nullptr) { + if (FoundNolintIndex) + *FoundNolintIndex = StringRef::npos; + if (SuppressionIsSpecific) + *SuppressionIsSpecific = false; + + size_t NolintIndex = Line.find(NolintDirectiveText); if (NolintIndex == StringRef::npos) return false; size_t BracketIndex = NolintIndex + NolintDirectiveText.size(); - // Check if the specific checks are specified in brackets. + 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); @@ -323,16 +347,22 @@ Line.substr(BracketIndex, BracketEndIndex - BracketIndex); // Allow disabling all the checks with "*". if (ChecksStr != "*") { - std::string CheckName = Context.getCheckName(DiagID); // Allow specifying a few check names, delimited with comma. SmallVector Checks; ChecksStr.split(Checks, ',', -1, false); llvm::transform(Checks, Checks.begin(), [](StringRef S) { return S.trim(); }); - return llvm::find(Checks, CheckName) != Checks.end(); + if (llvm::find(Checks, CheckName) == Checks.end()) + return false; + if (SuppressionIsSpecific) + *SuppressionIsSpecific = true; } } } + + if (FoundNolintIndex) + *FoundNolintIndex = NolintIndex; + return true; } @@ -342,34 +372,154 @@ : SM.getBufferDataIfLoaded(File); } -static bool LineIsMarkedWithNOLINT(const SourceManager &SM, SourceLocation Loc, - unsigned DiagID, - const ClangTidyContext &Context, - bool AllowIO) { +static ClangTidyError createNolintError(const ClangTidyContext &Context, + const SourceManager &SM, + SourceLocation Loc, + bool IsNolintBegin) { + ClangTidyError Error("clang-tidy-nolint", ClangTidyError::Error, + Context.getCurrentBuildDirectory(), false); + StringRef Message = + IsNolintBegin + ? "unmatched 'NOLINTBEGIN' comment without a subsequent 'NOLINTEND' " + "comment" + : "unmatched 'NOLINTEND' comment without a previous 'NOLINTBEGIN' " + "comment"; + Error.Message = tooling::DiagnosticMessage(Message, SM, Loc); + return Error; +} + +static Optional +tallyNolintBegins(const ClangTidyContext &Context, const SourceManager &SM, + StringRef CheckName, SmallVector Lines, + SourceLocation LinesLoc, + SmallVector &SpecificNolintBegins, + SmallVector &GlobalNolintBegins) { + // Keep a running total of how many NOLINT(BEGIN...END) blocks are active. + size_t NolintIndex; + bool SuppressionIsSpecific; + auto List = [&]() -> SmallVector * { + return SuppressionIsSpecific ? &SpecificNolintBegins : &GlobalNolintBegins; + }; + for (const auto &Line : Lines) { + if (isNOLINTFound("NOLINTBEGIN", CheckName, Line, &NolintIndex, + &SuppressionIsSpecific)) { + // Check if a new block is being started. + List()->emplace_back(LinesLoc.getLocWithOffset(NolintIndex)); + } else if (isNOLINTFound("NOLINTEND", CheckName, Line, &NolintIndex, + &SuppressionIsSpecific)) { + // Check if the previous block is being closed. + if (!List()->empty()) { + List()->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 &SuppressionErrors, + const SourceManager &SM, SourceLocation Loc, + StringRef CheckName, StringRef TextBeforeDiag, + StringRef TextAfterDiag) { + Loc = SM.getExpansionRange(Loc).getBegin(); + SourceLocation FileStartLoc = SM.getLocForStartOfFile(SM.getFileID(Loc)); + + // Check if there's an open NOLINT(BEGIN...END) block on the previous lines. + SmallVector PrevLines; + TextBeforeDiag.split(PrevLines, '\n'); + SmallVector SpecificNolintBegins; + SmallVector GlobalNolintBegins; + auto Error = + tallyNolintBegins(Context, SM, CheckName, PrevLines, FileStartLoc, + SpecificNolintBegins, GlobalNolintBegins); + if (Error) { + SuppressionErrors.emplace_back(Error.getValue()); + return false; + } + bool WithinNolintBegin = + !SpecificNolintBegins.empty() || !GlobalNolintBegins.empty(); + + // Check that every block is terminated correctly on the following lines. + SmallVector FollowingLines; + TextAfterDiag.split(FollowingLines, '\n'); + Error = tallyNolintBegins(Context, SM, CheckName, FollowingLines, Loc, + SpecificNolintBegins, GlobalNolintBegins); + if (Error) { + SuppressionErrors.emplace_back(Error.getValue()); + return false; + } + + // 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 : SpecificNolintBegins) { + auto Error = createNolintError(Context, SM, NolintBegin, true); + SuppressionErrors.emplace_back(Error); + } + for (const auto NolintBegin : GlobalNolintBegins) { + auto Error = createNolintError(Context, SM, NolintBegin, true); + SuppressionErrors.emplace_back(Error); + } + + return WithinNolintBegin && SuppressionErrors.empty(); +} + +static bool +lineIsMarkedWithNOLINT(const ClangTidyContext &Context, + SmallVectorImpl &SuppressionErrors, + bool AllowIO, const SourceManager &SM, + SourceLocation Loc, StringRef CheckName) { + // Get source code for this location. FileID File; unsigned Offset; std::tie(File, Offset) = SM.getDecomposedSpellingLoc(Loc); - llvm::Optional Buffer = getBuffer(SM, File, AllowIO); + Optional Buffer = getBuffer(SM, File, AllowIO); if (!Buffer) return false; // Check if there's a NOLINT on this line. - StringRef RestOfLine = Buffer->substr(Offset).split('\n').first; - if (IsNOLINTFound("NOLINT", RestOfLine, DiagID, Context)) + StringRef TextAfterDiag = Buffer->substr(Offset); + StringRef RestOfThisLine, FollowingLines; + std::tie(RestOfThisLine, FollowingLines) = TextAfterDiag.split('\n'); + if (isNOLINTFound("NOLINT", CheckName, RestOfThisLine)) return true; // 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); + 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; + + // Check if this line is within a NOLINT(BEGIN...END) block. + return lineIsWithinNolintBegin(Context, SuppressionErrors, SM, Loc, CheckName, + TextBeforeDiag, TextAfterDiag); } -static bool LineIsMarkedWithNOLINTinMacro(const SourceManager &SM, - SourceLocation Loc, unsigned DiagID, - const ClangTidyContext &Context, - bool AllowIO) { +static bool lineIsMarkedWithNOLINTinMacro( + const Diagnostic &Info, const ClangTidyContext &Context, + SmallVectorImpl &SuppressionErrors, bool AllowIO) { + const SourceManager &SM = Info.getSourceManager(); + SourceLocation Loc = Info.getLocation(); + std::string CheckName = Context.getCheckName(Info.getID()); while (true) { - if (LineIsMarkedWithNOLINT(SM, Loc, DiagID, Context, AllowIO)) + if (lineIsMarkedWithNOLINT(Context, SuppressionErrors, AllowIO, SM, Loc, + CheckName)) return true; if (!Loc.isMacroID()) return false; @@ -384,12 +534,22 @@ bool shouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info, ClangTidyContext &Context, bool AllowIO) { + SmallVector Unused; + bool ShouldSuppress = + shouldSuppressDiagnostic(DiagLevel, Info, Context, Unused, AllowIO); + assert(Unused.empty()); + return ShouldSuppress; +} + +bool shouldSuppressDiagnostic( + DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info, + ClangTidyContext &Context, + SmallVectorImpl &SuppressionErrors, bool AllowIO) { return Info.getLocation().isValid() && DiagLevel != DiagnosticsEngine::Error && DiagLevel != DiagnosticsEngine::Fatal && - LineIsMarkedWithNOLINTinMacro(Info.getSourceManager(), - Info.getLocation(), Info.getID(), - Context, AllowIO); + lineIsMarkedWithNOLINTinMacro(Info, Context, SuppressionErrors, + AllowIO); } const llvm::StringMap * @@ -418,7 +578,8 @@ if (LastErrorWasIgnored && DiagLevel == DiagnosticsEngine::Note) return; - if (shouldSuppressDiagnostic(DiagLevel, Info, Context)) { + SmallVector SuppressionErrors; + if (shouldSuppressDiagnostic(DiagLevel, Info, Context, SuppressionErrors)) { ++Context.Stats.ErrorsIgnoredNOLINT; // Ignored a warning, should ignore related notes as well LastErrorWasIgnored = true; @@ -492,6 +653,10 @@ if (Info.hasSourceManager()) checkFilters(Info.getLocation(), Info.getSourceManager()); + + Context.DiagEngine->Clear(); + for (const auto &Error : SuppressionErrors) + Context.diag(Error); } bool ClangTidyDiagnosticConsumer::passesLineFilter(StringRef FileName, diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -67,7 +67,8 @@ Improvements to clang-tidy -------------------------- -The improvements are... +- Added support for `NOLINTBEGIN` ... `NOLINTEND` comments to suppress + Clang-Tidy warnings over multiple lines. New checks ^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/index.rst b/clang-tools-extra/docs/clang-tidy/index.rst --- a/clang-tools-extra/docs/clang-tidy/index.rst +++ b/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 +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 an 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 checks for all lines between the BEGIN and 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,22 @@ lint-command: **NOLINT** **NOLINTNEXTLINE** + **NOLINTBEGIN** + **NOLINTEND** -Note that whitespaces between ``NOLINT``/``NOLINTNEXTLINE`` and the opening +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``), whereas in check names list (inside the -parenthesis) whitespaces can be used and will be ignored. +``NOLINT``/``NOLINTNEXTLINE``/``NOLINTBEGIN``/``NOLINTEND``), whereas in the +check names list (inside the parentheses), whitespaces can be used and will be +ignored. + +All ``NOLINTBEGIN`` comments must be paired by an equal number of ``NOLINTEND`` +comments. Moreover, a pair of comments must have matching arguments -- for +example, ``NOLINTBEGIN(check-name)`` can be paired with +``NOLINTEND(check-name)`` but not with ``NOLINTEND`` `(zero arguments)`. +:program:`clang-tidy` will generate a ``clang-tidy-nolint`` error diagnostic if +any ``NOLINTBEGIN``/``NOLINTEND`` comment violates these requirements. .. _LibTooling: https://clang.llvm.org/docs/LibTooling.html .. _How To Setup Tooling For LLVM: https://clang.llvm.org/docs/HowToSetupToolingForLLVM.html diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/error_in_include.inc b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/error_in_include.inc new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/error_in_include.inc @@ -0,0 +1 @@ +class G { G(int i); }; diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/nolint_in_include.inc b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/nolint_in_include.inc new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/nolint_in_include.inc @@ -0,0 +1,3 @@ +// NOLINTBEGIN +class H { H(int i); }; +// NOLINTEND diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp --- a/clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp +++ b/clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp @@ -27,6 +27,9 @@ class C5 { C5(int i); }; // NOLINT without-brackets-skip-all, another-check +class C6 { C6(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() { int i; // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unused variable 'i' [clang-diagnostic-unused-variable] diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-at-eof.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-at-eof.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-at-eof.cpp @@ -0,0 +1,12 @@ +// RUN: not clang-tidy %s --checks='-*,google-explicit-constructor' 2>&1 | FileCheck %s + +// CHECK: :[[@LINE+8]]:11: warning: single-argument constructors must be marked explicit +// 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+5]]:4: error: unmatched 'NOLIN +// CHECK: TBEGIN' comment without a subsequent 'NOLIN +// CHECK: TEND' comment [clang-tidy-nolint] + +class A { A(int i); }; +// NOLINTBEGIN \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-global-end-specific.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-global-end-specific.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-global-end-specific.cpp @@ -0,0 +1,13 @@ +// 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] diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-global.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-global.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-global.cpp @@ -0,0 +1,13 @@ +// 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] diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-without-end.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-without-end.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-without-end.cpp @@ -0,0 +1,12 @@ +// RUN: not clang-tidy %s --checks='-*,google-explicit-constructor' 2>&1 | FileCheck %s + +// NOLINTBEGIN +class A { A(int i); }; + +// 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]]:4: error: unmatched 'NOLIN +// CHECK: TBEGIN' comment without a subsequent 'NOLIN +// CHECK: TEND' comment [clang-tidy-nolint] +// CHECK: :[[@LINE-8]]:11: warning: single-argument constructors must be marked explicit diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-end-at-sof.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-end-at-sof.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-end-at-sof.cpp @@ -0,0 +1,12 @@ +// NOLINTEND +class A { A(int i); }; + +// 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]]:4: error: unmatched 'NOLIN +// CHECK: TEND' comment without a previous 'NOLIN +// CHECK: TBEGIN' comment [clang-tidy-nolint] +// CHECK: :[[@LINE-8]]:11: warning: single-argument constructors must be marked explicit + +// RUN: not clang-tidy %s --checks='-*,google-explicit-constructor' 2>&1 | FileCheck %s diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-end-without-begin.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-end-without-begin.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-end-without-begin.cpp @@ -0,0 +1,12 @@ +// RUN: not clang-tidy %s --checks='-*,google-explicit-constructor' 2>&1 | FileCheck %s + +// NOLINTEND +class A { A(int i); }; + +// 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]]:4: error: unmatched 'NOLIN +// CHECK: TEND' comment without a previous 'NOLIN +// CHECK: TBEGIN' comment [clang-tidy-nolint] +// CHECK: :[[@LINE-8]]:11: warning: single-argument constructors must be marked explicit diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-mismatched-check-names.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-mismatched-check-names.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-mismatched-check-names.cpp @@ -0,0 +1,18 @@ +// RUN: not clang-tidy %s --checks='-*,google-explicit-constructor,google-readability-casting' 2>&1 | FileCheck %s + +// NOLINTBEGIN(google-explicit-constructor) +class A { A(int i); }; +auto Num = (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-8]]:4: error: unmatched 'NOLIN +// CHECK: TBEGIN' comment without a subsequent 'NOLIN +// CHECK: TEND' comment [clang-tidy-nolint] +// CHECK: :[[@LINE-10]]:11: warning: single-argument constructors must be marked explicit +// CHECK: :[[@LINE-10]]:12: warning: C-style casts are discouraged; use static_cast +// CHECK: :[[@LINE-10]]:4: error: unmatched 'NOLIN +// CHECK: TEND' comment without a previous 'NOLIN +// CHECK: TBEGIN' comment [clang-tidy-nolint] diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-mismatched-delims.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-mismatched-delims.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-mismatched-delims.cpp @@ -0,0 +1,14 @@ +// RUN: not clang-tidy %s --checks='-*,google-explicit-constructor' 2>&1 | FileCheck %s + +// NOLINTBEGIN +// NOLINTBEGIN +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-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 diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-typo-in-check-name.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-typo-in-check-name.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-typo-in-check-name.cpp @@ -0,0 +1,13 @@ +// RUN: not clang-tidy %s --checks='-*,google-explicit-constructor' 2>&1 | FileCheck %s + +// NOLINTBEGIN(google-explicit-constructor) +class A { A(int i); }; +// NOLINTEND(google-explicit-constructo) <-- typo: missing 'r' + +// 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 diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp @@ -0,0 +1,127 @@ +// 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 + +// NOLINTBEGIN +class B1 { B1(int i); }; +// NOLINTEND + +// NOLINTBEGIN +// NOLINTEND +// NOLINTBEGIN +class B2 { B2(int i); }; +// NOLINTEND + +// NOLINTBEGIN +// NOLINTBEGIN +class B3 { B3(int i); }; +// NOLINTEND +// NOLINTEND + +// NOLINTBEGIN +// NOLINTBEGIN +// NOLINTEND +class B4 { B4(int i); }; +// NOLINTEND + +// NOLINTBEGIN +// NOLINTEND +class B5 { B5(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(google-explicit-constructor) +// NOLINTBEGIN(some-other-check) +class C8 { C8(int i); }; +// NOLINTEND(google-explicit-constructor) +// NOLINTEND(some-other-check) + +// NOLINTBEGIN(google-explicit-constructor) +// NOLINTBEGIN +class C9 { C9(int i); }; +// NOLINTEND +// NOLINTEND(google-explicit-constructor) + +// NOLINTBEGIN +// NOLINTBEGIN(google-explicit-constructor) +class C10 { C10(int i); }; +// NOLINTEND(google-explicit-constructor) +// NOLINTEND + +// NOLINTBEGIN(not-closed-bracket-is-treated-as-skip-all +class C11 { C11(int i); }; +// NOLINTEND(not-closed-bracket-is-treated-as-skip-all + +// NOLINTBEGIN without-brackets-skip-all, another-check +class C12 { C12(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 +// CHECK-MESSAGES: :[[@LINE-4]]:28: note: expanded from macro 'MACRO + +// NOLINTBEGIN +MACRO(D2) +// NOLINTEND + +#define MACRO_NOARG class E { E(int i); }; + +// NOLINTBEGIN +MACRO_NOARG +// NOLINTEND + +#include "error_in_include.inc" +// CHECK-MESSAGES: error_in_include.inc:1:11: warning: single-argument constructors must be marked explicit + +#include "nolint_in_include.inc" + +// NOLINTBEGIN +#define MACRO_WRAPPED_WITH_NO_LINT class I { I(int i); }; +// NOLINTEND + +MACRO_WRAPPED_WITH_NO_LINT + +#define MACRO_NO_LINT_INSIDE_MACRO \ + /* NOLINTBEGIN */ \ + class J { J(int i); }; \ + /* NOLINTEND */ + +MACRO_NO_LINT_INSIDE_MACRO + +// CHECK-MESSAGES: Suppressed 19 warnings (19 NOLINT). diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp --- a/clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp +++ b/clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp @@ -1,49 +1,51 @@ +// NOLINTNEXTLINE class A { A(int i); }; + +class B { B(int i); }; // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit // NOLINTNEXTLINE -class B { B(int i); }; +class C { C(int i); }; // NOLINTNEXTLINE(for-some-other-check) -class C { C(int i); }; +class D { D(int i); }; // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit // NOLINTNEXTLINE(*) -class C1 { C1(int i); }; +class D1 { D1(int i); }; // NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all -class C2 { C2(int i); }; +class D2 { D2(int i); }; // NOLINTNEXTLINE(google-explicit-constructor) -class C3 { C3(int i); }; +class D3 { D3(int i); }; // NOLINTNEXTLINE(some-check, google-explicit-constructor) -class C4 { C4(int i); }; +class D4 { D4(int i); }; // NOLINTNEXTLINE without-brackets-skip-all, another-check -class C5 { C5(int i); }; - +class D5 { D5(int i); }; // NOLINTNEXTLINE -class D { D(int i); }; +class E { E(int i); }; // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit // NOLINTNEXTLINE // -class E { E(int i); }; +class F { F(int i); }; // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit #define MACRO(X) class X { X(int i); }; -MACRO(F) +MACRO(G) // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit // NOLINTNEXTLINE -MACRO(G) +MACRO(H) -#define MACRO_NOARG class H { H(int i); }; +#define MACRO_NOARG class I { I(int i); }; // NOLINTNEXTLINE MACRO_NOARG -// CHECK-MESSAGES: Suppressed 8 warnings (8 NOLINT) +// CHECK-MESSAGES: Suppressed 9 warnings (9 NOLINT) // RUN: %check_clang_tidy %s google-explicit-constructor %t --