Skip to content

Commit 263c39b

Browse files
committed
Merge remote-tracking branch 'origin/fix/field-typemap-class-scope-1458' into fix/v3120-regressions-1446-1447
2 parents aa25c44 + 76a4380 commit 263c39b

5 files changed

Lines changed: 173 additions & 18 deletions

File tree

crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -713,20 +713,21 @@ fn resolve_call_targets<'a>(
713713
// Phase 8.3f: callee-scoped rest-param key (`callee::restName`) avoids
714714
// same-name rest-binding collisions across functions in the same file (#1358).
715715
let rest_param_key = format!("{}::{}", caller_name, effective_receiver);
716-
// Class-scoped key (`ClassName.prop`) seeded by `this.prop = new Ctor()`
717-
// property writes — prevents false edges when multiple classes define the
718-
// same property name (issue #1323). Only consulted for `this.` receivers.
716+
// Class-scoped key (`ClassName.prop`) seeded by `this.prop = new Ctor()` and
717+
// field annotations — prevents false edges when multiple classes define the same
718+
// property name (issues #1323, #1458). Consulted first for `this.` receivers so
719+
// bare fallback keys (confidence 0.6) don't shadow the correct per-class entry.
719720
let class_scoped_key = if receiver.starts_with("this.") && !caller_name.is_empty() {
720721
caller_name
721722
.rfind('.')
722723
.map(|dot| format!("{}.{}", &caller_name[..dot], effective_receiver))
723724
} else {
724725
None
725726
};
726-
let type_lookup = type_map.get(effective_receiver)
727+
let type_lookup = class_scoped_key.as_deref().and_then(|k| type_map.get(k))
728+
.or_else(|| type_map.get(effective_receiver))
727729
.or_else(|| type_map.get(receiver.as_str()))
728-
.or_else(|| if caller_name.is_empty() { None } else { type_map.get(rest_param_key.as_str()) })
729-
.or_else(|| class_scoped_key.as_deref().and_then(|k| type_map.get(k)));
730+
.or_else(|| if caller_name.is_empty() { None } else { type_map.get(rest_param_key.as_str()) });
730731
// Inline new-expression receiver: `(new Foo).bar()` — extract the constructor name
731732
// when no typeMap entry exists for the complex receiver expression.
732733
// Mirrors the regex `/^\(?\s*new\s+([A-Z_$][A-Za-z0-9_$]*)/` in call-resolver.ts.

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2975,6 +2975,15 @@ mod tests {
29752975
JsExtractor.extract(&tree, code.as_bytes(), "test.js")
29762976
}
29772977

2978+
fn parse_ts(code: &str) -> FileSymbols {
2979+
let mut parser = Parser::new();
2980+
parser
2981+
.set_language(&tree_sitter_typescript::LANGUAGE_TYPESCRIPT.into())
2982+
.unwrap();
2983+
let tree = parser.parse(code.as_bytes(), None).unwrap();
2984+
JsExtractor.extract(&tree, code.as_bytes(), "test.ts")
2985+
}
2986+
29782987
#[test]
29792988
fn finds_function_declaration() {
29802989
let s = parse_js("function greet(name) { return name; }");
@@ -3598,6 +3607,38 @@ mod tests {
35983607
assert_eq!(tm.unwrap().type_name, "HttpClient");
35993608
}
36003609

3610+
/// Issue #1458: two classes with identically-named field annotations must
3611+
/// produce separate class-scoped typeMap keys, not overwrite each other.
3612+
/// Mirrors the TS `prevents cross-class collision` test.
3613+
#[test]
3614+
fn field_annotation_multi_class_seeds_separate_scoped_keys() {
3615+
let s = parse_ts(
3616+
"class OrderService {\n\
3617+
private repo: OrderRepository;\n\
3618+
}\n\
3619+
class UserService {\n\
3620+
private repo: UserRepository;\n\
3621+
}",
3622+
);
3623+
let order_entry = s.type_map.iter().find(|t| t.name == "OrderService.repo");
3624+
assert!(
3625+
order_entry.is_some(),
3626+
"type_map should contain 'OrderService.repo'; got: {:?}",
3627+
s.type_map.iter().map(|e| &e.name).collect::<Vec<_>>()
3628+
);
3629+
assert_eq!(order_entry.unwrap().type_name, "OrderRepository");
3630+
assert_eq!(order_entry.unwrap().confidence, 0.9);
3631+
3632+
let user_entry = s.type_map.iter().find(|t| t.name == "UserService.repo");
3633+
assert!(
3634+
user_entry.is_some(),
3635+
"type_map should contain 'UserService.repo'; got: {:?}",
3636+
s.type_map.iter().map(|e| &e.name).collect::<Vec<_>>()
3637+
);
3638+
assert_eq!(user_entry.unwrap().type_name, "UserRepository");
3639+
assert_eq!(user_entry.unwrap().confidence, 0.9);
3640+
}
3641+
36013642
/// Issue #1453 (edge 4): `const f = fn.bind(ctx)` must record a
36023643
/// fnRefBinding f → fn so later `f()` calls resolve through pts.
36033644
#[test]

