Skip to content

[BUG] Normalize OAS 3.1 nullable map type array under NORMALIZE_31SPEC - #24519

Open
renechoi wants to merge 2 commits into
OpenAPITools:masterfrom
renechoi:fix/issue-24517-nullable-map-type-array
Open

[BUG] Normalize OAS 3.1 nullable map type array under NORMALIZE_31SPEC#24519
renechoi wants to merge 2 commits into
OpenAPITools:masterfrom
renechoi:fix/issue-24517-nullable-map-type-array

Conversation

@renechoi

@renechoi renechoi commented Jul 29, 2026

Copy link
Copy Markdown

Fixes #24517

Root cause

OpenAPINormalizer.normalizeSchema strips the OAS 3.1 "null" entry out of a type array for arrays (normalizeArraySchema -> processNormalize31Spec) and, since #24139, for schemas with properties. The additionalProperties (map) branch has no equivalent, so a map that declares its own nullability with a type array keeps both entries:

map:
  type: ["null", object]
  additionalProperties:
    $ref: "#/components/schemas/Child"

nullable is therefore never set, and ModelUtils.getType() returns the first entry of the type array, which is "null". That has two separate consequences:

  1. The property loses its nullability — on every nullable map, whatever the value schema is.
  2. If the value schema is a $ref, the property additionally loses that import and picks up an import of a Null model the generator never writes. That is what makes the output fail to compile.

The first revision of this description mentioned only the second one ("generators resolve the property to a model that is never written and lose the import of the value schema"). That was the incomplete half. @rony-remedio measured the ratio on a production 3.1.1 spec (486 generated models): of its six nullable maps, one was the $ref form and five were primitive-valued — silently non-nullable, compiling cleanly, no symptom at all. The quiet failure is the common one; the compile error is the visible minority.

This is also not specific to typescript-axios — the cause is in the shared normalizer, so other generators produce uncompilable output from the same spec (java shown below).

It is not a recent regression either. The fixture below generates identical models/ and docs/ on every release from 7.16.0 to 7.24.0 (typescript-axios, one jar per release). #24520 reaches the same never-generated Null model through the bare type: "null" path and does bisect to 7.17.0; this one does not, so the map branch has simply never had the handling.

Scope: not opt-in

NORMALIZE_31SPEC is not a rule consumers have to enable. DefaultGenerator sets it for every spec at 3.1.0 or above (DefaultGenerator.java:261-265), DefaultCodegen.getUseOpenapiNormalizer() returns true, and no generator overrides it. Measured rather than read off the source: generating the fixture below with 7.24.0 with and without --openapi-normalizer NORMALIZE_31SPEC=true gives byte-identical trees.

So this change reaches every 3.1 spec by default, and cannot affect a 3.0 one.

Fix

Extracted the existing null-in-type-array handling from the hasProperties branch into normalizeNullTypeArray31(Schema) and called it from the map branch as well.

It deliberately stays narrower than processNormalize31Spec: that method replaces a JsonSchema carrying no explicit type with an empty schema, which would discard properties / additionalProperties. That is the reason the hasProperties branch never called it either, and the rationale now lives on the extracted method.

Before / after

typescript-axios, withSeparateModelsAndApi=true, no normalizer flags (see above — the rule is on by default here).

models/parent-nullable-map.ts, the $ref form, which does not compile before:

// before
// @ts-ignore
import type { Null } from './null';        // ./null is never generated
export interface ParentNullableMap {
    'map'?: { [key: string]: Child; };     // Child is never imported
}

// after
// @ts-ignore
import type { Child } from './child';
export interface ParentNullableMap {
    'map'?: { [key: string]: Child; } | null;
}

models/parent-nullable-primitive-map.ts, the quiet form, which compiles before and after:

// before
// @ts-ignore
import type { Null } from './null';        // dangling, but suppressed by the @ts-ignore above it
export interface ParentNullablePrimitiveMap {
    'counts'?: { [key: string]: number; };     // nullability silently gone
}

// after
export interface ParentNullablePrimitiveMap {
    'counts'?: { [key: string]: number; } | null;
}

