Parse computed string literal enum member names#31858
Conversation
TypeScript accepts enum members whose name is a computed property with a
string literal or a substitution-free template literal, e.g.
enum E { ['a'] = 1, [`b`] = 2 }. Bun rejected these with "Expected
identifier". Accept them and treat the name exactly like a string
literal member name.
|
Warning Review limit reached
More reviews will be available in 72 minutes and 14 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — narrow parser extension that mirrors the existing string-literal enum-member path, with positive and negative test coverage.
Extended reasoning...
Overview
This PR adds a single TOpenBracket arm to parse_typescript_enum_stmt in src/js_parser/parse/parse_typescript.rs so that enum member names written as ['literal'] or [\literal`]are accepted, matching tsc. The new arm is a near-verbatim copy of the adjacentTStringLiteralarm (sameto_utf8_e_stringextraction, sameneeds_symbol = is_identifier(name)rule) wrapped in[/]consumption. The shared trailingp.lexer.next()?was pushed into each arm to accommodate the bracket arm's extraexpect(TCloseBracket). Tests in transpiler.test.js` pin both the lowered output for the newly-accepted forms and the error messages for the still-rejected ones.
Security risks
None. This is a syntax-acceptance widening in the TypeScript enum parser; no auth, crypto, filesystem, or network surface is touched. The change strictly turns a parse error into the same lowering that bare string-literal member names already produce.
Level of scrutiny
Moderate — parser code, but the change is ~18 lines, mechanically mirrors an existing branch, and reuses the lexer's shared string/template-literal decoding (TStringLiteral and TNoSubstitutionTemplateLiteral are already handled identically in parse_prefix.rs via to_e_string). The error path follows the established expect(...) ?; return Err(...) pattern used three lines below it. No CODEOWNERS cover these paths.
Other factors
The bug-hunting pass found nothing. The PR notes this diverges from esbuild, but Bun's compatibility target for TS syntax is tsc, and the description verifies output against tsc 6.0.2. Test coverage is thorough: quoted, backtick, cross-member reference, auto-increment, const-enum inlining on the accept side; numeric, concatenation, substitution-template, and identifier-expression on the reject side. The pre-existing "both live arms" comment above needs_symbol is now slightly stale (three live arms), but that's cosmetic and not introduced by this diff.
There was a problem hiding this comment.
I didn't find any issues — the new branch cleanly mirrors the existing string-literal path and tests are thorough — but since this is a parser change that intentionally diverges from esbuild, it's worth a quick human sign-off.
Extended reasoning...
Overview
This PR extends parse_typescript_enum_stmt in src/js_parser/parse/parse_typescript.rs to accept computed enum member names when the bracketed expression is a string literal or a no-substitution template literal (e.g. enum E { ['a'] = 1 }). The implementation adds a TOpenBracket arm that reuses the same to_utf8_e_string / needs_symbol logic as the existing TStringLiteral arm, then expects the closing ]. To accommodate the extra token consumption, the trailing p.lexer.next()? is moved into each arm — a mechanical refactor. The accompanying test changes in transpiler.test.js pin the lowered output for the valid forms (quoted, template, cross-reference, auto-increment, const-enum inlining) and the error messages for the still-invalid forms (['a'+'b'], template with substitution, identifier, numeric).
Security risks
None. This is a syntactic relaxation in the TypeScript enum-name grammar; the accepted forms reduce to the exact same code path as bare string-literal member names, which were already supported. No new untrusted input surface, no filesystem/network/auth/crypto involvement.
Level of scrutiny
Moderate. The change is small (~17 net lines of parser logic plus a mechanical next() relocation) and well-covered by exact-output tests, but it lives in the TypeScript parser — a hot, correctness-critical path — and is an explicit, deliberate divergence from esbuild's behavior. The PR description does an excellent job justifying that divergence (citing isComputedNonLiteralName in the tsc checker and verifying acceptance across tsc 3.9 → 6.0), so the remaining question is just whether maintainers want to accept that divergence, which is a project-policy call rather than a code-correctness one.
Other factors
- I verified that
to_utf8_e_string()/to_e_string()already handleTNoSubstitutionTemplateLiteral(used the same way inparse_string_literalinparse/mod.rs), so the template-literal branch is sound. - The
needs_symbol/declare_symbolreuse means cross-member references and const-enum inlining behave identically to the string-literal case, which the tests confirm. - The error path (
p.lexer.expect(T::TStringLiteral)?when the bracketed token isn't a string) follows the established convention in this file for producing diagnostics before returningSyntaxError. - No CODEOWNERS entries cover these paths; no prior human review comments are outstanding. CI is still building at the time of this review.
1 participant
The TypeScript parser rejected enum members whose name is a computed property name, which tsc accepts when the expression is a string literal or a substitution-free template literal.
Repro
tsc compiles this and prints
1 with space. Verified against tsc 3.9.10, 4.0.8, 4.4.4, 4.9.5, 5.0.4, 5.2.2, 5.4.5, 5.6.3, 5.8.3, 5.9.3, and 6.0.2: all accept it (--noEmitexit 0), so this is longstanding tsc behavior, not a recent addition. In the compiler this is explicit:computeEnumMemberValuein the checker only emits TS1164 ("Computed property names are not allowed in enums") forisComputedNonLiteralName, defined asname.kind === ComputedPropertyName && !isStringOrNumericLiteralLike(name.expression), whereisStringLiteralLikecovers string literals and no-substitution template literals. Computed numeric literal names ([1]) fall through to the "An enum member cannot have a numeric name" error (TS2452), same as non-computed numeric names.Cause
parse_typescript_enum_stmtinsrc/js_parser/parse/parse_typescript.rsonly accepted an identifier or a string literal as the member name. esbuild also rejects the computed forms, so this is a deliberate divergence from esbuild to match tsc.Fix
Add a
TOpenBracketbranch to the member-name parse that accepts a string literal or aTNoSubstitutionTemplateLiteral, reuses the string-literal handling (including the "referenceable when the name is a valid identifier" rule), and requires the closing]. Other expressions inside the brackets remain parse errors (they are errors in tsc too: TS1164 for non-literals, TS2452 for numeric literals).Cross-member references and const enum inlining work the same as for string literal names:
Verification
New
enum members with computed string literal namestest intest/bundler/transpiler/transpiler.test.jsasserts the exact lowered output for the quoted, template, cross-reference, auto-increment, and const enum cases, and themalformed enumstest now pins the errors for the still-invalid computed forms (['a' + 'b'],[`t${1}`],[Symbol.iterator],[2]). Both fail on an unfixed build (Expected identifier but found "[") and pass with this change. Lowered output was cross-checked against tsc's emit for the same inputs (E[E['with space'] = 1] = 'with space').