[clang-tidy][NFC] Move CachedGlobList to GlobList.h

Currently it's hidden inside ClangTidyDiagnosticConsumer,
so it's hard to know it exists.

Given that there are multiple uses of globs in clang-tidy,
it makes sense to have these classes publicly available
for other use cases that might benefit from it.

Also, add unit test by converting the existing tests
for GlobList into typed tests.

Reviewed By: salman-javed-nz

Differential Revision: https://reviews.llvm.org/D113422

Author: carlosgalvezp

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp

@@ -155,29 +155,6 @@ ClangTidyError::ClangTidyError(StringRef CheckName,
     : tooling::Diagnostic(CheckName, DiagLevel, BuildDirectory),
       IsWarningAsError(IsWarningAsError) {}
 
-class ClangTidyContext::CachedGlobList {
-public:
-  CachedGlobList(StringRef Globs) : Globs(Globs) {}
-
-  bool contains(StringRef S) {
-    switch (auto &Result = Cache[S]) {
-    case Yes:
-      return true;
-    case No:
-      return false;
-    case None:
-      Result = Globs.contains(S) ? Yes : No;
-      return Result == Yes;
-    }
-    llvm_unreachable("invalid enum");
-  }
-
-private:
-  GlobList Globs;
-  enum Tristate { None, Yes, No };
-  llvm::StringMap<Tristate> Cache;
-};
-
 ClangTidyContext::ClangTidyContext(
     std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider,
     bool AllowEnablingAnalyzerAlphaCheckers)

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h

@@ -29,6 +29,7 @@ class CompilationDatabase;
 } // namespace tooling
 
 namespace tidy {
+class CachedGlobList;
 
 /// A detected error complete with information to display diagnostic and
 /// automatic fix.
@@ -191,7 +192,7 @@ class ClangTidyContext {
 
   std::string CurrentFile;
   ClangTidyOptions CurrentOptions;
-  class CachedGlobList;
+
   std::unique_ptr<CachedGlobList> CheckFilter;
   std::unique_ptr<CachedGlobList> WarningAsErrorFilter;
 

clang-tools-extra/clang-tidy/GlobList.cpp

@@ -9,8 +9,8 @@
 #include "GlobList.h"
 #include "llvm/ADT/SmallString.h"
 
-using namespace clang;
-using namespace tidy;
+namespace clang {
+namespace tidy {
 
 // Returns true if GlobList starts with the negative indicator ('-'), removes it
 // from the GlobList.
@@ -62,3 +62,19 @@ bool GlobList::contains(StringRef S) const {
   }
   return false;
 }
+
+bool CachedGlobList::contains(StringRef S) const {
+  switch (auto &Result = Cache[S]) {
+  case Yes:
+    return true;
+  case No:
+    return false;
+  case None:
+    Result = GlobList::contains(S) ? Yes : No;
+    return Result == Yes;
+  }
+  llvm_unreachable("invalid enum");
+}
+
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/GlobList.h

@@ -11,6 +11,7 @@
 
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Regex.h"
 
@@ -24,6 +25,8 @@ namespace tidy {
 /// them in the order of appearance in the list.
 class GlobList {
 public:
+  virtual ~GlobList() = default;
+
   /// \p Globs is a comma-separated list of globs (only the '*' metacharacter is
   /// supported) with an optional '-' prefix to denote exclusion.
   ///
@@ -36,18 +39,31 @@ class GlobList {
 
   /// Returns \c true if the pattern matches \p S. The result is the last
   /// matching glob's Positive flag.
-  bool contains(StringRef S) const;
+  virtual bool contains(StringRef S) const;
 
 private:
-
   struct GlobListItem {
     bool IsPositive;
     llvm::Regex Regex;
   };
   SmallVector<GlobListItem, 0> Items;
 };
 
-} // end namespace tidy
-} // end namespace clang
+/// A \p GlobList that caches search results, so that search is performed only
+/// once for the same query.
+class CachedGlobList final : public GlobList {
+public:
+  using GlobList::GlobList;
+
+  /// \see GlobList::contains
+  bool contains(StringRef S) const override;
+
+private:
+  enum Tristate { None, Yes, No };
+  mutable llvm::StringMap<Tristate> Cache;
+};
+
+} // namespace tidy
+} // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GLOBLIST_H

clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp

@@ -4,15 +4,20 @@
 namespace clang {
 namespace tidy {
 
-TEST(GlobList, Empty) {
-  GlobList Filter("");
+template <typename GlobListT> struct GlobListTest : public ::testing::Test {};
+
+using GlobListTypes = ::testing::Types<GlobList, CachedGlobList>;
+TYPED_TEST_SUITE(GlobListTest, GlobListTypes);
+
+TYPED_TEST(GlobListTest, Empty) {
+  TypeParam Filter("");
 
   EXPECT_TRUE(Filter.contains(""));
   EXPECT_FALSE(Filter.contains("aaa"));
 }
 
-TEST(GlobList, Nothing) {
-  GlobList Filter("-*");
+TYPED_TEST(GlobListTest, Nothing) {
+  TypeParam Filter("-*");
 
   EXPECT_FALSE(Filter.contains(""));
   EXPECT_FALSE(Filter.contains("a"));
@@ -21,8 +26,8 @@ TEST(GlobList, Nothing) {
   EXPECT_FALSE(Filter.contains("*"));
 }
 
-TEST(GlobList, Everything) {
-  GlobList Filter("*");
+TYPED_TEST(GlobListTest, Everything) {
+  TypeParam Filter("*");
 
   EXPECT_TRUE(Filter.contains(""));
   EXPECT_TRUE(Filter.contains("aaaa"));
@@ -31,8 +36,8 @@ TEST(GlobList, Everything) {
   EXPECT_TRUE(Filter.contains("*"));
 }
 
-TEST(GlobList, OneSimplePattern) {
-  GlobList Filter("aaa");
+TYPED_TEST(GlobListTest, OneSimplePattern) {
+  TypeParam Filter("aaa");
 
   EXPECT_TRUE(Filter.contains("aaa"));
   EXPECT_FALSE(Filter.contains(""));
@@ -41,8 +46,8 @@ TEST(GlobList, OneSimplePattern) {
   EXPECT_FALSE(Filter.contains("bbb"));
 }
 
-TEST(GlobList, TwoSimplePatterns) {
-  GlobList Filter("aaa,bbb");
+TYPED_TEST(GlobListTest, TwoSimplePatterns) {
+  TypeParam Filter("aaa,bbb");
 
   EXPECT_TRUE(Filter.contains("aaa"));
   EXPECT_TRUE(Filter.contains("bbb"));
@@ -52,11 +57,11 @@ TEST(GlobList, TwoSimplePatterns) {
   EXPECT_FALSE(Filter.contains("bbbb"));
 }
 
-TEST(GlobList, PatternPriority) {
+TYPED_TEST(GlobListTest, PatternPriority) {
   // The last glob that matches the string decides whether that string is
   // included or excluded.
   {
-    GlobList Filter("a*,-aaa");
+    TypeParam Filter("a*,-aaa");
 
     EXPECT_FALSE(Filter.contains(""));
     EXPECT_TRUE(Filter.contains("a"));
@@ -65,7 +70,7 @@ TEST(GlobList, PatternPriority) {
     EXPECT_TRUE(Filter.contains("aaaa"));
   }
   {
-    GlobList Filter("-aaa,a*");
+    TypeParam Filter("-aaa,a*");
 
     EXPECT_FALSE(Filter.contains(""));
     EXPECT_TRUE(Filter.contains("a"));
@@ -75,15 +80,16 @@ TEST(GlobList, PatternPriority) {
   }
 }
 
-TEST(GlobList, WhitespacesAtBegin) {
-  GlobList Filter("-*,   a.b.*");
+TYPED_TEST(GlobListTest, WhitespacesAtBegin) {
+  TypeParam Filter("-*,   a.b.*");
 
   EXPECT_TRUE(Filter.contains("a.b.c"));
   EXPECT_FALSE(Filter.contains("b.c"));
 }
 
-TEST(GlobList, Complex) {
-  GlobList Filter("*,-a.*, -b.*, \r  \n  a.1.* ,-a.1.A.*,-..,-...,-..+,-*$, -*qwe* ");
+TYPED_TEST(GlobListTest, Complex) {
+  TypeParam Filter(
+      "*,-a.*, -b.*, \r  \n  a.1.* ,-a.1.A.*,-..,-...,-..+,-*$, -*qwe* ");
 
   EXPECT_TRUE(Filter.contains("aaa"));
   EXPECT_TRUE(Filter.contains("qqq"));