feat: total fnmatch with proven glob semantics - #46
Open
tskovlund wants to merge 2 commits into
Open
Conversation
Eight cases covering what the existing tests left open: repeated-star backtracking, trailing wildcards after a star, `*` crossing `/` while `?` refuses it, brackets having no separator rule, unterminated brackets, the `[]` and `[!]` edge classes, and empty pattern/name. All pass against the current backtracking matcher, so they characterize today's behaviour rather than a future implementation's. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the two-pointer backtracking matcher with structural recursion on `pattern.length + name.length`. The bracket case is justified by `scanBracket_shrinks`: a bracket expression always consumes at least its closing `]`, so what remains is strictly shorter. `Qed/Ignore.lean` now has no `partial`, and a spec criterion keeps it that way. `matchBracket` is restructured from a `let rec` closure into a top-level `scanBracket` taking its state explicitly, so the shrink lemma can be stated and proven by functional induction. Behaviour is unchanged. Before swapping, the new tests were written against the old implementation and pass there; the two were then compared exhaustively over ~2.3M pattern/name pairs — every pattern up to length 4 over `a b * ? / [ ] ! -` against every name up to length 4 over `a b /`, every pattern up to length 5 over the glob metacharacters, and every star-heavy pattern up to length 7 — with zero differences. Totality buys the semantics, proven rather than documented: - `matchGlob_star_iff_suffix` — `*` matches iff some suffix of the name matches the rest of the pattern - `star_matches_everything` — a bare `*` matches every name, separators included, so `*` ignores the whole tree - `question_matches_one` — `?` matches exactly one character, never `/` - `literal_matches_iff` — a wildcard-free pattern matches its own text and nothing else - `literal_star_matches_prefix` — a literal prefix plus `*` matches every name carrying that prefix The asymmetry the proofs pin down is deliberate and was already the behaviour: `*` crosses `/`, `?` does not. New `specs/ignore.spec.toml` owns the subsystem — seven proof criteria, the no-`partial` structural assertion, and an agent review. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Qed/Ignore.leandecides which specs get verified at all, so a wrong answer silently drops verification coverage rather than failing loudly. Until now its matcher waspartial: star backtracking reset the pattern pointer while advancing the string pointer, so it was not structurally decreasing, and apartial defis opaque to the kernel with no equational lemmas. Nothing about glob matching could be stated, let alone proven — the precedence proofs onmainonly work because they treatfnmatchas an abstract predicate.This replaces the matcher with a structurally terminating one and proves the semantics.
Based on
feat/lockfile-roundtrip-proof(#40), notmain. Review #40 first; this PR's diff against it is the three files below plus docs.The matcher
matchGlobrecurses onpattern.length + name.length, which strictly decreases in every branch. The bracket case is the only one that needs an argument:scanBracket_shrinksproves a bracket expression always consumes at least its closing], so the pattern remaining after it is strictly shorter.To state that lemma,
matchBracketis restructured from alet recclosure capturingnegateandcharacterinto a top-levelscanBracketthat takes its state explicitly. The lemma then falls out by functional induction.Qed/Ignore.leannow contains nopartial, andspecs/ignore.spec.tomlhas a structural criterion that fails the build if one reappears.Behaviour is unchanged, and that was checked before the swap
The eight new cases in
Tests/Ignore.leanwere written and committed against the old implementation (1ae6ead), where they pass — they characterize today's behaviour, not the new code's. They cover repeated-star backtracking, trailing wildcards after a star, separator handling, unterminated brackets, the[]and[!]edge classes, and empty pattern/name.On top of that, old and new were compared exhaustively during development:
a b * ? / [ ] ! -, names ≤ 4 overa b /a * ? / [ ], names ≤ 5 overa /a b *, names ≤ 7 overa bThe old implementation is deleted outright — no compatibility shim, no equivalence theorem against dead code.
What is now proven
matchGlob_star_iff_suffix—*semantics: a leading*matches exactly when some suffix of the name matches the rest of the patternstar_matches_everything— a bare*matches every name, path separators included, so*in a.qedignoreignores the whole tree rather than just its top levelquestion_matches_one—?semantics: exactly one character, and never/literal_matches_iff— a pattern with no wildcards is an exact-match test: it matches its own text and nothing else, soarchivecannot accidentally ignorearchived-specsliteral_star_matches_prefix— a literal prefix followed by*matches every name carrying that prefixOne thing worth flagging for review, because it is now pinned by proof:
*crosses/but?does not. That asymmetry is pre-existing behaviour, not something this PR introduces — I verified it against the old matcher before writing the theorems. If it is wrong, it is a deliberate separate decision to make, and the proofs will now force the conversation instead of leaving it implicit.Dogfooding
New
specs/ignore.spec.tomlowns the subsystem: seven proof criteria, the no-partialstructural assertion, and an agent review covering pattern-form coverage and the honesty of the termination measure. It brings qed's self-verification to 10 specs.Test plan
lake build— clean, no warningslake test— all 165 tests passed (157 + the 8 new characterization cases)devbox run check— build, tests, and both docgen freshness diffs cleanqed verify --auto— all 10 specs passednpx prettier@3 --check .— cleanqed lockregenerated for the new speclake build+qed verify --auto --pin) passed;--pinchanged no hashesNo
partialand nosorryinQed/Ignore.lean; nonative_decideanywhere in the repo.GitHub Actions reports no checks on this PR —
.github/workflows/ci.ymltriggers only on pull requests targetingmain, and this one targetsfeat/lockfile-roundtrip-proof. CI will run once #40 merges and this retargets. The local run above is the same gate CI executes.🤖 Generated with Claude Code