nkakuev created this revision.
nkakuev added reviewers: malcolm.parsons, alexfh.
nkakuev added a subscriber: cfe-commits.
Currently clang-tidy doesn't provide a practical way to suppress diagnostics
from headers you don't have control over. If using a function defined in such
header causes a false positive, there is no easy way to deal with this issue.
Since you //cannot// modify a header, you //cannot// add NOLINT (or any other
source annotations) to it. And adding NOLINT to each invocation of such
function is either impossible or hugely impractical if invocations are
ubiquitous.
Using the '-header-filter' doesn't help to address this issue as well. Unless
your own headers are strategically named, it is virtually impossible to craft a
regex that will filter out only the third-party ones.
The '-line-filter' can be used to suppress such warnings, but it's not very
convenient. Instead of excluding a line that produces a warning you have to
include all other lines. Plus, it provides no way to suppress only certain
warnings from a file.
The option I propose solves aforementioned problems. It allows a user to
specify a set of regular expressions to suppress diagnostics from files whose
names match these expressions. Additionally, a set of checks can be specified
to suppress only certain diagnostics.
The option can be used in the following way:
`clang-tidy
-suppress-checks-filter='[{"name":"falty_header.h"},{"name":"third-party/","checks":"google-explicit-constructor"}]'
source.cpp -- -c`
https://reviews.llvm.org/D26418
Files:
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tidy/ClangTidyOptions.cpp
clang-tidy/ClangTidyOptions.h
clang-tidy/tool/ClangTidyMain.cpp
test/clang-tidy/Inputs/suppress-checks-filter/1/header1.h
test/clang-tidy/Inputs/suppress-checks-filter/1/header2.h
test/clang-tidy/Inputs/suppress-checks-filter/2/header3.h
test/clang-tidy/Inputs/suppress-checks-filter/2/header4.h
test/clang-tidy/suppress-checks-filter.cpp
Index: test/clang-tidy/suppress-checks-filter.cpp
===================================================================
--- test/clang-tidy/suppress-checks-filter.cpp
+++ test/clang-tidy/suppress-checks-filter.cpp
@@ -0,0 +1,17 @@
+// RUN: clang-tidy -checks='-*,google-explicit-constructor,google-readability-casting' -header-filter='header*' -suppress-checks-filter='[{"name":"header1.h"},{"name":"header2.h","checks":"google-explicit-constructor"},{"name":"2/"}]' %s -- -I %S/Inputs/suppress-checks-filter 2>&1 | FileCheck %s
+
+#include "1/header1.h"
+// CHECK-NOT: header1.h:{{.*}} warning
+
+#include "1/header2.h"
+// CHECK-NOT: 1/header2.h:{{.*}} warning: single-argument constructors {{.*}}
+// CHECK: 1/header2.h:{{.*}} warning: redundant cast to the same type {{.*}}
+
+#include "2/header3.h"
+// CHECK-NOT: 2/header3.h:{{.*}} warning: single-argument constructors
+
+#include "2/header4.h"
+// CHECK-NOT: 2/header4.h:{{.*}} warning: single-argument constructors
+
+// CHECK: Suppressed 4 warnings (4 with suppressed checks filters)
+
Index: test/clang-tidy/Inputs/suppress-checks-filter/2/header4.h
===================================================================
--- test/clang-tidy/Inputs/suppress-checks-filter/2/header4.h
+++ test/clang-tidy/Inputs/suppress-checks-filter/2/header4.h
@@ -0,0 +1,2 @@
+class A4 { A4(int); };
+
Index: test/clang-tidy/Inputs/suppress-checks-filter/2/header3.h
===================================================================
--- test/clang-tidy/Inputs/suppress-checks-filter/2/header3.h
+++ test/clang-tidy/Inputs/suppress-checks-filter/2/header3.h
@@ -0,0 +1,2 @@
+class A3 { A3(int); };
+
Index: test/clang-tidy/Inputs/suppress-checks-filter/1/header2.h
===================================================================
--- test/clang-tidy/Inputs/suppress-checks-filter/1/header2.h
+++ test/clang-tidy/Inputs/suppress-checks-filter/1/header2.h
@@ -0,0 +1,6 @@
+class A2 { A2(int); };
+
+class B2 {
+ B2(int &In, int &Out) { Out = (int)In; }
+};
+
Index: test/clang-tidy/Inputs/suppress-checks-filter/1/header1.h
===================================================================
--- test/clang-tidy/Inputs/suppress-checks-filter/1/header1.h
+++ test/clang-tidy/Inputs/suppress-checks-filter/1/header1.h
@@ -0,0 +1,2 @@
+class A1 { A1(int); };
+
Index: clang-tidy/tool/ClangTidyMain.cpp
===================================================================
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -105,6 +105,20 @@
cl::init(""),
cl::cat(ClangTidyCategory));
+static cl::opt<std::string>
+ SuppressWarningsFromHeaders("suppress-checks-filter", cl::desc(R"(
+Suppress diagnostics from files whose names match
+provided regular expression. If a list of checks
+is specified, only diagnostics produces by these
+checks will be suppressed. The format of
+the argument is a JSON array of objects:
+[
+ {"name":"header1.h","checks":"boost-*,cert-*"},
+ {"name":"header2.h"}
+]
+)"),
+ cl::init(""), cl::cat(ClangTidyCategory));
+
static cl::opt<bool> Fix("fix", cl::desc(R"(
Apply suggested fixes. Without -fix-errors
clang-tidy will bail out if any compilation
@@ -201,9 +215,14 @@
llvm::errs() << Separator << Stats.ErrorsIgnoredNOLINT << " NOLINT";
Separator = ", ";
}
- if (Stats.ErrorsIgnoredCheckFilter)
+ if (Stats.ErrorsIgnoredCheckFilter) {
llvm::errs() << Separator << Stats.ErrorsIgnoredCheckFilter
<< " with check filters";
+ Separator = ", ";
+ }
+ if (Stats.ErrorsIgnoredSuppressChecksFilter)
+ llvm::errs() << Separator << Stats.ErrorsIgnoredSuppressChecksFilter
+ << " with suppressed checks filters";
llvm::errs() << ").\n";
if (Stats.ErrorsIgnoredNonUserCode)
llvm::errs() << "Use -header-filter=.* to display errors from all "
@@ -253,8 +272,21 @@
static std::unique_ptr<ClangTidyOptionsProvider> createOptionsProvider() {
ClangTidyGlobalOptions GlobalOptions;
+ bool ArgumentsAreValid = true;
if (std::error_code Err = parseLineFilter(LineFilter, GlobalOptions)) {
- llvm::errs() << "Invalid LineFilter: " << Err.message() << "\n\nUsage:\n";
+ llvm::errs() << "Invalid LineFilter: " << Err.message();
+ ArgumentsAreValid = false;
+ }
+
+ if (std::error_code Err = parseSuppressChecksFilter(
+ SuppressWarningsFromHeaders, GlobalOptions)) {
+ llvm::errs() << "Cannot parse '-suppress-checks-filter': "
+ << Err.message();
+ ArgumentsAreValid = false;
+ }
+
+ if (!ArgumentsAreValid) {
+ llvm::errs() << "\n\nUsage:\n";
llvm::cl::PrintHelpMessage(/*Hidden=*/false, /*Categorized=*/true);
return nullptr;
}
Index: clang-tidy/ClangTidyOptions.h
===================================================================
--- clang-tidy/ClangTidyOptions.h
+++ clang-tidy/ClangTidyOptions.h
@@ -36,12 +36,27 @@
std::vector<LineRange> LineRanges;
};
+/// \brief If a file name matches \c FileNameRegex, checks listed in \c Checks
+/// will be suppressed.
+struct SuppressChecksFilterEntry {
+ /// \brief File name pattern.
+ std::string FileNameRegex;
+
+ /// \brief List of checks to suppress.
+ std::string Checks;
+};
+
/// \brief Global options. These options are neither stored nor read from
/// configuration files.
struct ClangTidyGlobalOptions {
/// \brief Output warnings from certain line ranges of certain files only.
/// If empty, no warnings will be filtered.
std::vector<FileFilter> LineFilter;
+
+ /// \brief Suppress warnings from certain checks if a file name matches a
+ /// pattern. If no checks are specified, all warnings from a file will be
+ /// suppressed.
+ std::vector<SuppressChecksFilterEntry> SuppressChecksFilter;
};
/// \brief Contains options for clang-tidy. These options may be read from
@@ -247,6 +262,11 @@
std::error_code parseLineFilter(llvm::StringRef LineFilter,
ClangTidyGlobalOptions &Options);
+/// \brief Parses SuppressChecksFilter from JSON and stores it to the \p
+/// Options.
+std::error_code parseSuppressChecksFilter(llvm::StringRef SuppressChecksFilter,
+ ClangTidyGlobalOptions &Options);
+
/// \brief Parses configuration from JSON and returns \c ClangTidyOptions or an
/// error.
llvm::ErrorOr<ClangTidyOptions> parseConfiguration(llvm::StringRef Config);
Index: clang-tidy/ClangTidyOptions.cpp
===================================================================
--- clang-tidy/ClangTidyOptions.cpp
+++ clang-tidy/ClangTidyOptions.cpp
@@ -23,10 +23,12 @@
using clang::tidy::ClangTidyOptions;
using clang::tidy::FileFilter;
+using clang::tidy::SuppressChecksFilterEntry;
using OptionsSource = clang::tidy::ClangTidyOptionsProvider::OptionsSource;
LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(FileFilter)
LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(FileFilter::LineRange)
+LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(SuppressChecksFilterEntry)
LLVM_YAML_IS_SEQUENCE_VECTOR(ClangTidyOptions::StringPair)
LLVM_YAML_IS_SEQUENCE_VECTOR(std::string)
@@ -61,6 +63,18 @@
}
};
+template <> struct MappingTraits<SuppressChecksFilterEntry> {
+ static void mapping(IO &IO, SuppressChecksFilterEntry &Filter) {
+ IO.mapRequired("name", Filter.FileNameRegex);
+ IO.mapOptional("checks", Filter.Checks);
+ }
+ static StringRef validate(IO &io, SuppressChecksFilterEntry &Filter) {
+ if (Filter.FileNameRegex.empty())
+ return "No file name pattern specified";
+ return StringRef();
+ }
+};
+
template <> struct MappingTraits<ClangTidyOptions::StringPair> {
static void mapping(IO &IO, ClangTidyOptions::StringPair &KeyValue) {
IO.mapRequired("key", KeyValue.first);
@@ -315,6 +329,16 @@
return Input.error();
}
+/// \brief Parses -suppress-checks-filter option and stores it to the \c
+/// Options.
+std::error_code
+parseSuppressChecksFilter(StringRef SuppressChecksFilter,
+ clang::tidy::ClangTidyGlobalOptions &Options) {
+ llvm::yaml::Input Input(SuppressChecksFilter);
+ Input >> Options.SuppressChecksFilter;
+ return Input.error();
+}
+
llvm::ErrorOr<ClangTidyOptions> parseConfiguration(StringRef Config) {
llvm::yaml::Input Input(Config);
ClangTidyOptions Options;
Index: clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -15,6 +15,7 @@
#include "clang/Basic/SourceManager.h"
#include "clang/Tooling/Refactoring.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/Support/Regex.h"
#include "llvm/Support/Timer.h"
@@ -105,17 +106,20 @@
struct ClangTidyStats {
ClangTidyStats()
: ErrorsDisplayed(0), ErrorsIgnoredCheckFilter(0), ErrorsIgnoredNOLINT(0),
- ErrorsIgnoredNonUserCode(0), ErrorsIgnoredLineFilter(0) {}
+ ErrorsIgnoredNonUserCode(0), ErrorsIgnoredLineFilter(0),
+ ErrorsIgnoredSuppressChecksFilter(0) {}
unsigned ErrorsDisplayed;
unsigned ErrorsIgnoredCheckFilter;
unsigned ErrorsIgnoredNOLINT;
unsigned ErrorsIgnoredNonUserCode;
unsigned ErrorsIgnoredLineFilter;
+ unsigned ErrorsIgnoredSuppressChecksFilter;
unsigned errorsIgnored() const {
return ErrorsIgnoredNOLINT + ErrorsIgnoredCheckFilter +
- ErrorsIgnoredNonUserCode + ErrorsIgnoredLineFilter;
+ ErrorsIgnoredNonUserCode + ErrorsIgnoredLineFilter +
+ ErrorsIgnoredSuppressChecksFilter;
}
};
@@ -135,6 +139,10 @@
/// \endcode
class ClangTidyContext {
public:
+ /// \brief Collection of pairs that map filenames to suppressed checks.
+ typedef std::vector<std::pair<llvm::Regex, llvm::Optional<GlobList>>>
+ SuppressChecksMap;
+
/// \brief Initializes \c ClangTidyContext instance.
ClangTidyContext(std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider);
@@ -173,6 +181,9 @@
/// The \c CurrentFile can be changed using \c setCurrentFile.
GlobList &getChecksFilter();
+ /// \brief Returns \c SuppressChecksFilter.
+ SuppressChecksMap &getSuppressChecksFilter();
+
/// \brief Returns check filter for the \c CurrentFile which
/// selects checks for upgrade to error.
GlobList &getWarningAsErrorFilter();
@@ -228,6 +239,9 @@
/// \brief Store an \p Error.
void storeError(const ClangTidyError &Error);
+ /// \brief Initializes \c SuppressChecksFilter from \c ClangTidyGlobalOptions.
+ void InitializeSuppressChecksFilter();
+
std::vector<ClangTidyError> Errors;
DiagnosticsEngine *DiagEngine;
std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider;
@@ -237,6 +251,8 @@
std::unique_ptr<GlobList> CheckFilter;
std::unique_ptr<GlobList> WarningAsErrorFilter;
+ SuppressChecksMap SuppressChecksFilter;
+
LangOptions LangOpts;
ClangTidyStats Stats;
@@ -280,6 +296,10 @@
void checkFilters(SourceLocation Location);
bool passesLineFilter(StringRef FileName, unsigned LineNumber) const;
+ /// \brief Returns true if diagnostic \c Info is matched by \c
+ /// SuppressChecksFilter.
+ bool diagnosticMatchesSuppressChecksFilter(const Diagnostic &Info) const;
+
ClangTidyContext &Context;
std::unique_ptr<DiagnosticsEngine> Diags;
SmallVector<ClangTidyError, 8> Errors;
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -175,6 +175,7 @@
// 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("");
+ InitializeSuppressChecksFilter();
}
DiagnosticBuilder ClangTidyContext::diag(
@@ -229,6 +230,11 @@
return *CheckFilter;
}
+ClangTidyContext::SuppressChecksMap &
+ClangTidyContext::getSuppressChecksFilter() {
+ return SuppressChecksFilter;
+};
+
GlobList &ClangTidyContext::getWarningAsErrorFilter() {
assert(WarningAsErrorFilter != nullptr);
return *WarningAsErrorFilter;
@@ -239,6 +245,15 @@
Errors.push_back(Error);
}
+void ClangTidyContext::InitializeSuppressChecksFilter() {
+ for (const SuppressChecksFilterEntry &Entry :
+ getGlobalOptions().SuppressChecksFilter)
+ SuppressChecksFilter.emplace_back(llvm::Regex(Entry.FileNameRegex),
+ Entry.Checks.empty()
+ ? llvm::Optional<GlobList>()
+ : GlobList(Entry.Checks));
+}
+
StringRef ClangTidyContext::getCheckName(unsigned DiagnosticID) const {
llvm::DenseMap<unsigned, std::string>::const_iterator I =
CheckNamesByDiagnosticID.find(DiagnosticID);
@@ -312,13 +327,20 @@
return;
if (Info.getLocation().isValid() && DiagLevel != DiagnosticsEngine::Error &&
- DiagLevel != DiagnosticsEngine::Fatal &&
- LineIsMarkedWithNOLINTinMacro(Diags->getSourceManager(),
- Info.getLocation())) {
- ++Context.Stats.ErrorsIgnoredNOLINT;
- // Ignored a warning, should ignore related notes as well
- LastErrorWasIgnored = true;
- return;
+ DiagLevel != DiagnosticsEngine::Fatal) {
+ if (LineIsMarkedWithNOLINTinMacro(Diags->getSourceManager(),
+ Info.getLocation())) {
+ ++Context.Stats.ErrorsIgnoredNOLINT;
+ // Ignored a warning, should ignore related notes as well
+ LastErrorWasIgnored = true;
+ return;
+ }
+
+ if (diagnosticMatchesSuppressChecksFilter(Info)) {
+ ++Context.Stats.ErrorsIgnoredSuppressChecksFilter;
+ LastErrorWasIgnored = true;
+ return;
+ }
}
LastErrorWasIgnored = false;
@@ -402,6 +424,28 @@
return false;
}
+bool ClangTidyDiagnosticConsumer::diagnosticMatchesSuppressChecksFilter(
+ const Diagnostic &Info) const {
+ const SourceLocation &Location = Info.getLocation();
+ if (!Location.isValid() || !Location.isFileID() || !Info.hasSourceManager())
+ return false;
+
+ const SourceManager &SourceManager = Info.getSourceManager();
+ const StringRef FileName = SourceManager.getFilename(Location);
+ const StringRef CheckName = Context.getCheckName(Info.getID());
+
+ for (std::pair<llvm::Regex, llvm::Optional<GlobList>> &Entry :
+ Context.getSuppressChecksFilter()) {
+ if (Entry.first.match(FileName)) {
+ llvm::Optional<GlobList> &Checks = Entry.second;
+ if (!Checks.hasValue() || Checks.getValue().contains(CheckName))
+ return true;
+ }
+ }
+
+ return false;
+}
+
void ClangTidyDiagnosticConsumer::checkFilters(SourceLocation Location) {
// Invalid location may mean a diagnostic in a command line, don't skip these.
if (!Location.isValid()) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
