Skip to content
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

codegen: Support default values in schema objects #3614

Conversation

hughsimpson
Copy link
Contributor

@hughsimpson hughsimpson commented Mar 15, 2024

Adds support for default values in schema objects. Actually just adds the default value to the constructed case class and expects the json lib to handle the rest of the functionality. Optional fields with no explicit default in the schema are treated as having a default of None. Constructing the representation of default values traverses the schema tree, and so should generate valid syntax for all currently supported spec objects.

Copy link
Member

@kciesielski kciesielski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! What would happen for nested fields with default values? Would they just be ignored, or should we expect some errors?

@hughsimpson
Copy link
Contributor Author

hughsimpson commented Mar 18, 2024

In general nested fields would only work if the actual type was the guessed type from parsing the default value. I think the render def would need to take the full schema model to be able to resolve that limitation... As it stands, in cases where we guessed wrong, then the generated code should fail to compile

@kciesielski
Copy link
Member

Thank for clarifying. I'm ok with going forward with this solution. In case we have users with really sophisticated schemas not covered by this, the implementation could be revisited for enhancements, but IMO the current scope is good enough :)
Happy to merge after the conflicts are resolved.

@hughsimpson hughsimpson force-pushed the support_default_values_in_codegen_objects branch from f96bfcb to 8ff8e9f Compare March 18, 2024 13:27
@hughsimpson
Copy link
Contributor Author

hughsimpson commented Mar 18, 2024

Thanks! I'm currently working with a fairly large spec and haven't run into any examples where the defaults this generates breaks compilation, but I'll revisit this if anyone raises an issue once it's released (and may do so anyway, depending on how the wind blows)

Edit: I took a look at making it more principled, and actually it wasn't too nightmarish in the end. It should now handle complex default values, and the choice of repr for 'stringy' values is now considerably less arbitrary. I think it looks a lot nicer now.

@kciesielski
Copy link
Member

Excellent, thank you for this contribution @hughsimpson!

@kciesielski kciesielski merged commit 3880806 into softwaremill:master Mar 20, 2024
23 checks passed
@hughsimpson hughsimpson deleted the support_default_values_in_codegen_objects branch March 20, 2024 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants