[Rust] Handle optional and nullable parameters #4016
Conversation
As stated in OpenAPITools#3250 * Making this optinal gives more complicated templates, harder to maintain. * Such breaking change is acceptable, since it's more a behavior correction.
Generated sample code compiles (with both Reqwest/Hyper). Optional path parameters have no meaning. So we just call `unwrap()` in this case.
|
Just noticed I forgot to update generated markdown documentation, to reflect new optional types. I will correct that soon. |
…llable model fields and parameters).
|
This version looks good to me: you can review it. |
This patch includes: - Handle optional parameters [#4016](OpenAPITools/openapi-generator#4016) - Export type modules so that enums can be accessed [#4079](OpenAPITools/openapi-generator#4079) - Derive `Clone` and `Copy` for enums - Fix warnings: "trait objects without an explicit `dyn` are deprecated"
This patch includes: - Add all missing explicit dyn to trait types to remove warnings OpenAPITools/openapi-generator#3895 - Add missing re-export for properties that are enum OpenAPITools/openapi-generator#3895 - Handle optional parameters OpenAPITools/openapi-generator#4016 - Derive more traits borsboom/openapi-generator@4e3bc31
Built using borsboom/openapi-generator@4e3bc31 This patch includes: - Add all missing explicit dyn to trait types to remove warnings OpenAPITools/openapi-generator#3895 - Add missing re-export for properties that are enum OpenAPITools/openapi-generator#3895 - Handle optional parameters OpenAPITools/openapi-generator#4016 - Derive more traits borsboom/openapi-generator@4e3bc31
|
Pinging @bjgill and/or @richardwhiuk for a review. Merging master regularly to resolve conflicts with others Rust evolutions is not a very interesting activity… |
|
👍 I've needed and been using this PR for a project I'm working on, would love to see it merged too. |
|
I should be able to take a look tomorrow. Has all the merging gone OK? I can see an awful lot of |
|
@bjgill Last merge looks ok... but maybe a previous one (or a commit) was not. Do you have a ".kt" file example that should not be here to verify where it comes from? |
|
@bjgill I verified the commits and you are right: theses files were added during the second merge with "master". To correct that, I am going to revert the two last merge commits, and do a new clean merge from master. |
303d247 to
e0601bd
Compare
|
@bjgill It should be much better now. |
There was a problem hiding this comment.
I've had a look. This is indeed a breaking change. However, I agree that the previous handling of optional parameters was very wrong, and so we're fine to do the breaking.
It looks as if this won't handle things that are both optional and nullable, but that seems like a rather niche feature that we don't need to add now.
Nothing jumps out at me, so I think we're good to go. Thanks for the contribution!
|
@bjgill Thanks for the merge! For me (but I may have a bad comprehension of theses attributes), both Optional and Nullable should be treated as "Optional". The only distinction I see is for a "required nullable" attribute. It must be serialized with |
|
@bcourtine thanks for the PR, which has been included in v4.2.0 release: https://twitter.com/oas_generator/status/1189824932345069569 |
PR checklist
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.sh,./bin/openapi3/{LANG}-petstore.shif updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in.\bin\windows\. If contributing template-only or documentation-only changes which will change sample output, be sure to build the project first.master,4.1.x,5.0.x. Default:master./cc @jonahgeorge @wing328 @frol @farcaller @bjgill @richardwhiuk
Description of the PR
Fix #3052. This PR is based on @jonahgeorge work. It follows (and replace) PR #3250.
This PR includes theses changes:
For now,nullableattribut is not handled.