Type-checking the generated models with the command from the issue (tsc --noEmit --strict --skipLibCheck --target es2020 --module esnext --moduleResolution bundler models/*.ts): before, error TS2304: Cannot find name 'Child' (exit 2); after, clean (exit 0).

java (library=native), ParentNullableMap.java, from the same spec:

// before
import org.openapitools.client.model.ModelNull;               // never generated either
private ModelNull<String, Child> map = new HashMap<>();

// after
import org.openapitools.client.model.Child;
private JsonNullable<Map<String, Child>> map = JsonNullable.<Map<String, Child>>undefined();

The generated docs improve on the same specs as a side effect: docs/ParentNullableMap.md linked the property to (.md) (an empty target) and now links to (Child.md), and the primitive-valued one drops the empty link entirely.

Tests

New fixture modules/openapi-generator/src/test/resources/3_1/issue_24517.yaml: the minimal spec from the issue, its nullable-array control, and a primitive-valued nullable map for the quiet form.

  • OpenAPINormalizerTest.testIssue24517NullableMapWithRefValueUsingTypeArray — the map normalizes to a nullable object and keeps its $ref value schema; the nullable array is asserted alongside as the control that already worked.
  • TypeScriptAxiosClientCodegenTest.testNullableMapWithRefValueImportsReferencedModel — end to end: the generated model imports Child, the property is | null, and there is no import from ./null.
  • TypeScriptAxiosClientCodegenTest.testNullableMapWithPrimitiveValueKeepsNullability — end to end for the quiet form: 'counts'?: { [key: string]: number; } | null; and no dangling ./null import.
  • OpenAPINormalizerTest.testOpenAPINormalizer31SpecNullMapAdditionalProperties — extended. 3_1/issue_23945.yaml already declared its maps as type: [object, "null"], but only the value schemas were asserted, so the map-level half of the same bug was uncovered.

All four fail on master and pass with this change.

The last two are not redundant with each other. Replacing the fix with a $ref-only variant — if (((Schema) schema.getAdditionalProperties()).get$ref() != null) around the new call, which is what a fix aimed only at the missing import would look like — leaves testNullableMapWithRefValueImportsReferencedModel green and fails testNullableMapWithPrimitiveValueKeepsNullability and testOpenAPINormalizer31SpecNullMapAdditionalProperties. So at the codegen level the primitive-valued test is the only guard on the quiet half.

Samples

No sample changed. Of the 246 distinct inputSpec files referenced from bin/configs/**, none declares a schema carrying both a null-containing type array and an additionalProperties schema, so the change cannot alter existing output. Verified rather than assumed: regenerating the 21 configs that use a 3.1 spec plus the 15 typescript-axios configs (36 generators) leaves samples/ clean.

./mvnw -pl modules/openapi-generator test passes locally on JDK 21 (4608 tests, 0 failures, 7 skipped).

cc @nicokoenig (TypeScript / Axios)

PR checklist

  • Read the contribution guidelines.
  • Built the project and checked the samples (see above — no sample output changes).
  • Filed the PR against the correct branch: master.
  • Copied the technical committee to review the pull request.

Summary by cubic

Fix normalization for OAS 3.1 nullable map schemas (type: ["null", object]) under NORMALIZE_31SPEC. Maps are now correctly marked nullable and keep their value schema, so generated code compiles.

  • Bug Fixes
    • Added normalizeNullTypeArray31 and applied it to map schemas to move "null" from the type array to nullable: true, and set type: "object" when appropriate.
    • Keeps additionalProperties intact (including $ref value schemas).
    • Prevents generators like typescript-axios and java from resolving to fake Null/ModelNull models and dropping imports.
    • Tests: normalizer regression, end-to-end for $ref maps, plus a new typescript-axios test guarding primitive map values to keep | null. Minimal 3.1 fixture added; no sample changes.

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

Review in cubic

A map schema that declares its own nullability with an OAS 3.1 type array
(`type: ["null", object]` alongside `additionalProperties`) was never
normalized. normalizeSchema strips the `null` entry for arrays and for schemas
with properties, but the additionalProperties branch had no equivalent, so the
schema kept both types, `nullable` was never set, and ModelUtils.getType()
returned `null` first. Generators then resolved the property to a fictional
`Null` / `ModelNull` model and dropped the import of the value schema, emitting
output that does not compile.

Extract the existing null-in-type-array handling into normalizeNullTypeArray31
and call it from the map branch as well. It stays narrower than
processNormalize31Spec, which would replace a typeless JsonSchema with an empty
schema and discard additionalProperties.

@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

@rony-remedio

Copy link
Copy Markdown

Reporter of #24517 here. Confirming against a production 3.1.1 spec (490 component schemas, 486 generated models) rather than just the minimal repro, in case a second data point helps review.

The shape is what Swashbuckle emits for a nullable dictionary. Dictionary<string, T> on a nullable-oblivious C# type becomes exactly type: ["null", object] + additionalProperties: {$ref}, so this is reachable from an ordinary ASP.NET API rather than a hand-written spec. In that spec it occurs once, and it is the only hard compile error in the whole generated client.

Enabling NORMALIZE_31SPEC alone does not resolve it, which matches your diagnosis that the map branch has no equivalent handling. Generating from the unmodified spec with --openapi-normalizer NORMALIZE_31SPEC=true on v7.24.0:

models/<parent>.ts(21,34): error TS2304: Cannot find name '<ValueSchema>'
1 error

(names elided; the shape is identical to the ParentNullableMap / Child pair in the issue.)

The flag looks safe to adopt as a prerequisite for this fix. Since the fix only takes effect under NORMALIZE_31SPEC and that rule is not in the defaults, adopting it is a precondition for consumers — so I measured what turning it on costs on a real spec, with no other changes:

without the flag with NORMALIZE_31SPEC=true
models generated 486 486
properties keeping a | null union 2507 2507
tsc --strict errors 1 1

No drift in nullability across 2507 properties, so the rule is not disruptive at scale on 3.1 input.


Adjacent, and explicitly not blocking this PR — flagging only because it lives in the same code path and you may want it in the same pass or a separate issue:

A property whose schema is a bare {"type": "null"} (which Swashbuckle emits for an untyped/dynamic member) generates an import for the same never-written Null model. Because the generated // @ts-ignore sits directly above that import, the dangling import is suppressed and the property silently becomes any — it compiles, so no error surfaces, and it reads as a real type named Null. With NORMALIZE_31SPEC=true on v7.24.0 this happens on 5 model files in the spec above. Same never-written Null model as this issue, different trigger, and the failure mode is quieter: uncompilable output is at least visible.

Happy to test a build of this branch against that spec if a real-world verification is useful before merge.

@renechoi

Copy link
Copy Markdown
Author

Thanks — the Swashbuckle provenance is the useful part here. My repro is hand-written, so "an ordinary Dictionary<string, T> on a nullable-oblivious type emits exactly this" is a much better argument for the shape being worth normalizing than anything I could show.

On the offer to test a build: yes please, if it isn't much trouble. Branch is fix/issue-24517-nullable-map-type-array on my fork, built with the documented ./mvnw clean install -DskipTests -Dmaven.javadoc.skip=true. Two things I can't get from a 3-schema repro:

  1. tsc --strict errors go 1 → 0.
  2. Nothing else moves. diff -r the tree from 7.24.0 + NORMALIZE_31SPEC=true against the same tree from this branch — I'd expect exactly one parent model to differ. That's the one I actually care about; 486 models is a far better regression net for "surgical" than the samples I regenerated.

On the adjacent bare type: "null" property

Reproduced, and it's worse than "adjacent" — it's a regression with a clean boundary. Still not something I want to fold into this PR, for reasons below.

It's a 7.17.0 regression, in both generators. Same property (dyn: {type: "null"}), no normalizer rules enabled:

7.16.0 7.17.0 → 7.24.0
typescript-axios 'dyn'?: any | null;, no import 'dyn'?: Null; + import type { Null } from './null'
java, webclient JsonNullable<Object> dyn ModelNull dyn, no ModelNull.java

So it isn't typescript-specific, and TypeScript is the quiet one — Java fails to compile, TS doesn't.

NORMALIZE_31SPEC is incidental to it. On 7.24.0 the output is byte-identical with and without the flag (diff -r clean). It's also byte-identical between 7.24.0 and this branch, so this PR neither causes nor fixes it — which matches your read that it isn't blocking.

Root cause, and why it's a different fix from this one. normalizeSchema grew this guard in 7.17.0 (absent at v7.16.0, present at v7.17.0), currently OpenAPINormalizer.java:940:

if (ModelUtils.isNullTypeSchema(openAPI, schema)) {
    return schema;
}

A bare type: "null" satisfies it, so the schema is handed straight back regardless of which rules are on. #23967 worked around this for map values by intercepting in the parent's map branch before recursing — which is why additionalProperties: {type: "null"} correctly yields { [key: string]: any | null; } today while the property form does not. The properties branch has no equivalent interception. This PR is about null inside a type array on a schema that has properties/additionalProperties; that path never reaches line 940.

Worth noting #23945 already pinned the map-value form at "7.17+" — same regression, and #23967 fixed one of its two surfaces.

On the silent-any part, I checked rather than assumed what Null degrades to:

const p: ParentBareNull = { dyn: 12345 };
const q: ParentBareNull = { dyn: 'a string instead' };
const callIt = (v: ParentBareNull) => v.dyn!.whateverMethod().deeply.nested;

tsc --strict exits 0 on all of it, so the field is any wearing a type name.

Prior art: #23908 is the Java-side report, and #23961 proposes fixing it in AbstractJavaCodegen (open since June). Neither frames it at the normalizer level nor covers the TypeScript surface, and a per-generator fix leaves the other generators broken. I'll open it as its own issue with the bisect and the repro, and credit you for finding it — it wants #23967's shape of fix one level up. Happy to leave it to you instead if you'd rather file it.

@rony-remedio

Copy link
Copy Markdown

Built fix/issue-24517-nullable-map-type-array at ae6b3e12 with your documented command (in a maven:3-eclipse-temurin-17 container, since I have no local JDK) and ran both checks against the production spec — 490 component schemas, 486 generated models.

First, a correction to my previous numbers. I reported "2507 nullable properties, unchanged with and without NORMALIZE_31SPEC" as evidence the flag was non-disruptive. That count was produced by a regex that stops at the first ;, which in { [key: string]: number; } is inside the braces — so it never counted map-typed properties at all. It was blind to precisely the class this PR changes. Corrected figures below. The conclusion about the flag being safe still holds; the number I used to support it did not measure what I implied.

1. tsc --strict: 1 → 0

Confirmed. Same spec, same flags, only the generator differs.

2. Not one model — three. All three are improvements.

diff -r between 7.25.0-SNAPSHOT without the fix and this branch, both with NORMALIZE_31SPEC=true:

models/<parent-a>.ts   differ
models/<parent-b>.ts   differ
models/<parent-c>.ts   differ    <- the $ref one from the issue

api/ is byte-identical (0 files differ). The changes:

// <parent-c> — the case this PR is about
- import type { Null } from './null';
- 'devices'?: { [key: string]: <ValueSchema>; };
+ import type { <ValueSchema> } from './<value-schema>';
+ 'devices'?: { [key: string]: <ValueSchema>; } | null;

// <parent-a> and <parent-b> — 5 properties across the two, all primitive-valued maps
- import type { Null } from './null';
- 'someCounts'?: { [key: string]: number; };
+ 'someCounts'?: { [key: string]: number; } | null;

The fix is broader than the PR claims, in a good way

without fix with fix
nullable property declarations 2507 2513
of those, map-typed 0 6
nullable declarations removed 0
files importing the never-written Null 5 2

Before the fix no nullable map in the whole client kept its | null — not one of six. Only one of those six was a hard compile error (the $ref case); the other five are primitive-valued maps that type-checked cleanly while silently dropping nullability. So this PR fixes a visible compile failure and a silent correctness bug on the same code path, and the silent one is the more common shape in this spec by 5 to 1.

Two suggestions from that, both minor:

  • Worth saying so in the description. "Generators lose the import of the value schema" undersells it — nullability is lost on every nullable map, $ref or not.
  • testNullableMapWithRefValueImportsReferencedModel asserts only the $ref form. The normalizer-level test covers stringMap, but at the codegen level nothing pins the primitive-valued case, which is the silent half. A 'someMap'?: { [key: string]: number; } | null; assertion would catch a future regression that keeps imports working while dropping | null again.

On the 7.24.0 comparison you asked for

I ran that too, and the drift concern turned out to be nil: v7.24.0 vs this branch differs in the same three files, so for this spec nothing moved between 7.24.0 and current master here. I used openapitools/openapi-generator-cli:latest (sha256:4ea782db…, also 7.25.0-SNAPSHOT) as the no-fix baseline so the diff isolates the fix rather than mixing in release drift. Both comparisons agree.

Also worth noting: the bug reproduces on current master, not only on the 7.24.0 release.

The bare type: "null" regression

Please do file it — the bisect to 7.17.0, the OpenAPINormalizer.java:940 early return, and the #23967 / #23945 / #23908 / #23961 prior art are the substance, and none of it is mine. Two things from this spec you are welcome to use: it occurs on 4 properties there, and after this PR it is what the 2 remaining Null imports are. Your v.dyn!.whateverMethod().deeply.nested check matches what I saw — it compiles, so nothing surfaces it. Tag me and I will add the measurements.

The build is still on disk, so if you push changes I can re-run both checks against the same spec cheaply.

The existing typescript-axios test only covers the $ref form of the nullable
map, which is the form that fails to compile. The primitive-valued form was
uncovered at the codegen level even though it is the more common shape: it
compiles either way and only loses its `| null`.

Replacing the fix with a $ref-only variant keeps
testNullableMapWithRefValueImportsReferencedModel green, so that half needs its
own assertion.

Reported by rony-remedio while reviewing this PR against a production 3.1 spec.
@renechoi

Copy link
Copy Markdown
Author

Both checks against 486 real models are worth more than the samples run, thanks. Three things came out of it, one of them a correction back in your direction.

Description rewritten

You're right that "generators lose the import of the value schema" undersells it. It now separates the two consequences: nullability is dropped on every nullable map, and the missing import — hence the compile error — only happens when the value schema is a $ref. Your 5-to-1 ratio is in there, attributed to you, because it's the part that establishes which half matters more in practice. Also noted that the docs output improves on the same models: the property used to link to (.md).

The primitive-valued test

Added in b1f4c290, with a fixture schema to hang it on: testNullableMapWithPrimitiveValueKeepsNullability, asserting 'counts'?: { [key: string]: number; } | null; and no ./null import.

I wanted to know what it actually guards before claiming it guards anything, since the $ref test also asserts | null and would catch a plain nullability regression on its own. So I replaced the fix with a $ref-only variant — what a fix aimed at the missing import in the issue title would plausibly look like:

if (((Schema) schema.getAdditionalProperties()).get$ref() != null) {
    normalizeNullTypeArray31(schema);
}
test under the $ref-only fix
testNullableMapWithRefValueImportsReferencedModel passes
testNullableMapWithPrimitiveValueKeepsNullability fails
testOpenAPINormalizer31SpecNullMapAdditionalProperties fails

So at the codegen level it is the only guard on the quiet half, and the normalizer-level assertion is the only one below it. That's in the description now.

One correction going the other way

Since the fix only takes effect under NORMALIZE_31SPEC and that rule is not in the defaults, adopting it is a precondition for consumers

There's nothing to adopt. DefaultGenerator puts NORMALIZE_31SPEC=true into the normalizer config for every spec at 3.1.0 or above before constructing it (DefaultGenerator.java:261-265); DefaultCodegen.getUseOpenapiNormalizer() returns true and no generator overrides it. Measured rather than inferred from the source: on 7.24.0 the fixture generates byte-identically with and without --openapi-normalizer NORMALIZE_31SPEC=true, diff -r clean.

That is also why your with/without table showed zero drift across 2507 properties: assuming both runs went through the CLI, the rule was already on in the "without" column, so there was nothing for the comparison to vary. The practical consequence is the opposite of a precondition — this change lands on every 3.1 spec by default, which is the part a reviewer should weigh.

The bare type: "null" property

Filed as #24520 — the 7.17.0 bisect, the OpenAPINormalizer.java:940 early return, the #23967 / #23945 / #23908 / #23961 prior art, and the minimal spec. You're credited and tagged there; the two figures you offered (4 properties, and the 2 remaining Null imports after this PR) are quoted as yours since I can't measure them.

No need to re-run anything here — b1f4c290 is a test and a fixture schema, no generator behaviour changes, so your numbers still describe the current branch.

@rony-remedio

Copy link
Copy Markdown

You're right, and I've confirmed it independently rather than just taking the source reading — the claim was mine to check.

diff -r on the 486-model spec, 7.24.0, with and without --openapi-normalizer NORMALIZE_31SPEC=true, everything else identical: 0 files differ. And DefaultGenerator.java:261-265 is exactly as you describe — NORMALIZE_31SPEC=true goes into the config for any spec at 3.1.0 or above, before the normalizer is constructed.

My error was reading OpenAPINormalizer's own defaults block, seeing only SIMPLIFY_ONEOF_ANYOF, SIMPLIFY_BOOLEAN_ENUM, SIMPLIFY_ONEOF_ANYOF_ENUM and REFACTOR_ALLOF_WITH_PROPERTIES_ONLY, and concluding the rule was opt-in — without checking whether a caller injects it. It does.

Two things follow, and both cut against what I said:

My with/without table was vacuous. You spotted why: the rule was on in both columns, so there was nothing for the comparison to vary. "No drift across 2507 properties" measured nothing. Combined with the regex problem I already corrected, that table was wrong twice over — once in what it counted, once in what it was comparing. The remaining numbers in that comment (1 → 0 errors, 3 files, 2507 → 2513, 0 → 6 map-typed) come from generator-version comparisons, so they stand.

The consequence is the opposite of what I implied. I framed the flag as a precondition consumers must adopt. In fact this lands on every 3.1 spec that goes through DefaultGenerator, with no opt-in — which raises what a reviewer should weigh, not lowers it. On the spec I measured that default reach is 6 properties gaining | null and one compile error fixed, with nothing else moving; but "nothing else moves" is a claim about one spec, and it is now a claim about default behaviour rather than opt-in behaviour.

On b1f4c290: substituting a $ref-only fix to prove the primitive test actually fails without it is a better answer than the suggestion. A test that passes under the wrong fix would have been worse than no test.

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][typescript-axios] Missing import for a $ref under additionalProperties on a 3.1 nullable property

2 participants