From c8838fb3a57e21e29ff1fffd355ba98a86046a27 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 11:35:00 +0000 Subject: [PATCH 1/4] fix(codegen): read newtype structs as the value they wrap Serde writes a newtype struct as the value it wraps, flattened or not. `Shape::Newtype` now says so in the two places that didn't: - OpenAPI described `struct UserId(Uuid)` as a one-element array (`prefixItems`), so a client generated from the spec sent `["..."]`. It is now the inner schema, as for a transparent struct. Unit structs, a newtype of `()` in the schema, are `null` instead of `[null]`. - `Layout` treated a flattened newtype as having unknown keys, so Python codegen failed and OpenAPI described the group with that array. It now contributes the keys of the type it wraps. The codegen coverage fixture flattens a required and an optional newtype, so CI replays serde's JSON through the generated Python models and checks their JSON schemas against it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VjkNsfHSd5osRevK95McpQ --- CHANGELOG.md | 3 + reflectapi-demo/openapi.json | 22 +- reflectapi-demo/src/tests/serde.rs | 29 ++ ...s__basic__reflectapi_struct_newtype-4.snap | 12 +- ..._basic__reflectapi_struct_unit_type-4.snap | 12 +- ...tapi_struct_with_additional_derives-4.snap | 10 +- ...amespace__generic_and_type_conflict-4.snap | 14 +- ...ests__serde__flatten_newtype_struct-2.snap | 86 +++++ ...ests__serde__flatten_newtype_struct-3.snap | 100 ++++++ ...ests__serde__flatten_newtype_struct-4.snap | 152 ++++++++ ...ests__serde__flatten_newtype_struct-5.snap | 231 ++++++++++++ ..._tests__serde__flatten_newtype_struct.snap | 333 ++++++++++++++++++ ...api_demo__tests__serde__unit_struct-4.snap | 12 +- ...mo__tests__serde__unit_tuple_struct-4.snap | 12 +- ...ctapi_demo__tests__write_openapi_spec.snap | 22 +- reflectapi-demo/tests/codegen_coverage.rs | 38 ++ reflectapi/src/codegen/openapi.rs | 5 + reflectapi/src/codegen/python.rs | 5 +- reflectapi/src/codegen/schema/layout.rs | 24 +- 19 files changed, 1018 insertions(+), 104 deletions(-) create mode 100644 reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__flatten_newtype_struct-2.snap create mode 100644 reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__flatten_newtype_struct-3.snap create mode 100644 reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__flatten_newtype_struct-4.snap create mode 100644 reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__flatten_newtype_struct-5.snap create mode 100644 reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__flatten_newtype_struct.snap diff --git a/CHANGELOG.md b/CHANGELOG.md index a15715db..240e1757 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ - A generated Python module with an omittable field of a unit type, `()` or a unit struct, imports again. The field was annotated `None | None`, which Python can't evaluate, so importing the module raised `TypeError`. It is now `None`. - A generated Rust client compiles when an optional field's type is a generated struct or enum without `Default`, such as a `#[serde(default)] memo: Memo` whose `Memo` implements `Default` only on the server. The field got `#[serde(default)]`, which needs `Default` on the generated type, and generated types derive it only when configured to. Such a field is now `Option`: `None` leaves the key out and the server applies its default. Fields whose generated type derives `Default`, through `base_derives` or the type's `additional_derives`, are unchanged. +- **OpenAPI describes a newtype struct as the value it wraps**, as serde writes it. `struct UserId(Uuid)` was a one-element array (`"prefixItems": [...]`), so a client generated from the spec sent `["..."]` where the server expects `"..."`; the demo's `pets.create` request body, `PetsCreateRequest(Pet)`, was `[Pet]` instead of `Pet`. Unit structs, which the schema describes as a newtype of `()`, are now `null` instead of `[null]`. Generated Rust, TypeScript and Python clients already treated newtypes this way and are unchanged. +- **Flattening a newtype struct** works in Python and OpenAPI. Serde writes `#[serde(flatten)] w: Wrapper`, where `struct Wrapper(Inner)`, as `Inner`'s keys next to the parent's, and `Option` as all of them or none. Python codegen failed with "cannot flatten 'Wrapper' into a model", and OpenAPI described the group with the one-element array above, which no object matches. +- A single-field tuple variant whose field has a serde rename, such as `A(#[serde(rename = "foo")] u32)`, is a newtype in OpenAPI variants and Python models, as serde writes it; it was described as an object with a `foo` key. ([#212](https://github.com/thepartly/reflectapi/pull/212)) ### Changed - Generated clients decode responses after the server removes a deprecated field. Every generator now treats a deprecated field of a response-only type as optional: Python reads it as `None` and types a non-`Option` field `T | None`, TypeScript marks it `?:`, the Rust client types it `Option`, unless it already is one, and OpenAPI leaves it out of `required`. Before, a client generated while the field was deprecated failed to decode every response once the server dropped it, although the client never read it. TypeScript and Rust code that reads such a field must now handle its absence. Types also used in requests keep deprecated fields required, because the server may still require them until it removes them, and so do types the client decodes by trying them in turn: untagged enums and untagged variants, the type behind a flattened `Option`, and every type these reach. There, an optional field could change what a payload decodes as. diff --git a/reflectapi-demo/openapi.json b/reflectapi-demo/openapi.json index e22b0e3f..d1dff2c8 100644 --- a/reflectapi-demo/openapi.json +++ b/reflectapi-demo/openapi.json @@ -55,7 +55,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/myapi.proto.UnauthorizedError" + "$ref": "#/components/schemas/std.tuple.Tuple0" } } } @@ -83,7 +83,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/myapi.proto.PetsCreateRequest" + "$ref": "#/components/schemas/myapi.model.input.Pet" } } }, @@ -292,7 +292,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/myapi.proto.UnauthorizedError" + "$ref": "#/components/schemas/std.tuple.Tuple0" } } } @@ -816,14 +816,6 @@ } ] }, - "myapi.proto.PetsCreateRequest": { - "type": "array", - "prefixItems": [ - { - "$ref": "#/components/schemas/myapi.model.input.Pet" - } - ] - }, "myapi.proto.PetsListError": { "oneOf": [ { @@ -998,14 +990,6 @@ } } }, - "myapi.proto.UnauthorizedError": { - "type": "array", - "prefixItems": [ - { - "$ref": "#/components/schemas/std.tuple.Tuple0" - } - ] - }, "myapi.proto.ValidationA": { "type": "object", "title": "myapi.proto.ValidationA", diff --git a/reflectapi-demo/src/tests/serde.rs b/reflectapi-demo/src/tests/serde.rs index 8536a0cd..1aa79986 100644 --- a/reflectapi-demo/src/tests/serde.rs +++ b/reflectapi-demo/src/tests/serde.rs @@ -1832,3 +1832,32 @@ struct TestWrapperWithNestedTypevarArg { fn test_generic_flatten_typevar_nested_in_generic_arg() { assert_snapshot!(TestWrapperWithNestedTypevarArg); } + +#[test] +fn test_flatten_newtype_struct() { + #[derive(reflectapi::Input, reflectapi::Output, serde::Deserialize, serde::Serialize)] + struct Inner { + a: u32, + b: String, + } + #[derive(reflectapi::Input, reflectapi::Output, serde::Deserialize, serde::Serialize)] + struct Wrapper(Inner); + #[derive(reflectapi::Input, reflectapi::Output, serde::Deserialize, serde::Serialize)] + struct OptionalInner { + c: u32, + } + #[derive(reflectapi::Input, reflectapi::Output, serde::Deserialize, serde::Serialize)] + struct OptionalWrapper(OptionalInner); + // Serde writes a flattened newtype struct as the keys of the struct it + // wraps: `{"x": 1, "a": 2, "b": "s"}`, with `"c"` only when `o` is `Some`. + #[derive(reflectapi::Input, reflectapi::Output, serde::Deserialize, serde::Serialize)] + struct TestFlattenNewtypeStruct { + x: u32, + #[serde(flatten)] + w: Wrapper, + #[serde(flatten)] + o: Option, + } + + assert_snapshot!(TestFlattenNewtypeStruct); +} diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_struct_newtype-4.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_struct_newtype-4.snap index 9acbe592..f56ed00f 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_struct_newtype-4.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_struct_newtype-4.snap @@ -18,7 +18,7 @@ expression: "reflectapi::codegen::openapi::Spec::from(&schema)" "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/reflectapi_demo.tests.basic.TestStructNewtype" + "$ref": "#/components/schemas/std.string.String" } } }, @@ -30,7 +30,7 @@ expression: "reflectapi::codegen::openapi::Spec::from(&schema)" "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/reflectapi_demo.tests.basic.TestStructNewtype" + "$ref": "#/components/schemas/std.string.String" } } } @@ -41,14 +41,6 @@ expression: "reflectapi::codegen::openapi::Spec::from(&schema)" }, "components": { "schemas": { - "reflectapi_demo.tests.basic.TestStructNewtype": { - "type": "array", - "prefixItems": [ - { - "$ref": "#/components/schemas/std.string.String" - } - ] - }, "std.string.String": { "description": "UTF-8 encoded string", "type": "string" diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_struct_unit_type-4.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_struct_unit_type-4.snap index 2d5ecdc3..e7bdb99e 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_struct_unit_type-4.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_struct_unit_type-4.snap @@ -18,7 +18,7 @@ expression: "reflectapi::codegen::openapi::Spec::from(&schema)" "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/reflectapi_demo.tests.basic.TestStructUnitType" + "$ref": "#/components/schemas/std.tuple.Tuple0" } } }, @@ -30,7 +30,7 @@ expression: "reflectapi::codegen::openapi::Spec::from(&schema)" "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/reflectapi_demo.tests.basic.TestStructUnitType" + "$ref": "#/components/schemas/std.tuple.Tuple0" } } } @@ -41,14 +41,6 @@ expression: "reflectapi::codegen::openapi::Spec::from(&schema)" }, "components": { "schemas": { - "reflectapi_demo.tests.basic.TestStructUnitType": { - "type": "array", - "prefixItems": [ - { - "$ref": "#/components/schemas/std.tuple.Tuple0" - } - ] - }, "std.tuple.Tuple0": { "description": "Unit type", "type": "null" diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_struct_with_additional_derives-4.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_struct_with_additional_derives-4.snap index 86d31450..1a1eb6bb 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_struct_with_additional_derives-4.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_struct_with_additional_derives-4.snap @@ -55,7 +55,7 @@ snapshot_kind: text "description": "Value set type", "type": "array", "items": { - "$ref": "#/components/schemas/reflectapi_demo.tests.basic.U" + "$ref": "#/components/schemas/std.tuple.Tuple0" }, "uniqueItems": true }, @@ -77,14 +77,6 @@ snapshot_kind: text } } }, - "reflectapi_demo.tests.basic.U": { - "type": "array", - "prefixItems": [ - { - "$ref": "#/components/schemas/std.tuple.Tuple0" - } - ] - }, "reflectapi_demo.tests.basic.X": { "type": "object", "title": "reflectapi_demo.tests.basic.X", diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__namespace__generic_and_type_conflict-4.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__namespace__generic_and_type_conflict-4.snap index cbca623e..bac79b58 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__namespace__generic_and_type_conflict-4.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__namespace__generic_and_type_conflict-4.snap @@ -26,12 +26,7 @@ snapshot_kind: text ], "properties": { "t": { - "type": "array", - "prefixItems": [ - { - "$ref": "#/components/schemas/std.tuple.Tuple0" - } - ] + "$ref": "#/components/schemas/std.tuple.Tuple0" } } } @@ -52,12 +47,7 @@ snapshot_kind: text ], "properties": { "t": { - "type": "array", - "prefixItems": [ - { - "$ref": "#/components/schemas/std.tuple.Tuple0" - } - ] + "$ref": "#/components/schemas/std.tuple.Tuple0" } } } diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__flatten_newtype_struct-2.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__flatten_newtype_struct-2.snap new file mode 100644 index 00000000..53dde0d1 --- /dev/null +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__flatten_newtype_struct-2.snap @@ -0,0 +1,86 @@ +--- +source: reflectapi-demo/src/tests/serde.rs +expression: "super :: into_typescript_code :: < TestFlattenNewtypeStruct > ()" +--- +// DO NOT MODIFY THIS FILE MANUALLY +// This file was generated by reflectapi-cli +// +// Schema name: + +export function client(base: string | Client): __definition.Interface { + return __implementation.__client(base); +} + +export namespace __definition { + export interface Interface { + inout_test: ( + input: reflectapi_demo.tests.serde.input.TestFlattenNewtypeStruct, + headers: {}, + options?: RequestOptions, + ) => AsyncResult< + reflectapi_demo.tests.serde.output.TestFlattenNewtypeStruct, + {} + >; + } +} +export namespace reflectapi { + /** + * Struct object with no fields + */ + export interface Empty {} + + /** + * Error object which is expected to be never returned + */ + export interface Infallible {} +} + +export namespace reflectapi_demo { + export namespace tests { + export namespace serde { + export interface Inner { + a: number /* u32 */; + b: string; + } + + export interface OptionalInner { + c: number /* u32 */; + } + + export type OptionalWrapper = reflectapi_demo.tests.serde.OptionalInner; + + export type Wrapper = reflectapi_demo.tests.serde.Inner; + + export namespace input { + export type TestFlattenNewtypeStruct = { + x: number /* u32 */; + } & NullToEmptyObject & + Partial; + } + + export namespace output { + export type TestFlattenNewtypeStruct = { + x: number /* u32 */; + } & NullToEmptyObject & + NullToEmptyObject; + } + } + } +} + +namespace __implementation { + + function inout_test(client: Client) { + return ( + input: reflectapi_demo.tests.serde.input.TestFlattenNewtypeStruct, + headers: {}, + options?: RequestOptions, + ) => + __request< + reflectapi_demo.tests.serde.input.TestFlattenNewtypeStruct, + {}, + reflectapi_demo.tests.serde.output.TestFlattenNewtypeStruct, + {} + >(client, "/inout_test", input, headers, options); + } +} diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__flatten_newtype_struct-3.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__flatten_newtype_struct-3.snap new file mode 100644 index 00000000..6909b6ec --- /dev/null +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__flatten_newtype_struct-3.snap @@ -0,0 +1,100 @@ +--- +source: reflectapi-demo/src/tests/serde.rs +expression: "super :: into_rust_code :: < TestFlattenNewtypeStruct > ()" +--- +// DO NOT MODIFY THIS FILE MANUALLY +// This file was generated by reflectapi-cli +// +// Schema name: + +#![allow(non_camel_case_types)] +#![allow(dead_code)] + +pub use interface::Interface; +pub use reflectapi::rt::*; + +pub mod interface { + + #[derive(Debug)] + pub struct Interface { + client: C, + } + + impl Interface { + pub fn new(client: C) -> Self { + Self { client } + } + pub async fn inout_test( + &self, + input: super::types::reflectapi_demo::tests::serde::input::TestFlattenNewtypeStruct, + headers: reflectapi::Empty, + ) -> Result< + super::types::reflectapi_demo::tests::serde::output::TestFlattenNewtypeStruct, + reflectapi::rt::Error, + > { + reflectapi::rt::__request_impl(&self.client, "/inout_test", input, headers).await + } + } + + #[cfg(feature = "reqwest")] + impl Interface> { + /// Convenience: build the client backed by a bare `reqwest::Client` + /// and the given base URL. Hides the + /// [`reflectapi::rt::ReqwestClient`] adapter so callers don't need + /// to name it. + pub fn try_new( + client: reqwest::Client, + base_url: reflectapi::rt::Url, + ) -> std::result::Result { + Ok(Self::new(reflectapi::rt::ReqwestClient::try_new( + client, base_url, + )?)) + } + } +} +pub mod types { + pub mod reflectapi_demo { + pub mod tests { + pub mod serde { + + #[derive(Debug, serde::Deserialize, serde::Serialize)] + pub struct Inner { + pub a: u32, + pub b: std::string::String, + } + + #[derive(Debug, serde::Deserialize, serde::Serialize)] + pub struct OptionalInner { + pub c: u32, + } + + pub type OptionalWrapper = + super::super::super::reflectapi_demo::tests::serde::OptionalInner; + + pub type Wrapper = super::super::super::reflectapi_demo::tests::serde::Inner; + pub mod input { + + #[derive(Debug, serde::Serialize)] + pub struct TestFlattenNewtypeStruct { + pub x: u32, + #[serde(flatten)] + pub w: super::super::super::super::reflectapi_demo::tests::serde::Wrapper, + #[serde(default = "Default::default", skip_serializing_if = "std::option::Option::is_none", flatten)] + pub o: std::option::Option, +} + } + pub mod output { + + #[derive(Debug, serde::Deserialize)] + pub struct TestFlattenNewtypeStruct { + pub x: u32, + #[serde(flatten)] + pub w: super::super::super::super::reflectapi_demo::tests::serde::Wrapper, + #[serde(flatten)] + pub o: std::option::Option, +} + } + } + } + } +} diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__flatten_newtype_struct-4.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__flatten_newtype_struct-4.snap new file mode 100644 index 00000000..d7824629 --- /dev/null +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__flatten_newtype_struct-4.snap @@ -0,0 +1,152 @@ +--- +source: reflectapi-demo/src/tests/serde.rs +expression: "reflectapi::codegen::openapi::Spec::from(&schema)" +--- +{ + "openapi": "3.1.0", + "info": { + "title": "", + "description": "", + "version": "1.0.0" + }, + "paths": { + "/inout_test": { + "description": "", + "post": { + "operationId": "inout_test", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/reflectapi_demo.tests.serde.input.TestFlattenNewtypeStruct" + } + } + }, + "required": true + }, + "responses": { + "200": { + "description": "200 OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/reflectapi_demo.tests.serde.output.TestFlattenNewtypeStruct" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "reflectapi_demo.tests.serde.Inner": { + "type": "object", + "title": "reflectapi_demo.tests.serde.Inner", + "required": [ + "a", + "b" + ], + "properties": { + "a": { + "$ref": "#/components/schemas/u32" + }, + "b": { + "$ref": "#/components/schemas/std.string.String" + } + } + }, + "reflectapi_demo.tests.serde.OptionalInner": { + "type": "object", + "title": "reflectapi_demo.tests.serde.OptionalInner", + "required": [ + "c" + ], + "properties": { + "c": { + "$ref": "#/components/schemas/u32" + } + } + }, + "reflectapi_demo.tests.serde.input.TestFlattenNewtypeStruct": { + "allOf": [ + { + "$ref": "#/components/schemas/reflectapi_demo.tests.serde.Inner" + }, + { + "oneOf": [ + { + "$ref": "#/components/schemas/reflectapi_demo.tests.serde.OptionalInner" + }, + { + "not": { + "type": "object", + "required": [ + "c" + ], + "properties": {} + } + } + ] + }, + { + "type": "object", + "title": "reflectapi_demo.tests.serde.input.TestFlattenNewtypeStruct", + "required": [ + "x" + ], + "properties": { + "x": { + "$ref": "#/components/schemas/u32" + } + } + } + ] + }, + "reflectapi_demo.tests.serde.output.TestFlattenNewtypeStruct": { + "allOf": [ + { + "$ref": "#/components/schemas/reflectapi_demo.tests.serde.Inner" + }, + { + "oneOf": [ + { + "$ref": "#/components/schemas/reflectapi_demo.tests.serde.OptionalInner" + }, + { + "not": { + "type": "object", + "required": [ + "c" + ], + "properties": {} + } + } + ] + }, + { + "type": "object", + "title": "reflectapi_demo.tests.serde.output.TestFlattenNewtypeStruct", + "required": [ + "x" + ], + "properties": { + "x": { + "$ref": "#/components/schemas/u32" + } + } + } + ] + }, + "std.string.String": { + "description": "UTF-8 encoded string", + "type": "string" + }, + "u32": { + "description": "32-bit unsigned integer", + "type": "integer" + } + } + } +} diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__flatten_newtype_struct-5.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__flatten_newtype_struct-5.snap new file mode 100644 index 00000000..cce76858 --- /dev/null +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__flatten_newtype_struct-5.snap @@ -0,0 +1,231 @@ +--- +source: reflectapi-demo/src/tests/serde.rs +expression: "super :: into_python_code :: < TestFlattenNewtypeStruct > ()" +--- +""" +DO NOT MODIFY THIS FILE MANUALLY +This file was generated by reflectapi-cli + +Schema name: +""" + +from __future__ import annotations + + +# Standard library imports +from typing import Annotated, Any, Generic, Optional, TypeVar, Union + +# Third-party imports +from pydantic import BaseModel, ConfigDict, Field, field_validator + +# Runtime imports +from reflectapi_runtime import AsyncClientBase, ClientBase, ApiResponse +from reflectapi_runtime import ReflectapiEmpty +from reflectapi_runtime import ReflectapiInfallible +from reflectapi_runtime import ReflectapiPartialModel + + +class ReflectapiDemoTestsSerdeInner(BaseModel): + model_config = ConfigDict( + extra="ignore", populate_by_name=True, protected_namespaces=(), defer_build=True + ) + + a: int + b: str + + +class ReflectapiDemoTestsSerdeOptionalInner(BaseModel): + model_config = ConfigDict( + extra="ignore", populate_by_name=True, protected_namespaces=(), defer_build=True + ) + + c: int + + +class ReflectapiDemoTestsSerdeInputTestFlattenNewtypeStruct(ReflectapiPartialModel): + model_config = ConfigDict( + extra="ignore", + populate_by_name=True, + validate_assignment=True, + protected_namespaces=(), + defer_build=True, + ) + + x: int + a: int + b: str + c: int | None = None + + @field_validator("c", mode="before") + @classmethod + def _reflectapi_reject_null(cls, value): + if value is None: + raise ValueError("Field may be omitted but cannot be null") + return value + + +class ReflectapiDemoTestsSerdeOutputTestFlattenNewtypeStruct(ReflectapiPartialModel): + model_config = ConfigDict( + extra="ignore", + populate_by_name=True, + validate_assignment=True, + protected_namespaces=(), + defer_build=True, + ) + + x: int + a: int + b: str + c: int | None = None + + @field_validator("c", mode="before") + @classmethod + def _reflectapi_reject_null(cls, value): + if value is None: + raise ValueError("Field may be omitted but cannot be null") + return value + + +# Namespace classes for dotted access to types +class reflectapi_demo: + """Namespace for reflectapi_demo types.""" + + class tests: + """Namespace for tests types.""" + + class serde: + """Namespace for serde types.""" + + Inner = ReflectapiDemoTestsSerdeInner + OptionalInner = ReflectapiDemoTestsSerdeOptionalInner + + class input: + """Namespace for input types.""" + + TestFlattenNewtypeStruct = ( + ReflectapiDemoTestsSerdeInputTestFlattenNewtypeStruct + ) + + class output: + """Namespace for output types.""" + + TestFlattenNewtypeStruct = ( + ReflectapiDemoTestsSerdeOutputTestFlattenNewtypeStruct + ) + + +class AsyncInoutClient: + """Async client for inout operations.""" + + def __init__(self, client: AsyncClientBase) -> None: + self._client = client + + async def test( + self, + data: Optional[ + reflectapi_demo.tests.serde.input.TestFlattenNewtypeStruct + ] = None, + ) -> ApiResponse[reflectapi_demo.tests.serde.output.TestFlattenNewtypeStruct]: + """ + + Args: + data: Request data for the test operation. + + Returns: + ApiResponse[reflectapi_demo.tests.serde.output.TestFlattenNewtypeStruct]: Response containing reflectapi_demo.tests.serde.output.TestFlattenNewtypeStruct data + """ + path = "/inout_test" + + params: dict[str, Any] = {} + return await self._client._make_request( + path, + params=params if params else None, + json_model=data, + response_model=reflectapi_demo.tests.serde.output.TestFlattenNewtypeStruct, + ) + + +class AsyncClient(AsyncClientBase): + """Async client for the API.""" + + def __init__( + self, + base_url: str, + **kwargs: Any, + ) -> None: + super().__init__(base_url, **kwargs) + + self.inout: AsyncInoutClient = AsyncInoutClient(self) + + +class InoutClient: + """Synchronous client for inout operations.""" + + def __init__(self, client: ClientBase) -> None: + self._client = client + + def test( + self, + data: Optional[ + reflectapi_demo.tests.serde.input.TestFlattenNewtypeStruct + ] = None, + ) -> ApiResponse[reflectapi_demo.tests.serde.output.TestFlattenNewtypeStruct]: + """ + + Args: + data: Request data for the test operation. + + Returns: + ApiResponse[reflectapi_demo.tests.serde.output.TestFlattenNewtypeStruct]: Response containing reflectapi_demo.tests.serde.output.TestFlattenNewtypeStruct data + """ + path = "/inout_test" + + params: dict[str, Any] = {} + return self._client._make_request( + path, + params=params if params else None, + json_model=data, + response_model=reflectapi_demo.tests.serde.output.TestFlattenNewtypeStruct, + ) + + +class Client(ClientBase): + """Synchronous client for the API.""" + + def __init__( + self, + base_url: str, + **kwargs: Any, + ) -> None: + super().__init__(base_url, **kwargs) + + self.inout: InoutClient = InoutClient(self) + + +# External type definitions +StdNumNonZeroU32 = Annotated[int, "Rust NonZero u32 type"] +StdNumNonZeroU64 = Annotated[int, "Rust NonZero u64 type"] +StdNumNonZeroI32 = Annotated[int, "Rust NonZero i32 type"] +StdNumNonZeroI64 = Annotated[int, "Rust NonZero i64 type"] + +# Rebuild models to resolve forward references +_rebuild_errors: list[str] = [] +for _model in [ + ReflectapiDemoTestsSerdeInner, + ReflectapiDemoTestsSerdeOptionalInner, + ReflectapiDemoTestsSerdeInputTestFlattenNewtypeStruct, + ReflectapiDemoTestsSerdeOutputTestFlattenNewtypeStruct, +]: + if not hasattr(_model, "model_rebuild"): + continue + try: + _model.model_rebuild() + except Exception as _exc: + _rebuild_errors.append(f" - {_model.__name__}: {type(_exc).__name__}: {_exc}") +if _rebuild_errors: + raise RuntimeError( + "reflectapi: failed to rebuild " + + str(len(_rebuild_errors)) + + " generated model(s). This usually means the codegen emitted an annotation pointing at a symbol that was never defined (a dangling type reference). Fix the codegen rather than catching this error.\n" + + "\n".join(_rebuild_errors) + ) diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__flatten_newtype_struct.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__flatten_newtype_struct.snap new file mode 100644 index 00000000..729c679c --- /dev/null +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__flatten_newtype_struct.snap @@ -0,0 +1,333 @@ +--- +source: reflectapi-demo/src/tests/serde.rs +expression: schema +--- +{ + "name": "", + "functions": [ + { + "name": "inout_test", + "path": "", + "input_type": { + "name": "reflectapi_demo::tests::serde::input::TestFlattenNewtypeStruct" + }, + "output_kind": "complete", + "output_type": { + "name": "reflectapi_demo::tests::serde::output::TestFlattenNewtypeStruct" + }, + "serialization": [ + "json", + "msgpack" + ] + } + ], + "input_types": { + "types": [ + { + "kind": "struct", + "name": "reflectapi::Empty", + "description": "Struct object with no fields", + "fields": "none" + }, + { + "kind": "struct", + "name": "reflectapi_demo::tests::serde::Inner", + "fields": { + "named": [ + { + "name": "a", + "type": { + "name": "u32" + }, + "required": true + }, + { + "name": "b", + "type": { + "name": "std::string::String" + }, + "required": true + } + ] + } + }, + { + "kind": "struct", + "name": "reflectapi_demo::tests::serde::OptionalInner", + "fields": { + "named": [ + { + "name": "c", + "type": { + "name": "u32" + }, + "required": true + } + ] + } + }, + { + "kind": "struct", + "name": "reflectapi_demo::tests::serde::OptionalWrapper", + "fields": { + "unnamed": [ + { + "name": "0", + "type": { + "name": "reflectapi_demo::tests::serde::OptionalInner" + }, + "required": true + } + ] + } + }, + { + "kind": "struct", + "name": "reflectapi_demo::tests::serde::input::TestFlattenNewtypeStruct", + "fields": { + "named": [ + { + "name": "x", + "type": { + "name": "u32" + }, + "required": true + }, + { + "name": "w", + "type": { + "name": "reflectapi_demo::tests::serde::Wrapper" + }, + "required": true, + "flattened": true + }, + { + "name": "o", + "type": { + "name": "std::option::Option", + "arguments": [ + { + "name": "reflectapi_demo::tests::serde::OptionalWrapper" + } + ] + }, + "flattened": true + } + ] + } + }, + { + "kind": "struct", + "name": "reflectapi_demo::tests::serde::Wrapper", + "fields": { + "unnamed": [ + { + "name": "0", + "type": { + "name": "reflectapi_demo::tests::serde::Inner" + }, + "required": true + } + ] + } + }, + { + "kind": "enum", + "name": "std::option::Option", + "description": "Optional nullable type", + "parameters": [ + { + "name": "T" + } + ], + "representation": "none", + "variants": [ + { + "name": "None", + "description": "The value is not provided, i.e. null", + "fields": "none" + }, + { + "name": "Some", + "description": "The value is provided and set to some value", + "fields": { + "unnamed": [ + { + "name": "0", + "type": { + "name": "T" + } + } + ] + } + } + ] + }, + { + "kind": "primitive", + "name": "std::string::String", + "description": "UTF-8 encoded string" + }, + { + "kind": "primitive", + "name": "u32", + "description": "32-bit unsigned integer" + } + ] + }, + "output_types": { + "types": [ + { + "kind": "struct", + "name": "reflectapi::Infallible", + "description": "Error object which is expected to be never returned", + "fields": "none" + }, + { + "kind": "struct", + "name": "reflectapi_demo::tests::serde::Inner", + "fields": { + "named": [ + { + "name": "a", + "type": { + "name": "u32" + }, + "required": true + }, + { + "name": "b", + "type": { + "name": "std::string::String" + }, + "required": true + } + ] + } + }, + { + "kind": "struct", + "name": "reflectapi_demo::tests::serde::OptionalInner", + "fields": { + "named": [ + { + "name": "c", + "type": { + "name": "u32" + }, + "required": true + } + ] + } + }, + { + "kind": "struct", + "name": "reflectapi_demo::tests::serde::OptionalWrapper", + "fields": { + "unnamed": [ + { + "name": "0", + "type": { + "name": "reflectapi_demo::tests::serde::OptionalInner" + }, + "required": true + } + ] + } + }, + { + "kind": "struct", + "name": "reflectapi_demo::tests::serde::output::TestFlattenNewtypeStruct", + "fields": { + "named": [ + { + "name": "x", + "type": { + "name": "u32" + }, + "required": true + }, + { + "name": "w", + "type": { + "name": "reflectapi_demo::tests::serde::Wrapper" + }, + "required": true, + "flattened": true + }, + { + "name": "o", + "type": { + "name": "std::option::Option", + "arguments": [ + { + "name": "reflectapi_demo::tests::serde::OptionalWrapper" + } + ] + }, + "required": true, + "flattened": true + } + ] + } + }, + { + "kind": "struct", + "name": "reflectapi_demo::tests::serde::Wrapper", + "fields": { + "unnamed": [ + { + "name": "0", + "type": { + "name": "reflectapi_demo::tests::serde::Inner" + }, + "required": true + } + ] + } + }, + { + "kind": "enum", + "name": "std::option::Option", + "description": "Optional nullable type", + "parameters": [ + { + "name": "T" + } + ], + "representation": "none", + "variants": [ + { + "name": "None", + "description": "The value is not provided, i.e. null", + "fields": "none" + }, + { + "name": "Some", + "description": "The value is provided and set to some value", + "fields": { + "unnamed": [ + { + "name": "0", + "type": { + "name": "T" + } + } + ] + } + } + ] + }, + { + "kind": "primitive", + "name": "std::string::String", + "description": "UTF-8 encoded string" + }, + { + "kind": "primitive", + "name": "u32", + "description": "32-bit unsigned integer" + } + ] + } +} diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__unit_struct-4.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__unit_struct-4.snap index 0d777024..904cb7d3 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__unit_struct-4.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__unit_struct-4.snap @@ -18,7 +18,7 @@ expression: "reflectapi::codegen::openapi::Spec::from(&schema)" "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/reflectapi_demo.tests.serde.TestUnitStruct" + "$ref": "#/components/schemas/std.tuple.Tuple0" } } }, @@ -30,7 +30,7 @@ expression: "reflectapi::codegen::openapi::Spec::from(&schema)" "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/reflectapi_demo.tests.serde.TestUnitStruct" + "$ref": "#/components/schemas/std.tuple.Tuple0" } } } @@ -41,14 +41,6 @@ expression: "reflectapi::codegen::openapi::Spec::from(&schema)" }, "components": { "schemas": { - "reflectapi_demo.tests.serde.TestUnitStruct": { - "type": "array", - "prefixItems": [ - { - "$ref": "#/components/schemas/std.tuple.Tuple0" - } - ] - }, "std.tuple.Tuple0": { "description": "Unit type", "type": "null" diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__unit_tuple_struct-4.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__unit_tuple_struct-4.snap index ce2135e6..904cb7d3 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__unit_tuple_struct-4.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__unit_tuple_struct-4.snap @@ -18,7 +18,7 @@ expression: "reflectapi::codegen::openapi::Spec::from(&schema)" "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/reflectapi_demo.tests.serde.TestUnitTupleStruct" + "$ref": "#/components/schemas/std.tuple.Tuple0" } } }, @@ -30,7 +30,7 @@ expression: "reflectapi::codegen::openapi::Spec::from(&schema)" "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/reflectapi_demo.tests.serde.TestUnitTupleStruct" + "$ref": "#/components/schemas/std.tuple.Tuple0" } } } @@ -41,14 +41,6 @@ expression: "reflectapi::codegen::openapi::Spec::from(&schema)" }, "components": { "schemas": { - "reflectapi_demo.tests.serde.TestUnitTupleStruct": { - "type": "array", - "prefixItems": [ - { - "$ref": "#/components/schemas/std.tuple.Tuple0" - } - ] - }, "std.tuple.Tuple0": { "description": "Unit type", "type": "null" diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__write_openapi_spec.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__write_openapi_spec.snap index 214a121a..e24d964a 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__write_openapi_spec.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__write_openapi_spec.snap @@ -59,7 +59,7 @@ expression: s "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/myapi.proto.UnauthorizedError" + "$ref": "#/components/schemas/std.tuple.Tuple0" } } } @@ -87,7 +87,7 @@ expression: s "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/myapi.proto.PetsCreateRequest" + "$ref": "#/components/schemas/myapi.model.input.Pet" } } }, @@ -296,7 +296,7 @@ expression: s "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/myapi.proto.UnauthorizedError" + "$ref": "#/components/schemas/std.tuple.Tuple0" } } } @@ -820,14 +820,6 @@ expression: s } ] }, - "myapi.proto.PetsCreateRequest": { - "type": "array", - "prefixItems": [ - { - "$ref": "#/components/schemas/myapi.model.input.Pet" - } - ] - }, "myapi.proto.PetsListError": { "oneOf": [ { @@ -1002,14 +994,6 @@ expression: s } } }, - "myapi.proto.UnauthorizedError": { - "type": "array", - "prefixItems": [ - { - "$ref": "#/components/schemas/std.tuple.Tuple0" - } - ] - }, "myapi.proto.ValidationA": { "type": "object", "title": "myapi.proto.ValidationA", diff --git a/reflectapi-demo/tests/codegen_coverage.rs b/reflectapi-demo/tests/codegen_coverage.rs index 7b1c4869..9e3b6fa9 100644 --- a/reflectapi-demo/tests/codegen_coverage.rs +++ b/reflectapi-demo/tests/codegen_coverage.rs @@ -574,6 +574,10 @@ mod wire { pub one_or_many: OneOrMany, #[serde(flatten)] pub trace: Trace, + #[serde(flatten)] + pub span: Span, + #[serde(flatten, default, skip_serializing_if = "Option::is_none")] + pub parent: Option, } /// Flattened, it contributes the keys of the struct it wraps. @@ -592,6 +596,32 @@ mod wire { pub trace_id: String, } + /// A newtype struct: flattened, it also contributes the keys of the + /// struct it wraps. + #[derive( + Debug, Clone, serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output, + )] + pub struct Span(pub SpanContext); + + #[derive( + Debug, Clone, serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output, + )] + pub struct SpanContext { + pub span_id: String, + } + + #[derive( + Debug, Clone, serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output, + )] + pub struct Parent(pub ParentContext); + + #[derive( + Debug, Clone, serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output, + )] + pub struct ParentContext { + pub parent_id: String, + } + pub fn samples() -> Vec { let variants = [ Identifier::Text("1HGCM82633A004352".into()), @@ -623,6 +653,14 @@ mod wire { trace_id: format!("trace-{i}"), }, }, + span: Span(SpanContext { + span_id: format!("span-{i}"), + }), + parent: (i % 2 == 1).then(|| { + Parent(ParentContext { + parent_id: format!("parent-{i}"), + }) + }), }; let json = serde_json::to_value(&request).unwrap(); // Each sample must also deserialize back to the variant it came from, diff --git a/reflectapi/src/codegen/openapi.rs b/reflectapi/src/codegen/openapi.rs index 36a86ff8..c990ecb9 100644 --- a/reflectapi/src/codegen/openapi.rs +++ b/reflectapi/src/codegen/openapi.rs @@ -1551,6 +1551,11 @@ impl Converter<'_> { assert_eq!(strukt.fields().len(), 1); return self.convert_type_ref(schema, kind, strukt.fields[0].type_ref()); } + // Serde writes a newtype struct as the value it wraps, not as a + // one-element array. + if let schema_codegen::Shape::Newtype(inner) = schema_codegen::Shape::of(&strukt.fields) { + return self.convert_type_ref(schema, kind, inner.type_ref()); + } let (flattened_fields, fields) = strukt.fields().partition::, _>(|f| f.flattened); diff --git a/reflectapi/src/codegen/python.rs b/reflectapi/src/codegen/python.rs index 7cf5efd2..0addad86 100644 --- a/reflectapi/src/codegen/python.rs +++ b/reflectapi/src/codegen/python.rs @@ -9655,7 +9655,10 @@ mod tests { let err = generate_flattening( serde_json::json!({"name": "Tuple"}), vec![serde_json::json!({"kind": "struct", "name": "Tuple", - "fields": {"unnamed": [{"name": "0", "type": {"name": "u32"}, "required": true}]}})], + "fields": {"unnamed": [ + {"name": "0", "type": {"name": "u32"}, "required": true}, + {"name": "1", "type": {"name": "u32"}, "required": true} + ]}})], ) .unwrap_err(); assert!( diff --git a/reflectapi/src/codegen/schema/layout.rs b/reflectapi/src/codegen/schema/layout.rs index 93cb8418..83aeeac4 100644 --- a/reflectapi/src/codegen/schema/layout.rs +++ b/reflectapi/src/codegen/schema/layout.rs @@ -56,8 +56,8 @@ pub(crate) enum Opaque<'a> { Unresolved(String), /// Nested deeper than `MAX_DEPTH` flattens. TooDeep, - /// A tuple struct, a transparent struct without exactly one field, or an - /// `Option` without an argument. + /// A tuple struct with more than one field, a transparent struct without + /// exactly one field, or an `Option` without an argument. Unsupported, } @@ -109,7 +109,11 @@ impl<'a> Layout<'a> { _ => Layout::Unknown(Opaque::Unsupported), } } - Some(Type::Struct(strukt)) => Layout::of_fields(&strukt.fields, lookup, depth), + // Serde writes a newtype struct as the value it wraps, flattened or not. + Some(Type::Struct(strukt)) => match Shape::of(&strukt.fields) { + Shape::Newtype(inner) => Layout::of_type(&inner.type_ref, lookup, depth + 1), + _ => Layout::of_fields(&strukt.fields, lookup, depth), + }, Some(Type::Enum(enm)) => match &enm.representation { Representation::Internal { tag } => Layout::InternallyTagged { enm, @@ -401,7 +405,7 @@ mod tests { let mut external = Enum::new("External".into()); external.representation = Representation::External; let mut tuple = Struct::new("Tuple"); - tuple.fields = Fields::Unnamed(vec![field("0", "u32", true)]); + tuple.fields = Fields::Unnamed(vec![field("0", "u32", true), field("1", "u32", true)]); let types = [ Type::Enum(external), Type::Struct(tuple), @@ -461,4 +465,16 @@ mod tests { let layout = Layout::of_type(&"Wrapper".into(), &lookup_in(&types), 0); assert_eq!(layout.keys().unwrap(), BTreeSet::from(["a"])); } + + #[test] + fn a_newtype_struct_has_the_keys_of_the_type_it_wraps() { + let mut wrapper = Struct::new("Wrapper"); + wrapper.fields = Fields::Unnamed(vec![field("0", "Inner", true)]); + let types = [ + Type::Struct(wrapper), + strukt("Inner", vec![field("a", "u32", true)]), + ]; + let layout = Layout::of_type(&"Wrapper".into(), &lookup_in(&types), 0); + assert_eq!(layout.keys().unwrap(), BTreeSet::from(["a"])); + } } From bf82a05bb969ae18464367c36bb2244cc76148a4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 11:46:20 +0000 Subject: [PATCH 2/4] fix(python): render a flattened wrapper of an enum as the enum Python picks how to render a flattened field from the type the field names. Behind a newtype or transparent struct, an enum was missed, so the model had a nested field (`event: Event`) where serde writes the tag and fields on the parent. Before #217 a newtype failed codegen instead; a transparent wrapper had this bug unless the schema was built with `fold_transparent_types()`. The struct is now rendered with each such field naming the wrapped type, which serde writes identically. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VjkNsfHSd5osRevK95McpQ --- CHANGELOG.md | 1 + reflectapi/src/codegen/python.rs | 81 ++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 240e1757..4225b00a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - **OpenAPI describes a newtype struct as the value it wraps**, as serde writes it. `struct UserId(Uuid)` was a one-element array (`"prefixItems": [...]`), so a client generated from the spec sent `["..."]` where the server expects `"..."`; the demo's `pets.create` request body, `PetsCreateRequest(Pet)`, was `[Pet]` instead of `Pet`. Unit structs, which the schema describes as a newtype of `()`, are now `null` instead of `[null]`. Generated Rust, TypeScript and Python clients already treated newtypes this way and are unchanged. - **Flattening a newtype struct** works in Python and OpenAPI. Serde writes `#[serde(flatten)] w: Wrapper`, where `struct Wrapper(Inner)`, as `Inner`'s keys next to the parent's, and `Option` as all of them or none. Python codegen failed with "cannot flatten 'Wrapper' into a model", and OpenAPI described the group with the one-element array above, which no object matches. +- **Flattening a newtype or `#[serde(transparent)]` struct that wraps an enum** generates the same Python model as flattening the enum: its tag and fields sit on the parent, as serde writes them. Python decided by the wrapper's own type, so it generated a nested field the wire never has. Schemas built with `fold_transparent_types()` weren't affected for transparent wrappers. - A single-field tuple variant whose field has a serde rename, such as `A(#[serde(rename = "foo")] u32)`, is a newtype in OpenAPI variants and Python models, as serde writes it; it was described as an object with a `foo` key. ([#212](https://github.com/thepartly/reflectapi/pull/212)) ### Changed diff --git a/reflectapi/src/codegen/python.rs b/reflectapi/src/codegen/python.rs index 0addad86..15117cb3 100644 --- a/reflectapi/src/codegen/python.rs +++ b/reflectapi/src/codegen/python.rs @@ -728,6 +728,8 @@ fn render_struct_with_flatten( class_names: &BTreeMap, used_type_vars: &mut BTreeSet, ) -> anyhow::Result { + let unwrapped = unwrap_flattened_wrappers(struct_def, schema); + let struct_def = unwrapped.as_ref().unwrap_or(struct_def); // Materializing loses the group structure, so it's read beforehand. let flattened_groups = optional_flattened_groups(struct_def.fields.iter(), schema); let effective = materialize_nested_policy_flatten(struct_def, schema)?; @@ -929,6 +931,48 @@ fn variant_flattened_groups<'a>( } /// Resolve the target type name for a flattened field, unwrapping `Option` +/// `struct_def` with each flattened field that names a newtype or transparent +/// struct naming the type it wraps instead, or `None` if no field does. +/// +/// Serde writes a flattened wrapper exactly as the type it wraps, so the +/// renderers, which decide a flattened field's handling from the type it +/// names, would otherwise miss an enum behind one. +fn unwrap_flattened_wrappers( + struct_def: &reflectapi_schema::Struct, + schema: &Schema, +) -> Option { + let mut unwrapped = struct_def.clone(); + let mut changed = false; + for field in unwrapped.fields.iter_mut().filter(|f| f.flattened()) { + let is_option = (field.type_ref.name == "std::option::Option" + || field.type_ref.name == "reflectapi::Option") + && field.type_ref.arguments.len() == 1; + let target = if is_option { + &mut field.type_ref.arguments[0] + } else { + &mut field.type_ref + }; + // Bounded, as a schema can describe a wrapper of itself. + for _ in 0..10 { + let Some(Type::Struct(wrapper)) = schema.get_type(&target.name) else { + break; + }; + // A generic wrapper's field names its parameter, not a type. + if !wrapper.parameters.is_empty() { + break; + } + let inner = match schema_codegen::Shape::of(&wrapper.fields) { + schema_codegen::Shape::Newtype(inner) => inner, + _ if wrapper.transparent() && wrapper.fields.len() == 1 => &wrapper.fields[0], + _ => break, + }; + *target = inner.type_ref.clone(); + changed = true; + } + } + changed.then_some(unwrapped) +} + fn resolve_flattened_type_name(type_ref: &TypeReference) -> &str { if (type_ref.name == "std::option::Option" || type_ref.name == "reflectapi::Option") && !type_ref.arguments.is_empty() @@ -9572,6 +9616,43 @@ mod tests { ) } + /// A flattened newtype or transparent struct around an enum renders as + /// the enum does when flattened directly: serde writes it the same way. + #[test] + fn flattening_a_wrapper_of_an_enum_renders_the_enum() { + let event = serde_json::json!({"kind": "enum", "name": "Event", + "representation": {"internal": {"tag": "type"}}, + "variants": [{"name": "Created", "fields": {"named": [ + {"name": "id", "type": {"name": "u32"}, "required": true} + ]}}]}); + let newtype = serde_json::json!({"kind": "struct", "name": "Newtype", + "fields": {"unnamed": [{"name": "0", "type": {"name": "Event"}, "required": true}]}}); + let transparent = serde_json::json!({"kind": "struct", "name": "Transparent", + "transparent": true, + "fields": {"named": [{"name": "event", "type": {"name": "Event"}, "required": true}]}}); + let outer = |py: &str| { + let start = py.find("class Outer").unwrap(); + let end = py[start..] + .find("\n\n\n") + .map_or(py.len(), |end| start + end); + py[start..end].to_owned() + }; + let direct = outer( + &generate_flattening(serde_json::json!({"name": "Event"}), vec![event.clone()]) + .unwrap(), + ); + assert!(direct.contains("class OuterCreated"), "{direct}"); + for wrapper in [newtype, transparent] { + let name = wrapper["name"].as_str().unwrap().to_owned(); + let py = generate_flattening( + serde_json::json!({"name": name}), + vec![event.clone(), wrapper], + ) + .unwrap(); + assert_eq!(outer(&py), direct, "{name}"); + } + } + #[test] fn flattening_stops_after_ten_nested_levels() { let mut types = Vec::new(); From f50af6121ae59c8c601b30596bb7c8a23ce9e472 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 03:24:41 +0000 Subject: [PATCH 3/4] test(openapi): newtype structs in snapshots added since this branch #219 and #220 added fixtures with newtype and unit structs, whose OpenAPI components were one-element arrays; they are now the wrapped value, like every other newtype here. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VjkNsfHSd5osRevK95McpQ --- CHANGELOG.md | 2 +- ...ruct_with_serde_default_named_types-4.snap | 17 +++++------- ...truct_with_serde_default_none_types-4.snap | 27 +++++-------------- 3 files changed, 14 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4225b00a..af563a13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,11 +7,11 @@ - A generated Python model rejects an explicit `None` for an omittable field whose type serde can't read from `null`. Since 0.21 the client sends an explicit `None` as `null`, which the server refuses, so `Model(page=None)` for a `#[serde(default)] page: Page` failed with a 400. It now raises a `ValidationError` where the model is built. Leaving the field out is unchanged. Whether a value may be `null` is now part of the shared field contract and is decided from the schema, not from Python's rendered annotation. Before, only primitives and containers were checked. Now these reject `null` too: structs with fields, tuple structs, tagged enums, newtypes and wrappers such as `Box` of those, and primitives such as `chrono::DateTime`, `uuid::Uuid` and `url::Url`. Unit structs, untagged enums with a variant that accepts `null`, newtypes of an `Option`, generic parameters and fields with a custom codec on their side (`deserialize_with` in a request, `serialize_with` in a response), including a newtype's inner field, still accept `None`. - A generated Python module with an omittable field of a unit type, `()` or a unit struct, imports again. The field was annotated `None | None`, which Python can't evaluate, so importing the module raised `TypeError`. It is now `None`. - A generated Rust client compiles when an optional field's type is a generated struct or enum without `Default`, such as a `#[serde(default)] memo: Memo` whose `Memo` implements `Default` only on the server. The field got `#[serde(default)]`, which needs `Default` on the generated type, and generated types derive it only when configured to. Such a field is now `Option`: `None` leaves the key out and the server applies its default. Fields whose generated type derives `Default`, through `base_derives` or the type's `additional_derives`, are unchanged. - - **OpenAPI describes a newtype struct as the value it wraps**, as serde writes it. `struct UserId(Uuid)` was a one-element array (`"prefixItems": [...]`), so a client generated from the spec sent `["..."]` where the server expects `"..."`; the demo's `pets.create` request body, `PetsCreateRequest(Pet)`, was `[Pet]` instead of `Pet`. Unit structs, which the schema describes as a newtype of `()`, are now `null` instead of `[null]`. Generated Rust, TypeScript and Python clients already treated newtypes this way and are unchanged. - **Flattening a newtype struct** works in Python and OpenAPI. Serde writes `#[serde(flatten)] w: Wrapper`, where `struct Wrapper(Inner)`, as `Inner`'s keys next to the parent's, and `Option` as all of them or none. Python codegen failed with "cannot flatten 'Wrapper' into a model", and OpenAPI described the group with the one-element array above, which no object matches. - **Flattening a newtype or `#[serde(transparent)]` struct that wraps an enum** generates the same Python model as flattening the enum: its tag and fields sit on the parent, as serde writes them. Python decided by the wrapper's own type, so it generated a nested field the wire never has. Schemas built with `fold_transparent_types()` weren't affected for transparent wrappers. - A single-field tuple variant whose field has a serde rename, such as `A(#[serde(rename = "foo")] u32)`, is a newtype in OpenAPI variants and Python models, as serde writes it; it was described as an object with a `foo` key. ([#212](https://github.com/thepartly/reflectapi/pull/212)) + ### Changed - Generated clients decode responses after the server removes a deprecated field. Every generator now treats a deprecated field of a response-only type as optional: Python reads it as `None` and types a non-`Option` field `T | None`, TypeScript marks it `?:`, the Rust client types it `Option`, unless it already is one, and OpenAPI leaves it out of `required`. Before, a client generated while the field was deprecated failed to decode every response once the server dropped it, although the client never read it. TypeScript and Rust code that reads such a field must now handle its absence. Types also used in requests keep deprecated fields required, because the server may still require them until it removes them, and so do types the client decodes by trying them in turn: untagged enums and untagged variants, the type behind a flattened `Option`, and every type these reach. There, an optional field could change what a payload decodes as. diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_serde_default_named_types-4.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_serde_default_named_types-4.snap index e57bb72a..cd81f3ff 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_serde_default_named_types-4.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_serde_default_named_types-4.snap @@ -42,18 +42,13 @@ expression: "reflectapi :: codegen :: openapi :: Spec :: from(& schema)" "components": { "schemas": { "reflectapi_demo.tests.serde.TestDefaultNullableNewtype": { - "type": "array", - "prefixItems": [ + "oneOf": [ { - "oneOf": [ - { - "description": "Null", - "type": "null" - }, - { - "$ref": "#/components/schemas/u8" - } - ] + "description": "Null", + "type": "null" + }, + { + "$ref": "#/components/schemas/u8" } ] }, diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_serde_default_none_types-4.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_serde_default_none_types-4.snap index 2bdbb999..61964c37 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_serde_default_none_types-4.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_serde_default_none_types-4.snap @@ -42,26 +42,13 @@ expression: "reflectapi :: codegen :: openapi :: Spec :: from(& schema)" "components": { "schemas": { "reflectapi_demo.tests.serde.TestOptionalNewtype": { - "type": "array", - "prefixItems": [ + "oneOf": [ { - "oneOf": [ - { - "description": "Null", - "type": "null" - }, - { - "$ref": "#/components/schemas/std.string.String" - } - ] - } - ] - }, - "reflectapi_demo.tests.serde.TestUnitMarker": { - "type": "array", - "prefixItems": [ + "description": "Null", + "type": "null" + }, { - "$ref": "#/components/schemas/std.tuple.Tuple0" + "$ref": "#/components/schemas/std.string.String" } ] }, @@ -71,7 +58,7 @@ expression: "reflectapi :: codegen :: openapi :: Spec :: from(& schema)" "title": "reflectapi_demo.tests.serde.input.TestStructWithSerdeDefaultNoneTypes", "properties": { "marker": { - "$ref": "#/components/schemas/reflectapi_demo.tests.serde.TestUnitMarker" + "$ref": "#/components/schemas/std.tuple.Tuple0" }, "nothing": { "$ref": "#/components/schemas/std.tuple.Tuple0" @@ -92,7 +79,7 @@ expression: "reflectapi :: codegen :: openapi :: Spec :: from(& schema)" ], "properties": { "marker": { - "$ref": "#/components/schemas/reflectapi_demo.tests.serde.TestUnitMarker" + "$ref": "#/components/schemas/std.tuple.Tuple0" }, "nothing": { "$ref": "#/components/schemas/std.tuple.Tuple0" From ad38c7140c653fd41ef645441c25d6ec88da9e32 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 03:24:41 +0000 Subject: [PATCH 4/4] refactor(codegen): one rule for which structs serde writes as their field `Layout`, OpenAPI's struct schema and Python's flattened-wrapper rewrite each decided for themselves that a newtype, or a transparent struct with one field, is written as the field it wraps. `wrapped_field` in `codegen/schema/shape.rs` now decides it once. Generated output is unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VjkNsfHSd5osRevK95McpQ --- reflectapi/src/codegen/openapi.rs | 14 ++++----- reflectapi/src/codegen/python.rs | 6 ++-- reflectapi/src/codegen/schema/layout.rs | 17 ++++------- reflectapi/src/codegen/schema/mod.rs | 2 +- reflectapi/src/codegen/schema/shape.rs | 40 ++++++++++++++++++++++++- 5 files changed, 54 insertions(+), 25 deletions(-) diff --git a/reflectapi/src/codegen/openapi.rs b/reflectapi/src/codegen/openapi.rs index c990ecb9..0219035b 100644 --- a/reflectapi/src/codegen/openapi.rs +++ b/reflectapi/src/codegen/openapi.rs @@ -1547,15 +1547,15 @@ impl Converter<'_> { "expect generic struct to be instantiated" ); - if strukt.transparent() { - assert_eq!(strukt.fields().len(), 1); - return self.convert_type_ref(schema, kind, strukt.fields[0].type_ref()); - } - // Serde writes a newtype struct as the value it wraps, not as a - // one-element array. - if let schema_codegen::Shape::Newtype(inner) = schema_codegen::Shape::of(&strukt.fields) { + // Serde writes a newtype or transparent struct as the value it wraps, + // not as a one-element array. + if let Some(inner) = schema_codegen::wrapped_field(strukt) { return self.convert_type_ref(schema, kind, inner.type_ref()); } + assert!( + !strukt.transparent(), + "a transparent struct has exactly one field" + ); let (flattened_fields, fields) = strukt.fields().partition::, _>(|f| f.flattened); diff --git a/reflectapi/src/codegen/python.rs b/reflectapi/src/codegen/python.rs index 15117cb3..f9dbb47f 100644 --- a/reflectapi/src/codegen/python.rs +++ b/reflectapi/src/codegen/python.rs @@ -961,10 +961,8 @@ fn unwrap_flattened_wrappers( if !wrapper.parameters.is_empty() { break; } - let inner = match schema_codegen::Shape::of(&wrapper.fields) { - schema_codegen::Shape::Newtype(inner) => inner, - _ if wrapper.transparent() && wrapper.fields.len() == 1 => &wrapper.fields[0], - _ => break, + let Some(inner) = schema_codegen::wrapped_field(wrapper) else { + break; }; *target = inner.type_ref.clone(); changed = true; diff --git a/reflectapi/src/codegen/schema/layout.rs b/reflectapi/src/codegen/schema/layout.rs index 83aeeac4..e554f67d 100644 --- a/reflectapi/src/codegen/schema/layout.rs +++ b/reflectapi/src/codegen/schema/layout.rs @@ -10,7 +10,7 @@ use std::collections::BTreeSet; use reflectapi_schema::{Enum, Field, Fields, Representation, Type, TypeReference}; use super::presence::{key_presence, FieldWireFacts, KeyPresence}; -use super::shape::Shape; +use super::shape::{wrapped_field, Shape}; /// Flattening deeper than this is treated as unknown. The outermost flattened /// field's type is at depth 0, as in Python's `collect_flattened_fields`. @@ -102,17 +102,10 @@ impl<'a> Layout<'a> { return Layout::Unknown(Opaque::TooDeep); } match lookup(&type_ref.name) { - Some(Type::Struct(strukt)) if strukt.transparent() => { - let mut fields = strukt.fields(); - match (fields.next(), fields.next()) { - (Some(inner), None) => Layout::of_type(&inner.type_ref, lookup, depth + 1), - _ => Layout::Unknown(Opaque::Unsupported), - } - } - // Serde writes a newtype struct as the value it wraps, flattened or not. - Some(Type::Struct(strukt)) => match Shape::of(&strukt.fields) { - Shape::Newtype(inner) => Layout::of_type(&inner.type_ref, lookup, depth + 1), - _ => Layout::of_fields(&strukt.fields, lookup, depth), + Some(Type::Struct(strukt)) => match wrapped_field(strukt) { + Some(inner) => Layout::of_type(&inner.type_ref, lookup, depth + 1), + None if strukt.transparent() => Layout::Unknown(Opaque::Unsupported), + None => Layout::of_fields(&strukt.fields, lookup, depth), }, Some(Type::Enum(enm)) => match &enm.representation { Representation::Internal { tag } => Layout::InternallyTagged { diff --git a/reflectapi/src/codegen/schema/mod.rs b/reflectapi/src/codegen/schema/mod.rs index 8b664c2b..ab7de837 100644 --- a/reflectapi/src/codegen/schema/mod.rs +++ b/reflectapi/src/codegen/schema/mod.rs @@ -31,5 +31,5 @@ pub(crate) use self::semantic::{ SemanticOutputType, SemanticPrimitive, SemanticSchema, SemanticStruct, SemanticType, SemanticTypeParameter, SemanticVariant, SymbolInfo, SymbolTable, }; -pub(crate) use self::shape::Shape; +pub(crate) use self::shape::{wrapped_field, Shape}; pub(crate) use self::symbol::{SymbolId, SymbolKind}; diff --git a/reflectapi/src/codegen/schema/shape.rs b/reflectapi/src/codegen/schema/shape.rs index 11ebfa07..95076533 100644 --- a/reflectapi/src/codegen/schema/shape.rs +++ b/reflectapi/src/codegen/schema/shape.rs @@ -6,7 +6,7 @@ //! as an object). How a variant appears on the wire follows from its shape //! and its enum's representation, so backends read the shape from here. -use reflectapi_schema::{Field, Fields}; +use reflectapi_schema::{Field, Fields, Struct}; #[derive(Debug, Clone, Copy)] pub(crate) enum Shape<'a> { @@ -42,6 +42,22 @@ impl<'a> Shape<'a> { } } +/// The field a struct is written as when it wraps one: a newtype, or a +/// `#[serde(transparent)]` struct with one field. Serde writes either as that +/// field's value, flattened or not. +pub(crate) fn wrapped_field(strukt: &Struct) -> Option<&Field> { + if strukt.transparent() { + return match strukt.fields.iter().as_slice() { + [field] => Some(field), + _ => None, + }; + } + match Shape::of(&strukt.fields) { + Shape::Newtype(field) => Some(field), + _ => None, + } +} + #[cfg(test)] mod tests { use super::*; @@ -50,6 +66,28 @@ mod tests { Field::new(name.into(), "u32".into()) } + #[test] + fn a_newtype_or_a_one_field_transparent_struct_wraps_its_field() { + let mut newtype = Struct::new("Newtype"); + newtype.fields = Fields::Unnamed(vec![field("0")]); + assert_eq!(wrapped_field(&newtype).map(|f| f.name.as_str()), Some("0")); + + let mut transparent = Struct::new("Transparent"); + transparent.transparent = true; + transparent.fields = Fields::Named(vec![field("inner")]); + assert_eq!( + wrapped_field(&transparent).map(|f| f.name.as_str()), + Some("inner") + ); + + let mut named = Struct::new("Named"); + named.fields = Fields::Named(vec![field("inner")]); + assert!(wrapped_field(&named).is_none()); + let mut pair = Struct::new("Pair"); + pair.fields = Fields::Unnamed(vec![field("0"), field("1")]); + assert!(wrapped_field(&pair).is_none()); + } + #[test] fn one_unnamed_field_is_a_newtype_and_any_other_number_a_tuple() { let newtype = Fields::Unnamed(vec![field("0")]);