-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Add nullability annotations to Java generated clients #19617
Add nullability annotations to Java generated clients #19617
Conversation
91c1658
to
b723dd9
Compare
@@ -76,6 +76,7 @@ public class {{classname}} {{#parent}}extends {{{.}}} {{/parent}}{{#vendorExtens | |||
private {{{datatypeWithEnum}}} {{name}}{{#defaultValue}} = {{{.}}}{{/defaultValue}}; | |||
{{/isContainer}} | |||
{{^isContainer}} | |||
{{>nullable_var_annotations}} |
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.
Hi @nvervelle, thanks for the PR!
For feign
, the annotations on the field are currently only generated if the field is not a container. Moving the {{>nullable_var_annotations}}
to line 75 should fix that. Other than that the PR looks good to me.
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.
Hi @martin-mfg !
Thanks for the comment. I modified the template for feign and updated the PR accordingly (I edited each commit so that I still have one commit for the manual changes, and one commit for the changes made by the script)
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.
Hi @martin-mfg !
Any hope of merging this PR in the near future ? I will be off for the next 3 weeks, so I won't be able to answer or edit the PR until then.
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.
I can't merge PRs. We'll have to wait for @wing328
to merge this.
Thanks for the heads-up in case further input from you is needed. But hopefully the PR can simply be merged without further changes.
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.
^ @wing328 the mention didn't work. @nvervelle Unfortunately this PR needs a rebase now, sorry :(
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.
@martin-mfg , @joscha , @wing328 : I've rebased the PR.
b723dd9
to
cb753a9
Compare
cb753a9
to
fcfc4ea
Compare
https://github.com/OpenAPITools/openapi-generator/actions/runs/11327217987/job/31599273360?pr=19617 can you please update the samples when you've time? |
fcfc4ea
to
90285e4
Compare
Motivations: Have generated clients properly annotated for nullability to be able to check code using them with tools like NullAway Modifications: * Add nullable_var_annotations template to handle nullability annotation on vars * Add pojo templates to use the nullability template * Adapt tests
Modifications: * Run export_docs_generator.sh script to update samples
90285e4
to
3f273b4
Compare
@wing328 I've rerun the scripts and also rebased : it seems ok now |
thanks. will merge after all tests pass |
Motivations:
Have generated clients properly annotated for nullability to be able to check code using them with tools like NullAway (to close #19600)
Modifications:
PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*
.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master
(upcoming 7.6.0 minor release - breaking changes with fallbacks),8.0.x
(breaking changes without fallbacks)