Feature/fix issues - #24961
Draft
Picazsoo wants to merge 6 commits into
Draft
Feature/fix issues#24961Picazsoo wants to merge 6 commits into
Picazsoo wants to merge 6 commits into
Conversation
…le errors Fixes 4 compile-breaking Kotlin output issues for allOf/discriminator inheritance hierarchies in the kotlin-spring generator: 1. Missing override modifier when a subtype re-declares a property inherited through an allOf-composed parent (AbstractKotlinCodegen now resolves the parent's fully-flattened allOf property set via a new collectAllOfPropertyNames helper instead of only reading the parent schema's direct properties). 2. Discriminator property typed as a narrower per-subtype enum instead of the parent's String type, causing an invalid Kotlin override. The existing discriminator-normalization pass (previously oneOf-only) now also runs for allOf-based discriminator children, fixing the property's dataType/isEnum without altering existing default-value behavior. 3. Free-form/map-typed schemas with a discriminator were rendered as interface X : HashMap<...>(), which Kotlin forbids (an interface cannot extend a class). Map-typed models are now unconditionally excluded from interface promotion and rendered as open class instead. 4. A schema used as an allOf parent by other schemas, but with no discriminator of its own, was emitted as a data class, which Kotlin disallows extending. Added new opt-in additionalProperty ixPolymorphicInheritance (default false) that promotes such models to interface (reusing existing interface/override machinery), gated behind a flag since it changes the generated type shape. Added regression tests and a minimal repro spec (polymorphism-allof-discriminator-inheritance.yaml) covering all 4 issues, verified against kotlinc compilation with the flag on/off. Regenerated affected kotlin-spring samples (whitespace-only diffs from removing dead template code) and docs/generators/kotlin-spring.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…pped interfaces
Fixes a Jackson runtime deserialization gap: a model rendered as an
interface (via a genuine own discriminator or via
ixPolymorphicInheritance promotion of an �llOf parent) that is also
named as a value in some discriminator.mapping cannot be instantiated by
Jackson, since @JsonSubTypes would otherwise point at an abstract type.
This adds a synthetic concrete <Schema>Impl data class implementing the
interface whenever the interface's own schema name appears as a
discriminator mapping value anywhere in the document (covering both a
promoted allOf-parent used directly as a payload type, and a
genuinely-discriminated root that maps to itself in its own mapping -
a pre-existing, fix-independent bug). The corresponding @JsonSubTypes
entry is redirected to the Impl class while keeping the wire-format
ame unchanged.
- KotlinSpringServerCodegen.java: compute discriminatorMappedModelNames and
set x-kotlin-poly-impl-needed vendor extension.
- dataClass.mustache: emit the additional {{classname}}Impl data class.
- implClassReqVar.mustache / implClassOptVar.mustache: new partials with
unconditional override for the Impl class constructor properties.
- typeInfoAnnotation.mustache: redirect @JsonSubTypes.Type value to the
Impl class when applicable.
- Extend polymorphism-allof-discriminator-inheritance.yaml regression spec
and add 2 new tests to KotlinSpringServerCodegenTest covering the
self-mapped-root and promoted-interface-referenced-elsewhere cases.
- Update fixPolymorphicInheritance CLI option description and regenerate
docs/generators/kotlin-spring.md.
Verified: full kotlin/kotlin-spring test suite passes (0 failures);
regenerated all 34 bin/configs/kotlin-spring*.yaml samples with no
unintended diffs; repro spec compiles cleanly with kotlinc and Jackson
deserializes correctly at runtime for all previously-failing cases.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…hemas The synthetic `<Schema>Impl` concrete leaf (introduced for interfaces used as discriminator mapping values) previously hardcoded the literal Impl suffix independently in both dataClass.mustache and typeInfoAnnotation.mustache, with no check for whether a schema was already declared under that name in the document. A spec with e.g. both Foo (needing a synthetic leaf) and an unrelated real schema literally named FooImpl would produce a duplicate-class compile error. - KotlinSpringServerCodegen.java: compute a collision-free resolved name once, stored in a new x-kotlin-poly-impl-name vendor extension: try `<classname>Impl`, falling back to `<classname>Impl2`, `Impl3`, ... against the set of all real schema classnames in the document. - dataClass.mustache / typeInfoAnnotation.mustache: reference x-kotlin-poly-impl-name instead of hardcoding the Impl literal, removing the duplicated/hardcoded suffix logic. - Extend polymorphism-allof-discriminator-inheritance.yaml with a deliberately colliding PlaceImpl schema and add a regression test asserting the fallback name (PlaceImpl2) is used and correctly wired into @JsonSubTypes. Verified: full kotlin/kotlin-spring suite passes (276 tests, 0 failures); regenerated regression spec compiles cleanly with kotlinc/Maven; all 34 bin/configs/kotlin-spring*.yaml samples regenerated with no unintended diffs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ing)
Previously the synthetic <Schema>Impl concrete leaf class (introduced to
fix a Jackson runtime deserialization gap for interfaces promoted by
fixPolymorphicInheritance, or self-mapped discriminator roots) was
appended as extra content inside the interface's own generated .kt file.
This meant it could not be suppressed or substituted via the standard
schemaMapping/importMapping conventions the way real schemas can.
This change moves the Impl class into its own genuinely separate
generated model file:
- KotlinSpringServerCodegen#postProcessAllModels now builds a synthetic
CodegenModel/ModelMap/ModelsMap for the Impl class and injects it into
the map returned by postProcessAllModels, which is what drives
DefaultGenerator's per-model file-generation loop. This gets
schemaMapping-based suppression for free, since DefaultGenerator's
file-generation loop already checks config.schemaMapping() against
every key in that map, including injected ones.
- Extracted the Impl class's body out of dataClass.mustache into a new
standalone implDataClass.mustache, dispatched from model.mustache via
a new x-kotlin-poly-impl-class vendor extension (mirroring the
existing isEnum/x-is-one-of-interface dispatch pattern). This keeps
dataClass.mustache limited to its original interface/data-class/
open-class-for-map branching, with zero awareness of the Impl
mechanism.
- implClassReqVar.mustache/implClassOptVar.mustache updated to qualify
nested enum-typed properties via {{parent}} instead of {{classname}},
since the Impl class now renders in its own separate model context.
Also documents in the fixPolymorphicInheritance CLI option description
that the synthetic Impl class is generated in its own file and can be
suppressed via schema-mappings.
Verified: full kotlin/kotlin-spring codegen test suite passes (0
failures); regenerated regression spec with the flag enabled compiles
cleanly with kotlinc via a Maven scaffold, with the Impl classes
(including the Phase 3 collision-fallback PlaceImpl2 case) now living in
their own separate files; regenerated all 34 kotlin-spring sample
configs with no unintended diffs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…terfaces gap A model promoted to interface by fixPolymorphicInheritance (allOf parent with children, no own discriminator) that is also a member of a discriminator-free oneOf union (useDeductionForOneOfInterfaces) compiled but failed at runtime: oneof_interface.mustache's deduction @JsonSubTypes block still named the now-abstract interface directly instead of redirecting to the synthetic <Schema>Impl leaf that the existing discriminator-mapping path already uses. - KotlinSpringServerCodegen.java: collect classnames from interfaceModels of every discriminator-free, non-empty oneOf model (deductionOneOfMemberModelNames), union with discriminatorMappedModelNames when computing needsSyntheticImpl. - oneof_interface.mustache: deduction JsonSubTypes entries now redirect to x-kotlin-poly-impl-name when set, mirroring typeInfoAnnotation.mustache. - New regression spec + two tests (flag-on/flag-off) covering the interaction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…criminator inheritance)
Fixes three real-world (TMForum-sourced) kotlin-spring codegen bugs found in addition to
the earlier fixPolymorphicInheritance work. All three are pre-existing, always-on bugs
(reproduced identically regardless of fixPolymorphicInheritance), so the fixes are
unconditional default behavior changes, not gated behind any flag.
- Regression A: an anyOf-single-ref to a discriminated schema (e.g. a synthesized
recursive "map value" inline model) was falsely marked isInherited by the Kotlin
override-detection step even though anyOf never sets a real Kotlin parent, emitting
override with no supertype clause at all. Fixed by guarding that detection step on
m.parent != null in AbstractKotlinCodegen.fromModel.
- Regression B: a oneOf+discriminator "grouping" interface with no properties of its own
(its abstract discriminator property is synthesized purely from
discriminator.propertyName) could be implemented by models that never supply a value for
that property (e.g. because they extend an unrelated allOf base instead), failing to
compile ("does not implement abstract member"). Fixed by synthesizing a computed
(getter-only) override using the model's own entry in the discriminator's mapping as
the literal wire value, scoped precisely to oneof_interface.mustache-rendered interfaces
(x-is-one-of-interface) so genuine allOf discriminator roots (whose discriminator is
handled purely via @JsonTypeInfo/@JsonIgnoreProperties, with no abstract Kotlin member at
all) are left untouched.
- Regression C: an inherited (not redeclared) enum-typed property's nested type reference
was wrongly qualified with the child/composing model's own classname instead of the
model that actually declares the nested enum, producing an unresolved reference. Fixed by
generalizing the discriminator-only enum retargeting into a general mechanism: any
enum-typed property present in a model's requiredVars/optionalVars/allVars but absent
from its own �ars (so it won't render a nested enum there) is retargeted to whichever
candidate model (parent, or any anyOf/allOf/oneOf composition sibling) actually declares
it, via a new x-kotlin-enum-owner vendor extension consumed by the
dataClass/interface Req/OptVar templates.
Added 3 new minimal regression specs and 3 new tests in KotlinSpringServerCodegenTest.
Verified: full kotlin/kotlin-spring codegen test suite (575 tests, 0 failures); the
original combined TMForum-derived repro spec now compiles cleanly end-to-end via
kotlinc/Maven; all 34 kotlin-spring sample configs regenerated with no unintended diffs.
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.
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.
Summary by cubic
Fixes compile-breaking Kotlin output for
allOf/discriminatorinheritance hierarchies in thekotlin-springgenerator, and adds an opt-infixPolymorphicInheritanceflag that promotes non-discriminatedallOfparents tointerfaceso subtypes can legally extend them. Also fixes three pre-existing, always-on compile regressions found in real-world specs.Bug Fixes
allOf-composed parent now get the requiredoverridemodifier.Stringtype forallOfchildren.discriminatornow render asopen classinstead ofinterface : HashMap<...>(), which Kotlin forbids.<Schema>Implleaf so Jackson can construct them.overridewithout a real Kotlin parent.fixPolymorphicInheritanceinterfaces used in deduction-based oneOf unions now redirect@JsonSubTypesto the synthetic Impl leaf.New Features
fixPolymorphicInheritance(default false) renders a non-discriminatedallOfparent as aninterfaceinstead of a finaldata class, so affected schemas can no longer be instantiated directly.<Schema>Impldata class in its own file, named collision-free (Impl,Impl2, ...), suppressible viaschemaMapping.Written for commit 5c8412a. Summary will update on new commits.