Add enum_field_rename option in build.yaml for default enum value renaming#1556
Add enum_field_rename option in build.yaml for default enum value renaming#1556jlalvarez18 wants to merge 20 commits intogoogle:masterfrom
enum_field_rename option in build.yaml for default enum value renaming#1556Conversation
- Introduced `enumFieldRename` property to `JsonSerializable` for default field renaming of enum values. - Updated serialization and deserialization methods to handle the new `enumFieldRename` field. - Modified README to document the new configuration option. - Adjusted encoding logic to utilize the `enumFieldRename` setting when processing enums.
Summary of ChangesHello @jlalvarez18, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces the enum_field_rename option, allowing a package-wide default for renaming enum values during serialization. The implementation is comprehensive, with updates to configuration handling, code generation, tests, and documentation. The changes are well-structured and align with the feature's goal. I've identified a couple of areas for improvement: one regarding a contradiction between documentation and implementation, and another suggesting a user-facing warning for a potentially confusing configuration scenario. Overall, this is a solid contribution that enhances the flexibility of json_serializable.
|
Gah! i like this idea, but I'm working on something else this should plug into. Please hold! |
|
Actually, I take that back. This approach is fine. But I'd like to see NOW tests that cover this! |
kevmoo
left a comment
There was a problem hiding this comment.
You need some NEW tests to cover this new behavior, please!
|
@kevmoo ok cool! Adding tests now |
|
@kevmoo what do you mean by NOW tests? |
|
@jlalvarez18 – I meant |
- enum_field_rename_test: programmatic JsonEnumGenerator tests for config enum_field_rename pascal, default (none), and @JsonEnum(fieldRename) overriding config. - enum_utils_test: unit tests for enumValueMapFromType with defaultEnumFieldRename (pascal, null, annotation override). - _json_enum_default_rename_test_input: EnumForDefaultRename and EnumWithKebabOverride used by both test files.
Log a build warning when JsonEnum.valueField is set and JsonEnum.fieldRename is not none, since fieldRename is ignored in that case. This avoids silently ignoring fieldRename and makes the behavior clear to users. - In enum_utils.dart: import build, and call log.warning() when valueField != null and fieldRename != FieldRename.none - Add test input enum with both options and a unit test that asserts the warning is recorded via buildLogItems
|
@jlalvarez18 – FYI I pushed a couple of tweaks here. In case you wonder why your |
json_serializable/test/src/_json_enum_valuefield_fieldrename_warning_test_input.dart
Outdated
Show resolved
Hide resolved
no need to churn this line!
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature by adding the enum_field_rename option, allowing for a package-wide default for enum field renaming. The implementation is solid and includes good test coverage. I've identified one high-severity issue where the code's behavior for enumFieldRename contradicts the documentation, and a medium-severity issue regarding a potentially duplicated and unused test file. Addressing these points will make this a great addition.
json_serializable/test/src/_json_enum_valuefield_fieldrename_warning_test_input.dart
Outdated
Show resolved
Hide resolved
|
@kevmoo anything else needed on my end to get this moving? |
|
Just ran the robot. Will let you know! |
|
Let's get CI green! |
|
I'm reviewing the code now. Note that I pushed to get things green! |
|
@jlalvarez18 – okay, I did a bit more cleanup, but I'm...concerned about this plan. We're adding a field to Which I really don't like. I'm wondering now if we need to update the logic here to create a subclass of What do you think? |
Summary
Adds an
enum_field_renameoption to thejson_serializablebuilder config so packages can set a default field-rename strategy for enum values (e.g. PascalCase from the backend) without annotating every enum with@JsonEnum(fieldRename: ...).Fixes #1021.
Problem
field_renameinbuild.yamlonly affects class fields, not enum value names. Enums had to set@JsonEnum(fieldRename: ...)on each type. There was no way to configure a package-wide default for enum value renaming.Solution
Introduce a separate option
enum_field_renamein the build config. When generating enum value maps:@JsonEnum(fieldRename: ...)is set to something other thannone, that value is used.enum_field_renameis used (defaultnone).Changes
json_annotation
enumFieldRenametoJsonSerializable(used when parsingbuild.yaml).json_serializable.g.dartforfromJson/toJsonand allowed keys.json_serializable
enumFieldRename(defaultFieldRename.none) and wired it fromJsonSerializableand inmergeConfig.enumValueMapFromType(and helpers) accept optionaldefaultEnumFieldRename; effective rename is annotation value if non-none, else config default, elsenone.Settingsand passesconfig.enumFieldRenameas the default for enums.context.config.enumFieldRename(orconfig.enumFieldRename) into enum helpers so the default is applied when encoding/decoding enum fields.Examples
1. build.yaml – default PascalCase for all enums
Enums without
@JsonEnum(fieldRename: ...)(or withfieldRename: none) will serialize with PascalCase:2. Per-enum override
3. Same options as other build config
4. Supported values
Same as
field_rename:none,kebab,snake,pascal,screamingSnake.Documentation
enum_field_rename: nonein the build configuration options.enum_field_rename); in Dart use camelCase (enumFieldRename), consistent withfield_rename/fieldRename.Testing
enum_field_renamein default config, invalid config handling, and key ordering.enumFieldRenameingeneratorConfigNonDefaultJson.ConfigurationExplicitDefaultsincludesenumFieldRename: FieldRename.noneso annotation-over-config behavior is tested.JsonEnumGeneratorupdated to useJsonEnumGenerator(Settings())so enum generation uses the new default.All existing tests pass (624 tests).
fixes #1021