Repository navigation
fix(java): escape null/true/false discriminator values in generated identifiers #17430
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,6 +59,7 @@ public final class CasingConfiguration { | |
| "else", | ||
| "enum", | ||
| "extends", | ||
| "false", | ||
| "final", | ||
| "finally", | ||
| "float", | ||
|
|
@@ -73,6 +74,7 @@ public final class CasingConfiguration { | |
| "long", | ||
| "native", | ||
| "new", | ||
| "null", | ||
| "package", | ||
| "private", | ||
| "protected", | ||
|
|
@@ -88,6 +90,7 @@ public final class CasingConfiguration { | |
| "throw", | ||
| "throws", | ||
| "transient", | ||
| "true", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 suggestion Two independent reserved-word lists (
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tried that — turns out
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. |
||
| "try", | ||
| "void", | ||
| "volatile", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| # yaml-language-server: $schema=../../../../../fern-changes-yml.schema.json | ||
|
|
||
| - summary: | | ||
| Escape the Java literals `null`, `true`, and `false` when they're used as a | ||
| discriminated union's discriminator value. Previously the generated Java | ||
| identifier was left unescaped (e.g. a static factory method literally named | ||
| `null`), which JavaPoet rejected with `not a valid name: null`, aborting | ||
| generation. The identifier is now escaped to `null_`/`true_`/`false_` | ||
| (matching the existing convention for other reserved words like `static`), | ||
| while the serialized discriminator value is unchanged. | ||
| type: fix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Java generator fix ships without a changelog/version entry
The Java generator bug fix changes shared casing logic (
JAVA_RESERVED_KEYWORDSatgenerators/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.ymlentry in the appropriate generator directory" and "Thetypefield in changelog entries should match the PR type:fixfor bug fixes". This PR only touchesCasingConfiguration.javaand its test; no entry was added togenerators/java/sdk/versions.yml(or the other Java generator directories that consumegenerator-utils).Prompt for agents
Was this helpful? React with 👍 or 👎 to provide feedback.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
added a changelog entry at
generators/java/sdk/changes/unreleased/fix-null-true-false-discriminator.ymlper the REVIEW.md convention.