Rename ChecksFilter to GlobList, as there's nothing specific to checks in it.

[alexfh]

Summary

Rename ChecksFilter to GlobList, as there's nothing specific to checks in it.
It's a rather generic way to represent sets of strings (or patterns), so it may
be used for something else in ClangTidy. The new name would not look strange
when used to filter other entities.

[klimek]

lg

clang-tidy/ClangTidy.h

 public:
   ClangTidyASTConsumerFactory(ClangTidyContext &Context);
 
   /// \brief Returns an ASTConsumer that runs the specified clang-tidy checks.
   clang::ASTConsumer *CreateASTConsumer(clang::CompilerInstance &Compiler,
                                         StringRef File);
 
   /// \brief Get the list of enabled checks.
-  std::vector<std::string> getCheckNames(ChecksFilter &Filter);
+  std::vector<std::string> getCheckNames(GlobList &Filter);
 
 private:
   typedef std::vector<std::pair<std::string, bool> > CheckersList;
-  CheckersList getCheckersControlList(ChecksFilter &Filter);
+  CheckersList getCheckersControlList(GlobList &Filter);
 
   ClangTidyContext &Context;
   std::unique_ptr<ClangTidyCheckFactories> CheckFactories;
 };
 
 /// \brief Fills the list of check names that are enabled when the provided
 /// filters are applied.
 std::vector<std::string> getCheckNames(const ClangTidyOptions &Options);

clang-tidy/ClangTidy.cpp

clang::ASTConsumer *ClangTidyASTConsumerFactory::CreateASTConsumer(
     clang::CompilerInstance &Compiler, StringRef File) {
   // FIXME: Move this to a separate method, so that CreateASTConsumer doesn't
   // modify Compiler.
   Context.setSourceManager(&Compiler.getSourceManager());
   Context.setCurrentFile(File);
   Context.setASTContext(&Compiler.getASTContext());
 
   std::vector<std::unique_ptr<ClangTidyCheck>> Checks;
-  ChecksFilter &Filter = Context.getChecksFilter();
+  GlobList &Filter = Context.getChecksFilter();
   CheckFactories->createChecks(Filter, Checks);
 
   std::unique_ptr<ast_matchers::MatchFinder> Finder(
       new ast_matchers::MatchFinder);
   for (auto &Check : Checks) {
     Check->setContext(&Context);
     Check->registerMatchers(&*Finder);
     Check->registerPPCallbacks(Compiler);
AnalysisConsumer->AddDiagnosticConsumer(
         new AnalyzerDiagnosticConsumer(Context));
     Consumers.push_back(AnalysisConsumer);
   }
   return new ClangTidyASTConsumer(Consumers, std::move(Finder),
                                   std::move(Checks));
 }
 
 std::vector<std::string>
