Land the six-file fix from upstream PR #1691 by danusha2345 (pr-1691 at 6d0e80d52ae953b22d615bd20c4a4c7758814e60), preserving wasm/native extraction parity and exclusive field-type resolution. Preserve coexistence with the #1566 Map/collection fix merged in #1790, including nested holder.values.get coverage and the unchanged #1566 Unreleased changelog bullet. EXTRACTION_VERSION remains unchanged. Align the existing chained-receiver regression with the fix: a declared service field calls its method, while an anonymous field type does not bind to unrelated same-named project functions. Verified on Linux with Node 22.19.0: - Rebuilt the native kernel and TypeScript/browser distribution. - Both backends change Outbox::send -> Outbox::send into Outbox::send -> Mailer::send, keep Relay::forward -> Mailer::send, and store no self-edges in the issue repro. - Wasm: 224 tests passed; native kernel: 255 tests passed, no skips. - All 10 #1566 resolution cases pass on each backend, plus all four nested-receiver extraction parity cases. Co-authored-by: Colby McHenry <colbymchenry@users.noreply.github.com>
This commit is contained in:
Colby Mchenry
co-authored by
Colby McHenry
Colby McHenry
parent
de5adba7ea
commit
cece0720e3
@@ -219,6 +219,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
|
||||
#### Symbols, tests and the viewer
|
||||
|
||||
- TypeScript/JavaScript: a call through a field of the enclosing class — `this.mailer.send()` — now resolves on the field's declared type, so a delegating wrapper that shares the method's name no longer records itself as its own callee and `callers`, `impact` and trace stop lying on that shape. A field whose type is external or a builtin stays unresolved rather than guessed. Re-index after upgrading. (#1496)
|
||||
- TypeScript and JavaScript collection calls through local variables and their nested properties no longer link to unrelated project methods; re-index after upgrading. (#1566)
|
||||
|
||||
- Objective-C headers now index in a project that has no `.m` file. A `.h` file is read as C from its name alone, and only later — once its contents are read — recognized as Objective-C; the grammar for that was never loaded up front, so the file failed with a parser error and nothing in it reached the index. Adding any `.m` file used to make the same header work, which is what made this look arbitrary. Thanks @Juddd. (#1628)
|
||||
|
||||
@@ -213,6 +213,14 @@ new NS.Widget(makeArg());
|
||||
new Map<string, number>();
|
||||
super_weird?.();
|
||||
|
||||
// --- call through a field of the enclosing class (#1496) ---------------------
|
||||
export class FieldDelegator {
|
||||
constructor(private readonly mailer: { send(m: string): string }, private items: string[]) {}
|
||||
send(msg: string): string { return this.mailer.send(msg); }
|
||||
push(msg: string): void { this.items.push(msg); this.mailer.send(msg).trim(); }
|
||||
direct(): void { this.send('x'); super.toString(); }
|
||||
}
|
||||
|
||||
// --- const-bound functions inside a body (#1669) -----------------------------
|
||||
export function NestedHandlers({ items, onPick }: { items: string[]; onPick: (a: unknown, b: unknown) => void }) {
|
||||
const handleClear = () => { onPick(null, null); };
|
||||
|
||||
@@ -43,7 +43,12 @@ beforeAll(async () => {
|
||||
'export function viaGlobal(): string {\n' +
|
||||
' return window.MyNs.ping();\n' +
|
||||
'}\n' +
|
||||
'export class PingService { ping(): string { return "service"; } }\n' +
|
||||
'export class Runner {\n' +
|
||||
' constructor(private svc: PingService) {}\n' +
|
||||
' run(): string { return this.svc.ping(); }\n' +
|
||||
'}\n' +
|
||||
'export class AnonymousRunner {\n' +
|
||||
' constructor(private svc: { ping(): string }) {}\n' +
|
||||
' run(): string { return this.svc.ping(); }\n' +
|
||||
'}\n'
|
||||
@@ -86,6 +91,11 @@ describe('TS/JS call through a host-global chain (#1707)', () => {
|
||||
it('keeps a chain rooted at a project value — window.MyNs.m() and this.<field>.m()', () => {
|
||||
const ping = fn('ping', 'service.ts').id;
|
||||
expect(callTargets(fn('viaGlobal', 'service.ts').id)).toContain(ping);
|
||||
expect(callTargets(method('Runner::run').id)).toContain(ping);
|
||||
expect(callTargets(method('Runner::run').id)).toEqual([method('PingService::ping').id]);
|
||||
});
|
||||
|
||||
it('does not guess a same-named project target for an anonymous field type (#1496)', () => {
|
||||
// Neither the top-level ping nor PingService::ping establishes what svc is.
|
||||
expect(callTargets(method('AnonymousRunner::run').id)).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,106 @@
|
||||
/**
|
||||
* A TS/JS call through a field of the enclosing class resolves on the field's
|
||||
* declared type, never by bare name (#1496).
|
||||
*
|
||||
* `this.mailer.send(msg)` inside `Notifier.send()` used to be emitted as the
|
||||
* bare `send`, which exact-matched the nearest same-named method — the
|
||||
* calling method itself. The stored self-edge `Notifier::send → Notifier::send`
|
||||
* made callers, callees, impact and trace silently wrong on exactly the
|
||||
* shape a delegating wrapper takes. The identical call resolved correctly
|
||||
* whenever the wrapper had any other name.
|
||||
*/
|
||||
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { CodeGraph } from '../src';
|
||||
import { initGrammars, loadAllGrammars } from '../src/extraction/grammars';
|
||||
|
||||
let dir: string;
|
||||
let cg: CodeGraph;
|
||||
|
||||
beforeAll(async () => {
|
||||
await initGrammars();
|
||||
await loadAllGrammars();
|
||||
dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1496-'));
|
||||
fs.mkdirSync(path.join(dir, 'src'));
|
||||
const w = (rel: string, body: string) => fs.writeFileSync(path.join(dir, 'src', rel), body);
|
||||
w('mailer.ts', 'export class Mailer {\n send(msg: string): string { return msg; }\n}\n');
|
||||
w(
|
||||
'notifier.ts',
|
||||
"import { Mailer } from './mailer';\n" +
|
||||
'export class Notifier {\n' +
|
||||
' constructor(private readonly mailer: Mailer, private items: string[]) {}\n' +
|
||||
' send(msg: string): string { return this.mailer.send(msg); }\n' +
|
||||
' other(msg: string): string { return this.mailer.send(msg); }\n' +
|
||||
' push(msg: string): void { this.items.push(msg); }\n' +
|
||||
'}\n'
|
||||
);
|
||||
// Plain JS: the field's type is only known from its `new` initializer.
|
||||
// (resolveMethodOnType matches within one language, so the JS wrapper gets a JS Mailer.)
|
||||
w('legacy-mailer.js', 'class LegacyMailer {\n send(msg) { return msg; }\n}\nmodule.exports = { LegacyMailer };\n');
|
||||
w(
|
||||
'legacy.js',
|
||||
"const { LegacyMailer } = require('./legacy-mailer');\n" +
|
||||
'class LegacyNotifier {\n' +
|
||||
' constructor() { this.mailer = new LegacyMailer(); }\n' +
|
||||
' send(msg) { return this.mailer.send(msg); }\n' +
|
||||
'}\n' +
|
||||
'module.exports = { LegacyNotifier };\n'
|
||||
);
|
||||
// A field typed as the type OF a value: an object literal used as a namespace.
|
||||
w(
|
||||
'storage.ts',
|
||||
'export const DraftHubStorage = {\n' +
|
||||
' async get(key: string): Promise<string> { return key; },\n' +
|
||||
' async getSettings(): Promise<object> { return {}; },\n' +
|
||||
'};\n'
|
||||
);
|
||||
w(
|
||||
'keeper.ts',
|
||||
"import { DraftHubStorage } from './storage';\n" +
|
||||
'export class Keeper {\n' +
|
||||
' constructor(private readonly storage: typeof DraftHubStorage) {}\n' +
|
||||
' async get(key: string): Promise<string> { return this.storage.get(key); }\n' +
|
||||
' async settings(): Promise<object> { return this.storage.getSettings(); }\n' +
|
||||
'}\n'
|
||||
);
|
||||
cg = CodeGraph.initSync(dir);
|
||||
await cg.indexAll();
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
cg.destroy();
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
const method = (qn: string) => cg.getNodesByKind('method').find((n) => n.qualifiedName === qn)!;
|
||||
const calleesOf = (qn: string) => cg.getCallees(method(qn).id).map(({ node }) => node.qualifiedName).sort();
|
||||
|
||||
describe('this.<field>.<method>() (#1496)', () => {
|
||||
it('resolves on the field\'s declared type even when the wrapper shares the method name', () => {
|
||||
expect(calleesOf('Notifier::send')).toEqual(['Mailer::send']);
|
||||
expect(calleesOf('Notifier::other')).toEqual(['Mailer::send']);
|
||||
// No self-edge anywhere.
|
||||
const self = cg.getCallers(method('Notifier::send').id).some(({ node }) => node.id === method('Notifier::send').id);
|
||||
expect(self).toBe(false);
|
||||
});
|
||||
|
||||
it('reads a JS field initialized in the constructor', () => {
|
||||
expect(calleesOf('LegacyNotifier::send')).toEqual(['LegacyMailer::send']);
|
||||
});
|
||||
|
||||
it('leaves a builtin-typed field unresolved rather than guessing a same-named method', () => {
|
||||
// `this.items.push()` — `string[]` names no project type; the wrapper `push`
|
||||
// must not become its own callee.
|
||||
expect(calleesOf('Notifier::push')).toEqual([]);
|
||||
});
|
||||
|
||||
it('resolves a field typed `typeof <objectLiteral>` onto the literal\'s member', () => {
|
||||
// The members are bare-named functions inside the constant's extent (#1573).
|
||||
expect(calleesOf('Keeper::settings')).toEqual(['getSettings']);
|
||||
expect(calleesOf('Keeper::get')).toEqual(['get']);
|
||||
const self = cg.getCallers(method('Keeper::get').id).some(({ node }) => node.id === method('Keeper::get').id);
|
||||
expect(self).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -1185,6 +1185,11 @@ impl<'t> Walker<'t> {
|
||||
} else {
|
||||
callee_name = method_name.to_string();
|
||||
}
|
||||
} else if let Some(field) = receiver.and_then(|r| self.this_field_of(r)) {
|
||||
// `this.<field>.<method>()` — keep the field so the
|
||||
// resolver can read its declared type (#1496). Mirrors
|
||||
// TreeSitterExtractor.extractCall.
|
||||
callee_name = format!("this.{field}.{method_name}");
|
||||
} else if let Some(r) = receiver.filter(|r| r.kind() == "call_expression") {
|
||||
// Call receiver — `make().run()` (#1683): keep the inner
|
||||
// callee as `<inner>().<method>`, or emit nothing when it
|
||||
@@ -1215,6 +1220,19 @@ impl<'t> Walker<'t> {
|
||||
|
||||
// --- extractInstantiation -----------------------------------------------------------
|
||||
|
||||
/// `this.<field>` as a member_expression receiver → Some(field) (#1496).
|
||||
fn this_field_of(&self, receiver: Node<'t>) -> Option<String> {
|
||||
if receiver.kind() != "member_expression" {
|
||||
return None;
|
||||
}
|
||||
let object = receiver.child_by_field_name("object")?;
|
||||
let property = receiver.child_by_field_name("property")?;
|
||||
if object.kind() != "this" || property.kind() != "property_identifier" {
|
||||
return None;
|
||||
}
|
||||
Some(self.text(property).to_string())
|
||||
}
|
||||
|
||||
/// The callee of a call-expression receiver when it is a plain identifier
|
||||
/// or member chain (`make`, `d.setdefault`), whitespace stripped (#1683).
|
||||
fn plain_inner_callee(&self, call: Node<'t>) -> Option<String> {
|
||||
|
||||
@@ -4728,6 +4728,29 @@ export class TreeSitterExtractor {
|
||||
// scope keywords: such calls previously emitted a bare method
|
||||
// name, which either failed to resolve or resolved ambiguously.
|
||||
calleeName = `${getNodeText(receiver, this.source)}.${methodName}`;
|
||||
} else if (
|
||||
(this.language === 'typescript' ||
|
||||
this.language === 'javascript' ||
|
||||
this.language === 'tsx' ||
|
||||
this.language === 'jsx') &&
|
||||
receiver &&
|
||||
receiver.type === 'member_expression' &&
|
||||
getChildByField(receiver, 'object')?.type === 'this' &&
|
||||
getChildByField(receiver, 'property')?.type === 'property_identifier'
|
||||
) {
|
||||
// TS/JS call through a field of the enclosing class —
|
||||
// `this.mailer.send()` (#1496). Keep the `this.<field>` prefix:
|
||||
// the resolver reads the field's declared type off the class's
|
||||
// own declaration (`private mailer: Mailer`, `mailer = new
|
||||
// Mailer()`) and resolves the method on THAT type — or leaves the
|
||||
// ref unresolved when the type is external or unknown. Previously
|
||||
// this collapsed to the bare method name, which exact-matched
|
||||
// whichever same-named method was nearest — the calling method
|
||||
// itself when the two share a name, a self-edge not in the
|
||||
// source. Same discipline as Rust's `self.<field>` (#1585).
|
||||
// Mirrored in the kernel's extract_call (tsjs/extractors.rs).
|
||||
const fieldName = getNodeText(getChildByField(receiver, 'property')!, this.source);
|
||||
calleeName = `this.${fieldName}.${methodName}`;
|
||||
} else if (
|
||||
(this.language === 'typescript' ||
|
||||
this.language === 'javascript' ||
|
||||
|
||||
@@ -2232,6 +2232,21 @@ export function matchMethodCall(
|
||||
return matchRustSelfFieldCall(objectOrClass!.slice('self.'.length), methodName!, ref, context);
|
||||
}
|
||||
|
||||
// TS/JS call through a field of the enclosing class — `this.mailer.send()`,
|
||||
// emitted as `this.mailer.send` (#1496). Same discipline as the Rust branch
|
||||
// above, and EXCLUSIVE for the same reason: the field's declared type off
|
||||
// the class's own declaration, validated by resolveMethodOnType, or nothing.
|
||||
// Letting the bare name through is how `this.mailer.send()` inside
|
||||
// `Notifier.send()` resolved to the calling method itself — a self-edge the
|
||||
// source does not contain — whenever the two shared a name.
|
||||
if (
|
||||
(ref.language === 'typescript' || ref.language === 'javascript' || ref.language === 'tsx' || ref.language === 'jsx') &&
|
||||
dotMatch &&
|
||||
objectOrClass!.startsWith('this.')
|
||||
) {
|
||||
return matchTsThisFieldCall(objectOrClass!.slice('this.'.length), methodName!, ref, context);
|
||||
}
|
||||
|
||||
// Java/Kotlin: receiver may be a field whose name doesn't match the type by
|
||||
// Java naming convention (`userbo` → class `UserBO`, abbreviated). Look up
|
||||
// the field in the enclosing class to get its declared type, then resolve
|
||||
@@ -2618,6 +2633,111 @@ function matchRustSelfFieldCall(
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a TS/JS `this.<field>.<method>()` call (#1496) through the field's
|
||||
* declared type, read off the ENCLOSING class's own declaration lines:
|
||||
* a field or constructor-parameter property (`private mailer: Mailer`,
|
||||
* `mailer?: Mailer`, `readonly mailer: Mailer`) or an initializer
|
||||
* (`mailer = new Mailer()`, `this.mailer = new Mailer()`). The method is then
|
||||
* VALIDATED on that type by resolveMethodOnType. Null — never a bare-name
|
||||
* fallback — when the field is not declared there or its type is external,
|
||||
* a builtin (`this.items.push()`) or not spelled out.
|
||||
*/
|
||||
function matchTsThisFieldCall(
|
||||
field: string,
|
||||
methodName: string,
|
||||
ref: UnresolvedRef,
|
||||
context: ResolutionContext,
|
||||
): ResolvedRef | null {
|
||||
if (!field || field.includes('.')) return null;
|
||||
const caller = context.getNodeById?.(ref.fromNodeId);
|
||||
if (!caller) return null;
|
||||
const sep = caller.qualifiedName.lastIndexOf('::');
|
||||
if (sep <= 0) return null; // not inside a class
|
||||
const owner = caller.qualifiedName.slice(0, sep).split('::').pop();
|
||||
if (!owner) return null;
|
||||
|
||||
const owners = preferCallSiteFile(context.getNodesByName(owner), ref.filePath).filter(
|
||||
(n) => (n.kind === 'class' || n.kind === 'component') && sameLanguageFamily(n.language, ref.language)
|
||||
);
|
||||
const fieldEsc = field.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
|
||||
const patterns: Array<{ re: RegExp; valueType: boolean }> = [
|
||||
// `storage: typeof DraftHubStorage` — the type OF a value: an object
|
||||
// literal used as a namespace. Its members are bare-named functions inside
|
||||
// the constant's extent (#1573), so they are found by containment, not by
|
||||
// `Type::method`. Tried first: the declared-type pattern below would
|
||||
// otherwise capture the word `typeof`.
|
||||
{
|
||||
re: new RegExp(`\\b${fieldEsc}\\b\\s*[?!]?\\s*:\\s*(?:readonly\\s+)?typeof\\s+([A-Za-z_$][\\w.$]*)`),
|
||||
valueType: true,
|
||||
},
|
||||
// `private readonly mailer?: Mailer` — a class field or a constructor
|
||||
// parameter property; the capture stops at `<`, `[` or `|`, so a generic
|
||||
// or union type yields its head and resolveMethodOnType decides.
|
||||
{
|
||||
re: new RegExp(`\\b${fieldEsc}\\b\\s*[?!]?\\s*:\\s*(?:readonly\\s+)?([A-Za-z_$][\\w.$]*)`),
|
||||
valueType: false,
|
||||
},
|
||||
// `mailer = new Mailer()` / `this.mailer = new Mailer()`
|
||||
{ re: new RegExp(`\\b${fieldEsc}\\b\\s*=\\s*new\\s+([A-Za-z_$][\\w.$]*)`), valueType: false },
|
||||
];
|
||||
for (const cls of owners) {
|
||||
const source = context.readFile(cls.filePath);
|
||||
if (!source) continue;
|
||||
const declLines = source.split('\n').slice(Math.max(0, cls.startLine - 1), cls.endLine);
|
||||
for (const rawLine of declLines) {
|
||||
const line = rawLine.replace(/\/\/.*$/, '').replace(/\/\*.*?\*\//g, '');
|
||||
for (const { re, valueType } of patterns) {
|
||||
const m = line.match(re);
|
||||
if (!m || !m[1]) continue;
|
||||
if (valueType) {
|
||||
// The value's declaration may live in another file (it is imported);
|
||||
// the call site's file is preferred when several share the name.
|
||||
const holderName = m[1].split('.').pop()!;
|
||||
const holders = preferCallSiteFile(context.getNodesByName(holderName), ref.filePath).filter(
|
||||
(n) => (n.kind === 'constant' || n.kind === 'variable') && sameLanguageFamily(n.language, ref.language)
|
||||
);
|
||||
for (const holder of holders) {
|
||||
const hit = resolveObjectLiteralMember(holder, methodName, ref, context, 0.85, 'instance-method');
|
||||
if (hit) return hit;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
// `ns.Mailer` → `Mailer`; a primitive or builtin names no project type.
|
||||
const typeName = m[1].split('.').pop()!;
|
||||
if (!/^[A-Z]/.test(typeName)) return null;
|
||||
// Two apps in one repo may each declare a `UserService`. The bare-name
|
||||
// path this replaces broke that tie by directory proximity, so keep the
|
||||
// same signal: among the type's declarations of the method, prefer the
|
||||
// one closest to the call site's directory (its own app), never index
|
||||
// order. resolveMethodOnType still answers the single-declaration and
|
||||
// supertype cases.
|
||||
const declared = context
|
||||
.getNodesByName(methodName)
|
||||
.filter(
|
||||
(n) =>
|
||||
n.kind === 'method' &&
|
||||
sameLanguageFamily(n.language, ref.language) &&
|
||||
(n.qualifiedName === `${typeName}::${methodName}` || n.qualifiedName.endsWith(`::${typeName}::${methodName}`))
|
||||
);
|
||||
if (declared.length > 1) {
|
||||
const callDirs = ref.filePath.split('/').slice(0, -1);
|
||||
const shared = (fp: string) => {
|
||||
const dirs = fp.split('/').slice(0, -1);
|
||||
let i = 0;
|
||||
while (i < dirs.length && i < callDirs.length && dirs[i] === callDirs[i]) i++;
|
||||
return i;
|
||||
};
|
||||
const nearest = [...declared].sort((a, b) => shared(b.filePath) - shared(a.filePath) || a.filePath.localeCompare(b.filePath))[0]!;
|
||||
return { original: ref, targetNodeId: nearest.id, confidence: 0.85, resolvedBy: 'instance-method' };
|
||||
}
|
||||
return resolveMethodOnType(typeName, methodName, ref, context, 0.85, 'instance-method');
|
||||
}
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* The one fallback a TS/JS/Python call-receiver chain keeps (#1683): a STORE
|
||||
* ACCESSOR. Zustand's `get()` inside the store factory and
|
||||
|
||||
Reference in New Issue
Block a user