Add clang-tidy -line-filter option to filter findings by line ranges.

[alexfh]

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.

[klimek]

clang-tidy/ClangTidyDiagnosticConsumer.cpp

273

Debug output?

clang-tidy/ClangTidyDiagnosticConsumer.h

175

That's a strange interface. I assume you want to remove some duplication that way? If true, I'd rather pull out the common code into a 3rd helper function and use that from both places.

clang-tidy/ClangTidyOptions.h

23

// LineRange is a pair<start, end> (inclusive).
Or something like that.

[alexfh]

Removed debug output, added a comment for LineRange, changed the interface of
ClangTidyDiagnosticConsumer::relatesToUserCodeAndPassesLineFilter (and renamed it).

[klimek]

lg

clang-tidy/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Support
   )
 
 add_clang_library(clangTidy
   ClangTidy.cpp
   ClangTidyModule.cpp
   ClangTidyDiagnosticConsumer.cpp
+  ClangTidyOptions.cpp
 
   DEPENDS
   ClangSACheckers
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic

clang-tidy/ClangTidyDiagnosticConsumer.h

private:
   bool Positive;
   llvm::Regex Regex;
   std::unique_ptr<ChecksFilter> NextFilter;
 };
 
 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
 /// provided by this context.
 ///
 /// A \c ClangTidyCheck always has access to the active context to report
 /// warnings like:
 /// \code
public:
   void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
                         const Diagnostic &Info) override;
 
   // Flushes the internal diagnostics buffer to the ClangTidyContext.
   void finish() override;
 
 private:
   void finalizeLastError();
-  bool relatesToUserCode(SourceLocation Location);
+  std::pair<bool, bool>
+  relatesToUserCodeAndPassesLineFilter(SourceLocation Location);
+
+  bool passesLineFilter(StringRef FileName, unsigned LineNumber) const;
 
   ClangTidyContext &Context;
   llvm::Regex HeaderFilter;
   std::unique_ptr<DiagnosticsEngine> Diags;
   SmallVector<ClangTidyError, 8> Errors;
   bool LastErrorRelatesToUserCode;
+  bool LastErrorPassesLineFilter;
 };
 
 } // end namespace tidy
 } // end namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANG_TIDY_DIAGNOSTIC_CONSUMER_H

clang-tidy/ClangTidyDiagnosticConsumer.cpp

llvm::DenseMap<unsigned, std::string>::const_iterator I =
       CheckNamesByDiagnosticID.find(DiagnosticID);
   if (I != CheckNamesByDiagnosticID.end())
     return I->second;
   return "";
 }
 
 ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(ClangTidyContext &Ctx)
     : Context(Ctx), HeaderFilter(Ctx.getOptions().HeaderFilterRegex),
