fix(java): escape null/true/false discriminator values in generated identifiers - #17430
fix(java): escape null/true/false discriminator values in generated identifiers#17430ChoMinGi wants to merge 2 commits into
Conversation
…dentifiers CasingConfiguration.JAVA_RESERVED_KEYWORDS omitted the Java literals null, true, and false, so a discriminated union whose discriminator value is one of those strings produced an unescaped Java identifier and crashed JavaPoet with "not a valid name: null". KeyWordUtils already treats these as reserved; this brings CasingConfiguration's fallback set in line with it. Fixes fern-api#17429
![nitpickybot[bot]](https://avatars.githubusercontent.com/in/2901896?s=60&v=4){kind=small}
| class JavaLiteralKeywordTests { | ||
|
|
||
| @ParameterizedTest | ||
| @CsvSource({"null,null_", "true,true_", "false,false_"}) |
There was a problem hiding this comment.
🔵 suggestion
@CsvSource is safe here only because nullValues defaults to empty — but if anyone ever sets nullValues = "null" on this class, the null case silently becomes an NPE-flavored mystery. Quote the inputs to make the intent explicit.
| @CsvSource({"null,null_", "true,true_", "false,false_"}) | |
| @CsvSource({"'null',null_", "'true',true_", "'false',false_"}) |
There was a problem hiding this comment.
Fixed.
quoted the literals in @CsvSource ('null',null_ etc.) so it's unambiguous regardless of nullValues config.
| "throw", | ||
| "throws", | ||
| "transient", | ||
| "true", |
There was a problem hiding this comment.
🔵 suggestion
Two independent reserved-word lists (KeyWordUtils and this one) drifting apart is exactly how this bug happened. Consider having one derive from the other, or at minimum add a test asserting JAVA_RESERVED_KEYWORDS is a superset of KeyWordUtils' set so the next divergence fails in CI instead of in a user's generate run.
There was a problem hiding this comment.
Tried that — turns out _ doesn't work the same way.
KeyWordUtils does treat _ as reserved, but in CasingConfiguration it never gets that far. splitWords()'s regex doesn't match underscores at all, so computeName("_") collapses to an empty string before sanitizeName ever runs — I checked, and camelSafe comes back as "", not "_". So adding _ to JAVA_RESERVED_KEYWORDS would just be a no-op.
That looks like a bug in the word-splitting logic rather than the list drifting, so I'd rather leave it out of this PR. Happy to file it separately if that's useful.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| "else", | ||
| "enum", | ||
| "extends", | ||
| "false", |
There was a problem hiding this comment.
🟡 Java generator fix ships without a changelog/version entry
The Java generator bug fix changes shared casing logic (JAVA_RESERVED_KEYWORDS at generators/java/generator-utils/src/main/java/com/fern/java/utils/CasingConfiguration.java:62) without adding a corresponding release entry, so users get no released version containing the fix and no record of it.
Impact: The fix is not published or documented for users of the Java generator.
Repository rule requiring a versions.yml entry for generator changes
REVIEW.md states: "Generator changes (bug fixes or features) should include a versions.yml entry in the appropriate generator directory" and "The type field in changelog entries should match the PR type: fix for bug fixes". This PR only touches CasingConfiguration.java and its test; no entry was added to generators/java/sdk/versions.yml (or the other Java generator directories that consume generator-utils).
Prompt for agents
REVIEW.md requires generator changes to include a versions.yml changelog entry in the appropriate generator directory with type matching the PR type. This PR fixes a Java generator crash caused by unescaped null/true/false discriminator values in CasingConfiguration, but no entry was added. Add a new version entry with a `fix` changelog summary to generators/java/sdk/versions.yml (and any other Java generator versions.yml files that ship generator-utils, e.g. spring/model, if applicable), following the formatting of existing entries.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed.
added a changelog entry at generators/java/sdk/changes/unreleased/fix-null-true-false-discriminator.yml per the REVIEW.md convention.
- Add versions.yml changelog entry per REVIEW.md convention for generator changes - Quote null/true/false in @CsvSource so intent is explicit regardless of nullValues configuration (nitpickybot suggestion)
1 participant
Description
Closes #17429
CasingConfiguration.JAVA_RESERVED_KEYWORDSomitted the Java literalsnull,true, andfalse. When a discriminated union's discriminator value is one of those strings, the generated Java identifier is left unescaped and JavaPoet rejects it withIllegalArgumentException: not a valid name: null, aborting generation for the whole SDK.KeyWordUtils.java, a separate reserved-token list used elsewhere in the same generator, already includes all three — this bringsCasingConfiguration's fallback set in line with it, using the same escaping convention already used for ordinary keywords (e.g.static→static_(...), seeseed/java-sdk/reserved-keywords).Changes Made
"null","true","false"toCasingConfiguration.JAVA_RESERVED_KEYWORDSJavaLiteralKeywordTeststoCasingConfigurationTestcovering all three values plus a control case (default)Testing
fern generate --localagainstfernapi/fern-java-sdk:4.19.0and a minimal repro spec (see Java SDK generation fails on discriminator values null, true, and false #17429), confirmed the fix resolves it, and ran the fullgenerator-utilstest suite (186 tests, all passing) to confirm no regressions