fix(resolution): resolve this.<field>.<method>() on the field's declared type - #1691
fix(resolution): resolve this.<field>.<method>() on the field's declared type#1691danusha2345 wants to merge 2 commits into
Conversation
…red type (colbymchenry#1496) `this.mailer.send(msg)` inside `Notifier.send()` was emitted as the bare `send`, which exact-matched the nearest same-named method — the calling method itself — and stored a self-edge the source does not contain. Keep the `this.<field>` receiver (wasm walker and kernel), and resolve it the way Rust's `self.<field>` already is: the field's type read off the enclosing class's own declaration, the method validated on that type, or no edge at all.
|
Verified on a real TypeScript repo (Chrome MV3 extension, 582 files, TS/JS/Vue/markdown, Windows 11, tree-sitter wasm walker, kernel off). Branch: this PR merged onto current Merges clean. On our tree: self-call edges 31 → 30, and the one removed is exactly the issue's shape ( The PR also drops 41 other Ten false self-edges remain on our tree that this PR does not target, all a different shape: a method calling a same-named free function it imports ( PR test files: 3/3 pass. |
|
Re-verified on top of #1706 (the
So the trade-off is unchanged by the import fix: net positive here, with the |
|
A patch for the What it does: a Test added to diff--- a/src/resolution/name-matcher.ts
+++ b/src/resolution/name-matcher.ts
@@ -2288,23 +2288,48 @@ function matchTsThisFieldCall(
const fieldEsc = field.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
- const patterns = [
+ 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.
- new RegExp(`\\b${fieldEsc}\\b\\s*[?!]?\\s*:\\s*(?:readonly\\s+)?([A-Za-z_$][\\w.$]*)`),
+ {
+ re: new RegExp(`\\b${fieldEsc}\\b\\s*[?!]?\\s*:\\s*(?:readonly\\s+)?([A-Za-z_$][\\w.$]*)`),
+ valueType: false,
+ },
// `mailer = new Mailer()` / `this.mailer = new Mailer()`
- new RegExp(`\\b${fieldEsc}\\b\\s*=\\s*new\\s+([A-Za-z_$][\\w.$]*)`),
+ { 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 of patterns) {
+ 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()!; |
…ectLiteral> resolves by containment (cherry picked from commit 0745140)
|
Thanks for the trade-off numbers and for the patch — pulled it in as-is with your authorship ( |
|
Issue is up: #1714. Thanks for taking the One correction I owe you on the ten remaining self-edges, since it changes where you'd look. I described them as a method calling a same-named imported free function. That was wrong. I re-tested four variants against this PR's head (
Your import resolver already handles every imported case. The gap is only where there is no import statement to consult, so You called it a different receiver-less path and that reads exactly right: in JS/TS a bare call can never bind to a class method at all, so the candidate should not be in the set. #1714 has the reproduction and two possible rules, broad and narrow. |
Fixes #1496.
Problem
this.mailer.send(msg)insideNotifier.send()was emitted as the baresend. Exact-name matching then took the nearest same-named method — the calling method itself — and storedNotifier::send → Notifier::send, a self-edge the source does not contain, socallers,callees,impactand trace were silently wrong on exactly the shape a delegating wrapper takes. The identical call resolved correctly whenever the wrapper had any other name.Change
this.<field>keeps the field —this.mailer.send— the way Rust'sself.<field>(Rust: a method call on a typed field resolves to a same-named method on the wrong type #1585) and Go's field chains (Go: external receiver calls resolve to unrelated local interface methods #1276) already do.private readonly mailer: Mailer), or an initializer (mailer = new Mailer(),this.mailer = new Mailer()) — and resolves the method on that type, validated byresolveMethodOnType. When two apps each declare the type (the CodeGraph mixes up TypeScript classes with same name #764 fixture), the declaration nearest the call site's directory wins, the same signal the bare-name path used. A field whose type is external, a builtin (this.items.push()) or not spelled out resolves to nothing rather than a guess.Verification
__tests__/ts-this-field-call.test.ts: the issue's repro (no self-edge,Mailer::sendfrom both wrappers), a plain-JS field initialized in the constructor, and a builtin-typed field left unresolved; fails onmainon two of three.torture.tsxgains the shape;kernel-tsjs-paritypins both arms.same-name-disambiguation(CodeGraph mixes up TypeScript classes with same name #764).Re-index after upgrading.
🤖 Generated with Claude Code