fix(jinja): exhaustively handle CallArg variants in used-variable collection - #2505
Merged
Conversation
wolfv
force-pushed
the
fix/jinja-callarg-exhaustive
branch
2 times, most recently
from
May 20, 2026 11:54
aa9fb11 to
e826be4
Compare
… variables `collect_variables_from_call_arg` previously matched only `CallArg::Pos` with a wildcard fallback, silently dropping any variables passed via `Kwarg`, `PosSplat`, or `KwargSplat`. The same Pos-only filter was duplicated inside the `pin_subpackage`, `pin_compatible`, and `match` arms of `collect_variables_from_call`, so e.g. `pin_subpackage(name=foo)` or `my_fn(**kwargs)` would not track `foo` / `kwargs` for variant detection. Introduce a small `call_arg_expr` helper that exhaustively destructures every `CallArg` variant into its inner `Expr`. No wildcard arm: a future minijinja release that adds a new variant will fail to compile here instead of quietly losing variables — same property we just relied on for the `Expr::Compare` bump. Regression tests added: - chained-comparison form (`lo <= n < hi`) — guards Expr::Compare - `pin_subpackage(name=somevar)` — kwarg form on a special function - `my_fn(foo, bar=baz, *args, **kwargs)` — generic call coverage of all four CallArg variants Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
wolfv
force-pushed
the
fix/jinja-callarg-exhaustive
branch
from
May 20, 2026 12:53
e826be4 to
e11ae1a
Compare
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
While fixing the minijinja 2.20 build break (PR #2427,
Expr::Compare), I audited the rest of the AST walker inrattler_build_jinja/src/ast_variables.rsand found a pre-existing gap:collect_variables_from_call_argmatches onlyCallArg::Posand uses_ => {}to swallow the other three variants. The same Pos-only filter is duplicated in thepin_subpackage,pin_compatible, andmatchspecial-function arms.Result: templates using kwarg or splat call forms lose variables for variant tracking. E.g.
${{ pin_subpackage(name=somevar) }} # somevar dropped ${{ my_fn(**kwargs) }} # kwargs droppedIn conda-build idioms these are always positional, so existing recipes are unlikely to hit this — but user-written macros that pass kwargs or splats would silently lose variant detection.
This PR:
call_arg_expr— a small helper that exhaustively destructures everyCallArgvariant (Pos | Kwarg | PosSplat | KwargSplat) into its innerExpr. No wildcard arm, so a future minijinja release that adds a new variant will fail to compile here instead of silently losing variables — same property that just saved us forExpr::Compare.collect_variables_from_call_argand the three special-function arms through the helper.Expr::Compare),pin_subpackagekwarg form, and a generic call covering all fourCallArgvariants.This is stacked on top of #2427 so the new chained-comparison test compiles. Once #2427 merges to main, GitHub will auto-rebase this diff down to just the audit changes.
Test plan
cargo test -p rattler_build_jinja— 48/48 pass (up from 45)cargo check --workspaceclean🤖 Generated with Claude Code