Skip to content

Commit 5f5d4d2

Browse files
committed
fix: class-scope field annotation typeMap keys to prevent cross-class collision
Field type annotations (`private repo: OrderRepository`) were seeded as bare file-wide typeMap keys, causing `this.repo` inside `UserService` to resolve to `OrderRepository` when both classes had a `repo` field (issue #1458). Both extractors (TS `handleFieldDefTypeMap` and Rust `field_definition` branch) now seed `ClassName.field` keys at confidence 0.9, matching the `CallerClass.X` resolver fallback added in PR #1382. Bare keys are kept at confidence 0.6 as fallbacks for single-class files or class expressions where no enclosing class name is available. Both engines change identically — parity preserved.
1 parent 61a9839 commit 5f5d4d2

3 files changed

Lines changed: 85 additions & 16 deletions

File tree

crates/codegraph-core/src/extractors/javascript.rs

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -202,8 +202,13 @@ fn match_js_type_map(node: &Node, source: &[u8], symbols: &mut FileSymbols, _dep
202202
}
203203
}
204204
// TypeScript class field declarations: `private repo: Repository<User>`
205-
// Seeds both "repo" and "this.repo" so that `this.repo.method()` calls
206-
// can be resolved to the interface/class type via the type map.
205+
// Seeds a class-scoped key `ClassName.field` (confidence 0.9) as the primary
206+
// entry so that two classes with identically-named fields don't overwrite each
207+
// other's typeMap entry (issue #1458). The resolver's `CallerClass.X` fallback
208+
// looks up exactly this key.
209+
// Bare `field` and `this.field` keys are kept at lower confidence (0.6) as
210+
// fallbacks for single-class files where the resolver may lack callerClass context.
211+
// Mirrors handleFieldDefTypeMap in src/extractors/javascript.ts.
207212
"public_field_definition" | "field_definition" => {
208213
let name_node = node.child_by_field_name("name")
209214
.or_else(|| node.child_by_field_name("property"))
@@ -216,9 +221,33 @@ fn match_js_type_map(node: &Node, source: &[u8], symbols: &mut FileSymbols, _dep
216221
let field_name = node_text(&name_node, source).to_string();
217222
if let Some(type_anno) = find_child(node, "type_annotation") {
218223
if let Some(type_name) = extract_simple_type_name(&type_anno, source) {
219-
push_type_map_entry(symbols, field_name.clone(), type_name.to_string());
220-
// "this.fieldName" key resolves `this.repo.method()` calls.
221-
push_type_map_entry(symbols, format!("this.{}", field_name), type_name.to_string());
224+
match enclosing_type_map_class(node, source) {
225+
Some(class_name) => {
226+
// Primary: class-scoped key prevents cross-class collision.
227+
push_type_map_entry(
228+
symbols,
229+
format!("{}.{}", class_name, field_name),
230+
type_name.to_string(),
231+
);
232+
// Fallback bare keys at lower confidence.
233+
symbols.type_map.push(TypeMapEntry {
234+
name: field_name.clone(),
235+
type_name: type_name.to_string(),
236+
confidence: 0.6,
237+
});
238+
symbols.type_map.push(TypeMapEntry {
239+
name: format!("this.{}", field_name),
240+
type_name: type_name.to_string(),
241+
confidence: 0.6,
242+
});
243+
}
244+
None => {
245+
// No enclosing class declaration (e.g. class expression)
246+
// — use bare keys only at full confidence.
247+
push_type_map_entry(symbols, field_name.clone(), type_name.to_string());
248+
push_type_map_entry(symbols, format!("this.{}", field_name), type_name.to_string());
249+
}
250+
}
222251
}
223252
}
224253
}

src/extractors/javascript.ts

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1867,7 +1867,7 @@ function runContextCollectorWalk(rootNode: TreeSitterNode, out: ContextCollector
18671867
} else if (t === 'required_parameter' || t === 'optional_parameter') {
18681868
handleParamTypeMap(node, out.typeMap);
18691869
} else if (t === 'public_field_definition' || t === 'field_definition') {
1870-
handleFieldDefTypeMap(node, out.typeMap);
1870+
handleFieldDefTypeMap(node, out.typeMap, typeMapClass);
18711871
} else if (t === 'assignment_expression') {
18721872
handlePropWriteTypeMap(node, out.typeMap, typeMapClass);
18731873
} else if (t === 'call_expression') {
@@ -2094,11 +2094,23 @@ function handleParamTypeMap(node: TreeSitterNode, typeMap: Map<string, TypeMapEn
20942094

20952095
/**
20962096
* Extract type info from a class field declaration: `private repo: Repository<User>`.
2097-
* Seeds both "repo" and "this.repo" so `this.repo.method()` calls resolve to the
2098-
* declared type via the type map. Mirrors the field_definition branch of
2099-
* match_js_type_map in crates/codegraph-core/src/extractors/javascript.rs.
2097+
*
2098+
* Seeds a class-scoped key `ClassName.field` (confidence 0.9) as the primary entry
2099+
* so that two classes with identically-named fields don't overwrite each other's
2100+
* typeMap entry (issue #1458). The resolver's `CallerClass.X` fallback (call-resolver.ts
2101+
* line 110) looks up exactly this key.
2102+
*
2103+
* Bare `field` and `this.field` keys are kept at lower confidence (0.6) as fallbacks
2104+
* for single-class files where the resolver may not have a callerClass context.
2105+
*
2106+
* Mirrors the field_definition branch of match_js_type_map in
2107+
* crates/codegraph-core/src/extractors/javascript.rs.
21002108
*/
2101-
function handleFieldDefTypeMap(node: TreeSitterNode, typeMap: Map<string, TypeMapEntry>): void {
2109+
function handleFieldDefTypeMap(
2110+
node: TreeSitterNode,
2111+
typeMap: Map<string, TypeMapEntry>,
2112+
currentClass: string | null,
2113+
): void {
21022114
const nameNode =
21032115
node.childForFieldName('name') ||
21042116
node.childForFieldName('property') ||
@@ -2115,9 +2127,18 @@ function handleFieldDefTypeMap(node: TreeSitterNode, typeMap: Map<string, TypeMa
21152127
if (!typeAnno) return;
21162128
const typeName = extractSimpleTypeName(typeAnno);
21172129
if (!typeName) return;
2118-
setTypeMapEntry(typeMap, nameNode.text, typeName, 0.9);
2119-
// "this.fieldName" key resolves `this.repo.method()` calls.
2120-
setTypeMapEntry(typeMap, `this.${nameNode.text}`, typeName, 0.9);
2130+
if (currentClass) {
2131+
// Primary: class-scoped key prevents cross-class collision (issue #1458).
2132+
setTypeMapEntry(typeMap, `${currentClass}.${nameNode.text}`, typeName, 0.9);
2133+
// Fallback: bare keys at lower confidence for single-class files or when
2134+
// the resolver does not have a callerClass in scope.
2135+
setTypeMapEntry(typeMap, nameNode.text, typeName, 0.6);
2136+
setTypeMapEntry(typeMap, `this.${nameNode.text}`, typeName, 0.6);
2137+
} else {
2138+
// No enclosing class declaration (e.g. class expression) — use bare keys only.
2139+
setTypeMapEntry(typeMap, nameNode.text, typeName, 0.9);
2140+
setTypeMapEntry(typeMap, `this.${nameNode.text}`, typeName, 0.9);
2141+
}
21212142
}
21222143

21232144
/**

tests/parsers/javascript.test.ts

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -217,15 +217,34 @@ describe('JavaScript parser', () => {
217217
expect(symbols.typeMap.get('res')).toEqual({ type: 'Response', confidence: 0.9 });
218218
});
219219

220-
it('extracts class field annotations into typeMap with confidence 0.9', () => {
220+
it('extracts class field annotations into class-scoped typeMap key (issue #1458)', () => {
221221
const symbols = parseTS(`
222222
class UserService {
223223
private repo: Repository;
224224
run() { this.repo.save(); }
225225
}
226226
`);
227-
expect(symbols.typeMap.get('repo')).toEqual({ type: 'Repository', confidence: 0.9 });
228-
expect(symbols.typeMap.get('this.repo')).toEqual({ type: 'Repository', confidence: 0.9 });
227+
// Primary: class-scoped key at 0.9 — prevents cross-class collision.
228+
expect(symbols.typeMap.get('UserService.repo')).toEqual({ type: 'Repository', confidence: 0.9 });
229+
// Fallback bare keys at lower confidence for single-class files.
230+
expect(symbols.typeMap.get('repo')).toEqual({ type: 'Repository', confidence: 0.6 });
231+
expect(symbols.typeMap.get('this.repo')).toEqual({ type: 'Repository', confidence: 0.6 });
232+
});
233+
234+
it('prevents cross-class collision for same-named fields (issue #1458)', () => {
235+
const symbols = parseTS(`
236+
class OrderService {
237+
private repo: OrderRepository;
238+
}
239+
class UserService {
240+
private repo: UserRepository;
241+
}
242+
`);
243+
// Each class gets its own scoped key — no collision.
244+
expect(symbols.typeMap.get('OrderService.repo')).toEqual({ type: 'OrderRepository', confidence: 0.9 });
245+
expect(symbols.typeMap.get('UserService.repo')).toEqual({ type: 'UserRepository', confidence: 0.9 });
246+
// Bare "repo" key should hold the first class's type at 0.6 (second write is same confidence, no overwrite).
247+
expect(symbols.typeMap.get('repo')?.confidence).toBe(0.6);
229248
});
230249

231250
it('returns empty typeMap when no annotations', () => {

0 commit comments

Comments
 (0)