refactor: unified NameRegistry for all generated name deduplication by halotukozak · Pull Request #27 · AVSystem/justworks

halotukozak · 2026-03-23T16:32:40Z

Summary

New NameRegistry class with numeric suffix collision resolution (Foo → Foo2 → Foo3)
Extract InlineSchemaKey to standalone file (structural equality concern separate from naming)
Wire NameRegistry into ModelGenerator (inline schemas, enum constants), ClientGenerator (API class names, method names), CodeGenerator (registry creation + pre-population)
Delete InlineSchemaDeduplicator — replaced entirely by NameRegistry
Per-package scope: separate registries for model and API packages

Test plan

NameRegistryTest — 7 tests for collision resolution, reserve, case sensitivity
InlineSchemaDedupTest — rewritten for numeric suffix behavior
All existing tests updated and passing (ModelGenerator, ClientGenerator, Integration)

🤖 Generated with Claude Code

- register() returns desired name or appends numeric suffix (Foo2, Foo3) - reserve() pre-populates names to block subsequent registrations - 7 test cases covering collisions, reservations, and independence Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Move InlineSchemaKey data class from InlineSchemaDeduplicator.kt to own file - InlineSchemaDeduplicator unchanged, references InlineSchemaKey from same package - Pure extraction refactor, no behavior changes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- CodeGenerator creates per-package registries, pre-populates model registry - ModelGenerator uses NameRegistry for inline schemas and enum constants - ClientGenerator uses NameRegistry for API class and method names - Update test instantiations to pass NameRegistry parameter Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…eric suffixes - Remove InlineSchemaDeduplicator.kt (replaced by NameRegistry) - Rewrite collision tests to use NameRegistry directly - Expect Pet2 instead of PetInline, Context2 instead of ContextInline Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR refactors generated-name de-duplication by introducing a shared NameRegistry (numeric-suffix collisions) and wiring it into the model/client generators, while extracting InlineSchemaKey into its own file and removing InlineSchemaDeduplicator.

Changes:

Add NameRegistry and use it across generators to resolve class/method/enum-constant name collisions via Foo, Foo2, Foo3.
Extract InlineSchemaKey into a standalone file; remove InlineSchemaDeduplicator.
Update generators and tests to pass registries and align expectations with numeric-suffix behavior.

Reviewed changes

Copilot reviewed 12 out of 12 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/NameRegistry.kt	New registry implementing numeric-suffix collision resolution and reservation.
core/src/main/kotlin/com/avsystem/justworks/core/gen/InlineSchemaKey.kt	New standalone structural key for inline schema equality.
core/src/main/kotlin/com/avsystem/justworks/core/gen/InlineSchemaDeduplicator.kt	Removed; prior inline name-dedup logic deleted.
core/src/main/kotlin/com/avsystem/justworks/core/gen/ModelGenerator.kt	Inject `NameRegistry`; use it for inline schema naming and enum constant de-dup.
core/src/main/kotlin/com/avsystem/justworks/core/gen/ClientGenerator.kt	Inject `NameRegistry`; use it for API class names and per-client method name collisions.
core/src/main/kotlin/com/avsystem/justworks/core/gen/CodeGenerator.kt	Create per-package registries; pre-reserve model schema/enum names; pass registries to generators.
core/src/test/kotlin/com/avsystem/justworks/core/gen/NameRegistryTest.kt	New unit tests for registry collision/reserve behavior.
core/src/test/kotlin/com/avsystem/justworks/core/gen/InlineSchemaDedupTest.kt	Updated to focus on `InlineSchemaKey` equality and numeric-suffix behavior.
core/src/test/kotlin/com/avsystem/justworks/core/gen/ModelGeneratorTest.kt	Update constructor usage to pass a `NameRegistry`.
core/src/test/kotlin/com/avsystem/justworks/core/gen/ModelGeneratorPolymorphicTest.kt	Update constructor usage to pass a `NameRegistry`.
core/src/test/kotlin/com/avsystem/justworks/core/gen/ClientGeneratorTest.kt	Update constructor usage to pass a `NameRegistry`.
core/src/test/kotlin/com/avsystem/justworks/core/gen/IntegrationTest.kt	Update generator construction to pass a `NameRegistry`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

core/src/test/kotlin/com/avsystem/justworks/core/gen/IntegrationTest.kt

core/src/main/kotlin/com/avsystem/justworks/core/gen/ModelGenerator.kt

core/src/main/kotlin/com/avsystem/justworks/core/gen/InlineSchemaKey.kt

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

github-actions · 2026-03-24T10:18:40Z

Coverage Report

Overall Project	94.85% `-0.73%`	🍏
Files changed	93.07%	🍏

File	Coverage
NameRegistry.kt	100%	🍏
ClientGenerator.kt	99.76%	🍏
ModelGenerator.kt	94.87% `-1.21%`	🍏
InlineSchemaKey.kt	92.75% `-7.25%`	🍏
InlineTypeResolver.kt	90.54% `-9.46%`	🍏
CodeGenerator.kt	86.15%	🍏

Address PR review feedback: - Pre-populate NameRegistry in IntegrationTest to mirror CodeGenerator behavior - Add InlineTypeResolver that rewrites TypeRef.Inline → TypeRef.Reference after name registration, ensuring type references match generated class names - Normalize nested TypeRef.Inline in InlineSchemaKey equality (ignore contextHint) - Add CodeGeneratorTest covering the full generate() facade Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

core/src/test/kotlin/com/avsystem/justworks/core/gen/IntegrationTest.kt

core/src/main/kotlin/com/avsystem/justworks/core/gen/InlineSchemaKey.kt

core/src/main/kotlin/com/avsystem/justworks/core/gen/ModelGenerator.kt

core/src/main/kotlin/com/avsystem/justworks/core/gen/InlineTypeResolver.kt

…lution, inline schema type rewriting - InlineSchemaKey: recursively normalize nested TypeRef.Inline properties - InlineTypeResolver: fail-fast with error() instead of silent fallback - ModelGenerator: resolve inline schema properties through nameMap before generation - Remove redundant same-package import in IntegrationTest - Add test for nested inline schema deduplication across contextHints Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-25T12:47:10Z

core/src/main/kotlin/com/avsystem/justworks/core/gen/InlineSchemaKey.kt

+    )
+
+    companion object {
+        fun from(properties: List<PropertyModel>, required: Set<String>) = InlineSchemaKey(
+            properties = properties.map { PropertyKey(it.name, normalizeType(it.type), it.name in required) }.toSet(),


InlineSchemaKey currently ignores PropertyModel.nullable and defaultValue when computing structural equality. This can incorrectly deduplicate two inline schemas that differ only by nullability/defaults, causing the generated inline class to have the wrong Kotlin nullability and/or constructor defaults for some usages (since ModelGenerator keeps the first encountered schema’s PropertyModel values). Consider including nullable (and likely defaultValue) in PropertyKey (and in from()/normalizeType as needed) so only truly equivalent schemas dedupe.

Suggested change

)

companion object {

fun from(properties: List<PropertyModel>, required: Set<String>) = InlineSchemaKey(

properties = properties.map { PropertyKey(it.name, normalizeType(it.type), it.name in required) }.toSet(),

val nullable: Boolean,

val defaultValue: Any?,

)

companion object {

fun from(properties: List<PropertyModel>, required: Set<String>) = InlineSchemaKey(

properties = properties

.map { property ->

PropertyKey(

name = property.name,

type = normalizeType(property.type),

required = property.name in required,

nullable = property.nullable,

defaultValue = property.defaultValue,

)

}

.toSet(),

Copilot · 2026-03-25T12:47:11Z

core/src/main/kotlin/com/avsystem/justworks/core/gen/CodeGenerator.kt

-        val modelFiles = ModelGenerator(modelPackage).generate(spec)
+        val modelRegistry = NameRegistry().apply {
+            spec.schemas.forEach { reserve(it.name) }
+            spec.enums.forEach { reserve(it.name) }


ModelGenerator relies on the provided NameRegistry being pre-populated with all names that must not be used by generated inline schema classes. CodeGenerator currently reserves only spec schema/enum names; it does not reserve fixed generator outputs like "SerializersModule" (file) and "UuidSerializer" (class). An inline schema whose contextHint maps to one of these names could cause file overwrite or duplicate declarations. Consider reserving these fixed names in modelRegistry as well (or having ModelGenerator reserve its own fixed outputs).

Suggested change

spec.enums.forEach { reserve(it.name) }

spec.enums.forEach { reserve(it.name) }

// Reserve fixed generator outputs to avoid collisions with inline schema classes

reserve("SerializersModule")

reserve("UuidSerializer")

Copilot · 2026-03-25T12:47:11Z

core/src/main/kotlin/com/avsystem/justworks/core/gen/ModelGenerator.kt

+
+    fun generate(spec: ApiSpec): List<FileSpec> = generateWithResolvedSpec(spec).files
+
+    fun generateWithResolvedSpec(spec: ApiSpec): GenerateResult {


ModelGenerator now requires callers to supply a NameRegistry, but correctness depends on callers reserving component schema/enum names (and other generated names) before generateWithResolvedSpec(). Tests pass an empty registry in some places, which can allow inline schemas to take the same name as a component schema and produce duplicate types. Consider making ModelGenerator self-contained (e.g., create/populate an internal registry from the spec inside generateWithResolvedSpec, or provide a defaulted NameRegistry parameter and reserve spec.schemas/spec.enums internally).

Suggested change

fun generate(spec: ApiSpec): List<FileSpec> = generateWithResolvedSpec(spec).files

fun generateWithResolvedSpec(spec: ApiSpec): GenerateResult {

/**

* Ensures that the [nameRegistry] is aware of all top-level component schema and enum names

* before we start generating names for inline schemas. This prevents inline schemas from

* accidentally taking the same name as a component schema/enum and producing duplicate types,

* even when callers provide an empty [NameRegistry].

*/

private fun prepopulateNameRegistry(spec: ApiSpec) {

// Reserve all schema names

spec.schemas.forEach { schema ->

nameRegistry.reserve(schema.name)

}

// Reserve all enum names

spec.enums.forEach { enumModel ->

nameRegistry.reserve(enumModel.name)

}

}

fun generate(spec: ApiSpec): List<FileSpec> = generateWithResolvedSpec(spec).files

fun generateWithResolvedSpec(spec: ApiSpec): GenerateResult {

prepopulateNameRegistry(spec)

Copilot · 2026-03-25T12:47:11Z

core/src/test/kotlin/com/avsystem/justworks/core/gen/NameRegistryTest.kt

+class NameRegistryTest {
+    @Test
+    fun `register on empty registry returns desired name`() {
+        val registry = NameRegistry()
+        assertEquals("Foo", registry.register("Foo"))
+    }
+


PR description mentions NameRegistryTest covering case sensitivity, but there’s currently no test asserting case-sensitive/insensitive behavior (e.g., "Foo" vs "foo"). Either add the missing test or update the PR description to match the actual coverage.

- Add `ensureReserved` method to reserve top-level schema/enum names in NameRegistry - Extend `InlineSchemaKey` with nullable and defaultValue properties - Normalize and sort nested inline schema properties in `InlineSchemaKey` - Update `TypeRef` resolution logic for inline types - Add case-sensitive name registration test in `NameRegistryTest` - Pre-reserve additional keywords in `CodeGenerator` to avoid collisions

halotukozak and others added 4 commits March 23, 2026 17:19

Copilot AI review requested due to automatic review settings March 23, 2026 16:32

Copilot started reviewing on behalf of halotukozak March 23, 2026 16:33 View session

Copilot AI reviewed Mar 23, 2026

View reviewed changes

Merge branch 'master' into feat/12-unified-name-registry

79951e3

halotukozak mentioned this pull request Mar 24, 2026

feat: nested sealed hierarchies for oneOf/anyOf subtypes #28

Closed

5 tasks

fix: pass NameRegistry to ModelGenerator in IntegrationTest

b7e1d9b

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

halotukozak mentioned this pull request Mar 24, 2026

feat: nested sealed hierarchies for oneOf/anyOf subtypes #29

Open

5 tasks

halotukozak requested a review from Copilot March 24, 2026 13:26

halotukozak self-assigned this Mar 24, 2026

Copilot started reviewing on behalf of halotukozak March 24, 2026 13:26 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

halotukozak requested a review from Copilot March 25, 2026 12:40

Copilot started reviewing on behalf of halotukozak March 25, 2026 12:41 View session

fmt

cc2a63c

Copilot AI reviewed Mar 25, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: unified NameRegistry for all generated name deduplication#27

refactor: unified NameRegistry for all generated name deduplication#27
halotukozak wants to merge 10 commits intomasterfrom
feat/12-unified-name-registry

halotukozak commented Mar 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Mar 24, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-            spec.enums.forEach { reserve(it.name) }
+            spec.enums.forEach { reserve(it.name) }
+            // Reserve fixed generator outputs to avoid collisions with inline schema classes
+            reserve("SerializersModule")
+            reserve("UuidSerializer")


		fun generate(spec: ApiSpec): List<FileSpec> = generateWithResolvedSpec(spec).files

		fun generateWithResolvedSpec(spec: ApiSpec): GenerateResult {

-    fun generate(spec: ApiSpec): List<FileSpec> = generateWithResolvedSpec(spec).files
-    fun generateWithResolvedSpec(spec: ApiSpec): GenerateResult {
+    /**
+     * Ensures that the [nameRegistry] is aware of all top-level component schema and enum names
+     * before we start generating names for inline schemas. This prevents inline schemas from
+     * accidentally taking the same name as a component schema/enum and producing duplicate types,
+     * even when callers provide an empty [NameRegistry].
+     */
+    private fun prepopulateNameRegistry(spec: ApiSpec) {
+        // Reserve all schema names
+        spec.schemas.forEach { schema ->
+            nameRegistry.reserve(schema.name)
+        }
+        // Reserve all enum names
+        spec.enums.forEach { enumModel ->
+            nameRegistry.reserve(enumModel.name)
+        }
+    }
+    fun generate(spec: ApiSpec): List<FileSpec> = generateWithResolvedSpec(spec).files
+    fun generateWithResolvedSpec(spec: ApiSpec): GenerateResult {
+        prepopulateNameRegistry(spec)

Conversation

halotukozak commented Mar 23, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Mar 24, 2026 •

edited

Loading