Skip to content

Commit ccd9110

Browse files
hellovaiclaude
andauthored
Fix LSP navigation for class fields, enum variants, and methods (#3298)
[Artifacts](https://cloud.codelayer.cloud/tasks/019d31f9-344d-731b-bc97-b0610fd44836/artifacts) | [Task](https://cloud.codelayer.cloud/deep/tasks/019d31f9-344d-731b-bc97-b0610fd44836) ## Problem "Go to Definition" and "Find All References" didn't work for class fields, enum variants, constructor fields, or methods. Clicking `p.name`, `Status.Active`, `Person { name: ... }`, or `s.Celebrate()` did nothing. This made navigating BAML code painful — you had to manually search for field definitions instead of just ctrl-clicking. Also: when you misspelled a field like `s.feelin`, the red squiggly underlined the whole `s.feelin` expression instead of just `feelin`. ## What changed for users - **Goto-definition** works for field access, enum variants, constructor fields, and method calls - **Find All References** on a class field definition finds all usage sites across the package - **Document outline** shows real spans for fields/variants (not zero-width placeholders) - **Diagnostic squigglies** for unresolved members now highlight just the member name - **LSP no longer crashes** on certain edge cases in MIR lowering (panics replaced with graceful error emission) ## How it works ### The core idea The type inference layer (TIR) already resolves every `x.foo` during type checking — it knows whether `foo` is a field, variant, method, or free function. But that information was thrown away. We now store it as `MemberResolution` entries and use them in the LSP to navigate. ### Preserving span data (AST + HIR) Field and variant name spans were dropped during HIR lowering. Two new source maps fix this: - **`ItemTreeSourceMap`** (`item_tree.rs`, `builder.rs`, `semantic_index.rs`, `lib.rs`) — stores class field spans, enum variant spans, and function name spans. Follows the existing source-map pattern so it doesn't pollute Salsa cache comparisons. - **`AstSourceMap::field_access_member_spans`** (`ast.rs`, `lower_expr_body.rs`, `lower_cst.rs`) — stores the text range of just the member name token (after the dot) for each `FieldAccess` expression. Used for narrowing diagnostic spans. ### MemberResolution (TIR) Renamed `MethodResolution` → `MemberResolution` and added `Field` and `Variant` variants alongside the existing `Method` and `Free` (`inference.rs`). The TIR builder (`builder.rs`) now inserts these during field lookup, variant validation, and method resolution. Added `resolve_class_loc`/`resolve_enum_loc` helpers. ### Narrower diagnostic spans (TIR) Added `DiagnosticLocation::ExprMember` (`infer_context.rs`) — at render time it looks up the member-name span instead of the full expression span. All 9 `UnresolvedMember` diagnostic sites in `builder.rs` now use this, so `s.feelin` only underlines `feelin`. ### MIR safety The new `MemberResolution::Field`/`Variant` variants flow into MIR lowering. Instead of crashing on unexpected variants (`unreachable!`, `panic!`), the MIR now emits runtime panic instructions and continues (`lower.rs`). This keeps the LSP alive. ### LSP goto-definition (`definition.rs`) When `resolve_name_at` returns `Unknown` (which happens for all dotted access), we now fall back to `resolve_member_at`, which: - If cursor is inside a class/enum scope: navigates to the field/variant definition directly - If cursor is inside a function: finds the enclosing function, gets its `ScopeInference`, and searches for the expression at the cursor offset. Then dispatches based on `MemberResolution` type (field → field def span, variant → variant def span, method → function name span) or checks for constructor field keys. ### LSP find-references (`usages.rs`) When on a class field definition, `find_field_definition_usages` scans all files in the package. For each function body, it checks `ScopeInference` for `MemberResolution::Field` matches and also scans `Object` constructor field keys. Uses a text pre-filter (field name appears in source) to skip files that can't possibly match. ### LSP outline (`outline.rs`) Replaced zero-width placeholder spans with real field/variant spans from `ItemTreeSourceMap`. ### Tests - Flipped 5 goto-def tests and 1 find-refs test from negative to positive assertions (`definition_at_tests.rs`, `usages_at_tests.rs`) - Added 3 new diagnostic span tests: `unresolved_field`, `unresolved_field_chained_access`, `unresolved_field_span_should_narrow_to_member` (`inference.rs`) - Updated 4 inline snapshot byte offsets for narrower spans (`phase3a.rs`) - Updated 4 file snapshots (`control_flow` and `format_checks` TIR + diagnostics) ### Deleted old LSP v1 crates - **`crates/baml_lsp_actions/`** — 15 source files, fully superseded by `baml_lsp2_actions` - **`crates/baml_lsp_actions_tests/`** — 7 source files + 281 test fixtures - Removed from `Cargo.toml` workspace members, updated `Cargo.lock` ## Deviations from plan All 5 planned phases were implemented. Notable additions beyond the plan: - **Diagnostic span narrowing** (`AstSourceMap::field_access_member_spans` + `DiagnosticLocation::ExprMember`) was not in the plan — discovered the wide-span issue during implementation - **`function_name_spans`** in `ItemTreeSourceMap` was needed because methods aren't top-level symbols, so the planned `utils::definition_span` approach didn't work - **MIR panic safety** — plan used `unreachable!` for the new variants, we made it graceful instead since it runs in the LSP - **`resolve_enum_loc`** needed more logic than planned for cross-package namespace lookups ## How to verify ### Manual - [ ] Ctrl-click `p.name` → jumps to field definition - [ ] Ctrl-click `Status.Active` → jumps to variant definition - [ ] Ctrl-click `name` in `Person { name: "..." }` → jumps to field definition - [ ] Ctrl-click `s.Celebrate()` → jumps to method definition - [ ] Right-click field definition → Find All References → finds all usages - [ ] Type `s.feelin` → squiggly only on `feelin` ### Automated ```bash cargo test -p baml_lsp2_actions # 26 tests cargo test -p baml_tests --lib # 889 tests ``` ## Changelog Fix LSP goto-definition and find-references for class fields, enum variants, constructor fields, and methods. Narrow unresolved-member diagnostic spans to just the member name. Remove panics from MIR lowering for LSP stability. Delete old LSP v1 crates. --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 6a2b167 commit ccd9110

333 files changed

Lines changed: 1157 additions & 20959 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

baml_language/Cargo.lock

Lines changed: 0 additions & 27 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

baml_language/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ sys_types = { path = "crates/sys_types" }
4949
sys_native = { path = "crates/sys_native" }
5050
baml_db = { path = "crates/baml_db" }
5151
baml_fmt = { path = "crates/baml_fmt" }
52-
baml_lsp_actions = { path = "crates/baml_lsp_actions" }
5352
baml_lsp2_actions = { path = "crates/baml_lsp2_actions" }
5453
baml_lsp_server = { path = "crates/baml_lsp_server" }
5554
baml_lsp_types = { path = "crates/baml_lsp_types" }

baml_language/crates/baml_compiler2_ast/src/ast.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
//! CST `Option` handling in one layer so everything downstream gets clean
77
//! typed data and can be constructed directly in tests without parsing.
88
9+
use std::collections::HashMap;
10+
911
use baml_base::Name;
1012
use la_arena::{Arena, Idx};
1113
use text_size::TextRange;
@@ -225,6 +227,8 @@ pub struct AstSourceMap {
225227
pub match_arm_spans: Arena<TextRange>,
226228
pub type_annotation_spans: Arena<TextRange>,
227229
pub catch_arm_spans: Arena<TextRange>,
230+
/// For `FieldAccess` expressions, the span of just the member name (after the dot).
231+
pub field_access_member_spans: HashMap<ExprId, TextRange>,
228232
}
229233

230234
impl AstSourceMap {
@@ -236,6 +240,7 @@ impl AstSourceMap {
236240
match_arm_spans: Arena::new(),
237241
type_annotation_spans: Arena::new(),
238242
catch_arm_spans: Arena::new(),
243+
field_access_member_spans: HashMap::new(),
239244
}
240245
}
241246

@@ -262,6 +267,15 @@ impl AstSourceMap {
262267
.unwrap_or_default()
263268
}
264269

270+
/// Look up the member-name span for a `FieldAccess` expression.
271+
/// Returns the full expression span as fallback if no member span was recorded.
272+
pub fn field_access_member_span(&self, id: ExprId) -> TextRange {
273+
self.field_access_member_spans
274+
.get(&id)
275+
.copied()
276+
.unwrap_or_else(|| self.expr_span(id))
277+
}
278+
265279
/// Look up the source span of a pattern by its `PatId`.
266280
pub fn pattern_span(&self, id: PatId) -> TextRange {
267281
let raw: u32 = id.into_raw().into_u32();

baml_language/crates/baml_compiler2_ast/src/lower_cst.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,7 @@ pub(crate) fn synthesize_llm_builtin_call(
348348
match_arm_spans: Arena::new(),
349349
type_annotation_spans: Arena::new(),
350350
catch_arm_spans: Arena::new(),
351+
field_access_member_spans: std::collections::HashMap::new(),
351352
};
352353

353354
(body, source_map)
@@ -617,6 +618,7 @@ fn synthesize_retry_policy_let(node: &SyntaxNode) -> Option<Item> {
617618
match_arm_spans: la_arena::Arena::new(),
618619
type_annotation_spans: la_arena::Arena::new(),
619620
catch_arm_spans: la_arena::Arena::new(),
621+
field_access_member_spans: std::collections::HashMap::new(),
620622
};
621623

622624
Some(Item::Let(LetDef {
@@ -831,6 +833,7 @@ fn synthesize_client_let(
831833
match_arm_spans: la_arena::Arena::new(),
832834
type_annotation_spans: la_arena::Arena::new(),
833835
catch_arm_spans: la_arena::Arena::new(),
836+
field_access_member_spans: std::collections::HashMap::new(),
834837
};
835838

836839
Item::Let(LetDef {
@@ -1046,6 +1049,7 @@ fn synthesize_client_new_companion(
10461049
match_arm_spans: la_arena::Arena::new(),
10471050
type_annotation_spans: la_arena::Arena::new(),
10481051
catch_arm_spans: la_arena::Arena::new(),
1052+
field_access_member_spans: std::collections::HashMap::new(),
10491053
};
10501054

10511055
let func_name = format!("{client_name}$new");

baml_language/crates/baml_compiler2_ast/src/lower_expr_body.rs

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1319,12 +1319,12 @@ impl LoweringContext {
13191319

13201320
fn lower_path_expr(&mut self, node: &SyntaxNode) -> ExprId {
13211321
// PATH_EXPR contains WORD (or keyword-as-ident) tokens joined by DOTs.
1322-
let mut segments = Vec::new();
1322+
let mut segments: Vec<(Name, TextRange)> = Vec::new();
13231323

13241324
for elem in node.children_with_tokens() {
13251325
if let rowan::NodeOrToken::Token(token) = elem {
13261326
if is_ident_token(token.kind()) {
1327-
segments.push(Name::new(token.text()));
1327+
segments.push((Name::new(token.text()), token.text_range()));
13281328
}
13291329
}
13301330
}
@@ -1335,7 +1335,7 @@ impl LoweringContext {
13351335

13361336
// Check if single segment is a literal keyword
13371337
if segments.len() == 1 {
1338-
match segments[0].as_str() {
1338+
match segments[0].0.as_str() {
13391339
"true" => {
13401340
return self.alloc_expr(Expr::Literal(Literal::Bool(true)), node.text_range());
13411341
}
@@ -1351,22 +1351,27 @@ impl LoweringContext {
13511351
// Color.Red → FieldAccess { base: Path(["Color"]), field: "Red" }
13521352
// a.b.c → FieldAccess { base: FieldAccess { base: Path(["a"]), field: "b" }, field: "c" }
13531353
// After this, Path is always single-segment (a bare identifier).
1354-
let mut base = self.alloc_expr(Expr::Path(vec![segments[0].clone()]), node.text_range());
1355-
for seg in &segments[1..] {
1356-
base = self.alloc_expr(
1354+
let mut base = self.alloc_expr(Expr::Path(vec![segments[0].0.clone()]), node.text_range());
1355+
for (seg, seg_range) in &segments[1..] {
1356+
let id = self.alloc_expr(
13571357
Expr::FieldAccess {
13581358
base,
13591359
field: seg.clone(),
13601360
},
13611361
node.text_range(),
13621362
);
1363+
self.source_map
1364+
.field_access_member_spans
1365+
.insert(id, *seg_range);
1366+
base = id;
13631367
}
13641368
base
13651369
}
13661370

13671371
fn lower_field_access_expr(&mut self, node: &SyntaxNode) -> ExprId {
13681372
let mut base = None;
13691373
let mut field = None;
1374+
let mut field_range = None;
13701375

13711376
for elem in node.children_with_tokens() {
13721377
match elem {
@@ -1378,6 +1383,7 @@ impl LoweringContext {
13781383
rowan::NodeOrToken::Token(token) => {
13791384
if is_ident_token(token.kind()) && base.is_some() {
13801385
field = Some(Name::new(token.text()));
1386+
field_range = Some(token.text_range());
13811387
}
13821388
}
13831389
}
@@ -1386,7 +1392,11 @@ impl LoweringContext {
13861392
let base = base.unwrap_or_else(|| self.alloc_expr(Expr::Missing, node.text_range()));
13871393
let field = field.unwrap_or_else(|| Name::new("_"));
13881394

1389-
self.alloc_expr(Expr::FieldAccess { base, field }, node.text_range())
1395+
let id = self.alloc_expr(Expr::FieldAccess { base, field }, node.text_range());
1396+
if let Some(range) = field_range {
1397+
self.source_map.field_access_member_spans.insert(id, range);
1398+
}
1399+
id
13901400
}
13911401

13921402
fn lower_env_access_expr(&mut self, node: &SyntaxNode) -> ExprId {

baml_language/crates/baml_compiler2_hir/src/builder.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ pub struct SemanticIndexBuilder<'db> {
4444
expr_scopes: Vec<(ast::ExprId, FileScopeId)>,
4545

4646
item_tree: ItemTree,
47+
item_tree_source_map: crate::item_tree::ItemTreeSourceMap,
4748
type_contributions: Vec<(Name, Contribution<'db>)>,
4849
value_contributions: Vec<(Name, Contribution<'db>)>,
4950
diagnostics: Vec<Hir2Diagnostic>,
@@ -60,6 +61,7 @@ impl<'db> SemanticIndexBuilder<'db> {
6061
class_depth: 0,
6162
expr_scopes: Vec::new(),
6263
item_tree: ItemTree::new(),
64+
item_tree_source_map: crate::item_tree::ItemTreeSourceMap::default(),
6365
type_contributions: Vec::new(),
6466
value_contributions: Vec::new(),
6567
diagnostics: Vec::new(),
@@ -129,6 +131,7 @@ impl<'db> SemanticIndexBuilder<'db> {
129131
scope_bindings: self.scope_bindings,
130132
scope_ids,
131133
item_tree: Arc::new(self.item_tree),
134+
item_tree_source_map: Arc::new(self.item_tree_source_map),
132135
symbol_contributions: Arc::new(FileSymbolContributions {
133136
types: self.type_contributions,
134137
values: self.value_contributions,
@@ -349,6 +352,7 @@ impl<'db> SemanticIndexBuilder<'db> {
349352

350353
fn lower_function(&mut self, f: &ast::FunctionDef) -> LocalItemId<FunctionMarker> {
351354
let local_id = self.item_tree.alloc_function(f);
355+
ItemTree::collect_function_span(&mut self.item_tree_source_map, local_id, f);
352356
let loc = FunctionLoc::new(self.db, self.file, local_id);
353357

354358
// Only contribute as a top-level symbol if not inside a class.
@@ -382,6 +386,7 @@ impl<'db> SemanticIndexBuilder<'db> {
382386

383387
fn lower_class(&mut self, c: &ast::ClassDef) {
384388
let local_id = self.item_tree.alloc_class(c);
389+
ItemTree::collect_class_spans(&mut self.item_tree_source_map, local_id, c);
385390
let loc = ClassLoc::new(self.db, self.file, local_id);
386391
self.type_contributions.push((
387392
c.name.clone(),
@@ -428,6 +433,7 @@ impl<'db> SemanticIndexBuilder<'db> {
428433

429434
fn lower_enum(&mut self, e: &ast::EnumDef) {
430435
let local_id = self.item_tree.alloc_enum(e);
436+
ItemTree::collect_enum_spans(&mut self.item_tree_source_map, local_id, e);
431437
let loc = EnumLoc::new(self.db, self.file, local_id);
432438
self.type_contributions.push((
433439
e.name.clone(),

baml_language/crates/baml_compiler2_hir/src/item_tree.rs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,24 @@ pub struct Let {
178178
pub name_span: TextRange,
179179
}
180180

181+
// ── ItemTreeSourceMap ─────────────────────────────────────────────────────────
182+
183+
/// Parallel source map for `ItemTree` — stores name spans that are
184+
/// deliberately excluded from the semantic `ItemTree` to avoid polluting
185+
/// Salsa's early-cutoff comparisons with position data.
186+
///
187+
/// Follows the same body/signature source-map pattern used by
188+
/// `function_body` / `function_body_source_map`.
189+
#[derive(Debug, Clone, Default)]
190+
pub struct ItemTreeSourceMap {
191+
/// `name_span` for each class's fields, parallel to `Class::fields`.
192+
pub class_field_spans: FxHashMap<LocalItemId<ClassMarker>, Vec<TextRange>>,
193+
/// `name_span` for each enum's variants, parallel to `Enum::variants`.
194+
pub enum_variant_spans: FxHashMap<LocalItemId<EnumMarker>, Vec<TextRange>>,
195+
/// `name_span` for each function.
196+
pub function_name_spans: FxHashMap<LocalItemId<FunctionMarker>, TextRange>,
197+
}
198+
181199
// ── ItemTree ─────────────────────────────────────────────────────────────────
182200

183201
/// Position-independent item storage for a single file.
@@ -315,6 +333,37 @@ impl ItemTree {
315333
id
316334
}
317335

336+
/// Populate source map spans for a class that was allocated via `alloc_class`.
337+
pub fn collect_class_spans(
338+
source_map: &mut ItemTreeSourceMap,
339+
id: LocalItemId<ClassMarker>,
340+
class_def: &ast::ClassDef,
341+
) {
342+
let spans: Vec<TextRange> = class_def.fields.iter().map(|f| f.name_span).collect();
343+
source_map.class_field_spans.insert(id, spans);
344+
}
345+
346+
/// Populate source map name span for a function.
347+
pub fn collect_function_span(
348+
source_map: &mut ItemTreeSourceMap,
349+
id: LocalItemId<FunctionMarker>,
350+
func_def: &ast::FunctionDef,
351+
) {
352+
source_map
353+
.function_name_spans
354+
.insert(id, func_def.name_span);
355+
}
356+
357+
/// Populate source map spans for an enum that was allocated via `alloc_enum`.
358+
pub fn collect_enum_spans(
359+
source_map: &mut ItemTreeSourceMap,
360+
id: LocalItemId<EnumMarker>,
361+
enum_def: &ast::EnumDef,
362+
) {
363+
let spans: Vec<TextRange> = enum_def.variants.iter().map(|v| v.name_span).collect();
364+
source_map.enum_variant_spans.insert(id, spans);
365+
}
366+
318367
pub fn alloc_type_alias(&mut self, ta: &ast::TypeAliasDef) -> LocalItemId<TypeAliasMarker> {
319368
let id = self.alloc_id(ItemKind::TypeAlias, &ta.name);
320369
self.type_aliases.insert(

baml_language/crates/baml_compiler2_hir/src/lib.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ pub use builder::SemanticIndexBuilder;
3333

3434
use crate::{
3535
contributions::FileSymbolContributions,
36-
item_tree::ItemTree,
36+
item_tree::{ItemTree, ItemTreeSourceMap},
3737
semantic_index::{FileSemanticIndex, ScopeBindings},
3838
};
3939

@@ -128,6 +128,14 @@ pub fn file_item_tree(db: &dyn Db, file: SourceFile) -> Arc<ItemTree> {
128128
Arc::clone(&index.item_tree)
129129
}
130130

131+
/// Returns the item tree source map for a file (clones the Arc — O(1)).
132+
///
133+
/// Not tracked — the source map is cached via `file_semantic_index`.
134+
pub fn file_item_tree_source_map(db: &dyn Db, file: SourceFile) -> Arc<ItemTreeSourceMap> {
135+
let index = file_semantic_index(db, file);
136+
Arc::clone(&index.item_tree_source_map)
137+
}
138+
131139
/// Returns the `ScopeBindings` for a given scope.
132140
///
133141
/// Not tracked — callers use the pre-interned `ScopeId` to look up bindings.

baml_language/crates/baml_compiler2_hir/src/semantic_index.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use text_size::{TextRange, TextSize};
1313
use crate::{
1414
contributions::FileSymbolContributions,
1515
diagnostic::Hir2Diagnostic,
16-
item_tree::ItemTree,
16+
item_tree::{ItemTree, ItemTreeSourceMap},
1717
scope::{FileScopeId, Scope, ScopeId},
1818
};
1919

@@ -100,6 +100,9 @@ pub struct FileSemanticIndex<'db> {
100100
/// Per-file item tree — maps `LocalItemId` to item data.
101101
pub item_tree: Arc<ItemTree>,
102102

103+
/// Source map for item tree — field/variant name spans.
104+
pub item_tree_source_map: Arc<ItemTreeSourceMap>,
105+
103106
/// Names this file contributes to its package namespace.
104107
pub symbol_contributions: Arc<FileSymbolContributions<'db>>,
105108

0 commit comments

Comments
 (0)