Skip to content

functionScopeDeclaresVar treats a var inside a class static block as hoisting to the enclosing function #2644

Description

@carlos-alm

Found during the /oversee review of #2088's plan (PR #2612, round 22).

What

Neither engine's FUNCTION_SCOPE_NODE_TYPES list includes class_static_block:

  • src/extractors/javascript.ts:4634-4641
  • crates/codegraph-core/src/extractors/javascript.rs:4698-4705

functionScopeDeclaresVar (src/extractors/javascript.ts:4660-4672) walks every child of a function body looking for a hoisted var, skipping only children whose type is in FUNCTION_SCOPE_NODE_TYPES:

for (let i = 0; i < node.childCount; i++) {
  const child = node.child(i);
  if (!child) continue;
  if (FUNCTION_SCOPE_NODE_TYPES.has(child.type)) continue;
  if (functionScopeDeclaresVar(child, name, depth + 1)) return true;
}

Since class_static_block isn't in that set, the walk descends straight through class_declarationclass_bodyclass_static_blockstatement_block and finds a var declared there — even though a class static block's own statement list is collected in ECMA-262 via the same TopLevelVarDeclaredNames/TopLevelVarScopedDeclarations static-semantics operations used for a function body, not the plain VarDeclaredNames used for an ordinary nested block, making it a var-scope boundary in its own right rather than a transparent block; a var inside static { ... } is scoped to the block itself and never hoists to any enclosing function. Verified against the real grammar (tree-sitter-javascript@0.25.0's node-types.json: class_static_block carries its own body field, a statement_block) and against real Node:

function fnA() { return 1; }
function sink(x) { return x.alpha(); }
const T = { alpha: fnA };
function go() {
  class C { static { var T = 1; } }   // var is scoped to the STATIC BLOCK, not to go()
  sink(T);                            // genuinely reads the module-level T
}
go();

go() returns 1sink(T) genuinely reaches the module-level T. But functionScopeDeclaresVar(go's body, 'T') returns true (it finds the static block's var T = 1 and, having no reason to stop at the static block's own boundary, attributes it to go), so introducesShadowedBinding(go, 'T') also returns truego looks like it shadows T, even though it doesn't.

Control isolating the cause to the var path specifically: the identical shape with let T = 1 in the static block does not trigger this — functionScopeDeclaresVar only ever tests node.type === 'variable_declaration', never lexical_declaration, so it was never going to see a let here regardless of the class_static_block gap.

Reproduces identically across every function-shape container (function_declaration, arrow_function, method_definition, ...), with the static block held directly, inside a class expression, inside a class-field-held class expression, and nested two levels deep — the defect is in the walk itself, not in any one AST shape around it.

Why it matters

Direction: UNDER-escape — not recall-only, the opposite of #2643's direction. functionScopeDeclaresVar's only current consumer, introducesShadowedBinding's function-shape case, uses a spurious true here to prune a subtree from a reference walk (blockContainsIdentifierExcluding) that would otherwise find a genuine reference inside it. A subtree that's wrongly pruned hides a real reference outright — this is a false-positive shadow, not a missed one (contrast #2643, where a missed shadow only ever adds a spurious candidate to a conjunction, which can under-correlate but never hides an existing, already-reached reference). Concretely, this is exactly the shape of dead-code false positive this whole review has been hunting: a function that genuinely invokes something gets pruned from the search, a this whole search finds nothing result gets trusted, and a live symbol is reported dead.

#2088's own new escape-analysis design (allReferencesTracked, PR #2612 / WU-2) is not exposed to this in its shipped form: the plan's round-22 revision routes its own body-hoist check through a locally re-derived helper that additionally skips class_static_block, rather than through the shared, unmodified functionScopeDeclaresVar. The already-shipped consumer — introducesShadowedBinding, used today by the fallback-value-ref dead-code check predating this plan (PR #2432) — has no such workaround and remains exposed in production.

Why it wasn't fixed inline

Widening functionScopeDeclaresVar (or FUNCTION_SCOPE_NODE_TYPES) changes behavior for every consumer built on them — introducesShadowedBinding's full switch (for_statement, statement_block, switch_body, catch_clause cases too), and anything else that leans on FUNCTION_SCOPE_NODE_TYPES's current six-member definition. Every round of the #2088 review has declined to touch these two primitives directly for that reason, fixing locally in the new consumer's own re-derivation instead (mirroring the same discipline round 18 established for a different bug in the same function). Doing it here would need its own review of every existing consumer, not only the new one.

Suggested fix shape (not binding)

Add a class_static_block case to functionScopeDeclaresVar's own skip test (FUNCTION_SCOPE_NODE_TYPES.has(child.type) || child.type === 'class_static_block'), or add class_static_block to a new, dedicated boundary set if FUNCTION_SCOPE_NODE_TYPES's existing six members are relied upon elsewhere to mean "function-shaped, with name/parameters/body fields" (a class_static_block node has no such shape, only a body field). Re-verify introducesShadowedBinding's own behavior afterward, since a newly-detected non-shadow could in principle change something for its for_statement/statement_block/switch_body cases too, even though none of those currently descend through a class body in a way this gap would affect.

Mirror in crates/codegraph-core/src/extractors/javascript.rs (function_scope_declares_var, lines 4724-4745) — confirmed to carry the byte-identical gap, same six-member FUNCTION_SCOPE_NODE_TYPES list, same unbounded recursion into any non-listed child type.

Provenance note

Found during the round-22 revision of PR #2612 (docs/plans/issue-2088.md), verified against the real tree-sitter-javascript@0.25.0 grammar and eight executed Node repros exercising the trigger (base shape, five further function-shape containers, and the identical trick placed one hop into each of the plan's own rebinding-alias and for-of recursions), plus two further executed controls (a let-vs-var swap, and a genuine non-static-block var shadow) proving the shape does not over- or under-fire, before filing.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions