feat: nested sealed hierarchies for oneOf/anyOf subtypes#29
feat: nested sealed hierarchies for oneOf/anyOf subtypes#29halotukozak wants to merge 18 commits intomasterfrom
Conversation
…archies - ModelGenerator produces sealed class with nested data class subtypes for oneOf/anyOf-with-discriminator - Sealed interface pattern preserved only for anyOf-without-discriminator (JsonContentPolymorphicSerializer) - TypeMapping.toTypeName accepts classNameLookup for nested ClassName resolution - SerializersModuleGenerator uses parentClass.nestedClass(variant) for qualified references - One FileSpec per hierarchy, variant schemas filtered from separate file generation - All polymorphic tests updated for sealed class assertions, superclass checks, nested type verification Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…erator - ClientGenerator accepts classNameLookup for nested ClassName resolution in endpoint signatures - CodeGenerator builds classNameLookup via ModelGenerator companion and passes to ClientGenerator - All three TypeMapping.toTypeName call sites in ClientGenerator updated - Full test suite passes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the Kotlin model/code generation pipeline to emit polymorphic oneOf / discriminated anyOf schemas as sealed classes with nested subtypes (e.g., Shape.Circle) and wires a classNameLookup through generators so references resolve to nested ClassNames.
Changes:
- Generate sealed class hierarchies with nested data-class variants for
oneOf/ discriminatedanyOf(keepanyOfwithout discriminator as sealed interface +JsonContentPolymorphicSerializer). - Add
classNameLookup: Map<String, ClassName>support toTypeMapping.toTypeName()and propagate it throughCodeGenerator→ClientGenerator. - Update
SerializersModuleGeneratorto register nested variant classes and expand polymorphic generation tests accordingly.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| core/src/main/kotlin/com/avsystem/justworks/core/gen/ModelGenerator.kt | Implements sealed-class-with-nested-variants generation, builds classNameLookup, and adapts serializer generation paths. |
| core/src/main/kotlin/com/avsystem/justworks/core/gen/TypeMapping.kt | Extends type mapping API with classNameLookup for nested ClassName resolution. |
| core/src/main/kotlin/com/avsystem/justworks/core/gen/SerializersModuleGenerator.kt | Switches subclass registration to parentClass.nestedClass(variant). |
| core/src/main/kotlin/com/avsystem/justworks/core/gen/CodeGenerator.kt | Computes and passes classNameLookup into ClientGenerator. |
| core/src/main/kotlin/com/avsystem/justworks/core/gen/ClientGenerator.kt | Uses TypeMapping.toTypeName(..., classNameLookup) for params/return types to resolve nested models. |
| core/src/test/kotlin/com/avsystem/justworks/core/gen/ModelGeneratorPolymorphicTest.kt | Updates/extends tests to assert nested sealed hierarchies and classNameLookup behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
core/src/main/kotlin/com/avsystem/justworks/core/gen/TypeMapping.kt
Outdated
Show resolved
Hide resolved
core/src/main/kotlin/com/avsystem/justworks/core/gen/model/ModelGenerator.kt
Outdated
Show resolved
Hide resolved
core/src/main/kotlin/com/avsystem/justworks/core/gen/model/ModelGenerator.kt
Outdated
Show resolved
Hide resolved
Resolve conflicts between master's refactoring (modular generators, context receivers, package reorganization) and this branch's features (NameRegistry, classNameLookup, nested sealed hierarchies). - Keep master's internal object pattern and context receivers - Integrate NameRegistry replacing InlineSchemaDeduplicator - Add classNameLookup support to toTypeName() for nested class resolution - Thread classNameLookup through CodeGenerator and ClientGenerator - Update all test files to use master's generate() helper pattern
Resolve conflicts between unified NameRegistry refactor (master) and nested sealed hierarchy support (feature branch), preserving both inline type resolution and classNameLookup threading. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Without this, nested variant references like List<Circle> or Map<String, Circle> resolved to flat names instead of Shape.Circle. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ierarchy support - Introduce the `Hierarchy` class to encapsulate schema hierarchy logic. - Update all context receiver calls to use `Hierarchy` instead of `ModelPackage`. - Clean up unused methods and remove redundant `classNameLookup` arguments. - Optimize type resolution logic with hierarchy-aware lookups in `toTypeName`.
…`Hierarchy` for better performance - Refactor `Hierarchy` to use constructor initialization instead of lazy properties. - Simplify hierarchy-related lookups and sealed hierarchy resolution logic. - Adjust `ModelGenerator` and `ClientGenerator` to align with the updated `Hierarchy` design. - Remove unused parameters from `generate` and `collectAllInlineSchemas` methods. - Ensure consistent usage of `Hierarchy` operator function for type lookups.
…REFIX` constant - Move `SCHEMA_PREFIX` to a shared location in `Utils` for reuse. - Convert `resolveSerialName` to an extension function on `SchemaModel`. - Update all usages to align with the simplified and centralized design.
…iminator bug Tests referenced the removed ModelGenerator.HierarchyInfo and old ModelPackage context. Also fixed a bug where anyOfWithoutDiscriminator was incorrectly assigned variant names instead of parent names. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…in ModelGenerator - Remove variantParents and anyOfWithoutDiscriminatorVariants from Hierarchy, derive them inline where needed in ModelGenerator - Use lazy fields in Hierarchy with shared polymorphicSchemas computation - Precompute schemasById in Hierarchy instead of ad-hoc associateBy calls - Extract buildConstructorAndProperties to deduplicate generateDataClass and buildNestedVariant Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…-checks and sequence usage - Replace redundant null-checks and improve filtering logic with `filterNot` and `orEmpty`. - Optimize `nestedVariantNames` computation using sequences for better efficiency. - Minor code formatting improvements across classes.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (3)
core/src/main/kotlin/com/avsystem/justworks/core/gen/model/ModelGenerator.kt:227
generateSealedHierarchydoesn't add the UUID-related file annotations (@OptIn(ExperimentalUuidApi)+@UseSerializers(UUIDSerializer::class)) thatgenerateDataClassadds when a schema has UUID properties. For nested variants, this means a UUID field inside a variant would not trigger the required file-level annotations, likely breaking compilation/serialization. Consider scanning nested variant schemas for UUID usage and adding the same annotations to the parent file builder when needed.
val serialName = schema.resolveSerialName(variantName)
val nestedType = buildNestedVariant(variantSchema, variantName, className, serialName)
parentBuilder.addType(nestedType)
}
val fileBuilder = FileSpec.builder(className).addType(parentBuilder.build())
if (schema.discriminator != null) {
fileBuilder.addAnnotation(
AnnotationSpec
.builder(OPT_IN)
.addMember("%T::class", EXPERIMENTAL_SERIALIZATION_API)
core/src/main/kotlin/com/avsystem/justworks/core/gen/Hierarchy.kt:46
Hierarchy.lookupis built via...flatMap { ... }.toMap(). If the same schema name appears as a variant in multiple discriminated hierarchies,toMap()will silently keep only the last mapping, makinghierarchy[name]nondeterministic/incorrect and potentially generating/using the wrong nestedClassName. Either detect and fail fast on duplicates, or change the lookup to include parent context (or store a multimap and resolve based on the referencing parent).
.flatMap { (parent, variants) ->
val parentClass = ClassName(modelPackage, parent)
variants.map { variant -> variant to parentClass.nestedClass(variant) } +
(parent to parentClass)
}.toMap()
}
operator fun get(name: String): ClassName = lookup[name] ?: ClassName(modelPackage, name)
}
private fun SchemaModel.variants() = oneOf ?: anyOf
core/src/main/kotlin/com/avsystem/justworks/core/gen/shared/SerializersModuleGenerator.kt:10
- This file still imports
ModelPackageandModelGenerator, but neither is used after switching the context toHierarchy. With ktlint enabled in the repo, unused imports are likely to fail CI/lint. Remove the unused imports.
import com.avsystem.justworks.core.gen.GENERATED_SERIALIZERS_MODULE
import com.avsystem.justworks.core.gen.Hierarchy
import com.avsystem.justworks.core.gen.ModelPackage
import com.avsystem.justworks.core.gen.POLYMORPHIC_FUN
import com.avsystem.justworks.core.gen.SERIALIZERS_MODULE
import com.avsystem.justworks.core.gen.SUBCLASS_FUN
import com.avsystem.justworks.core.gen.invoke
import com.avsystem.justworks.core.gen.model.ModelGenerator
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
core/src/main/kotlin/com/avsystem/justworks/core/gen/model/ModelGenerator.kt
Show resolved
Hide resolved
core/src/main/kotlin/com/avsystem/justworks/core/gen/model/ModelGenerator.kt
Show resolved
Hide resolved
…ates, add regression test for inline variants - Introduce `CacheGroup` for memoized caching in `Hierarchy`. - Add `add` method to support incremental updates to schemas. - Replace lazy properties with `memoized` caching across `Hierarchy`. - Update `ModelGenerator` and `CodeGenerator` to align with the refactored `Hierarchy`. - Add regression test to ensure inline variant handling does not crash.
…y` updates and memoization logic - Replace `CacheGroup` with thread-safe `MemoScope` for improved memoization control. - Rename `add` method to `addSchemas` for better clarity. - Update tests to align with new `Hierarchy` design and method signature changes. - Add `MemoTest` to validate memoization behavior and thread safety.
… thread-safe memoization - Remove redundant trailing comma in `GenerateResult`. - Replace Unicode characters with middle dot for clarity in statements. - Refactor `generateNestedInlineClass` in `ModelGenerator` for better constructor and property handling. - Update `Memo` to use thread-safe `Atomic` for lazy initialization. - Simplify tests with reusable helper for schema creation.
…th nested subtypes - Replace sealed interface checks with sealed class validations in `Shape.kt`. - Update tests to confirm nested subtype definitions and eliminate separate files for variants. - Ensure generated files align with the new sealed class design.
Coverage Report
|
…vements - Add `anyOfParents` for reverse mapping of variants to parent schemas. - Add detailed KDoc comments for hierarchy-related properties. - Simplify `anyOfWithoutDiscriminator` resolution in `ModelGenerator`.
Integrate master's KDoc/documentation improvements (sanitizeKdoc, endpoint descriptions, enum value descriptions, property KDoc) with the branch's refactored ModelGenerator (extracted buildConstructorAndProperties helper, parentBuilder rename, anyOfParents field). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| } else { | ||
| typeSpec.addAnnotation(SERIALIZABLE) | ||
| } | ||
| val parentBuilder = TypeSpec.classBuilder(className).addModifiers(KModifier.SEALED) |
There was a problem hiding this comment.
Consider using sealed interfaces instead of sealed classes.
Based on DeviceStatus example:
The parent type has no constructor parameters or shared state, so sealed class is unnecessarily heavy. It forces every variant to emit : DeviceStatus() with ugly empty parens, and requires the addSuperclassConstructorParameter("") hack in buildNestedVariant. With sealed interface the output is cleaner (: DeviceStatus), and @JsonClassDiscriminator works identically - the anyOf-without-discriminator path already uses sealed interface in generateSealedInterface, so this would be consistent.
| code.beginControlFlow("%M(%T::class)", POLYMORPHIC_FUN, parentClass) | ||
| for (variant in variants) { | ||
| val variantClass = ClassName(modelPackage, variant) | ||
| val variantClass = parentClass.nestedClass(variant) |
There was a problem hiding this comment.
Cosnider sanitizing variant class names. Again on DeviceStatus example:
Schema names like "true" and "false" are used verbatim as Kotlin class names, producing backtick-escaped true/false. These should be capitalized to produce valid identifiers (True, False) while @SerialName("true") preserves the wire format.
| parentClassName: ClassName, | ||
| serialName: String, | ||
| ): TypeSpec { | ||
| val variantClassName = parentClassName.nestedClass(variantName) |
There was a problem hiding this comment.
Filter out discriminator property from variant schemas
Again based on DeviceStatus example:
When an OpenAPI spec defines discriminator.propertyName: "online", that property typically also appears in each variant's schema (per OpenAPI convention). But kotlinx.serialization manages the discriminator field automatically via @JsonClassDiscriminator - having it as a regular constructor parameter is redundant and can cause serialization conflicts (duplicate key).
| serialName: String, | ||
| ): TypeSpec { | ||
| val variantClassName = parentClassName.nestedClass(variantName) | ||
| val builder = TypeSpec.classBuilder(variantClassName).addModifiers(KModifier.DATA) |
There was a problem hiding this comment.
Also, use object for zero-property variants.
After filtering the discriminator property (previous comment), some variants may end up with zero properties. These should generate as object
| } | ||
|
|
||
| @Test | ||
| fun `CacheGroup should reset all memoized instances`() { |
There was a problem hiding this comment.
CacheGroup should be changed to MemoScope I guess
| /** | ||
| * Updates the underlying schemas and invalidates all cached derived views. | ||
| * This is necessary when schemas are updated (e.g., after inlining types). | ||
| */ |
There was a problem hiding this comment.
Shouldn't it be just above addSchemas function?
| ) | ||
|
|
||
| if (schema.description != null) { | ||
| typeSpec.addKdoc("%L", schema.description) |
There was a problem hiding this comment.
shouldnt be .sanitizeKdoc() included here?
Summary
Shape.Circle,Shape.Squareinstead of separateCircle.kt,Square.ktclassNameLookup: Map<String, ClassName>toTypeMapping.toTypeName()for nested ClassName resolutionSerializersModuleGeneratorto useparentClass.nestedClass(variant)classNameLookupthroughCodeGenerator→ClientGeneratorDepends on: #27 (feat/12-unified-name-registry)
Test plan
Shape.Circle::class@JsonClassDiscriminator+@SerialNamepreserved🤖 Generated with Claude Code