baml_language: prototype class destructure/pattern refactor#3423
baml_language: prototype class destructure/pattern refactor#3423codeshaunted wants to merge 1 commit intocanaryfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR refactors pattern syntax and representation across the BAML compiler. Match and catch arms now require Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Binary size checks passed✅ 7 passed
Generated by |
Merging this PR will improve performance by 27.13%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | WallTime | bench_scale_deep_nesting |
33.6 ms | 30.5 ms | +10.14% |
| ⚡ | WallTime | bench_incremental_modify_function |
12.7 ms | 11.5 ms | +10.9% |
| ⚡ | WallTime | bench_lexer_only_simple |
21.2 µs | 16.7 µs | +26.94% |
| ⚡ | WallTime | bench_incremental_close_string |
30 ms | 26.6 ms | +13.08% |
| ⚡ | WallTime | bench_incremental_add_field |
12.7 ms | 11.5 ms | +10.62% |
| ⚡ | WallTime | bench_parse_only_simple |
37.9 µs | 29.8 µs | +27.13% |
| ⚡ | WallTime | bench_incremental_add_attribute |
29.2 ms | 26.5 ms | +10.07% |
| ⚡ | WallTime | bench_empty_project |
19.2 ms | 16.9 ms | +13.32% |
| ⚡ | WallTime | bench_scale_100_functions |
134 ms | 115.6 ms | +15.92% |
Comparing avery/pattern-proto (869a32f) with canary (36ac995)
Footnotes
-
105 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/match/match_scrutinee_narrowing.baml (1)
20-21: Consider using consistent variable references across both match arms.Line 20 binds the pattern to
obut usesr.value(the scrutinee), while line 21 binds toeand usese.message(the binding). If the test intends to demonstrate that both the bound name and the narrowed scrutinee can be used interchangeably, consider adding a clarifying comment. Otherwise, using consistent references (either both use the binding or both use the scrutinee) would make the test's intent clearer.♻️ Option 1: Use bindings consistently
- let o: Ok => r.value, + let o: Ok => o.value, let e: Err => e.message♻️ Option 2: Use scrutinee consistently
let o: Ok => r.value, - let e: Err => e.message + let e: Err => r.message🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/match/match_scrutinee_narrowing.baml` around lines 20 - 21, The two match arms use inconsistent references: the first arm binds as "o" but accesses "r.value" (the scrutinee), while the second binds "e" and uses the binding "e.message"; make them consistent by either (A) using the binding names in both arms (change "r.value" to "o.value") or (B) using the scrutinee in both arms (change "e.message" to "r.message"), or alternatively add a one-line comment clarifying the test is demonstrating both forms; update the patterns/expressions around the Ok/Err arms (symbols: o, r, e, r.value, e.message) accordingly.baml_language/crates/baml_compiler2_ast/src/lower_expr_body.rs (1)
1082-1300: Add focused unit coverage for chained and union-pattern lowering.This path is doing a lot of semantic reconstruction now; a few Rust unit tests for
let x: A: B,let x: int | string, and typed destructures would catch regressions here much earlier than snapshots. Please also runcargo test --libon the final patch. As per coding guidelines, "Prefer writing Rust unit tests over integration tests where possible" and "Always runcargo test --libif you changed any Rust code".Also applies to: 2448-2462
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baml_language/crates/baml_compiler2_ast/src/lower_expr_body.rs` around lines 1082 - 1300, Add focused unit tests covering chained colon patterns, union-patterns and typed destructures by exercising lower_match_pattern, lower_pattern_position and type_expr_to_pattern: write Rust unit tests that construct CST nodes (or helper builders) for cases like "let x: A: B" (chained positions), "let x: int | string" (union in TYPE_EXPR -> or-pattern), and a typed destructure (TYPE_EXPR + PATTERN_FIELD) asserting the resulting PatId shapes (bindings, class patterns, wildcard/chain/or as appropriate). Place tests adjacent to the lowering code (e.g., in the same crate tests module) and run cargo test --lib to verify; update/add assertions to catch regressions in PatternPosition composition and type_expr_to_pattern handling (including "_" -> wildcard). Ensure tests cover both pos.has_let branches and pos.fields branches so lower_pattern_position behavior is validated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@baml_language/crates/baml_compiler_parser/src/parser.rs`:
- Around line 3364-3367: looks_like_for_in_loop currently only recognizes "let
WORD in" so typed bindings and destructures like "let x: int in" or "let User {
name } in" are misclassified; change looks_like_for_in_loop to perform a
non-consuming top-level pattern lookahead (use a snapshot/clone of the token
stream or a peek-only helper) that runs the same logic as parse_pattern for the
pattern head and confirms the next top-level token is `in` before deciding;
update or reuse parse_for_in_pattern/parse_pattern semantics in the lookahead to
handle colons and braces, then add two Rust unit tests in the parser tests
(prefer unit tests inside the parser crate) verifying parsing of "for (let x:
int in xs) { }" and "for (let User { name } in xs) { }" so both typed-binding
and destructure forms are accepted as for-in loops.
In `@baml_language/crates/baml_compiler2_ast/src/lower_expr_body.rs`:
- Around line 1281-1299: type_expr_to_pattern currently expands a
TypeExpr::Union into Pattern::or of its variant patterns which loses the
original union type annotation; instead preserve the annotation by allocating a
type-match pattern for the union itself (i.e. return
self.alloc_pattern(Pattern::type_match(ty), span) when encountering a
TypeExpr::Union) or, if you must build an Or, ensure each part is a type-match
of the corresponding variant and attach the original union type to the resulting
pattern; update the TypeExpr::Union branch in type_expr_to_pattern (and the
similar code at the other location referenced) to return a type_match for the
union to avoid dropping annotations.
- Around line 1183-1218: The loop currently attaches a chain pattern via
self.patterns[tip].chain = Some(chain_pat) but never advances tip, so
intermediate chain nodes are lost (e.g., let x: A: B becomes let x: B); after
creating/assigning the chain node returned by type_expr_to_pattern (chain_pat)
update tip to that new pattern index (tip = chain_pat). Reference: the for
positions loop, variable tip, assignments to self.patterns[tip].chain, the call
to type_expr_to_pattern, and existing alloc_pattern/destr/binding branches that
already set tip — mirror those by assigning tip to the newly attached chain
node.
In `@baml_language/crates/baml_compiler2_tir/src/builder.rs`:
- Around line 2442-2449: The current report uses base_expr_id which points at
the caught expression; change the span passed to self.context.report_simple so
the InvalidCatchBindingType error is anchored to the offending chained pattern
instead—use the chained pattern's id (chain_id) or its span from
body.patterns[chain_id] (where PatternKind::Type(ty) is matched) when calling
self.context.report_simple for TirTypeError::InvalidCatchBindingType rather than
base_expr_id.
In `@baml_language/crates/baml_compiler2_tir/src/lower_type_expr.rs`:
- Around line 175-219: The fallback treats the whole dotted enum path as a
single short name (enum_item_name) causing lookups and qualify_def to be wrong;
change the logic to split segments into enum_ns (all but last segment) and
enum_item (last short name) like earlier in the file, then call
package_items.lookup_type(&enum_ns_with_context, &enum_item) (and in the
root/pkg branches use package_items.lookup_type(&enum_name[1..], &enum_item) or
pkg.lookup_type(&enum_name[1..], &enum_item)), and pass &enum_item to
qualify_def(db, def, &enum_item) instead of &enum_item_name; update the branches
that build the namespace vector (using ns_context when present) and add unit
tests for ns.Enum.Variant, root.ns.Enum.Variant, and pkg.ns.Enum.Variant, then
run cargo test --lib.
In `@baml_language/crates/baml_fmt/src/ast/pattern.rs`:
- Around line 61-73: The formatter AST currently loses struct/record destructure
tails because FmtPattern / ChainedPosition only model WORD | TYPE_EXPR plus
colon links; extend the pattern representation to preserve destructures by
adding a Destructure (or PatternFields) variant to PatternPosition (or an
additional field on ChainedPosition) that stores the brace tokens and a Vec of
PATTERN_FIELD entries (including commas and optional `let`/nested patterns),
update any places that build FmtPattern/ChainedPosition to populate this new
variant, and update the printing/formatting code that consumes FmtPattern and
PatternPosition to render the `{ ... }` tail (fields, commas, nested patterns)
when the Destructure variant is present so patterns like `User { name, age: let
a }` are preserved instead of being dropped.
---
Nitpick comments:
In `@baml_language/crates/baml_compiler2_ast/src/lower_expr_body.rs`:
- Around line 1082-1300: Add focused unit tests covering chained colon patterns,
union-patterns and typed destructures by exercising lower_match_pattern,
lower_pattern_position and type_expr_to_pattern: write Rust unit tests that
construct CST nodes (or helper builders) for cases like "let x: A: B" (chained
positions), "let x: int | string" (union in TYPE_EXPR -> or-pattern), and a
typed destructure (TYPE_EXPR + PATTERN_FIELD) asserting the resulting PatId
shapes (bindings, class patterns, wildcard/chain/or as appropriate). Place tests
adjacent to the lowering code (e.g., in the same crate tests module) and run
cargo test --lib to verify; update/add assertions to catch regressions in
PatternPosition composition and type_expr_to_pattern handling (including "_" ->
wildcard). Ensure tests cover both pos.has_let branches and pos.fields branches
so lower_pattern_position behavior is validated.
In
`@baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/match/match_scrutinee_narrowing.baml`:
- Around line 20-21: The two match arms use inconsistent references: the first
arm binds as "o" but accesses "r.value" (the scrutinee), while the second binds
"e" and uses the binding "e.message"; make them consistent by either (A) using
the binding names in both arms (change "r.value" to "o.value") or (B) using the
scrutinee in both arms (change "e.message" to "r.message"), or alternatively add
a one-line comment clarifying the test is demonstrating both forms; update the
patterns/expressions around the Ok/Err arms (symbols: o, r, e, r.value,
e.message) accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 72ef82c6-9759-43a3-93dd-d5192ba81a31
⛔ Files ignored due to path filters (167)
baml_language/crates/baml_tests/snapshots/__baml_std__/baml_tests____baml_std____03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/__baml_std__/baml_tests____baml_std____04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/__testing_std__/baml_tests____testing_std____03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/__testing_std__/baml_tests____testing_std____04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/__testing_std__/baml_tests____testing_std____04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/__testing_std__/baml_tests____testing_std____06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/banned_expressions/baml_tests__banned_expressions__02_parser__banned_expressions.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/banned_expressions/baml_tests__banned_expressions__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/basic_types/baml_tests__basic_types__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/builtin_io/baml_tests__builtin_io__02_parser__fs_ops.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/builtin_io/baml_tests__builtin_io__02_parser__net_ops.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/builtin_io/baml_tests__builtin_io__02_parser__shell_ops.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/builtin_io/baml_tests__builtin_io__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/byte_string_literals/baml_tests__byte_string_literals__02_parser__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/captured_field_chain/baml_tests__captured_field_chain__02_parser__captured_field_chain.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/captured_field_chain/baml_tests__captured_field_chain__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/catch_all_keyword/baml_tests__catch_all_keyword__02_parser__catch_all_keyword.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/catch_return_type_mismatch/baml_tests__catch_return_type_mismatch__02_parser__catch_return_type_mismatch.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/catch_return_type_mismatch/baml_tests__catch_return_type_mismatch__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/catch_return_type_mismatch/baml_tests__catch_return_type_mismatch__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/catch_throw/baml_tests__catch_throw__02_parser__catch_throw.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/catch_throw_regressions/baml_tests__catch_throw_regressions__02_parser__regressions.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/catch_throw_regressions/baml_tests__catch_throw_regressions__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/closure_errors/baml_tests__closure_errors__02_parser__closure_errors.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/closure_errors/baml_tests__closure_errors__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/closure_errors/baml_tests__closure_errors__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/closure_errors/baml_tests__closure_errors__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/closure_loop_variable/baml_tests__closure_loop_variable__02_parser__demo.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/closure_loop_variable/baml_tests__closure_loop_variable__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/closure_loop_variable/baml_tests__closure_loop_variable__10_formatter__demo.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/closures/baml_tests__closures__02_parser__closures.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/closures/baml_tests__closures__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/control_flow/baml_tests__control_flow__02_parser__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/control_flow/baml_tests__control_flow__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/deep_method_call/baml_tests__deep_method_call__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/error_cases/baml_tests__error_cases__02_parser__syntax_errors.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/error_cases/baml_tests__error_cases__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/error_cases/baml_tests__error_cases__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/error_cases/baml_tests__error_cases__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/error_cases/baml_tests__error_cases__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/error_cases/baml_tests__error_cases__10_formatter__syntax_errors.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/event_system/baml_tests__event_system__02_parser__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/format_checks/baml_tests__format_checks__01_lexer__match_exprs.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/format_checks/baml_tests__format_checks__02_parser__binary_expr.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/format_checks/baml_tests__format_checks__02_parser__function_decls.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/format_checks/baml_tests__format_checks__02_parser__loop_stmts.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/format_checks/baml_tests__format_checks__02_parser__match_exprs.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/format_checks/baml_tests__format_checks__02_parser__other_exprs.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/format_checks/baml_tests__format_checks__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/format_checks/baml_tests__format_checks__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/format_checks/baml_tests__format_checks__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/format_checks/baml_tests__format_checks__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/format_checks/baml_tests__format_checks__10_formatter__function_decls.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/format_checks/baml_tests__format_checks__10_formatter__loop_stmts.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/format_checks/baml_tests__format_checks__10_formatter__match_exprs.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_call/baml_tests__function_call__02_parser__builtin_call.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_call/baml_tests__function_call__02_parser__function_call.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_call/baml_tests__function_call__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__negative.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__positive.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_types/baml_tests__function_types__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/generic_field_chain/baml_tests__generic_field_chain__02_parser__generic_field_chain.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/generic_field_chain/baml_tests__generic_field_chain__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/generics/baml_tests__generics__01_lexer__classes.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/generics/baml_tests__generics__02_parser__classes.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/generics/baml_tests__generics__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/headers_edge_cases/baml_tests__headers_edge_cases__02_parser__edge_cases.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/headers_edge_cases/baml_tests__headers_edge_cases__10_formatter__edge_cases.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/lambda_advanced/baml_tests__lambda_advanced__02_parser__lambda_advanced.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/lambda_advanced/baml_tests__lambda_advanced__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/lambda_advanced/baml_tests__lambda_advanced__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/lambda_advanced/baml_tests__lambda_advanced__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/lambda_advanced/baml_tests__lambda_advanced__10_formatter__lambda_advanced.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/lambda_basic/baml_tests__lambda_basic__02_parser__lambda_basic.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/lambda_basic/baml_tests__lambda_basic__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/lambda_basic/baml_tests__lambda_basic__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/lambda_basic/baml_tests__lambda_basic__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/lambda_errors/baml_tests__lambda_errors__02_parser__lambda_errors.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/lambda_errors/baml_tests__lambda_errors__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/lambda_errors/baml_tests__lambda_errors__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/lambda_errors/baml_tests__lambda_errors__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/lambda_fat_arrow/baml_tests__lambda_fat_arrow__02_parser__lambda_fat_arrow.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/lambda_fat_arrow/baml_tests__lambda_fat_arrow__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/lambda_field_access/baml_tests__lambda_field_access__02_parser__lambda_field_access.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/lambda_field_access/baml_tests__lambda_field_access__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/literal_union_arithmetic/baml_tests__literal_union_arithmetic__02_parser__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/literal_union_widening/baml_tests__literal_union_widening__02_parser__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/literal_union_widening/baml_tests__literal_union_widening__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/literal_union_widening/baml_tests__literal_union_widening__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/match_exhaustiveness/baml_tests__match_exhaustiveness__01_lexer__match_exhaustiveness.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/match_exhaustiveness/baml_tests__match_exhaustiveness__02_parser__match_exhaustiveness.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/match_exhaustiveness/baml_tests__match_exhaustiveness__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/match_exhaustiveness/baml_tests__match_exhaustiveness__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/match_exhaustiveness/baml_tests__match_exhaustiveness__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/match_exhaustiveness/baml_tests__match_exhaustiveness__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/match_exhaustiveness/baml_tests__match_exhaustiveness__10_formatter__match_exhaustiveness.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/namespaces_bare_name_rejected/baml_tests__namespaces_bare_name_rejected__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/namespaces_type_resolution/baml_tests__namespaces_type_resolution__02_parser__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/namespaces_type_resolution/baml_tests__namespaces_type_resolution__02_parser__ns_auth_auth.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/namespaces_type_resolution/baml_tests__namespaces_type_resolution__02_parser__ns_llm_models.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/namespaces_type_resolution/baml_tests__namespaces_type_resolution__02_parser__ns_llm_ns_openai_client.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/namespaces_type_resolution/baml_tests__namespaces_type_resolution__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/namespaces_type_resolution/baml_tests__namespaces_type_resolution__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/namespaces_type_resolution/baml_tests__namespaces_type_resolution__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/namespaces_type_resolution/baml_tests__namespaces_type_resolution__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/namespaces_type_resolution/baml_tests__namespaces_type_resolution__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/null_handling/baml_tests__null_handling__02_parser__null_handling.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/null_handling/baml_tests__null_handling__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/null_handling/baml_tests__null_handling__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/null_handling/baml_tests__null_handling__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/parser_error_recovery/baml_tests__parser_error_recovery__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/parser_expressions/baml_tests__parser_expressions__02_parser__binary_ops.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/parser_expressions/baml_tests__parser_expressions__02_parser__field_access.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/parser_expressions/baml_tests__parser_expressions__02_parser__index_access.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/parser_expressions/baml_tests__parser_expressions__02_parser__precedence.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/parser_speculative/baml_tests__parser_speculative__02_parser__expr_function.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/parser_speculative/baml_tests__parser_speculative__02_parser__mixed_functions.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/parser_speculative/baml_tests__parser_speculative__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/parser_statements/baml_tests__parser_statements__02_parser__assignments.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/parser_statements/baml_tests__parser_statements__02_parser__break_continue.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/parser_statements/baml_tests__parser_statements__02_parser__for_loops.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/parser_statements/baml_tests__parser_statements__02_parser__let.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/parser_statements/baml_tests__parser_statements__02_parser__while_loop.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/parser_statements/baml_tests__parser_statements__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/parser_statements/baml_tests__parser_statements__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/parser_statements/baml_tests__parser_statements__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/parser_statements/baml_tests__parser_statements__10_formatter__for_loops.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/parser_strings/baml_tests__parser_strings__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/pending_greaters_fix/baml_tests__pending_greaters_fix__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/simple_function/baml_tests__simple_function__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/test_expr_basic/baml_tests__test_expr_basic__02_parser__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/test_expr_basic/baml_tests__test_expr_basic__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/test_expr_name_concat/baml_tests__test_expr_name_concat__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/test_expr_throwing_body/baml_tests__test_expr_throwing_body__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/test_expr_with_runner/baml_tests__test_expr_with_runner__02_parser__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/test_expr_with_runner/baml_tests__test_expr_with_runner__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/test_invalid_contexts/baml_tests__test_invalid_contexts__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/test_old_and_new/baml_tests__test_old_and_new__02_parser__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/test_old_and_new/baml_tests__test_old_and_new__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/test_raw_string_name/baml_tests__test_raw_string_name__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/test_with_not_keyword/baml_tests__test_with_not_keyword__02_parser__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/test_with_not_keyword/baml_tests__test_with_not_keyword__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/test_with_runner_ambiguity/baml_tests__test_with_runner_ambiguity__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/testset_basic/baml_tests__testset_basic__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/testset_dynamic/baml_tests__testset_dynamic__02_parser__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/testset_dynamic/baml_tests__testset_dynamic__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/testset_nested/baml_tests__testset_nested__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/testset_vibes_nested/baml_tests__testset_vibes_nested__02_parser__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/testset_vibes_nested/baml_tests__testset_vibes_nested__10_formatter__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/testset_with_setup/baml_tests__testset_with_setup__02_parser__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/testset_with_setup/baml_tests__testset_with_setup__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/top_level_let/baml_tests__top_level_let__02_parser__test.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/type_aliases/baml_tests__type_aliases__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/type_annotation/baml_tests__type_annotation__02_parser__type_annotation.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/type_annotation/baml_tests__type_annotation__02_parser__type_positions.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/type_annotation/baml_tests__type_annotation__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/type_annotation/baml_tests__type_annotation__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/unknown_type_error/baml_tests__unknown_type_error__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/void_return_type/baml_tests__void_return_type__02_parser__void_function_type.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/void_return_type/baml_tests__void_return_type__02_parser__void_lambda.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/void_return_type/baml_tests__void_return_type__02_parser__void_return_type.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/void_return_type/baml_tests__void_return_type__02_parser__void_type_position_errors.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/void_return_type/baml_tests__void_return_type__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/void_return_type/baml_tests__void_return_type__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/void_return_type/baml_tests__void_return_type__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_textual.snapis excluded by!**/*.snap
📒 Files selected for processing (36)
baml_language/crates/baml_builtins2/baml_std/baml/ns_llm/llm_types.bamlbaml_language/crates/baml_builtins2/baml_std/testing/registry.bamlbaml_language/crates/baml_compiler2_ast/src/ast.rsbaml_language/crates/baml_compiler2_ast/src/disambiguate.rsbaml_language/crates/baml_compiler2_ast/src/lower_expr_body.rsbaml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_compiler2_tir/src/lower_type_expr.rsbaml_language/crates/baml_compiler2_visualization/src/control_flow/from_ast.rsbaml_language/crates/baml_compiler_parser/src/parser.rsbaml_language/crates/baml_compiler_syntax/src/ast.rsbaml_language/crates/baml_compiler_syntax/src/syntax_kind.rsbaml_language/crates/baml_fmt/src/ast/expressions.rsbaml_language/crates/baml_fmt/src/ast/pattern.rsbaml_language/crates/baml_fmt/src/ast/statements.rsbaml_language/crates/baml_lsp2_actions_tests/test_files/semantic_tokens/match_expr.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/catch/catch_wildcard_only.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/catch/invalid_arm_syntax.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/catch/unknown_binding_types.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/match/match_bare_type_sugar.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/match/match_exhaustive_valid.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/match/match_scrutinee_narrowing.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/match/match_with_guards.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throw_in_catch.bamlbaml_language/crates/baml_tests/projects/format_checks/match_exprs.bamlbaml_language/crates/baml_tests/projects/generics/classes.bamlbaml_language/crates/baml_tests/projects/match_exhaustiveness/match_exhaustiveness.bamlbaml_language/crates/baml_tests/src/compiler2_tir/mod.rsbaml_language/crates/baml_tests/src/compiler2_tir/phase3a.rsbaml_language/crates/baml_tests/src/compiler2_tir/phase8_exceptions.rsbaml_language/crates/baml_tests/tests/bytecode_format/bytecode_display.bamlbaml_language/crates/baml_tests/tests/exceptions.rsbaml_language/crates/baml_tests/tests/match_basics.rsbaml_language/crates/baml_tests/tests/match_optimization.rsbaml_language/crates/baml_tests/tests/match_types.rsbaml_language/crates/tools_onionskin/src/compiler.rs
💤 Files with no reviewable changes (1)
- baml_language/crates/baml_compiler2_ast/src/disambiguate.rs
| /// Parse a for-in loop pattern: `let var` (without initializer). | ||
| fn parse_for_in_pattern(&mut self) { | ||
| self.with_node(SyntaxKind::LET_STMT, |p| { | ||
| p.expect(TokenKind::Let); | ||
|
|
||
| // Variable name | ||
| if p.at(TokenKind::Word) { | ||
| p.bump(); | ||
| } else { | ||
| p.error_unexpected_token("variable name".to_string()); | ||
| } | ||
|
|
||
| // Optional type annotation | ||
| if p.eat(TokenKind::Colon) { | ||
| p.parse_type(); | ||
| } | ||
|
|
||
| // No initializer for for-in loops - don't emit error | ||
| }); | ||
| self.parse_pattern(true); | ||
| } |
There was a problem hiding this comment.
Update the for-in discriminator for full patterns.
Line 3366 now accepts destructures and : chains, but looks_like_for_in_loop() still only recognizes let WORD in. That means for (let x: int in xs) and for (let User { name } in xs) will fall into the C-style branch and fail to parse as iterator loops.
Please make the lookahead scan a full top-level pattern before deciding, and add a parser unit test for one typed-binding case and one destructure case. As per coding guidelines, "Prefer writing Rust unit tests over integration tests where possible".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@baml_language/crates/baml_compiler_parser/src/parser.rs` around lines 3364 -
3367, looks_like_for_in_loop currently only recognizes "let WORD in" so typed
bindings and destructures like "let x: int in" or "let User { name } in" are
misclassified; change looks_like_for_in_loop to perform a non-consuming
top-level pattern lookahead (use a snapshot/clone of the token stream or a
peek-only helper) that runs the same logic as parse_pattern for the pattern head
and confirms the next top-level token is `in` before deciding; update or reuse
parse_for_in_pattern/parse_pattern semantics in the lookahead to handle colons
and braces, then add two Rust unit tests in the parser tests (prefer unit tests
inside the parser crate) verifying parsing of "for (let x: int in xs) { }" and
"for (let User { name } in xs) { }" so both typed-binding and destructure forms
are accepted as for-in loops.
| for pos in positions { | ||
| let pos_span = pos.span; | ||
| if pos.has_let { | ||
| let name = pos | ||
| .binding_name | ||
| .or_else(|| pos.type_expr.as_ref().and_then(type_expr_single_name)) | ||
| .unwrap_or_else(|| Name::new("_")); | ||
| let binding = self.alloc_pattern(Pattern::binding(name), pos_span); | ||
| if let PatternKind::Bind { inner, .. } = &mut self.patterns[tip].kind { | ||
| *inner = Some(binding); | ||
| } | ||
| rowan::NodeOrToken::Node(child) => { | ||
| match child.kind() { | ||
| SyntaxKind::TYPE_EXPR => { | ||
| // Could be typed binding's type or part of pattern | ||
| if let Some(PatternElement::TypedBindingStart(name, _)) = | ||
| current_element.take() | ||
| { | ||
| if let Some(type_expr) = | ||
| baml_compiler_syntax::ast::TypeExpr::cast(child.clone()) | ||
| { | ||
| let ty = | ||
| crate::lower_type_expr::lower_type_expr_node(&type_expr); | ||
| let pat = Pattern::typed_binding(name, ty); | ||
| elements.push(self.alloc_pattern(pat, child.text_range())); | ||
| } | ||
| } | ||
| } | ||
| SyntaxKind::STRING_LITERAL => { | ||
| if let Some(el) = current_element.take() { | ||
| elements.push(self.finalize_pattern_element(el)); | ||
| } | ||
| let text = child.text().to_string(); | ||
| let content = strip_string_delimiters(&text); | ||
| elements.push(self.alloc_pattern( | ||
| Pattern::literal(Literal::String(content)), | ||
| child.text_range(), | ||
| )); | ||
| } | ||
| SyntaxKind::MATCH_PATTERN | SyntaxKind::CATCH_PATTERN => { | ||
| if let Some(el) = current_element.take() { | ||
| elements.push(self.finalize_pattern_element(el)); | ||
| } | ||
| let nested_pat = self.lower_match_pattern(&child); | ||
| match &self.patterns[nested_pat].kind { | ||
| crate::ast::PatternKind::Or(sub) => { | ||
| elements.extend(sub.iter().copied()); | ||
| } | ||
| _ => elements.push(nested_pat), | ||
| } | ||
| } | ||
| _ => {} | ||
| } | ||
| tip = binding; | ||
| } else if !pos.fields.is_empty() { | ||
| let class = pos | ||
| .type_expr | ||
| .as_ref() | ||
| .and_then(type_expr_single_name) | ||
| .unwrap_or_else(|| Name::new("_")); | ||
| let destr = self.alloc_pattern( | ||
| Pattern { | ||
| kind: PatternKind::Class { | ||
| class, | ||
| fields: pos.fields, | ||
| }, | ||
| chain: None, | ||
| }, | ||
| pos_span, | ||
| ); | ||
| if let PatternKind::Bind { inner, .. } = &mut self.patterns[tip].kind { | ||
| *inner = Some(destr); | ||
| } | ||
| tip = destr; | ||
| } else if let Some(ty) = pos.type_expr { | ||
| let chain_pat = self.type_expr_to_pattern(ty, pos_span); | ||
| self.patterns[tip].chain = Some(chain_pat); | ||
| } |
There was a problem hiding this comment.
Preserve intermediate chain nodes when multiple refinements are present.
Each non-let position writes to self.patterns[tip].chain, but tip never moves to the newly attached node. That means a pattern like let x: A: B collapses to let x: B, which defeats the new chained-pattern representation.
Possible fix
} else if let Some(ty) = pos.type_expr {
let chain_pat = self.type_expr_to_pattern(ty, pos_span);
self.patterns[tip].chain = Some(chain_pat);
+ tip = chain_pat;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for pos in positions { | |
| let pos_span = pos.span; | |
| if pos.has_let { | |
| let name = pos | |
| .binding_name | |
| .or_else(|| pos.type_expr.as_ref().and_then(type_expr_single_name)) | |
| .unwrap_or_else(|| Name::new("_")); | |
| let binding = self.alloc_pattern(Pattern::binding(name), pos_span); | |
| if let PatternKind::Bind { inner, .. } = &mut self.patterns[tip].kind { | |
| *inner = Some(binding); | |
| } | |
| rowan::NodeOrToken::Node(child) => { | |
| match child.kind() { | |
| SyntaxKind::TYPE_EXPR => { | |
| // Could be typed binding's type or part of pattern | |
| if let Some(PatternElement::TypedBindingStart(name, _)) = | |
| current_element.take() | |
| { | |
| if let Some(type_expr) = | |
| baml_compiler_syntax::ast::TypeExpr::cast(child.clone()) | |
| { | |
| let ty = | |
| crate::lower_type_expr::lower_type_expr_node(&type_expr); | |
| let pat = Pattern::typed_binding(name, ty); | |
| elements.push(self.alloc_pattern(pat, child.text_range())); | |
| } | |
| } | |
| } | |
| SyntaxKind::STRING_LITERAL => { | |
| if let Some(el) = current_element.take() { | |
| elements.push(self.finalize_pattern_element(el)); | |
| } | |
| let text = child.text().to_string(); | |
| let content = strip_string_delimiters(&text); | |
| elements.push(self.alloc_pattern( | |
| Pattern::literal(Literal::String(content)), | |
| child.text_range(), | |
| )); | |
| } | |
| SyntaxKind::MATCH_PATTERN | SyntaxKind::CATCH_PATTERN => { | |
| if let Some(el) = current_element.take() { | |
| elements.push(self.finalize_pattern_element(el)); | |
| } | |
| let nested_pat = self.lower_match_pattern(&child); | |
| match &self.patterns[nested_pat].kind { | |
| crate::ast::PatternKind::Or(sub) => { | |
| elements.extend(sub.iter().copied()); | |
| } | |
| _ => elements.push(nested_pat), | |
| } | |
| } | |
| _ => {} | |
| } | |
| tip = binding; | |
| } else if !pos.fields.is_empty() { | |
| let class = pos | |
| .type_expr | |
| .as_ref() | |
| .and_then(type_expr_single_name) | |
| .unwrap_or_else(|| Name::new("_")); | |
| let destr = self.alloc_pattern( | |
| Pattern { | |
| kind: PatternKind::Class { | |
| class, | |
| fields: pos.fields, | |
| }, | |
| chain: None, | |
| }, | |
| pos_span, | |
| ); | |
| if let PatternKind::Bind { inner, .. } = &mut self.patterns[tip].kind { | |
| *inner = Some(destr); | |
| } | |
| tip = destr; | |
| } else if let Some(ty) = pos.type_expr { | |
| let chain_pat = self.type_expr_to_pattern(ty, pos_span); | |
| self.patterns[tip].chain = Some(chain_pat); | |
| } | |
| for pos in positions { | |
| let pos_span = pos.span; | |
| if pos.has_let { | |
| let name = pos | |
| .binding_name | |
| .or_else(|| pos.type_expr.as_ref().and_then(type_expr_single_name)) | |
| .unwrap_or_else(|| Name::new("_")); | |
| let binding = self.alloc_pattern(Pattern::binding(name), pos_span); | |
| if let PatternKind::Bind { inner, .. } = &mut self.patterns[tip].kind { | |
| *inner = Some(binding); | |
| } | |
| tip = binding; | |
| } else if !pos.fields.is_empty() { | |
| let class = pos | |
| .type_expr | |
| .as_ref() | |
| .and_then(type_expr_single_name) | |
| .unwrap_or_else(|| Name::new("_")); | |
| let destr = self.alloc_pattern( | |
| Pattern { | |
| kind: PatternKind::Class { | |
| class, | |
| fields: pos.fields, | |
| }, | |
| chain: None, | |
| }, | |
| pos_span, | |
| ); | |
| if let PatternKind::Bind { inner, .. } = &mut self.patterns[tip].kind { | |
| *inner = Some(destr); | |
| } | |
| tip = destr; | |
| } else if let Some(ty) = pos.type_expr { | |
| let chain_pat = self.type_expr_to_pattern(ty, pos_span); | |
| self.patterns[tip].chain = Some(chain_pat); | |
| tip = chain_pat; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@baml_language/crates/baml_compiler2_ast/src/lower_expr_body.rs` around lines
1183 - 1218, The loop currently attaches a chain pattern via
self.patterns[tip].chain = Some(chain_pat) but never advances tip, so
intermediate chain nodes are lost (e.g., let x: A: B becomes let x: B); after
creating/assigning the chain node returned by type_expr_to_pattern (chain_pat)
update tip to that new pattern index (tip = chain_pat). Reference: the for
positions loop, variable tip, assignments to self.patterns[tip].chain, the call
to type_expr_to_pattern, and existing alloc_pattern/destr/binding branches that
already set tip — mirror those by assigning tip to the newly attached chain
node.
| fn type_expr_to_pattern(&mut self, ty: crate::ast::TypeExpr, span: TextRange) -> PatId { | ||
| if let crate::ast::TypeExpr::Path { | ||
| segments, | ||
| generic_args, | ||
| .. | ||
| } = &ty | ||
| { | ||
| if generic_args.is_empty() && segments.len() == 1 && segments[0].as_str() == "_" { | ||
| return self.alloc_pattern(Pattern::wildcard(), span); | ||
| } | ||
| } | ||
| if let crate::ast::TypeExpr::Union { variants, .. } = ty { | ||
| let parts: Vec<PatId> = variants | ||
| .into_iter() | ||
| .map(|v| self.type_expr_to_pattern(v, span)) | ||
| .collect(); | ||
| return self.alloc_pattern(Pattern::or(parts), span); | ||
| } | ||
| self.alloc_pattern(Pattern::type_match(ty), span) |
There was a problem hiding this comment.
Union let annotations are dropped on the PATTERN path.
type_expr_to_pattern() lowers int | string into PatternKind::Or, but this recovery path only rebuilds type_annotation from PatternKind::Type. let x: int | string = ... will therefore lose its annotation whenever the parser routes it through PATTERN.
Also applies to: 2448-2462
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@baml_language/crates/baml_compiler2_ast/src/lower_expr_body.rs` around lines
1281 - 1299, type_expr_to_pattern currently expands a TypeExpr::Union into
Pattern::or of its variant patterns which loses the original union type
annotation; instead preserve the annotation by allocating a type-match pattern
for the union itself (i.e. return self.alloc_pattern(Pattern::type_match(ty),
span) when encountering a TypeExpr::Union) or, if you must build an Or, ensure
each part is a type-match of the corresponding variant and attach the original
union type to the resulting pattern; update the TypeExpr::Union branch in
type_expr_to_pattern (and the similar code at the other location referenced) to
return a type_match for the union to avoid dropping annotations.
| if let Some(chain_id) = clause_pat.chain { | ||
| if let PatternKind::Type(ty) = &body.patterns[chain_id].kind { | ||
| if let Some(banned) = crate::throw_inference::is_banned_catch_binding_type(ty) { | ||
| self.context.report_simple( | ||
| TirTypeError::InvalidCatchBindingType { | ||
| type_name: banned.to_string(), | ||
| }, | ||
| base_expr_id, |
There was a problem hiding this comment.
Report InvalidCatchBindingType on the offending chained pattern.
Using base_expr_id here makes every bad clause point at the caught expression instead of the invalid type pattern, so multiple invalid clauses in one catch collapse onto the same span. Please anchor this to chain_id or the chained type span instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@baml_language/crates/baml_compiler2_tir/src/builder.rs` around lines 2442 -
2449, The current report uses base_expr_id which points at the caught
expression; change the span passed to self.context.report_simple so the
InvalidCatchBindingType error is anchored to the offending chained pattern
instead—use the chained pattern's id (chain_id) or its span from
body.patterns[chain_id] (where PatternKind::Type(ty) is matched) when calling
self.context.report_simple for TirTypeError::InvalidCatchBindingType rather than
base_expr_id.
| // Enum variant: try interpreting as Enum.Variant (all but last | ||
| // segment = enum name, last segment = variant). | ||
| if segments.len() >= 2 { | ||
| let enum_name = &segments[..segments.len() - 1]; | ||
| let variant = segments.last().unwrap(); | ||
| let enum_item_name = if enum_name.len() == 1 { | ||
| enum_name[0].clone() | ||
| } else { | ||
| baml_base::Name::new( | ||
| enum_name | ||
| .iter() | ||
| .map(smol_str::SmolStr::as_str) | ||
| .collect::<Vec<_>>() | ||
| .join("."), | ||
| ) | ||
| }; | ||
| let enum_resolved = if !ns_context.is_empty() { | ||
| let ns: Vec<baml_base::Name> = ns_context.to_vec(); | ||
| package_items.lookup_type(&ns, &enum_item_name) | ||
| } else { | ||
| package_items.lookup_type(&[], &enum_item_name) | ||
| }; | ||
| let enum_resolved = enum_resolved.or_else(|| { | ||
| if enum_name.len() >= 2 { | ||
| if enum_name[0].as_str() == "root" { | ||
| package_items.lookup_type(&enum_name[1..], &enum_item_name) | ||
| } else { | ||
| let pkg_id = PackageId::new(db, enum_name[0].clone()); | ||
| let pkg = baml_compiler2_ppir::package_items(db, pkg_id); | ||
| pkg.lookup_type(&enum_name[1..], &enum_item_name) | ||
| } | ||
| } else { | ||
| None | ||
| } | ||
| }); | ||
| if let Some(def) = enum_resolved { | ||
| if matches!(def, Definition::Enum(_)) { | ||
| return Ty::EnumVariant( | ||
| qualify_def(db, def, &enum_item_name), | ||
| variant.clone(), | ||
| TyAttr::default(), | ||
| ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file and understand the context around lines 175-219
cd /repo && wc -l baml_language/crates/baml_compiler2_tir/src/lower_type_expr.rsRepository: BoundaryML/baml
Length of output: 115
🏁 Script executed:
# Let's look at the full context around the flagged lines
cd /repo && sed -n '150,230p' baml_language/crates/baml_compiler2_tir/src/lower_type_expr.rs | cat -nRepository: BoundaryML/baml
Length of output: 115
🏁 Script executed:
# Check how lookup_type is used elsewhere in this file
cd /repo && rg -n "lookup_type" baml_language/crates/baml_compiler2_tir/src/lower_type_expr.rs -B 3 -A 3Repository: BoundaryML/baml
Length of output: 115
🏁 Script executed:
# Compare with the pattern mentioned in learnings - check lower.rs from PR `#3417`
cd /repo && rg -n "ItemRef::EnumType\|lower_pattern_test" baml_language/crates/baml_compiler2_mir/src/lower.rs -B 2 -A 8Repository: BoundaryML/baml
Length of output: 115
🏁 Script executed:
# Check if there are tests for qualified enum variant resolution
cd /repo && find . -name "*.rs" -type f -path "*/test*" -exec grep -l "enum.*variant\|EnumVariant" {} \; | head -10Repository: BoundaryML/baml
Length of output: 115
🏁 Script executed:
# Check the qualify_def function to understand what it expects
cd /repo && rg -n "fn qualify_def" baml_language/crates/baml_compiler2_tir/src/ -A 15Repository: BoundaryML/baml
Length of output: 115
🏁 Script executed:
wc -l baml_language/crates/baml_compiler2_tir/src/lower_type_expr.rsRepository: BoundaryML/baml
Length of output: 126
🏁 Script executed:
sed -n '150,230p' baml_language/crates/baml_compiler2_tir/src/lower_type_expr.rs | cat -nRepository: BoundaryML/baml
Length of output: 4670
🏁 Script executed:
rg -n "lookup_type" baml_language/crates/baml_compiler2_tir/src/lower_type_expr.rs -B 2 -A 2Repository: BoundaryML/baml
Length of output: 2309
🏁 Script executed:
rg -n "ItemRef::EnumType\|EnumVariant" baml_language/crates/baml_compiler2_mir/src/lower.rs -B 1 -A 5 | head -60Repository: BoundaryML/baml
Length of output: 41
🏁 Script executed:
rg -n "fn qualify_def" baml_language/crates/baml_compiler2_tir/src/ -A 10Repository: BoundaryML/baml
Length of output: 972
Split the enum path into namespace + short name before lookup.
The enum variant fallback (lines 175–219) constructs a dotted enum_item_name and passes it to lookup_type, but lookup_type(namespace_segments, item_name) expects a short name, not a dotted compound. This causes qualified cases like ns.Enum.Variant, root.ns.Enum.Variant, and pkg.ns.Enum.Variant to fail resolution, and qualify_def(..., &enum_item_name) would stamp the wrong enum name.
Mirror the correct pattern used earlier in this file (lines 42–68): resolve enum_ns (all but last segment) and enum_item (last segment) separately, then pass only enum_item to lookup_type and qualify_def.
Suggested fix
- if segments.len() >= 2 {
- let enum_name = &segments[..segments.len() - 1];
- let variant = segments.last().unwrap();
- let enum_item_name = if enum_name.len() == 1 {
- enum_name[0].clone()
- } else {
- baml_base::Name::new(
- enum_name
- .iter()
- .map(smol_str::SmolStr::as_str)
- .collect::<Vec<_>>()
- .join("."),
- )
- };
+ if segments.len() >= 2 {
+ let enum_name = &segments[..segments.len() - 1];
+ let variant = segments.last().unwrap();
+ let enum_item = enum_name.last().expect("non-empty enum path").clone();
+ let enum_ns = &enum_name[..enum_name.len().saturating_sub(1)];
let enum_resolved = if !ns_context.is_empty() {
- let ns: Vec<baml_base::Name> = ns_context.to_vec();
- package_items.lookup_type(&ns, &enum_item_name)
+ let ns: Vec<baml_base::Name> =
+ ns_context.iter().chain(enum_ns.iter()).cloned().collect();
+ package_items.lookup_type(&ns, &enum_item)
} else {
- package_items.lookup_type(&[], &enum_item_name)
+ package_items.lookup_type(enum_ns, &enum_item)
};
let enum_resolved = enum_resolved.or_else(|| {
if enum_name.len() >= 2 {
if enum_name[0].as_str() == "root" {
- package_items.lookup_type(&enum_name[1..], &enum_item_name)
+ let ns = &enum_name[1..enum_name.len() - 1];
+ package_items.lookup_type(ns, &enum_item)
} else {
let pkg_id = PackageId::new(db, enum_name[0].clone());
let pkg = baml_compiler2_ppir::package_items(db, pkg_id);
- pkg.lookup_type(&enum_name[1..], &enum_item_name)
+ let ns = &enum_name[1..enum_name.len() - 1];
+ pkg.lookup_type(ns, &enum_item)
}
} else {
None
}
});
if let Some(def) = enum_resolved {
if matches!(def, Definition::Enum(_)) {
return Ty::EnumVariant(
- qualify_def(db, def, &enum_item_name),
+ qualify_def(db, def, &enum_item),
variant.clone(),
TyAttr::default(),
);
}
}
}Add a Rust unit test for qualified enum variant cases (ns.Enum.Variant, root.ns.Enum.Variant, pkg.ns.Enum.Variant), and run cargo test --lib after the fix.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@baml_language/crates/baml_compiler2_tir/src/lower_type_expr.rs` around lines
175 - 219, The fallback treats the whole dotted enum path as a single short name
(enum_item_name) causing lookups and qualify_def to be wrong; change the logic
to split segments into enum_ns (all but last segment) and enum_item (last short
name) like earlier in the file, then call
package_items.lookup_type(&enum_ns_with_context, &enum_item) (and in the
root/pkg branches use package_items.lookup_type(&enum_name[1..], &enum_item) or
pkg.lookup_type(&enum_name[1..], &enum_item)), and pass &enum_item to
qualify_def(db, def, &enum_item) instead of &enum_item_name; update the branches
that build the namespace vector (using ns_context when present) and add unit
tests for ns.Enum.Variant, root.ns.Enum.Variant, and pkg.ns.Enum.Variant, then
run cargo test --lib.
| pub struct FmtPattern { | ||
| pub kw_let: Option<t::Let>, | ||
| pub first: PatternPosition, | ||
| pub chain: Vec<ChainedPosition>, | ||
| } | ||
|
|
||
| /// A single `: <position>` in the chain. | ||
| #[derive(Debug)] | ||
| pub struct UnionPattern { | ||
| pub first: Box<UnionPatternMember>, | ||
| pub rest: Vec<(t::Pipe, UnionPatternMember)>, | ||
| } | ||
|
|
||
| impl UnionPattern { | ||
| /// Returns an iterator over the members in the union pattern, in order. | ||
| pub fn iter_patterns(&self) -> impl Iterator<Item = &UnionPatternMember> { | ||
| std::iter::once(&*self.first).chain(self.rest.iter().map(|(_, p)| p)) | ||
| } | ||
| pub struct ChainedPosition { | ||
| pub colon: t::Colon, | ||
| pub kw_let: Option<t::Let>, | ||
| pub position: PatternPosition, | ||
| } |
There was a problem hiding this comment.
Class destructure patterns are dropped during formatting.
This model only stores WORD | TYPE_EXPR positions plus : links. For a pattern like User { name, age: let a }, Lines 75-117 consume TYPE_EXPR(User) and never represent the { ... } / PATTERN_FIELD tail, so Lines 133-162 will print only User. That is a destructive rewrite of valid syntax.
The formatter AST needs a destructure-aware variant (or equivalent field storage) before this ships.
Also applies to: 75-124, 133-176
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@baml_language/crates/baml_fmt/src/ast/pattern.rs` around lines 61 - 73, The
formatter AST currently loses struct/record destructure tails because FmtPattern
/ ChainedPosition only model WORD | TYPE_EXPR plus colon links; extend the
pattern representation to preserve destructures by adding a Destructure (or
PatternFields) variant to PatternPosition (or an additional field on
ChainedPosition) that stores the brace tokens and a Vec of PATTERN_FIELD entries
(including commas and optional `let`/nested patterns), update any places that
build FmtPattern/ChainedPosition to populate this new variant, and update the
printing/formatting code that consumes FmtPattern and PatternPosition to render
the `{ ... }` tail (fields, commas, nested patterns) when the Destructure
variant is present so patterns like `User { name, age: let a }` are preserved
instead of being dropped.
Summary by CodeRabbit
Release Notes
letkeyword for type-annotated bindings (e.g.,n: int =>becomeslet n: int =>).