nik created this revision.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
The clang-tidy standalone tool implements the NOLINT filtering in
ClangTidyDiagnosticConsumer::HandleDiagnostic. For the plugin case no
ClangTidyDiagnosticConsumer is set up as it would have side effects with
the already set up diagnostic consumer.
This change introduces filtering in ClangTidyContext::diag() and returns
an active dummy DiagnosticBuilder in case the check was NOLINT-ed, so
that no check needs to be adapted. The filtering in HandleDiagnostic
needs to stay there as it is still relevant for non-tidy diagnostics.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D61487
Files:
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
test/clang-tidy/nolint-plugin.cpp
Index: test/clang-tidy/nolint-plugin.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/nolint-plugin.cpp
@@ -0,0 +1,49 @@
+// REQUIRES: static-analyzer
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin -Xclang clang-tidy -Xclang -plugin-arg-clang-tidy -Xclang -checks='-*,google-explicit-constructor,clang-diagnostic-unused-variable,clang-analyzer-core.UndefinedBinaryOperatorResult' -Wunused-variable -I%S/Inputs/nolint 2>&1 | FileCheck %s
+
+#include "trigger_warning.h"
+void I(int& Out) {
+ int In;
+ A1(In, Out);
+}
+// CHECK-NOT: trigger_warning.h:{{.*}} warning
+// CHECK-NOT: :[[@LINE-4]]:{{.*}} note
+
+class A { A(int i); };
+// CHECK-DAG: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+class B { B(int i); }; // NOLINT
+
+class C { C(int i); }; // NOLINT(for-some-other-check)
+// CHECK-DAG: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+class C1 { C1(int i); }; // NOLINT(*)
+
+class C2 { C2(int i); }; // NOLINT(not-closed-bracket-is-treated-as-skip-all
+
+class C3 { C3(int i); }; // NOLINT(google-explicit-constructor)
+
+class C4 { C4(int i); }; // NOLINT(some-check, google-explicit-constructor)
+
+class C5 { C5(int i); }; // NOLINT without-brackets-skip-all, another-check
+
+void f() {
+ int i;
+// CHECK-DAG: :[[@LINE-1]]:7: warning: unused variable 'i'
+ int j; // NOLINT
+ int k; // NOLINT(clang-diagnostic-unused-variable)
+}
+
+#define MACRO(X) class X { X(int i); };
+MACRO(D)
+// CHECK-DAG: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
+MACRO(E) // NOLINT
+
+#define MACRO_NOARG class F { F(int i); };
+MACRO_NOARG // NOLINT
+
+#define MACRO_NOLINT class G { G(int i); }; // NOLINT
+MACRO_NOLINT
+
+#define DOUBLE_MACRO MACRO(H) // NOLINT
+DOUBLE_MACRO
Index: clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -215,6 +215,8 @@
std::string ProfilePrefix;
bool AllowEnablingAnalyzerAlphaCheckers;
+
+ bool LastErrorWasIgnored;
};
/// \brief A diagnostic consumer that turns each \c Diagnostic into a
@@ -255,7 +257,6 @@
std::unique_ptr<llvm::Regex> HeaderFilter;
bool LastErrorRelatesToUserCode;
bool LastErrorPassesLineFilter;
- bool LastErrorWasIgnored;
};
} // end namespace tidy
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -186,7 +186,8 @@
bool AllowEnablingAnalyzerAlphaCheckers)
: DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)),
Profile(false),
- AllowEnablingAnalyzerAlphaCheckers(AllowEnablingAnalyzerAlphaCheckers) {
+ AllowEnablingAnalyzerAlphaCheckers(AllowEnablingAnalyzerAlphaCheckers),
+ LastErrorWasIgnored(false) {
// Before the first translation unit we can get errors related to command-line
// parsing, use empty string for the file name in this case.
setCurrentFile("");
@@ -194,12 +195,112 @@
ClangTidyContext::~ClangTidyContext() = default;
+static bool IsNOLINTFound(StringRef NolintDirectiveText, StringRef Line,
+ unsigned DiagID, const ClangTidyContext &Context) {
+ const 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() && Line[BracketIndex] == '(') {
+ ++BracketIndex;
+ const size_t BracketEndIndex = Line.find(')', BracketIndex);
+ if (BracketEndIndex != StringRef::npos) {
+ StringRef ChecksStr =
+ 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();
+ }
+ }
+ }
+ return true;
+}
+
+static bool LineIsMarkedWithNOLINT(const SourceManager &SM, SourceLocation Loc,
+ unsigned DiagID,
+ const ClangTidyContext &Context) {
+ bool Invalid;
+ const char *CharacterData = SM.getCharacterData(Loc, &Invalid);
+ if (Invalid)
+ return false;
+
+ // Check if there's a NOLINT on this line.
+ const char *P = CharacterData;
+ while (*P != '\0' && *P != '\r' && *P != '\n')
+ ++P;
+ StringRef RestOfLine(CharacterData, P - CharacterData + 1);
+ if (IsNOLINTFound("NOLINT", RestOfLine, DiagID, Context))
+ return true;
+
+ // Check if there's a NOLINTNEXTLINE on the previous line.
+ const char *BufBegin =
+ SM.getCharacterData(SM.getLocForStartOfFile(SM.getFileID(Loc)), &Invalid);
+ if (Invalid || P == BufBegin)
+ return false;
+
+ // Scan backwards over the current line.
+ P = CharacterData;
+ while (P != BufBegin && *P != '\n')
+ --P;
+
+ // If we reached the begin of the file there is no line before it.
+ if (P == BufBegin)
+ return false;
+
+ // Skip over the newline.
+ --P;
+ const char *LineEnd = P;
+
+ // Now we're on the previous line. Skip to the beginning of it.
+ while (P != BufBegin && *P != '\n')
+ --P;
+
+ RestOfLine = StringRef(P, LineEnd - P + 1);
+ if (IsNOLINTFound("NOLINTNEXTLINE", RestOfLine, DiagID, Context))
+ return true;
+
+ return false;
+}
+
+static bool LineIsMarkedWithNOLINTinMacro(const SourceManager &SM,
+ SourceLocation Loc, unsigned DiagID,
+ const ClangTidyContext &Context) {
+ while (true) {
+ if (LineIsMarkedWithNOLINT(SM, Loc, DiagID, Context))
+ return true;
+ if (!Loc.isMacroID())
+ return false;
+ Loc = SM.getImmediateExpansionRange(Loc).getBegin();
+ }
+ return false;
+}
+
DiagnosticBuilder ClangTidyContext::diag(
StringRef CheckName, SourceLocation Loc, StringRef Description,
DiagnosticIDs::Level Level /* = DiagnosticIDs::Warning*/) {
assert(Loc.isValid());
+ if (LastErrorWasIgnored && Level == DiagnosticIDs::Level::Note)
+ return DiagnosticBuilder::getDummy(DiagEngine);
+
unsigned ID = DiagEngine->getDiagnosticIDs()->getCustomDiagID(
Level, (Description + " [" + CheckName + "]").str());
+ if (LineIsMarkedWithNOLINTinMacro(DiagEngine->getSourceManager(), Loc, ID,
+ *this)) {
+ ++Stats.ErrorsIgnoredNOLINT;
+ // Ignored a warning, should ignore related notes as well
+ LastErrorWasIgnored = true;
+ return DiagnosticBuilder::getDummy(DiagEngine);
+ }
+
+ LastErrorWasIgnored = false;
CheckNamesByDiagnosticID.try_emplace(ID, CheckName);
return DiagEngine->Report(Loc, ID);
}
@@ -275,8 +376,7 @@
ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(
ClangTidyContext &Ctx, bool RemoveIncompatibleErrors)
: Context(Ctx), RemoveIncompatibleErrors(RemoveIncompatibleErrors),
- LastErrorRelatesToUserCode(false), LastErrorPassesLineFilter(false),
- LastErrorWasIgnored(false) {}
+ LastErrorRelatesToUserCode(false), LastErrorPassesLineFilter(false) {}
void ClangTidyDiagnosticConsumer::finalizeLastError() {
if (!Errors.empty()) {
@@ -299,97 +399,9 @@
LastErrorPassesLineFilter = false;
}
-static bool IsNOLINTFound(StringRef NolintDirectiveText, StringRef Line,
- unsigned DiagID, const ClangTidyContext &Context) {
- const 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() && Line[BracketIndex] == '(') {
- ++BracketIndex;
- const size_t BracketEndIndex = Line.find(')', BracketIndex);
- if (BracketEndIndex != StringRef::npos) {
- StringRef ChecksStr =
- 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();
- }
- }
- }
- return true;
-}
-
-static bool LineIsMarkedWithNOLINT(const SourceManager &SM, SourceLocation Loc,
- unsigned DiagID,
- const ClangTidyContext &Context) {
- bool Invalid;
- const char *CharacterData = SM.getCharacterData(Loc, &Invalid);
- if (Invalid)
- return false;
-
- // Check if there's a NOLINT on this line.
- const char *P = CharacterData;
- while (*P != '\0' && *P != '\r' && *P != '\n')
- ++P;
- StringRef RestOfLine(CharacterData, P - CharacterData + 1);
- if (IsNOLINTFound("NOLINT", RestOfLine, DiagID, Context))
- return true;
-
- // Check if there's a NOLINTNEXTLINE on the previous line.
- const char *BufBegin =
- SM.getCharacterData(SM.getLocForStartOfFile(SM.getFileID(Loc)), &Invalid);
- if (Invalid || P == BufBegin)
- return false;
-
- // Scan backwards over the current line.
- P = CharacterData;
- while (P != BufBegin && *P != '\n')
- --P;
-
- // If we reached the begin of the file there is no line before it.
- if (P == BufBegin)
- return false;
-
- // Skip over the newline.
- --P;
- const char *LineEnd = P;
-
- // Now we're on the previous line. Skip to the beginning of it.
- while (P != BufBegin && *P != '\n')
- --P;
-
- RestOfLine = StringRef(P, LineEnd - P + 1);
- if (IsNOLINTFound("NOLINTNEXTLINE", RestOfLine, DiagID, Context))
- return true;
-
- return false;
-}
-
-static bool LineIsMarkedWithNOLINTinMacro(const SourceManager &SM,
- SourceLocation Loc, unsigned DiagID,
- const ClangTidyContext &Context) {
- while (true) {
- if (LineIsMarkedWithNOLINT(SM, Loc, DiagID, Context))
- return true;
- if (!Loc.isMacroID())
- return false;
- Loc = SM.getImmediateExpansionRange(Loc).getBegin();
- }
- return false;
-}
-
void ClangTidyDiagnosticConsumer::HandleDiagnostic(
DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) {
- if (LastErrorWasIgnored && DiagLevel == DiagnosticsEngine::Note)
+ if (Context.LastErrorWasIgnored && DiagLevel == DiagnosticsEngine::Note)
return;
if (Info.getLocation().isValid() && DiagLevel != DiagnosticsEngine::Error &&
@@ -399,11 +411,11 @@
Context)) {
++Context.Stats.ErrorsIgnoredNOLINT;
// Ignored a warning, should ignore related notes as well
- LastErrorWasIgnored = true;
+ Context.LastErrorWasIgnored = true;
return;
}
- LastErrorWasIgnored = false;
+ Context.LastErrorWasIgnored = false;
// Count warnings/errors.
DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
