fix: treat top-level array schemas with non-trivial items as real models, not aliases#23268
Open
marineotter wants to merge 2 commits intoOpenAPITools:masterfrom
Open
fix: treat top-level array schemas with non-trivial items as real models, not aliases#23268marineotter wants to merge 2 commits intoOpenAPITools:masterfrom
marineotter wants to merge 2 commits intoOpenAPITools:masterfrom
Conversation
…posed schema Previously, shouldGenerateArrayModel() only checked schema.getProperties() to decide whether to generate a model class for a top-level array schema. Since array schemas use 'items' rather than 'properties', this check always returned false, causing all array schemas to be treated as simple aliases and skipped during model generation. After InlineModelResolver.flatten() runs, complex inline items are extracted to #/components/schemas/ and replaced with a $ref. The updated check now inspects the items schema for: - $ref (resolved reference to an extracted component) - inline properties - composed schema (oneOf/anyOf/allOf) Simple primitive aliases like List<String> or List<Integer> remain correctly skipped as they match none of the above conditions. Fixes OpenAPITools#7802
Contributor
There was a problem hiding this comment.
No issues found across 1 file
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Top-level
type: arrayschemas likeAnimalFarm(an array ofAnimal) are legitimate named models that users expect to see generated. However, the current code treats all array schemas as simple aliases (e.g.List<String>) and skips model generation entirely.This happens because
shouldGenerateArrayModel()only checksschema.getProperties()— a field that array schemas never use. Array schemas describe their element type viaitems, notproperties, so the check always returns false.This means any top-level array schema with a
$ref, composed type (oneOf/anyOf/allOf), or inline object in itsitemssilently disappears from the generated output.For reference, tested against the GitHub REST API OpenAPI schema (12MB, 916 schemas): out of 14 top-level array schemas, 12 were silently skipped despite having non-trivial items — only the 2 primitive aliases (List, List) should have been skipped.
Intent
The fix teaches
shouldGenerateArrayModel()to look atitems— the same place every other part of the codebase already looks for array element types. This brings it to parity withshouldGenerateMapModel(), which already handles the analogous case for maps.Primitive aliases like
List<String>orList<Integer>remain correctly skipped.Note on sample regeneration
Running generate-samples.sh produces a large number of new files across many generators, as previously skipped array models are now correctly generated. The regenerated samples are intentionally not included in this PR to keep the review focused. Happy to add them in a follow-up commit if preferred.
Related Issues