#include "clang/AST/Attr.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Frontend/DiagnosticRenderer.h"
#include "clang/Lex/Lexer.h"
return DiagEngine->Report(ID);
}
+DiagnosticBuilder ClangTidyContext::diag(const ClangTidyError &Error) {
+ SourceManager &SM = DiagEngine->getSourceManager();
+ llvm::ErrorOr<const FileEntry *> File =
+ SM.getFileManager().getFile(Error.Message.FilePath);
+ FileID ID = SM.getOrCreateFileID(*File, SrcMgr::C_User);
+ SourceLocation FileStartLoc = SM.getLocForStartOfFile(ID);
+ SourceLocation Loc = FileStartLoc.getLocWithOffset(Error.Message.FileOffset);
+ return diag(Error.DiagnosticName, Loc, Error.Message.Message,
+ static_cast<DiagnosticIDs::Level>(Error.DiagLevel));
+}
+
DiagnosticBuilder ClangTidyContext::configurationDiag(
StringRef Message,
DiagnosticIDs::Level Level /* = DiagnosticIDs::Warning*/) {
LastErrorPassesLineFilter = false;
}
-static bool IsNOLINTFound(StringRef NolintDirectiveText, StringRef Line,
- unsigned DiagID, const ClangTidyContext &Context) {
- const size_t NolintIndex = Line.find(NolintDirectiveText);
+static bool isNOLINTFound(StringRef NolintDirectiveText, StringRef CheckName,
+ StringRef Line, size_t *FoundNolintIndex = nullptr,
+ bool *SuppressionIsSpecific = nullptr) {
+ if (FoundNolintIndex)
+ *FoundNolintIndex = StringRef::npos;
+ if (SuppressionIsSpecific)
+ *SuppressionIsSpecific = false;
+
+ size_t NolintIndex = Line.find(NolintDirectiveText);
if (NolintIndex == StringRef::npos)
return false;
size_t BracketIndex = NolintIndex + NolintDirectiveText.size();
- // Check if the specific checks are specified in brackets.
+ if (BracketIndex < Line.size() && isalnum(Line[BracketIndex])) {
+ // Reject this search result, otherwise it will cause false positives when
+ // NOLINT is found as a substring of NOLINT(NEXTLINE/BEGIN/END).
+ return false;
+ }
+
+ // Check if specific checks are specified in brackets.
if (BracketIndex < Line.size() && Line[BracketIndex] == '(') {
++BracketIndex;
const size_t BracketEndIndex = Line.find(')', BracketIndex);
Line.substr(BracketIndex, BracketEndIndex - BracketIndex);
// Allow disabling all the checks with "*".
if (ChecksStr != "*") {
- std::string CheckName = Context.getCheckName(DiagID);
// Allow specifying a few check names, delimited with comma.
SmallVector<StringRef, 1> Checks;
ChecksStr.split(Checks, ',', -1, false);
llvm::transform(Checks, Checks.begin(),
[](StringRef S) { return S.trim(); });
- return llvm::find(Checks, CheckName) != Checks.end();
+ if (llvm::find(Checks, CheckName) == Checks.end())
+ return false;
+ if (SuppressionIsSpecific)
+ *SuppressionIsSpecific = true;
}
}
}
+
+ if (FoundNolintIndex)
+ *FoundNolintIndex = NolintIndex;
+
return true;
}
: SM.getBufferDataIfLoaded(File);
}
-static bool LineIsMarkedWithNOLINT(const SourceManager &SM, SourceLocation Loc,
- unsigned DiagID,
- const ClangTidyContext &Context,
- bool AllowIO) {
+static ClangTidyError createNolintError(const ClangTidyContext &Context,
+ const SourceManager &SM,
+ SourceLocation Loc,
+ bool IsNolintBegin) {
+ ClangTidyError Error("clang-tidy-nolint", ClangTidyError::Error,
+ Context.getCurrentBuildDirectory(), false);
+ StringRef Message =
+ IsNolintBegin
+ ? "unmatched 'NOLINTBEGIN' comment without a subsequent 'NOLINTEND' "
+ "comment"
+ : "unmatched 'NOLINTEND' comment without a previous 'NOLINTBEGIN' "
+ "comment";
+ Error.Message = tooling::DiagnosticMessage(Message, SM, Loc);
+ return Error;
+}
+
+static Optional<ClangTidyError>
+tallyNolintBegins(const ClangTidyContext &Context, const SourceManager &SM,
+ StringRef CheckName, SmallVector<StringRef> Lines,
+ SourceLocation LinesLoc,
+ SmallVector<SourceLocation> &SpecificNolintBegins,
+ SmallVector<SourceLocation> &GlobalNolintBegins) {
+ // Keep a running total of how many NOLINT(BEGIN...END) blocks are active.
+ size_t NolintIndex;
+ bool SuppressionIsSpecific;
+ auto List = [&]() -> SmallVector<SourceLocation> * {
+ return SuppressionIsSpecific ? &SpecificNolintBegins : &GlobalNolintBegins;
+ };
+ for (const auto &Line : Lines) {
+ if (isNOLINTFound("NOLINTBEGIN", CheckName, Line, &NolintIndex,
+ &SuppressionIsSpecific)) {
+ // Check if a new block is being started.
+ List()->emplace_back(LinesLoc.getLocWithOffset(NolintIndex));
+ } else if (isNOLINTFound("NOLINTEND", CheckName, Line, &NolintIndex,
+ &SuppressionIsSpecific)) {
+ // Check if the previous block is being closed.
+ if (!List()->empty()) {
+ List()->pop_back();
+ } else {
+ // Trying to close a nonexistent block. Return a diagnostic about this
+ // misuse that can be displayed along with the original clang-tidy check
+ // that the user was attempting to suppress.
+ return createNolintError(Context, SM,
+ LinesLoc.getLocWithOffset(NolintIndex), false);
+ }
+ }
+ // Advance source location to the next line.
+ LinesLoc = LinesLoc.getLocWithOffset(Line.size() + sizeof('\n'));
+ }
+ return None; // All NOLINT(BEGIN/END) use has been consistent so far.
+}
+
+static bool
+lineIsWithinNolintBegin(const ClangTidyContext &Context,
+ SmallVectorImpl<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));
+
+ // Check if there's an open NOLINT(BEGIN...END) block on the previous lines.
+ SmallVector<StringRef> PrevLines;
+ TextBeforeDiag.split(PrevLines, '\n');
+ SmallVector<SourceLocation> SpecificNolintBegins;
+ SmallVector<SourceLocation> GlobalNolintBegins;
+ auto Error =
+ tallyNolintBegins(Context, SM, CheckName, PrevLines, FileStartLoc,
+ SpecificNolintBegins, GlobalNolintBegins);
+ if (Error) {
+ SuppressionErrors.emplace_back(Error.getValue());
+ return false;
+ }
+ bool WithinNolintBegin =
+ !SpecificNolintBegins.empty() || !GlobalNolintBegins.empty();
+
+ // Check that every block is terminated correctly on the following lines.
+ SmallVector<StringRef> FollowingLines;
+ TextAfterDiag.split(FollowingLines, '\n');
+ Error = tallyNolintBegins(Context, SM, CheckName, FollowingLines, Loc,
+ SpecificNolintBegins, GlobalNolintBegins);
+ if (Error) {
+ SuppressionErrors.emplace_back(Error.getValue());
+ return false;
+ }
+
+ // The following blocks were never closed. Return diagnostics for each
+ // instance that can be displayed along with the original clang-tidy check
+ // that the user was attempting to suppress.
+ for (const auto NolintBegin : SpecificNolintBegins) {
+ auto Error = createNolintError(Context, SM, NolintBegin, true);
+ SuppressionErrors.emplace_back(Error);
+ }
+ for (const auto NolintBegin : GlobalNolintBegins) {
+ auto Error = createNolintError(Context, SM, NolintBegin, true);
+ SuppressionErrors.emplace_back(Error);
+ }
+
+ return WithinNolintBegin && SuppressionErrors.empty();
+}
+
+static bool
+lineIsMarkedWithNOLINT(const ClangTidyContext &Context,
+ SmallVectorImpl<ClangTidyError> &SuppressionErrors,
+ bool AllowIO, const SourceManager &SM,
+ SourceLocation Loc, StringRef CheckName) {
+ // Get source code for this location.
FileID File;
unsigned Offset;
std::tie(File, Offset) = SM.getDecomposedSpellingLoc(Loc);
- llvm::Optional<StringRef> Buffer = getBuffer(SM, File, AllowIO);
+ Optional<StringRef> Buffer = getBuffer(SM, File, AllowIO);
if (!Buffer)
return false;
// Check if there's a NOLINT on this line.
- StringRef RestOfLine = Buffer->substr(Offset).split('\n').first;
- if (IsNOLINTFound("NOLINT", RestOfLine, DiagID, Context))
+ StringRef TextAfterDiag = Buffer->substr(Offset);
+ StringRef RestOfThisLine, FollowingLines;
+ std::tie(RestOfThisLine, FollowingLines) = TextAfterDiag.split('\n');
+ if (isNOLINTFound("NOLINT", CheckName, RestOfThisLine))
return true;
// Check if there's a NOLINTNEXTLINE on the previous line.
- StringRef PrevLine =
- Buffer->substr(0, Offset).rsplit('\n').first.rsplit('\n').second;
- return IsNOLINTFound("NOLINTNEXTLINE", PrevLine, DiagID, Context);
+ StringRef TextBeforeDiag = Buffer->substr(0, Offset);
+ size_t LastNewLinePos = TextBeforeDiag.rfind('\n');
+ StringRef PrevLines = (LastNewLinePos == StringRef::npos)
+ ? StringRef()
+ : TextBeforeDiag.slice(0, LastNewLinePos);
+ LastNewLinePos = PrevLines.rfind('\n');
+ StringRef PrevLine = (LastNewLinePos == StringRef::npos)
+ ? PrevLines
+ : PrevLines.substr(LastNewLinePos + 1);
+ if (isNOLINTFound("NOLINTNEXTLINE", CheckName, PrevLine))
+ return true;
+
+ // Check if this line is within a NOLINT(BEGIN...END) block.
+ return lineIsWithinNolintBegin(Context, SuppressionErrors, SM, Loc, CheckName,
+ TextBeforeDiag, TextAfterDiag);
}
-static bool LineIsMarkedWithNOLINTinMacro(const SourceManager &SM,
- SourceLocation Loc, unsigned DiagID,
- const ClangTidyContext &Context,
- bool AllowIO) {
+static bool lineIsMarkedWithNOLINTinMacro(
+ const Diagnostic &Info, const ClangTidyContext &Context,
+ SmallVectorImpl<ClangTidyError> &SuppressionErrors, bool AllowIO) {
+ const SourceManager &SM = Info.getSourceManager();
+ SourceLocation Loc = Info.getLocation();
+ std::string CheckName = Context.getCheckName(Info.getID());
while (true) {
- if (LineIsMarkedWithNOLINT(SM, Loc, DiagID, Context, AllowIO))
+ if (lineIsMarkedWithNOLINT(Context, SuppressionErrors, AllowIO, SM, Loc,
+ CheckName))
return true;
if (!Loc.isMacroID())
return false;
bool shouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel,
const Diagnostic &Info, ClangTidyContext &Context,
bool AllowIO) {
+ SmallVector<ClangTidyError, 1> Unused;
+ bool ShouldSuppress =
+ shouldSuppressDiagnostic(DiagLevel, Info, Context, Unused, AllowIO);
+ assert(Unused.empty());
+ return ShouldSuppress;
+}
+
+bool shouldSuppressDiagnostic(
+ DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info,
+ ClangTidyContext &Context,
+ SmallVectorImpl<ClangTidyError> &SuppressionErrors, bool AllowIO) {
return Info.getLocation().isValid() &&
DiagLevel != DiagnosticsEngine::Error &&
DiagLevel != DiagnosticsEngine::Fatal &&
- LineIsMarkedWithNOLINTinMacro(Info.getSourceManager(),
- Info.getLocation(), Info.getID(),
- Context, AllowIO);
+ lineIsMarkedWithNOLINTinMacro(Info, Context, SuppressionErrors,
+ AllowIO);
}
const llvm::StringMap<tooling::Replacements> *
if (LastErrorWasIgnored && DiagLevel == DiagnosticsEngine::Note)
return;
- if (shouldSuppressDiagnostic(DiagLevel, Info, Context)) {
+ SmallVector<ClangTidyError, 1> SuppressionErrors;
+ if (shouldSuppressDiagnostic(DiagLevel, Info, Context, SuppressionErrors)) {
++Context.Stats.ErrorsIgnoredNOLINT;
// Ignored a warning, should ignore related notes as well
LastErrorWasIgnored = true;
if (Info.hasSourceManager())
checkFilters(Info.getLocation(), Info.getSourceManager());
+
+ Context.DiagEngine->Clear();
+ for (const auto &Error : SuppressionErrors)
+ Context.diag(Error);
}
bool ClangTidyDiagnosticConsumer::passesLineFilter(StringRef FileName,
If a specific suppression mechanism is not available for a certain warning, or
its use is not desired for some reason, :program:`clang-tidy` has a generic
-mechanism to suppress diagnostics using ``NOLINT`` or ``NOLINTNEXTLINE``
-comments.
+mechanism to suppress diagnostics using ``NOLINT``, ``NOLINTNEXTLINE``, and
+``NOLINTBEGIN`` ... ``NOLINTEND`` comments.
The ``NOLINT`` comment instructs :program:`clang-tidy` to ignore warnings on the
*same line* (it doesn't apply to a function, a block of code or any other
-language construct, it applies to the line of code it is on). If introducing the
-comment in the same line would change the formatting in undesired way, the
+language construct; it applies to the line of code it is on). If introducing the
+comment in the same line would change the formatting in an undesired way, the
``NOLINTNEXTLINE`` comment allows to suppress clang-tidy warnings on the *next
-line*.
+line*. The ``NOLINTBEGIN`` and ``NOLINTEND`` comments allow suppressing
+clang-tidy warnings on *multiple lines* (affecting all lines between the two
+comments).
-Both comments can be followed by an optional list of check names in parentheses
+All comments can be followed by an optional list of check names in parentheses
(see below for the formal syntax).
For example:
// Silence only the specified diagnostics for the next line
// NOLINTNEXTLINE(google-explicit-constructor, google-runtime-int)
Foo(bool param);
+
+ // Silence only the specified checks for all lines between the BEGIN and END
+ // NOLINTBEGIN(google-explicit-constructor, google-runtime-int)
+ Foo(short param);
+ Foo(long param);
+ // NOLINTEND(google-explicit-constructor, google-runtime-int)
};
-The formal syntax of ``NOLINT``/``NOLINTNEXTLINE`` is the following:
+The formal syntax of ``NOLINT``, ``NOLINTNEXTLINE``, and ``NOLINTBEGIN`` ...
+``NOLINTEND`` is the following:
.. parsed-literal::
lint-command:
**NOLINT**
**NOLINTNEXTLINE**
+ **NOLINTBEGIN**
+ **NOLINTEND**
-Note that whitespaces between ``NOLINT``/``NOLINTNEXTLINE`` and the opening
+Note that whitespaces between
+``NOLINT``/``NOLINTNEXTLINE``/``NOLINTBEGIN``/``NOLINTEND`` and the opening
parenthesis are not allowed (in this case the comment will be treated just as
-``NOLINT``/``NOLINTNEXTLINE``), whereas in check names list (inside the
-parenthesis) whitespaces can be used and will be ignored.
+``NOLINT``/``NOLINTNEXTLINE``/``NOLINTBEGIN``/``NOLINTEND``), whereas in the
+check names list (inside the parentheses), whitespaces can be used and will be
+ignored.
+
+All ``NOLINTBEGIN`` comments must be paired by an equal number of ``NOLINTEND``
+comments. Moreover, a pair of comments must have matching arguments -- for
+example, ``NOLINTBEGIN(check-name)`` can be paired with
+``NOLINTEND(check-name)`` but not with ``NOLINTEND`` `(zero arguments)`.
+:program:`clang-tidy` will generate a ``clang-tidy-nolint`` error diagnostic if
+any ``NOLINTBEGIN``/``NOLINTEND`` comment violates these requirements.
.. _LibTooling: https://clang.llvm.org/docs/LibTooling.html
.. _How To Setup Tooling For LLVM: https://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
--- /dev/null
+// RUN: %check_clang_tidy %s google-explicit-constructor %t
+
+class A { A(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTBEGIN
+class B1 { B1(int i); };
+// NOLINTEND
+
+// NOLINTBEGIN
+// NOLINTEND
+// NOLINTBEGIN
+class B2 { B2(int i); };
+// NOLINTEND
+
+// NOLINTBEGIN
+// NOLINTBEGIN
+class B3 { B3(int i); };
+// NOLINTEND
+// NOLINTEND
+
+// NOLINTBEGIN
+// NOLINTBEGIN
+// NOLINTEND
+class B4 { B4(int i); };
+// NOLINTEND
+
+// NOLINTBEGIN
+// NOLINTEND
+class B5 { B5(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+
+// NOLINTBEGIN(google-explicit-constructor)
+class C1 { C1(int i); };
+// NOLINTEND(google-explicit-constructor)
+
+// NOLINTBEGIN(*)
+class C2 { C2(int i); };
+// NOLINTEND(*)
+
+// NOLINTBEGIN(some-other-check)
+class C3 { C3(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+// NOLINTEND(some-other-check)
+
+// NOLINTBEGIN(some-other-check, google-explicit-constructor)
+class C4 { C4(int i); };
+// NOLINTEND(some-other-check, google-explicit-constructor)
+
+// NOLINTBEGIN(some-other-check, google-explicit-constructor)
+// NOLINTEND(some-other-check)
+class C5 { C5(int i); };
+// NOLINTEND(google-explicit-constructor)
+
+// NOLINTBEGIN(some-other-check, google-explicit-constructor)
+// NOLINTEND(google-explicit-constructor)
+class C6 { C6(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+// NOLINTEND(some-other-check)
+
+// NOLINTBEGIN(google-explicit-constructor)
+// NOLINTBEGIN(some-other-check)
+class C7 { C7(int i); };
+// NOLINTEND(some-other-check)
+// NOLINTEND(google-explicit-constructor)
+
+// NOLINTBEGIN(google-explicit-constructor)
+// NOLINTBEGIN(some-other-check)
+class C8 { C8(int i); };
+// NOLINTEND(google-explicit-constructor)
+// NOLINTEND(some-other-check)
+
+// NOLINTBEGIN(google-explicit-constructor)
+// NOLINTBEGIN
+class C9 { C9(int i); };
+// NOLINTEND
+// NOLINTEND(google-explicit-constructor)
+
+// NOLINTBEGIN
+// NOLINTBEGIN(google-explicit-constructor)
+class C10 { C10(int i); };
+// NOLINTEND(google-explicit-constructor)
+// NOLINTEND
+
+// NOLINTBEGIN(not-closed-bracket-is-treated-as-skip-all
+class C11 { C11(int i); };
+// NOLINTEND(not-closed-bracket-is-treated-as-skip-all
+
+// NOLINTBEGIN without-brackets-skip-all, another-check
+class C12 { C12(int i); };
+// NOLINTEND without-brackets-skip-all, another-check
+
+#define MACRO(X) class X { X(int i); };
+
+MACRO(D1)
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
+// CHECK-MESSAGES: :[[@LINE-4]]:28: note: expanded from macro 'MACRO
+
+// NOLINTBEGIN
+MACRO(D2)
+// NOLINTEND
+
+#define MACRO_NOARG class E { E(int i); };
+
+// NOLINTBEGIN
+MACRO_NOARG
+// NOLINTEND
+
+// NOLINTBEGIN
+#define MACRO_WRAPPED_WITH_NO_LINT class I { I(int i); };
+// NOLINTEND
+
+MACRO_WRAPPED_WITH_NO_LINT
+
+#define MACRO_NO_LINT_INSIDE_MACRO \
+ /* NOLINTBEGIN */ \
+ class J { J(int i); }; \
+ /* NOLINTEND */
+
+MACRO_NO_LINT_INSIDE_MACRO
+
+// CHECK-MESSAGES: Suppressed 18 warnings (18 NOLINT).