From: Alexander Kornienko Date: Thu, 22 May 2014 16:07:11 +0000 (+0000) Subject: Add clang-tidy -line-filter option to filter findings by line ranges. X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=dad4acbc52403cf8cfc32970d8295b8191e43802;p=platform%2Fupstream%2Fllvm.git Add clang-tidy -line-filter option to filter findings by line ranges. Summary: This is going to be used for a clang-tidy-diff script to display warnings in changed lines only. The option uses JSON, as its value is not intended to be entered manually. Reviewers: klimek Reviewed By: klimek Subscribers: cfe-commits Differential Revision: http://reviews.llvm.org/D3873 llvm-svn: 209450 --- diff --git a/clang-tools-extra/clang-tidy/CMakeLists.txt b/clang-tools-extra/clang-tidy/CMakeLists.txt index 0354d73..052aa99 100644 --- a/clang-tools-extra/clang-tidy/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/CMakeLists.txt @@ -6,6 +6,7 @@ add_clang_library(clangTidy ClangTidy.cpp ClangTidyModule.cpp ClangTidyDiagnosticConsumer.cpp + ClangTidyOptions.cpp DEPENDS ClangSACheckers diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp index 6ef1c34..5a031e2 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -203,7 +203,7 @@ StringRef ClangTidyContext::getCheckName(unsigned DiagnosticID) const { ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(ClangTidyContext &Ctx) : Context(Ctx), HeaderFilter(Ctx.getOptions().HeaderFilterRegex), - LastErrorRelatesToUserCode(false) { + LastErrorRelatesToUserCode(false), LastErrorPassesLineFilter(false) { IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions(); Diags.reset(new DiagnosticsEngine( IntrusiveRefCntPtr(new DiagnosticIDs), &*DiagOpts, this, @@ -220,11 +220,15 @@ void ClangTidyDiagnosticConsumer::finalizeLastError() { } else if (!LastErrorRelatesToUserCode) { ++Context.Stats.ErrorsIgnoredNonUserCode; Errors.pop_back(); + } else if (!LastErrorPassesLineFilter) { + ++Context.Stats.ErrorsIgnoredLineFilter; + Errors.pop_back(); } else { ++Context.Stats.ErrorsDisplayed; } } LastErrorRelatesToUserCode = false; + LastErrorPassesLineFilter = false; } void ClangTidyDiagnosticConsumer::HandleDiagnostic( @@ -259,32 +263,58 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic( llvm::makeArrayRef(Info.getFixItHints(), Info.getNumFixItHints()), Sources); - // Let argument parsing-related warnings through. - if (relatesToUserCode(Info.getLocation())) { - LastErrorRelatesToUserCode = true; + checkFilters(Info.getLocation()); +} + +bool ClangTidyDiagnosticConsumer::passesLineFilter(StringRef FileName, + unsigned LineNumber) const { + if (Context.getOptions().LineFilter.empty()) + return true; + for (const FileFilter& Filter : Context.getOptions().LineFilter) { + if (FileName.endswith(Filter.Name)) { + if (Filter.LineRanges.empty()) + return true; + for (const FileFilter::LineRange &Range : Filter.LineRanges) { + if (Range.first <= LineNumber && LineNumber <= Range.second) + return true; + } + return false; + } } + return false; } -bool ClangTidyDiagnosticConsumer::relatesToUserCode(SourceLocation Location) { +void ClangTidyDiagnosticConsumer::checkFilters(SourceLocation Location) { // Invalid location may mean a diagnostic in a command line, don't skip these. - if (!Location.isValid()) - return true; + if (!Location.isValid()) { + LastErrorRelatesToUserCode = true; + LastErrorPassesLineFilter = true; + return; + } const SourceManager &Sources = Diags->getSourceManager(); if (Sources.isInSystemHeader(Location)) - return false; + return; // FIXME: We start with a conservative approach here, but the actual type of // location needed depends on the check (in particular, where this check wants // to apply fixes). FileID FID = Sources.getDecomposedExpansionLoc(Location).first; - if (FID == Sources.getMainFileID()) - return true; - const FileEntry *File = Sources.getFileEntryForID(FID); + // -DMACRO definitions on the command line have locations in a virtual buffer // that doesn't have a FileEntry. Don't skip these as well. - return !File || HeaderFilter.match(File->getName()); + if (!File) { + LastErrorRelatesToUserCode = true; + LastErrorPassesLineFilter = true; + return; + } + + StringRef FileName(File->getName()); + unsigned LineNumber = Sources.getExpansionLineNumber(Location); + LastErrorRelatesToUserCode |= + Sources.isInMainFile(Location) || HeaderFilter.match(FileName); + LastErrorPassesLineFilter |= passesLineFilter(FileName, LineNumber); } namespace { diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h index 90e0610..1240ab9 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h @@ -79,12 +79,18 @@ private: struct ClangTidyStats { ClangTidyStats() : ErrorsDisplayed(0), ErrorsIgnoredCheckFilter(0), ErrorsIgnoredNOLINT(0), - ErrorsIgnoredNonUserCode(0) {} + ErrorsIgnoredNonUserCode(0), ErrorsIgnoredLineFilter(0) {} unsigned ErrorsDisplayed; unsigned ErrorsIgnoredCheckFilter; unsigned ErrorsIgnoredNOLINT; unsigned ErrorsIgnoredNonUserCode; + unsigned ErrorsIgnoredLineFilter; + + unsigned errorsIgnored() const { + return ErrorsIgnoredNOLINT + ErrorsIgnoredCheckFilter + + ErrorsIgnoredNonUserCode + ErrorsIgnoredLineFilter; + } }; /// \brief Every \c ClangTidyCheck reports errors through a \c DiagnosticEngine @@ -165,13 +171,15 @@ public: private: void finalizeLastError(); - bool relatesToUserCode(SourceLocation Location); + void checkFilters(SourceLocation Location); + bool passesLineFilter(StringRef FileName, unsigned LineNumber) const; ClangTidyContext &Context; llvm::Regex HeaderFilter; std::unique_ptr Diags; SmallVector Errors; bool LastErrorRelatesToUserCode; + bool LastErrorPassesLineFilter; }; } // end namespace tidy diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp new file mode 100644 index 0000000..fcf66ee --- /dev/null +++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp @@ -0,0 +1,64 @@ +//===--- ClangTidyOptions.cpp - clang-tidy ----------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "ClangTidyOptions.h" +#include "llvm/Support/YAMLTraits.h" + +using clang::tidy::FileFilter; + +LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(FileFilter) +LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(FileFilter::LineRange) + +namespace llvm { +namespace yaml { + +// Map std::pair to a JSON array of size 2. +template <> struct SequenceTraits { + static size_t size(IO &IO, FileFilter::LineRange &Range) { + return Range.first == 0 ? 0 : Range.second == 0 ? 1 : 2; + } + static unsigned &element(IO &IO, FileFilter::LineRange &Range, size_t Index) { + if (Index > 1) + IO.setError("Too many elements in line range."); + return Index == 0 ? Range.first : Range.second; + } +}; + +template <> struct MappingTraits { + static void mapping(IO &IO, FileFilter &File) { + IO.mapRequired("name", File.Name); + IO.mapOptional("lines", File.LineRanges); + } + static StringRef validate(IO &io, FileFilter &File) { + if (File.Name.empty()) + return "No file name specified"; + for (const FileFilter::LineRange &Range : File.LineRanges) { + if (Range.first <= 0 || Range.second <= 0) + return "Invalid line range"; + } + return StringRef(); + } +}; + +} // namespace yaml +} // namespace llvm + +namespace clang { +namespace tidy { + +/// \brief Parses -line-filter option and stores it to the \c Options. +llvm::error_code parseLineFilter(const std::string &LineFilter, + clang::tidy::ClangTidyOptions &Options) { + llvm::yaml::Input Input(LineFilter); + Input >> Options.LineFilter; + return Input.error(); +} + +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.h b/clang-tools-extra/clang-tidy/ClangTidyOptions.h index b7397fa..60b38ea 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyOptions.h +++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.h @@ -10,21 +10,41 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANG_TIDY_OPTIONS_H #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANG_TIDY_OPTIONS_H +#include "llvm/Support/system_error.h" #include +#include +#include namespace clang { namespace tidy { +struct FileFilter { + std::string Name; + // LineRange is a pair (inclusive). + typedef std::pair LineRange; + std::vector LineRanges; +}; + /// \brief Contains options for clang-tidy. struct ClangTidyOptions { ClangTidyOptions() : Checks("*"), AnalyzeTemporaryDtors(false) {} std::string Checks; + // Output warnings from headers matching this filter. Warnings from main files // will always be displayed. std::string HeaderFilterRegex; + + // Output warnings from certain line ranges of certain files only. If this + // list is emtpy, it won't be applied. + std::vector LineFilter; + bool AnalyzeTemporaryDtors; }; +/// \brief Parses LineFilter from JSON and stores it to the \c Options. +llvm::error_code parseLineFilter(const std::string &LineFilter, + clang::tidy::ClangTidyOptions &Options); + } // end namespace tidy } // end namespace clang diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp index a43af88..41b66d6 100644 --- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp +++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp @@ -32,6 +32,7 @@ const char DefaultChecks[] = "-clang-analyzer-alpha*," // Too many false positives. "-llvm-include-order," // Not implemented yet. "-google-*,"; // Doesn't apply to LLVM. + static cl::opt Checks("checks", cl::desc("Comma-separated list of globs with optional '-'\n" "prefix. Globs are processed in order of appearance\n" @@ -40,13 +41,28 @@ Checks("checks", cl::desc("Comma-separated list of globs with optional '-'\n" "prefix remove checks with matching names from the\n" "set of enabled checks."), cl::init(""), cl::cat(ClangTidyCategory)); + static cl::opt HeaderFilter("header-filter", cl::desc("Regular expression matching the names of the\n" - "headers to output diagnostics from.\n" - "Diagnostics from the main file of each\n" - "translation unit are always displayed."), + "headers to output diagnostics from. Diagnostics\n" + "from the main file of each translation unit are\n" + "always displayed.\n" + "Can be used together with -line-filter."), cl::init(""), cl::cat(ClangTidyCategory)); + +static cl::opt +LineFilter("line-filter", + cl::desc("List of files with line ranges to filter the\n" + "warnings. Can be used together with\n" + "-header-filter. The format of the list is a JSON\n" + "array of objects:\n" + " [\n" + " {\"name\":\"file1.cpp\",\"lines\":[[1,3],[5,7]]},\n" + " {\"name\":\"file2.h\"}\n" + " ]"), + cl::init(""), cl::cat(ClangTidyCategory)); + static cl::opt Fix("fix", cl::desc("Fix detected errors if possible."), cl::init(false), cl::cat(ClangTidyCategory)); @@ -63,16 +79,18 @@ AnalyzeTemporaryDtors("analyze-temporary-dtors", cl::init(false), cl::cat(ClangTidyCategory)); static void printStats(const clang::tidy::ClangTidyStats &Stats) { - unsigned ErrorsIgnored = Stats.ErrorsIgnoredNOLINT + - Stats.ErrorsIgnoredCheckFilter + - Stats.ErrorsIgnoredNonUserCode; - if (ErrorsIgnored) { - llvm::errs() << "Suppressed " << ErrorsIgnored << " warnings ("; + if (Stats.errorsIgnored()) { + llvm::errs() << "Suppressed " << Stats.errorsIgnored() << " warnings ("; StringRef Separator = ""; if (Stats.ErrorsIgnoredNonUserCode) { llvm::errs() << Stats.ErrorsIgnoredNonUserCode << " in non-user code"; Separator = ", "; } + if (Stats.ErrorsIgnoredLineFilter) { + llvm::errs() << Separator << Stats.ErrorsIgnoredLineFilter + << " due to line filter"; + Separator = ", "; + } if (Stats.ErrorsIgnoredNOLINT) { llvm::errs() << Separator << Stats.ErrorsIgnoredNOLINT << " NOLINT"; Separator = ", "; @@ -94,6 +112,12 @@ int main(int argc, const char **argv) { Options.Checks = DefaultChecks + Checks; Options.HeaderFilterRegex = HeaderFilter; Options.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors; + if (llvm::error_code Err = + clang::tidy::parseLineFilter(LineFilter, Options)) { + llvm::errs() << "Invalid LineFilter: " << Err.message() << "\n\nUsage:\n"; + llvm::cl::PrintHelpMessage(/*Hidden=*/false, /*Categorized=*/true); + return 1; + } // FIXME: Allow using --list-checks without positional arguments. if (ListChecks) { diff --git a/clang-tools-extra/test/clang-tidy/Inputs/line-filter/header1.h b/clang-tools-extra/test/clang-tidy/Inputs/line-filter/header1.h new file mode 100644 index 0000000..35d59ce --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/Inputs/line-filter/header1.h @@ -0,0 +1,3 @@ +class A1 { A1(int); }; +class B1 { B1(int); }; +class C1 { C1(int); }; diff --git a/clang-tools-extra/test/clang-tidy/Inputs/line-filter/header2.h b/clang-tools-extra/test/clang-tidy/Inputs/line-filter/header2.h new file mode 100644 index 0000000..bb12598 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/Inputs/line-filter/header2.h @@ -0,0 +1,3 @@ +class A2 { A2(int); }; +class B2 { B2(int); }; +class C2 { C2(int); }; diff --git a/clang-tools-extra/test/clang-tidy/Inputs/line-filter/header3.h b/clang-tools-extra/test/clang-tidy/Inputs/line-filter/header3.h new file mode 100644 index 0000000..b5198ca --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/Inputs/line-filter/header3.h @@ -0,0 +1 @@ +class A3 { A3(int); }; diff --git a/clang-tools-extra/test/clang-tidy/line-filter.cpp b/clang-tools-extra/test/clang-tidy/line-filter.cpp new file mode 100644 index 0000000..5b20e77 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/line-filter.cpp @@ -0,0 +1,27 @@ +// RUN: clang-tidy -checks='-*,google-explicit-constructor' -line-filter='[{"name":"%s","lines":[[18,18],[22,22]]},{"name":"header1.h","lines":[[1,2]]},{"name":"header2.h"},{"name":"header3.h"}]' -header-filter='header[12]\.h$' %s -- -I %S/Inputs/line-filter 2>&1 | FileCheck %s + +#include "header1.h" +// CHECK-NOT: header1.h:{{.*}} warning +// CHECK: header1.h:1:12: warning: Single-argument constructors must be explicit [google-explicit-constructor] +// CHECK: header1.h:2:12: warning: Single-argument constructors {{.*}} +// CHECK-NOT: header1.h:{{.*}} warning + +#include "header2.h" +// CHECK: header2.h:1:12: warning: Single-argument constructors {{.*}} +// CHECK: header2.h:2:12: warning: Single-argument constructors {{.*}} +// CHECK: header2.h:3:12: warning: Single-argument constructors {{.*}} +// CHECK-NOT: header2.h:{{.*}} warning + +#include "header3.h" +// CHECK-NOT: header3.h:{{.*}} warning + +class A { A(int); }; +// CHECK: :[[@LINE-1]]:11: warning: Single-argument constructors {{.*}} +class B { B(int); }; +// CHECK-NOT: :[[@LINE-1]]:{{.*}} warning +class C { C(int); }; +// CHECK: :[[@LINE-1]]:11: warning: Single-argument constructors {{.*}} + +// CHECK-NOT: warning: + +// CHECK: Suppressed 3 warnings (1 in non-user code, 2 due to line filter) diff --git a/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt b/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt index fc1217d..346f63a 100644 --- a/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt +++ b/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt @@ -8,6 +8,7 @@ include_directories(${CLANG_LINT_SOURCE_DIR}) add_extra_unittest(ClangTidyTests ClangTidyDiagnosticConsumerTest.cpp + ClangTidyOptionsTest.cpp LLVMModuleTest.cpp GoogleModuleTest.cpp MiscModuleTest.cpp) diff --git a/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp b/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp new file mode 100644 index 0000000..c2c3b6f --- /dev/null +++ b/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp @@ -0,0 +1,58 @@ +#include "ClangTidyOptions.h" +#include "gtest/gtest.h" + +namespace clang { +namespace tidy { +namespace test { + +TEST(ParseLineFilter, EmptyFilter) { + ClangTidyOptions Options; + EXPECT_FALSE(parseLineFilter("", Options)); + EXPECT_TRUE(Options.LineFilter.empty()); + EXPECT_FALSE(parseLineFilter("[]", Options)); + EXPECT_TRUE(Options.LineFilter.empty()); +} + +TEST(ParseLineFilter, InvalidFilter) { + ClangTidyOptions Options; + // TODO: Figure out why parsing succeeds here. + EXPECT_FALSE(parseLineFilter("asdf", Options)); + EXPECT_TRUE(Options.LineFilter.empty()); + + EXPECT_TRUE(parseLineFilter("[{}]", Options)); + EXPECT_TRUE(parseLineFilter(R"([{"name":""}])", Options)); + EXPECT_TRUE(parseLineFilter(R"([{"name":"test","lines":[[1]]}])", Options)); + EXPECT_TRUE( + parseLineFilter(R"([{"name":"test","lines":[[1,2,3]]}])", Options)); + EXPECT_TRUE( + parseLineFilter(R"([{"name":"test","lines":[[1,-1]]}])", Options)); +} + +TEST(ParseLineFilter, ValidFilter) { + ClangTidyOptions Options; + llvm::error_code Error = parseLineFilter( + R"([{"name":"file1.cpp","lines":[[3,15],[20,30],[42,42]]}, + {"name":"file2.h"}, + {"name":"file3.cc","lines":[[100,1000]]}])", + Options); + EXPECT_FALSE(Error); + EXPECT_EQ(3u, Options.LineFilter.size()); + EXPECT_EQ("file1.cpp", Options.LineFilter[0].Name); + EXPECT_EQ(3u, Options.LineFilter[0].LineRanges.size()); + EXPECT_EQ(3u, Options.LineFilter[0].LineRanges[0].first); + EXPECT_EQ(15u, Options.LineFilter[0].LineRanges[0].second); + EXPECT_EQ(20u, Options.LineFilter[0].LineRanges[1].first); + EXPECT_EQ(30u, Options.LineFilter[0].LineRanges[1].second); + EXPECT_EQ(42u, Options.LineFilter[0].LineRanges[2].first); + EXPECT_EQ(42u, Options.LineFilter[0].LineRanges[2].second); + EXPECT_EQ("file2.h", Options.LineFilter[1].Name); + EXPECT_EQ(0u, Options.LineFilter[1].LineRanges.size()); + EXPECT_EQ("file3.cc", Options.LineFilter[2].Name); + EXPECT_EQ(1u, Options.LineFilter[2].LineRanges.size()); + EXPECT_EQ(100u, Options.LineFilter[2].LineRanges[0].first); + EXPECT_EQ(1000u, Options.LineFilter[2].LineRanges[0].second); +} + +} // namespace test +} // namespace tidy +} // namespace clang