Skip to content

Commit a2f35d1

Browse files
committed
Merge remote-tracking branch 'origin/main' into fix/resolver-static-confidence-1398-2
2 parents 9b1eca9 + be34695 commit a2f35d1

6 files changed

Lines changed: 158 additions & 10 deletions

File tree

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,22 @@ export function resolveByMethodOrGlobal(
7272
const effectiveReceiver = call.receiver.startsWith('this.')
7373
? call.receiver.slice('this.'.length)
7474
: call.receiver;
75-
const typeEntry =
75+
// For this.prop receivers, also try the class-scoped key (ClassName.prop) seeded by
76+
// handlePropWriteTypeMap — prevents false edges when multiple classes define the same
77+
// property name (issue #1323).
78+
let typeEntry =
7679
typeMap.get(effectiveReceiver) ??
7780
typeMap.get(call.receiver) ??
81+
// Phase 8.3f: callee-scoped rest-param key (`callee::restName`) to avoid
82+
// same-name rest-binding collision across functions in the same file (#1358).
7883
(callerName ? typeMap.get(`${callerName}::${effectiveReceiver}`) : undefined);
84+
if (!typeEntry && call.receiver.startsWith('this.') && callerName) {
85+
const dotIdx = callerName.lastIndexOf('.');
86+
if (dotIdx > -1) {
87+
const callerClass = callerName.slice(0, dotIdx);
88+
typeEntry = typeMap.get(`${callerClass}.${effectiveReceiver}`);
89+
}
90+
}
7991
let typeName = typeEntry
8092
? typeof typeEntry === 'string'
8193
? typeEntry

src/extractors/javascript.ts

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1715,23 +1715,41 @@ function extractTypeMapWalk(
17151715
callAssignments?: CallAssignment[],
17161716
fnRefBindings?: FnRefBinding[],
17171717
): void {
1718-
function walk(node: TreeSitterNode, depth: number): void {
1718+
function walk(node: TreeSitterNode, depth: number, currentClass: string | null): void {
17191719
if (depth >= MAX_WALK_DEPTH) return;
17201720
const t = node.type;
1721+
if (t === 'class_declaration' || t === 'abstract_class_declaration') {
1722+
const nameNode = node.childForFieldName('name');
1723+
const className = nameNode?.text ?? null;
1724+
for (let i = 0; i < node.childCount; i++) {
1725+
walk(node.child(i)!, depth + 1, className);
1726+
}
1727+
return;
1728+
}
1729+
// Class expressions (e.g. `const Foo = class Bar { ... }`): the expression-internal
1730+
// name (`Bar`) is never visible to the resolver, which derives callerClass from the
1731+
// binding name (`Foo`). Walking with null preserves the pre-fix `this.prop` fallback
1732+
// so the second lookup in resolveByMethodOrGlobal still finds the entry.
1733+
if (t === 'class') {
1734+
for (let i = 0; i < node.childCount; i++) {
1735+
walk(node.child(i)!, depth + 1, null);
1736+
}
1737+
return;
1738+
}
17211739
if (t === 'variable_declarator') {
17221740
handleVarDeclaratorTypeMap(node, typeMap, returnTypeMap, callAssignments, fnRefBindings);
17231741
} else if (t === 'required_parameter' || t === 'optional_parameter') {
17241742
handleParamTypeMap(node, typeMap);
17251743
} else if (t === 'assignment_expression') {
1726-
handlePropWriteTypeMap(node, typeMap);
1744+
handlePropWriteTypeMap(node, typeMap, currentClass);
17271745
} else if (t === 'call_expression') {
17281746
handleDefinePropertyTypeMap(node, typeMap);
17291747
}
17301748
for (let i = 0; i < node.childCount; i++) {
1731-
walk(node.child(i)!, depth + 1);
1749+
walk(node.child(i)!, depth + 1, currentClass);
17321750
}
17331751
}
1734-
walk(rootNode, 0);
1752+
walk(rootNode, 0, null);
17351753
}
17361754

17371755
/** Extract type info from a variable_declarator: type annotation, constructor, or factory. */
@@ -1931,12 +1949,17 @@ function handleParamTypeMap(node: TreeSitterNode, typeMap: Map<string, TypeMapEn
19311949
* Phase 8.3d: seed the pts map from object property writes.
19321950
*
19331951
* `handlers.auth = authMiddleware` → typeMap.set('handlers.auth', { type: 'authMiddleware', confidence: 0.85 })
1934-
* `this.logger = new Logger(...)` → typeMap.set('this.logger', { type: 'Logger', confidence: 1.0 })
1952+
* `this.logger = new Logger(...)` → typeMap.set('UserService.logger', { type: 'Logger', confidence: 1.0 })
1953+
* (keyed as ClassName.prop when currentClass is known, to avoid collisions across classes)
19351954
*
19361955
* Only simple `obj.prop = identifier` and `this.prop = new Ctor()` writes are tracked
19371956
* (not chained `a.b.c = x`). BUILTIN_GLOBALS are skipped (e.g. `console.log = fn`).
19381957
*/
1939-
function handlePropWriteTypeMap(node: TreeSitterNode, typeMap: Map<string, TypeMapEntry>): void {
1958+
function handlePropWriteTypeMap(
1959+
node: TreeSitterNode,
1960+
typeMap: Map<string, TypeMapEntry>,
1961+
currentClass: string | null,
1962+
): void {
19401963
const lhsN = node.childForFieldName('left');
19411964
const rhsN = node.childForFieldName('right');
19421965
if (!lhsN || !rhsN) return;
@@ -1949,10 +1972,15 @@ function handlePropWriteTypeMap(node: TreeSitterNode, typeMap: Map<string, TypeM
19491972
// computed subscript expressions — consistent with the adjacent fnRefBindings block.
19501973
if (prop.type !== 'property_identifier' && prop.type !== 'identifier') return;
19511974

1952-
// this.prop = new ClassName(...) — constructor-assigned property type
1975+
// this.prop = new ClassName(...) — constructor-assigned property type.
1976+
// Key as ClassName.prop (class-scoped) so two classes with identically-named
1977+
// properties don't overwrite each other's typeMap entry.
19531978
if (obj.type === 'this' && rhsN.type === 'new_expression') {
19541979
const ctorType = extractNewExprTypeName(rhsN);
1955-
if (ctorType) setTypeMapEntry(typeMap, `this.${prop.text}`, ctorType, 1.0);
1980+
if (ctorType) {
1981+
const key = currentClass ? `${currentClass}.${prop.text}` : `this.${prop.text}`;
1982+
setTypeMapEntry(typeMap, key, ctorType, 1.0);
1983+
}
19561984
return;
19571985
}
19581986

tests/benchmarks/resolution/fixtures/javascript/driver.mjs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
import { directInstantiation, main } from './index.js';
1414
import { Logger } from './logger.js';
15+
import { ClassA, ClassB } from './multi-class.js';
1516
import { buildService } from './service.js';
1617
import { normalize, validate } from './validators.js';
1718

@@ -39,6 +40,10 @@ try {
3940
svc.createUser({ name: 'Direct' });
4041
svc.deleteUser(99);
4142

43+
// Multi-class fixture — exercises class-scoped this.prop typeMap (issue #1323)
44+
new ClassA().runA();
45+
new ClassB().runB();
46+
4247
globalThis.__tracer.popCall();
4348
} catch {
4449
// Swallow errors — we only care about call edges

tests/benchmarks/resolution/fixtures/javascript/expected-edges.json

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,34 @@
247247
"kind": "calls",
248248
"mode": "defineProperty-accessor",
249249
"notes": "this.accessMethod() inside accessorGetter — this === accessorTarget (get accessor via Object.defineProperty); accessorTarget.accessMethod is the qualified node name for the arrow function property of accessorTarget"
250+
},
251+
{
252+
"source": { "name": "ClassA.constructor", "file": "multi-class.js" },
253+
"target": { "name": "ServiceA", "file": "multi-class.js" },
254+
"kind": "calls",
255+
"mode": "constructor",
256+
"notes": "new ServiceA() — constructor call in ClassA.constructor"
257+
},
258+
{
259+
"source": { "name": "ClassB.constructor", "file": "multi-class.js" },
260+
"target": { "name": "ServiceB", "file": "multi-class.js" },
261+
"kind": "calls",
262+
"mode": "constructor",
263+
"notes": "new ServiceB() — constructor call in ClassB.constructor"
264+
},
265+
{
266+
"source": { "name": "ClassA.runA", "file": "multi-class.js" },
267+
"target": { "name": "ServiceA.doA", "file": "multi-class.js" },
268+
"kind": "calls",
269+
"mode": "receiver-typed",
270+
"notes": "this.service.doA() — receiver-typed via ClassA.service = new ServiceA() (class-scoped typeMap key prevents collision with ClassB.service)"
271+
},
272+
{
273+
"source": { "name": "ClassB.runB", "file": "multi-class.js" },
274+
"target": { "name": "ServiceB.doB", "file": "multi-class.js" },
275+
"kind": "calls",
276+
"mode": "receiver-typed",
277+
"notes": "this.service.doB() — receiver-typed via ClassB.service = new ServiceB() (class-scoped typeMap key prevents collision with ClassA.service)"
250278
}
251279
]
252280
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/**
2+
* Fixture: two classes in the same file that both use `this.service`,
3+
* but assign different types. Before the fix, the second class's
4+
* typeMap entry overwrote the first, causing one class to resolve
5+
* `this.service.method()` against the wrong type (false edge).
6+
*/
7+
8+
export class ServiceA {
9+
doA() {}
10+
}
11+
12+
export class ServiceB {
13+
doB() {}
14+
}
15+
16+
export class ClassA {
17+
constructor() {
18+
this.service = new ServiceA();
19+
}
20+
21+
runA() {
22+
this.service.doA();
23+
}
24+
}
25+
26+
export class ClassB {
27+
constructor() {
28+
this.service = new ServiceB();
29+
}
30+
31+
runB() {
32+
this.service.doB();
33+
}
34+
}

tests/parsers/javascript.test.ts

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,24 +351,65 @@ describe('JavaScript parser', () => {
351351
expect(symbols.typeMap.has('b.c')).toBe(false);
352352
});
353353

354-
it('seeds typeMap for this.prop = new ClassName() with confidence 1.0', () => {
354+
it('seeds typeMap for this.prop = new ClassName() using class-scoped key', () => {
355355
const symbols = parseJS(`
356356
class UserService {
357357
constructor() {
358358
this.logger = new Logger('UserService');
359359
}
360360
}
361361
`);
362+
expect(symbols.typeMap.get('UserService.logger')).toEqual({
363+
type: 'Logger',
364+
confidence: 1.0,
365+
});
366+
expect(symbols.typeMap.has('this.logger')).toBe(false);
367+
});
368+
369+
it('uses this.prop key when no enclosing class is present', () => {
370+
const symbols = parseJS(`
371+
function setup() {
372+
this.logger = new Logger();
373+
}
374+
`);
362375
expect(symbols.typeMap.get('this.logger')).toEqual({ type: 'Logger', confidence: 1.0 });
363376
});
364377

378+
it('scopes this.prop typeMap key to enclosing class — no collision across classes', () => {
379+
const symbols = parseJS(`
380+
class ClassA {
381+
constructor() { this.service = new ServiceA(); }
382+
}
383+
class ClassB {
384+
constructor() { this.service = new ServiceB(); }
385+
}
386+
`);
387+
expect(symbols.typeMap.get('ClassA.service')).toEqual({ type: 'ServiceA', confidence: 1.0 });
388+
expect(symbols.typeMap.get('ClassB.service')).toEqual({ type: 'ServiceB', confidence: 1.0 });
389+
expect(symbols.typeMap.has('this.service')).toBe(false);
390+
});
391+
392+
it('uses this.prop fallback for named class expressions (expression name not resolver-visible)', () => {
393+
// `const Foo = class Bar { ... }` — the resolver derives callerClass from the
394+
// binding name `Foo`, never from the expression name `Bar`. Storing as `Bar.x`
395+
// would produce an unreachable key, so we fall back to `this.x` instead.
396+
const symbols = parseJS(`
397+
const Foo = class Bar {
398+
constructor() { this.x = new X(); }
399+
};
400+
`);
401+
expect(symbols.typeMap.get('this.x')).toEqual({ type: 'X', confidence: 1.0 });
402+
expect(symbols.typeMap.has('Bar.x')).toBe(false);
403+
});
404+
365405
it('does not seed typeMap for this.prop = identifier (only new expressions)', () => {
366406
const symbols = parseJS(`
367407
class Foo {
368408
init(logger) { this.logger = logger; }
369409
}
370410
`);
371411
expect(symbols.typeMap.has('this.logger')).toBe(false);
412+
expect(symbols.typeMap.has('Foo.logger')).toBe(false);
372413
});
373414

374415
it('ignores non-identifier RHS (a.prop = obj.method)', () => {

0 commit comments

Comments
 (0)