[BUG][GO] Include imports for fields when using polymorphism #19958
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.
The current implementation treats
anyOf
andoneOf
slightly differently thanallOf
. The intention is clear from the comment "the model is no model, but is a anyOf or oneOf". However, the generator does include all properties and just makes this distinction when dealing with imports. So one would only see the issue if the property on the parent also requires importing something, e.g. it is a datetime and requiresimport time
.With this update we instead treat all the polymorphism features the same way when dealing with imports.
Testing
Tested separately using https://gist.github.com/perhallgren/d2af2441d7c265a06407313b064f68d6 and also using the 3GPP standard as inputs.
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
(upcoming7.x.0
minor release - breaking changes with fallbacks),8.0.x
(breaking changes without fallbacks)Discussion
What's the correct way to read the specs?
I didn't find the specs very explicit on if it is valid to use properties on a model that uses
anyOf
/oneOf
polymorphism.allOf
is pretty straight forward, like normal OOP inheritence - theDog extends Pet
.oneOf
/anyOf
are applied in the parent model instead of the child, and intuitively if aPet
isoneOf
Dog
orCat
it doesn't make sense that it has properties that doesn't exist on either dog or cat.I think the spec gives the impression that the discriminator property should be dealt with in the same way for all of them. It also says this:
Which actually doesn't spell out that you can or cannot add the discriminator in the parent object also for
oneOf
andanyOf
. But since that would make everyone's life easier, users will refrain from copy-pasting and using the parent object. An example where this was expected to be possible can be found in #16016.Usage in the wild
The 3GPP 5G standard is using
anyOf
on models that also has properties defined. There, it is expected that even if a model has ananyOf
/oneOf
, the properties specified on the model are treated as for any other model.