src/domain/graph/builder/call-resolver.ts

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -94,22 +94,25 @@ export function resolveByMethodOrGlobal(
9494
const effectiveReceiver = call.receiver.startsWith('this.')
9595
? call.receiver.slice('this.'.length)
9696
: call.receiver;
97-
// For this.prop receivers, also try the class-scoped key (ClassName.prop) seeded by
98-
// handlePropWriteTypeMap — prevents false edges when multiple classes define the same
99-
// property name (issue #1323).
100-
let typeEntry =
101-
typeMap.get(effectiveReceiver) ??
102-
typeMap.get(call.receiver) ??
103-
// Phase 8.3f: callee-scoped rest-param key (`callee::restName`) to avoid
104-
// same-name rest-binding collision across functions in the same file (#1358).
105-
(callerName ? typeMap.get(`${callerName}::${effectiveReceiver}`) : undefined);
106-
if (!typeEntry && call.receiver.startsWith('this.') && callerName) {
97+
// For this.prop receivers, prefer the class-scoped key (ClassName.prop) seeded by
98+
// handlePropWriteTypeMap / handleFieldDefTypeMap — prevents false edges when multiple
99+
// classes define the same property name (issues #1323, #1458).
100+
// Class-scoped lookup runs first so bare fallback keys (confidence 0.6) don't shadow
101+
// the correct per-class entry when callerName is available.
102+
let typeEntry: unknown;
103+
if (call.receiver.startsWith('this.') && callerName) {
107104
const dotIdx = callerName.lastIndexOf('.');
108105
if (dotIdx > -1) {
109106
const callerClass = callerName.slice(0, dotIdx);
110107
typeEntry = typeMap.get(`${callerClass}.${effectiveReceiver}`);
111108
}
112109
}
110+
typeEntry ??=
111+
typeMap.get(effectiveReceiver) ??
112+
typeMap.get(call.receiver) ??
113+
// Phase 8.3f: callee-scoped rest-param key (`callee::restName`) to avoid
114+
// same-name rest-binding collision across functions in the same file (#1358).
115+
(callerName ? typeMap.get(`${callerName}::${effectiveReceiver}`) : undefined);
113116
let typeName = typeEntry
114117
? typeof typeEntry === 'string'
115118
? typeEntry

src/domain/parser.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1253,8 +1253,7 @@ async function parseFilesWasmInline(
12531253
/**
12541254
* Backfill helper: small batches use the inline (main-thread) path; larger
12551255
* batches keep the worker-pool isolation against tree-sitter WASM crashes
1256-
* (#965). Threshold matches typical engine-parity drop sizes (a few fixture
1257-
* files in one or two languages).
1256+
* (#965). See INLINE_BACKFILL_THRESHOLD for threshold rationale.
12581257
*
12591258
* `opts.symbolsOnly` skips the AST/complexity/CFG/dataflow visitors in the
12601259
* worker (and their result serialization across the thread boundary) for
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
/**
2+
* Integration test for #1458: cross-class field annotation typeMap collision.
3+
*
4+
* Two classes with identically-named fields (`repo`) caused the later class's
5+
* annotation to overwrite the earlier one's bare typeMap key. `this.repo.save()`
6+
* inside UserService would resolve to OrderRepository instead of UserRepository.
7+
*
8+
* Fix: handleFieldDefTypeMap seeds `ClassName.field` at confidence 0.9 as the
9+
* primary key; the resolver checks the class-scoped key before bare fallback keys
10+
* for `this.` receivers so the correct type is always chosen.
11+
*/
12+
13+
import fs from 'node:fs';
14+
import os from 'node:os';
15+
import path from 'node:path';
16+
import Database from 'better-sqlite3';
17+
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
18+
import { buildGraph } from '../../src/domain/graph/builder.js';
19+
20+
const FIXTURE = {
21+
'services.ts': `
22+
class OrderRepository {
23+
save(order: unknown) {}
24+
}
25+
class UserRepository {
26+
save(user: unknown) {}
27+
}
28+
class OrderService {
29+
private repo: OrderRepository;
30+
run() { this.repo.save({}); }
31+
}
32+
class UserService {
33+
private repo: UserRepository;
34+
run() { this.repo.save({}); }
35+
}
36+
`,
37+
};
38+
39+
let tmpDir: string;
40+
41+
beforeAll(async () => {
42+
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1458-'));
43+
for (const [rel, content] of Object.entries(FIXTURE)) {
44+
fs.writeFileSync(path.join(tmpDir, rel), content);
45+
}
46+
await buildGraph(tmpDir, { engine: 'wasm', incremental: false, skipRegistry: true });
47+
});
48+
49+
afterAll(() => {
50+
fs.rmSync(tmpDir, { recursive: true, force: true });
51+
});
52+
53+
function readCallEdges(dbPath: string) {
54+
const db = new Database(dbPath, { readonly: true });
55+
try {
56+
return db
57+
.prepare(
58+
`SELECT n1.name AS src, n2.name AS tgt
59+
FROM edges e
60+
JOIN nodes n1 ON e.source_id = n1.id
61+
JOIN nodes n2 ON e.target_id = n2.id
62+
WHERE e.kind = 'calls'
63+
ORDER BY n1.name, n2.name`,
64+
)
65+
.all() as Array<{ src: string; tgt: string }>;
66+
} finally {
67+
db.close();
68+
}
69+
}
70+
71+
describe('cross-class field annotation typeMap collision (#1458)', () => {
72+
it('resolves this.repo.save() inside OrderService.run to OrderRepository.save', () => {
73+
const dbPath = path.join(tmpDir, '.codegraph', 'graph.db');
74+
const edges = readCallEdges(dbPath);
75+
const edge = edges.find(
76+
(e) => e.src === 'OrderService.run' && e.tgt === 'OrderRepository.save',
77+
);
78+
expect(
79+
edge,
80+
'OrderService.run → OrderRepository.save edge missing; cross-class collision may be present',
81+
).toBeDefined();
82+
});
83+
84+
it('resolves this.repo.save() inside UserService.run to UserRepository.save', () => {
85+
const dbPath = path.join(tmpDir, '.codegraph', 'graph.db');
86+
const edges = readCallEdges(dbPath);
87+
const edge = edges.find((e) => e.src === 'UserService.run' && e.tgt === 'UserRepository.save');
88+
expect(
89+
edge,
90+
'UserService.run → UserRepository.save edge missing; cross-class collision may be present',
91+
).toBeDefined();
92+
});
93+
94+
it('does not emit a false edge from OrderService.run to UserRepository.save', () => {
95+
const dbPath = path.join(tmpDir, '.codegraph', 'graph.db');
96+
const edges = readCallEdges(dbPath);
97+
const falseEdge = edges.find(
98+
(e) => e.src === 'OrderService.run' && e.tgt === 'UserRepository.save',
99+
);
100+
expect(falseEdge).toBeUndefined();
101+
});
102+
103+
it('does not emit a false edge from UserService.run to OrderRepository.save', () => {
104+
const dbPath = path.join(tmpDir, '.codegraph', 'graph.db');
105+
const edges = readCallEdges(dbPath);
106+
const falseEdge = edges.find(
107+
(e) => e.src === 'UserService.run' && e.tgt === 'OrderRepository.save',
108+
);
109+
expect(falseEdge).toBeUndefined();
110+
});
111+
});

0 commit comments

Comments
 (0)