Skip to content

TS resolution precision regression from PR #947: member_expression args mis-extracted as callback refs #971

@carlos-alm

Description

@carlos-alm

Summary

The regression-guard.test.ts check fails on main with:

```
typescript precision: 100.0% → 93.8% (−6.2pp, threshold 5pp)
```

This is a genuine precision regression surfaced when PR #958 committed v3.9.4 benchmark data. The regression predates PR #970 — CI on main is red with the same failure and it reproduces on any freshly-checked-out branch.

Root cause

PR #947 (`feat(js-extractor): resolve named function references passed as arguments`) added `extractCallbackReferenceCalls` in `src/extractors/javascript.ts`. The function iterates over every argument of every call expression and emits a dynamic call edge for each `identifier` or `member_expression` argument.

That's correct for real callback positions like:

  • `router.use(handleToken)`
  • `app.use(auth.validate)`
  • `promise.then(onSuccess)`
  • `items.map(transform)`

But wrong for plain method arguments where a `member_expression` is just a property read, not a callback reference. Example from the TS benchmark fixture `repository.ts`:

```ts
save(user: User): void {
this.store.set(user.id, user); // user.id is a property read
}
```

The extractor emits a dynamic call `save → id` with receiver `user`. The edge resolver then matches that against `User.id` (the `property_signature` extracted from `interface User { id: string; ... }` as a `method` node), producing a false positive:

```

This flips TS resolution from 15/15 (100%) in 3.9.3 to 15/16 (93.8%) in 3.9.4.

Reproduction

```bash
git checkout main
npx vitest run tests/benchmarks/regression-guard.test.ts

FAIL: resolution — 3.9.4 vs 3.9.3

```

See the FP directly:

```bash
npx vitest run tests/benchmarks/resolution/resolution-benchmark.test.ts -t typescript --reporter=verbose

Unexpected edges (false positives):

+ UserRepository.save@repository.ts -> User.id@types.ts

```

Candidate fixes

  1. Restrict `extractCallbackReferenceCalls` to `identifier` args only — drops the `member_expression` branch. Would regress the `app.use(auth.validate)` test but removes the whole class of property-read FPs.
  2. Arity heuristic — only emit callback refs when the call has ≤1 argument. `app.use(x)` and `promise.then(x)` fit; `store.set(user.id, user)` does not.
  3. Receiver/callee allowlist — treat arg as callback only when the callee method is in a known list (`use`, `then`, `catch`, `map`, `filter`, `forEach`, `on`, `addEventListener`, etc.). Most precise, most maintenance.
  4. Resolver-side: don't resolve property-read names against `property_signature`-derived methods — attacks the other half of the pipeline.

Needs follow-up discussion on which trade-off to accept before implementing.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    follow-upDeferred work from PR reviews that needs tracking

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions