Skip to content

Commit 68a9d47

Browse files
Elon Muskclaude
andauthored
fix(scripts): parseParamNames strips the full TypeScript modifier run, so a parameter property is no longer read as readonly (#13292)
* fix(scripts): parseParamNames strips the full TypeScript modifier run `private readonly code: string` parsed as a parameter named `readonly`: the strip was one anchored alternation carrying a `g` flag, and `^` with no `m` matches at position 0 once, so exactly one modifier came off. `helperCodesFor`'s `indexOf(ident)` then answered -1 and the whole code helper was dropped in silence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CPrUz21stTFhJRUirdc4yw * test(scripts): pin the parameter-property modifier run in --self-test Positive cases at every run length TypeScript admits (up to three: accessibility, override, readonly), negative cases for parameters genuinely NAMED after a modifier word, and the end-to-end pair through deriveSites with the single-modifier positive control beside it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CPrUz21stTFhJRUirdc4yw --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 39df668 commit 68a9d47

1 file changed

Lines changed: 132 additions & 3 deletions

File tree

scripts/check-dispatcher-error-vocabulary.mjs

Lines changed: 132 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1085,12 +1085,69 @@ export function splitTopLevel(args) {
10851085
return out;
10861086
}
10871087

1088-
/** Parameter NAMES, in order: `readonly a: T = x` → `a`. */
1088+
/**
1089+
* [#13227] The leading MODIFIER RUN of a TypeScript parameter, enumerated from
1090+
* the grammar rather than approximated.
1091+
*
1092+
* A parameter property admits an accessibility modifier, then `override`, then
1093+
* `readonly` — in that fixed order, up to THREE of them, and the compiler
1094+
* rejects any other order (`readonly public x` → "'public' modifier must
1095+
* precede 'readonly' modifier"; `readonly override x` → "'override' modifier
1096+
* must precede 'readonly' modifier"). Measured against the repo's own
1097+
* TypeScript 6.0.3 on a derived class, because `override` is only legal where a
1098+
* base class exists and a check on a standalone class reports it as a class
1099+
* error rather than as a parameter one. `static` and `abstract` are rejected on
1100+
* a parameter outright, and `in`/`out` are TYPE-parameter modifiers, which
1101+
* never appear in the value-parameter slice `enclosingDeclaration` hands over.
1102+
*
1103+
* The order is encoded rather than looped deliberately: a "strip any word in
1104+
* this set, repeatedly" loop accepts spellings TypeScript does not, which is
1105+
* the match-everything direction this gate has paid for before.
1106+
*/
1107+
const PARAM_MODIFIER_RUN = /^(?:(?:public|private|protected)\s+)?(?:override\s+)?(?:readonly\s+)?/;
1108+
1109+
/**
1110+
* Parameter NAMES, in order: `readonly a: T = x` → `a`,
1111+
* [#13227] `private readonly code: string` → `code`.
1112+
*
1113+
* The strip used to be a single anchored alternation carrying a `g` flag. The
1114+
* flag reads as "strip them all", but `^` with no `m` matches at position 0
1115+
* once, so exactly ONE modifier came off: `private readonly code: string`
1116+
* parsed as a parameter literally named `readonly`, and `helperCodesFor`'s
1117+
* `indexOf(ident)` then answered -1 — no site, no unresolved, the whole helper
1118+
* dropped in silence, one layer inside the same failure class as #9223 /
1119+
* #9460 / #10918 / #13131.
1120+
*
1121+
* ⚠️ A modifier word is only a modifier when a NAME follows it. `readonly`,
1122+
* `override` and the accessibility words are not reserved, so each is a legal
1123+
* parameter name in its own right — `override: MetricsRegistry | undefined` is
1124+
* live in this tree three times over. `override:` / `override?:` never enter
1125+
* the run (no whitespace follows the word), and the guard below covers the
1126+
* spaced spelling `readonly : T`, where the run would otherwise eat the
1127+
* parameter's own name and report nothing.
1128+
*
1129+
* ⛔ Textual on purpose, and the reason is measured rather than inherited.
1130+
* `enclosingDeclaration`'s `DECL_HEADER_RE` also matches `const x = someCall(`,
1131+
* so of the 7646 slices this function is handed on `packages/**` non-test
1132+
* source, 1218 are not a valid parameter list at all — they are ARGUMENT
1133+
* lists — and on 1072 of those a recovering TypeScript parse invents MORE THAN
1134+
* ONE parameter: `authService as any` becomes three confident parameters named
1135+
* `authService`, `as` and `any`; `await res.json()` becomes `await`, `res`,
1136+
* `json`. Those names are exactly what `helperCodesFor` searches with
1137+
* `indexOf(ident)`, so an AST route would MANUFACTURE the wrong-INDEX hazard
1138+
* this card only warns about — a finding that reads as ordinary while naming a
1139+
* value from another argument position — across a thousand slices. A textual
1140+
* reader degrades to one bad name instead of several. The over-matching header
1141+
* regex is #13226's subject and is deliberately untouched here.
1142+
*/
10891143
export function parseParamNames(params) {
10901144
if (!params.trim()) return [];
10911145
return splitTopLevel(params).map((raw) => {
1092-
const cleaned = raw.replace(/^\s*(?:readonly|public|private|protected|\.\.\.)\s+/g, '').trim();
1093-
const m = /^([A-Za-z_$][\w$]*)/.exec(cleaned.replace(/^\.\.\./, ''));
1146+
const rest = raw.trim().replace(/^\.\.\.\s*/, '');
1147+
const run = PARAM_MODIFIER_RUN.exec(rest)[0];
1148+
const tail = rest.slice(run.length);
1149+
const cleaned = /^[A-Za-z_$]/.test(tail) ? tail : rest;
1150+
const m = /^([A-Za-z_$][\w$]*)/.exec(cleaned);
10941151
return m ? m[1] : '';
10951152
});
10961153
}
@@ -1844,6 +1901,78 @@ function selfTest() {
18441901
'splitTopLevel counted a nested or templated comma as a separator',
18451902
);
18461903
ok(parseParamNames('readonly a: Map<string, number> = x, b?: string').join(',') === 'a,b', 'parseParamNames mis-read a parameter list');
1904+
1905+
// [#13227] The MODIFIER RUN, pinned at every length TypeScript admits and
1906+
// in the order it admits them. `private readonly code: string` parsed as a
1907+
// parameter named `readonly` because the strip was anchored-plus-`g`,
1908+
// which takes exactly ONE modifier off. The run is up to three long
1909+
// (accessibility → `override` → `readonly`), measured against the repo's
1910+
// own TypeScript rather than assumed from the card's two-modifier example.
1911+
for (const [params, expected, note] of [
1912+
['code: string, msg: string', 'code,msg', 'no modifier'],
1913+
['readonly code: string', 'code', 'one modifier'],
1914+
['private code: string', 'code', 'one modifier, accessibility'],
1915+
['private readonly code: string, private readonly msg: string', 'code,msg', 'two modifiers, both parameters'],
1916+
['public readonly code: string, msg: string', 'code,msg', 'two modifiers, mixed list'],
1917+
['protected readonly code: string', 'code', 'two modifiers, protected'],
1918+
['override readonly code: string', 'code', 'two modifiers, no accessibility'],
1919+
['private override readonly code: string', 'code', 'THREE modifiers — the maximum'],
1920+
['public override readonly code: string, msg: string', 'code,msg', 'three modifiers, mixed list'],
1921+
['private readonly code: Map<string, number> = x, b?: string', 'code,b', 'modifier run plus a generic default'],
1922+
['...rest: string[]', 'rest', 'rest parameter, unchanged by the run'],
1923+
]) {
1924+
ok(
1925+
parseParamNames(params).join(',') === expected,
1926+
`parseParamNames mis-read a parameter list (${note}): ${JSON.stringify(params)} → ` +
1927+
`${JSON.stringify(parseParamNames(params))}, expected ${JSON.stringify(expected.split(','))}`,
1928+
);
1929+
}
1930+
1931+
// ⛔ The NEGATIVE half — the strip must not become a match-everything.
1932+
// None of these words is reserved, so each is a legal parameter name, and
1933+
// `override` is one THREE TIMES in this repo's own `packages/**` source
1934+
// (`observability-service-plugin.ts`, `cache-service-plugin.ts`,
1935+
// `storage-service-plugin.ts`). A run that ate them would rename a real
1936+
// parameter and hand `helperCodesFor` a wrong INDEX — a finding that reads
1937+
// as ordinary while naming a value from another argument position.
1938+
for (const [params, expected, note] of [
1939+
['ctx: PluginContext, override?: ErrorReporter', 'ctx,override', 'a parameter named `override`'],
1940+
['override: MetricsRegistry | undefined', 'override', '`override` alone'],
1941+
['readonly: string, message: string', 'readonly,message', 'a parameter named `readonly`'],
1942+
['public: number', 'public', 'a parameter named `public`'],
1943+
['readonly : string', 'readonly', '`readonly` spaced off its own colon — no name follows the run'],
1944+
['private: string, readonly: string', 'private,readonly', 'two modifier-WORDS used as names'],
1945+
]) {
1946+
ok(
1947+
parseParamNames(params).join(',') === expected,
1948+
`parseParamNames ate a real parameter name (${note}): ${JSON.stringify(params)} → ` +
1949+
`${JSON.stringify(parseParamNames(params))}, expected ${JSON.stringify(expected.split(','))}`,
1950+
);
1951+
}
1952+
1953+
// [#13227] End to end through the real `deriveSites`, because the parse is
1954+
// only interesting for what it costs downstream: the whole helper was
1955+
// DROPPED — no site AND no unresolved — which is the silent-drop class this
1956+
// gate exists to refuse. Paired with the single-modifier POSITIVE CONTROL,
1957+
// so the zero on `main` was a reading rather than a dead harness.
1958+
{
1959+
const helper = (params, arg) =>
1960+
`class E {\n constructor(${params}) {\n (this as any).code = code;\n }\n}\n` +
1961+
`throw new E('${arg}', 'x');\n`;
1962+
const derive = (source) =>
1963+
deriveSites({ registered: new Set(['ALREADY_REGISTERED']), files: [{ rel: 'packages/x/src/a.ts', source }], readFile: () => '' });
1964+
for (const [params, probe, note] of [
1965+
['readonly code: string, readonly message: string', 'ONE_MODIFIER_HELPER', 'the single-modifier positive control'],
1966+
['private readonly code: string, readonly message: string', 'PARAM_PROPERTY_HELPER', 'the two-modifier parameter property'],
1967+
]) {
1968+
const { sites, unresolved } = derive(helper(params, probe));
1969+
ok(
1970+
sites.some((s) => s.shape === 'codehelper' && s.code === probe),
1971+
`a constructor code helper was dropped — ${note} derived ${sites.length} site(s) and ` +
1972+
`${unresolved.length} unresolved, neither naming ${probe}`,
1973+
);
1974+
}
1975+
}
18471976
}
18481977

18491978
// [#9568] The value-level reduction: a constant holding a TERNARY or a

0 commit comments

Comments
 (0)