[clang-tools-extra] r362702 - [clang-tidy] Make the plugin honor NOLINT
Nikolai Kosjar via cfe-commits
cfe-commits at lists.llvm.org
Thu Jun 6 06:13:27 PDT 2019
Author: nik
Date: Thu Jun 6 06:13:27 2019
New Revision: 362702
URL: http://llvm.org/viewvc/llvm-project?rev=362702&view=rev
Log:
[clang-tidy] Make the plugin honor NOLINT
Instantiate a ClangTidyDiagnosticConsumer also for the plugin case and
let it forward the diagnostics to the external diagnostic engine that is
already in place.
One minor difference to the clang-tidy executable case is that the
compiler checks/diagnostics are referred to with their original name.
For example, for -Wunused-variable the plugin will refer to the check as
"-Wunused-variable" while the clang-tidy executable will refer to that
as "clang-diagnostic- unused-variable". This is because the compiler
diagnostics never reach ClangTidyDiagnosticConsumer.
Differential Revision: https://reviews.llvm.org/D61487
Added:
clang-tools-extra/trunk/test/clang-tidy/nolint-plugin.cpp
clang-tools-extra/trunk/test/clang-tidy/nolintnextline-plugin.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tools-extra/trunk/clang-tidy/plugin/ClangTidyPlugin.cpp
clang-tools-extra/trunk/test/clang-tidy/basic.cpp
Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp?rev=362702&r1=362701&r2=362702&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp Thu Jun 6 06:13:27 2019
@@ -275,8 +275,10 @@ std::string ClangTidyContext::getCheckNa
}
ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(
- ClangTidyContext &Ctx, bool RemoveIncompatibleErrors)
- : Context(Ctx), RemoveIncompatibleErrors(RemoveIncompatibleErrors),
+ ClangTidyContext &Ctx, DiagnosticsEngine *ExternalDiagEngine,
+ bool RemoveIncompatibleErrors)
+ : Context(Ctx), ExternalDiagEngine(ExternalDiagEngine),
+ RemoveIncompatibleErrors(RemoveIncompatibleErrors),
LastErrorRelatesToUserCode(false), LastErrorPassesLineFilter(false),
LastErrorWasIgnored(false) {}
@@ -461,16 +463,22 @@ void ClangTidyDiagnosticConsumer::Handle
IsWarningAsError);
}
- ClangTidyDiagnosticRenderer Converter(
- Context.getLangOpts(), &Context.DiagEngine->getDiagnosticOptions(),
- Errors.back());
- SmallString<100> Message;
- Info.FormatDiagnostic(Message);
- FullSourceLoc Loc;
- if (Info.getLocation().isValid() && Info.hasSourceManager())
- Loc = FullSourceLoc(Info.getLocation(), Info.getSourceManager());
- Converter.emitDiagnostic(Loc, DiagLevel, Message, Info.getRanges(),
- Info.getFixItHints());
+ if (ExternalDiagEngine) {
+ // If there is an external diagnostics engine, like in the
+ // ClangTidyPluginAction case, forward the diagnostics to it.
+ forwardDiagnostic(Info);
+ } else {
+ ClangTidyDiagnosticRenderer Converter(
+ Context.getLangOpts(), &Context.DiagEngine->getDiagnosticOptions(),
+ Errors.back());
+ SmallString<100> Message;
+ Info.FormatDiagnostic(Message);
+ FullSourceLoc Loc;
+ if (Info.getLocation().isValid() && Info.hasSourceManager())
+ Loc = FullSourceLoc(Info.getLocation(), Info.getSourceManager());
+ Converter.emitDiagnostic(Loc, DiagLevel, Message, Info.getRanges(),
+ Info.getFixItHints());
+ }
if (Info.hasSourceManager())
checkFilters(Info.getLocation(), Info.getSourceManager());
@@ -494,6 +502,68 @@ bool ClangTidyDiagnosticConsumer::passes
return false;
}
+void ClangTidyDiagnosticConsumer::forwardDiagnostic(const Diagnostic &Info) {
+ // Acquire a diagnostic ID also in the external diagnostics engine.
+ auto DiagLevelAndFormatString =
+ Context.getDiagLevelAndFormatString(Info.getID(), Info.getLocation());
+ unsigned ExternalID = ExternalDiagEngine->getDiagnosticIDs()->getCustomDiagID(
+ DiagLevelAndFormatString.first, DiagLevelAndFormatString.second);
+
+ // Forward the details.
+ auto builder = ExternalDiagEngine->Report(Info.getLocation(), ExternalID);
+ for (auto Hint : Info.getFixItHints())
+ builder << Hint;
+ for (auto Range : Info.getRanges())
+ builder << Range;
+ for (unsigned Index = 0; Index < Info.getNumArgs(); ++Index) {
+ DiagnosticsEngine::ArgumentKind kind = Info.getArgKind(Index);
+ switch (kind) {
+ case clang::DiagnosticsEngine::ak_std_string:
+ builder << Info.getArgStdStr(Index);
+ break;
+ case clang::DiagnosticsEngine::ak_c_string:
+ builder << Info.getArgCStr(Index);
+ break;
+ case clang::DiagnosticsEngine::ak_sint:
+ builder << Info.getArgSInt(Index);
+ break;
+ case clang::DiagnosticsEngine::ak_uint:
+ builder << Info.getArgUInt(Index);
+ break;
+ case clang::DiagnosticsEngine::ak_tokenkind:
+ builder << static_cast<tok::TokenKind>(Info.getRawArg(Index));
+ break;
+ case clang::DiagnosticsEngine::ak_identifierinfo:
+ builder << Info.getArgIdentifier(Index);
+ break;
+ case clang::DiagnosticsEngine::ak_qual:
+ builder << Qualifiers::fromOpaqueValue(Info.getRawArg(Index));
+ break;
+ case clang::DiagnosticsEngine::ak_qualtype:
+ builder << QualType::getFromOpaquePtr((void *)Info.getRawArg(Index));
+ break;
+ case clang::DiagnosticsEngine::ak_declarationname:
+ builder << DeclarationName::getFromOpaqueInteger(Info.getRawArg(Index));
+ break;
+ case clang::DiagnosticsEngine::ak_nameddecl:
+ builder << reinterpret_cast<const NamedDecl *>(Info.getRawArg(Index));
+ break;
+ case clang::DiagnosticsEngine::ak_nestednamespec:
+ builder << reinterpret_cast<NestedNameSpecifier *>(Info.getRawArg(Index));
+ break;
+ case clang::DiagnosticsEngine::ak_declcontext:
+ builder << reinterpret_cast<DeclContext *>(Info.getRawArg(Index));
+ break;
+ case clang::DiagnosticsEngine::ak_qualtype_pair:
+ assert(false); // This one is not passed around.
+ break;
+ case clang::DiagnosticsEngine::ak_attr:
+ builder << reinterpret_cast<Attr *>(Info.getRawArg(Index));
+ break;
+ }
+ }
+}
+
void ClangTidyDiagnosticConsumer::checkFilters(SourceLocation Location,
const SourceManager &Sources) {
// Invalid location may mean a diagnostic in a command line, don't skip these.
Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h?rev=362702&r1=362701&r2=362702&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h Thu Jun 6 06:13:27 2019
@@ -190,6 +190,15 @@ public:
return AllowEnablingAnalyzerAlphaCheckers;
}
+ using DiagLevelAndFormatString = std::pair<DiagnosticIDs::Level, std::string>;
+ DiagLevelAndFormatString getDiagLevelAndFormatString(unsigned DiagnosticID,
+ SourceLocation Loc) {
+ return DiagLevelAndFormatString(
+ static_cast<DiagnosticIDs::Level>(
+ DiagEngine->getDiagnosticLevel(DiagnosticID, Loc)),
+ DiagEngine->getDiagnosticIDs()->getDescription(DiagnosticID));
+ }
+
private:
// Writes to Stats.
friend class ClangTidyDiagnosticConsumer;
@@ -242,6 +251,7 @@ bool ShouldSuppressDiagnostic(Diagnostic
class ClangTidyDiagnosticConsumer : public DiagnosticConsumer {
public:
ClangTidyDiagnosticConsumer(ClangTidyContext &Ctx,
+ DiagnosticsEngine *ExternalDiagEngine = nullptr,
bool RemoveIncompatibleErrors = true);
// FIXME: The concept of converting between FixItHints and Replacements is
@@ -266,7 +276,10 @@ private:
void checkFilters(SourceLocation Location, const SourceManager &Sources);
bool passesLineFilter(StringRef FileName, unsigned LineNumber) const;
+ void forwardDiagnostic(const Diagnostic &Info);
+
ClangTidyContext &Context;
+ DiagnosticsEngine *ExternalDiagEngine;
bool RemoveIncompatibleErrors;
std::vector<ClangTidyError> Errors;
std::unique_ptr<llvm::Regex> HeaderFilter;
Modified: clang-tools-extra/trunk/clang-tidy/plugin/ClangTidyPlugin.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/plugin/ClangTidyPlugin.cpp?rev=362702&r1=362701&r2=362702&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/plugin/ClangTidyPlugin.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/plugin/ClangTidyPlugin.cpp Thu Jun 6 06:13:27 2019
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "../ClangTidy.h"
+#include "../ClangTidyDiagnosticConsumer.h"
#include "../ClangTidyForceLinker.h"
#include "../ClangTidyModule.h"
#include "clang/Frontend/CompilerInstance.h"
@@ -19,29 +20,39 @@ namespace tidy {
/// The core clang tidy plugin action. This just provides the AST consumer and
/// command line flag parsing for using clang-tidy as a clang plugin.
class ClangTidyPluginAction : public PluginASTAction {
- /// Wrapper to grant the context the same lifetime as the action. We use
- /// MultiplexConsumer to avoid writing out all the forwarding methods.
+ /// Wrapper to grant the context and diagnostics engine the same lifetime as
+ /// the action.
+ /// We use MultiplexConsumer to avoid writing out all the forwarding methods.
class WrapConsumer : public MultiplexConsumer {
std::unique_ptr<ClangTidyContext> Context;
+ std::unique_ptr<DiagnosticsEngine> DiagEngine;
public:
WrapConsumer(std::unique_ptr<ClangTidyContext> Context,
+ std::unique_ptr<DiagnosticsEngine> DiagEngine,
std::vector<std::unique_ptr<ASTConsumer>> Consumer)
- : MultiplexConsumer(std::move(Consumer)), Context(std::move(Context)) {}
+ : MultiplexConsumer(std::move(Consumer)), Context(std::move(Context)),
+ DiagEngine(std::move(DiagEngine)) {}
};
public:
std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &Compiler,
StringRef File) override {
- // Insert the current diagnostics engine.
- Context->setDiagnosticsEngine(&Compiler.getDiagnostics());
+ // Create and set diagnostics engine
+ auto ExternalDiagEngine = &Compiler.getDiagnostics();
+ auto DiagConsumer =
+ new ClangTidyDiagnosticConsumer(*Context, ExternalDiagEngine);
+ auto DiagEngine = llvm::make_unique<DiagnosticsEngine>(
+ new DiagnosticIDs, new DiagnosticOptions, DiagConsumer);
+ Context->setDiagnosticsEngine(DiagEngine.get());
// Create the AST consumer.
ClangTidyASTConsumerFactory Factory(*Context);
std::vector<std::unique_ptr<ASTConsumer>> Vec;
Vec.push_back(Factory.CreateASTConsumer(Compiler, File));
- return llvm::make_unique<WrapConsumer>(std::move(Context), std::move(Vec));
+ return llvm::make_unique<WrapConsumer>(
+ std::move(Context), std::move(DiagEngine), std::move(Vec));
}
bool ParseArgs(const CompilerInstance &,
Modified: clang-tools-extra/trunk/test/clang-tidy/basic.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/basic.cpp?rev=362702&r1=362701&r2=362702&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/basic.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/basic.cpp Thu Jun 6 06:13:27 2019
@@ -1,4 +1,5 @@
// RUN: clang-tidy %s -checks='-*,llvm-namespace-comment' -- | FileCheck %s
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin -Xclang clang-tidy -Xclang -plugin-arg-clang-tidy -Xclang -checks='-*,llvm-namespace-comment' 2>&1 | FileCheck %s
namespace i {
}
Added: clang-tools-extra/trunk/test/clang-tidy/nolint-plugin.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/nolint-plugin.cpp?rev=362702&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/nolint-plugin.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/nolint-plugin.cpp Thu Jun 6 06:13:27 2019
@@ -0,0 +1,50 @@
+// 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' [-Wunused-variable]
+// 31:7: warning: unused variable 'i' [-Wunused-variable]
+// 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
Added: clang-tools-extra/trunk/test/clang-tidy/nolintnextline-plugin.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/nolintnextline-plugin.cpp?rev=362702&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/nolintnextline-plugin.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/nolintnextline-plugin.cpp Thu Jun 6 06:13:27 2019
@@ -0,0 +1,48 @@
+// 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' 2>&1 | FileCheck %s
+
+class A { A(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE
+class B { B(int i); };
+
+// NOLINTNEXTLINE(for-some-other-check)
+class C { C(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE(*)
+class C1 { C1(int i); };
+
+// NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all
+class C2 { C2(int i); };
+
+// NOLINTNEXTLINE(google-explicit-constructor)
+class C3 { C3(int i); };
+
+// NOLINTNEXTLINE(some-check, google-explicit-constructor)
+class C4 { C4(int i); };
+
+// NOLINTNEXTLINE without-brackets-skip-all, another-check
+class C5 { C5(int i); };
+
+
+// NOLINTNEXTLINE
+
+class D { D(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE
+//
+class E { E(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+#define MACRO(X) class X { X(int i); };
+MACRO(F)
+// CHECK: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
+// NOLINTNEXTLINE
+MACRO(G)
+
+#define MACRO_NOARG class H { H(int i); };
+// NOLINTNEXTLINE
+MACRO_NOARG
+
More information about the cfe-commits
mailing list