Repository navigation
fix(diagnostics): exclude leading/trailing trivia from spans - #3860
Merged
Merged
Conversation
Rowan keeps whitespace/newlines/comments as child tokens, so a node's
`text_range()` includes leading trivia. Diagnostic and editor spans built
from raw `text_range()` underlined the inter-token gap before the real
token (e.g. the space after `->` in a return type), and multi-line body
spans pulled in the newline + indentation after `{`.
Route node-derived spans through `SyntaxNodeExt::span_range` (= the
existing-but-unused `trimmed_range`), which trims to the first/last
non-trivia token. Parser end-of-input error spans now collapse to a
zero-width caret at the end of the last real token (rustc's
`shrink_to_hi` idiom) instead of reaching into `tokens.last()` and
covering a trailing newline.
Add a byte-level regression check in `test_05_diagnostics` asserting no
diagnostic annotation span begins or ends on whitespace, enforced across
the whole snapshot corpus.
Also line-align the `describe` body slice, which had been relying on the
span bug to preserve the first line's indentation.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe PR adds ChangesSpan-range alignment
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
baml_language/crates/baml_compiler2_ast/src/lower_expr_body.rs (2)
3863-3874: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFinish
span_range()adoption in template C-styleforrecovery.These node-backed fallbacks still use
text_range(), which can reintroduce whitespace/comment-padded spans in malformed templateforheaders.Proposed fix
let cond = expr_nodes .first() .map(|n| self.lower_expr(n)) - .unwrap_or_else(|| self.alloc_expr(Expr::Missing, for_seg.open.text_range())); + .unwrap_or_else(|| self.alloc_expr(Expr::Missing, for_seg.open.span_range())); // Step is an assignment (`i += 1`) or a bare expression; absent for // `for (let i = 0; cond; )`. let step = expr_nodes.get(1).map(|n| { - let r = n.text_range(); + let r = n.span_range(); self.try_lower_assignment(n).unwrap_or_else(|| { let e = self.lower_expr(n); self.alloc_stmt(Stmt::Expr(e), r)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/crates/baml_compiler2_ast/src/lower_expr_body.rs` around lines 3863 - 3874, The C-style `for` recovery path in `lower_expr_body` still uses `text_range()` for node-backed fallback spans, which can reintroduce padded ranges in malformed template headers. Update the fallback span selection in this `for` lowering logic to use `span_range()` consistently for the `cond` and `step` node-derived expressions/statements, matching the rest of the template recovery behavior and keeping spans tight when nodes are missing or partially recovered.
1574-1575: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAnchor the missing match scrutinee to the match node span.
Line 1575 still allocates
Expr::Missingat byte 0 viaTextRange::default(), so malformedmatchexpressions can point diagnostics/editor locations at the start of the file instead of the localmatch.Proposed fix
- let scrutinee = - scrutinee.unwrap_or_else(|| self.alloc_expr(Expr::Missing, TextRange::default())); + let span = node.span_range(); + let scrutinee = scrutinee.unwrap_or_else(|| self.alloc_expr(Expr::Missing, span)); self.alloc_expr( Expr::Match { scrutinee, scrutinee_type, arms: arm_ids, }, - node.span_range(), + span, )Also applies to: 1583-1583
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/crates/baml_compiler2_ast/src/lower_expr_body.rs` around lines 1574 - 1575, The fallback for a missing match scrutinee currently uses Expr::Missing with TextRange::default(), which anchors diagnostics at byte 0 instead of the local match expression. Update the scrutinee fallback in lower_expr_body.rs around the match lowering logic so that self.alloc_expr(Expr::Missing, ...) uses the match node’s own span/text range rather than a default range, and make the same adjustment for the related match arm path noted in the comment so missing-match diagnostics stay attached to the match node.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@baml_language/crates/baml_compiler2_ast/src/lower_expr_body.rs`:
- Around line 3863-3874: The C-style `for` recovery path in `lower_expr_body`
still uses `text_range()` for node-backed fallback spans, which can reintroduce
padded ranges in malformed template headers. Update the fallback span selection
in this `for` lowering logic to use `span_range()` consistently for the `cond`
and `step` node-derived expressions/statements, matching the rest of the
template recovery behavior and keeping spans tight when nodes are missing or
partially recovered.
- Around line 1574-1575: The fallback for a missing match scrutinee currently
uses Expr::Missing with TextRange::default(), which anchors diagnostics at byte
0 instead of the local match expression. Update the scrutinee fallback in
lower_expr_body.rs around the match lowering logic so that
self.alloc_expr(Expr::Missing, ...) uses the match node’s own span/text range
rather than a default range, and make the same adjustment for the related match
arm path noted in the comment so missing-match diagnostics stay attached to the
match node.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5e7e394b-f51f-4e97-8a38-54ad10f7d4d3
⛔ Files ignored due to path filters (78)
baml_language/crates/baml_tests/snapshots/broken_syntax/banned_expressions/baml_tests__broken_syntax__banned_expressions__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/broken_syntax/error_cases/baml_tests__broken_syntax__error_cases__02_parser__syntax_errors.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/broken_syntax/error_cases/baml_tests__broken_syntax__error_cases__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/broken_syntax/header_in_llm_function/baml_tests__broken_syntax__header_in_llm_function__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/broken_syntax/is_in_type_position/baml_tests__broken_syntax__is_in_type_position__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/broken_syntax/mismatched_brackets/baml_tests__broken_syntax__mismatched_brackets__02_parser__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/broken_syntax/mismatched_brackets/baml_tests__broken_syntax__mismatched_brackets__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/broken_syntax/parser_error_recovery/baml_tests__broken_syntax__parser_error_recovery__02_parser__unclosed_string.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/broken_syntax/parser_error_recovery/baml_tests__broken_syntax__parser_error_recovery__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/broken_syntax/parser_speculative/baml_tests__broken_syntax__parser_speculative__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/broken_syntax/parser_strings/baml_tests__broken_syntax__parser_strings__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/broken_syntax/pending_greaters_fix/baml_tests__broken_syntax__pending_greaters_fix__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/broken_syntax/postfix_non_null_assertion/baml_tests__broken_syntax__postfix_non_null_assertion__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/broken_syntax/test_invalid_contexts/baml_tests__broken_syntax__test_invalid_contexts__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/broken_syntax/type_annotation_errors/baml_tests__broken_syntax__type_annotation_errors__02_parser__type_malformed.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/broken_syntax/type_annotation_errors/baml_tests__broken_syntax__type_annotation_errors__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/catch_throw/baml_tests__compiles__catch_throw__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/catch_throw/baml_tests__compiles__catch_throw__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/assignment_expr_position/baml_tests__diagnostic_errors__assignment_expr_position__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/assignment_expr_position/baml_tests__diagnostic_errors__assignment_expr_position__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/basic_types/baml_tests__diagnostic_errors__basic_types__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/basic_types/baml_tests__diagnostic_errors__basic_types__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/builtin_io/baml_tests__diagnostic_errors__builtin_io__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/builtin_io/baml_tests__diagnostic_errors__builtin_io__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/captured_field_chain/baml_tests__diagnostic_errors__captured_field_chain__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/captured_field_chain/baml_tests__diagnostic_errors__captured_field_chain__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/catch_return_type_mismatch/baml_tests__diagnostic_errors__catch_return_type_mismatch__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/catch_return_type_mismatch/baml_tests__diagnostic_errors__catch_return_type_mismatch__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/catch_throw_regressions/baml_tests__diagnostic_errors__catch_throw_regressions__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/catch_throw_regressions/baml_tests__diagnostic_errors__catch_throw_regressions__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/client_option_types/baml_tests__diagnostic_errors__client_option_types__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/closure_errors/baml_tests__diagnostic_errors__closure_errors__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/closure_errors/baml_tests__diagnostic_errors__closure_errors__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/control_flow/baml_tests__diagnostic_errors__control_flow__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/control_flow/baml_tests__diagnostic_errors__control_flow__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/format_checks/baml_tests__diagnostic_errors__format_checks__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/format_checks/baml_tests__diagnostic_errors__format_checks__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generics/baml_tests__diagnostic_errors__generics__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generics/baml_tests__diagnostic_errors__generics__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/is_pattern_negative/baml_tests__diagnostic_errors__is_pattern_negative__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/is_pattern_negative/baml_tests__diagnostic_errors__is_pattern_negative__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/json_static_method_arity/baml_tests__diagnostic_errors__json_static_method_arity__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/json_static_method_arity/baml_tests__diagnostic_errors__json_static_method_arity__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/lambda_errors/baml_tests__diagnostic_errors__lambda_errors__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/lambda_errors/baml_tests__diagnostic_errors__lambda_errors__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/match_exhaustiveness/baml_tests__diagnostic_errors__match_exhaustiveness__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/match_exhaustiveness/baml_tests__diagnostic_errors__match_exhaustiveness__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/namespaces_bare_name_rejected/baml_tests__diagnostic_errors__namespaces_bare_name_rejected__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/namespaces_bare_name_rejected/baml_tests__diagnostic_errors__namespaces_bare_name_rejected__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/namespaces_stream_direct_ref/baml_tests__diagnostic_errors__namespaces_stream_direct_ref__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/namespaces_stream_direct_ref/baml_tests__diagnostic_errors__namespaces_stream_direct_ref__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/null_handling/baml_tests__diagnostic_errors__null_handling__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/null_handling/baml_tests__diagnostic_errors__null_handling__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/optional_parameter_defaults/baml_tests__diagnostic_errors__optional_parameter_defaults__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/optional_parameter_defaults/baml_tests__diagnostic_errors__optional_parameter_defaults__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/patterns_class_destructure/baml_tests__diagnostic_errors__patterns_class_destructure__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/patterns_class_destructure/baml_tests__diagnostic_errors__patterns_class_destructure__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/patterns_class_destructure_namespaces/baml_tests__diagnostic_errors__patterns_class_destructure_namespaces__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/patterns_class_destructure_namespaces/baml_tests__diagnostic_errors__patterns_class_destructure_namespaces__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/patterns_new/baml_tests__diagnostic_errors__patterns_new__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/patterns_new/baml_tests__diagnostic_errors__patterns_new__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/runtime_id_misuse/baml_tests__diagnostic_errors__runtime_id_misuse__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/runtime_id_misuse/baml_tests__diagnostic_errors__runtime_id_misuse__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/simple_function/baml_tests__diagnostic_errors__simple_function__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/simple_function/baml_tests__diagnostic_errors__simple_function__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/test_expr_name_type_error/baml_tests__diagnostic_errors__test_expr_name_type_error__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/test_expr_name_type_error/baml_tests__diagnostic_errors__test_expr_name_type_error__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/type_aliases/baml_tests__diagnostic_errors__type_aliases__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/type_reflection_strict/baml_tests__diagnostic_errors__type_reflection_strict__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/type_reflection_strict/baml_tests__diagnostic_errors__type_reflection_strict__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/unknown_type_error/baml_tests__diagnostic_errors__unknown_type_error__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/unknown_type_error/baml_tests__diagnostic_errors__unknown_type_error__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/void_return_type/baml_tests__diagnostic_errors__void_return_type__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/void_return_type/baml_tests__diagnostic_errors__void_return_type__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase3a__optional_param_default_declaration_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase3a__optional_param_default_forward_reference_checks_lambda_bodies.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded.snapis excluded by!**/*.snap
📒 Files selected for processing (13)
baml_language/crates/baml_compiler2_ast/src/lower_config_item.rsbaml_language/crates/baml_compiler2_ast/src/lower_cst.rsbaml_language/crates/baml_compiler2_ast/src/lower_expr_body.rsbaml_language/crates/baml_compiler2_ast/src/lower_type_expr.rsbaml_language/crates/baml_compiler_parser/src/parser.rsbaml_language/crates/baml_compiler_syntax/src/traversal.rsbaml_language/crates/baml_lsp2_actions/src/describe.rsbaml_language/crates/baml_tests/build.rsbaml_language/crates/baml_tests/src/compiler2_tir/explicit_type_args.rsbaml_language/crates/baml_tests/src/compiler2_tir/phase3a.rsbaml_language/crates/baml_tests/src/compiler2_tir/phase3a_recursion.rsbaml_language/crates/baml_tests/src/compiler2_tir/phase6.rsbaml_language/crates/baml_tests/src/utils/mod.rs
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
|
No description provided. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/expr/watch_when.baml`:
- Around line 48-54: Remove the stale “Known issue” note from the watch_when
snapshot fixture so it matches the updated diagnostics. Update the relevant
snapshot comments around the affected expr_watch_when.baml snapshots to reflect
that the error now points at x.$watch directly, and make the same cleanup for
the additional duplicated snapshots in this test file so the fixture and
snapshots stay consistent.
In
`@baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/loops/break.baml`:
- Around line 58-65: Unreachable-code diagnostics are incorrectly reusing the
TypeMismatch error code E0001, which makes the warning text and snapshots
misleading. Update the diagnostic code mapping in the unreachable-code path so
it uses its own warning-specific code rather than the code reserved by
diagnostic.rs for TypeMismatch, and then adjust the break.baml snapshot
expectations to match the new code emitted by the unreachable-code diagnostic.
🪄 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: 0d5828cb-7f89-405b-b65a-1459a579aae1
📒 Files selected for processing (94)
baml_language/crates/baml_lsp2_actions_tests/test_files/semantic_tokens/break_continue.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/semantic_tokens/class.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/semantic_tokens/for_loops.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/semantic_tokens/function.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/semantic_tokens/if_else.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/semantic_tokens/match_expr.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/semantic_tokens/multi_segment_path.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/catch/catch_all_exhaustive.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/exhaustiveness_checks.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/nested_catch.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/class/map_types2.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/class/misspeled_boolean_literals.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/class/secure_types.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/class/spelling_error.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/class/unknown_type.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/enum/enum_unquoted_description.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/expr/constructors_invalid.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/expr/instanceof_narrowing.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/expr/missing_semicolons.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/expr/unresolved_builtin_namespace.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/expr/var_keyword_let_await.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/expr/watch_when.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/functions_v2/duplicate_names.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/functions_v2/invalid.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/functions_v2/invalid2.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/functions_v2/match.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/functions_v2/shorthand_clients.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/headers/if_then_only_header.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/hover/catch_rethrow.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/hover/catch_rethrow_caller.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/hover/function_throws.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/if_let/if_let_binding_scope.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/if_let/if_let_chained.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/if_let/if_let_destructure.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/if_let/if_let_irrefutable_warns.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/if_let/if_let_no_else_in_value_position.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/if_let/if_let_throws.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/duplicate_implements.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_ambiguous_interface_field_access.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_ambiguous_method_call.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_default_method_non_exhaustive.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_default_on_required_method.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_default_outside_implements.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_field_declared_in_implements_block.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_field_rule_invariant_union.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_generic_bound_violation.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_generic_requires_cycle_changes_args.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_match_without_wildcard.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_missing_field_in_implements_block.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_missing_required_interface.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_multi_self_call_on_interface_receiver.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_old_qualified_projection.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_out_of_body_field_interface.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_self_outside_type.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/field_type_mismatch.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/missing_required_method.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/unknown_interface.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/invalid_assignment/arrays.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/invalid_assignment/class_arrays.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/invalid_assignment/maps.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/invalid_assignment/nested_arrays.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/iterator/stdlib_iterator_errors.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/let_else/let_else_destructure.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/let_else/let_else_diverges_loop.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/let_else/let_else_else_no_bindings.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/let_else/let_else_irrefutable_warns.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/let_else/let_else_must_diverge.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/let_else/let_else_throws.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/let_else/let_else_wildcard_irrefutable.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/loops/break.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/loops/c_for.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/loops/continue.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/match/match_non_exhaustive.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/match/match_with_guards.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/misc/dynamic_types_parser_errors.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/parens.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throw_basic.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throw_expression.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throw_in_catch.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throw_with_catch.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_basic_syntax.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_caller_sees_contract.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_caller_variant_contract.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_contract_violation.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_enum_exact_match.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_enum_extraneous.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_enum_variant_precise.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_enum_variant_violation.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_mixed.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_never_unreachable_rethrow.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_never_violation.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_type_alias_repro.baml
💤 Files with no reviewable changes (6)
- baml_language/crates/baml_lsp2_actions_tests/test_files/semantic_tokens/function.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/semantic_tokens/break_continue.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/semantic_tokens/match_expr.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/semantic_tokens/if_else.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/semantic_tokens/for_loops.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/semantic_tokens/multi_segment_path.baml
✅ Files skipped from review due to trivial changes (62)
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/class/unknown_type.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/if_let/if_let_irrefutable_warns.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/let_else/let_else_wildcard_irrefutable.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_missing_required_interface.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_missing_field_in_implements_block.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/let_else/let_else_irrefutable_warns.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/let_else/let_else_diverges_loop.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/match/match_with_guards.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/field_type_mismatch.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/catch/catch_wildcard_only.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_default_method_non_exhaustive.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_field_rule_invariant_union.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/functions_v2/shorthand_clients.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_ambiguous_method_call.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/let_else/let_else_must_diverge.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/hover/catch_rethrow_caller.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_match_without_wildcard.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_out_of_body_field_interface.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/if_let/if_let_destructure.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/invalid_assignment/maps.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_ambiguous_interface_field_access.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_never_unreachable_rethrow.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/invalid_assignment/nested_arrays.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/misc/dynamic_types_parser_errors.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_default_outside_implements.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/parens.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_mixed.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/hover/catch_rethrow.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_never_violation.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_enum_variant_precise.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throw_expression.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/let_else/let_else_else_no_bindings.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/if_let/if_let_throws.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/let_else/let_else_destructure.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/hover/function_throws.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/unknown_interface.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_field_declared_in_implements_block.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/catch/exhaustiveness_checks.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/duplicate_implements.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/let_else/let_else_throws.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/class/misspeled_boolean_literals.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/expr/instanceof_narrowing.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/missing_required_method.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_generic_requires_cycle_changes_args.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_contract_violation.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/expr/missing_semicolons.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/catch/invalid_arm_syntax.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/catch/catch_all_exhaustive.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_basic_syntax.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/expr/unresolved_builtin_namespace.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/headers/if_then_only_header.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_enum_exact_match.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/expr/constructors_invalid.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_caller_variant_contract.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/invalid_assignment/arrays.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/catch/nested_catch.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_caller_sees_contract.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_self_outside_type.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_enum_extraneous.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_enum_variant_violation.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/functions_v2/invalid2.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/class/secure_types.baml
codeshaunted
enabled auto-merge
June 26, 2026 22:59
codeshaunted
disabled auto-merge
June 26, 2026 23:08
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Jun 27, 2026
…s) (#3866) Fixes **B-538** — an unresolved type inside a compound (union/map/tuple) underlined the *whole* compound instead of the offending identifier. > Stacked on #3860 (avery/diag-spans). Until that merges, this PR's diff includes its commits; it'll narrow to just the B-538 changes once #3860 lands. ## Root cause `ast::TypeExpr` was span-less except for a single outer `SpannedTypeExpr` wrapper, so every diagnostic from a type-expr was reported at the whole annotation's span. ## Fix — recursive spanning (chosen over a targeted patch) `TypeExpr` is now a fully-spanned tree: `struct TypeExpr { kind: TypeExprKind, span }` with `Deref`/`DerefMut` to the kind enum and a `TypeExprKind::at(span)` constructor. The old `enum TypeExpr` became `enum TypeExprKind`; `SpannedTypeExpr` is deleted (redundant). Every node carries its real CST span (`span_range()` per node; union members get the offending-identifier range via `UnionMemberParts::span()`), so **any** sub-type diagnostic points exactly at it — no per-diagnostic special-casing. - Equality/Hash on `TypeExpr` are **structural** (ignore span) to preserve dedup / Salsa early-cutoff. - Migrated across ~12 crates (ast/hir/ppir/tir/mir/lsp/emit/builtins-codegen/visualization/tests). - Fixed one latent double-emission of impl-target unresolved-type errors that the now-precise spans unmasked. ## Effect ``` 3 │ a map<string[], (int | bool[]) | apple_pie[][]> │ ╰────── unresolved type: apple_pie ``` (was: the whole `map<…>` underlined.) ## Verification Full `baml_tests` (2654) + `baml_lsp2_actions_tests` (375) green after rebless. Only span-narrowing snapshot changes — verified no message/error-code/diagnostic-count changes (balanced line counts). Workspace compiles `--all-targets`. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved type-expression error reporting with more precise, member-level source highlights for unresolved types. * Fixed inconsistent validation and rendering of method/function signatures, interface `requires`, `implements` targets, and field/type-alias types. * Refined LSP/compiler diagnostic span selection for clearer caret/underline locations, including better end-of-input handling near trailing comments. * **Tests** * Updated golden diagnostics and snapshot expectations for the new type-expression formatting and span behavior, including nested/compound types and throws/never-related cases. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This branch was previously deployed
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.
What
Diagnostic (and editor) spans were including the leading/trailing whitespace around the construct they point at. The rowan CST keeps trivia (whitespace, newlines, comments) as child tokens, so a node's raw
text_range()starts on the inter-token gap before its first real token. An audit of the snapshot corpus found 69 over-extension spans across 12 error codes (E0001–E0137), plus a much larger multi-line class where body/block spans pulled in the newline + indentation after{.Fix
SyntaxNodeExt::span_range(= the existing-but-unusedtrimmed_range) trims a node range to its first/last non-trivia token. All node-derived spans inbaml_compiler2_astnow build through it; token spans (already tight) are untouched.expect/error_unexpected_token/error_here) now collapse to a zero-width caret at the end of the last real token (rustc'sprev_token.span.shrink_to_hi()idiom) instead of reaching into the rawtokens.last(), which could be a trailing newline and produced a span over"\n".test_05_diagnosticsnow asserts, at the byte level, that no diagnostic annotation span begins or ends on whitespace — enforced across the whole snapshot corpus, so this can't regress.describebody slice had been relying on the span bug to keep its first line's indentation; it now line-aligns the slice itself.Effect
Spans tighten to the construct; sprawling multi-line block underlines collapse to precise single-line spans (e.g. a "type mismatch" that underlined a whole function body now points exactly at
self.data). 78 snapshots updated — all span-only (diagnostic underlines, TIR/bytecode span byte-ranges & line attribution, parser carets); no structural IR changes.Verification
Full
baml_testsrun: 0 whitespace-assertion failures, 0 non-snapshot failures, 0 structural IR drift. LSP + CLI suites green.Summary by CodeRabbit