fix(typescript,python,csharp,java,php,ruby,unity,mcp): derive valid member names for underscore-digit enum values - #70
AshGodfrey wants to merge 13 commits into
Conversation
Enum values such as `_1ST` passed through the shared sanitizer unchanged because the digit-to-words rewrite only ran when the first character was a digit. The caser then dropped the leading underscore and TypeScript, Python (enumFormat: enum), C#, PHP, Ruby, Unity and MCP TypeScript emitted a digit-leading member name that failed to format or compile. Go, Java and Terraform already re-sanitize the cased name, so they were unaffected. The affected targets now re-sanitize the cased name when it starts with a digit, producing the same word-based names Go and Java derive.
github-actions
Bot
added
csharp
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 9 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Fixes enum member name generation for underscore-digit values across seven SDK targets by re-sanitizing digit-leading cased names, with a new test fragment covering the failing inputs. Existing valid names are unchanged, making this a focused, clearly beneficial bug fix.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Fixes enum member name generation across seven SDK targets for underscore-digit values like _1ST by re-sanitizing digit-leading cased names, with a new test fragment pinning the corrected behavior. Existing valid names are unchanged, making this a focused, clearly beneficial bug fix.
Re-trigger cubic
Generator Snapshot Testing
Snapshot results: https://github.com/speakeasy-api/openapi-generation-snapshots/issues/107#issuecomment-6007671653 Snapshot results are summarized above. Additional run context is linked for maintainers with access. |
…ame and casing Values such as `_1` and `1` now both derive the member name `One`, and the casing-class suffix alone left both as `OneUpper`, so TypeScript, MCP TypeScript and Unity emitted duplicate members. These targets now apply the same numeric suffix fallback C#, PHP, Python and Ruby already use. The changeset also records that TypeScript and MCP TypeScript union enums rename members for digits-only values such as `_1` from `1` to `One`.
…n validator The enum collision validator mirrors each target's member name derivation to predict collisions before generation. The C#, Java and Python mirrors still cased `_1` to `1`, while the templates now derive `One`, so a document with both `_1` and `1` passed validation and reached the templates as a collision. The mirrors now apply the same leading-digit re-sanitize step as the templates, so such documents are reported up front like any other pair that normalizes to the same name.
…y enum values TypeScript and MCP TypeScript union enums accept a numeric key for values that are only digits once the leading underscore is removed, so `_1` has always generated and compiled as `1: "_1"`. Applying the leading-digit re-sanitize to those values would rename working members to `One`. The re-sanitize now runs unconditionally for values with letters after the digits (`_1ST`), which never produced a valid identifier, and for digits-only values only when the new `numericEnumMemberNames` option is `words`. The option defaults to `words` for new SDKs and `legacy` for existing ones, following the other new-SDK-defaulted options.
There was a problem hiding this comment.
No issues found across 19 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Focused bug fix: re-sanitizes underscore-digit enum values into valid member names across affected targets, adds collision validation, and preserves existing SDK output via the backward-compatible numericEnumMemberNames option.
Re-trigger cubic
…enums A numeric member name such as `1 = "_1"` is rejected by the TypeScript compiler in a native enum, so the `legacy` setting of `numericEnumMemberNames` only ever produced a working member for union enums. The leading-digit rewrite now runs regardless of the option when the enum renders as `enumFormat: enum`, including per-type overrides via `x-speakeasy-enum-format`. The shared fragment gains a digits-only value so both forms are covered.
There was a problem hiding this comment.
0 issues found across 5 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Focused bug fix: re-sanitizes underscore-digit enum values into valid member names across affected codegen targets, adds validator collision detection, and preserves existing SDK output via the new backward-compatible numericEnumMemberNames option.
Re-trigger cubic
…uth2 scopes An x-speakeasy-enums override or OAuth2 scope name that starts with an underscore followed by digits was cased to a digit-leading identifier (`_401K` became `401K`), which javac rejects. getEnumName now re-sanitizes a cased name that starts with a digit, matching the value-derived member path, so these produce `FourHundredAndOneK`. Names that already cased to a valid identifier are unchanged.
AshGodfrey
changed the title
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
…ions Documents that pair an underscore-digit value with its plain form (`_1` next to `1`) generate today, and the templates give such pairs distinct member names. Reporting them as collisions turned working documents into validation errors, so the validator mirrors are restored to their previous derivations.
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
…git fragment The fragment only exercised value-derived member names. C#, Unity and Java reach the leading-digit step only through an `x-speakeasy-enums` override, so the fragment now includes an enum whose override keeps the underscore-digit name.
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Fixes enum member name derivation across generators for underscore-digit values, adds a legacy config option that preserves existing TypeScript/MCP union output, and adds a regression test. Bounded bug fix with the risky behavior change gated behind legacy defaults.
Re-trigger cubic
…y enum member names The legacy check only matched plain digits or an uppercase E exponent, so values such as _1e5, _0x1F, _0b101 and _0o17, which already generate as valid numeric keys in a union enum, were renamed even under legacy. Match any numeric literal that strict mode accepts, and stop keeping leading-zero keys (_007, _08), which strict mode rejects.
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Fixes enum member name derivation for underscore-digit values across generators, with a numericEnumMemberNames option preserving legacy TypeScript/MCP union output and a regression test. Bounded bug fix with existing SDK output kept by default.
Re-trigger cubic
getEnumNamesFromValues counted collisions on the Pascal name, so _1ST and 1ST (OneSt and OneST) looked distinct and were both snake-cased to ONE_ST, which javac rejects as a duplicate. Count on the member that is actually emitted so such pairs receive the casing suffix instead. Pairs that already deduplicated are unchanged.
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
…ckserver Add a third enum to the underscore-digit fragment that pairs _1ST with 1ST so CI exercises the collision path for every target. The mockserver counted collisions on the cased name (1St vs OneSt) before sanitizeFieldName collapsed both to OneSt and emitted a duplicate Go constant; count on the emitted field name and suffix the pair instead.
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
TristanSpeakEasy
left a comment
TristanSpeakEasy
left a comment
There was a problem hiding this comment.
Looks good Ash, left two non-blocking comments on collisions introduced by the new words default. Both produce duplicate keys in TypeScript and MCP TypeScript, so worth sorting before you merge. CI still needs to finish cleanly too.
| if (names[name] > 1) { | ||
| name = `${name}${caser().ToPascal(getCasing(value))}`; | ||
| let candidate = `${name}${caser().ToPascal(getCasing(value))}`; | ||
| if (seen[candidate]) { |
There was a problem hiding this comment.
Not blocking, but the new words default makes a previously valid enum [_1, "1", OneUpper] emit OneUpper, OneUpper1, OneUpper, which TypeScript rejects as duplicate properties. I generated this on both branches: main emits distinct keys 1, One, OneUpper. seen only tracks collision candidates, so it doesn't reserve the untouched OneUpper member. Can you reserve all member names and keep advancing the suffix until the final candidate is unused? The same case fails in MCP TypeScript too, and would be worth adding to the fragment.
There was a problem hiding this comment.
Fixed. Every target now reserves the member names that need no change up front and keeps advancing the numeric suffix until the candidate is unused, so [_1, "1", OneUpper] emits OneUpper1, OneUpper2, OneUpper. Legacy mode is unchanged (1, One, OneUpper). The fragment now carries this enum.
| return t.Enum.Names.map((n) => sanitizeName(n.trim() || "Unknown")); | ||
| } | ||
| return t.Enum.Names.map((n) => getEnumName(n)); | ||
| return t.Enum.Names.map((n) => getEnumName(n, format)); |
There was a problem hiding this comment.
There's a separate collision on the override path: with enum: [first, second] and x-speakeasy-enums: [_1, One], the new words default emits One twice. Main emits distinct keys 1 and One, but this map bypasses the collision handling, so the PR output no longer compiles. I reproduced this in both TypeScript and MCP TypeScript. Can you disambiguate the transformed overrides as a group too, while keeping the explicit fixEnumNameSanitization behaviour unchanged? Not blocking, just another case to cover alongside the value-derived names.
There was a problem hiding this comment.
Fixed. Override names now go through the same group disambiguation as value-derived names in every target, so x-speakeasy-enums: [_1, One] emits OneUpper and OneMixed. The explicit fixEnumNameSanitization path is untouched, and legacy mode still emits 1 and One. The fragment now carries this enum too.
…erve every enum member name when suffixing collisions Values that derive the same member name receive a casing suffix, but the suffix was only checked against other suffixed candidates, not against members that needed no change. An enum listing `_1`, `1` and `OneUpper` therefore emitted `OneUpper` twice. Every target now reserves the untouched member names up front and keeps advancing the numeric suffix until the candidate is unused. `x-speakeasy-enums` override names bypassed collision handling entirely, so overrides such as `_1` and `One` emitted `One` twice once digits are spelled out. Overrides are now disambiguated as a group with the same rules as value-derived names; the explicit `fixEnumNameSanitization` path is unchanged. The shared fragment gains both shapes.
There was a problem hiding this comment.
1 issue found across 19 files (changes from recent commits).
Confidence score: 3/5
- In
templates/templates/mcp-typescript/includes/types.ts, legacy mode can emit an invalid member when values share a preserved numeric name, making the generated TypeScript invalid. Re-sanitize the candidate before adding the collision suffix, so1Upperbecomes a valid name such asOneUpper.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="templates/templates/mcp-typescript/includes/types.ts">
<violation number="1" location="templates/templates/mcp-typescript/includes/types.ts:136">
P1: Legacy mode still generates invalid members when two values share a preserved numeric name. Re-sanitize the suffixed candidate so `1Upper` becomes a valid name such as `OneUpper` before collision suffixing.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| return values.map((value, i) => { | ||
| let name = baseNames[i]; | ||
| if (counts.get(name) > 1) { | ||
| const candidate = `${name}${caser().ToPascal(getCasing(value))}`; |
There was a problem hiding this comment.
P1: Legacy mode still generates invalid members when two values share a preserved numeric name. Re-sanitize the suffixed candidate so 1Upper becomes a valid name such as OneUpper before collision suffixing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At templates/templates/mcp-typescript/includes/types.ts, line 136:
<comment>Legacy mode still generates invalid members when two values share a preserved numeric name. Re-sanitize the suffixed candidate so `1Upper` becomes a valid name such as `OneUpper` before collision suffixing.</comment>
<file context>
@@ -116,37 +116,32 @@ function enumLiteralFromValue(
+ return values.map((value, i) => {
+ let name = baseNames[i];
+ if (counts.get(name) > 1) {
+ const candidate = `${name}${caser().ToPascal(getCasing(value))}`;
name = candidate;
+ for (let suffix = 1; used.has(name); suffix++) {
</file context>
| const candidate = `${name}${caser().ToPascal(getCasing(value))}`; | |
| const candidate = sanitizeEnumName(`${name}${caser().ToPascal(getCasing(value))}`); |
2 participants
Why
Enum values that start with an underscore followed by digits (
_1ST,_2FA,_401K) made generation fail in most targets. Protobuf-derived documents use this shape for numeric-looking identifiers.The shared sanitizer only spells out a leading number when the first character is a digit, so
_1STpasses through unchanged. Each target's caser then drops the underscore and emits1ST, which is not a valid identifier.What changed
Each affected target now re-sanitizes a cased member name that starts with a digit. For the value
_1ST:1St, generation abortsOneStenumFormat: enum)1_ST, generation abortsONE_ST1St, PHPStan failsOneSt1_ST, compile failsONE_STx-speakeasy-enumsoverrides (and Java OAuth2 scope names)enumFormat: union)Two related fixes:
x-speakeasy-enumsnames go through the same derivation, so an override of_401Know generates asFourHundredAndOneK.fixEnumNameSanitizationstill returns overrides verbatim._1and1, or_1STand1ST), the members are suffixed (OneUpper/OneUpper1). TypeScript, MCP TypeScript and Unity gain the numeric suffix that C#, PHP, Python and Ruby already had. Java and the mockserver now detect collisions on the name they emit, where previously they compared an intermediate name and declared the same member twice.New option:
numericEnumMemberNames(TypeScript, MCP TypeScript)A digits-only value such as
_1already works in a TypeScript union enum, because1: "_1"is a valid key. Renaming it would change working output, so that case is behind an option:words(default for new SDKs):_1becomesOne.legacy(default for existing SDKs): the numeric key is kept. This covers any numeric literal that strict mode accepts, including1e5and0x1F.Values that never produced a valid member are always spelled out: anything with letters after the digits (
_1ST), leading-zero keys (_007), and all digits-only values in native enums (enumFormat: enum).Impact on existing SDKs
numericEnumMemberNames: legacywritten togen.yamland keep their numeric keys.Testing
tests/specs/fragments/uber/enum-leading-underscore-digit.yaml, which covers_1ST,_2FA,_3D, the digits-only_10, anx-speakeasy-enumsoverride of_1ST, and the colliding pair_1ST+1ST. It is generated and compiled for every target in CI; thesecondaryvariant coversenumFormat: enumfor TypeScript and Python.mainand from this branch.mainfails in TypeScript, MCP TypeScript, PHP, Ruby and Python (enumFormat: enum); this branch produces valid members in all of them, and the Java build compiles with overrides and OAuth2 scopes.numericEnumMemberNamesby hand on TypeScript and MCP TypeScript: a new SDK emitsOne/Ten, an existing SDK emits1/10exactly asmaindoes, and native enums always emit words.tsc --noEmitpasses on each.foo_bar/FooBar/FOO_BAR) produce byte-identical output.make check-template-<target>,make lint, prettier andgo test ./internal/validation/...pass.Out of scope
Enum member naming is implemented separately in each target's templates, so this fix had to be applied target by target and the collision handling differed between them.
internal/namerhas an enum namer, but it only covers Go, the mockserver and Terraform, and only predicts names for conflict detection.generator-validate-enumsalso derives names on its own, so it can disagree with a target about whether two values collide.Moving member derivation and collision handling into one shared namer, used by the targets and the validator, would remove this duplication. That is a refactor with output implications for every target, so it is left for separate investigation.
Public-safety check
.claude/skills/public-repo-communication/SKILL.md).git diff --check.