Skip to content

issue-2088 plan: for-of enclosing-scope name (funcStack/enclosing_func_context) diverges from findCaller for TS class methods, class-field arrows, and object-literal arrow props #2647

Description

@carlos-alm

What

While verifying WU-5(a)'s resolveReceiverSites (PR #2612, plan for issue #2088, round 28) against the real pipeline, found that the enclosing-scope name the JS/TS extractor writes for a for-of binding (ForOfBinding.enclosingFunc, consumed by buildForOfConstraints to key its points-to constraint) can diverge from the name findCaller/find_enclosing_caller derives for a call at the same source position — for the same call, in the same file. Both engines are affected identically, and the divergence is pre-existing (not introduced by #2088).

Two independent root causes, both symmetric across the TS and Rust extractors:

(a) TS class names parse as type_identifier, not identifier. The context-collector that builds the qualified enclosing-scope name only qualifies a class method when the class-name node's grammar kind is identifier:

  • TS: src/extractors/javascript.ts's computeClassNameContext (classNameIsIdentifier: nameNode?.type === 'identifier', ~line 3395) gates pushClassContext (~line 3253-3263), consumed by pushMethodDefContext (~line 3321-3334).
  • Rust: crates/codegraph-core/src/extractors/javascript.rs's enclosing_func_context (~line 7196-7210) filters .filter(|name| name.kind() == "identifier") — the doc comment on the function (~line 7180-7184) already states this caveat explicitly.

Since a TS class's name node is type_identifier, both engines fail to qualify the method, and a for-of directly inside a TS instance/static/getter/async method gets enclosingFunc/enclosing_func_context = the bare method name (e.g. run) instead of ClassName.run.

Meanwhile, the SEPARATE mechanism that produces the Definition/Def entry findCaller/find_enclosing_caller reads from — TS's findParentClassfindParentNode (javascript.ts:6739-6741), Rust's find_parent_classfind_enclosing_type_name (javascript.rs:6684-6686) — reads the class name node's .text regardless of its grammar kind, so it correctly returns ClassName.run. The two mechanisms disagree only for TS classes; a JS class (name node identifier) gets the same qualified name from both.

(b) The context-collector has no case for a class-field arrow or an object-literal arrow-valued property. run = (x) => { ... } (a field_definition/public_field_definition) and { run: (x) => { ... } } (a pair inside an object) both DO get a qualified Definition/Def entry (ClassName.run / varName.run) from handleFieldDef/extractObjectLiteralFunctions (TS) and handle_field_def/the object-literal-pair extraction (Rust) — both call the same correct, node-type-agnostic class/qualifier lookup as (a). But the context-collector's own dispatch has no branch for either node kind:

  • TS: pushEnclosingContext's switch (javascript.ts:3404-3434) only handles class_declaration/class/method_definition/variable_declarator/assignment_expression — no field_definition/public_field_definition, no pair.
  • Rust: enclosing_func_context's match (javascript.rs:7188-7246) only handles function_declaration/generator_function_declaration/method_definition/arrow_function+variable_declarator/arrow_function+assignment_expression — same gap.

So a for-of directly inside either shape gets enclosingFunc/enclosing_func_context = '<module>' (module scope, the loop's own fallback) even though findCaller/find_enclosing_caller correctly attributes the call to ClassName.run/varName.run.

Repro (verified against the real pipeline, both engines, codegraph@3.17.0 / current main)

function isFoo(x: number) { return x === 1; }
function doFoo(x: number) { return x; }
const RESOLVERS = [{ matches: isFoo, resolve: doFoo }];
class C {
  run(x: number) { for (const r of RESOLVERS) { if (r.matches(x)) return r.resolve(x); } }
}

forOfBindings (real dist/domain/parser.js, extractSymbols): [{ varName: 'r', sourceName: 'RESOLVERS', enclosingFunc: 'run' }] — bare, not C.run.

findCaller (real dist/domain/graph/builder/call-resolver.js) for either r.matches(x) or r.resolve(x): callerName = 'C.run'.

Any pts consumer that keys its lookup off findCaller's callerName (scoped as ${callerName}::${receiver}) therefore misses the key buildForOfConstraints actually wrote (run::r), for every TS instance/static/getter/async method — confirmed for all four with the real solver (dist/domain/graph/resolver/points-to.js, buildPointsToMapForFile). Confirmed identically for a class-field arrow (run = (x) => {...}) and an object-literal arrow-valued property ({ run: (x) => {...} }), both .js and .ts, where the actual key is <module>::r while findCaller returns ClassName.run/varName.run.

Also confirmed (collision case, real pipeline): two DIFFERENT TS classes each with their own run instance method and their own for-of over a different array both collapse onto the SAME bare key run::rpts.get('run::r') unions both classes' array elements. findCaller itself still correctly distinguishes ClassA.run vs ClassB.run for the two calls; only the pts key the extractor wrote is collapsed. Any consumer that falls back to the bare/unqualified enclosing-scope name (as a tolerance for root causes (a)/(b) above) inherits this pre-existing cross-class collision risk — bounded to over-crediting an unrelated same-named method's array contents as reachable, never to hiding a real one.

Why this wasn't fixed inline in #2088's plan

buildForOfConstraints/enclosing_func_context are shared, pre-existing primitives — the SAME funcStack/context-collector (TS) and the SAME enclosing_func_context walk (Rust) also supply the scope prefix for param-flow constraints, array-elem constraints, spread-arg constraints, and array-callback constraints (buildParamAndArrayConstraints and its Rust mirror), not just for-of. Fixing the root cause — qualifying TS class methods correctly and adding field_definition/pair cases — means changing a primitive every existing pts consumer in both engines depends on, for a plan (#2088) that is scoped to object-literal-property correlation, not to the extractor's general scope-naming. That is a larger, independently-scoped fix with its own blast radius across both engines, not something to fold into #2088's WU-5(a). #2088's own fix (round 28) instead widens resolveReceiverSites's reader-side probe set to tolerate the known divergence (trying the caller-name-derived scope, its last-dot segment, and the '<module>' sentinel, in addition to the existing bare fallback) rather than eliminating it.

Suggested fix (out of scope here, filed for its own PR)

  • (a): make the class-name lookup that feeds pushMethodDefContext/enclosing_func_context's method_definition arm node-type-agnostic (accept type_identifier alongside identifier), matching findParentClass/find_parent_class's already-correct behavior.
  • (b): add field_definition/public_field_definition and object-literal-pair cases to the context-collector dispatch (TS pushEnclosingContext / Rust enclosing_func_context's match), so a class-field arrow or object-literal arrow-valued property establishes its own qualified enclosing-scope context the same way a method_definition or variable_declarator arrow already does.

Either change should be verified against every existing pts consumer keyed by enclosingFunc/enclosing_func_context (not just for-of), since it changes a shared primitive's output shape for names that were previously bare.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions