[BUG][DART] Fix anyOf undeclared enum, empty class syntax, and nested map cast#23671
[BUG][DART] Fix anyOf undeclared enum, empty class syntax, and nested map cast#23671winrid wants to merge 3 commits intoOpenAPITools:masterfrom
Conversation
… map cast
Three bugs in the dart generator surfaced from the same end-user spec.
The repros are added to the dart fake-petstore yaml as PetReactionStatus,
PetReactionResponse, PetEmptyMetadata, PetReactionsResponse so each can
be reproduced and red/green tested against generated output.
1. anyOf of [$ref enum, inline literal enum] emitted a property typed as
`StatusEnum?` plus `StatusEnum.fromJson(...)`, but never declared the
enum class anywhere in lib/model/. The dart generator can't produce a
single enum class that covers all branches and the inline-enum
template is suppressed for composed schemas. Fix: in
AbstractDartCodegen.fromProperty, when the composed schema has 2+
branches and default codegen guessed at isEnum=true, drop the enum
projection (clear isEnum, enumName, _enum, allowableValues; reset
datatypeWithEnum=dataType). Single-element composed schemas are
unchanged.
2. Empty-property classes emitted invalid Dart syntax: `Foo({ })` (Dart
rejects empty named-args), a dangling `&&` on `==`, and a missing
rhs/`;` on `hashCode`. Fix: `{{#hasVars}}` guards in
dart_constructor.mustache and native_class.mustache. Empty classes
now emit `Foo()`, `==> identical(this, other) || other is Foo;`,
and `hashCode => 0;`.
3. `Map<String, Map<String, T>>` properties were decoded with
`mapCastOfType<String, dynamic>` (returns `Map<String, dynamic>`,
doesn't cast inner generics) which can't be assigned to the declared
field type. Fix: native_class.mustache's nested-map branch now emits
`(json[r'X'] as Map).map((k, v) => MapEntry(k as String, (v as Map).cast<String, T>()))`.
Also fixes the same broken pattern in the pre-existing
additional_properties_class.dart and map_test.dart samples.
Verified red/green: pre-fix `dart analyze` reported the exact symptoms
(undefined StatusEnum, empty-class syntax errors, Map<String, dynamic>
not assignable). Post-fix all three model files are clean; remaining
errors in the fake sample are pre-existing and unrelated. Plain
petstore_client_lib sample analyzes clean. All 88 org.openapitools
.codegen.dart.* tests pass.
Closes OpenAPITools#23665, closes OpenAPITools#16715
Refs OpenAPITools#9272, OpenAPITools#12914, OpenAPITools#15670
There was a problem hiding this comment.
3 issues found across 76 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed. cubic prioritises the most important files to review.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="samples/openapi3/client/petstore/dart2/petstore_client_lib_fake/lib/model/pet_reactions_response.dart">
<violation number="1" location="samples/openapi3/client/petstore/dart2/petstore_client_lib_fake/lib/model/pet_reactions_response.dart:16">
P2: Defaulting mutable map fields to `const {}` makes the zero-arg constructor return unmodifiable maps that throw on mutation.</violation>
<violation number="2" location="samples/openapi3/client/petstore/dart2/petstore_client_lib_fake/lib/model/pet_reactions_response.dart:33">
P2: Deep equality in `==` is not matched by `hashCode`, so equal instances can hash differently.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/dart2/serialization/native/native_class.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/dart2/serialization/native/native_class.mustache:233">
P1: Required nested-map deserialization can still return null because `!` only applies to the else branch of the ternary, breaking non-nullable required fields.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @override | ||
| int get hashCode => | ||
| // ignore: unnecessary_parenthesis | ||
| (myReacts.hashCode) + |
There was a problem hiding this comment.
P2: Deep equality in == is not matched by hashCode, so equal instances can hash differently.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/openapi3/client/petstore/dart2/petstore_client_lib_fake/lib/model/pet_reactions_response.dart, line 33:
<comment>Deep equality in `==` is not matched by `hashCode`, so equal instances can hash differently.</comment>
<file context>
@@ -0,0 +1,117 @@
+ @override
+ int get hashCode =>
+ // ignore: unnecessary_parenthesis
+ (myReacts.hashCode) +
+ (reactionCounts.hashCode);
+
</file context>
| class PetReactionsResponse { | ||
| /// Returns a new [PetReactionsResponse] instance. | ||
| PetReactionsResponse({ | ||
| this.myReacts = const {}, |
There was a problem hiding this comment.
P2: Defaulting mutable map fields to const {} makes the zero-arg constructor return unmodifiable maps that throw on mutation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/openapi3/client/petstore/dart2/petstore_client_lib_fake/lib/model/pet_reactions_response.dart, line 16:
<comment>Defaulting mutable map fields to `const {}` makes the zero-arg constructor return unmodifiable maps that throw on mutation.</comment>
<file context>
@@ -0,0 +1,117 @@
+class PetReactionsResponse {
+ /// Returns a new [PetReactionsResponse] instance.
+ PetReactionsResponse({
+ this.myReacts = const {},
+ this.reactionCounts = const {},
+ });
</file context>
…sion Cubic review on OpenAPITools#23671 caught that the prior shape json[X] == null ? null : (json[X] as Map).map(...).cast<...>()! applied the trailing ! only to the else branch, so a required non-nullable Map<String, Map<...>> field could still be assigned null from the ? branch. Restructure to a single nullable expression that matches the existing mapCastOfType pattern in the same file: (json[X] as Map?)?.map((k, v) => MapEntry(k as String, (v as Map).cast<String, T>())) {{#required}}{{^isNullable}}!{{/isNullable}}{{/required}} {{^required}}{{#defaultValue}} ?? <default>{{/defaultValue}}{{/required}} Now ! correctly applies to the whole nullable expression, ?? feeds the default consistently with the other map branches, and the optional case collapses to a single nullable expression instead of a ternary.
|
Thanks. Issue 1 (P1) is a real bug, fixed in 8df1ddd: restructured to Issues 2 and 3 are pre-existing dart template behavior (#15763 tracks the |
@winrid That makes sense. The fix for Issue 1 in Regarding Issues 2 and 3, I agree with keeping them out of scope here. Since the default collection initialization and the You can manage how I learn from these types of project-specific conventions in your learnings or adjust my behavior in settings. |
Added PetReactionStatus, PetReactionResponse, PetEmptyMetadata, PetReactionsResponse to the dart fake-petstore yaml; that yaml is also consumed by dart-dio configs. Regenerate to keep CI 'Samples up-to-date' green. Also strip the auto-generated test stub entries from the FILES index because CI's regen filters them out (see OpenAPITools#23669 for the same pattern).
There was a problem hiding this comment.
3 issues found across 32 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="samples/openapi3/client/petstore/dart-dio/petstore_client_lib_fake/lib/src/model/pet_reaction_response.dart">
<violation number="1" location="samples/openapi3/client/petstore/dart-dio/petstore_client_lib_fake/lib/src/model/pet_reaction_response.dart:88">
P2: Nullable field `petId` is deserialized with a non-null cast, so an explicit `null` value can crash deserialization.</violation>
<violation number="2" location="samples/openapi3/client/petstore/dart-dio/petstore_client_lib_fake/lib/src/model/pet_reaction_response.dart:95">
P2: Nullable field `status` is deserialized with a non-null cast, so an explicit `null` value can crash deserialization.</violation>
</file>
<file name="samples/openapi3/client/petstore/dart-dio/petstore_client_lib_fake-json_serializable/lib/src/model/pet_reactions_response.dart">
<violation number="1" location="samples/openapi3/client/petstore/dart-dio/petstore_client_lib_fake-json_serializable/lib/src/model/pet_reactions_response.dart:55">
P2: `operator ==` compares nested `Map` fields by reference instead of deep value equality, so structurally equal responses can compare unequal.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| final valueDes = serializers.deserialize( | ||
| value, | ||
| specifiedType: const FullType(String), | ||
| ) as String; |
There was a problem hiding this comment.
P2: Nullable field status is deserialized with a non-null cast, so an explicit null value can crash deserialization.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/openapi3/client/petstore/dart-dio/petstore_client_lib_fake/lib/src/model/pet_reaction_response.dart, line 95:
<comment>Nullable field `status` is deserialized with a non-null cast, so an explicit `null` value can crash deserialization.</comment>
<file context>
@@ -0,0 +1,126 @@
+ final valueDes = serializers.deserialize(
+ value,
+ specifiedType: const FullType(String),
+ ) as String;
+ result.status = valueDes;
+ break;
</file context>
| final valueDes = serializers.deserialize( | ||
| value, | ||
| specifiedType: const FullType(int), | ||
| ) as int; |
There was a problem hiding this comment.
P2: Nullable field petId is deserialized with a non-null cast, so an explicit null value can crash deserialization.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/openapi3/client/petstore/dart-dio/petstore_client_lib_fake/lib/src/model/pet_reaction_response.dart, line 88:
<comment>Nullable field `petId` is deserialized with a non-null cast, so an explicit `null` value can crash deserialization.</comment>
<file context>
@@ -0,0 +1,126 @@
+ final valueDes = serializers.deserialize(
+ value,
+ specifiedType: const FullType(int),
+ ) as int;
+ result.petId = valueDes;
+ break;
</file context>
|
|
||
|
|
||
| @override | ||
| bool operator ==(Object other) => identical(this, other) || other is PetReactionsResponse && |
There was a problem hiding this comment.
P2: operator == compares nested Map fields by reference instead of deep value equality, so structurally equal responses can compare unequal.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/openapi3/client/petstore/dart-dio/petstore_client_lib_fake-json_serializable/lib/src/model/pet_reactions_response.dart, line 55:
<comment>`operator ==` compares nested `Map` fields by reference instead of deep value equality, so structurally equal responses can compare unequal.</comment>
<file context>
@@ -0,0 +1,74 @@
+
+
+ @override
+ bool operator ==(Object other) => identical(this, other) || other is PetReactionsResponse &&
+ other.myReacts == myReacts &&
+ other.reactionCounts == reactionCounts;
</file context>
Closes #23665, closes #16715. Refs #9272, #12914, #15670.
Three dart generator bugs surfaced from the same end-user spec, fixed together because they share the same template + sample regen surface.
1. anyOf of $ref enum + inline literal enum emits undeclared
StatusEnum(closes #23665)Previously emitted
StatusEnum? statusplusStatusEnum.fromJson(...), butStatusEnumwas never declared anywhere inlib/model/. The dart generator can't produce a single enum class that covers all branches, and the inline-enum template is suppressed for composed schemas.Fix: in
AbstractDartCodegen.fromProperty, when the composed schema has 2+ branches and the default codegen guessed atisEnum=true, drop the enum projection (clearisEnum,enumName,_enum,allowableValues; resetdatatypeWithEnum=dataType). Single-element composed schemas are unchanged. Output is nowString? statuswithmapValueOfType<String>(...)deserialization.2. Empty-property classes emit invalid Dart syntax (closes #16715)
A schema resolving to a class with zero properties produced:
Foo({ })(Dart rejects empty named-args lists)&&on==with no rhs and no;hashCodeFix:
{{#hasVars}}guards indart_constructor.mustacheandnative_class.mustache. Empty classes now emitFoo(),bool operator ==(Object other) => identical(this, other) || other is Foo;, andint get hashCode => 0;.3.
Map<String, Map<String, T>>decoded with wrong generic args (refs #9272, #12914, #15670)Previously emitted
mapCastOfType<String, dynamic>(json, r'myReacts')which returnsMap<String, dynamic>and can't be assigned to the declared field typeMap<String, Map<String, bool>>.Fix:
native_class.mustache's nested-map branch now emitsThis also fixes the same broken pattern that was already in the pre-existing
additional_properties_class.dartandmap_test.dartsamples.Reproducers + verification
PetReactionStatus,PetReactionResponse,PetEmptyMetadata,PetReactionsResponseadded to the dart fake-petstore yaml as red-green test fixtures. Pre-fixdart analyzereported each bug verbatim (undefinedStatusEnum, empty-class syntax errors,Map<String, dynamic>not assignable). Post-fix all three model files are clean; remaining errors in the fake sample are pre-existing and unrelated to these bugs. Plainpetstore_client_libsample analyzes clean. All 88org.openapitools.codegen.dart.*Java tests pass.PR checklist
master(5.x.x) (patches will be cascaded frommasterto other branches by the maintainers)cc @jaumard @amondnet @sbu-WBT @kuhnroyal @agilob @ahmednfwela