Skip to content

fix: keep allOf-inherited "any type" maps from degrading to free-form object - #24547

Open
donald wants to merge 2 commits into
OpenAPITools:masterfrom
donald:fix/allof-inherited-freeform-map
Open

fix: keep allOf-inherited "any type" maps from degrading to free-form object#24547
donald wants to merge 2 commits into
OpenAPITools:masterfrom
donald:fix/allof-inherited-freeform-map

Conversation

@donald

@donald donald commented Aug 1, 2026

Copy link
Copy Markdown

Fixes #24546

What

A property whose additionalProperties schema declares no type is an "any type" map. When such a
property is inherited through allOf, the generated value type silently changed from "any" to
"free-form object" — so the same property produced two different data types in the same run:

Base:
  properties:
    style: { type: object, additionalProperties: { description: can be any type } }
Child:
  allOf: [{ $ref: '#/components/schemas/Base' }]
// before
Base.style:  { [key: string]: any; }      // declared here
Child.style: { [key: string]: object; }   // inherited via allOf

// after
Base.style:  { [key: string]: any; }
Child.style: { [key: string]: any; }

Why

The allOf composition branch in DefaultCodegen.fromModel calls mergeProperties(), which
deep-copies every inherited property schema via ModelUtils.cloneSchema(). cloneSchema delegates to
AnnotationsUtils.clone — a JSON round-trip — and swagger-core's deserializer materializes a sub-schema
that has no type as an ObjectSchema (type: object):

Schema inner = new Schema().description("can be any type");
Schema outer = new ObjectSchema().additionalProperties(inner);

// before: class = Schema,       type = null,   isFreeFormObject = false
Schema cloned = ModelUtils.cloneSchema(outer, false);
// after:  class = ObjectSchema, type = object, isFreeFormObject = true

DefaultCodegen.getPrimitiveType() then matches the ModelUtils.isFreeFormObject(...) branch and returns
"object", never reaching the ModelUtils.isAnyType(...) branch that returns "AnyType" (mapped to any
by the TypeScript generators).

This is generator-agnostic — typescript-fetch is just where it is most visible.

How

cloneSchema() already guards a custom top-level type across the round-trip, but does not cover
sub-schemas. This extends the same idea: after cloning, walk the schema tree and clear the type wherever
the original did not declare one (additionalProperties, items, properties, allOf/anyOf/oneOf,
not). An identity-based visited set guards against cycles.

Schemas that genuinely declare type: object are untouched — there is an explicit test for that so the fix
cannot over-reach.

