refactor(config): migrate template variables through MiniJinja's own parser

[worktrunk-bot]

Problem

Follow-up to @max-sixty's question on #4124: "Do we need our own minijinja hand-rolled scanner?"

No. src/config/deprecation.rs located the identifiers it migrates with a scanner that re-derived MiniJinja's grammar from the raw string — where a tag ends, how whitespace control is spelled, what a quoted string is, what {% raw %} makes literal, and how set/for/with target lists are shaped. Every place that second reading disagreed with the parser was visible in the file worktrunk writes: #4117 was a }} inside a string ending a tag early, and the two review rounds on #4124 added parenthesized tuple targets, + whitespace control, and tags inside {% raw %}. Each fix made the scanner longer and the next disagreement harder to rule out, because nothing structurally tied it to what MiniJinja actually does.

Solution

Enable MiniJinja's unstable_machinery and parse the template instead of scanning it.

TemplateVars walks the AST once and collects two things: the reads, which are the Expr::Var nodes — by construction the set the renderer resolves against the context — and the bound names, which are the statements' own target expressions. The rewrite then splices at the byte ranges the parser gave those nodes. Everything the scanner had to decide for itself is now the parser's answer: quoted delimiters, whitespace control, comments, {% raw %} contents, attribute names, keyword-argument names, nested tuple targets.

That deletes seventeen functions — the tag walker, the target-list reader with its TargetsEndAt modes, the quote and identifier scanners, the {% raw %} state machine — for nine short visitor methods. It also collapses detection and migration into one call (migrate_retired_vars) instead of two functions sharing a predicate, so the framework invariant is now structural rather than maintained.

The shadowing policy is unchanged: a retired or canonical name bound anywhere still drops the whole pair rather than renaming per scope. The AST would now support real scope tracking, but that is a behavior change, not a simplification, so it stays out of this PR.

What this costs

Two things worth weighing against the deletion, both stated up front:

• unstable_machinery has no semver guarantee. MiniJinja last changed it in a patch release (1.0.16 moved tokenize/parse signatures), so a version bound wouldn't reliably prevent a break. The coupling here is to the AST's shape alone and both matches are exhaustive, so the failure mode is a compile error naming the node to handle — never a silent mis-migration. CI catches it on a bump; a cargo install against a future MiniJinja would fail to build rather than misbehave.
• ~116KB of release binary (~67KB stripped), measured by building both ways. unstable_machinery pulls in internal_debug, which derives Debug across the AST and instruction types. The Cargo.toml comment now records this against the ~200KB the trimmed feature set saves.

Behavior change

One, and it is a fix: {{ repo_root }}{% raw %}{{ " {% endraw %} now migrates. The scanner had to find that tag's end itself, couldn't, and left the whole template alone; MiniJinja reads the raw block as the literal it is. test_normalize_bails_on_a_tag_it_cannot_terminate pinned the old behavior and is replaced by test_normalize_migrates_past_a_tag_opened_inside_a_raw_block plus test_normalize_leaves_an_unparsable_template_untouched, which pins what the bail-out is actually for.

Testing

Every existing test in the module passes unchanged apart from that one, including the whole test_warning_fires_iff_update_changes battery and the edge cases #4124 added. New pins for what the parser now decides:

• test_normalize_skips_identifiers_that_are_not_reads — a keyword-argument name and a map key.
• test_normalize_rewrites_reads_out_of_visit_order — the AST hands reads back test-first for a conditional and keys-before-values for a map, so two migrations in one tag must still splice coherently. Removing the sort makes this panic.
• test_normalize_rewrites_inside_a_do_statement — {% do %} is its own statement, not an emitted expression.
• test_normalize_rewrites_reads_in_every_expression_shape — one case per AST node the walk descends, since a missed visitor arm loses a migration silently rather than failing.
• test_warning_fires_iff_update_changes gains an unparseable template to untouched.

cargo test --lib --bins passes except copy::tests::test_copy_dir_recursive_does_not_count_a_non_regular_file, which fails on main in this sandbox too (socket(AF_UNIX, …) is blocked — verified by stashing the change and re-running). cargo test --test integration config is 582/582. cargo clippy --all-targets and cargo fmt --check are clean.

[worktrunk-bot]

Self-review. Three findings; the first two I'll push as a follow-up commit.

test_normalize_rewrites_reads_in_every_expression_shape doesn't reach Expr::Compare or IfExpr.false_expr, which is what codecov/patch is failing on. The test's stated contract is "one case per AST node the walk descends, since a missed visitor arm loses a migration silently rather than failing", and those two arms are the gap — codecov reports exactly six missed lines, the Compare arm and the self.expr(false_expr) line. The Compare miss is not obvious from the tests that exist: {% set a = repo_root == "x" %} looks like it covers it, but MiniJinja's parse_compare lowers a single comparison to Expr::BinOp and only emits Expr::Compare for a chained one (ops.len() >= 2). So {{ 1 < repo_root < 3 }} and {{ a if b else repo_root }} are the two cases that both meet the contract and close the check; I verified both migrate correctly against the current code.

The comment on TemplateVars::of justifying Default::default() says the whitespace default is "what the environments that render these templates use", and that isn't true. expand_template_with in src/config/expansion.rs calls env.set_keep_trailing_newline(true) for every ShellEscapeMode other than Literal, so the parse here and the render disagree. The disagreement is harmless, but for a reason worth stating instead: WhitespaceConfig only shapes literal text — keep_trailing_newline decides whether the tokenizer drops a trailing \n before lexing, trim_blocks/lstrip_blocks only move the boundaries of EmitRaw — so it can never move an Expr::Var span, and the rewrite copies everything outside those spans from the original string. As written the comment invites a reader to "fix" the mismatch by threading the render environment's config through, which would add a coupling the rewrite doesn't need.

The cargo install exposure is larger than the PR body allows, and the stated reason for accepting it doesn't hold. The body argues "a version bound wouldn't reliably prevent a break" because MiniJinja last moved this API in a patch release. That's true of the caret bound minijinja = "2.20" carries today, but not of an upper bound: ">=2.20, <2.25" pins out exactly the release that could break. It matters because cargo install worktrunk — the install path in README.md, docs/src/content/docs/worktrunk.md, and the release-notes template in .github/workflows/release.yaml — ignores Cargo.lock and resolves the newest matching 2.x, so a future machinery change breaks source installs of every already-published release, not just main. Nothing on our side would see it first: minimal-versions in the nightly workflow resolves minimum versions, and every other job builds from the lock. Against that, no dependency in Cargo.toml carries an upper bound today, and one would need bumping on each MiniJinja release — so this is a maintainer call on whether the ongoing pin maintenance is worth removing the failure mode, not something I should decide in a refactor PR.

The rest holds up. I checked the two matches against the 2.24.0 AST: Stmt and Expr are exhaustive over the variants this feature set compiles, and CallArg is too, so a reshaped node is a compile error rather than a dropped migration. Span offsets are byte offsets (Tokenizer::advance accumulates current_offset in bytes), which I confirmed by running the splice over templates with multi-byte text on both sides of a tag.