Author: Salman Javed Date: 2022-01-27T00:52:44+13:00 New Revision: 8e29d19b8d29db1ad60af3a8921aad7bbfc24435
URL: https://github.com/llvm/llvm-project/commit/8e29d19b8d29db1ad60af3a8921aad7bbfc24435 DIFF: https://github.com/llvm/llvm-project/commit/8e29d19b8d29db1ad60af3a8921aad7bbfc24435.diff LOG: Revert "[clang-tidy] Cache the locations of NOLINTBEGIN/END blocks" Build warning here: https://lab.llvm.org/buildbot/#/builders/57/builds/14322 Added: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-global-end-specific.cpp clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-global.cpp Modified: clang-tools-extra/clang-tidy/CMakeLists.txt clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h clang-tools-extra/clangd/ParsedAST.cpp clang-tools-extra/test/clang-tidy/infrastructure/nolint.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 Removed: clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp clang-tools-extra/clang-tidy/NoLintDirectiveHandler.h clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/1st-translation-unit.cpp clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/2nd-translation-unit.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-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-multiple-TUs.cpp ################################################################################ diff --git a/clang-tools-extra/clang-tidy/CMakeLists.txt b/clang-tools-extra/clang-tidy/CMakeLists.txt index 8a953eeea2759..075e9f9909d65 100644 --- a/clang-tools-extra/clang-tidy/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/CMakeLists.txt @@ -17,7 +17,6 @@ add_clang_library(clangTidy ClangTidyProfiling.cpp ExpandModularHeadersPPCallbacks.cpp GlobList.cpp - NoLintDirectiveHandler.cpp DEPENDS ClangSACheckers diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp index 04721a2d3a020..66f60ec60b605 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -18,7 +18,6 @@ #include "ClangTidyDiagnosticConsumer.h" #include "ClangTidyOptions.h" #include "GlobList.h" -#include "NoLintDirectiveHandler.h" #include "clang/AST/ASTContext.h" #include "clang/AST/ASTDiagnostic.h" #include "clang/AST/Attr.h" @@ -189,7 +188,7 @@ DiagnosticBuilder ClangTidyContext::diag( return DiagEngine->Report(ID); } -DiagnosticBuilder ClangTidyContext::diag(const tooling::Diagnostic &Error) { +DiagnosticBuilder ClangTidyContext::diag(const ClangTidyError &Error) { SourceManager &SM = DiagEngine->getSourceManager(); llvm::ErrorOr<const FileEntry *> File = SM.getFileManager().getFile(Error.Message.FilePath); @@ -207,15 +206,6 @@ DiagnosticBuilder ClangTidyContext::configurationDiag( return diag("clang-tidy-config", Message, Level); } -bool ClangTidyContext::shouldSuppressDiagnostic( - DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info, - SmallVectorImpl<tooling::Diagnostic> &NoLintErrors, bool AllowIO, - bool EnableNoLintBlocks) { - std::string CheckName = getCheckName(Info.getID()); - return NoLintHandler.shouldSuppress(DiagLevel, Info, CheckName, NoLintErrors, - AllowIO, EnableNoLintBlocks); -} - void ClangTidyContext::setSourceManager(SourceManager *SourceMgr) { DiagEngine->setSourceManager(SourceMgr); } @@ -317,9 +307,218 @@ void ClangTidyDiagnosticConsumer::finalizeLastError() { 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; +} + +static llvm::Optional<StringRef> getBuffer(const SourceManager &SM, FileID File, + bool AllowIO) { + return AllowIO ? SM.getBufferDataOrNone(File) + : SM.getBufferDataIfLoaded(File); +} + +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 'NOLINT" + "END' comment") + : ("unmatched 'NOLINTEND' comment without a previous 'NOLINT" + "BEGIN' comment"); + Error.Message = tooling::DiagnosticMessage(Message, SM, 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(); +} + +static bool +lineIsMarkedWithNOLINT(const ClangTidyContext &Context, + SmallVectorImpl<ClangTidyError> &SuppressionErrors, + bool AllowIO, const SourceManager &SM, + SourceLocation Loc, StringRef CheckName, + bool EnableNolintBlocks) { + // Get source code for this location. + FileID File; + unsigned Offset; + std::tie(File, Offset) = SM.getDecomposedSpellingLoc(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; + + // 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; + + // Check if this line is within a NOLINT(BEGIN...END) block. + return EnableNolintBlocks && + lineIsWithinNolintBegin(Context, SuppressionErrors, SM, Loc, CheckName, + TextBeforeDiag, TextAfterDiag); +} + +static bool lineIsMarkedWithNOLINTinMacro( + const Diagnostic &Info, const ClangTidyContext &Context, + SmallVectorImpl<ClangTidyError> &SuppressionErrors, bool AllowIO, + bool EnableNolintBlocks) { + const SourceManager &SM = Info.getSourceManager(); + SourceLocation Loc = Info.getLocation(); + std::string CheckName = Context.getCheckName(Info.getID()); + while (true) { + if (lineIsMarkedWithNOLINT(Context, SuppressionErrors, AllowIO, SM, Loc, + CheckName, EnableNolintBlocks)) + return true; + if (!Loc.isMacroID()) + return false; + Loc = SM.getImmediateExpansionRange(Loc).getBegin(); + } + return false; +} + namespace clang { namespace tidy { +bool shouldSuppressDiagnostic( + DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info, + ClangTidyContext &Context, + SmallVectorImpl<ClangTidyError> &SuppressionErrors, bool AllowIO, + bool EnableNolintBlocks) { + return Info.getLocation().isValid() && + DiagLevel != DiagnosticsEngine::Error && + DiagLevel != DiagnosticsEngine::Fatal && + lineIsMarkedWithNOLINTinMacro(Info, Context, SuppressionErrors, + AllowIO, EnableNolintBlocks); +} + const llvm::StringMap<tooling::Replacements> * getFixIt(const tooling::Diagnostic &Diagnostic, bool GetFixFromNotes) { if (!Diagnostic.Message.Fix.empty()) @@ -346,15 +545,12 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic( if (LastErrorWasIgnored && DiagLevel == DiagnosticsEngine::Note) return; - SmallVector<tooling::Diagnostic, 1> SuppressionErrors; - if (Context.shouldSuppressDiagnostic(DiagLevel, Info, SuppressionErrors, - EnableNolintBlocks)) { + SmallVector<ClangTidyError, 1> SuppressionErrors; + if (shouldSuppressDiagnostic(DiagLevel, Info, Context, SuppressionErrors, + EnableNolintBlocks)) { ++Context.Stats.ErrorsIgnoredNOLINT; // Ignored a warning, should ignore related notes as well LastErrorWasIgnored = true; - Context.DiagEngine->Clear(); - for (const auto &Error : SuppressionErrors) - Context.diag(Error); return; } diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h index e41003262cccf..69f1ceddd9bd2 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h @@ -11,7 +11,6 @@ #include "ClangTidyOptions.h" #include "ClangTidyProfiling.h" -#include "NoLintDirectiveHandler.h" #include "clang/Basic/Diagnostic.h" #include "clang/Tooling/Core/Diagnostic.h" #include "llvm/ADT/DenseMap.h" @@ -96,33 +95,13 @@ class ClangTidyContext { DiagnosticBuilder diag(StringRef CheckName, StringRef Message, DiagnosticIDs::Level Level = DiagnosticIDs::Warning); - DiagnosticBuilder diag(const tooling::Diagnostic &Error); + DiagnosticBuilder diag(const ClangTidyError &Error); /// Report any errors to do with reading the configuration using this method. DiagnosticBuilder configurationDiag(StringRef Message, DiagnosticIDs::Level Level = DiagnosticIDs::Warning); - /// Check whether a given diagnostic should be suppressed due to the presence - /// of a "NOLINT" suppression comment. - /// 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 as it requires maintaining state in - /// between calls to this function. - /// If any NOLINT is malformed, e.g. a BEGIN without a subsequent END, an - /// error about it will be returned in output \param NoLintErrors. - /// If \param 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. - /// \param EnableNoLintBlocks controls whether to honor NOLINTBEGIN/NOLINTEND - /// blocks; if false, only considers line-level disabling. - bool - shouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel, - const Diagnostic &Info, - SmallVectorImpl<tooling::Diagnostic> &NoLintErrors, - bool AllowIO = true, bool EnableNoLintBlocks = true); - /// Sets the \c SourceManager of the used \c DiagnosticsEngine. /// /// This is called from the \c ClangTidyCheck base class. @@ -229,10 +208,30 @@ class ClangTidyContext { std::string ProfilePrefix; bool AllowEnablingAnalyzerAlphaCheckers; - - NoLintDirectiveHandler NoLintHandler; }; +/// Check whether a given diagnostic should be suppressed due to the presence +/// of a "NOLINT" suppression comment. +/// 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 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. +/// \param EnableNolintBlocks controls whether to honor NOLINTBEGIN/NOLINTEND +/// blocks; if false, only considers line-level disabling. +/// 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, + SmallVectorImpl<ClangTidyError> &SuppressionErrors, bool AllowIO = true, + bool EnableNolintBlocks = 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/NoLintDirectiveHandler.cpp b/clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp deleted file mode 100644 index 6723b752fc848..0000000000000 --- a/clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp +++ /dev/null @@ -1,415 +0,0 @@ -//===-- clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp -----------===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// -/// -/// \file This file implements the NoLintDirectiveHandler class, which is used -/// to locate NOLINT comments in the file being analyzed, to decide whether a -/// diagnostic should be suppressed. -/// -//===----------------------------------------------------------------------===// - -#include "NoLintDirectiveHandler.h" -#include "GlobList.h" -#include "clang/Basic/LLVM.h" -#include "clang/Basic/SourceLocation.h" -#include "clang/Basic/SourceManager.h" -#include "clang/Tooling/Core/Diagnostic.h" -#include "llvm/ADT/ArrayRef.h" -#include "llvm/ADT/None.h" -#include "llvm/ADT/Optional.h" -#include "llvm/ADT/STLExtras.h" -#include "llvm/ADT/SmallVector.h" -#include "llvm/ADT/StringExtras.h" -#include "llvm/ADT/StringMap.h" -#include "llvm/ADT/StringSwitch.h" -#include <cassert> -#include <cstddef> -#include <iterator> -#include <string> -#include <tuple> -#include <type_traits> -#include <utility> - -namespace clang { -namespace tidy { - -//===----------------------------------------------------------------------===// -// NoLintType -//===----------------------------------------------------------------------===// - -// The type - one of NOLINT[NEXTLINE/BEGIN/END]. -enum class NoLintType { NoLint, NoLintNextLine, NoLintBegin, NoLintEnd }; - -// Convert a string like "NOLINTNEXTLINE" to its enum `Type::NoLintNextLine`. -// Return `None` if the string is unrecognized. -static Optional<NoLintType> strToNoLintType(StringRef Str) { - auto Type = llvm::StringSwitch<Optional<NoLintType>>(Str) - .Case("NOLINT", NoLintType::NoLint) - .Case("NOLINTNEXTLINE", NoLintType::NoLintNextLine) - .Case("NOLINTBEGIN", NoLintType::NoLintBegin) - .Case("NOLINTEND", NoLintType::NoLintEnd) - .Default(None); - return Type; -} - -//===----------------------------------------------------------------------===// -// NoLintToken -//===----------------------------------------------------------------------===// - -// Whitespace within a NOLINT's check list shall be ignored. -// "NOLINT( check1, check2 )" is equivalent to "NOLINT(check1,check2)". -// Return the check list with all extraneous whitespace removed. -static std::string trimWhitespace(StringRef Checks) { - SmallVector<StringRef> Split; - Checks.split(Split, ','); - for (StringRef &Check : Split) - Check = Check.trim(); - return llvm::join(Split, ","); -} - -namespace { - -// Record the presence of a NOLINT comment - its type, location, checks - -// as parsed from the file's character contents. -class NoLintToken { -public: - // \param Checks: - // - If unspecified (i.e. `None`) then ALL checks are suppressed - equivalent - // to NOLINT(*). - // - An empty string means nothing is suppressed - equivalent to NOLINT(). - // - Negative globs ignored (which would effectively disable the suppression). - NoLintToken(NoLintType Type, size_t Pos, const Optional<std::string> &Checks) - : Type(Type), Pos(Pos), ChecksGlob(std::make_unique<CachedGlobList>( - Checks.getValueOr("*"), - /*KeepNegativeGlobs=*/false)) { - if (Checks) - this->Checks = trimWhitespace(*Checks); - } - - // The type - one of NOLINT[NEXTLINE/BEGIN/END]. - NoLintType Type; - - // The location of the first character, "N", in "NOLINT". - size_t Pos; - - // If this NOLINT specifies checks, return the checks. - Optional<std::string> checks() const { return Checks; } - - // Whether this NOLINT applies to the provided check. - bool suppresses(StringRef Check) const { return ChecksGlob->contains(Check); } - -private: - Optional<std::string> Checks; - std::unique_ptr<CachedGlobList> ChecksGlob; -}; - -} // namespace - -// Consume the entire buffer and return all `NoLintToken`s that were found. -static SmallVector<NoLintToken> getNoLints(StringRef Buffer) { - static constexpr llvm::StringLiteral NOLINT = "NOLINT"; - SmallVector<NoLintToken> NoLints; - - size_t Pos = 0; - while (Pos < Buffer.size()) { - // Find NOLINT: - const size_t NoLintPos = Buffer.find(NOLINT, Pos); - if (NoLintPos == StringRef::npos) - break; // Buffer exhausted - - // Read [A-Z] characters immediately after "NOLINT", e.g. the "NEXTLINE" in - // "NOLINTNEXTLINE". - Pos = NoLintPos + NOLINT.size(); - while (Pos < Buffer.size() && llvm::isAlpha(Buffer[Pos])) - ++Pos; - - // Is this a recognized NOLINT type? - const Optional<NoLintType> NoLintType = - strToNoLintType(Buffer.slice(NoLintPos, Pos)); - if (!NoLintType) - continue; - - // Get checks, if specified. - Optional<std::string> Checks; - if (Pos < Buffer.size() && Buffer[Pos] == '(') { - size_t ClosingBracket = Buffer.find_first_of("\n)", ++Pos); - if (ClosingBracket != StringRef::npos && Buffer[ClosingBracket] == ')') { - Checks = Buffer.slice(Pos, ClosingBracket).str(); - Pos = ClosingBracket + 1; - } - } - - NoLints.emplace_back(*NoLintType, NoLintPos, Checks); - } - - return NoLints; -} - -//===----------------------------------------------------------------------===// -// NoLintBlockToken -//===----------------------------------------------------------------------===// - -namespace { - -// Represents a source range within a pair of NOLINT(BEGIN/END) comments. -class NoLintBlockToken { -public: - NoLintBlockToken(NoLintToken Begin, const NoLintToken &End) - : Begin(std::move(Begin)), EndPos(End.Pos) { - assert(this->Begin.Type == NoLintType::NoLintBegin); - assert(End.Type == NoLintType::NoLintEnd); - assert(this->Begin.Pos < End.Pos); - assert(this->Begin.checks() == End.checks()); - } - - // Whether the provided diagnostic is within and is suppressible by this block - // of NOLINT(BEGIN/END) comments. - bool suppresses(size_t DiagPos, StringRef DiagName) const { - return (Begin.Pos < DiagPos) && (DiagPos < EndPos) && - Begin.suppresses(DiagName); - } - -private: - NoLintToken Begin; - size_t EndPos; -}; - -} // namespace - -// Match NOLINTBEGINs with their corresponding NOLINTENDs and move them into -// `NoLintBlockToken`s. If any BEGINs or ENDs are left over, they are moved to -// `UnmatchedTokens`. -static SmallVector<NoLintBlockToken> -formNoLintBlocks(SmallVector<NoLintToken> NoLints, - SmallVectorImpl<NoLintToken> &UnmatchedTokens) { - SmallVector<NoLintBlockToken> CompletedBlocks; - SmallVector<NoLintToken> Stack; - - // Nested blocks must be fully contained within their parent block. What this - // means is that when you have a series of nested BEGIN tokens, the END tokens - // shall appear in the reverse order, starting with the closing of the - // inner-most block first, then the next level up, and so on. This is - // essentially a last-in-first-out/stack system. - for (NoLintToken &NoLint : NoLints) { - if (NoLint.Type == NoLintType::NoLintBegin) - // A new block is being started. Add it to the stack. - Stack.emplace_back(std::move(NoLint)); - else if (NoLint.Type == NoLintType::NoLintEnd) { - if (!Stack.empty() && Stack.back().checks() == NoLint.checks()) - // The previous block is being closed. Pop one element off the stack. - CompletedBlocks.emplace_back(std::move(Stack.pop_back_val()), NoLint); - else - // Trying to close the wrong block. - UnmatchedTokens.emplace_back(std::move(NoLint)); - } - } - - llvm::move(Stack, std::back_inserter(UnmatchedTokens)); - return CompletedBlocks; -} - -//===----------------------------------------------------------------------===// -// NoLintDirectiveHandler::Impl -//===----------------------------------------------------------------------===// - -class NoLintDirectiveHandler::Impl { -public: - bool shouldSuppress(DiagnosticsEngine::Level DiagLevel, - const Diagnostic &Diag, StringRef DiagName, - SmallVectorImpl<tooling::Diagnostic> &NoLintErrors, - bool AllowIO, bool EnableNoLintBlocks); - -private: - bool diagHasNoLintInMacro(const Diagnostic &Diag, StringRef DiagName, - SmallVectorImpl<tooling::Diagnostic> &NoLintErrors, - bool AllowIO, bool EnableNoLintBlocks); - - bool diagHasNoLint(StringRef DiagName, SourceLocation DiagLoc, - const SourceManager &SrcMgr, - SmallVectorImpl<tooling::Diagnostic> &NoLintErrors, - bool AllowIO, bool EnableNoLintBlocks); - - void generateCache(const SourceManager &SrcMgr, StringRef FileName, - FileID File, StringRef Buffer, - SmallVectorImpl<tooling::Diagnostic> &NoLintErrors); - - llvm::StringMap<SmallVector<NoLintBlockToken>> Cache; -}; - -bool NoLintDirectiveHandler::Impl::shouldSuppress( - DiagnosticsEngine::Level DiagLevel, const Diagnostic &Diag, - StringRef DiagName, SmallVectorImpl<tooling::Diagnostic> &NoLintErrors, - bool AllowIO, bool EnableNoLintBlocks) { - if (DiagLevel >= DiagnosticsEngine::Error) - return false; - return diagHasNoLintInMacro(Diag, DiagName, NoLintErrors, AllowIO, - EnableNoLintBlocks); -} - -// Look at the macro's spelling location for a NOLINT. If none is found, keep -// looking up the call stack. -bool NoLintDirectiveHandler::Impl::diagHasNoLintInMacro( - const Diagnostic &Diag, StringRef DiagName, - SmallVectorImpl<tooling::Diagnostic> &NoLintErrors, bool AllowIO, - bool EnableNoLintBlocks) { - SourceLocation DiagLoc = Diag.getLocation(); - if (DiagLoc.isInvalid()) - return false; - const SourceManager &SrcMgr = Diag.getSourceManager(); - while (true) { - if (diagHasNoLint(DiagName, DiagLoc, SrcMgr, NoLintErrors, AllowIO, - EnableNoLintBlocks)) - return true; - if (!DiagLoc.isMacroID()) - return false; - DiagLoc = SrcMgr.getImmediateMacroCallerLoc(DiagLoc); - } - return false; -} - -// 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 Buffer, - size_t From) { - size_t StartPos = Buffer.find_last_of('\n', From) + 1; - size_t EndPos = std::min(Buffer.find('\n', From), Buffer.size()); - return std::make_pair(StartPos, EndPos); -} - -// Whether the line has a NOLINT of type = `Type` that can suppress the -// diagnostic `DiagName`. -static bool lineHasNoLint(StringRef Buffer, - std::pair<size_t, size_t> LineStartAndEnd, - NoLintType Type, StringRef DiagName) { - // Get all NOLINTs on the line. - Buffer = Buffer.slice(LineStartAndEnd.first, LineStartAndEnd.second); - SmallVector<NoLintToken> NoLints = getNoLints(Buffer); - - // Do any of these NOLINTs match the desired type and diag name? - return llvm::any_of(NoLints, [&](const NoLintToken &NoLint) { - return NoLint.Type == Type && NoLint.suppresses(DiagName); - }); -} - -// Whether the provided diagnostic is located within and is suppressible by a -// block of NOLINT(BEGIN/END) comments. -static bool withinNoLintBlock(ArrayRef<NoLintBlockToken> NoLintBlocks, - size_t DiagPos, StringRef DiagName) { - return llvm::any_of(NoLintBlocks, [&](const NoLintBlockToken &NoLintBlock) { - return NoLintBlock.suppresses(DiagPos, DiagName); - }); -} - -// Get the file contents as a string. -static Optional<StringRef> getBuffer(const SourceManager &SrcMgr, FileID File, - bool AllowIO) { - return AllowIO ? SrcMgr.getBufferDataOrNone(File) - : SrcMgr.getBufferDataIfLoaded(File); -} - -// We will check for NOLINTs and NOLINTNEXTLINEs first. Checking for these is -// not so expensive (just need to parse the current and previous lines). Only if -// that fails do we look for NOLINT(BEGIN/END) blocks (which requires reading -// the entire file). -bool NoLintDirectiveHandler::Impl::diagHasNoLint( - StringRef DiagName, SourceLocation DiagLoc, const SourceManager &SrcMgr, - SmallVectorImpl<tooling::Diagnostic> &NoLintErrors, bool AllowIO, - bool EnableNoLintBlocks) { - // Translate the diagnostic's SourceLocation to a raw file + offset pair. - FileID File; - unsigned int Pos = 0; - std::tie(File, Pos) = SrcMgr.getDecomposedSpellingLoc(DiagLoc); - - // We will only see NOLINTs in user-authored sources. No point reading the - // file if it is a <built-in>. - Optional<StringRef> FileName = SrcMgr.getNonBuiltinFilenameForID(File); - if (!FileName) - return false; - - // Get file contents. - Optional<StringRef> Buffer = getBuffer(SrcMgr, File, AllowIO); - if (!Buffer) - return false; - - // Check if there's a NOLINT on this line. - auto ThisLine = getLineStartAndEnd(*Buffer, Pos); - if (lineHasNoLint(*Buffer, ThisLine, NoLintType::NoLint, DiagName)) - return true; - - // Check if there's a NOLINTNEXTLINE on the previous line. - if (ThisLine.first > 0) { - auto PrevLine = getLineStartAndEnd(*Buffer, ThisLine.first - 1); - if (lineHasNoLint(*Buffer, PrevLine, NoLintType::NoLintNextLine, DiagName)) - return true; - } - - // Check if this line is within a NOLINT(BEGIN/END) block. - if (!EnableNoLintBlocks) - return false; - - // Do we have cached NOLINT block locations for this file? - if (Cache.count(*FileName) == 0) - // Warning: heavy operation - need to read entire file. - generateCache(SrcMgr, *FileName, File, *Buffer, NoLintErrors); - - return withinNoLintBlock(Cache[*FileName], Pos, DiagName); -} - -// Construct a [clang-tidy-nolint] diagnostic to do with the unmatched -// NOLINT(BEGIN/END) pair. -static tooling::Diagnostic makeNoLintError(const SourceManager &SrcMgr, - FileID File, - const NoLintToken &NoLint) { - tooling::Diagnostic Error; - Error.DiagLevel = tooling::Diagnostic::Error; - Error.DiagnosticName = "clang-tidy-nolint"; - StringRef Message = - (NoLint.Type == NoLintType::NoLintBegin) - ? ("unmatched 'NOLINTBEGIN' comment without a subsequent 'NOLINT" - "END' comment") - : ("unmatched 'NOLINTEND' comment without a previous 'NOLINT" - "BEGIN' comment"); - SourceLocation Loc = SrcMgr.getComposedLoc(File, NoLint.Pos); - Error.Message = tooling::DiagnosticMessage(Message, SrcMgr, Loc); - return Error; -} - -// Find all NOLINT(BEGIN/END) blocks in a file and store in the cache. -void NoLintDirectiveHandler::Impl::generateCache( - const SourceManager &SrcMgr, StringRef FileName, FileID File, - StringRef Buffer, SmallVectorImpl<tooling::Diagnostic> &NoLintErrors) { - // Read entire file to get all NOLINTs. - SmallVector<NoLintToken> NoLints = getNoLints(Buffer); - - // Match each BEGIN with its corresponding END. - SmallVector<NoLintToken> UnmatchedTokens; - Cache[FileName] = formNoLintBlocks(std::move(NoLints), UnmatchedTokens); - - // Raise error for any BEGIN/END left over. - for (const NoLintToken &NoLint : UnmatchedTokens) - NoLintErrors.emplace_back(makeNoLintError(SrcMgr, File, NoLint)); -} - -//===----------------------------------------------------------------------===// -// NoLintDirectiveHandler -//===----------------------------------------------------------------------===// - -NoLintDirectiveHandler::NoLintDirectiveHandler() - : PImpl(std::make_unique<Impl>()) {} - -NoLintDirectiveHandler::~NoLintDirectiveHandler() = default; - -bool NoLintDirectiveHandler::shouldSuppress( - DiagnosticsEngine::Level DiagLevel, const Diagnostic &Diag, - StringRef DiagName, SmallVectorImpl<tooling::Diagnostic> &NoLintErrors, - bool AllowIO, bool EnableNoLintBlocks) { - return PImpl->shouldSuppress(DiagLevel, Diag, DiagName, NoLintErrors, AllowIO, - EnableNoLintBlocks); -} - -} // namespace tidy -} // namespace clang diff --git a/clang-tools-extra/clang-tidy/NoLintDirectiveHandler.h b/clang-tools-extra/clang-tidy/NoLintDirectiveHandler.h deleted file mode 100644 index db9fd593283c7..0000000000000 --- a/clang-tools-extra/clang-tidy/NoLintDirectiveHandler.h +++ /dev/null @@ -1,51 +0,0 @@ -//===-- clang-tools-extra/clang-tidy/NoLintDirectiveHandler.h ----*- C++ *-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_NOLINTDIRECTIVEHANDLER_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_NOLINTDIRECTIVEHANDLER_H - -#include "clang/Basic/Diagnostic.h" -#include "llvm/ADT/StringRef.h" -#include <memory> - -namespace clang { -namespace tooling { -struct Diagnostic; -} // namespace tooling -} // namespace clang - -namespace llvm { -template <typename T> class SmallVectorImpl; -} // namespace llvm - -namespace clang { -namespace tidy { - -/// This class is used to locate NOLINT comments in the file being analyzed, to -/// decide whether a diagnostic should be suppressed. -/// This class keeps a cache of every NOLINT comment found so that files do not -/// have to be repeatedly parsed each time a new diagnostic is raised. -class NoLintDirectiveHandler { -public: - NoLintDirectiveHandler(); - ~NoLintDirectiveHandler(); - - bool shouldSuppress(DiagnosticsEngine::Level DiagLevel, - const Diagnostic &Diag, llvm::StringRef DiagName, - llvm::SmallVectorImpl<tooling::Diagnostic> &NoLintErrors, - bool AllowIO, bool EnableNoLintBlocks); - -private: - class Impl; - std::unique_ptr<Impl> PImpl; -}; - -} // namespace tidy -} // namespace clang - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_NOLINTDIRECTIVEHANDLER_H diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp index 86c4b2151243c..9c64c645130bf 100644 --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -10,7 +10,6 @@ #include "../clang-tidy/ClangTidyCheck.h" #include "../clang-tidy/ClangTidyDiagnosticConsumer.h" #include "../clang-tidy/ClangTidyModuleRegistry.h" -#include "../clang-tidy/NoLintDirectiveHandler.h" #include "AST.h" #include "Compiler.h" #include "Config.h" @@ -469,11 +468,12 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, bool IsInsideMainFile = Info.hasSourceManager() && isInsideMainFile(Info.getLocation(), Info.getSourceManager()); - SmallVector<tooling::Diagnostic, 1> TidySuppressedErrors; - if (IsInsideMainFile && CTContext->shouldSuppressDiagnostic( - DiagLevel, Info, TidySuppressedErrors, - /*AllowIO=*/false, - /*EnableNolintBlocks=*/false)) { + SmallVector<tidy::ClangTidyError, 1> TidySuppressedErrors; + if (IsInsideMainFile && + tidy::shouldSuppressDiagnostic(DiagLevel, Info, *CTContext, + TidySuppressedErrors, + /*AllowIO=*/false, + /*EnableNolintBlocks=*/false)) { // FIXME: should we expose the suppression error (invalid use of // NOLINT comments)? return DiagnosticsEngine::Ignored; diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/1st-translation-unit.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/1st-translation-unit.cpp deleted file mode 100644 index d90fc069c9789..0000000000000 --- a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/1st-translation-unit.cpp +++ /dev/null @@ -1,5 +0,0 @@ -// NOLINTBEGIN -class A { A(int i); }; -// NOLINTEND - -class B { B(int i); }; diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/2nd-translation-unit.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/2nd-translation-unit.cpp deleted file mode 100644 index 0497037d31b82..0000000000000 --- a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/nolintbeginend/2nd-translation-unit.cpp +++ /dev/null @@ -1,6 +0,0 @@ - -class A { A(int i); }; - -// NOLINTBEGIN -class B { B(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 index 9ef194232b6b7..4b80bbe4eb3ca 100644 --- a/clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp +++ b/clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp @@ -28,56 +28,11 @@ class C4 { C4(int i); }; // NOLINT(google-explicit-constructor) class C5 { C5(int i); }; // NOLINT(some-check, google-explicit-constructor) -class C6 { C6(int i); }; // NOLINT without-brackets-skip-all +class C6 { C6(int i); }; // NOLINT without-brackets-skip-all, another-check -// Other NOLINT* types (e.g. NEXTLINE) should not be misconstrued as a NOLINT: -class C7 { C7(int i); }; // NOLINTNEXTLINE +class C7 { C7(int i); }; // NOLINTNEXTLINE doesn't get misconstrued as a NOLINT // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit -// NOLINT must be UPPERCASE: -// NOLINTnextline -class C8 { C8(int i); }; // nolint -// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit - -// Unrecognized marker: -// NOLINTNEXTLINEXYZ -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) - -// NOLINTNEXTLINE(google-explicit-constructor) NOLINT(some-other-check) -class C15 { C15(int i); }; - -// Any text after a NOLINT expression is treated as a comment: -class C16 { C16(int i); }; // NOLINT: suppress check because <reason> -class C17 { C17(int i); }; // NOLINT(google-explicit-constructor): suppress check because <reason> - -// NOLINT must appear in its entirety on one line: -class C18 { C18(int i); }; /* NOL -INT */ -// CHECK-MESSAGES: :[[@LINE-2]]:13: warning: single-argument constructors must be marked explicit - -/* NO -LINT */ class C19 { C19(int i); }; -// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: single-argument constructors must be marked explicit - -// Spaces between items in the comma-separated check list are ignroed: -class C20 { C20(int i); }; // NOLINT( google-explicit-constructor ) -class C21 { C21(int i); }; // NOLINT( google-explicit-constructor , some-other-check ) -class C22 { C22(int i); }; // NOLINT(google-explicit- constructor) -// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: single-argument constructors must be marked explicit - -// If there is a space between "NOLINT" and the bracket, it is treated as a regular NOLINT: -class C23 { C23(int i); }; // NOLINT (some-other-check) - void f() { int i; // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unused variable 'i' [clang-diagnostic-unused-variable] @@ -116,4 +71,4 @@ int array2[10]; // NOLINT(cppcoreguidelines-avoid-c-arrays) int array3[10]; // NOLINT(cppcoreguidelines-avoid-c-arrays,modernize-avoid-c-arrays) int array4[10]; // NOLINT(*-avoid-c-arrays) -// CHECK-MESSAGES: Suppressed 34 warnings (34 NOLINT) +// CHECK-MESSAGES: Suppressed 23 warnings (23 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 deleted file mode 100644 index ee5b1cca755ab..0000000000000 --- a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-LIFO.cpp +++ /dev/null @@ -1,19 +0,0 @@ -// 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 deleted file mode 100644 index 90b9fa9883024..0000000000000 --- a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-all-end-glob.cpp +++ /dev/null @@ -1,16 +0,0 @@ -// 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 deleted file mode 100644 index 6ffa914e4ef0b..0000000000000 --- a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-all-end-specific.cpp +++ /dev/null @@ -1,16 +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-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 deleted file mode 100644 index 3697d5c11e2e2..0000000000000 --- a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-glob-end-all.cpp +++ /dev/null @@ -1,16 +0,0 @@ -// 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 deleted file mode 100644 index 5bdb117f20242..0000000000000 --- a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-glob-end-specific.cpp +++ /dev/null @@ -1,16 +0,0 @@ -// 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 new file mode 100644 index 0000000000000..01e69ddf6352a --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-global-end-specific.cpp @@ -0,0 +1,25 @@ +// 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 deleted file mode 100644 index 150913ce0ec69..0000000000000 --- a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-multiple-end-single.cpp +++ /dev/null @@ -1,22 +0,0 @@ -// 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 deleted file mode 100644 index f9a915c890cbb..0000000000000 --- a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-single-end-multiple.cpp +++ /dev/null @@ -1,22 +0,0 @@ -// 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 deleted file mode 100644 index decfe2dd5a4c1..0000000000000 --- a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-all.cpp +++ /dev/null @@ -1,16 +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-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 deleted file mode 100644 index a9f904ccce138..0000000000000 --- a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-glob.cpp +++ /dev/null @@ -1,16 +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-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 new file mode 100644 index 0000000000000..c6fcfddd09ba8 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-global.cpp @@ -0,0 +1,25 @@ +// 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 index 8d7786fb8c712..957b0b341a3cc 100644 --- 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,3 +16,24 @@ auto Num = (unsigned int)(-1); // 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 index 4b8947e369f92..7ed5fb820e509 100644 --- 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,3 +11,4 @@ class A { A(int i); }; // 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-multiple-TUs.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-multiple-TUs.cpp deleted file mode 100644 index 773e023cb22c9..0000000000000 --- a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-multiple-TUs.cpp +++ /dev/null @@ -1,6 +0,0 @@ -// RUN: clang-tidy %S/Inputs/nolintbeginend/1st-translation-unit.cpp %S/Inputs/nolintbeginend/2nd-translation-unit.cpp --checks='-*,google-explicit-constructor' 2>&1 | FileCheck %s - -// CHECK-NOT: 1st-translation-unit.cpp:2:11: warning: single-argument constructors must be marked explicit -// CHECK: 1st-translation-unit.cpp:5:11: warning: single-argument constructors must be marked explicit -// CHECK: 2nd-translation-unit.cpp:2:11: warning: single-argument constructors must be marked explicit -// CHECK-NOT: 2nd-translation-unit.cpp:5: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 index 57e1ff331c8ba..0f2f9994c1fa0 100644 --- 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,6 +11,3 @@ class A { A(int i); }; // 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 index ae97bc5b51441..8379128530180 100644 --- a/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp +++ b/clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp @@ -59,23 +59,29 @@ class C6 { C6(int i); }; // NOLINTEND(google-explicit-constructor) // NOLINTBEGIN(google-explicit-constructor) -// NOLINTBEGIN +// 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); }; // NOLINTEND // NOLINTEND(google-explicit-constructor) // NOLINTBEGIN // NOLINTBEGIN(google-explicit-constructor) -class C8 { C8(int i); }; +class C9 { C9(int i); }; // NOLINTEND(google-explicit-constructor) // NOLINTEND // NOLINTBEGIN(not-closed-bracket-is-treated-as-skip-all -class C9 { C9(int i); }; +class C10 { C10(int i); }; // NOLINTEND(not-closed-bracket-is-treated-as-skip-all // NOLINTBEGIN without-brackets-skip-all, another-check -class C10 { C10(int i); }; +class C11 { C11(int i); }; // NOLINTEND without-brackets-skip-all, another-check #define MACRO(X) class X { X(int i); }; @@ -108,28 +114,28 @@ MACRO_WRAPPED_WITH_NO_LINT MACRO_NO_LINT_INSIDE_MACRO // NOLINTBEGIN(google*) -class C11 { C11(int i); }; +class C12 { C12(int i); }; // NOLINTEND(google*) // NOLINTBEGIN(*explicit-constructor) -class C12 { C12(int i); }; +class C15 { C15(int i); }; // NOLINTEND(*explicit-constructor) // NOLINTBEGIN(*explicit*) -class C13 { C13(int i); }; +class C16 { C16(int i); }; // NOLINTEND(*explicit*) // NOLINTBEGIN(-explicit-constructor) -class C14 { C14(int x); }; +class C17 { C17(int x); }; // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: single-argument constructors must be marked explicit // NOLINTEND(-explicit-constructor) // NOLINTBEGIN(google*,-google*) -class C15 { C15(int x); }; +class C18 { C18(int x); }; // NOLINTEND(google*,-google*) // NOLINTBEGIN(*,-google*) -class C16 { C16(int x); }; +class C19 { C19(int x); }; // NOLINTEND(*,-google*) int array1[10]; @@ -148,4 +154,4 @@ int array3[10]; int array4[10]; // NOLINTEND(*-avoid-c-arrays) -// CHECK-MESSAGES: Suppressed 26 warnings (26 NOLINT). +// CHECK-MESSAGES: Suppressed 27 warnings (27 NOLINT). _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
