Skip to content

fix(resolution): follow alias bindings to the function they name - #1485

Open
valkyriweb wants to merge 1 commit into
colbymchenry:mainfrom
valkyriweb:fix/alias-binding-resolution
Open

fix(resolution): follow alias bindings to the function they name#1485
valkyriweb wants to merge 1 commit into
colbymchenry:mainfrom
valkyriweb:fix/alias-binding-resolution

Conversation

@valkyriweb

Copy link
Copy Markdown

Fixes the second of the two root causes behind #1482.

The bug

When a name is bound to nothing but another symbol, resolution stops on the binding — one hop short of the callable:

export const alias = fn;          // consumer calls alias()
export { fn as alias };           // the declaration has is_exported = 0
export const api = { run: fn };   // consumer calls api.run()
const local = fn;                 // same-file indirection

An edge is produced, so nothing looks broken. But callers fn omits every caller that went through the alias and reports a confident 0, while callers alias finds them. impact under-reports the same way. That is the failure mode from #1482 — the graph is silently wrong rather than visibly incomplete, which is the expensive kind.

The fix

Two parts:

1. src/resolution/alias-binding.ts (new)

  • aliasTargetName — reads the initializer already stored in Node.signature (= implFn, = { run: implFn }) and returns the aliased name. No re-parsing.
  • resolveAliasBinding — resolves that name to a callable.
  • extractLocalExportAliases — parses local export { impl as alias } clauses.

2. resolveOne takes one extra hop when the resolved target turns out to be an alias binding.

The hop is applied once in resolveOne rather than per-strategy, because any strategy can land on an alias.

3. getFileExportIndex now indexes all declarations plus local export clauses, so export { impl as alias } binds to impl — previously the declaration carried no export keyword, wasn't flagged isExported, and the importer's request for alias fell through to the name-matcher.

Safety

The conservative choices are deliberate — a wrong edge is worse than a missing one:

  • calls refs only. A references edge to the binding is correct as-is and is left alone.
  • No hop on ambiguity. A same-file callable is preferred; a cross-file one is accepted only when it is unique project-wide. Otherwise: no hop.
  • Confidence capped at 0.85, so an inferred hop never outranks a directly resolved edge.
  • Genuine wrappers are untouched. export const wrapper = () => fn() is a real function; the call site calls it.

Tests

__tests__/alias-binding-resolution.test.ts, 6 cases: the four shapes above, plus two negative guards (genuine wrapper stays put; ambiguous name yields no hop).

On main the four positive cases fail and both negative guards pass. With the fix all six pass.

Specifiers in these fixtures are extensionless on purpose, so this PR stands completely independent of the .js-specifier companion.

Verification

  • full suite on this branch: 2,536 passed, 0 failed, 175 skipped (three consecutive runs)
  • the 175 skips are the kernel-*-parity files, expected without a native kernel build (CODEGRAPH_KERNEL=0)
  • one flaky failure appeared in an earlier run and did not reproduce in the three runs since; I could not attribute it to this change

Not covered

  • Kernel path unvalidated — I did not build the Rust kernel. The change sits in the TS resolution layer above it.
  • Only TypeScript-family export shapes are handled. Swift/Kotlin/Go/Python still name-match.
  • There is an unresolved_refs table with status = 'failed' that already records misses like these, and callers/impact never surface it. Turning silent wrong answers into caveated ones looks like the cheapest remaining trust win, but it's out of scope here.

Companion PR for the first root cause (.js specifiers) is independent and can land in either order; both touch import-resolver.ts in different regions, so whichever lands second may need a trivial rebase.

A name bound to nothing but another symbol resolved to the BINDING, one hop
short of the callable:

  export const alias = fn;          // consumer calls alias()
  export { fn as alias };           // declaration has is_exported = 0
  export const api = { run: fn };   // consumer calls api.run()
  const local = fn;                 // same-file indirection

The edge existed, so nothing looked broken — but `callers fn` omitted every
caller that went through the alias and reported a confident zero, while
`callers alias` found them. `impact` under-reported the same way.

`resolveOne` now takes one extra hop when the resolved target is an alias
binding, retargeting `calls` refs only (a `references` edge to the binding is
correct as-is). The initializer is already stored in `Node.signature`, so no
re-parsing is needed.

Ambiguity yields no hop: a same-file callable is preferred, a cross-file one is
accepted only when it is unique project-wide, and confidence is capped at 0.85.
A wrong edge is worse than a missing one.

Refs: colbymchenry#1482
@valkyriweb

Copy link
Copy Markdown
Author

Companion: #1484 (.js specifiers). Independent — either order; whichever lands second may need a trivial rebase of import-resolver.ts.

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.

1 participant