Remove BuiltinMatcher enum

Explanation added as comments in code
Using plain `Lazy<Option<GlobMatcher>>` is just better

build/syntax_mapping.rs

@@ -41,7 +41,7 @@ impl MappingTarget {
 #[derive(Clone, Debug, DeserializeFromStr)]
 /// A single matcher.
 ///
-/// Corresponds to `syntax_mapping::BuiltinMatcher`.
+/// Codegen converts this into a `Lazy<GlobMatcher>`.
 struct Matcher(Vec<MatcherSegment>);
 /// Parse a matcher.
 ///
@@ -110,13 +110,12 @@ impl Matcher {
                 let MatcherSegment::Text(ref s) = self.0[0] else {
                     unreachable!()
                 };
-                format!(r###"BuiltinMatcher::Fixed(r#"{s}"#)"###)
+                format!(r###"Lazy::new(|| Some(build_matcher_fixed(r#"{s}"#)))"###)
             }
             // parser logic ensures that this case can only happen when there are dynamic segments
             _ => {
-                let segments_codegen = self.0.iter().map(MatcherSegment::codegen).join(", ");
-                let closure = format!("|| build_glob_string(&[{segments_codegen}])");
-                format!("BuiltinMatcher::Dynamic(Lazy::new({closure}))")
+                let segs = self.0.iter().map(MatcherSegment::codegen).join(", ");
+                format!(r###"Lazy::new(|| build_matcher_dynamic(&[{segs}]))"###)
             }
         }
     }
@@ -174,7 +173,7 @@ impl MappingList {
         let len = array_items.len();
 
         format!(
-            "static BUILTIN_MAPPINGS: [(BuiltinMatcher, MappingTarget); {len}] = [\n{items}\n];",
+            "static BUILTIN_MAPPINGS: [(Lazy<Option<GlobMatcher>>, MappingTarget); {len}] = [\n{items}\n];",
             items = array_items.join(",\n")
         )
     }

src/syntax_mapping.rs

@@ -15,38 +15,60 @@ include!(concat!(
     "/codegen_static_syntax_mappings.rs"
 ));
 
-/// A glob matcher generated from analysing the matcher string at compile time.
+// The defined matcher strings are analysed at compile time and converted into
+// lazily-compiled `GlobMatcher`s. This is so that the string searches are moved
+// from run time to compile time, thus improving startup performance.
+//
+// To any future maintainer (including possibly myself) wondering why there is
+// not a `BuiltinMatcher` enum that looks like this:
+//
+// ```
+// enum BuiltinMatcher {
+//     Fixed(&'static str),
+//     Dynamic(Lazy<Option<String>>),
+// }
+// ```
+//
+// Because there was. I tried it and threw it out.
+//
+// Naively looking at the problem from a distance, this may seem like a good
+// design (strongly typed etc. etc.). It would also save on compiled size by
+// extracting out common behaviour into functions. But while actually
+// implementing the lazy matcher compilation logic, I realised that it's most
+// convenient for `BUILTIN_MAPPINGS` to have the following type:
+//
+// `[(Lazy<Option<GlobMatcher>>, MappingTarget); N]`
+//
+// The benefit for this is that operations like listing all builtin mappings
+// would be effectively memoised. The caller would not have to compile another
+// `GlobMatcher` for rules that they have previously visited.
+//
+// Unfortunately, this means we are going to have to store a distinct closure
+// for each rule anyway, which makes a `BuiltinMatcher` enum a pointless layer
+// of indirection.
+//
+// In the current implementation, the closure within each generated rule simply
+// calls either `build_matcher_fixed` or `build_matcher_dynamic`, depending on
+// whether the defined matcher contains dynamic segments or not.
+
+/// Compile a fixed glob string into a glob matcher.
 ///
-/// This is so that the string searches are moved from run time to compile time,
-/// thus improving startup performance.
-#[derive(Debug)]
-enum BuiltinMatcher {
-    /// A plaintext matcher.
-    Fixed(&'static str),
-    /// A matcher that needs dynamic environment variable replacement.
-    ///
-    /// Evaluates to `None` when any environment variable replacement fails.
-    Dynamic(Lazy<Option<String>>),
-}
-impl BuiltinMatcher {
-    /// Finalise into a glob matcher.
-    ///
-    /// Returns `None` if any environment variable replacement fails (only
-    /// possible for dynamic matchers).
-    fn to_glob_matcher(&self) -> Option<GlobMatcher> {
-        let glob_str = match self {
-            Self::Fixed(s) => *s,
-            Self::Dynamic(s) => s.as_ref()?.as_str(),
-        };
-        Some(make_glob_matcher(glob_str).expect("A builtin glob matcher failed to compile"))
-    }
+/// A failure to compile is a fatal error.
+///
+/// Used internally by `Lazy<GlobMatcher>`'s lazy evaluation closure.
+fn build_matcher_fixed(from: &str) -> GlobMatcher {
+    make_glob_matcher(from).expect("A builtin fixed glob matcher failed to compile")
 }
 
 /// Join a list of matcher segments to create a glob string, replacing all
-/// environment variables. Returns `None` if any replacement fails.
+/// environment variables, then compile to a glob matcher.
 ///
-/// Used internally by `BuiltinMatcher::Dynamic`'s lazy evaluation closure.
-fn build_glob_string(segs: &[MatcherSegment]) -> Option<String> {
+/// Returns `None` if any replacement fails, or if the joined glob string fails
+/// to compile.
+///
+/// Used internally by `Lazy<GlobMatcher>`'s lazy evaluation closure.
+fn build_matcher_dynamic(segs: &[MatcherSegment]) -> Option<GlobMatcher> {
+    // join segments
     let mut buf = String::new();
     for seg in segs {
         match seg {
@@ -57,12 +79,14 @@ fn build_glob_string(segs: &[MatcherSegment]) -> Option<String> {
             }
         }
     }
-    Some(buf)
+    // compile glob matcher
+    let matcher = make_glob_matcher(&buf).ok()?;
+    Some(matcher)
 }
 
 /// A segment of a dynamic builtin matcher.
 ///
-/// Used internally by `BuiltinMatcher::Dynamic`'s lazy evaluation closure.
+/// Used internally by `Lazy<GlobMatcher>`'s lazy evaluation closure.
 #[derive(Clone, Debug)]
 enum MatcherSegment {
     Text(&'static str),