Skip to content

Commit 54bac8a

Browse files
committed
Reject reserved words used as identifiers / bindings / shorthand
The Ident token deliberately lexes keywords as identifiers, so any reserved word could fall back to a bare identifier — letting invalid code through (often by splitting a statement in two). Add zero-width guards before identifier positions: - notReserved: the always-reserved set (break/case/catch/class/.../void/while/with), valid as an identifier in NO context. Placed before the bound name in Binding, ForBinding, BindingElement, ArrayBindingElement rest, Param rest, BindingProperty shorthand, and the Prop shorthand. (BindingProperty was split so a reserved word stays valid as a KEY `{ while: y }` while the bound name is guarded.) - notReservedExpr: a NARROWER set (catch/delete/enum/throw/typeof/void) for the expression identifier-NUD. The full set can't be used there — most reserved words legitimately begin an expression via their own forms (new/class/function/import/ super/this/...), and TS error-recovery slides a few into the identifier fallback. These five have no expression role, so forbidding their fallback rejects `catch(x){}` with no try, and the operatorless prefix ops `void ;`/`typeof ;`/ `delete ;`/`throw ;`. FN-safe by construction: a reserved word is never a valid binding name; KEYS, member/method names, property access, and import/export specifiers use other rules and are unaffected. A plain param name is left unguarded so `function f(this, a)` (implicit-any this-param) still parses. Strict-only (let/static/...) and conditional (await/yield) words stay valid since a CFG can't see the mode. FP 88->80 (8 cases: keyword bindings/shorthand + operatorless prefix ops + catch-no-try), FN stays 0 (3376/3376), TP unchanged, highlighter 99.3% unchanged. agnostic 5/5, refactor-guard 112/112. Artifacts regenerated. (Two sibling fixes from the same batch were dropped: class-body-statements needed an invasive heritage-clause rewrite; the type-alias-name guard regressed type-name highlighting — both deferred.)
1 parent c8ecef5 commit 54bac8a

3 files changed

Lines changed: 66 additions & 15 deletions

File tree

