Skip to content

fix(ts): a call to a parameter of any type links nothing elsewhere - #2732

Merged
colbymchenry merged 1 commit into
mainfrom
claude/param-call-gate
Oct 21, 2026
Merged

colbymchenry merged 1 commit into
mainfrom
claude/param-call-gate

Conversation

@colbymchenry

Copy link
Copy Markdown
Owner

Summary

A name the calling JS/TS function binds itself is that binding. #2226 holds a call to a parameter or local to that rule, so it never reaches a same-named function elsewhere. Three gaps let such calls through:

  1. The parameter reader stopped at the first parenthesis. It read a type or default as running "to a comma or a parenthesis", so it never saw a parameter typed as a function, defaulted to one, or listed after one:
    • bitwarden's pick(formats, isAvailable: (f: string) => boolean) calling isAvailable(f) linked to the napi isAvailable;
    • vscode's merge editor's compare(a, b) on a comparator parameter linked to strings.ts's compare;
    • mastodon's setTimeout(callback, delay) inside useCallback((callback: () => void, delay: number) => …) linked to the delay callback declared below it.
  2. A parameter never hid the file's own import of its name. The import resolver answers before the matcher's check runs. So new Promise((resolve) => { … resolve(x) }) in a file importing path's resolve linked to path.resolve, and a telemetry: TelemetryData parameter passed on linked to the imported telemetry function.
  3. A name ending in $ was read as the name without it. \b stops before a $. So legend-state's const syncState$ = syncState(obs$) counted as declaring syncState, and const $el counted as declaring $, while function $() did not.

Follow-up 3 of 3 from #2718.

Fix

All of it is in src/resolution/name-matcher.ts, plus one gate in index.ts.