-      LastErrorRelatesToUserCode(false) {
+      LastErrorRelatesToUserCode(false), LastErrorPassesLineFilter(false) {
   IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
   Diags.reset(new DiagnosticsEngine(
       IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs), &*DiagOpts, this,
       /*ShouldOwnClient=*/false));
   Context.setDiagnosticsEngine(Diags.get());
 }
 
 void ClangTidyDiagnosticConsumer::finalizeLastError() {
   if (!Errors.empty()) {
     ClangTidyError &Error = Errors.back();
     if (!Context.getChecksFilter().isCheckEnabled(Error.CheckName)) {
       ++Context.Stats.ErrorsIgnoredCheckFilter;
       Errors.pop_back();
     } 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(
     DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) {
   if (DiagLevel == DiagnosticsEngine::Note) {
     assert(!Errors.empty() &&
            "A diagnostic note can only be appended to a message.");
   } else {
void ClangTidyDiagnosticConsumer::HandleDiagnostic(
   SourceManager *Sources = nullptr;
   if (Info.hasSourceManager())
     Sources = &Info.getSourceManager();
   Converter.emitDiagnostic(
       Info.getLocation(), DiagLevel, Message, Info.getRanges(),
       llvm::makeArrayRef(Info.getFixItHints(), Info.getNumFixItHints()),
       Sources);
 
-  // Let argument parsing-related warnings through.
-  if (relatesToUserCode(Info.getLocation())) {
-    LastErrorRelatesToUserCode = true;
+  auto Result = relatesToUserCodeAndPassesLineFilter(Info.getLocation());
+  LastErrorRelatesToUserCode |= Result.first;
+  LastErrorPassesLineFilter |= Result.second;
+}
+
+bool ClangTidyDiagnosticConsumer::passesLineFilter(StringRef FileName,
+                                                   unsigned LineNumber) const {
+  llvm::errs() << "passesLineFilter " << FileName << ":" << LineNumber << "\n";
+  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) {
+std::pair<bool, bool>
+ClangTidyDiagnosticConsumer::relatesToUserCodeAndPassesLineFilter(
+    SourceLocation Location) {
   // Invalid location may mean a diagnostic in a command line, don't skip these.
   if (!Location.isValid())
-    return true;
+    return std::make_pair(true, true);
 
   const SourceManager &Sources = Diags->getSourceManager();
   if (Sources.isInSystemHeader(Location))
-    return false;
+    return std::make_pair(false, false);
 
   // 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)
+    return std::make_pair(true, true);
+
+  StringRef FileName(File->getName());
+  unsigned LineNumber = Sources.getExpansionLineNumber(Location);
+  return std::make_pair(Sources.isInMainFile(Location) ||
+                            HeaderFilter.match(FileName),
+                        passesLineFilter(FileName, LineNumber));
 }
 
 namespace {
 struct LessClangTidyError {
   bool operator()(const ClangTidyError *LHS, const ClangTidyError *RHS) const {
     const ClangTidyMessage &M1 = LHS->Message;
     const ClangTidyMessage &M2 = RHS->Message;
 

clang-tidy/ClangTidyOptions.h

 //===--- ClangTidyOptions.h - clang-tidy ------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #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 <string>
+#include <utility>
+#include <vector>
 
 namespace clang {
 namespace tidy {
 
+struct FileFilter {
+  std::string Name;
+  typedef std::pair<unsigned, unsigned> LineRange;
+  std::vector<LineRange> 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<FileFilter> 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
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANG_TIDY_OPTIONS_H

clang-tidy/ClangTidyOptions.cpp

+//===--- 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<int, int> to a JSON array of size 2.
+template <> struct SequenceTraits<FileFilter::LineRange> {
+  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<FileFilter> {
+  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

clang-tidy/tool/ClangTidyMain.cpp

 
 static cl::extrahelp CommonHelp(CommonOptionsParser::HelpMessage);
 
 const char DefaultChecks[] =
     "*,"                       // Enable all checks, except these:
     "-clang-analyzer-alpha*,"  // Too many false positives.
     "-llvm-include-order,"     // Not implemented yet.
     "-google-*,";              // Doesn't apply to LLVM.
+
 static cl::opt<std::string>
 Checks("checks", cl::desc("Comma-separated list of globs with optional '-'\n"
                           "prefix. Globs are processed in order of appearance\n"
                           "in the list. Globs without '-' prefix add checks\n"
                           "with matching names to the set, globs with the '-'\n"
                           "prefix remove checks with matching names from the\n"
                           "set of enabled checks."),
        cl::init(""), cl::cat(ClangTidyCategory));
+
 static cl::opt<std::string>
 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."),
-             cl::init(""), cl::cat(ClangTidyCategory));
+                      "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<std::string>
+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"
+                    "  ]"),
+
 static cl::opt<bool> Fix("fix", cl::desc("Fix detected errors if possible."),
                          cl::init(false), cl::cat(ClangTidyCategory));
 
 static cl::opt<bool>
 ListChecks("list-checks",
            cl::desc("List all enabled checks and exit. Use with\n"
                     "-checks='*' to list all available checks."),
            cl::init(false), cl::cat(ClangTidyCategory));
 
 static cl::opt<bool>
 AnalyzeTemporaryDtors("analyze-temporary-dtors",
                       cl::desc("Enable temporary destructor-aware analysis in\n"
                                "clang-analyzer- checks."),
                       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 = ", ";
     }
     if (Stats.ErrorsIgnoredCheckFilter)
       llvm::errs() << Separator << Stats.ErrorsIgnoredCheckFilter
                    << " with check filters";
     llvm::errs() << ").\n";
     if (Stats.ErrorsIgnoredNonUserCode)
       llvm::errs() << "Use -header-filter='.*' to display errors from all "
                       "non-system headers.\n";
   }
 }
 
 int main(int argc, const char **argv) {
   CommonOptionsParser OptionsParser(argc, argv, ClangTidyCategory);
 
   clang::tidy::ClangTidyOptions Options;
   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) {
     llvm::outs() << "Enabled checks:";
     for (auto CheckName : clang::tidy::getCheckNames(Options))
       llvm::outs() << "\n    " << CheckName;
     llvm::outs() << "\n\n";
     return 0;

test/clang-tidy/Inputs/line-filter/header1.h

+class A1 { A1(int); };
+class B1 { B1(int); };
+class C1 { C1(int); };
+class A1 { A1(int); };
+class B1 { B1(int); };
+class C1 { C1(int); };

test/clang-tidy/Inputs/line-filter/header2.h

+class A2 { A2(int); };
+class B2 { B2(int); };
+class C2 { C2(int); };
+class A2 { A2(int); };
+class B2 { B2(int); };
+class C2 { C2(int); };

test/clang-tidy/Inputs/line-filter/header3.h

+class A3 { A3(int); };
+class A3 { A3(int); };

test/clang-tidy/line-filter.cpp

+// 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)

unittests/clang-tidy/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   support
   )
 
 get_filename_component(CLANG_LINT_SOURCE_DIR
   ${CMAKE_CURRENT_SOURCE_DIR}/../../clang-tidy REALPATH)
 include_directories(${CLANG_LINT_SOURCE_DIR})
 
 add_extra_unittest(ClangTidyTests
   ClangTidyDiagnosticConsumerTest.cpp
+  ClangTidyOptionsTest.cpp
   LLVMModuleTest.cpp
   GoogleModuleTest.cpp
   MiscModuleTest.cpp)
 
 target_link_libraries(ClangTidyTests
   clangAST
   clangASTMatchers
   clangBasic
   clangFrontend
   clangTidy
   clangTidyGoogleModule
   clangTidyLLVMModule
   clangTidyMiscModule
   clangTooling
   )

unittests/clang-tidy/ClangTidyOptionsTest.cpp

+#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