diff --git a/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs b/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs index 39108e3d..801532ed 100644 --- a/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs +++ b/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs @@ -713,9 +713,10 @@ fn resolve_call_targets<'a>( // Phase 8.3f: callee-scoped rest-param key (`callee::restName`) avoids // same-name rest-binding collisions across functions in the same file (#1358). let rest_param_key = format!("{}::{}", caller_name, effective_receiver); - // Class-scoped key (`ClassName.prop`) seeded by `this.prop = new Ctor()` - // property writes — prevents false edges when multiple classes define the - // same property name (issue #1323). Only consulted for `this.` receivers. + // Class-scoped key (`ClassName.prop`) seeded by `this.prop = new Ctor()` and + // field annotations — prevents false edges when multiple classes define the same + // property name (issues #1323, #1458). Consulted first for `this.` receivers so + // bare fallback keys (confidence 0.6) don't shadow the correct per-class entry. let class_scoped_key = if receiver.starts_with("this.") && !caller_name.is_empty() { caller_name .rfind('.') @@ -723,10 +724,10 @@ fn resolve_call_targets<'a>( } else { None }; - let type_lookup = type_map.get(effective_receiver) + let type_lookup = class_scoped_key.as_deref().and_then(|k| type_map.get(k)) + .or_else(|| type_map.get(effective_receiver)) .or_else(|| type_map.get(receiver.as_str())) - .or_else(|| if caller_name.is_empty() { None } else { type_map.get(rest_param_key.as_str()) }) - .or_else(|| class_scoped_key.as_deref().and_then(|k| type_map.get(k))); + .or_else(|| if caller_name.is_empty() { None } else { type_map.get(rest_param_key.as_str()) }); // Inline new-expression receiver: `(new Foo).bar()` — extract the constructor name // when no typeMap entry exists for the complex receiver expression. // Mirrors the regex `/^\(?\s*new\s+([A-Z_$][A-Za-z0-9_$]*)/` in call-resolver.ts. diff --git a/crates/codegraph-core/src/extractors/javascript.rs b/crates/codegraph-core/src/extractors/javascript.rs index 2c00a615..bcebe43b 100644 --- a/crates/codegraph-core/src/extractors/javascript.rs +++ b/crates/codegraph-core/src/extractors/javascript.rs @@ -202,8 +202,13 @@ fn match_js_type_map(node: &Node, source: &[u8], symbols: &mut FileSymbols, _dep } } // TypeScript class field declarations: `private repo: Repository` - // Seeds both "repo" and "this.repo" so that `this.repo.method()` calls - // can be resolved to the interface/class type via the type map. + // Seeds a class-scoped key `ClassName.field` (confidence 0.9) as the primary + // entry so that two classes with identically-named fields don't overwrite each + // other's typeMap entry (issue #1458). The resolver's `CallerClass.X` fallback + // looks up exactly this key. + // Bare `field` and `this.field` keys are kept at lower confidence (0.6) as + // fallbacks for single-class files where the resolver may lack callerClass context. + // Mirrors handleFieldDefTypeMap in src/extractors/javascript.ts. "public_field_definition" | "field_definition" => { let name_node = node.child_by_field_name("name") .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 let field_name = node_text(&name_node, source).to_string(); if let Some(type_anno) = find_child(node, "type_annotation") { if let Some(type_name) = extract_simple_type_name(&type_anno, source) { - push_type_map_entry(symbols, field_name.clone(), type_name.to_string()); - // "this.fieldName" key resolves `this.repo.method()` calls. - push_type_map_entry(symbols, format!("this.{}", field_name), type_name.to_string()); + match enclosing_type_map_class(node, source) { + Some(class_name) => { + // Primary: class-scoped key prevents cross-class collision. + push_type_map_entry( + symbols, + format!("{}.{}", class_name, field_name), + type_name.to_string(), + ); + // Fallback bare keys at lower confidence. + symbols.type_map.push(TypeMapEntry { + name: field_name.clone(), + type_name: type_name.to_string(), + confidence: 0.6, + }); + symbols.type_map.push(TypeMapEntry { + name: format!("this.{}", field_name), + type_name: type_name.to_string(), + confidence: 0.6, + }); + } + None => { + // No enclosing class declaration (e.g. class expression) + // — use bare keys only at full confidence. + push_type_map_entry(symbols, field_name.clone(), type_name.to_string()); + push_type_map_entry(symbols, format!("this.{}", field_name), type_name.to_string()); + } + } } } } @@ -2946,6 +2975,15 @@ mod tests { JsExtractor.extract(&tree, code.as_bytes(), "test.js") } + fn parse_ts(code: &str) -> FileSymbols { + let mut parser = Parser::new(); + parser + .set_language(&tree_sitter_typescript::LANGUAGE_TYPESCRIPT.into()) + .unwrap(); + let tree = parser.parse(code.as_bytes(), None).unwrap(); + JsExtractor.extract(&tree, code.as_bytes(), "test.ts") + } + #[test] fn finds_function_declaration() { let s = parse_js("function greet(name) { return name; }"); @@ -3569,6 +3607,38 @@ mod tests { assert_eq!(tm.unwrap().type_name, "HttpClient"); } + /// Issue #1458: two classes with identically-named field annotations must + /// produce separate class-scoped typeMap keys, not overwrite each other. + /// Mirrors the TS `prevents cross-class collision` test. + #[test] + fn field_annotation_multi_class_seeds_separate_scoped_keys() { + let s = parse_ts( + "class OrderService {\n\ + private repo: OrderRepository;\n\ + }\n\ + class UserService {\n\ + private repo: UserRepository;\n\ + }", + ); + let order_entry = s.type_map.iter().find(|t| t.name == "OrderService.repo"); + assert!( + order_entry.is_some(), + "type_map should contain 'OrderService.repo'; got: {:?}", + s.type_map.iter().map(|e| &e.name).collect::>() + ); + assert_eq!(order_entry.unwrap().type_name, "OrderRepository"); + assert_eq!(order_entry.unwrap().confidence, 0.9); + + let user_entry = s.type_map.iter().find(|t| t.name == "UserService.repo"); + assert!( + user_entry.is_some(), + "type_map should contain 'UserService.repo'; got: {:?}", + s.type_map.iter().map(|e| &e.name).collect::>() + ); + assert_eq!(user_entry.unwrap().type_name, "UserRepository"); + assert_eq!(user_entry.unwrap().confidence, 0.9); + } + /// Issue #1453 (edge 4): `const f = fn.bind(ctx)` must record a /// fnRefBinding f → fn so later `f()` calls resolve through pts. #[test] diff --git a/src/domain/graph/builder/call-resolver.ts b/src/domain/graph/builder/call-resolver.ts index 85cc7767..22f025d3 100644 --- a/src/domain/graph/builder/call-resolver.ts +++ b/src/domain/graph/builder/call-resolver.ts @@ -94,22 +94,25 @@ export function resolveByMethodOrGlobal( const effectiveReceiver = call.receiver.startsWith('this.') ? call.receiver.slice('this.'.length) : call.receiver; - // For this.prop receivers, also try the class-scoped key (ClassName.prop) seeded by - // handlePropWriteTypeMap — prevents false edges when multiple classes define the same - // property name (issue #1323). - let typeEntry = - typeMap.get(effectiveReceiver) ?? - typeMap.get(call.receiver) ?? - // Phase 8.3f: callee-scoped rest-param key (`callee::restName`) to avoid - // same-name rest-binding collision across functions in the same file (#1358). - (callerName ? typeMap.get(`${callerName}::${effectiveReceiver}`) : undefined); - if (!typeEntry && call.receiver.startsWith('this.') && callerName) { + // For this.prop receivers, prefer the class-scoped key (ClassName.prop) seeded by + // handlePropWriteTypeMap / handleFieldDefTypeMap — prevents false edges when multiple + // classes define the same property name (issues #1323, #1458). + // Class-scoped lookup runs first so bare fallback keys (confidence 0.6) don't shadow + // the correct per-class entry when callerName is available. + let typeEntry: unknown; + if (call.receiver.startsWith('this.') && callerName) { const dotIdx = callerName.lastIndexOf('.'); if (dotIdx > -1) { const callerClass = callerName.slice(0, dotIdx); typeEntry = typeMap.get(`${callerClass}.${effectiveReceiver}`); } } + typeEntry ??= + typeMap.get(effectiveReceiver) ?? + typeMap.get(call.receiver) ?? + // Phase 8.3f: callee-scoped rest-param key (`callee::restName`) to avoid + // same-name rest-binding collision across functions in the same file (#1358). + (callerName ? typeMap.get(`${callerName}::${effectiveReceiver}`) : undefined); let typeName = typeEntry ? typeof typeEntry === 'string' ? typeEntry diff --git a/src/extractors/javascript.ts b/src/extractors/javascript.ts index 2be0ac4b..c4103e51 100644 --- a/src/extractors/javascript.ts +++ b/src/extractors/javascript.ts @@ -1867,7 +1867,7 @@ function runContextCollectorWalk(rootNode: TreeSitterNode, out: ContextCollector } else if (t === 'required_parameter' || t === 'optional_parameter') { handleParamTypeMap(node, out.typeMap); } else if (t === 'public_field_definition' || t === 'field_definition') { - handleFieldDefTypeMap(node, out.typeMap); + handleFieldDefTypeMap(node, out.typeMap, typeMapClass); } else if (t === 'assignment_expression') { handlePropWriteTypeMap(node, out.typeMap, typeMapClass); } else if (t === 'call_expression') { @@ -2094,11 +2094,23 @@ function handleParamTypeMap(node: TreeSitterNode, typeMap: Map`. - * Seeds both "repo" and "this.repo" so `this.repo.method()` calls resolve to the - * declared type via the type map. Mirrors the field_definition branch of - * match_js_type_map in crates/codegraph-core/src/extractors/javascript.rs. + * + * Seeds a class-scoped key `ClassName.field` (confidence 0.9) as the primary entry + * so that two classes with identically-named fields don't overwrite each other's + * typeMap entry (issue #1458). The resolver's `CallerClass.X` fallback (call-resolver.ts + * line 110) looks up exactly this key. + * + * Bare `field` and `this.field` keys are kept at lower confidence (0.6) as fallbacks + * for single-class files where the resolver may not have a callerClass context. + * + * Mirrors the field_definition branch of match_js_type_map in + * crates/codegraph-core/src/extractors/javascript.rs. */ -function handleFieldDefTypeMap(node: TreeSitterNode, typeMap: Map): void { +function handleFieldDefTypeMap( + node: TreeSitterNode, + typeMap: Map, + currentClass: string | null, +): void { const nameNode = node.childForFieldName('name') || node.childForFieldName('property') || @@ -2115,9 +2127,18 @@ function handleFieldDefTypeMap(node: TreeSitterNode, typeMap: Map { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1458-')); + for (const [rel, content] of Object.entries(FIXTURE)) { + fs.writeFileSync(path.join(tmpDir, rel), content); + } + await buildGraph(tmpDir, { engine: 'wasm', incremental: false, skipRegistry: true }); +}); + +afterAll(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); +}); + +function readCallEdges(dbPath: string) { + const db = new Database(dbPath, { readonly: true }); + try { + return db + .prepare( + `SELECT n1.name AS src, n2.name AS tgt + FROM edges e + JOIN nodes n1 ON e.source_id = n1.id + JOIN nodes n2 ON e.target_id = n2.id + WHERE e.kind = 'calls' + ORDER BY n1.name, n2.name`, + ) + .all() as Array<{ src: string; tgt: string }>; + } finally { + db.close(); + } +} + +describe('cross-class field annotation typeMap collision (#1458)', () => { + it('resolves this.repo.save() inside OrderService.run to OrderRepository.save', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const edges = readCallEdges(dbPath); + const edge = edges.find( + (e) => e.src === 'OrderService.run' && e.tgt === 'OrderRepository.save', + ); + expect( + edge, + 'OrderService.run → OrderRepository.save edge missing; cross-class collision may be present', + ).toBeDefined(); + }); + + it('resolves this.repo.save() inside UserService.run to UserRepository.save', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const edges = readCallEdges(dbPath); + const edge = edges.find((e) => e.src === 'UserService.run' && e.tgt === 'UserRepository.save'); + expect( + edge, + 'UserService.run → UserRepository.save edge missing; cross-class collision may be present', + ).toBeDefined(); + }); + + it('does not emit a false edge from OrderService.run to UserRepository.save', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const edges = readCallEdges(dbPath); + const falseEdge = edges.find( + (e) => e.src === 'OrderService.run' && e.tgt === 'UserRepository.save', + ); + expect(falseEdge).toBeUndefined(); + }); + + it('does not emit a false edge from UserService.run to OrderRepository.save', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const edges = readCallEdges(dbPath); + const falseEdge = edges.find( + (e) => e.src === 'UserService.run' && e.tgt === 'OrderRepository.save', + ); + expect(falseEdge).toBeUndefined(); + }); +}); diff --git a/tests/parsers/javascript.test.ts b/tests/parsers/javascript.test.ts index 1ab62c16..9d0ec99d 100644 --- a/tests/parsers/javascript.test.ts +++ b/tests/parsers/javascript.test.ts @@ -217,15 +217,43 @@ describe('JavaScript parser', () => { expect(symbols.typeMap.get('res')).toEqual({ type: 'Response', confidence: 0.9 }); }); - it('extracts class field annotations into typeMap with confidence 0.9', () => { + it('extracts class field annotations into class-scoped typeMap key (issue #1458)', () => { const symbols = parseTS(` class UserService { private repo: Repository; run() { this.repo.save(); } } `); - expect(symbols.typeMap.get('repo')).toEqual({ type: 'Repository', confidence: 0.9 }); - expect(symbols.typeMap.get('this.repo')).toEqual({ type: 'Repository', confidence: 0.9 }); + // Primary: class-scoped key at 0.9 — prevents cross-class collision. + expect(symbols.typeMap.get('UserService.repo')).toEqual({ + type: 'Repository', + confidence: 0.9, + }); + // Fallback bare keys at lower confidence for single-class files. + expect(symbols.typeMap.get('repo')).toEqual({ type: 'Repository', confidence: 0.6 }); + expect(symbols.typeMap.get('this.repo')).toEqual({ type: 'Repository', confidence: 0.6 }); + }); + + it('prevents cross-class collision for same-named fields (issue #1458)', () => { + const symbols = parseTS(` + class OrderService { + private repo: OrderRepository; + } + class UserService { + private repo: UserRepository; + } + `); + // Each class gets its own scoped key — no collision. + expect(symbols.typeMap.get('OrderService.repo')).toEqual({ + type: 'OrderRepository', + confidence: 0.9, + }); + expect(symbols.typeMap.get('UserService.repo')).toEqual({ + type: 'UserRepository', + confidence: 0.9, + }); + // Bare "repo" key should hold the first class's type at 0.6 (second write is same confidence, no overwrite). + expect(symbols.typeMap.get('repo')?.confidence).toBe(0.6); }); it('returns empty typeMap when no annotations', () => {