Parameter reader (localBindingPatterns' param):

  • A type or default is a run of pieces (PARAM_PART). A piece is a character other than a comma, or a balanced (…), {…}, […] or <…> group up to two levels deep, which may hold commas.
  • Every piece starts with a character of its own: => is a function type's arrow, = not followed by > starts a default, and a < or > spaced like an operator is no bracket. A failing search therefore stays linear. On lists of 12 to 30 parameters typed (a: A, b: B) => R or Map<K, (v: V) => W> = cond > 1 ? a : b, it takes microseconds.
  • A parameter property's modifiers (private readonly store: Store) and an earlier destructured parameter are passed over.
  • bindsAtSites finds the ( that opens a list by reading back over balanced groups (enclosingOpenParen). It no longer looks for "the last ( with no ) between", which no list holding a function type passes.

Names end at (?![\w$]) in every binding pattern, never at \b.

Import shadow (isImportShadowedByJsBinding, a gate in resolveOne after resolution):

  • An import-resolved target with the reference's name is dropped when the calling function binds that name in a scope that holds at the reference.
  • jsBindingEnclosesReference reads the function's text from its first line to the reference's column. Comments, strings, and template and regex literals are blanked first.
  • A binding counts only while its scope is still open:
    • A parameter list's body: a function's or a catch clause's block, or an arrow's block or expression. An expression ends at a comma, a semicolon, the bracket around it, or a line that starts the next statement where no semicolon ends the last.
    • A lone arrow parameter (resolve => …), only where an expression starts. The name a return type ends with ((): Bounds =>) doesn't count.
    • A const/let/var, function or class declaration's block, or a for head's statement.
  • An object type a function returns is not its body: in delta(…): { removed: T[]; added: T[] } {, the body starts at the second {.
  • Never hidden:
    • an interface or type alias: in TypeScript a value never hides a type;
    • const X = require('./x').default: it is the import.
  • The per-file isLocallyBoundJsName, read once per file and name, answers before the read. So the read only runs where the file binds the name at all.
  • The name-match path keeps fix(js): a name the calling function binds is its local #2226's check of the function's whole text through the reference line.

Decisions (made without asking)

  • A shadowed import gets no edge, never a fallback guess. A local function's own node, like a function resolve() {} nested where resolve is imported, isn't linked in its place. Main linked the import there.
  • Destructured parameters stay out (({ compare }) => compare()). fix(js): a name the calling function binds is its local #2226 left destructured declarations out the same way: const { t } = useI18n() usually re-binds the same function.
  • A local alias with the same name reads as the local. vscode's const $ = dom.$; $('div') is one (2 edges). Every other name already behaved this way; it only linked before because \b hid the $ local. Following such an alias to its target is a separate change.
  • What the reader can't place keeps the import, main's answer. That covers a var declared in a closed block, a comma inside an arrow expression's generics, and a function longer than 100,000 characters up to the reference.
  • The per-file check is unchanged in scope. A parameter anywhere in a file still blocks cross-file candidates for the file's bare calls, as before; only its reader is better.

Tests

__tests__/js-function-local-binding.test.ts has 12 tests: the 2 existing lodash ones and 10 in a new describe, built from the shapes the A/B found.

  • Linked to nothing:
    • a parameter typed as a function, defaulted to one, listed after function and generic types, or declared as a parameter property;
    • the same in an IIFE no function node holds;
    • a Promise executor's resolve, in both the (resolve) => and resolve => forms;
    • a for (const resolve of …) body;
    • a nested function resolve;
    • a parameter whose function returns an object type.
  • Still linked:
    • an import called from a function that binds nothing;
    • resolve(file) after an executor or a for statement;
    • a call after an arrow const with no semicolon;
    • const syncState$ = syncState(obs$);
    • const X = require('./x').default; new X();
    • (): Widget => new Widget();
    • a Size type annotation next to a Size parameter.

Each mutation below makes at least one test fail:

Mutation Fails
old type alphabet ([^,()]) 5 tests
import gate off 3 tests
old list-start rule (last (, no ) between) the IIFE test
function's whole text, no scope read 5 tests
names end at \b the syncState$ test
a require local hides the import the require test
a for head's declaration scoped like a block's the for test
no statement ends at a line break the no-semicolon test
a return type holds no object type the object-return test
no lone arrow parameter the resolve => test
no nested function/class declaration the nested-function test
a return type's name read as a lone parameter the (): Widget => test
a value hides a type the Size test

A/B

Arms. Dist and kernel snapshots of main 009e4377 and of this change on it. The head here is the same change rebased onto 5994edb3; #2730 and #2731 touched name-matcher.ts only away from these hunks. Each repo got codegraph init -y per arm, with two resolver workers for vscode and frontend. Edges were diffed by natural key (an SQL EXCEPT for vscode).

  • Every removed edge was read.
  • Every removed import-resolved edge was checked against the binding the reader found in scope, with a script that applies the same rules.
repo edges −/+ what changed (all wrong links removed)
microsoft/vscode −344 / +3 listed below
excalidraw −17 11 calls to an fn parameter that linked to TTDDialog's fn; newElement as an arrow parameter or a const newElement = this.state.newElement (3); line/curve parameters passed on as values (3)
bitwarden −5 generator, target, configType and t (new t(…) on a new (r: any) => T parameter) parameters; IconComponent(…) on a function-typed parameter
frontend −7 equal, setValue, getEntityId and localize parameters typed as functions
outline −7 resolve parameters typed as functions (2), a Promise executor's resolve, toggleBlock, build, replace, diff
legend-state −6 get/set parameters that linked to ObservableObject's; syncState parameters passed on
cord-field, happa, mastodon −1 each a plugin parameter; a Component: C rendered as <Component>; useTimeout's delay
bookbrainz-site, dho-web, ghostfolio, jira-clone, next-saas, takenote identical —

vscode:

  • 84 import-resolved links where a binding is in scope (60 parameters, 22 declarations, 2 lone arrow parameters):
    • Promise executors' resolve → path.resolve;
    • glob's basename?: string → path.basename;
    • telemetry: TelemetryData → the imported telemetry function;
    • transaction?: ITransaction → the observable transaction;
    • const dispose = () => … → lifecycle's dispose;
    • const status = … → aria's status;
    • size: IViewSize → dom's size;
    • …
  • 258 name-matched or function-ref links from a call or value use of a parameter. 242 are the calling function's own. 16 belong to an enclosing function, like debounce(event, merge, …)'s merge called in a nested callback, or are a destructured option, like { now = Date.now }, in a file that also has a now: () => number parameter.
  • 2 $ alias calls, as described under Decisions.
  • +3 type references the old reader took for parameter names. In (tree: WorkbenchAsyncDataTree<ITreeItem, ITreeItem, FuzzyScore>), ITreeItem read as a second parameter, so the references were dropped as shadowed.
  • 2,252 calls to a file's own $ helper keep their target, at 0.9 instead of 0.7. The file's function $() or const $ = … now counts, so other files' $ drop out of the pool.

Earlier runs, all fixed here before this head:

  • The first import gate read a function's whole text. It cut bitwarden's new OsBiometricsServiceMac(…) after a const OsBiometricsServiceMac = require(…).default, and legend-state's syncState(obs$), which is the $ bug.
  • A scope reader that saw neither lone arrow parameters nor object return types kept wrong import links in vscode: resolve() inside new Promise<void>(resolve => …), and the parameters of statWithLink(fsPath: string): Promise<{ … }> and delta(…, compare: …): { … }.
  • A lone-parameter rule that matched (): CipherView => cut 32 bitwarden constructions like new CipherView(), and 36 type annotations in bitwarden, excalidraw and frontend.

Cost.

  • The gate's own time, from the resolver's stage profiler (CODEGRAPH_RESOLVE_PROFILE=2, summed over workers): 0.6 s of 34.9 s of resolver CPU on outline, and 2.2 s of 141 s on bitwarden. That is under 2%: about 20 µs per call and 45 µs per reference.

  • Check order:

    1. The target's kind comes first, so a type annotation never pays for a read.
    2. Then the per-file binding check, read once per file and name.
    3. Only then the read of the function's text.

    Reordering the checks changed no edge; both repos diff empty against the A/B's indexes.

  • Whole-index CPU, interleaved, median of 3: outline 48.6 s → 50.7 s and bitwarden 143.2 s → 144.6 s. Paired runs on this machine spread ±7%, so that's within noise.

🤖 Generated with Claude Code

A JS/TS name the calling function binds as a parameter is that parameter,
never a same-named function in another file, as the file's own were made
in #2226. The parameter reader took a type or default to be anything up to
a comma or a parenthesis, so a parameter typed as a function, defaulted to
one, or after one was never seen: bitwarden's isAvailable(f) on a callback
typed (f: string) => boolean linked to the napi isAvailable, and vscode's
merge editor's compare(a, b) on a comparator parameter to strings.ts's.

A type or default may now hold balanced parentheses, braces, brackets and
generics, a parameter property's modifiers are read, and an earlier
destructured parameter is passed over; each alternative still has one
parse. A name ends where no identifier character follows, so a const
syncState$ no longer declares syncState.

The parameter also shadows the file's own import of the name, which the
import resolver answered before the check ran, but only where it is in
scope: in the body of the function, arrow or catch clause that binds it,
or the block or for statement of a const, let, var, function or class,
read from the function's text up to the reference. A lone arrow
parameter counts, and an object type a function returns is no body. A
Promise executor's resolve is the calls inside it, never a resolve(file)
after it, and a const X = require('./x').default is the import itself.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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