Skip to content

fix(resolution): resolve this.<field>.<method>() on the field's declared type - #1691

Open
danusha2345 wants to merge 2 commits into
colbymchenry:mainfrom
danusha2345:fix/1496-ts-this-field-call
Open

fix(resolution): resolve this.<field>.<method>() on the field's declared type#1691
danusha2345 wants to merge 2 commits into
colbymchenry:mainfrom
danusha2345:fix/1496-ts-this-field-call

Conversation

@danusha2345

Copy link
Copy Markdown
Contributor

Fixes #1496.

Problem

this.mailer.send(msg) inside Notifier.send() was emitted as the bare send. Exact-name matching then took the nearest same-named method — the calling method itself — and stored Notifier::send → Notifier::send, a self-edge the source does not contain, so callers, callees, impact and 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

Verification

  • New __tests__/ts-this-field-call.test.ts: the issue's repro (no self-edge, Mailer::send from both wrappers), a plain-JS field initialized in the constructor, and a builtin-typed field left unresolved; fails on main on two of three.
  • torture.tsx gains the shape; kernel-tsjs-parity pins both arms.
  • Full suite with the rebuilt kernel: 237 files, 4234 passed, 9 skipped — including same-name-disambiguation (CodeGraph mixes up TypeScript classes with same name #764).

Re-index after upgrading.

🤖 Generated with Claude Code

…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.
@bompus

bompus commented Sep 5, 2026

Copy link
Copy Markdown

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 main (b9ca4b7) plus our fork's markdown/literal extras; control build indexed the same tree without the PR.

Merges clean. On our tree: self-call edges 31 → 30, and the one removed is exactly the issue's shape (destroy() calling this.hover.destroy(), previously a self edge).

The PR also drops 41 other calls edges, all resolvedBy: exact-match name-only hits from this.<field>.<method>() sites. Breakdown by method name: get 15, set 13 (Map/Set calls resolved to whatever function was named get/set, so these were wrong), observe/disconnect 4 (MutationObserver, wrong), querySelector 1 (wrong), and 8 that were right (getSettings 3, saveSettings 2, getDraftStateOrNull, saveDraftState, injectLiveExtensionData, where the field's declared type does have that method). Net: ~33 wrong edges removed, ~8 correct ones lost. Fine as a trade, but if the resolver can keep a this.<field>.<m>() edge when the field's declared type is a class/interface in the index that declares m, those 8 come back.

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 (renderDockStyles() { return renderDockStyles(...) }, four static wrappers in a consensus engine, chrome.storage.local.get() inside a method named get). I will open a separate issue for that.

PR test files: 3/3 pass.

@bompus

bompus commented Sep 5, 2026

Copy link
Copy Markdown

Re-verified on top of #1706 (the .js-specifier import fix), same repo, same-state indexes, line-insensitive diff of calls edges: 17,762 → 17,721.

  • 8 this.<field>.<method>() edges re-resolve as instance-method@0.85 to the same targets they had before (the bare-name path had guessed right).
  • 36 wrong bare-name edges gone: Map#get/Map#set calls that had landed on a project get/set (26), MutationObserver#observe/disconnect onto test helpers (4), one querySelector, the destroy self-edge, and 4 others.
  • 5 correct edges lost, all the same shape: this.storage.getSettings() where the field is declared storage: typeof DraftHubStorage. The type regex captures typeof, which fails the capitalised-name check, so matchTsThisFieldCall returns null and nothing else runs. Handling typeof X (resolve the method on the object literal / const X) would close that gap.

So the trade-off is unchanged by the import fix: net positive here, with the typeof case as the one regression I can point to.

@bompus

bompus commented Sep 5, 2026

Copy link
Copy Markdown

A patch for the typeof X case from the comment above, on top of this PR's head (ef888f3): bompus@0745140 (branch pr-1691-typeof, compare: ef888f3...bompus:codegraph:pr-1691-typeof). Feel free to pull it in, or I can open it as a follow-up PR once this lands.

What it does: a typeof <Value> pattern is tried before the declared-type pattern (which otherwise captures the word typeof), and on a hit the method is resolved by containment inside the value's object literal through the existing resolveObjectLiteralMember (#1573), preferring a holder in the call site's file. Everything else in matchTsThisFieldCall is unchanged; the declined-when-unknown discipline stays.

Test added to ts-this-field-call.test.ts: Keeper with constructor(private readonly storage: typeof DraftHubStorage) calling this.storage.get(key) from its own get and this.storage.getSettings() from settings; asserts both resolve to the literal's members and that Keeper::get has no self-edge. Fails on this PR's head (1 of 4), passes with the patch (4 of 4). On my repo it recovers the five this.storage.* edges from the previous comment.

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)
@danusha2345

Copy link
Copy Markdown
Contributor Author

Thanks for the trade-off numbers and for the patch — pulled it in as-is with your authorship (15c7ea6, cherry-picked from bompus:pr-1691-typeof). typeof <Value> is tried before the declared-type pattern and resolved by containment through resolveObjectLiteralMember, so the five this.storage.getSettings() edges come back while the declined-when-unknown discipline stays. Full suite with the native kernel: 237 files, 4235 passed. On the ten remaining self-edges of the other shape (a method calling a same-named imported free function) — that is a different receiver-less path; happy to look once the issue is up.

@bompus

bompus commented Sep 6, 2026

Copy link
Copy Markdown

Issue is up: #1714.

Thanks for taking the typeof patch as-is — good to see the five this.storage.* edges back with the declined-when-unknown discipline intact.

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 (15c7ea6), and only the same-file one misresolves:

shape result
same-file module-scope function self-edge, exact-match @0.4
import { serialize } from './format' correct, import @0.9
barrel re-export correct, import @0.9
namespace import correct, import @0.9

Your import resolver already handles every imported case. The gap is only where there is no import statement to consult, so findBestMatch's same-file line-proximity term picks the nearest same-named definition — which, for a call inside a method, is always that method.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TypeScript: a call through this.<field> resolves to the ENCLOSING method when the two share a name — silent self-edge, 0% recall on that shape

2 participants