[Dart] [Client] Miscellaneous bug fixes for syntax errors, runtime exceptions, and deserialization errors #17548
Conversation
…n underscore to avoid name conficts with user supplied parameters
…es.isEmpty because .body implicitly converts to a utf-8 string which is will cause crashes when the data isnt utf-8
…eviously would fail when deserding doubles
…h is not strictly true, but allows for better generation of Enums that may use it
* Generated Enums now have equality and hashcode methods checking against their in-memory id then against their underlying value * Classes with a generated Numeric Enum in their constructor enums now instantiate sytactically correctly, without causing compile errors * Classes with default-available generated Enum in their fromJson() method now use a syntactically correct default, without causing compile errors
* Imported typed_data * File and Binary response return types are now marked as Uint8List instead of MultipartFile, which was never semantically correct * _decodeBodyBytes has been reworked to return either 'String' or 'Uint8List' based on the ContentType. * _deserialize now returns 'dynamic' to work with the new _decodeBodyBytes
kuhnroyal
left a comment
kuhnroyal
left a comment
There was a problem hiding this comment.
Thanks for your PR!
I left 2 comments but IMO this is way to big to be thoroughly reviewed. I would suggest splitting it into multiple PRs.
| yield | ||
| yield | ||
| value | ||
| apiClient |
There was a problem hiding this comment.
This is not a valid solution. If there are clashes then you need to find another solution.
There was a problem hiding this comment.
while I agree it's kind of a dirty solution, I'm not sure why it's not a valid solution. We have a mechanism already built into the generator logic for eliminating collisions, why not use it? What is the value in the keeping the purity of the list to what is strictly in the Dart language's reserved word list?
Would you be opposed to adding a second list, say openapitools_reserved_words_dart.txt that contains things owned by the generator itself, that then uses the same system for collision removal?
| required && !defaultValue in OAS | ||
| }} | ||
| {{#required}}{{^defaultValue}}required {{/defaultValue}}{{/required}}this.{{{name}}}{{#defaultValue}} = {{#isEnum}}{{^isContainer}}const {{{enumName}}}._({{/isContainer}}{{/isEnum}}{{{.}}}{{#isEnum}}{{^isContainer}}){{/isContainer}}{{/isEnum}}{{/defaultValue}}, | ||
| {{#required}}{{^defaultValue}} required{{/defaultValue}}{{/required}} this.{{{name}}}{{#defaultValue}} = {{#isEnum}}{{^isContainer}}{{^isNumeric}}const {{{enumName}}}._({{/isNumeric}}{{/isContainer}}{{/isEnum}}{{{.}}}{{#isEnum}}{{^isContainer}}{{^isNumeric}}){{/isNumeric}}{{/isContainer}}{{/isEnum}}{{/defaultValue}}, |
There was a problem hiding this comment.
This adds an extra white space which is not needed and now indented with 3 spaces.
| /// Returns a new [ModelClient] instance. | ||
| ModelClient({ | ||
| this.client, | ||
| this.client, |
|
If I can find the time, I'll try to break this out into n-many pull requests. |
|
Adding on this topic, I have found that override is also missing from reserved keywords, and in an example I have seen there is a parameter in the openapi spec named override. This will generate a field in the model I understand that override is not a keyword in dart per se, but since it being a const var in the SDK used for obvious reasons, maybe adding that to the reserved keywords list would be a good idea? |
|
This PR has been split into most of its constituent pieces:
I have two PRs that I haven't pushed yet, including an implementation of OneOf and AnyOf, and one to fix name collisions. After those, I have some other improvements to the generator I'd like to make as well, but first I'd like to get these any issues sorted out, and these merged in. |
4 participants
This update addresses a number of pain points I've encountered while using the Dart2 generator. I understand that this generator is considered deprecated, but as there has been no serious movement on the dart-next generator for 6+ months, I thought that it may as well get some attention.
This PR contains fixes for the following:
#15393 (f80b51c)
#12456 (6e5368c and 271bd9b)
#17546 (8ed191c)
#17547 and #13459 (83f5ecb)
#12161 (1a30296)
In summary:
File and Binary response return types are now marked as Uint8List instead of MultipartFile, which was never semantically correct
_decodeBodyBytes has been reworked to return either 'String' or 'Uint8List' based on the ContentType.
_deserialize now returns 'dynamic' to work with the new _decodeBodyBytes
Generated Enums now have equality and hashcode methods checking against their in-memory id then against their underlying value
Classes with a generated Numeric Enum in their constructor enums now instantiate sytactically correctly, without causing compile errors
Classes with default-available generated Enum in their fromJson() method now use a syntactically correct default, without causing compile errors
Added 'value' and 'apiClient' to reserved dart keywords, which is not strictly true, but allows for better generation of Enums that may use it
Added better double handling to 'mapValueOfType', which previously would fail when deserding doubles
Moved from checking Response.body.isEmpty to Response.bodyBytes.isEmpty because .body implicitly converts to a utf-8 string which is will cause crashes when the data isnt utf-8
All local variables in the 'Api' generations now begin with an underscore to avoid name conficts with user supplied parameters
You can use the following OpenAPI file to test the generation: https://gist.github.com/0xNF/f0119f6a6a49424570803d7d3ed450ff
PR checklist
master@jaumard (2018/09) @josh-burton (2019/12) @amondnet (2019/12) @sbu-WBT (2020/12) @kuhnroyal (2020/12) @agilob (2020/12) @ahmednfwela (2021/08)