examples/lezer/typescript.grammar

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ Block {
117117
BraceL Stmt* BraceR
118118
}
119119
BindingProperty {
120-
(Ident (Op_3d Expr | Colon BindingElement | ) | (String | Number | BracketL Expr BracketR) Colon BindingElement | Spread (Ident | BindingPattern))
120+
(Ident Colon BindingElement | Ident (Op_3d Expr)? | (String | Number | BracketL Expr BracketR) Colon BindingElement | Spread (Ident | BindingPattern))
121121
}
122122
BindingElement {
123123
(Ident | BindingPattern) (Op_3d Expr)?

examples/tree-sitter/grammar.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ module.exports = grammar({
118118

119119
block: $ => seq("{", repeat($.stmt), "}"),
120120

121-
binding_property: $ => choice(seq($.ident, choice(seq("=", $.expr), seq(":", $.binding_element), blank())), seq(choice($.string, $.number, seq("[", $.expr, "]")), ":", $.binding_element), seq("...", choice($.ident, $.binding_pattern))),
121+
binding_property: $ => choice(seq($.ident, ":", $.binding_element), seq($.ident, optional(seq("=", $.expr))), seq(choice($.string, $.number, seq("[", $.expr, "]")), ":", $.binding_element), seq("...", choice($.ident, $.binding_pattern))),
122122

123123
binding_element: $ => seq(choice($.ident, $.binding_pattern), optional(seq("=", $.expr))),
124124

examples/typescript.ts

Lines changed: 64 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,41 @@ const TripleSlash = token(/\/\/\/\s*<[^\n]*/, { skip: true, scope: 'comment.
6262
const LineComment = token(/\/\/[^\n]*/, { skip: true });
6363
const BlockComment = token(/\/\*[\s\S]*?\*\//, { skip: true });
6464

65+
// ── Always-reserved words ──
66+
// The `Ident` token deliberately swallows keywords (they lex as identifiers), so
67+
// every keyword can otherwise fall back to a bare identifier. These words are
68+
// reserved in EVERY context (ECMAScript ReservedWord ∪ TS's always-reserved), so
69+
// they are valid as an identifier NOWHERE — not as an expression, a shorthand
70+
// property, or a binding name. `notReserved` is a zero-width guard placed before an
71+
// identifier position to forbid exactly these. Excluded on purpose: contextual
72+
// keywords (as/async/from/type/of/…) and strict-mode-only reserved words
73+
// (let/static/implements/yield/await/…) — those ARE valid identifiers in some
74+
// context a CFG can't detect (sloppy mode, non-generator/non-async), so forbidding
75+
// them here would reject valid code (`var let = 1`, `function f(yield) {}`).
76+
const notReserved = not(alt(
77+
'break', 'case', 'catch', 'class', 'const', 'continue', 'debugger', 'default',
78+
'delete', 'do', 'else', 'enum', 'export', 'extends', 'false', 'finally', 'for',
79+
'function', 'if', 'import', 'in', 'instanceof', 'new', 'null', 'return', 'super',
80+
'switch', 'this', 'throw', 'true', 'try', 'typeof', 'var', 'void', 'while', 'with',
81+
));
82+
83+
// A NARROWER guard for the *expression* identifier-NUD only. The full `notReserved`
84+
// set can NOT be used at expression position: most always-reserved words legitimately
85+
// begin an expression via their own dedicated forms (`new`/`new.target`, `class`/
86+
// `function` expressions, `import(…)`/`import<T>`, `super`, `this`, `true`/`false`/
87+
// `null`, …), and TS's own error-recovery tolerates several reserved words sliding into
88+
// the bare-identifier fallback inside otherwise-valid files (e.g. `export default …`,
89+
// undeclared `for (x in …)`, `class … extends (e)`, a decorator before `export`). The
90+
// words below have NO such role: they are the prefix operators `void`/`typeof`/`delete`
91+
// (which must take an operand) plus the `catch`/`throw` keywords and `enum`. Forbidding
92+
// the bare-identifier fallback for exactly these rejects `catch(x){}` with no `try`,
93+
// `void ;`/`typeof ;`/`delete ;` (operatorless prefix op), and `throw ;` — while leaving
94+
// every valid expression (and TS's recovery cases) untouched. Verified: widening this
95+
// set to other reserved words regresses valid code; these five are the FN-safe maximum.
96+
const notReservedExpr = not(alt(
97+
'catch', 'delete', 'enum', 'throw', 'typeof', 'void',
98+
));
99+
65100
// ── Type query reference (typeof's argument: just dotted identifiers) ──
66101

67102
const TypeofRef = rule($ => [
@@ -155,8 +190,10 @@ const Prop = rule($ => {
155190
// value property — any member name incl computed `[e]: v` (MemberName covers `[Expr]`)
156191
[MemberName, ':', Expr],
157192
['[', Expr, many(',', Expr), ']', ':', Expr], // computed comma list (lenient)
158-
// shorthand (Ident only): x | x = v | x? | x?: v
159-
[Ident, alt(['=', Expr], ['?', opt(':', Expr)], [])],
193+
// shorthand (Ident only): x | x = v | x? | x?: v — a reserved word here is invalid
194+
// (`var v = { class }`); a reserved word as a property KEY (`{ class: 1 }`) is fine,
195+
// already handled by the `[MemberName, ':', Expr]` branch above.
196+
[notReserved, Ident, alt(['=', Expr], ['?', opt(':', Expr)], [])],
160197
];
161198
});
162199

@@ -175,10 +212,12 @@ const NewTarget = rule($ => [
175212
]);
176213

177214
const Expr = rule($ => [
178-
// `enum` is reserved — it can't be a standalone expression identifier. Without this
179-
// an invalid `enum E { a: 1 }` (where the enum Decl fails on `:`) would fall back to
180-
// parsing `enum` as an identifier expr + `E` + `{…}` block, wrongly accepting it.
181-
[not('enum'), Ident],
215+
// A standalone identifier expression, but never a reserved word that has no expression
216+
// role (see notReservedExpr). This kills the bare-identifier fallback for keywords like
217+
// `catch`/`throw` and the prefix operators `void`/`typeof`/`delete`, so `catch(x){}`
218+
// with no `try`, `void ;`, and `throw ;` are rejected as TS does. (`enum` is included —
219+
// it previously had its own `not('enum')` guard for the same reason.)
220+
[notReservedExpr, Ident],
182221
Number_,
183222
String_,
184223
Template,
@@ -245,18 +284,23 @@ const Block = rule($ => [
245284
// ── Destructuring Patterns ──
246285

247286
const BindingProperty = rule($ => [
248-
[Ident, alt(['=', Expr], [':', BindingElement], [])], // a | a = 1 | a: elem
287+
// `name: elem` — the KEY is a PropertyName, so a reserved word is allowed here
288+
// (`{ while: y }`); the bound name inside `elem` is guarded by BindingElement.
289+
[Ident, ':', BindingElement],
290+
// shorthand `a` / shorthand-with-default `a = 1` — the name is a BindingIdentifier,
291+
// so a reserved word is invalid (`{ while }`, `{ class }`).
292+
[notReserved, Ident, opt('=', Expr)],
249293
[alt(String_, Number_, ['[', Expr, ']']), ':', BindingElement], // "s"/0/[e]: elem
250-
['...', alt(Ident, BindingPattern)], // ...rest | ...{ a }
294+
['...', alt([notReserved, Ident], BindingPattern)], // ...rest | ...{ a }
251295
]);
252296

253297
const BindingElement = rule($ => [
254-
[alt(Ident, BindingPattern), opt('=', Expr)], // a | { a } (optionally = default)
298+
[alt([notReserved, Ident], BindingPattern), opt('=', Expr)], // a | { a } (optionally = default)
255299
]);
256300

257301
const ArrayBindingElement = rule($ => [
258302
BindingElement,
259-
['...', alt(Ident, BindingPattern)], // [...rest] | [...{ a }]
303+
['...', alt([notReserved, Ident], BindingPattern)], // [...rest] | [...{ a }]
260304
]);
261305

262306
const BindingPattern = rule($ => [
@@ -267,22 +311,29 @@ const BindingPattern = rule($ => [
267311
// ── Bindings & Parameters ──
268312

269313
const Binding = rule($ => [
270-
[alt([Ident, opt('!')], BindingPattern), opt(':', Type), opt('=', Expr)],
314+
[alt([notReserved, Ident, opt('!')], BindingPattern), opt(':', Type), opt('=', Expr)],
271315
]);
272316

273317
// A binding in a for-head: identical to Binding except the initializer is a
274318
// no-`in` expression, so `for (var a = 1 in xs)` reads `a = 1` then the for-in
275319
// `in` (TS's [~In] grammar), rather than greedily parsing `1 in xs`.
276320
const ForBinding = rule($ => [
277-
[alt([Ident, opt('!')], BindingPattern), opt(':', Type), opt('=', exclude('in', Expr))],
321+
[alt([notReserved, Ident, opt('!')], BindingPattern), opt(':', Type), opt('=', exclude('in', Expr))],
278322
]);
279323

280324
const Param = rule($ => {
281325
const tail = [opt('?'), opt(':', Type), opt('=', Expr)]; // ? : T = E
282326
const body = alt(
327+
// NOTE: a plain parameter name is NOT reserved-guarded — `this` is a valid first
328+
// parameter even without an annotation (`function f(this, a)`: the implicit-any
329+
// `this`-param), and `this` is an always-reserved word; guarding here would reject
330+
// that valid form. (A truly reserved param name like `function f(while)` stays an
331+
// accepted over-accept; it's out of this gap's scope.)
283332
[Ident, ...tail],
284333
[BindingPattern, ...tail],
285-
['...', alt(Ident, BindingPattern), opt('?'), opt(':', Type)], // rest
334+
// a rest element, by contrast, can never validly be a reserved word (`...while`),
335+
// and `...this` is invalid too, so guarding the rest name is FN-safe.
336+
['...', alt([notReserved, Ident], BindingPattern), opt('?'), opt(':', Type)], // rest
286337
);
287338
return [
288339
['this', ':', Type],

0 commit comments

Comments
 (0)