Skip to content

fix(graph): resolve a Python receiver that names a class - #316

Open
datrixlab wants to merge 1 commit into
trailhq:mainfrom
datrixlab:fix/python-classname-receiver
Open

datrixlab wants to merge 1 commit into
trailhq:mainfrom
datrixlab:fix/python-classname-receiver

Conversation

@datrixlab

Copy link
Copy Markdown

What

resolveRecvType did not treat a Python class name as a receiver type, so SomeClass.method() call sites never got a recvType and dropped during resolution.

One line, placed next to the branch that already does this for Swift:

(ctx.lang === "swift" && /^[A-Z]/.test(receiver) ? receiver : undefined) ??
(ctx.lang === "python" && /^[A-Z]/.test(receiver) ? receiver : undefined) ??

Fixes #305

Why the call sites disappeared

pyReceiver already returns the right thing — for SearchIndexService.variables_manquantes() it yields SearchIndexService. The loss is one step later, in resolveRecvType:

  • it is not self / cls / this / super, and does not start with self.
  • ctx.bindings.lookup(scope, "SearchIndexService") misses, because a class name is not a variable that was ever assigned a type
  • the PHP branch is language-gated, and so is the Swift one

…so it returned undefined, no recvType was stamped, and the member call was dropped as untyped. The internal cls.variables_manquantes() call still resolved through enclosingClass, which is why the reporter saw 1 caller where there were 4 — worse than an empty result, because it reads as "nearly unused".

The naming argument is the same one the Swift branch already makes, and Python states it as a convention rather than leaving it to style: PEP 8 puts classes in CapWords and variables in lower_case. A shadowing local binding is still tried first, so a variable that happens to be capitalised keeps its bound type.

Tests

Three, in test/graph-resolve-typed.test.ts:

  1. a Python call bound by class name stamps the class as recvType — the extraction half, which is where the bug lived.
  2. a lower_case Python receiver is still left untyped — the guard against over-reaching. A variable receiver must keep falling through to the bindings table.
  3. the class-bound call resolves to the classmethod, not a same-named function — the resolution half, with a same-named free function present so a bare-name fallback would pick the wrong target.

Two-way: reverting only src/graph/bindings.ts fails exactly test 1 and leaves the other 17 in that file passing — test 2 passes either way by design, which is the point of having it.

Scope

Python only. I did not touch the equivalent gap in any other language: resolveRecvType's fallthrough is deliberately conservative and each language's naming signal is its own argument to make.

`SomeClass.method()` never got a recvType, so every such call site dropped
as an untyped member call. pyReceiver already yields the class name; the
loss is in resolveRecvType, where a class name is not `self`/`cls`, does
not start with `self.`, and misses the bindings lookup because it is not a
variable that was ever assigned a type. The PHP and Swift branches that
handle exactly this shape are language-gated, so Python fell to undefined.

The internal `cls.method()` call still resolved through enclosingClass,
which makes the failure worse than an empty result: a classmethod with
four call sites reported one, reading as nearly unused.

Add the Python branch beside Swift's, on the same naming argument — PEP 8
puts classes in CapWords and variables in lower_case, and a shadowing
local binding is still tried first, so a capitalised variable keeps its
bound type.

Three tests: the extraction half (where the bug was), a lower_case
receiver that must stay untyped so the fix cannot over-reach, and the
resolution half with a same-named free function present so a bare-name
fallback would pick the wrong target.
@trailhq-graft

trailhq-graft Bot commented Sep 9, 2026

Copy link
Copy Markdown

🌱 graft blast radius

1 area changed → 1 area can be affected. 3 dependent symbols, depth 2.
Tests: no test reaches Type Binding Resolution.
Tag: @Frankie-Xu — Graph Extraction · @shhdwi — Type Binding Resolution · @anirudhkumar-nanonets — Graph Extraction

flowchart TB
  A0(("Graph Extraction<br/>3 symbols"))
  classDef reached fill:#D9EDF3,stroke:#3AA7C9,stroke-width:1.5px,color:#0E313C;
  class A0 reached;
Loading
Can be affected Symbols Nearest hop Reached from
Graph Extraction 3 src/graph/extract.ts:L563-L849 walk — calls, depth 1 Type Binding Resolution
Who knows this code — 3 people across 2 areas
Area Who knows it
Type Binding Resolution · changed @shhdwi — 3 commits, last 26d ago
Graph Extraction · affected @Frankie-Xu — 5 commits, last 13d ago · @anirudhkumar-nanonets — 5 commits, last 1mo ago

Ownership is git history over each area's own files, weighted towards recent work (120-day half-life). Merge commits and bots are dropped, and you are dropped from your own PR. A name with no @ has no GitHub handle in its commit email — tag them by hand, or add a .mailmap entry. A suggestion from history, not a CODEOWNERS rule.

All 3 dependent symbols, grouped by area

Graph Extraction — 3 symbols in 1 file

  • src/graph/extract.ts:L563-L849 — walk (calls, depth 1)
    768: const recvType = resolveRecvType(callee.receiver, ctx);
  • src/graph/extract.ts:L519-L561 — emitPhpCollapsedEnum (calls, depth 2)
  • src/graph/extract.ts:L487-L517 — walkNamedChildren (calls, depth 2)
Test signal per changed area — 1 ✗

Reached = a node under a test path has a resolved edge into the changed symbol. It undercounts anything called indirectly — through a CLI, a spawned process or a dynamic import — so read a low ratio as “look here”, never as a coverage gate.

  • Type Binding Resolution — 0 of 1 reached · no test file reaches it
    • not reached: resolveRecvType

graft blast · origin/main...HEAD · depth 2 · 2 changed files

Open the interactive graph → — click an area to see its dependent symbols at file:line.

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.

Graft callers misses Python calls bound by class name (SomeClass.method())

1 participant