Change the behavior of clang-tidy -checks=, remove -disable-checks.

Summary:
Make checks filtering more intuitive and easy to use. Remove
-disable-checks and change the format of -checks= to a comma-separated list of
globs with optional '-' prefix to denote exclusion. The -checks= option is now
cumulative, so it modifies defaults, not overrides them. Each glob adds or
removes to the current set of checks, so the filter can be refined or overriden
by adding globs.

Example:
  The default value for -checks= is
  '*,-clang-analyzer-alpha*,-llvm-include-order,-llvm-namespace-comment,-google-*',
  which allows all checks except for the ones named clang-analyzer-alpha* and
  others specified with the leading '-'. To allow all google-* checks one can
  write:
    clang-tidy -checks=google-* ...
  If one needs only google-* checks, we first need to remove everything (-*):
    clang-tidy -checks=-*,google-*
  etc.

I'm not sure if we need to change something here, so I didn't touch the docs
yet.

Reviewers: klimek, alexfh

Reviewed By: alexfh

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D3770

llvm-svn: 208883

18 files changed, 136 insertions, 47 deletions

diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 8a47cd9f63e..65f801cf65b 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -111,16 +111,49 @@ ClangTidyMessage::ClangTidyMessage(StringRef Message,
 
 ClangTidyError::ClangTidyError(StringRef CheckName) : CheckName(CheckName) {}
 
-ChecksFilter::ChecksFilter(const ClangTidyOptions &Options)
-    : EnableChecks(Options.EnableChecksRegex),
-      DisableChecks(Options.DisableChecksRegex) {}
+// Returns true if GlobList starts with the negative indicator ('-'), removes it
+// from the GlobList.
+static bool ConsumeNegativeIndicator(StringRef &GlobList) {
+  if (GlobList.startswith("-")) {
+    GlobList = GlobList.substr(1);
+    return true;
+  }
+  return false;
+}
+// Converts first glob from the comma-separated list of globs to Regex and
+// removes it and the trailing comma from the GlobList.
+static llvm::Regex ConsumeGlob(StringRef &GlobList) {
+  StringRef Glob = GlobList.substr(0, GlobList.find(','));
+  GlobList = GlobList.substr(Glob.size() + 1);
+  llvm::SmallString<128> RegexText("^");
+  StringRef MetaChars("()^$|*+?.[]\\{}");
+  for (char C : Glob) {
+    if (C == '*')
+      RegexText.push_back('.');
+    else if (MetaChars.find(C))
+      RegexText.push_back('\\');
+    RegexText.push_back(C);
+  }
+  RegexText.push_back('$');
+  return llvm::Regex(RegexText);
+}
+
+ChecksFilter::ChecksFilter(StringRef GlobList)
+    : Positive(!ConsumeNegativeIndicator(GlobList)),
+      Regex(ConsumeGlob(GlobList)),
+      NextFilter(GlobList.empty() ? nullptr : new ChecksFilter(GlobList)) {}
+
+bool ChecksFilter::isCheckEnabled(StringRef Name, bool Enabled) {
+  if (Regex.match(Name))
+    Enabled = Positive;
 
-bool ChecksFilter::isCheckEnabled(StringRef Name) {
-  return EnableChecks.match(Name) && !DisableChecks.match(Name);
+  if (NextFilter)
+    Enabled = NextFilter->isCheckEnabled(Name, Enabled);
+  return Enabled;
 }
 
 ClangTidyContext::ClangTidyContext(const ClangTidyOptions &Options)
-    : DiagEngine(nullptr), Options(Options), Filter(Options) {}
+    : DiagEngine(nullptr), Options(Options), Filter(Options.Checks) {}
 
 DiagnosticBuilder ClangTidyContext::diag(
     StringRef CheckName, SourceLocation Loc, StringRef Description,
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
index 425075df552..90e0610ba85 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -60,12 +60,20 @@ struct ClangTidyError {
 /// \brief Filters checks by name.
 class ChecksFilter {
 public:
-  ChecksFilter(const ClangTidyOptions& Options);
-  bool isCheckEnabled(StringRef Name);
+  // GlobList is a comma-separated list of globs (only '*' metacharacter is
+  // supported) with optional '-' prefix to denote exclusion.
+  ChecksFilter(StringRef GlobList);
+  // Returns true if the check with the specified Name should be enabled.
+  // The result is the last matching glob's Positive flag. If Name is not
+  // matched by any globs, the check is not enabled.
+  bool isCheckEnabled(StringRef Name) { return isCheckEnabled(Name, false); }
 
 private:
-  llvm::Regex EnableChecks;
-  llvm::Regex DisableChecks;
+  bool isCheckEnabled(StringRef Name, bool Enabled);
+
+  bool Positive;
+  llvm::Regex Regex;
+  std::unique_ptr<ChecksFilter> NextFilter;
 };
 
 struct ClangTidyStats {
diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.h b/clang-tools-extra/clang-tidy/ClangTidyOptions.h
index f3e5fd22df9..b7397fabe0c 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.h
@@ -17,9 +17,8 @@ namespace tidy {
 
 /// \brief Contains options for clang-tidy.
 struct ClangTidyOptions {
-  ClangTidyOptions() : EnableChecksRegex(".*"), AnalyzeTemporaryDtors(false) {}
-  std::string EnableChecksRegex;
-  std::string DisableChecksRegex;
+  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;
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index 5c136bc3a52..4e815c0d174 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -27,18 +27,17 @@ static cl::OptionCategory ClangTidyCategory("clang-tidy options");
 
 static cl::extrahelp CommonHelp(CommonOptionsParser::HelpMessage);
 
-static cl::opt<std::string> Checks(
-    "checks",
-    cl::desc("Regular expression matching the names of the checks to be run."),
-    cl::init(".*"), cl::cat(ClangTidyCategory));
-static cl::opt<std::string> DisableChecks(
-    "disable-checks",
-    cl::desc("Regular expression matching the names of the checks to disable."),
-    cl::init("(clang-analyzer-alpha.*" // Too many false positives.
-             "|llvm-include-order"     // Not implemented yet.
-             "|llvm-namespace-comment" // Not complete.
-             "|google-.*)"),           // Doesn't apply to LLVM.
-    cl::cat(ClangTidyCategory));
+const char DefaultChecks[] =
+    "*,"                       // Enable all checks, except these:
+    "-clang-analyzer-alpha*,"  // Too many false positives.
+    "-llvm-include-order,"     // Not implemented yet.
+    "-llvm-namespace-comment," // Not complete.
+    "-google-*,";              // Doesn't apply to LLVM.
+static cl::opt<std::string>
+Checks("checks",
+       cl::desc("Comma-separated list of positive and negative globs matching\n"
+                "the names of the checks to be run."),
+       cl::init(""), cl::cat(ClangTidyCategory));
 static cl::opt<std::string> HeaderFilter(
     "header-filter",
     cl::desc("Regular expression matching the names of the headers to output\n"
@@ -88,8 +87,7 @@ int main(int argc, const char **argv) {
   CommonOptionsParser OptionsParser(argc, argv, ClangTidyCategory);
 
   clang::tidy::ClangTidyOptions Options;
-  Options.EnableChecksRegex = Checks;
-  Options.DisableChecksRegex = DisableChecks;
+  Options.Checks = DefaultChecks + Checks;
   Options.HeaderFilterRegex = HeaderFilter;
   Options.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors;
 
diff --git a/clang-tools-extra/test/clang-tidy/arg-comments.cpp b/clang-tools-extra/test/clang-tidy/arg-comments.cpp
index 32b95767bd2..f5f19c5ccd7 100644
--- a/clang-tools-extra/test/clang-tidy/arg-comments.cpp
+++ b/clang-tools-extra/test/clang-tidy/arg-comments.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-tidy --checks=misc-argument-comment %s -- | FileCheck %s
+// RUN: clang-tidy --checks='-*,misc-argument-comment' %s -- | FileCheck %s
 
 // FIXME: clang-tidy should provide a -verify mode to make writing these checks
 // easier and more accurate.
diff --git a/clang-tools-extra/test/clang-tidy/basic.cpp b/clang-tools-extra/test/clang-tidy/basic.cpp
index b01a63f4ce2..4c9c57bb120 100644
--- a/clang-tools-extra/test/clang-tidy/basic.cpp
+++ b/clang-tools-extra/test/clang-tidy/basic.cpp
@@ -1,5 +1,5 @@
 // RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
-// RUN: clang-tidy %t.cpp -checks='llvm-namespace-comment' -disable-checks='' -- > %t2.cpp
+// RUN: clang-tidy %t.cpp -checks='-*,llvm-namespace-comment' -- > %t2.cpp
 // RUN: FileCheck -input-file=%t2.cpp %s
 
 namespace i {
diff --git a/clang-tools-extra/test/clang-tidy/check_clang_tidy_fix.sh b/clang-tools-extra/test/clang-tidy/check_clang_tidy_fix.sh
index 9e69609da4b..819abf666ed 100755
--- a/clang-tools-extra/test/clang-tidy/check_clang_tidy_fix.sh
+++ b/clang-tools-extra/test/clang-tidy/check_clang_tidy_fix.sh
@@ -7,6 +7,5 @@ CHECK_TO_RUN=$2
 TEMPORARY_FILE=$3.cpp
 
 grep -Ev "// *[A-Z-]+:" ${INPUT_FILE} > ${TEMPORARY_FILE}
-clang-tidy ${TEMPORARY_FILE} -fix --checks=${CHECK_TO_RUN} \
-    --disable-checks="" -- --std=c++11
+clang-tidy ${TEMPORARY_FILE} -fix --checks="-*,${CHECK_TO_RUN}" -- --std=c++11
 FileCheck -input-file=${TEMPORARY_FILE} ${INPUT_FILE}
diff --git a/clang-tools-extra/test/clang-tidy/check_clang_tidy_output.sh b/clang-tools-extra/test/clang-tidy/check_clang_tidy_output.sh
index 80c4dbd9220..a282ce12dbc 100755
--- a/clang-tools-extra/test/clang-tidy/check_clang_tidy_output.sh
+++ b/clang-tools-extra/test/clang-tidy/check_clang_tidy_output.sh
@@ -5,6 +5,5 @@
 INPUT_FILE=$1
 CHECK_TO_RUN=$2
 
-clang-tidy --checks=${CHECK_TO_RUN} --disable-checks="" ${INPUT_FILE} \
-    -- --std=c++11 -x c++ \
+clang-tidy --checks="-*,${CHECK_TO_RUN}" ${INPUT_FILE} -- --std=c++11 -x c++ \
   | FileCheck ${INPUT_FILE}
diff --git a/clang-tools-extra/test/clang-tidy/deduplication.cpp b/clang-tools-extra/test/clang-tidy/deduplication.cpp
index c4f921bf43d..1723dac7088 100644
--- a/clang-tools-extra/test/clang-tidy/deduplication.cpp
+++ b/clang-tools-extra/test/clang-tidy/deduplication.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-tidy -checks=google-explicit-constructor -disable-checks='' %s -- | FileCheck %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' %s -- | FileCheck %s
 
 template<typename T>
 class A { A(T); };
diff --git a/clang-tools-extra/test/clang-tidy/diagnostic.cpp b/clang-tools-extra/test/clang-tidy/diagnostic.cpp
index e8a17e0e81d..b012e0896e4 100644
--- a/clang-tools-extra/test/clang-tidy/diagnostic.cpp
+++ b/clang-tools-extra/test/clang-tidy/diagnostic.cpp
@@ -1,7 +1,7 @@
-// RUN: clang-tidy -disable-checks='' %s.nonexistent.cpp -- | FileCheck -check-prefix=CHECK1 %s
-// RUN: clang-tidy -disable-checks='' %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK2 %s
-// RUN: clang-tidy -checks='(google-explicit-constructor|clang-diagnostic-literal-conversion)' -disable-checks='' %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK3 %s
-// RUN: clang-tidy -checks='clang-diagnostic-macro-redefined' -disable-checks='' %s -- -DMACRO_FROM_COMMAND_LINE | FileCheck -check-prefix=CHECK4 %s
+// RUN: clang-tidy %s.nonexistent.cpp -- | FileCheck -check-prefix=CHECK1 %s
+// RUN: clang-tidy -checks='google-explicit-constructor' %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK2 %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor,clang-diagnostic-literal-conversion' %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK3 %s
+// RUN: clang-tidy -checks='-*,clang-diagnostic-macro-redefined' %s -- -DMACRO_FROM_COMMAND_LINE | FileCheck -check-prefix=CHECK4 %s
 
 // CHECK1-NOT: warning
 // CHECK2-NOT: warning
diff --git a/clang-tools-extra/test/clang-tidy/file-filter.cpp b/clang-tools-extra/test/clang-tidy/file-filter.cpp
index fa740f34d35..fc12a994251 100644
--- a/clang-tools-extra/test/clang-tidy/file-filter.cpp
+++ b/clang-tools-extra/test/clang-tidy/file-filter.cpp
@@ -1,6 +1,6 @@
-// RUN: clang-tidy -checks=google-explicit-constructor -disable-checks='' -header-filter='' %s -- -I %S/Inputs/file-filter 2>&1 | FileCheck %s
-// RUN: clang-tidy -checks=google-explicit-constructor -disable-checks='' -header-filter='.*' %s -- -I %S/Inputs/file-filter 2>&1 | FileCheck --check-prefix=CHECK2 %s
-// RUN: clang-tidy -checks=google-explicit-constructor -disable-checks='' -header-filter='header2\.h' %s -- -I %S/Inputs/file-filter 2>&1 | FileCheck --check-prefix=CHECK3 %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='' %s -- -I %S/Inputs/file-filter 2>&1 | FileCheck %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' %s -- -I %S/Inputs/file-filter 2>&1 | FileCheck --check-prefix=CHECK2 %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='header2\.h' %s -- -I %S/Inputs/file-filter 2>&1 | FileCheck --check-prefix=CHECK3 %s
 
 #include "header1.h"
 // CHECK-NOT: warning:
diff --git a/clang-tools-extra/test/clang-tidy/fix.cpp b/clang-tools-extra/test/clang-tidy/fix.cpp
index 21638c541d6..f82ed0b1670 100644
--- a/clang-tools-extra/test/clang-tidy/fix.cpp
+++ b/clang-tools-extra/test/clang-tidy/fix.cpp
@@ -1,5 +1,5 @@
 // RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
-// RUN: clang-tidy %t.cpp -checks='(google-explicit-constructor|llvm-namespace-comment)' -disable-checks='' -fix -- > %t.msg 2>&1
+// RUN: clang-tidy %t.cpp -checks='-*,google-explicit-constructor,llvm-namespace-comment' -fix -- > %t.msg 2>&1
 // RUN: FileCheck -input-file=%t.cpp %s
 // RUN: FileCheck -input-file=%t.msg -check-prefix=CHECK-MESSAGES %s
 
diff --git a/clang-tools-extra/test/clang-tidy/macros.cpp b/clang-tools-extra/test/clang-tidy/macros.cpp
index b3204052360..dc5d131dcb9 100644
--- a/clang-tools-extra/test/clang-tidy/macros.cpp
+++ b/clang-tools-extra/test/clang-tidy/macros.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-tidy -checks=google-explicit-constructor -disable-checks='' %s -- | FileCheck %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' %s -- | FileCheck %s
 
 #define Q(name) class name { name(int i); }
 
diff --git a/clang-tools-extra/test/clang-tidy/nolint.cpp b/clang-tools-extra/test/clang-tidy/nolint.cpp
index d4a5c4cd623..5b4b454c23a 100644
--- a/clang-tools-extra/test/clang-tidy/nolint.cpp
+++ b/clang-tools-extra/test/clang-tidy/nolint.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-tidy -checks=google-explicit-constructor -disable-checks='' %s -- 2>&1 | FileCheck %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' %s -- 2>&1 | FileCheck %s
 
 class A { A(int i); };
 // CHECK: :[[@LINE-1]]:11: warning: Single-argument constructors must be explicit [google-explicit-constructor]
diff --git a/clang-tools-extra/test/clang-tidy/select-checks.cpp b/clang-tools-extra/test/clang-tidy/select-checks.cpp
index 71c6ad12d03..09860a35aca 100644
--- a/clang-tools-extra/test/clang-tidy/select-checks.cpp
+++ b/clang-tools-extra/test/clang-tidy/select-checks.cpp
@@ -1,5 +1,5 @@
 // RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
-// RUN: clang-tidy %t.cpp -fix -checks=^llvm-.* -disable-checks='' --
+// RUN: clang-tidy %t.cpp -fix -checks='-*,llvm-*' --
 // RUN: FileCheck -input-file=%t.cpp %s
 
 namespace i {
diff --git a/clang-tools-extra/test/clang-tidy/static-analyzer.cpp b/clang-tools-extra/test/clang-tidy/static-analyzer.cpp
index 230d851b0b5..08bd4b291da 100644
--- a/clang-tools-extra/test/clang-tidy/static-analyzer.cpp
+++ b/clang-tools-extra/test/clang-tidy/static-analyzer.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-tidy %s -checks='clang-analyzer-.*' -disable-checks='alpha' -- | FileCheck %s
+// RUN: clang-tidy %s -checks='-*,clang-analyzer-*,-clang-analyzer-alpha*' -- | FileCheck %s
 extern void *malloc(unsigned long);
 extern void free(void *);
 
diff --git a/clang-tools-extra/test/clang-tidy/temporaries.cpp b/clang-tools-extra/test/clang-tidy/temporaries.cpp
index 8719880bedc..a32474d5ac8 100644
--- a/clang-tools-extra/test/clang-tidy/temporaries.cpp
+++ b/clang-tools-extra/test/clang-tidy/temporaries.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-tidy -checks=clang-analyzer-core.NullDereference -disable-checks='' -analyze-temporary-dtors %s -- | FileCheck %s
+// RUN: clang-tidy -checks='-*,clang-analyzer-core.NullDereference' -analyze-temporary-dtors %s -- | FileCheck %s
 
 struct NoReturnDtor {
   ~NoReturnDtor() __attribute__((noreturn));
diff --git a/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp b/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
index 81e90715903..8936c82b2b4 100644
--- a/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
@@ -28,6 +28,59 @@ TEST(ClangTidyDiagnosticConsumer, SortsErrors) {
   EXPECT_EQ("variable []", Errors[1].Message.Message);
 }
 
+TEST(ChecksFilter, Empty) {
+  ChecksFilter Filter("");
+
+  EXPECT_TRUE(Filter.isCheckEnabled(""));
+  EXPECT_FALSE(Filter.isCheckEnabled("aaa"));
+}
+
+TEST(ChecksFilter, Nothing) {
+  ChecksFilter Filter("-*");
+
+  EXPECT_FALSE(Filter.isCheckEnabled(""));
+  EXPECT_FALSE(Filter.isCheckEnabled("a"));
+  EXPECT_FALSE(Filter.isCheckEnabled("-*"));
+  EXPECT_FALSE(Filter.isCheckEnabled("-"));
+  EXPECT_FALSE(Filter.isCheckEnabled("*"));
+}
+
+TEST(ChecksFilter, Everything) {
+  ChecksFilter Filter("*");
+
+  EXPECT_TRUE(Filter.isCheckEnabled(""));
+  EXPECT_TRUE(Filter.isCheckEnabled("aaaa"));
+  EXPECT_TRUE(Filter.isCheckEnabled("-*"));
+  EXPECT_TRUE(Filter.isCheckEnabled("-"));
+  EXPECT_TRUE(Filter.isCheckEnabled("*"));
+}
+
+TEST(ChecksFilter, Simple) {
+  ChecksFilter Filter("aaa");
+
+  EXPECT_TRUE(Filter.isCheckEnabled("aaa"));
+  EXPECT_FALSE(Filter.isCheckEnabled(""));
+  EXPECT_FALSE(Filter.isCheckEnabled("aa"));
+  EXPECT_FALSE(Filter.isCheckEnabled("aaaa"));
+  EXPECT_FALSE(Filter.isCheckEnabled("bbb"));
+}
+
+TEST(ChecksFilter, Complex) {
+  ChecksFilter Filter("*,-a.*,-b.*,a.a.*,-a.a.a.*,-..,-...,-..+,-*$,-*qwe*");
+
+  EXPECT_TRUE(Filter.isCheckEnabled("aaa"));
+  EXPECT_TRUE(Filter.isCheckEnabled("qqq"));
+  EXPECT_FALSE(Filter.isCheckEnabled("a."));
+  EXPECT_FALSE(Filter.isCheckEnabled("a.b"));
+  EXPECT_FALSE(Filter.isCheckEnabled("b."));
+  EXPECT_FALSE(Filter.isCheckEnabled("b.b"));
+  EXPECT_TRUE(Filter.isCheckEnabled("a.a.b"));
+  EXPECT_FALSE(Filter.isCheckEnabled("a.a.a.a"));
+  EXPECT_FALSE(Filter.isCheckEnabled("qwe"));
+  EXPECT_FALSE(Filter.isCheckEnabled("asdfqweasdf"));
+  EXPECT_TRUE(Filter.isCheckEnabled("asdfqwEasdf"));
+}
+
 } // namespace test
 } // namespace tidy
 } // namespace clang