Skip to content

[finding] splitTopLevel counts the > of => as a closing bracket, so every parameter and argument after an arrow-typed one is swallowed #13291

Description

@os-elon

Found by measurement while fixing #13227 (parseParamNames stripping only one leading modifier). It is a different function and a different mechanism in the same file, so it is recorded here rather than ridden into that PR.

The shape

splitTopLevel in scripts/check-dispatcher-error-vocabulary.mjs tracks bracket depth so a nested comma is not read as a separator. Its bracket set includes the angle brackets, for generics:

if (c === '(' || c === '[' || c === '{' || c === '<') depth += 1;
else if (c === ')' || c === ']' || c === '}' || c === '>') depth -= 1;

An arrow contributes a > with no matching <. So () => void drives depth to -1, and every subsequent top-level comma is at -1 rather than 0 — never a separator again. The whole remainder of the list collapses into one entry. A comparison operator does the mirror image: a < b, c > d puts the comma at depth 1.

Measured through the real exported function:

"a: () => void, b: string"                    ->  ["a: () => void, b: string"]          <- 1 part, expected 2
"a: (x: T) => U, code: string, msg: string"   ->  [<the whole string>]                  <- 1 part, expected 3
"() => x, 'CODE'"                             ->  [<the whole string>]                  <- 1 part, expected 2
"a < b, c > d"                                ->  [<the whole string>]                  <- 1 part, expected 2
"onDone, code, msg"                           ->  ["onDone"," code"," msg"]             <- control, correct

Why it matters — it is the same silent-drop class, in BOTH of this file's uses

splitTopLevel is called twice, and the defect reaches both:

  • parseParamNames(decl.params) — the parameter names helperCodesFor searches with indexOf(ident);
  • splitTopLevel(args) inside helperCodesFor — the call-site argument list whose Nth element is read.

End to end through the real deriveSites, with an unregistered code and a same-genre positive control one arrow apart:

function fail(onDone: () => void, code: string) {
  const e = new Error('x');
  (e as any).code = code;
  return e;
}
export function deny() { throw fail(() => {}, 'ARROW_TYPED_HELPER_CODE'); }
arrow-typed helper : sites: []   unresolved: []
control (onDone: VoidFn, called with `noop`) : sites: [{ shape: 'codehelper', code: 'CONTROL_HELPER_CODE' }]

No site, no unresolved, nothing reported — the declared bound this gate states for itself (a value it cannot reduce is REPORTED, never dropped) does not hold, exactly as in #9223 / #9460 / #10918 / #13131 / #13226 / #13227.

The wrong-INDEX hazard #13227 records applies here too and is arguably worse: the argument splitter and the parameter splitter can be wrong by different amounts on the same helper, so a resolved index can name a value from another argument position rather than resolving to nothing.

Blast radius, measured on packages/** non-test source

Ground truth is the repo's own TypeScript 6.0.3 AST, over every parameter list the gate's DECL_HEADER_RE reaches and that is a valid parameter list (5979 of them):

after #13227 lands, parseParamNames agrees with the AST on   5919 / 5979  (99.00%)
remaining disagreements                                          60

All 60 of that residue are this defect. Examples, ours vs the AST:

packages/triggers/trigger-schedule/src/schedule-trigger.ts:168
  getJobService: () => JobServiceSurface | null, logger: TriggerLogger
  ours ["getJobService"]   ast ["getJobService","logger"]

packages/rest/src/rest-server.ts:914
  ours 6 names   ast 20 names

packages/spec/src/conversions/walk.ts:57
  region: unknown, path: string, mapper: (node: Dict, path: string) => Dict, depth: number
  ours ["region","path","mapper"]   ast ["region","path","mapper","depth"]

0 unregistered SCREAMING_SNAKE codes are hiding behind it on this tree today — the same property #13226 and #13227 measure — so the value is prevention, not a live escape.

What a fix would have to decide

Not decided here:

  • whether > should simply not decrement when it is the > of => (cheap, and it leaves the a < b, c > d comparison case still wrong), or whether angle brackets should be dropped from the depth set entirely and generics handled another way. The two differ in what they do to Map<string, number>, which the current --self-test pins;
  • whether the same reasoning applies to sliceBalanced, which shares the bracket vocabulary but not the </> pair — it should be re-read alongside rather than assumed clean;
  • scripts/check-cross-package-test-inputs.mjs carries its own splitTopLevel (line 541) with a different bracket set. Whether it has the same defect is unmeasured here.

Filed unassigned, ungraded and unrouted — domain:*, priority and type are triage's to produce. Related: #13227 (the modifier run, fixed), #13226 (the class-method declaration form), #13233 (the object-literal stamp position) — four distinct mechanisms, one failure class.

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions