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 @@ -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 & + getNoLintBlocks(FileID File, SmallVectorImpl &Errors, + bool AllowIO = true) const; + private: // Writes to Stats. friend class ClangTidyDiagnosticConsumer; @@ -208,6 +261,8 @@ std::string ProfilePrefix; bool AllowEnablingAnalyzerAlphaCheckers; + + mutable llvm::DenseMap> NoLintCache; }; /// Check whether a given diagnostic should be suppressed due to the presence 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 @@ -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(Error.Message.FileOffset)); return diag(Error.DiagnosticName, Loc, Error.Message.Message, static_cast(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 findNoLints(StringRef Buffer, + SourceLocation BufferLoc) { + static constexpr StringRef NoLintStr = "NOLINT"; + SmallVector 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(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 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 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 tallyNolintBegins( - const ClangTidyContext &Context, const SourceManager &SM, - StringRef CheckName, SmallVector Lines, SourceLocation LinesLoc, - SmallVector> &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 &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> NolintBegins; - - // Check if there's an open NOLINT(BEGIN...END) block on the previous lines. - SmallVector 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 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 +collectNoLintBlocks(const ClangTidyContext &Context, const SourceManager &SM, + StringRef Buffer, SourceLocation BufferLoc, + SmallVectorImpl &Errors) { + const SmallVector NoLints = findNoLints(Buffer, BufferLoc); + SmallVector NoLintBegins; + SmallVector 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 &ClangTidyContext::getNoLintBlocks( + FileID File, SmallVectorImpl &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 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 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 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 &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 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 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; } 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 @@ -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 +class C16 { C16(int i); }; // NOLINT(google-explicit-constructor): suppress check because + 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) diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-LIFO.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-LIFO.cpp new file mode 100644 --- /dev/null +++ b/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] diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-all-end-glob.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-all-end-glob.cpp new file mode 100644 --- /dev/null +++ b/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] diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-all-end-specific.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-all-end-specific.cpp new file mode 100644 --- /dev/null +++ b/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] diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-glob-end-all.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-glob-end-all.cpp new file mode 100644 --- /dev/null +++ b/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] diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-glob-end-specific.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-glob-end-specific.cpp new file mode 100644 --- /dev/null +++ b/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] 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 deleted file mode 100644 --- a/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] diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-multiple-end-single.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-multiple-end-single.cpp new file mode 100644 --- /dev/null +++ b/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] diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-single-end-multiple.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-single-end-multiple.cpp new file mode 100644 --- /dev/null +++ b/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] diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-all.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-all.cpp new file mode 100644 --- /dev/null +++ b/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] diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-glob.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-glob.cpp new file mode 100644 --- /dev/null +++ b/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] 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 deleted file mode 100644 --- a/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] 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 --- 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 @@ -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] 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 --- a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-mismatched-delims.cpp +++ b/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 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 --- 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 @@ -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] diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp --- a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp +++ b/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).