Skip to content

fix(native): skip callback-reference calls for this()/super() call expressions#1548

Merged
carlos-alm merged 3 commits into
mainfrom
fix/native-fun-classes2-cha-1543
Jun 15, 2026
Merged

fix(native): skip callback-reference calls for this()/super() call expressions#1548
carlos-alm merged 3 commits into
mainfrom
fix/native-fun-classes2-cha-1543

Conversation

@carlos-alm

Copy link
Copy Markdown
Contributor

Summary

  • In handle_call_expr (Rust JS extractor), when the callee node is this or super used as a function (this(b), super(a)), extract_callback_reference_calls was running and emitting identifier arguments as spurious dynamic calls
  • The pts resolver then matched those names globally, producing false cross-file call edges (e.g. fun/fun.js:foo → spread/spread.js:b, classes2/classes2.js:G.constructor → prototypes3/prototypes3.js:a)
  • Fix: add an early return in handle_call_expr for fn_node.kind() == "this" || fn_node.kind() == "super", mirroring the existing guard in the TS handleCallExpr (javascript.ts:1135)
  • Add two regression tests covering both this(b) and super(a) argument non-emission

Root cause

The JS extractor (javascript.ts) has an early return at line 1135 when the function node type is this:

if (fn.type === 'this') {
  ctx.calls.push({ name: 'this', line: nodeStartLine(node) });
  return; // no further processing needed for this()-style calls
}

The Rust extractor (javascript.rs) only checked for fn_node.kind() == "import" before calling extract_callback_reference_calls, so this(b) / super(a) calls would emit b and a as dynamic calls. The pts resolver then resolved those names globally, finding same-named functions in sibling fixture files.

Test plan

  • Two new Rust unit tests pass: this_call_args_do_not_emit_callback_reference_calls, super_call_args_do_not_emit_callback_reference_calls
  • All 408 Rust unit tests pass
  • All 180 JS test files pass (3048 tests)
  • node scripts/parity-compare.mjs --langs jelly-micro eliminates all 6 targeted false-positive edges (13 → 12 divergences; the 6 fixed are the exact ones listed in fix(parity): native emits extra dynamic CHA edges for fun/classes2 fixtures #1543; the remaining 12 are pre-existing issues)

Closes #1543

…pressions

In handle_call_expr, when the callee node is `this` or `super` (e.g. `this(b)`,
`super(a)`), extract_callback_reference_calls was running and emitting identifier
arguments as spurious dynamic calls. The pts resolver then matched those names
globally, producing false cross-file call edges in the jelly-micro fixture:
  fun/fun.js:foo → spread/spread.js:b, fun/fun.js:bar → prototypes3/prototypes3.js:c, etc.

Fix: add an early return for fn_node.kind() == "this" || fn_node.kind() == "super",
mirroring the existing guard in the TypeScript handleCallExpr (javascript.ts:1135).
Add two regression tests covering both cases.

Closes #1543
@greptile-apps

greptile-apps Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR addresses a previous review comment about a vacuously true assertion in the super_call_args_do_not_emit_callback_reference_calls test, and corrects an issue-reference comment from #1511 to #1543. The underlying production fix (early return in handle_call_expr for this/super callees) was already committed.

  • The super(a, b) fixture replaces the old super(a); this.bb = b; fixture so that b is a genuine second argument to super() — both assertions now meaningfully exercise the guard.
  • The parent class constructor is updated accordingly (constructor(c, d), this.dd = d) to remain a valid parse target.
  • The issue reference in the this_call_args doc comment is corrected from #1511 to #1543.

Confidence Score: 5/5

Safe to merge — changes are test-only, directly address the prior review comment, and do not touch production logic.

The diff is entirely within the test module: one fixture update that turns a vacuously true assertion into a meaningful one, and one corrected issue-reference in a doc comment. The production guard was already in place; these changes only tighten the test coverage around it.

No files require special attention.

Important Files Changed

Filename Overview
crates/codegraph-core/src/extractors/javascript.rs Test-only changes: corrects a vacuously true assertion in the super test fixture by using super(a, b) so both arguments are real call args, and fixes an issue-reference comment (#1511#1543). No production logic changed in this diff.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[handle_call_expr] --> B{fn_node kind}
    B -->|import| C[handle_dynamic_import then return]
    B -->|this or super| D[early return - no callback-reference calls emitted]
    B -->|other| E[extract_call_info - push call record]
    E --> F[extract_callback_definition]
    F --> G[extract_callback_reference_calls]

    H[match_js_pts_bindings walk] -->|call_expression| I[collect_this_call_and_bindings]
    I -->|fn is this| J[push Call named this]
    I -->|fn is super| K[no call recorded - constructor delegation]
Loading

Reviews (3): Last reviewed commit: "test(native): strengthen super-call test..." | Re-trigger Greptile

Comment on lines +4144 to +4149
assert!(
!s.calls.iter().any(|c| c.name == "b"),
"argument `b` of this.bb = b must not become a callback-reference call; got: {:?}",
s.calls.iter().map(|c| (&c.name, c.dynamic)).collect::<Vec<_>>()
);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Second assertion is vacuously true — doesn't exercise the fix

The second assert! checks that b is not emitted as a dynamic call, but b only appears in the assignment this.bb = b, not in any call expression. extract_callback_reference_calls only walks call_expression arguments, so b would never be emitted regardless of this fix. If the intent is to verify that both arguments to a multi-argument super call are suppressed, the fixture should use super(a, b) instead.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — changed the fixture from super(a); this.bb = b; to super(a, b); so that b is a genuine call argument rather than an assignment target. Both assertions now actually exercise the fix by verifying that neither argument of a two-argument super() call is emitted as a dynamic callback-reference call.

@github-actions

github-actions Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Codegraph Impact Analysis

1 functions changed0 callers affected across 0 files

  • super_call_args_do_not_emit_callback_reference_calls in crates/codegraph-core/src/extractors/javascript.rs:4261 (0 transitive callers)

docs check acknowledged: no language support, feature list, or design decision changes
@carlos-alm

Copy link
Copy Markdown
Contributor Author

@greptileai

@carlos-alm
carlos-alm merged commit 33bcd93 into main Jun 15, 2026
33 checks passed
@carlos-alm
carlos-alm deleted the fix/native-fun-classes2-cha-1543 branch June 15, 2026 08:48
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 15, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(parity): native emits extra dynamic CHA edges for fun/classes2 fixtures

1 participant