Testing

  • ModelUtilsTest — 3 new cases: typeless additionalProperties, nested typeless schemas
    (properties / items), and a declared type: object that must stay an object.
  • TypeScriptFetchClientCodegenTest.testAllOfInheritedFreeFormMapStaysAny — end-to-end, asserts the
    inherited properties on Child carry exactly the same types as the ones declared on Base.
    Verified that this test fails without the fix.
  • Regenerated all 787 sample configs (./bin/generate-samples.sh ./bin/configs/*.yaml) and
    ./bin/utils/export_docs_generators.shno diff. The fix is inert on the entire existing sample
    corpus; only the buggy case changes.

Note on the full test run: mvn test on modules/openapi-generator reports one pre-existing failure,
OpenApiSchemaValidationsTest.testNullTypeInOas31_noWarning. It fails identically on unmodified
master (4aed9c1c635) — I verified by reverting the patch and re-running — and is unrelated to this
change, which only touches the non-3.1 branch of cloneSchema.

PR checklist

  • Read the contribution guidelines.
  • Run the following to build the project and update samples:
    ./mvnw clean package || exit
    ./bin/generate-samples.sh ./bin/configs/*.yaml || exit
    ./bin/utils/export_docs_generators.sh || exit
    
    Commit all changed files.
    (Both scripts were run; neither produced any change, so there is nothing to commit beyond the fix and its tests.)
  • File the PR against the correct branch: master
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

The change is in the language-agnostic core (ModelUtils), so it is not owned by one language committee.
Pinging the TypeScript technical committee since that is where the symptom shows up:
@TiFu @taxpon @sebastianhaas @kenisteward @Vrolijkx @macjohnny @topce @akehir @petejohansonxo @amakhrov @davidgamero @mkusaka @joscha

CC also @wing328 as this touches shared codegen behaviour.


Summary by cubic

Fixes a bug where “any type” maps inherited via allOf were generated as free-form objects, causing parent and child models to disagree. Inherited properties now keep the correct any value type across generators (notably typescript-fetch).

  • Bug Fixes
    • In ModelUtils.cloneSchema, after cloning, walk sub-schemas and clear type wherever the original had none (additionalProperties, items, properties, allOf/anyOf/oneOf, not) with a visited set keyed on the clone to handle reused schemas; genuine type: object stays intact.
    • Added ModelUtilsTest cases (typeless additionalProperties, nested typeless schemas, and reused typeless schema occurrences) and an end-to-end typescript-fetch test; regenerating samples produced no diffs.

Written for commit f6e331e. Summary will update on new commits.

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 4 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

@donald

donald commented Aug 1, 2026

Copy link
Copy Markdown
Author

Good catch — fixed in 7416148.

You are right on both points. The parser does reuse a single Schema instance across several places (the InlineModelResolver tests document exactly this for external types), and since the clone comes from a JSON round-trip each of those places gets its own object — so keying visited on the original fixed up only the first occurrence and silently skipped the rest. And because the clone is built by JSON (de)serialization, the original graph cannot contain cycles anyway, so the set was providing no cycle protection, only the false skips.

I keyed the set on the clone rather than dropping it: each clone node is distinct, so every occurrence now gets fixed, and a cheap guard remains.

Added ModelUtilsTest.testCloneKeepsEveryOccurrenceOfAReusedTypelessSchemaTypeless, which reuses one typeless Schema instance across two properties. On the previous version it fails with:

second kept type: object expected [null] but found [object]

Full test suite and all 787 sample configs re-run — still no diff.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 4 files

Re-trigger cubic

Kejia Liu added 2 commits August 3, 2026 21:05
… object

A property whose additionalProperties schema declares no type is an "any
type" map. When such a property is inherited through allOf, the generated
value type silently changed from "any" to "free-form object", so the same
property produced two different data types in the same run:

    Base.style:  { [key: string]: any; }      // declared here
    Child.style: { [key: string]: object; }   // inherited via allOf

The allOf composition branch in DefaultCodegen.fromModel calls
mergeProperties(), which deep-copies every inherited property schema via
ModelUtils.cloneSchema(). cloneSchema delegates to AnnotationsUtils.clone,
a JSON round-trip, and swagger-core's deserializer materializes a
sub-schema that has no type as an ObjectSchema (type: object):

    Schema inner = new Schema().description("can be any type");
    Schema outer = new ObjectSchema().additionalProperties(inner);
    // before: class = Schema,       type = null,   isFreeFormObject = false
    // after:  class = ObjectSchema, type = object, isFreeFormObject = true

getPrimitiveType() then matches the isFreeFormObject branch and returns
"object", never reaching the isAnyType branch that returns "AnyType".

cloneSchema already guards a custom top-level type across the round-trip
but does not cover sub-schemas. This extends the same idea: after cloning,
walk the schema tree and clear the type wherever the original did not
declare one (additionalProperties, items, properties, allOf/anyOf/oneOf,
not). Schemas that genuinely declare `type: object` are untouched.

Tests: three ModelUtilsTest cases (typeless additionalProperties, nested
typeless schemas, and a declared `type: object` that must stay an object)
plus an end-to-end TypeScriptFetchClientCodegenTest that asserts inherited
properties keep the same types as the ones declared on the parent.

Regenerating all 787 sample configs produces no diff.
Addresses review feedback on the visited set in restoreTypelessSubSchemas.

The parser reuses a single Schema instance across several places in a spec
(the InlineModelResolver tests document this for external types), while the
JSON round-trip inside AnnotationsUtils.clone gives each of those places its
own clone object. Keying the visited set on the original meant only the
first occurrence got fixed up; every later one silently kept type: object,
so the fix did not hold for the second location.

Keying on the clone fixes every occurrence. The set is not needed for cycle
protection either -- the clone is produced by JSON (de)serialization, so the
original graph cannot contain cycles in the first place -- but keying on the
clone keeps a cheap guard without the false skips.

New test testCloneKeepsEveryOccurrenceOfAReusedTypelessSchemaTypeless
reproduces the reported scenario; on the previous version it fails with
"second kept type: object expected [null] but found [object]".

Regenerating all 787 sample configs still produces no diff.
@donald
donald force-pushed the fix/allof-inherited-freeform-map branch from 7416148 to f6e331e Compare August 3, 2026 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] allOf-inherited "any type" additionalProperties map degrades to free-form object

1 participant