-ClangTidyASTConsumerFactory::getCheckNames(ChecksFilter &Filter) {
+ClangTidyASTConsumerFactory::getCheckNames(GlobList &Filter) {
   std::vector<std::string> CheckNames;
   for (const auto &CheckFactory : *CheckFactories) {
-    if (Filter.isCheckEnabled(CheckFactory.first))
+    if (Filter.contains(CheckFactory.first))
       CheckNames.push_back(CheckFactory.first);
   }
 
   for (const auto &AnalyzerCheck : getCheckersControlList(Filter))
     CheckNames.push_back(AnalyzerCheckNamePrefix + AnalyzerCheck.first);
 
   std::sort(CheckNames.begin(), CheckNames.end());
   return CheckNames;
 }
 
 ClangTidyASTConsumerFactory::CheckersList
-ClangTidyASTConsumerFactory::getCheckersControlList(ChecksFilter &Filter) {
+ClangTidyASTConsumerFactory::getCheckersControlList(GlobList &Filter) {
   CheckersList List;
 
   bool AnalyzerChecksEnabled = false;
   for (StringRef CheckName : StaticAnalyzerChecks) {
     std::string Checker((AnalyzerCheckNamePrefix + CheckName).str());
     AnalyzerChecksEnabled =
         AnalyzerChecksEnabled ||
-        (!CheckName.startswith("debug") && Filter.isCheckEnabled(Checker));
+        (!CheckName.startswith("debug") && Filter.contains(Checker));
   }
 
   if (AnalyzerChecksEnabled) {
     // Run our regex against all possible static analyzer checkers.  Note that
     // debug checkers print values / run programs to visualize the CFG and are
     // thus not applicable to clang-tidy in general.
     //
     // Always add all core checkers if any other static analyzer checks are
     // enabled. This is currently necessary, as other path sensitive checks
     // rely on the core checkers.
     for (StringRef CheckName : StaticAnalyzerChecks) {
       std::string Checker((AnalyzerCheckNamePrefix + CheckName).str());
 
       if (CheckName.startswith("core") ||
-          (!CheckName.startswith("debug") && Filter.isCheckEnabled(Checker)))
+          (!CheckName.startswith("debug") && Filter.contains(Checker)))
         List.push_back(std::make_pair(CheckName, true));
     }
   }
   return List;
 }
 
 DiagnosticBuilder ClangTidyCheck::diag(SourceLocation Loc, StringRef Message,
                                        DiagnosticIDs::Level Level) {

clang-tidy/ClangTidyDiagnosticConsumer.h

struct ClangTidyError {
   std::string CheckName;
   ClangTidyMessage Message;
   tooling::Replacements Fix;
   SmallVector<ClangTidyMessage, 1> Notes;
 
   Level DiagLevel;
 };
 
-/// \brief Filters checks by name.
-class ChecksFilter {
+/// \brief Read-only set of strings represented as a list of positive and
+/// negative globs. Positive globs add all matched strings to the set, negative
+/// globs remove them in the order of appearance in the list.
+class GlobList {
 public:
   /// \brief \p GlobList is a comma-separated list of globs (only '*'
   /// metacharacter is supported) with optional '-' prefix to denote exclusion.
-  ChecksFilter(StringRef GlobList);
+  GlobList(StringRef Globs);
 
-  /// \brief Returns \c true if the check with the specified \p Name should be
-  /// enabled. The result is the last matching glob's Positive flag. If \p Name
-  /// is not matched by any globs, the check is not enabled.
-  bool isCheckEnabled(StringRef Name) { return isCheckEnabled(Name, false); }
+  /// \brief Returns \c true if the pattern matches \p S. The result is the last
+  /// matching glob's Positive flag.
+  bool contains(StringRef S) { return contains(S, false); }
 
 private:
-  bool isCheckEnabled(StringRef Name, bool Enabled);
+  bool contains(StringRef S, bool Contains);
 
   bool Positive;
   llvm::Regex Regex;
-  std::unique_ptr<ChecksFilter> NextFilter;
+  std::unique_ptr<GlobList> NextGlob;
 };
 
 /// \brief Contains displayed and ignored diagnostic counters for a ClangTidy
 /// run.
 struct ClangTidyStats {
   ClangTidyStats()
       : ErrorsDisplayed(0), ErrorsIgnoredCheckFilter(0), ErrorsIgnoredNOLINT(0),
         ErrorsIgnoredNonUserCode(0), ErrorsIgnoredLineFilter(0) {}
public:
   /// \brief Sets ASTContext for the current translation unit.
   void setASTContext(ASTContext *Context);
 
   /// \brief Returns the name of the clang-tidy check which produced this
   /// diagnostic ID.
   StringRef getCheckName(unsigned DiagnosticID) const;
 
   /// \brief Returns check filter for the \c CurrentFile.
-  ChecksFilter &getChecksFilter();
+  GlobList &getChecksFilter();
 
   /// \brief Returns global options.
   const ClangTidyGlobalOptions &getGlobalOptions() const;
 
   /// \brief Returns options for \c CurrentFile.
   const ClangTidyOptions &getOptions() const;
 
   /// \brief Returns \c ClangTidyStats containing issued and ignored diagnostic
private:
   /// \brief Store an \p Error.
   void storeError(const ClangTidyError &Error);
 
   std::vector<ClangTidyError> Errors;
   DiagnosticsEngine *DiagEngine;
   std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider;
 
   std::string CurrentFile;
-  std::unique_ptr<ChecksFilter> CheckFilter;
+  std::unique_ptr<GlobList> CheckFilter;
 
   ClangTidyStats Stats;
 
   llvm::DenseMap<unsigned, std::string> CheckNamesByDiagnosticID;
 };
 
 /// \brief A diagnostic consumer that turns each \c Diagnostic into a
 /// \c SourceManager-independent \c ClangTidyError.

clang-tidy/ClangTidyDiagnosticConsumer.cpp

for (char C : Glob) {
     else if (MetaChars.find(C) != StringRef::npos)
       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;
-
-  if (NextFilter)
-    Enabled = NextFilter->isCheckEnabled(Name, Enabled);
-  return Enabled;
+GlobList::GlobList(StringRef Globs)
+    : Positive(!ConsumeNegativeIndicator(Globs)),
+      Regex(ConsumeGlob(Globs)),
+      NextGlob(Globs.empty() ? nullptr : new GlobList(Globs)) {}
+
+bool GlobList::contains(StringRef S, bool Contains) {
+  if (Regex.match(S))
+    Contains = Positive;
+
+  if (NextGlob)
+    Contains = NextGlob->contains(S, Contains);
+  return Contains;
 }
 
 ClangTidyContext::ClangTidyContext(ClangTidyOptionsProvider *OptionsProvider)
     : DiagEngine(nullptr), OptionsProvider(OptionsProvider) {
   // 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("");
 }

 }
 
 void ClangTidyContext::setSourceManager(SourceManager *SourceMgr) {
   DiagEngine->setSourceManager(SourceMgr);
 }
 
 void ClangTidyContext::setCurrentFile(StringRef File) {
   CurrentFile = File;
-  CheckFilter.reset(new ChecksFilter(getOptions().Checks));
+  CheckFilter.reset(new GlobList(getOptions().Checks));
 }
 
 void ClangTidyContext::setASTContext(ASTContext *Context) {
   DiagEngine->SetArgToStringFn(&FormatASTNodeDiagnosticArgument, Context);
 }
 
 const ClangTidyGlobalOptions &ClangTidyContext::getGlobalOptions() const {
   return OptionsProvider->getGlobalOptions();
 }
 
 const ClangTidyOptions &ClangTidyContext::getOptions() const {
   return OptionsProvider->getOptions(CurrentFile);
 }
 
-ChecksFilter &ClangTidyContext::getChecksFilter() {
+GlobList &ClangTidyContext::getChecksFilter() {
   assert(CheckFilter != nullptr);
   return *CheckFilter;
 }
 
 /// \brief Store a \c ClangTidyError.
 void ClangTidyContext::storeError(const ClangTidyError &Error) {
   Errors.push_back(Error);
 }
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) &&
+    if (!Context.getChecksFilter().contains(Error.CheckName) &&
         Error.DiagLevel != ClangTidyError::Error) {
       ++Context.Stats.ErrorsIgnoredCheckFilter;
       Errors.pop_back();
     } else if (!LastErrorRelatesToUserCode) {
       ++Context.Stats.ErrorsIgnoredNonUserCode;
       Errors.pop_back();
     } else if (!LastErrorPassesLineFilter) {
       ++Context.Stats.ErrorsIgnoredLineFilter;

clang-tidy/ClangTidyModule.h

public:
   ///
   /// The \c ClangTidyCheckFactories object takes ownership of the \p Factory.
   void addCheckFactory(StringRef Name, CheckFactoryBase *Factory);
 
   /// \brief Create instances of all checks matching \p CheckRegexString and
   /// store them in \p Checks.
   ///
   /// The caller takes ownership of the return \c ClangTidyChecks.
-  void createChecks(ChecksFilter &Filter,
+  void createChecks(GlobList &Filter,
                     std::vector<std::unique_ptr<ClangTidyCheck>> &Checks);
 
   typedef std::map<std::string, CheckFactoryBase *> FactoryMap;
   FactoryMap::const_iterator begin() const { return Factories.begin(); }
   FactoryMap::const_iterator end() const { return Factories.end(); }
   bool empty() const { return Factories.empty(); }
 
 private:
   FactoryMap Factories;
 };
 
 } // end namespace tidy
 } // end namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANG_TIDY_MODULE_H

clang-tidy/ClangTidyModule.cpp

 }
 
 void ClangTidyCheckFactories::addCheckFactory(StringRef Name,
                                               CheckFactoryBase *Factory) {
   Factories[Name] = Factory;
 }
 
 void ClangTidyCheckFactories::createChecks(
-    ChecksFilter &Filter,
-    std::vector<std::unique_ptr<ClangTidyCheck>> &Checks) {
+    GlobList &Filter, std::vector<std::unique_ptr<ClangTidyCheck>> &Checks) {
   for (const auto &Factory : Factories) {
-    if (Filter.isCheckEnabled(Factory.first)) {
+    if (Filter.contains(Factory.first)) {
       ClangTidyCheck *Check = Factory.second->createCheck();
       Check->setName(Factory.first);
       Checks.emplace_back(Check);
     }
   }
 }
 
 } // namespace tidy
 } // namespace clang

unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp

TEST(ClangTidyDiagnosticConsumer, SortsErrors) {
   std::vector<ClangTidyError> Errors;
   runCheckOnCode<TestCheck>("int a;", &Errors);
   EXPECT_EQ(2ul, Errors.size());
   // FIXME: Remove " []" once the check name is removed from the message text.
   EXPECT_EQ("type specifier []", Errors[0].Message.Message);
   EXPECT_EQ("variable []", Errors[1].Message.Message);
 }
 
-TEST(ChecksFilter, Empty) {
-  ChecksFilter Filter("");
+TEST(GlobList, Empty) {
+  GlobList Filter("");
 
-  EXPECT_TRUE(Filter.isCheckEnabled(""));
-  EXPECT_FALSE(Filter.isCheckEnabled("aaa"));
+  EXPECT_TRUE(Filter.contains(""));
+  EXPECT_FALSE(Filter.contains("aaa"));
 }
 
-TEST(ChecksFilter, Nothing) {
-  ChecksFilter Filter("-*");
+TEST(GlobList, Nothing) {
+  GlobList Filter("-*");
 
-  EXPECT_FALSE(Filter.isCheckEnabled(""));
-  EXPECT_FALSE(Filter.isCheckEnabled("a"));
-  EXPECT_FALSE(Filter.isCheckEnabled("-*"));
-  EXPECT_FALSE(Filter.isCheckEnabled("-"));
-  EXPECT_FALSE(Filter.isCheckEnabled("*"));
+  EXPECT_FALSE(Filter.contains(""));
+  EXPECT_FALSE(Filter.contains("a"));
+  EXPECT_FALSE(Filter.contains("-*"));
+  EXPECT_FALSE(Filter.contains("-"));
+  EXPECT_FALSE(Filter.contains("*"));
 }
 
-TEST(ChecksFilter, Everything) {
-  ChecksFilter Filter("*");
+TEST(GlobList, Everything) {
+  GlobList Filter("*");
 
-  EXPECT_TRUE(Filter.isCheckEnabled(""));
-  EXPECT_TRUE(Filter.isCheckEnabled("aaaa"));
-  EXPECT_TRUE(Filter.isCheckEnabled("-*"));
-  EXPECT_TRUE(Filter.isCheckEnabled("-"));
-  EXPECT_TRUE(Filter.isCheckEnabled("*"));
+  EXPECT_TRUE(Filter.contains(""));
+  EXPECT_TRUE(Filter.contains("aaaa"));
+  EXPECT_TRUE(Filter.contains("-*"));
+  EXPECT_TRUE(Filter.contains("-"));
+  EXPECT_TRUE(Filter.contains("*"));
 }
 
-TEST(ChecksFilter, Simple) {
-  ChecksFilter Filter("aaa");
+TEST(GlobList, Simple) {
+  GlobList 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"));
+  EXPECT_TRUE(Filter.contains("aaa"));
+  EXPECT_FALSE(Filter.contains(""));
+  EXPECT_FALSE(Filter.contains("aa"));
+  EXPECT_FALSE(Filter.contains("aaaa"));
+  EXPECT_FALSE(Filter.contains("bbb"));
 }
 
-TEST(ChecksFilter, Complex) {
-  ChecksFilter Filter("*,-a.*,-b.*,a.1.*,-a.1.A.*,-..,-...,-..+,-*$,-*qwe*");
+TEST(GlobList, Complex) {
+  GlobList Filter("*,-a.*,-b.*,a.1.*,-a.1.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.1.b"));
-  EXPECT_FALSE(Filter.isCheckEnabled("a.1.A.a"));
-  EXPECT_FALSE(Filter.isCheckEnabled("qwe"));
-  EXPECT_FALSE(Filter.isCheckEnabled("asdfqweasdf"));
-  EXPECT_TRUE(Filter.isCheckEnabled("asdfqwEasdf"));
+  EXPECT_TRUE(Filter.contains("aaa"));
+  EXPECT_TRUE(Filter.contains("qqq"));
+  EXPECT_FALSE(Filter.contains("a."));
+  EXPECT_FALSE(Filter.contains("a.b"));
+  EXPECT_FALSE(Filter.contains("b."));
+  EXPECT_FALSE(Filter.contains("b.b"));
+  EXPECT_TRUE(Filter.contains("a.1.b"));
+  EXPECT_FALSE(Filter.contains("a.1.A.a"));
+  EXPECT_FALSE(Filter.contains("qwe"));
+  EXPECT_FALSE(Filter.contains("asdfqweasdf"));
+  EXPECT_TRUE(Filter.contains("asdfqwEasdf"));
 }
 
 } // namespace test
 } // namespace tidy
 } // namespace clang