From 869868750b315e5bb2bb0484383add2d7d8fa433 Mon Sep 17 00:00:00 2001 From: Brian Thorne Date: Fri, 12 Jun 2026 11:50:45 +1200 Subject: [PATCH 1/5] fix(codegen/typescript): emit option-typed fields as optional interface properties serde accepts a missing key for option-typed fields unconditionally: missing_field special-cases deserialize_option, so a missing std::option::Option deserializes to None even without #[serde(default)], and reflectapi::Option behaves the same. The generated TypeScript interfaces nevertheless required these keys unless skip_serializing_if was present, forcing callers to pass explicit nulls that the server never needed. Mark option-typed fields optional (field?: T | null) unconditionally in field_to_ts_field, mirroring the Python backend's resolve_field_optionality. Adds a test sample covering all four annotation combinations of std and reflectapi options. --- CHANGELOG.md | 4 + reflectapi-demo/src/tests/serde.rs | 24 +++ ...ts__basic__reflectapi_struct_option-2.snap | 2 +- ...ruct_with_all_primitive_type_fields-2.snap | 2 +- ...ly_tagged_enum_with_optional_fields-2.snap | 8 +- ...ly_tagged_enum_with_optional_fields-2.snap | 10 +- ..._tests__serde__box_field_unwrapping-2.snap | 2 +- ..._generic_flatten_drops_inner_fields-2.snap | 2 +- ...rde__generic_flatten_leaf_collision-2.snap | 2 +- ...ests__serde__generic_flatten_nested-2.snap | 2 +- ..._generic_flatten_two_instantiations-2.snap | 2 +- ...atten_typevar_nested_in_generic_arg-2.snap | 2 +- ...emo__tests__serde__option_of_option-2.snap | 2 +- ...option_fields_without_serde_default-2.snap | 64 +++++++ ...option_fields_without_serde_default-3.snap | 75 ++++++++ ...option_fields_without_serde_default-4.snap | 164 ++++++++++++++++++ ...h_option_fields_without_serde_default.snap | 142 +++++++++++++++ ...struct_with_serde_skip_serialize_if-2.snap | 2 +- .../rust-dependency-stubs/reflectapi.rs | 22 +++ reflectapi/src/codegen/typescript.rs | 12 +- 20 files changed, 525 insertions(+), 20 deletions(-) create mode 100644 reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-2.snap create mode 100644 reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-3.snap create mode 100644 reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-4.snap create mode 100644 reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default.snap diff --git a/CHANGELOG.md b/CHANGELOG.md index 37453699..ce2ebbfa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Unreleased + +- TypeScript codegen now emits option-typed fields (`std::option::Option` and `reflectapi::Option`) as optional interface properties (`field?: T | null`) regardless of `#[serde(default)]` / `skip_serializing_if` annotations. serde accepts a missing key for option-typed fields unconditionally, so generated clients no longer force callers to pass explicit nulls. This matches the Python backend's behavior. + ## 0.17.6 - New `#[reflectapi(hidden)]` field attribute: the field stays in the schema (marked `"hidden": true`) and remains functional at runtime (e.g. header extraction by the axum adapter), but is excluded from generated clients, documentation, and OpenAPI specs. Not allowed on unnamed (tuple) fields, since removing a positional element would shift indices and break wire compatibility. diff --git a/reflectapi-demo/src/tests/serde.rs b/reflectapi-demo/src/tests/serde.rs index 9de9b1c7..3001e0f9 100644 --- a/reflectapi-demo/src/tests/serde.rs +++ b/reflectapi-demo/src/tests/serde.rs @@ -324,6 +324,30 @@ fn test_struct_with_flatten_optional_and_required() { assert_snapshot!(TestStructWithFlattenOptionalAndRequired); } +#[test] +fn test_struct_with_option_fields_without_serde_default() { + // Option fields must be optional keys in generated clients regardless of + // `#[serde(default)]` / `skip_serializing_if`, because serde accepts a + // missing key for option-typed fields unconditionally (`missing_field` + // special-cases `deserialize_option`, and `reflectapi::Option` + // deserializes the same way). + #[allow(dead_code)] + #[derive(serde::Deserialize, reflectapi::Input)] + #[serde(deny_unknown_fields)] + pub struct ARequest { + pub reflect_option: reflectapi::Option, + + #[serde(default, skip_serializing_if = "reflectapi::Option::is_undefined")] + pub annotated_reflect_option: reflectapi::Option, + + pub std_option: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub annotated_std_option: Option, + } + + assert_input_snapshot!(ARequest); +} + #[derive(reflectapi::Input, reflectapi::Output, serde::Deserialize, serde::Serialize)] #[serde(rename = "struct-name&&")] struct TestStructWithRenameToInvalidChars { diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_struct_option-2.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_struct_option-2.snap index 4f48c12e..4c6c69a4 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_struct_option-2.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_struct_option-2.snap @@ -36,7 +36,7 @@ export namespace reflectapi_demo { export namespace tests { export namespace basic { export interface TestStructOption { - _f: number /* u8 */ | null; + _f?: number /* u8 */ | null; } } } diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_struct_with_all_primitive_type_fields-2.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_struct_with_all_primitive_type_fields-2.snap index 09863ba0..dbddec8c 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_struct_with_all_primitive_type_fields-2.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_struct_with_all_primitive_type_fields-2.snap @@ -57,7 +57,7 @@ export namespace reflectapi_demo { _f_char: string; _f_str: string; _f_unit: null; - _f_option: number /* u8 */ | null; + _f_option?: number /* u8 */ | null; _f_vec: Array; _f_hashmap: Record; _f_hashset: Array; diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__enums__internally_tagged_enum_with_optional_fields-2.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__enums__internally_tagged_enum_with_optional_fields-2.snap index deec637e..1f5a4bdb 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__enums__internally_tagged_enum_with_optional_fields-2.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__enums__internally_tagged_enum_with_optional_fields-2.snap @@ -42,12 +42,12 @@ export namespace reflectapi_demo { } & reflectapi_demo.tests.enums.input.VariantBody) | { type: "named"; - amount: string | null; + amount?: string | null; }; export interface VariantBody { - amount: string | null; - always_null_on_wire: string | null; + amount?: string | null; + always_null_on_wire?: string | null; } } @@ -63,7 +63,7 @@ export namespace reflectapi_demo { export interface VariantBody { amount?: string | null; - always_null_on_wire: string | null; + always_null_on_wire?: string | null; } } } diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__enums__struct_with_flattened_internally_tagged_enum_with_optional_fields-2.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__enums__struct_with_flattened_internally_tagged_enum_with_optional_fields-2.snap index a89cff42..a57330b8 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__enums__struct_with_flattened_internally_tagged_enum_with_optional_fields-2.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__enums__struct_with_flattened_internally_tagged_enum_with_optional_fields-2.snap @@ -38,7 +38,7 @@ export namespace reflectapi_demo { export namespace input { export type Offer = { id: string; - note: string | null; + note?: string | null; } & NullToEmptyObject; export type Thing = @@ -47,12 +47,12 @@ export namespace reflectapi_demo { } & reflectapi_demo.tests.enums.input.VariantBody) | { type: "named"; - amount: string | null; + amount?: string | null; }; export interface VariantBody { - amount: string | null; - always_null_on_wire: string | null; + amount?: string | null; + always_null_on_wire?: string | null; } } @@ -73,7 +73,7 @@ export namespace reflectapi_demo { export interface VariantBody { amount?: string | null; - always_null_on_wire: string | null; + always_null_on_wire?: string | null; } } } diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__box_field_unwrapping-2.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__box_field_unwrapping-2.snap index 417ab2b2..e1e8e09f 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__box_field_unwrapping-2.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__box_field_unwrapping-2.snap @@ -37,7 +37,7 @@ export namespace reflectapi_demo { export namespace serde { export interface TreeNode { label: string; - child: reflectapi_demo.tests.serde.TreeNode | null; + child?: reflectapi_demo.tests.serde.TreeNode | null; } } } diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__generic_flatten_drops_inner_fields-2.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__generic_flatten_drops_inner_fields-2.snap index b75f8d3c..7c031bfc 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__generic_flatten_drops_inner_fields-2.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__generic_flatten_drops_inner_fields-2.snap @@ -54,7 +54,7 @@ export namespace reflectapi_demo { } export type TestUpdateOrElse = { - if_else: C | null; + if_else?: C | null; } & NullToEmptyObject; } } diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__generic_flatten_leaf_collision-2.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__generic_flatten_leaf_collision-2.snap index 7e1af4f4..6dfa95e5 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__generic_flatten_leaf_collision-2.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__generic_flatten_leaf_collision-2.snap @@ -51,7 +51,7 @@ export namespace reflectapi_demo { } export type TestUpdateOrElse = { - if_else: C | null; + if_else?: C | null; } & NullToEmptyObject; export namespace module_a { diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__generic_flatten_nested-2.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__generic_flatten_nested-2.snap index 8448596a..cc315dcd 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__generic_flatten_nested-2.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__generic_flatten_nested-2.snap @@ -66,7 +66,7 @@ export namespace reflectapi_demo { NullToEmptyObject; export type TestUpdateOrElse = { - if_else: C | null; + if_else?: C | null; } & NullToEmptyObject; } } diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__generic_flatten_two_instantiations-2.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__generic_flatten_two_instantiations-2.snap index aed9baa0..b85a7414 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__generic_flatten_two_instantiations-2.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__generic_flatten_two_instantiations-2.snap @@ -60,7 +60,7 @@ export namespace reflectapi_demo { } export type TestUpdateOrElse = { - if_else: C | null; + if_else?: C | null; } & NullToEmptyObject; } } diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__generic_flatten_typevar_nested_in_generic_arg-2.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__generic_flatten_typevar_nested_in_generic_arg-2.snap index 4641dc80..c59a240c 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__generic_flatten_typevar_nested_in_generic_arg-2.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__generic_flatten_typevar_nested_in_generic_arg-2.snap @@ -48,7 +48,7 @@ export namespace reflectapi_demo { } export type TestUpdateOrElse = { - if_else: C | null; + if_else?: C | null; } & NullToEmptyObject; export interface TestWrapperWithNestedTypevarArg { diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__option_of_option-2.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__option_of_option-2.snap index 13ed7c22..863849d0 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__option_of_option-2.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__option_of_option-2.snap @@ -36,7 +36,7 @@ export namespace reflectapi_demo { export namespace tests { export namespace serde { export interface Nested { - value: (string | null) | null; + value?: (string | null) | null; } } } diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-2.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-2.snap new file mode 100644 index 00000000..aebb907f --- /dev/null +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-2.snap @@ -0,0 +1,64 @@ +--- +source: reflectapi-demo/src/tests/serde.rs +expression: "super :: into_input_typescript_code :: < ARequest > ()" +--- +// 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 { + input_test: ( + input: reflectapi_demo.tests.serde.ARequest, + headers: {}, + options?: RequestOptions, + ) => AsyncResult<{}, {}>; + } +} +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 ARequest { + reflect_option?: string | null | undefined; + annotated_reflect_option?: string | null | undefined; + std_option?: string | null; + annotated_std_option?: string | null; + } + } + } +} + +namespace __implementation { + + function input_test(client: Client) { + return ( + input: reflectapi_demo.tests.serde.ARequest, + headers: {}, + options?: RequestOptions, + ) => + __request( + client, + "/input_test", + input, + headers, + options, + ); + } +} diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-3.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-3.snap new file mode 100644 index 00000000..38c0dacd --- /dev/null +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-3.snap @@ -0,0 +1,75 @@ +--- +source: reflectapi-demo/src/tests/serde.rs +expression: "super :: into_input_rust_code :: < ARequest > ()" +--- +// 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 input_test( + &self, + input: super::types::reflectapi_demo::tests::serde::ARequest, + headers: reflectapi::Empty, + ) -> Result> { + reflectapi::rt::__request_impl(&self.client, "/input_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::Serialize)] + pub struct ARequest { + pub reflect_option: reflectapi::Option, + #[serde( + default = "Default::default", + skip_serializing_if = "reflectapi::Option::is_undefined" + )] + pub annotated_reflect_option: reflectapi::Option, + pub std_option: std::option::Option, + #[serde( + default = "Default::default", + skip_serializing_if = "std::option::Option::is_none" + )] + pub annotated_std_option: std::option::Option, + } + } + } + } +} diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-4.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-4.snap new file mode 100644 index 00000000..b616bd1c --- /dev/null +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-4.snap @@ -0,0 +1,164 @@ +--- +source: reflectapi-demo/src/tests/serde.rs +expression: "super :: into_input_python_code :: < ARequest > ()" +--- +""" +DO NOT MODIFY THIS FILE MANUALLY +This file was generated by reflectapi-cli + +Schema name: +""" + +from __future__ import annotations + + +# Standard library imports +from enum import Enum +from typing import Annotated, Any, Generic, Optional, TypeVar, Union + +# Third-party imports +from pydantic import BaseModel, ConfigDict, Field + +# 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 ReflectapiDemoTestsSerdeARequest(ReflectapiPartialModel): + model_config = ConfigDict( + extra="ignore", + populate_by_name=True, + validate_assignment=True, + protected_namespaces=(), + defer_build=True, + ) + + reflect_option: str | None = None + annotated_reflect_option: str | None = None + std_option: str | None = None + annotated_std_option: str | None = None + + +# 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.""" + + ARequest = ReflectapiDemoTestsSerdeARequest + + +class AsyncInputClient: + """Async client for input_ operations.""" + + def __init__(self, client: AsyncClientBase) -> None: + self._client = client + + async def test( + self, + data: Optional[reflectapi_demo.tests.serde.ARequest] = None, + ) -> ApiResponse[Any]: + """ + + Args: + data: Request data for the test operation. + + Returns: + ApiResponse[Any]: Response containing Any data + """ + path = "/input_test" + + params: dict[str, Any] = {} + return await self._client._make_request( + path, + params=params if params else None, + json_model=data, + response_model=None, + ) + + +class AsyncClient(AsyncClientBase): + """Async client for the API.""" + + def __init__( + self, + base_url: str, + **kwargs: Any, + ) -> None: + super().__init__(base_url, **kwargs) + + self.input_ = AsyncInputClient(self) + + +class InputClient: + """Synchronous client for input_ operations.""" + + def __init__(self, client: ClientBase) -> None: + self._client = client + + def test( + self, + data: Optional[reflectapi_demo.tests.serde.ARequest] = None, + ) -> ApiResponse[Any]: + """ + + Args: + data: Request data for the test operation. + + Returns: + ApiResponse[Any]: Response containing Any data + """ + path = "/input_test" + + params: dict[str, Any] = {} + return self._client._make_request( + path, + params=params if params else None, + json_model=data, + response_model=None, + ) + + +class Client(ClientBase): + """Synchronous client for the API.""" + + def __init__( + self, + base_url: str, + **kwargs: Any, + ) -> None: + super().__init__(base_url, **kwargs) + + self.input_ = InputClient(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 [ + ReflectapiDemoTestsSerdeARequest, +]: + 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__struct_with_option_fields_without_serde_default.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default.snap new file mode 100644 index 00000000..2864be6e --- /dev/null +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default.snap @@ -0,0 +1,142 @@ +--- +source: reflectapi-demo/src/tests/serde.rs +expression: "super :: into_input_schema :: < ARequest > ().input_types" +--- +{ + "types": [ + { + "kind": "struct", + "name": "reflectapi::Empty", + "description": "Struct object with no fields", + "fields": "none" + }, + { + "kind": "enum", + "name": "reflectapi::Option", + "description": "Undefinable Option type", + "parameters": [ + { + "name": "T" + } + ], + "representation": "none", + "variants": [ + { + "name": "Undefined", + "description": "The value is missing, i.e. undefined in JavaScript", + "fields": "none" + }, + { + "name": "None", + "description": "The value is provided but set to none, i.e. null in JavaScript", + "fields": "none" + }, + { + "name": "Some", + "description": "The value is provided and set to some value", + "fields": { + "unnamed": [ + { + "name": "0", + "type": { + "name": "T" + } + } + ] + } + } + ] + }, + { + "kind": "struct", + "name": "reflectapi_demo::tests::serde::ARequest", + "fields": { + "named": [ + { + "name": "reflect_option", + "type": { + "name": "reflectapi::Option", + "arguments": [ + { + "name": "std::string::String" + } + ] + }, + "required": true + }, + { + "name": "annotated_reflect_option", + "type": { + "name": "reflectapi::Option", + "arguments": [ + { + "name": "std::string::String" + } + ] + } + }, + { + "name": "std_option", + "type": { + "name": "std::option::Option", + "arguments": [ + { + "name": "std::string::String" + } + ] + }, + "required": true + }, + { + "name": "annotated_std_option", + "type": { + "name": "std::option::Option", + "arguments": [ + { + "name": "std::string::String" + } + ] + } + } + ] + } + }, + { + "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" + } + ] +} diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_serde_skip_serialize_if-2.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_serde_skip_serialize_if-2.snap index 6bdb3881..102ebdfc 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_serde_skip_serialize_if-2.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_serde_skip_serialize_if-2.snap @@ -40,7 +40,7 @@ export namespace reflectapi_demo { export namespace serde { export namespace input { export interface TestStructWithSerdeSkipSerializeIf { - f: number /* u8 */ | null; + f?: number /* u8 */ | null; } } diff --git a/reflectapi/src/codegen/rust-dependency-stubs/reflectapi.rs b/reflectapi/src/codegen/rust-dependency-stubs/reflectapi.rs index 9cff0fbe..ecf24080 100644 --- a/reflectapi/src/codegen/rust-dependency-stubs/reflectapi.rs +++ b/reflectapi/src/codegen/rust-dependency-stubs/reflectapi.rs @@ -1,3 +1,25 @@ +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Default)] +pub enum Option { + #[default] + Undefined, + None, + Some(T), +} + +impl Option { + pub fn is_undefined(&self) -> bool { + matches!(self, Self::Undefined) + } + + pub fn is_none(&self) -> bool { + matches!(self, Self::None) + } + + pub fn is_some(&self) -> bool { + matches!(self, Self::Some(_)) + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct Empty {} diff --git a/reflectapi/src/codegen/typescript.rs b/reflectapi/src/codegen/typescript.rs index 6a7191a4..6b4e2cca 100644 --- a/reflectapi/src/codegen/typescript.rs +++ b/reflectapi/src/codegen/typescript.rs @@ -1098,11 +1098,21 @@ fn field_to_ts_field( schema: &crate::Schema, implemented_types: &HashMap, ) -> templates::Field { + // A nullable option field can always be omitted by the sender: serde + // deserializes a missing key as `None` even without `#[serde(default)]` + // (`missing_field` special-cases `deserialize_option`), and the same + // holds for `reflectapi::Option`. Mark such fields optional so callers + // are not forced to pass an explicit null. This mirrors the Python + // backend's `resolve_field_optionality`. + let is_option_type = matches!( + field.type_ref.name.as_str(), + "std::option::Option" | "reflectapi::Option" + ); templates::Field { name: field.serde_name().into(), description: doc_to_ts_comments(&field.description, field.deprecation_note.as_deref(), 4), type_: type_ref_to_ts_ref(&field.type_ref, schema, implemented_types), - optional: !field.required, + optional: !field.required || is_option_type, } } From eca6498df43158a1741a8ccfa1715aa671dde404 Mon Sep 17 00:00:00 2001 From: Brian Thorne Date: Fri, 12 Jun 2026 14:48:27 +1200 Subject: [PATCH 2/5] fix(codegen/typescript): keep required authoritative for option fields with custom serde codecs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit serde's missing_field path cannot route through serialize_with/deserialize_with, so an option-typed field with a custom codec and no #[serde(default)] rejects a missing key — the optional-promotion must not apply to it. The derive now records a custom_codec flag on schema fields whenever a serde with-family attribute is present (direction-agnostic so input/output reflections stay identical for type consolidation), and the TypeScript backend skips the option promotion for such fields. Extends the test sample with a deserialize_with field that stays required. --- reflectapi-demo/src/tests/serde.rs | 13 ++++++++++++- ..._with_option_fields_without_serde_default-2.snap | 1 + ..._with_option_fields_without_serde_default-3.snap | 1 + ..._with_option_fields_without_serde_default-4.snap | 1 + ...ct_with_option_fields_without_serde_default.snap | 13 +++++++++++++ reflectapi-derive/src/derive.rs | 6 ++++++ reflectapi-derive/src/tokenizable_schema.rs | 2 ++ reflectapi-schema/src/lib.rs | 11 +++++++++++ reflectapi/src/codegen/typescript.rs | 13 +++++++++---- 9 files changed, 56 insertions(+), 5 deletions(-) diff --git a/reflectapi-demo/src/tests/serde.rs b/reflectapi-demo/src/tests/serde.rs index 3001e0f9..a0957e4d 100644 --- a/reflectapi-demo/src/tests/serde.rs +++ b/reflectapi-demo/src/tests/serde.rs @@ -330,7 +330,15 @@ fn test_struct_with_option_fields_without_serde_default() { // `#[serde(default)]` / `skip_serializing_if`, because serde accepts a // missing key for option-typed fields unconditionally (`missing_field` // special-cases `deserialize_option`, and `reflectapi::Option` - // deserializes the same way). + // deserializes the same way). The exception is a field with a custom + // serde codec: `missing_field` cannot route through `deserialize_with`, + // so a missing key is rejected and the field must stay required. + fn deserialize_option<'de, D: serde::Deserializer<'de>>( + deserializer: D, + ) -> Result, D::Error> { + serde::Deserialize::deserialize(deserializer) + } + #[allow(dead_code)] #[derive(serde::Deserialize, reflectapi::Input)] #[serde(deny_unknown_fields)] @@ -343,6 +351,9 @@ fn test_struct_with_option_fields_without_serde_default() { pub std_option: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub annotated_std_option: Option, + + #[serde(deserialize_with = "deserialize_option")] + pub custom_codec_option: Option, } assert_input_snapshot!(ARequest); diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-2.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-2.snap index aebb907f..da81aea5 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-2.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-2.snap @@ -40,6 +40,7 @@ export namespace reflectapi_demo { annotated_reflect_option?: string | null | undefined; std_option?: string | null; annotated_std_option?: string | null; + custom_codec_option: string | null; } } } diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-3.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-3.snap index 38c0dacd..b6e61d54 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-3.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-3.snap @@ -68,6 +68,7 @@ pub mod types { skip_serializing_if = "std::option::Option::is_none" )] pub annotated_std_option: std::option::Option, + pub custom_codec_option: std::option::Option, } } } diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-4.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-4.snap index b616bd1c..0ac874fc 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-4.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-4.snap @@ -39,6 +39,7 @@ class ReflectapiDemoTestsSerdeARequest(ReflectapiPartialModel): annotated_reflect_option: str | None = None std_option: str | None = None annotated_std_option: str | None = None + custom_codec_option: str | None = None # Namespace classes for dotted access to types diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default.snap index 2864be6e..0ef3d9b7 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default.snap @@ -97,6 +97,19 @@ expression: "super :: into_input_schema :: < ARequest > ().input_types" } ] } + }, + { + "name": "custom_codec_option", + "type": { + "name": "std::option::Option", + "arguments": [ + { + "name": "std::string::String" + } + ] + }, + "required": true, + "custom_codec": true } ] } diff --git a/reflectapi-derive/src/derive.rs b/reflectapi-derive/src/derive.rs index 1aef78fe..17c89a23 100644 --- a/reflectapi-derive/src/derive.rs +++ b/reflectapi-derive/src/derive.rs @@ -442,6 +442,12 @@ fn visit_field(cx: &Context, field: &ast::Field<'_>) -> Option { let required = self.inner.required; let flattened = self.inner.flattened; let hidden = self.inner.hidden; + let custom_codec = self.inner.custom_codec; let transform_callback = self.inner.transform_callback.as_str(); let mut transform_callback_fn = quote::quote! { None @@ -126,6 +127,7 @@ impl ToTokens for TokenizableField<'_> { required: #required, flattened: #flattened, hidden: #hidden, + custom_codec: #custom_codec, transform_callback: String::new(), transform_callback_fn: #transform_callback_fn, } diff --git a/reflectapi-schema/src/lib.rs b/reflectapi-schema/src/lib.rs index 056de5ae..25e8e331 100644 --- a/reflectapi-schema/src/lib.rs +++ b/reflectapi-schema/src/lib.rs @@ -1132,6 +1132,14 @@ pub struct Field { #[serde(skip_serializing_if = "is_false", default)] pub hidden: bool, + /// If true, the field's value passes through a custom serde codec + /// (`#[serde(with = ...)]` / `serialize_with` / `deserialize_with`), + /// so wire behavior — such as whether a missing key is accepted — + /// cannot be inferred from the field's type alone. + /// Default is false + #[serde(skip_serializing_if = "is_false", default)] + pub custom_codec: bool, + #[serde(skip, default)] pub transform_callback: String, #[serde(skip, default)] @@ -1150,6 +1158,7 @@ impl PartialEq for Field { required, flattened, hidden, + custom_codec, transform_callback, transform_callback_fn: _, }: &Self, @@ -1162,6 +1171,7 @@ impl PartialEq for Field { && self.required == *required && self.flattened == *flattened && self.hidden == *hidden + && self.custom_codec == *custom_codec && self.transform_callback == *transform_callback } } @@ -1191,6 +1201,7 @@ impl Field { required: Default::default(), flattened: Default::default(), hidden: Default::default(), + custom_codec: Default::default(), transform_callback: Default::default(), transform_callback_fn: Default::default(), } diff --git a/reflectapi/src/codegen/typescript.rs b/reflectapi/src/codegen/typescript.rs index 6b4e2cca..52b6f058 100644 --- a/reflectapi/src/codegen/typescript.rs +++ b/reflectapi/src/codegen/typescript.rs @@ -1104,10 +1104,15 @@ fn field_to_ts_field( // holds for `reflectapi::Option`. Mark such fields optional so callers // are not forced to pass an explicit null. This mirrors the Python // backend's `resolve_field_optionality`. - let is_option_type = matches!( - field.type_ref.name.as_str(), - "std::option::Option" | "reflectapi::Option" - ); + // + // The exception is a field with a custom serde codec: `missing_field` + // cannot route through `serialize_with`/`deserialize_with`, so a missing + // key is rejected by the server and `required` stays authoritative. + let is_option_type = !field.custom_codec + && matches!( + field.type_ref.name.as_str(), + "std::option::Option" | "reflectapi::Option" + ); templates::Field { name: field.serde_name().into(), description: doc_to_ts_comments(&field.description, field.deprecation_note.as_deref(), 4), From 18225da6052db5905987c6d5eae87ccfed996805 Mon Sep 17 00:00:00 2001 From: Brian Thorne Date: Fri, 12 Jun 2026 16:31:42 +1200 Subject: [PATCH 3/5] refactor(codegen): resolve field wire contracts in the shared schema layer Field optionality was re-derived independently by each backend from (required, type name, custom_codec) heuristics, and the copies drifted: Python knew option-typed fields are omittable but TypeScript did not, and neither knew custom serde codecs reject missing keys until patched separately. Introduce resolve_field_wire_contract in the compiler-side schema layer, resolving a field into three orthogonal facts: key presence, value nullability, and whether absence is distinct from null. The TypeScript and Python backends now render from this single resolution; Python's resolve_field_optionality becomes a thin mapping from the contract to Python syntax. This also closes the pre-existing Python gap where option fields with deserialize_with were generated as omittable keys the server rejects. First concrete step toward the SemanticSchema-as-backend-contract direction (#129): backend-independent meaning computed once in the shared layer, backend-specific rendering kept local. --- ...option_fields_without_serde_default-4.snap | 2 +- reflectapi/src/codegen/python.rs | 79 +++++----- reflectapi/src/codegen/schema/mod.rs | 2 + reflectapi/src/codegen/schema/presence.rs | 138 ++++++++++++++++++ reflectapi/src/codegen/typescript.rs | 22 +-- 5 files changed, 188 insertions(+), 55 deletions(-) create mode 100644 reflectapi/src/codegen/schema/presence.rs diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-4.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-4.snap index 0ac874fc..f7052f5a 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-4.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-4.snap @@ -39,7 +39,7 @@ class ReflectapiDemoTestsSerdeARequest(ReflectapiPartialModel): annotated_reflect_option: str | None = None std_option: str | None = None annotated_std_option: str | None = None - custom_codec_option: str | None = None + custom_codec_option: str | None # Namespace classes for dotted access to types diff --git a/reflectapi/src/codegen/python.rs b/reflectapi/src/codegen/python.rs index 593f3b06..0fa0dd33 100644 --- a/reflectapi/src/codegen/python.rs +++ b/reflectapi/src/codegen/python.rs @@ -73,26 +73,27 @@ struct PythonMetadataUsage { /// Resolve Python field optionality: determines whether a field needs a default /// value of `None` and whether `| None` should be appended to its type hint. +/// +/// Key presence comes from the shared wire-contract rules; this function only +/// maps the contract to Python rendering. Option-typed fields already carry +/// `| None` in their type hint via the type mapping, so `nullable` controls +/// whether it must be appended for fields that may be absent. fn resolve_field_optionality( - type_name: &str, + field: &reflectapi_schema::Field, field_type: String, - required: bool, + effective_required: bool, ) -> (bool, Option, String) { - let is_option_type = type_name == "std::option::Option" || type_name == "reflectapi::Option"; - if !required { - if is_option_type { + let contract = schema_codegen::resolve_field_wire_contract(field, effective_required); + match (contract.key, contract.nullable) { + (schema_codegen::KeyPresence::Optional, true) => { (true, Some("None".to_string()), field_type) - } else { - ( - true, - Some("None".to_string()), - format!("{field_type} | None"), - ) } - } else if is_option_type { - (true, Some("None".to_string()), field_type) - } else { - (false, None, field_type) + (schema_codegen::KeyPresence::Optional, false) => ( + true, + Some("None".to_string()), + format!("{field_type} | None"), + ), + (schema_codegen::KeyPresence::Required, _) => (false, None, field_type), } } @@ -754,7 +755,7 @@ fn render_struct_with_flattened_internal_enum( used_type_vars, )?; let (optional, default_value, final_type) = - resolve_field_optionality(&field.type_ref.name, field_type, field.required); + resolve_field_optionality(field, field_type, field.required); base_fields.push(templates::Field { name: python_name, type_annotation: final_type, @@ -804,7 +805,7 @@ fn render_struct_with_flattened_internal_enum( let (python_name, alias) = sanitize_field_name_with_alias(field.name(), field.serde_name()); let (optional, default_value, final_type) = - resolve_field_optionality(&field.type_ref.name, field_type, field.required); + resolve_field_optionality(field, field_type, field.required); base_fields.push(templates::Field { name: python_name, type_annotation: final_type, @@ -855,7 +856,7 @@ fn render_struct_with_flattened_internal_enum( used_type_vars, )?; let (optional, default_value, final_type) = - resolve_field_optionality(&vf.type_ref.name, field_type, vf.required); + resolve_field_optionality(vf, field_type, vf.required); let (sanitized, alias) = sanitize_field_name_with_alias(vf.name(), vf.serde_name()); fields.push(templates::Field { @@ -917,7 +918,7 @@ fn render_struct_with_flattened_internal_enum( let (sanitized, alias) = sanitize_field_name_with_alias(sf.name(), sf.serde_name()); let (optional, default_value, final_type) = - resolve_field_optionality(&sf.type_ref.name, field_type, sf.required); + resolve_field_optionality(sf, field_type, sf.required); fields.push(templates::Field { name: sanitized, type_annotation: final_type, @@ -1001,7 +1002,7 @@ fn render_struct_with_flatten_standard( )?; let (optional, default_value, final_type) = - resolve_field_optionality(&field.type_ref.name, field_type, field.required); + resolve_field_optionality(field, field_type, field.required); all_fields.push(templates::Field { name: python_name, type_annotation: final_type, @@ -3647,11 +3648,8 @@ fn make_flattened_field( used_type_vars, )?; - let (optional, default_value, final_field_type) = resolve_field_optionality( - &field.type_ref.name, - field_type, - field.required && parent_required, - ); + let (optional, default_value, final_field_type) = + resolve_field_optionality(field, field_type, field.required && parent_required); let (sanitized, alias) = sanitize_field_name_with_alias(field.name(), field.serde_name()); Ok(templates::Field { @@ -3716,8 +3714,16 @@ fn collect_flattened_enum_fields( }); let (sanitized, alias) = sanitize_field_name_with_alias(&field_name, &field_name); + // A flattened enum field has no schema `Field` of its own; synthesize one + // for the shared wire-contract resolution. serde rejects `flatten` + // combined with `with`-family codecs, so no codec flag can be lost here. + let synthetic_field = { + let mut synthetic = reflectapi_schema::Field::new(field_name.clone(), type_ref.clone()); + synthetic.required = parent_required; + synthetic + }; let (optional, default_value, final_type) = - resolve_field_optionality(&type_ref.name, enum_python_type, parent_required); + resolve_field_optionality(&synthetic_field, enum_python_type, parent_required); collected_fields.push(templates::Field { name: sanitized, @@ -3781,11 +3787,8 @@ fn render_struct( &active_generics, used_type_vars, )?; - let (_, _, field_type) = resolve_field_optionality( - &field.type_ref.name, - base_field_type, - field.required, - ); + let (_, _, field_type) = + resolve_field_optionality(field, base_field_type, field.required); Ok(field_type) }) .collect::, anyhow::Error>>()?; @@ -3820,7 +3823,7 @@ fn render_struct( )?; let (optional, default_value, field_type) = - resolve_field_optionality(&field.type_ref.name, base_field_type, field.required); + resolve_field_optionality(field, base_field_type, field.required); let (sanitized, alias) = sanitize_field_name_with_alias(field.name(), field.serde_name()); @@ -4074,7 +4077,7 @@ fn render_adjacently_tagged_enum( used_type_vars, )?; let (optional, default_value, final_field_type) = - resolve_field_optionality(&field.type_ref.name, field_type, field.required); + resolve_field_optionality(field, field_type, field.required); let (sanitized, alias) = sanitize_field_name_with_alias(field.name(), field.serde_name()); content_fields.push(templates::Field { @@ -4315,7 +4318,7 @@ fn render_externally_tagged_enum( )?; let (optional, default_value, final_field_type) = - resolve_field_optionality(&field.type_ref.name, field_type, field.required); + resolve_field_optionality(field, field_type, field.required); let (sanitized, alias) = sanitize_field_name_with_alias(field.name(), field.serde_name()); @@ -4552,7 +4555,7 @@ fn render_internally_tagged_enum( let (optional, default_value, final_field_type) = resolve_field_optionality( - &struct_field.type_ref.name, + struct_field, field_type, struct_field.required, ); @@ -4613,7 +4616,7 @@ fn render_internally_tagged_enum( )?; let (optional, default_value, final_field_type) = - resolve_field_optionality(&field.type_ref.name, field_type, field.required); + resolve_field_optionality(field, field_type, field.required); let (sanitized, alias) = sanitize_field_name_with_alias(field.name(), field.serde_name()); @@ -4758,7 +4761,7 @@ fn render_untagged_enum( used_type_vars, )?; let (optional, default_value, final_field_type) = - resolve_field_optionality(&field.type_ref.name, field_type, field.required); + resolve_field_optionality(field, field_type, field.required); let (sanitized, alias) = sanitize_field_name_with_alias(field.name(), field.serde_name()); @@ -4787,7 +4790,7 @@ fn render_untagged_enum( used_type_vars, )?; let (optional, default_value, final_field_type) = - resolve_field_optionality(&field.type_ref.name, field_type, field.required); + resolve_field_optionality(field, field_type, field.required); fields.push(templates::Field { name: if unnamed_fields.len() == 1 { diff --git a/reflectapi/src/codegen/schema/mod.rs b/reflectapi/src/codegen/schema/mod.rs index b49d2d74..4e5308be 100644 --- a/reflectapi/src/codegen/schema/mod.rs +++ b/reflectapi/src/codegen/schema/mod.rs @@ -8,6 +8,7 @@ mod ids; mod normalize; +mod presence; mod semantic; mod symbol; @@ -16,6 +17,7 @@ mod symbol; // `self::ids::*`, `self::normalize::*`, etc. for the module's own tests. pub(crate) use self::ids::{build_schema_ids, SchemaIds}; pub(crate) use self::normalize::{Consolidation, Naming, Normalizer, PipelineBuilder}; +pub(crate) use self::presence::{resolve_field_wire_contract, KeyPresence}; pub(crate) use self::semantic::{ FieldStyle, ResolvedTypeReference, SemanticEnum, SemanticField, SemanticFunction, SemanticOutputType, SemanticPrimitive, SemanticSchema, SemanticStruct, SemanticType, diff --git a/reflectapi/src/codegen/schema/presence.rs b/reflectapi/src/codegen/schema/presence.rs new file mode 100644 index 00000000..21a8e9ff --- /dev/null +++ b/reflectapi/src/codegen/schema/presence.rs @@ -0,0 +1,138 @@ +//! Field wire-contract resolution. +//! +//! A field's wire behavior decomposes into three orthogonal facts, resolved +//! here once so that every backend renders from the same meaning instead of +//! re-deriving it from `(required, type name, custom_codec)` heuristics: +//! +//! - **key presence** — may the key be absent on the wire? +//! - **value nullability** — may the value be `null`? +//! - **absence semantics** — is "absent" distinct from "null"? +//! +//! The resolution rules, with the serde behavior that justifies them: +//! +//! | Field shape | Missing key behavior (deserialize) | +//! |---|---| +//! | `T` | rejected — key required | +//! | `Option`, no attrs | accepted as `None` — `missing_field` special-cases `deserialize_option` | +//! | `reflectapi::Option`, no `default` | accepted as `None` (collapses the three-state type; a lint may catch this in future) | +//! | `reflectapi::Option` + `default` | accepted as `Undefined` | +//! | any + `serde(default)` | accepted — default value used | +//! | option + `with`/`deserialize_with`, no `default` | **rejected** — `missing_field` cannot route through a custom codec | +//! +//! On the serialize side the folded `required` flag (false when +//! `skip_serializing_if` is present) already captures whether the key can be +//! absent, and option types capture nullability. + +use reflectapi_schema::Field; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum KeyPresence { + /// The key must be present on the wire. + Required, + /// The key may be omitted by the sender. + Optional, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct FieldWireContract { + pub key: KeyPresence, + /// The value may be `null` on the wire (option-typed field). + pub nullable: bool, + /// Absence carries distinct meaning from an explicit `null` + /// (three-state `reflectapi::Option`). + pub absent_distinct_from_null: bool, +} + +/// Resolve a field's wire contract. +/// +/// `effective_required` is the folded serde requiredness for the rendering +/// context — usually `field.required`, but callers expanding flattened +/// fields combine it with the parent's requiredness. +pub(crate) fn resolve_field_wire_contract( + field: &Field, + effective_required: bool, +) -> FieldWireContract { + let is_std_option = field.type_ref.name == "std::option::Option"; + let is_reflect_option = field.type_ref.name == "reflectapi::Option"; + let is_option = is_std_option || is_reflect_option; + + let key = if !effective_required { + // serde(default) / skip_serializing_if: absence is always fine, + // custom codec or not (a default is applied without invoking it). + KeyPresence::Optional + } else if is_option && !field.custom_codec { + // serde accepts a missing key for plain option-typed fields even + // without serde(default); a custom codec breaks that path, so + // `required` stays authoritative there. + KeyPresence::Optional + } else { + KeyPresence::Required + }; + + FieldWireContract { + key, + nullable: is_option, + absent_distinct_from_null: is_reflect_option, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use reflectapi_schema::TypeReference; + + fn field(type_name: &str, required: bool, custom_codec: bool) -> Field { + let argument = TypeReference::new("u8".to_string(), vec![]); + let mut f = Field::new( + "f".to_string(), + TypeReference::new(type_name.to_string(), vec![argument]), + ); + f.required = required; + f.custom_codec = custom_codec; + f + } + + #[test] + fn plain_required_field_is_required() { + let c = resolve_field_wire_contract(&field("u8", true, false), true); + assert_eq!(c.key, KeyPresence::Required); + assert!(!c.nullable); + assert!(!c.absent_distinct_from_null); + } + + #[test] + fn std_option_is_optional_even_when_required() { + let c = resolve_field_wire_contract(&field("std::option::Option", true, false), true); + assert_eq!(c.key, KeyPresence::Optional); + assert!(c.nullable); + assert!(!c.absent_distinct_from_null); + } + + #[test] + fn reflectapi_option_is_optional_and_three_state() { + let c = resolve_field_wire_contract(&field("reflectapi::Option", true, false), true); + assert_eq!(c.key, KeyPresence::Optional); + assert!(c.nullable); + assert!(c.absent_distinct_from_null); + } + + #[test] + fn custom_codec_keeps_required_authoritative() { + let c = resolve_field_wire_contract(&field("std::option::Option", true, true), true); + assert_eq!(c.key, KeyPresence::Required); + assert!(c.nullable); + } + + #[test] + fn custom_codec_with_serde_default_is_optional() { + let c = resolve_field_wire_contract(&field("std::option::Option", false, true), false); + assert_eq!(c.key, KeyPresence::Optional); + } + + #[test] + fn non_required_non_option_is_optional_not_nullable() { + let c = resolve_field_wire_contract(&field("u8", false, false), false); + assert_eq!(c.key, KeyPresence::Optional); + assert!(!c.nullable); + } +} diff --git a/reflectapi/src/codegen/typescript.rs b/reflectapi/src/codegen/typescript.rs index 52b6f058..59293494 100644 --- a/reflectapi/src/codegen/typescript.rs +++ b/reflectapi/src/codegen/typescript.rs @@ -1098,26 +1098,16 @@ fn field_to_ts_field( schema: &crate::Schema, implemented_types: &HashMap, ) -> templates::Field { - // A nullable option field can always be omitted by the sender: serde - // deserializes a missing key as `None` even without `#[serde(default)]` - // (`missing_field` special-cases `deserialize_option`), and the same - // holds for `reflectapi::Option`. Mark such fields optional so callers - // are not forced to pass an explicit null. This mirrors the Python - // backend's `resolve_field_optionality`. - // - // The exception is a field with a custom serde codec: `missing_field` - // cannot route through `serialize_with`/`deserialize_with`, so a missing - // key is rejected by the server and `required` stays authoritative. - let is_option_type = !field.custom_codec - && matches!( - field.type_ref.name.as_str(), - "std::option::Option" | "reflectapi::Option" - ); + // Key presence is resolved by the shared wire-contract rules (a missing + // key is accepted for plain option-typed fields, but not when a custom + // serde codec is involved); nullability is already carried by the + // type mapping (`T | null`, `T | null | undefined`). + let contract = crate::codegen::schema::resolve_field_wire_contract(field, field.required); templates::Field { name: field.serde_name().into(), description: doc_to_ts_comments(&field.description, field.deprecation_note.as_deref(), 4), type_: type_ref_to_ts_ref(&field.type_ref, schema, implemented_types), - optional: !field.required || is_option_type, + optional: contract.key == crate::codegen::schema::KeyPresence::Optional, } } From f1c553d031697028992a4c46a25bd25767ad2696 Mon Sep 17 00:00:00 2001 From: Brian Thorne Date: Fri, 12 Jun 2026 18:03:38 +1200 Subject: [PATCH 4/5] refactor(schema): record serialize_with/deserialize_with facts instead of a custom_codec flag The raw schema should record facts; interpretation belongs to the compiler layer. The single direction-agnostic custom_codec flag was a pre-interpreted blend of two facts, and a lossy one: missing-key acceptance is purely a deserialize-side question, so a field with only a custom serializer was wrongly demoted to a required key. Record the presence of the serde attributes literally (serialize_with, deserialize_with; with sets both) and key the wire-contract carve-out on deserialize_with alone. Also adds both flags to Field's Hash impl, which the previous flag had missed. --- reflectapi-demo/src/tests/serde.rs | 7 +-- ...option_fields_without_serde_default-2.snap | 2 +- ...option_fields_without_serde_default-3.snap | 2 +- ...option_fields_without_serde_default-4.snap | 2 +- ...h_option_fields_without_serde_default.snap | 4 +- reflectapi-derive/src/derive.rs | 11 ++-- reflectapi-derive/src/tokenizable_schema.rs | 6 ++- reflectapi-schema/src/lib.rs | 28 +++++++--- reflectapi/src/codegen/python.rs | 3 +- reflectapi/src/codegen/schema/presence.rs | 51 ++++++++++++------- reflectapi/src/codegen/typescript.rs | 4 +- 11 files changed, 75 insertions(+), 45 deletions(-) diff --git a/reflectapi-demo/src/tests/serde.rs b/reflectapi-demo/src/tests/serde.rs index a0957e4d..def421cf 100644 --- a/reflectapi-demo/src/tests/serde.rs +++ b/reflectapi-demo/src/tests/serde.rs @@ -331,8 +331,9 @@ fn test_struct_with_option_fields_without_serde_default() { // missing key for option-typed fields unconditionally (`missing_field` // special-cases `deserialize_option`, and `reflectapi::Option` // deserializes the same way). The exception is a field with a custom - // serde codec: `missing_field` cannot route through `deserialize_with`, - // so a missing key is rejected and the field must stay required. + // serde deserializer: `missing_field` cannot route through + // `deserialize_with`, so a missing key is rejected and the field must + // stay required. fn deserialize_option<'de, D: serde::Deserializer<'de>>( deserializer: D, ) -> Result, D::Error> { @@ -353,7 +354,7 @@ fn test_struct_with_option_fields_without_serde_default() { pub annotated_std_option: Option, #[serde(deserialize_with = "deserialize_option")] - pub custom_codec_option: Option, + pub custom_deserializer_option: Option, } assert_input_snapshot!(ARequest); diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-2.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-2.snap index da81aea5..80318f54 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-2.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-2.snap @@ -40,7 +40,7 @@ export namespace reflectapi_demo { annotated_reflect_option?: string | null | undefined; std_option?: string | null; annotated_std_option?: string | null; - custom_codec_option: string | null; + custom_deserializer_option: string | null; } } } diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-3.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-3.snap index b6e61d54..f0ba3f0d 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-3.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-3.snap @@ -68,7 +68,7 @@ pub mod types { skip_serializing_if = "std::option::Option::is_none" )] pub annotated_std_option: std::option::Option, - pub custom_codec_option: std::option::Option, + pub custom_deserializer_option: std::option::Option, } } } diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-4.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-4.snap index f7052f5a..fab876c6 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-4.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-4.snap @@ -39,7 +39,7 @@ class ReflectapiDemoTestsSerdeARequest(ReflectapiPartialModel): annotated_reflect_option: str | None = None std_option: str | None = None annotated_std_option: str | None = None - custom_codec_option: str | None + custom_deserializer_option: str | None # Namespace classes for dotted access to types diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default.snap index 0ef3d9b7..24b59d65 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default.snap @@ -99,7 +99,7 @@ expression: "super :: into_input_schema :: < ARequest > ().input_types" } }, { - "name": "custom_codec_option", + "name": "custom_deserializer_option", "type": { "name": "std::option::Option", "arguments": [ @@ -109,7 +109,7 @@ expression: "super :: into_input_schema :: < ARequest > ().input_types" ] }, "required": true, - "custom_codec": true + "deserialize_with": true } ] } diff --git a/reflectapi-derive/src/derive.rs b/reflectapi-derive/src/derive.rs index 17c89a23..e6bb57ac 100644 --- a/reflectapi-derive/src/derive.rs +++ b/reflectapi-derive/src/derive.rs @@ -442,12 +442,11 @@ fn visit_field(cx: &Context, field: &ast::Field<'_>) -> Option { let required = self.inner.required; let flattened = self.inner.flattened; let hidden = self.inner.hidden; - let custom_codec = self.inner.custom_codec; + let serialize_with = self.inner.serialize_with; + let deserialize_with = self.inner.deserialize_with; let transform_callback = self.inner.transform_callback.as_str(); let mut transform_callback_fn = quote::quote! { None @@ -127,7 +128,8 @@ impl ToTokens for TokenizableField<'_> { required: #required, flattened: #flattened, hidden: #hidden, - custom_codec: #custom_codec, + serialize_with: #serialize_with, + deserialize_with: #deserialize_with, transform_callback: String::new(), transform_callback_fn: #transform_callback_fn, } diff --git a/reflectapi-schema/src/lib.rs b/reflectapi-schema/src/lib.rs index 25e8e331..d83f1af4 100644 --- a/reflectapi-schema/src/lib.rs +++ b/reflectapi-schema/src/lib.rs @@ -1132,13 +1132,20 @@ pub struct Field { #[serde(skip_serializing_if = "is_false", default)] pub hidden: bool, - /// If true, the field's value passes through a custom serde codec - /// (`#[serde(with = ...)]` / `serialize_with` / `deserialize_with`), - /// so wire behavior — such as whether a missing key is accepted — - /// cannot be inferred from the field's type alone. + /// If true, the field has a custom serde serializer: + /// `#[serde(serialize_with = ...)]` or `#[serde(with = ...)]`. + /// Serialized wire shape cannot be inferred from the field's type alone. /// Default is false #[serde(skip_serializing_if = "is_false", default)] - pub custom_codec: bool, + pub serialize_with: bool, + + /// If true, the field has a custom serde deserializer: + /// `#[serde(deserialize_with = ...)]` or `#[serde(with = ...)]`. + /// Deserialization behavior — such as whether a missing key is + /// accepted — cannot be inferred from the field's type alone. + /// Default is false + #[serde(skip_serializing_if = "is_false", default)] + pub deserialize_with: bool, #[serde(skip, default)] pub transform_callback: String, @@ -1158,7 +1165,8 @@ impl PartialEq for Field { required, flattened, hidden, - custom_codec, + serialize_with, + deserialize_with, transform_callback, transform_callback_fn: _, }: &Self, @@ -1171,7 +1179,8 @@ impl PartialEq for Field { && self.required == *required && self.flattened == *flattened && self.hidden == *hidden - && self.custom_codec == *custom_codec + && self.serialize_with == *serialize_with + && self.deserialize_with == *deserialize_with && self.transform_callback == *transform_callback } } @@ -1186,6 +1195,8 @@ impl std::hash::Hash for Field { self.required.hash(state); self.flattened.hash(state); self.hidden.hash(state); + self.serialize_with.hash(state); + self.deserialize_with.hash(state); self.transform_callback.hash(state); } } @@ -1201,7 +1212,8 @@ impl Field { required: Default::default(), flattened: Default::default(), hidden: Default::default(), - custom_codec: Default::default(), + serialize_with: Default::default(), + deserialize_with: Default::default(), transform_callback: Default::default(), transform_callback_fn: Default::default(), } diff --git a/reflectapi/src/codegen/python.rs b/reflectapi/src/codegen/python.rs index 0fa0dd33..2c984ed1 100644 --- a/reflectapi/src/codegen/python.rs +++ b/reflectapi/src/codegen/python.rs @@ -3716,7 +3716,8 @@ fn collect_flattened_enum_fields( // A flattened enum field has no schema `Field` of its own; synthesize one // for the shared wire-contract resolution. serde rejects `flatten` - // combined with `with`-family codecs, so no codec flag can be lost here. + // combined with `with`-family attributes, so no serializer or + // deserializer flag can be lost here. let synthetic_field = { let mut synthetic = reflectapi_schema::Field::new(field_name.clone(), type_ref.clone()); synthetic.required = parent_required; diff --git a/reflectapi/src/codegen/schema/presence.rs b/reflectapi/src/codegen/schema/presence.rs index 21a8e9ff..4fa33424 100644 --- a/reflectapi/src/codegen/schema/presence.rs +++ b/reflectapi/src/codegen/schema/presence.rs @@ -17,11 +17,14 @@ //! | `reflectapi::Option`, no `default` | accepted as `None` (collapses the three-state type; a lint may catch this in future) | //! | `reflectapi::Option` + `default` | accepted as `Undefined` | //! | any + `serde(default)` | accepted — default value used | -//! | option + `with`/`deserialize_with`, no `default` | **rejected** — `missing_field` cannot route through a custom codec | +//! | option + `with`/`deserialize_with`, no `default` | **rejected** — `missing_field` cannot route through a custom deserializer | //! -//! On the serialize side the folded `required` flag (false when -//! `skip_serializing_if` is present) already captures whether the key can be -//! absent, and option types capture nullability. +//! Only the deserialize side affects key presence: a custom *serializer* +//! never changes whether the key may be absent (`skip_serializing_if`, +//! folded into `required`, controls that), so `serialize_with` is +//! irrelevant here. On the serialize side the folded `required` flag +//! already captures whether the key can be absent, and option types +//! capture nullability. use reflectapi_schema::Field; @@ -58,12 +61,13 @@ pub(crate) fn resolve_field_wire_contract( let key = if !effective_required { // serde(default) / skip_serializing_if: absence is always fine, - // custom codec or not (a default is applied without invoking it). + // custom deserializer or not (a default is applied without + // invoking it). KeyPresence::Optional - } else if is_option && !field.custom_codec { + } else if is_option && !field.deserialize_with { // serde accepts a missing key for plain option-typed fields even - // without serde(default); a custom codec breaks that path, so - // `required` stays authoritative there. + // without serde(default); a custom deserializer breaks that path, + // so `required` stays authoritative there. KeyPresence::Optional } else { KeyPresence::Required @@ -81,20 +85,19 @@ mod tests { use super::*; use reflectapi_schema::TypeReference; - fn field(type_name: &str, required: bool, custom_codec: bool) -> Field { + fn field(type_name: &str, required: bool) -> Field { let argument = TypeReference::new("u8".to_string(), vec![]); let mut f = Field::new( "f".to_string(), TypeReference::new(type_name.to_string(), vec![argument]), ); f.required = required; - f.custom_codec = custom_codec; f } #[test] fn plain_required_field_is_required() { - let c = resolve_field_wire_contract(&field("u8", true, false), true); + let c = resolve_field_wire_contract(&field("u8", true), true); assert_eq!(c.key, KeyPresence::Required); assert!(!c.nullable); assert!(!c.absent_distinct_from_null); @@ -102,7 +105,7 @@ mod tests { #[test] fn std_option_is_optional_even_when_required() { - let c = resolve_field_wire_contract(&field("std::option::Option", true, false), true); + let c = resolve_field_wire_contract(&field("std::option::Option", true), true); assert_eq!(c.key, KeyPresence::Optional); assert!(c.nullable); assert!(!c.absent_distinct_from_null); @@ -110,28 +113,40 @@ mod tests { #[test] fn reflectapi_option_is_optional_and_three_state() { - let c = resolve_field_wire_contract(&field("reflectapi::Option", true, false), true); + let c = resolve_field_wire_contract(&field("reflectapi::Option", true), true); assert_eq!(c.key, KeyPresence::Optional); assert!(c.nullable); assert!(c.absent_distinct_from_null); } #[test] - fn custom_codec_keeps_required_authoritative() { - let c = resolve_field_wire_contract(&field("std::option::Option", true, true), true); + fn custom_deserializer_keeps_required_authoritative() { + let mut f = field("std::option::Option", true); + f.deserialize_with = true; + let c = resolve_field_wire_contract(&f, true); assert_eq!(c.key, KeyPresence::Required); assert!(c.nullable); } #[test] - fn custom_codec_with_serde_default_is_optional() { - let c = resolve_field_wire_contract(&field("std::option::Option", false, true), false); + fn custom_serializer_alone_does_not_affect_key_presence() { + let mut f = field("std::option::Option", true); + f.serialize_with = true; + let c = resolve_field_wire_contract(&f, true); + assert_eq!(c.key, KeyPresence::Optional); + } + + #[test] + fn custom_deserializer_with_serde_default_is_optional() { + let mut f = field("std::option::Option", false); + f.deserialize_with = true; + let c = resolve_field_wire_contract(&f, false); assert_eq!(c.key, KeyPresence::Optional); } #[test] fn non_required_non_option_is_optional_not_nullable() { - let c = resolve_field_wire_contract(&field("u8", false, false), false); + let c = resolve_field_wire_contract(&field("u8", false), false); assert_eq!(c.key, KeyPresence::Optional); assert!(!c.nullable); } diff --git a/reflectapi/src/codegen/typescript.rs b/reflectapi/src/codegen/typescript.rs index 59293494..7015c6e5 100644 --- a/reflectapi/src/codegen/typescript.rs +++ b/reflectapi/src/codegen/typescript.rs @@ -1100,8 +1100,8 @@ fn field_to_ts_field( ) -> templates::Field { // Key presence is resolved by the shared wire-contract rules (a missing // key is accepted for plain option-typed fields, but not when a custom - // serde codec is involved); nullability is already carried by the - // type mapping (`T | null`, `T | null | undefined`). + // serde deserializer is involved); nullability is already carried by + // the type mapping (`T | null`, `T | null | undefined`). let contract = crate::codegen::schema::resolve_field_wire_contract(field, field.required); templates::Field { name: field.serde_name().into(), From 70351286a539a6df0b7d10778ba92f6f70530bc2 Mon Sep 17 00:00:00 2001 From: Brian Thorne Date: Sat, 13 Jun 2026 09:35:16 +1200 Subject: [PATCH 5/5] refactor(codegen): address review findings on the wire-contract layer - resolve_field_wire_contract now takes a FieldWireFacts input naming exactly the facts resolution depends on (type name, deserialize_with, folded required), removing the redundant required parameter and the synthetic Field construction in the Python flattened-enum path. - Rename the contract's nullable to declared_nullable and document that whether null appears on the wire is direction-dependent. - Adopt absent_distinct_from_null: Python's is_partial sites and the ReflectapiPartialModel import detection resolve through the contract instead of string-comparing the reflectapi::Option type name. - Update the Field::required quadrant doc in reflectapi-schema to state the deserialize-side leniency the generated clients now reflect. - Extend the test sample to cover serialize_with and with end to end: a custom serializer alone leaves the key optional, with stays required, and the schema snapshot pins that with sets both flags. The sample now reflects both input and output typespaces. - Expand the changelog entry: Python deserialize_with fix, new schema Field flags, and the shared resolution pass. - Make the presence module pub(crate) so its types are nameable, drop a speculative lint reference from its docs, and comment why Python renders absent-but-not-nullable fields with a None default. --- CHANGELOG.md | 3 + reflectapi-demo/src/tests/serde.rs | 36 +- ...option_fields_without_serde_default-2.snap | 23 +- ...option_fields_without_serde_default-3.snap | 15 +- ...option_fields_without_serde_default-4.snap | 299 +++++------ ...option_fields_without_serde_default-5.snap | 167 +++++++ ...h_option_fields_without_serde_default.snap | 467 +++++++++++++----- reflectapi-derive/src/derive.rs | 6 +- reflectapi-schema/src/lib.rs | 8 + reflectapi/src/codegen/python.rs | 156 +++--- reflectapi/src/codegen/schema/mod.rs | 4 +- reflectapi/src/codegen/schema/presence.rs | 145 +++--- reflectapi/src/codegen/typescript.rs | 4 +- 13 files changed, 906 insertions(+), 427 deletions(-) create mode 100644 reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-5.snap diff --git a/CHANGELOG.md b/CHANGELOG.md index ce2ebbfa..1a0c0fd0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,9 @@ ## Unreleased - TypeScript codegen now emits option-typed fields (`std::option::Option` and `reflectapi::Option`) as optional interface properties (`field?: T | null`) regardless of `#[serde(default)]` / `skip_serializing_if` annotations. serde accepts a missing key for option-typed fields unconditionally, so generated clients no longer force callers to pass explicit nulls. This matches the Python backend's behavior. +- The exception is fields with a custom serde deserializer: `#[serde(deserialize_with = ...)]` / `#[serde(with = ...)]` without `#[serde(default)]` rejects a missing key, so such fields stay required. This also fixes the Python backend, which previously generated those fields as omittable keys the server rejects. A custom serializer alone (`serialize_with`) does not affect key optionality. +- Schema format: `Field` gains two additive flags, `serialize_with` and `deserialize_with`, recording the presence of the corresponding serde attributes (`with` sets both; omitted from the JSON when false). Code constructing `reflectapi_schema::Field` with exhaustive struct literals needs the new fields. +- Internals: field key-presence/nullability resolution now lives in one shared codegen schema pass (`resolve_field_wire_contract`) consumed by the TypeScript and Python backends, instead of per-backend heuristics. ## 0.17.6 diff --git a/reflectapi-demo/src/tests/serde.rs b/reflectapi-demo/src/tests/serde.rs index def421cf..db8534d9 100644 --- a/reflectapi-demo/src/tests/serde.rs +++ b/reflectapi-demo/src/tests/serde.rs @@ -333,15 +333,39 @@ fn test_struct_with_option_fields_without_serde_default() { // deserializes the same way). The exception is a field with a custom // serde deserializer: `missing_field` cannot route through // `deserialize_with`, so a missing key is rejected and the field must - // stay required. + // stay required. A custom serializer alone does not affect key + // presence, so `serialize_with` fields remain optional, while `with` + // implies a custom deserializer and the field stays required. fn deserialize_option<'de, D: serde::Deserializer<'de>>( deserializer: D, ) -> Result, D::Error> { serde::Deserialize::deserialize(deserializer) } + fn serialize_option( + value: &Option, + serializer: S, + ) -> Result { + serde::Serialize::serialize(value, serializer) + } + + mod option_passthrough { + pub fn serialize( + value: &std::option::Option, + serializer: S, + ) -> Result { + serde::Serialize::serialize(value, serializer) + } + + pub fn deserialize<'de, D: serde::Deserializer<'de>>( + deserializer: D, + ) -> Result, D::Error> { + serde::Deserialize::deserialize(deserializer) + } + } + #[allow(dead_code)] - #[derive(serde::Deserialize, reflectapi::Input)] + #[derive(serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output)] #[serde(deny_unknown_fields)] pub struct ARequest { pub reflect_option: reflectapi::Option, @@ -355,9 +379,15 @@ fn test_struct_with_option_fields_without_serde_default() { #[serde(deserialize_with = "deserialize_option")] pub custom_deserializer_option: Option, + + #[serde(serialize_with = "serialize_option")] + pub custom_serializer_option: Option, + + #[serde(with = "option_passthrough")] + pub with_module_option: Option, } - assert_input_snapshot!(ARequest); + assert_snapshot!(ARequest); } #[derive(reflectapi::Input, reflectapi::Output, serde::Deserialize, serde::Serialize)] diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-2.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-2.snap index 80318f54..0a778915 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-2.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-2.snap @@ -1,6 +1,6 @@ --- source: reflectapi-demo/src/tests/serde.rs -expression: "super :: into_input_typescript_code :: < ARequest > ()" +expression: "super :: into_typescript_code :: < ARequest > ()" --- // DO NOT MODIFY THIS FILE MANUALLY // This file was generated by reflectapi-cli @@ -13,11 +13,11 @@ export function client(base: string | Client): __definition.Interface { export namespace __definition { export interface Interface { - input_test: ( + inout_test: ( input: reflectapi_demo.tests.serde.ARequest, headers: {}, options?: RequestOptions, - ) => AsyncResult<{}, {}>; + ) => AsyncResult; } } export namespace reflectapi { @@ -41,6 +41,8 @@ export namespace reflectapi_demo { std_option?: string | null; annotated_std_option?: string | null; custom_deserializer_option: string | null; + custom_serializer_option?: string | null; + with_module_option: string | null; } } } @@ -48,18 +50,17 @@ export namespace reflectapi_demo { namespace __implementation { - function input_test(client: Client) { + function inout_test(client: Client) { return ( input: reflectapi_demo.tests.serde.ARequest, headers: {}, options?: RequestOptions, ) => - __request( - client, - "/input_test", - input, - headers, - options, - ); + __request< + reflectapi_demo.tests.serde.ARequest, + {}, + reflectapi_demo.tests.serde.ARequest, + {} + >(client, "/inout_test", input, headers, options); } } diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-3.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-3.snap index f0ba3f0d..c91edc11 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-3.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-3.snap @@ -1,6 +1,6 @@ --- source: reflectapi-demo/src/tests/serde.rs -expression: "super :: into_input_rust_code :: < ARequest > ()" +expression: "super :: into_rust_code :: < ARequest > ()" --- // DO NOT MODIFY THIS FILE MANUALLY // This file was generated by reflectapi-cli @@ -24,12 +24,15 @@ pub mod interface { pub fn new(client: C) -> Self { Self { client } } - pub async fn input_test( + pub async fn inout_test( &self, input: super::types::reflectapi_demo::tests::serde::ARequest, headers: reflectapi::Empty, - ) -> Result> { - reflectapi::rt::__request_impl(&self.client, "/input_test", input, headers).await + ) -> Result< + super::types::reflectapi_demo::tests::serde::ARequest, + reflectapi::rt::Error, + > { + reflectapi::rt::__request_impl(&self.client, "/inout_test", input, headers).await } } @@ -54,7 +57,7 @@ pub mod types { pub mod tests { pub mod serde { - #[derive(Debug, serde::Serialize)] + #[derive(Debug, serde::Deserialize, serde::Serialize)] pub struct ARequest { pub reflect_option: reflectapi::Option, #[serde( @@ -69,6 +72,8 @@ pub mod types { )] pub annotated_std_option: std::option::Option, pub custom_deserializer_option: std::option::Option, + pub custom_serializer_option: std::option::Option, + pub with_module_option: std::option::Option, } } } diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-4.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-4.snap index fab876c6..444868e0 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-4.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-4.snap @@ -1,165 +1,140 @@ --- source: reflectapi-demo/src/tests/serde.rs -expression: "super :: into_input_python_code :: < ARequest > ()" +expression: "reflectapi :: codegen :: openapi :: Spec :: from(& schema)" --- -""" -DO NOT MODIFY THIS FILE MANUALLY -This file was generated by reflectapi-cli - -Schema name: -""" - -from __future__ import annotations - - -# Standard library imports -from enum import Enum -from typing import Annotated, Any, Generic, Optional, TypeVar, Union - -# Third-party imports -from pydantic import BaseModel, ConfigDict, Field - -# 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 ReflectapiDemoTestsSerdeARequest(ReflectapiPartialModel): - model_config = ConfigDict( - extra="ignore", - populate_by_name=True, - validate_assignment=True, - protected_namespaces=(), - defer_build=True, - ) - - reflect_option: str | None = None - annotated_reflect_option: str | None = None - std_option: str | None = None - annotated_std_option: str | None = None - custom_deserializer_option: str | None - - -# 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.""" - - ARequest = ReflectapiDemoTestsSerdeARequest - - -class AsyncInputClient: - """Async client for input_ operations.""" - - def __init__(self, client: AsyncClientBase) -> None: - self._client = client - - async def test( - self, - data: Optional[reflectapi_demo.tests.serde.ARequest] = None, - ) -> ApiResponse[Any]: - """ - - Args: - data: Request data for the test operation. - - Returns: - ApiResponse[Any]: Response containing Any data - """ - path = "/input_test" - - params: dict[str, Any] = {} - return await self._client._make_request( - path, - params=params if params else None, - json_model=data, - response_model=None, - ) - - -class AsyncClient(AsyncClientBase): - """Async client for the API.""" - - def __init__( - self, - base_url: str, - **kwargs: Any, - ) -> None: - super().__init__(base_url, **kwargs) - - self.input_ = AsyncInputClient(self) - - -class InputClient: - """Synchronous client for input_ operations.""" - - def __init__(self, client: ClientBase) -> None: - self._client = client - - def test( - self, - data: Optional[reflectapi_demo.tests.serde.ARequest] = None, - ) -> ApiResponse[Any]: - """ - - Args: - data: Request data for the test operation. - - Returns: - ApiResponse[Any]: Response containing Any data - """ - path = "/input_test" - - params: dict[str, Any] = {} - return self._client._make_request( - path, - params=params if params else None, - json_model=data, - response_model=None, - ) - - -class Client(ClientBase): - """Synchronous client for the API.""" - - def __init__( - self, - base_url: str, - **kwargs: Any, - ) -> None: - super().__init__(base_url, **kwargs) - - self.input_ = InputClient(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 [ - ReflectapiDemoTestsSerdeARequest, -]: - 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) - ) +{ + "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.ARequest" + } + } + }, + "required": true + }, + "responses": { + "200": { + "description": "200 OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/reflectapi_demo.tests.serde.ARequest" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "reflectapi_demo.tests.serde.ARequest": { + "type": "object", + "title": "reflectapi_demo.tests.serde.ARequest", + "required": [ + "custom_deserializer_option", + "custom_serializer_option", + "reflect_option", + "std_option", + "with_module_option" + ], + "properties": { + "annotated_reflect_option": { + "oneOf": [ + { + "description": "Null", + "type": "null" + }, + { + "$ref": "#/components/schemas/std.string.String" + } + ] + }, + "annotated_std_option": { + "oneOf": [ + { + "description": "Null", + "type": "null" + }, + { + "$ref": "#/components/schemas/std.string.String" + } + ] + }, + "custom_deserializer_option": { + "oneOf": [ + { + "description": "Null", + "type": "null" + }, + { + "$ref": "#/components/schemas/std.string.String" + } + ] + }, + "custom_serializer_option": { + "oneOf": [ + { + "description": "Null", + "type": "null" + }, + { + "$ref": "#/components/schemas/std.string.String" + } + ] + }, + "reflect_option": { + "oneOf": [ + { + "description": "Null", + "type": "null" + }, + { + "$ref": "#/components/schemas/std.string.String" + } + ] + }, + "std_option": { + "oneOf": [ + { + "description": "Null", + "type": "null" + }, + { + "$ref": "#/components/schemas/std.string.String" + } + ] + }, + "with_module_option": { + "oneOf": [ + { + "description": "Null", + "type": "null" + }, + { + "$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__serde__struct_with_option_fields_without_serde_default-5.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-5.snap new file mode 100644 index 00000000..cb23ff4f --- /dev/null +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default-5.snap @@ -0,0 +1,167 @@ +--- +source: reflectapi-demo/src/tests/serde.rs +expression: "super :: into_python_code :: < ARequest > ()" +--- +""" +DO NOT MODIFY THIS FILE MANUALLY +This file was generated by reflectapi-cli + +Schema name: +""" + +from __future__ import annotations + + +# Standard library imports +from enum import Enum +from typing import Annotated, Any, Generic, Optional, TypeVar, Union + +# Third-party imports +from pydantic import BaseModel, ConfigDict, Field + +# 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 ReflectapiDemoTestsSerdeARequest(ReflectapiPartialModel): + model_config = ConfigDict( + extra="ignore", + populate_by_name=True, + validate_assignment=True, + protected_namespaces=(), + defer_build=True, + ) + + reflect_option: str | None = None + annotated_reflect_option: str | None = None + std_option: str | None = None + annotated_std_option: str | None = None + custom_deserializer_option: str | None + custom_serializer_option: str | None = None + with_module_option: str | None + + +# 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.""" + + ARequest = ReflectapiDemoTestsSerdeARequest + + +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.ARequest] = None, + ) -> ApiResponse[reflectapi_demo.tests.serde.ARequest]: + """ + + Args: + data: Request data for the test operation. + + Returns: + ApiResponse[reflectapi_demo.tests.serde.ARequest]: Response containing reflectapi_demo.tests.serde.ARequest 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.ARequest, + ) + + +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(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.ARequest] = None, + ) -> ApiResponse[reflectapi_demo.tests.serde.ARequest]: + """ + + Args: + data: Request data for the test operation. + + Returns: + ApiResponse[reflectapi_demo.tests.serde.ARequest]: Response containing reflectapi_demo.tests.serde.ARequest 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.ARequest, + ) + + +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(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 [ + ReflectapiDemoTestsSerdeARequest, +]: + 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__struct_with_option_fields_without_serde_default.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default.snap index 24b59d65..aede6493 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__serde__struct_with_option_fields_without_serde_default.snap @@ -1,155 +1,380 @@ --- source: reflectapi-demo/src/tests/serde.rs -expression: "super :: into_input_schema :: < ARequest > ().input_types" +expression: schema --- { - "types": [ + "name": "", + "functions": [ { - "kind": "struct", - "name": "reflectapi::Empty", - "description": "Struct object with no fields", - "fields": "none" - }, - { - "kind": "enum", - "name": "reflectapi::Option", - "description": "Undefinable Option type", - "parameters": [ - { - "name": "T" - } - ], - "representation": "none", - "variants": [ - { - "name": "Undefined", - "description": "The value is missing, i.e. undefined in JavaScript", - "fields": "none" - }, - { - "name": "None", - "description": "The value is provided but set to none, i.e. null in JavaScript", - "fields": "none" - }, - { - "name": "Some", - "description": "The value is provided and set to some value", - "fields": { - "unnamed": [ - { - "name": "0", - "type": { - "name": "T" - } - } - ] - } - } + "name": "inout_test", + "path": "", + "input_type": { + "name": "reflectapi_demo::tests::serde::ARequest" + }, + "output_kind": "complete", + "output_type": { + "name": "reflectapi_demo::tests::serde::ARequest" + }, + "serialization": [ + "json", + "msgpack" ] - }, - { - "kind": "struct", - "name": "reflectapi_demo::tests::serde::ARequest", - "fields": { - "named": [ + } + ], + "input_types": { + "types": [ + { + "kind": "struct", + "name": "reflectapi::Empty", + "description": "Struct object with no fields", + "fields": "none" + }, + { + "kind": "enum", + "name": "reflectapi::Option", + "description": "Undefinable Option type", + "parameters": [ { - "name": "reflect_option", - "type": { - "name": "reflectapi::Option", - "arguments": [ + "name": "T" + } + ], + "representation": "none", + "variants": [ + { + "name": "Undefined", + "description": "The value is missing, i.e. undefined in JavaScript", + "fields": "none" + }, + { + "name": "None", + "description": "The value is provided but set to none, i.e. null in JavaScript", + "fields": "none" + }, + { + "name": "Some", + "description": "The value is provided and set to some value", + "fields": { + "unnamed": [ { - "name": "std::string::String" + "name": "0", + "type": { + "name": "T" + } } ] + } + } + ] + }, + { + "kind": "struct", + "name": "reflectapi_demo::tests::serde::ARequest", + "fields": { + "named": [ + { + "name": "reflect_option", + "type": { + "name": "reflectapi::Option", + "arguments": [ + { + "name": "std::string::String" + } + ] + }, + "required": true }, - "required": true + { + "name": "annotated_reflect_option", + "type": { + "name": "reflectapi::Option", + "arguments": [ + { + "name": "std::string::String" + } + ] + } + }, + { + "name": "std_option", + "type": { + "name": "std::option::Option", + "arguments": [ + { + "name": "std::string::String" + } + ] + }, + "required": true + }, + { + "name": "annotated_std_option", + "type": { + "name": "std::option::Option", + "arguments": [ + { + "name": "std::string::String" + } + ] + } + }, + { + "name": "custom_deserializer_option", + "type": { + "name": "std::option::Option", + "arguments": [ + { + "name": "std::string::String" + } + ] + }, + "required": true, + "deserialize_with": true + }, + { + "name": "custom_serializer_option", + "type": { + "name": "std::option::Option", + "arguments": [ + { + "name": "std::string::String" + } + ] + }, + "required": true, + "serialize_with": true + }, + { + "name": "with_module_option", + "type": { + "name": "std::option::Option", + "arguments": [ + { + "name": "std::string::String" + } + ] + }, + "required": true, + "serialize_with": true, + "deserialize_with": 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": "annotated_reflect_option", - "type": { - "name": "reflectapi::Option", - "arguments": [ + "name": "Some", + "description": "The value is provided and set to some value", + "fields": { + "unnamed": [ { - "name": "std::string::String" + "name": "0", + "type": { + "name": "T" + } } ] } + } + ] + }, + { + "kind": "primitive", + "name": "std::string::String", + "description": "UTF-8 encoded string" + } + ] + }, + "output_types": { + "types": [ + { + "kind": "struct", + "name": "reflectapi::Infallible", + "description": "Error object which is expected to be never returned", + "fields": "none" + }, + { + "kind": "enum", + "name": "reflectapi::Option", + "description": "Undefinable Option type", + "parameters": [ + { + "name": "T" + } + ], + "representation": "none", + "variants": [ + { + "name": "Undefined", + "description": "The value is missing, i.e. undefined in JavaScript", + "fields": "none" }, { - "name": "std_option", - "type": { - "name": "std::option::Option", - "arguments": [ - { - "name": "std::string::String" - } - ] - }, - "required": true + "name": "None", + "description": "The value is provided but set to none, i.e. null in JavaScript", + "fields": "none" }, { - "name": "annotated_std_option", - "type": { - "name": "std::option::Option", - "arguments": [ + "name": "Some", + "description": "The value is provided and set to some value", + "fields": { + "unnamed": [ { - "name": "std::string::String" + "name": "0", + "type": { + "name": "T" + } } ] } + } + ] + }, + { + "kind": "struct", + "name": "reflectapi_demo::tests::serde::ARequest", + "fields": { + "named": [ + { + "name": "reflect_option", + "type": { + "name": "reflectapi::Option", + "arguments": [ + { + "name": "std::string::String" + } + ] + }, + "required": true + }, + { + "name": "annotated_reflect_option", + "type": { + "name": "reflectapi::Option", + "arguments": [ + { + "name": "std::string::String" + } + ] + } + }, + { + "name": "std_option", + "type": { + "name": "std::option::Option", + "arguments": [ + { + "name": "std::string::String" + } + ] + }, + "required": true + }, + { + "name": "annotated_std_option", + "type": { + "name": "std::option::Option", + "arguments": [ + { + "name": "std::string::String" + } + ] + } + }, + { + "name": "custom_deserializer_option", + "type": { + "name": "std::option::Option", + "arguments": [ + { + "name": "std::string::String" + } + ] + }, + "required": true, + "deserialize_with": true + }, + { + "name": "custom_serializer_option", + "type": { + "name": "std::option::Option", + "arguments": [ + { + "name": "std::string::String" + } + ] + }, + "required": true, + "serialize_with": true + }, + { + "name": "with_module_option", + "type": { + "name": "std::option::Option", + "arguments": [ + { + "name": "std::string::String" + } + ] + }, + "required": true, + "serialize_with": true, + "deserialize_with": 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": "custom_deserializer_option", - "type": { - "name": "std::option::Option", - "arguments": [ + "name": "Some", + "description": "The value is provided and set to some value", + "fields": { + "unnamed": [ { - "name": "std::string::String" + "name": "0", + "type": { + "name": "T" + } } ] - }, - "required": true, - "deserialize_with": true + } } ] + }, + { + "kind": "primitive", + "name": "std::string::String", + "description": "UTF-8 encoded string" } - }, - { - "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" - } - ] + ] + } } diff --git a/reflectapi-derive/src/derive.rs b/reflectapi-derive/src/derive.rs index e6bb57ac..b0ef2af5 100644 --- a/reflectapi-derive/src/derive.rs +++ b/reflectapi-derive/src/derive.rs @@ -442,9 +442,9 @@ fn visit_field(cx: &Context, field: &ast::Field<'_>) -> Option` is enum with Undefined, None and Some variants /// - TypeScript: T | null | undefined /// + /// Note: the quadrants above describe the serialize-side contract. + /// On the deserialize side serde is lenient for option-typed fields — + /// a missing key is accepted as None even when `required` is true — + /// unless the field has a custom deserializer (see `deserialize_with`). + /// Generated clients therefore render option-typed fields as optional + /// keys regardless of `required`; consumers of this schema should not + /// assume `required` alone implies the key is present on the wire. + /// /// Default is false #[serde(skip_serializing_if = "is_false", default)] pub required: bool, diff --git a/reflectapi/src/codegen/python.rs b/reflectapi/src/codegen/python.rs index 2c984ed1..cbeba446 100644 --- a/reflectapi/src/codegen/python.rs +++ b/reflectapi/src/codegen/python.rs @@ -76,18 +76,21 @@ struct PythonMetadataUsage { /// /// Key presence comes from the shared wire-contract rules; this function only /// maps the contract to Python rendering. Option-typed fields already carry -/// `| None` in their type hint via the type mapping, so `nullable` controls -/// whether it must be appended for fields that may be absent. +/// `| None` in their type hint via the type mapping, so `declared_nullable` +/// controls whether it must be appended for fields that may be absent. fn resolve_field_optionality( - field: &reflectapi_schema::Field, + facts: schema_codegen::FieldWireFacts<'_>, field_type: String, - effective_required: bool, ) -> (bool, Option, String) { - let contract = schema_codegen::resolve_field_wire_contract(field, effective_required); - match (contract.key, contract.nullable) { + let contract = schema_codegen::resolve_field_wire_contract(facts); + match (contract.key, contract.declared_nullable) { (schema_codegen::KeyPresence::Optional, true) => { (true, Some("None".to_string()), field_type) } + // The wire never carries null for these fields (the key is simply + // absent), but pydantic has no idiomatic "may be absent yet never + // null" without a sentinel type, so absence is modeled as a None + // default — the long-standing rendering choice for this quadrant. (schema_codegen::KeyPresence::Optional, false) => ( true, Some("None".to_string()), @@ -97,6 +100,13 @@ fn resolve_field_optionality( } } +/// A field rendered on `ReflectapiPartialModel`: absence must stay +/// distinguishable from an explicit null (`model_fields_set` semantics). +fn is_partial_field(field: &reflectapi_schema::Field) -> bool { + schema_codegen::resolve_field_wire_contract(schema_codegen::FieldWireFacts::of(field)) + .absent_distinct_from_null +} + struct PythonTypeMapping { type_hint: &'static str, imports: &'static [&'static str], @@ -755,7 +765,7 @@ fn render_struct_with_flattened_internal_enum( used_type_vars, )?; let (optional, default_value, final_type) = - resolve_field_optionality(field, field_type, field.required); + resolve_field_optionality(schema_codegen::FieldWireFacts::of(field), field_type); base_fields.push(templates::Field { name: python_name, type_annotation: final_type, @@ -765,7 +775,7 @@ fn render_struct_with_flattened_internal_enum( default_value, alias, - is_partial: field.type_ref.name == "reflectapi::Option", + is_partial: is_partial_field(field), }); } @@ -804,8 +814,10 @@ fn render_struct_with_flattened_internal_enum( )?; let (python_name, alias) = sanitize_field_name_with_alias(field.name(), field.serde_name()); - let (optional, default_value, final_type) = - resolve_field_optionality(field, field_type, field.required); + let (optional, default_value, final_type) = resolve_field_optionality( + schema_codegen::FieldWireFacts::of(field), + field_type, + ); base_fields.push(templates::Field { name: python_name, type_annotation: final_type, @@ -815,7 +827,7 @@ fn render_struct_with_flattened_internal_enum( default_value, alias, - is_partial: field.type_ref.name == "reflectapi::Option", + is_partial: is_partial_field(field), }); } } @@ -855,8 +867,10 @@ fn render_struct_with_flattened_internal_enum( active_generics, used_type_vars, )?; - let (optional, default_value, final_type) = - resolve_field_optionality(vf, field_type, vf.required); + let (optional, default_value, final_type) = resolve_field_optionality( + schema_codegen::FieldWireFacts::of(vf), + field_type, + ); let (sanitized, alias) = sanitize_field_name_with_alias(vf.name(), vf.serde_name()); fields.push(templates::Field { @@ -868,7 +882,7 @@ fn render_struct_with_flattened_internal_enum( default_value, alias, - is_partial: vf.type_ref.name == "reflectapi::Option", + is_partial: is_partial_field(vf), }); } } @@ -917,8 +931,10 @@ fn render_struct_with_flattened_internal_enum( )?; let (sanitized, alias) = sanitize_field_name_with_alias(sf.name(), sf.serde_name()); - let (optional, default_value, final_type) = - resolve_field_optionality(sf, field_type, sf.required); + let (optional, default_value, final_type) = resolve_field_optionality( + schema_codegen::FieldWireFacts::of(sf), + field_type, + ); fields.push(templates::Field { name: sanitized, type_annotation: final_type, @@ -928,7 +944,7 @@ fn render_struct_with_flattened_internal_enum( default_value, alias, - is_partial: sf.type_ref.name == "reflectapi::Option", + is_partial: is_partial_field(sf), }); } } @@ -1002,7 +1018,7 @@ fn render_struct_with_flatten_standard( )?; let (optional, default_value, final_type) = - resolve_field_optionality(field, field_type, field.required); + resolve_field_optionality(schema_codegen::FieldWireFacts::of(field), field_type); all_fields.push(templates::Field { name: python_name, type_annotation: final_type, @@ -1012,7 +1028,7 @@ fn render_struct_with_flatten_standard( default_value, alias, - is_partial: field.type_ref.name == "reflectapi::Option", + is_partial: is_partial_field(field), }); } @@ -1097,13 +1113,10 @@ fn strip_phantom_data_fields(schema: &mut Schema) { fn schema_has_partial_field(schema: &Schema) -> bool { fn iter_typespace_has_partial(typespace: &reflectapi_schema::Typespace) -> bool { typespace.types().any(|t| match t { - reflectapi_schema::Type::Struct(s) => { - s.fields().any(|f| f.type_ref.name == "reflectapi::Option") + reflectapi_schema::Type::Struct(s) => s.fields().any(is_partial_field), + reflectapi_schema::Type::Enum(e) => { + e.variants.iter().any(|v| v.fields().any(is_partial_field)) } - reflectapi_schema::Type::Enum(e) => e - .variants - .iter() - .any(|v| v.fields().any(|f| f.type_ref.name == "reflectapi::Option")), _ => false, }) } @@ -3648,8 +3661,10 @@ fn make_flattened_field( used_type_vars, )?; - let (optional, default_value, final_field_type) = - resolve_field_optionality(field, field_type, field.required && parent_required); + let (optional, default_value, final_field_type) = resolve_field_optionality( + schema_codegen::FieldWireFacts::of(field).with_required(field.required && parent_required), + field_type, + ); let (sanitized, alias) = sanitize_field_name_with_alias(field.name(), field.serde_name()); Ok(templates::Field { @@ -3669,7 +3684,7 @@ fn make_flattened_field( default_value, alias, - is_partial: field.type_ref.name == "reflectapi::Option", + is_partial: is_partial_field(field), }) } @@ -3714,17 +3729,17 @@ fn collect_flattened_enum_fields( }); let (sanitized, alias) = sanitize_field_name_with_alias(&field_name, &field_name); - // A flattened enum field has no schema `Field` of its own; synthesize one - // for the shared wire-contract resolution. serde rejects `flatten` - // combined with `with`-family attributes, so no serializer or - // deserializer flag can be lost here. - let synthetic_field = { - let mut synthetic = reflectapi_schema::Field::new(field_name.clone(), type_ref.clone()); - synthetic.required = parent_required; - synthetic - }; - let (optional, default_value, final_type) = - resolve_field_optionality(&synthetic_field, enum_python_type, parent_required); + // A flattened enum field has no schema `Field` of its own; assert its + // wire facts directly. serde rejects `flatten` combined with + // `with`-family attributes, so there is no deserializer flag to carry. + let (optional, default_value, final_type) = resolve_field_optionality( + schema_codegen::FieldWireFacts { + type_name: &type_ref.name, + deserialize_with: false, + required: parent_required, + }, + enum_python_type, + ); collected_fields.push(templates::Field { name: sanitized, @@ -3788,8 +3803,10 @@ fn render_struct( &active_generics, used_type_vars, )?; - let (_, _, field_type) = - resolve_field_optionality(field, base_field_type, field.required); + let (_, _, field_type) = resolve_field_optionality( + schema_codegen::FieldWireFacts::of(field), + base_field_type, + ); Ok(field_type) }) .collect::, anyhow::Error>>()?; @@ -3823,8 +3840,10 @@ fn render_struct( used_type_vars, )?; - let (optional, default_value, field_type) = - resolve_field_optionality(field, base_field_type, field.required); + let (optional, default_value, field_type) = resolve_field_optionality( + schema_codegen::FieldWireFacts::of(field), + base_field_type, + ); let (sanitized, alias) = sanitize_field_name_with_alias(field.name(), field.serde_name()); @@ -3837,7 +3856,7 @@ fn render_struct( default_value, alias, - is_partial: field.type_ref.name == "reflectapi::Option", + is_partial: is_partial_field(field), }) }) .collect::, anyhow::Error>>()?; @@ -4077,8 +4096,10 @@ fn render_adjacently_tagged_enum( &generic_params, used_type_vars, )?; - let (optional, default_value, final_field_type) = - resolve_field_optionality(field, field_type, field.required); + let (optional, default_value, final_field_type) = resolve_field_optionality( + schema_codegen::FieldWireFacts::of(field), + field_type, + ); let (sanitized, alias) = sanitize_field_name_with_alias(field.name(), field.serde_name()); content_fields.push(templates::Field { @@ -4090,7 +4111,7 @@ fn render_adjacently_tagged_enum( default_value, alias, - is_partial: field.type_ref.name == "reflectapi::Option", + is_partial: is_partial_field(field), }); } let content_model = templates::DataClass { @@ -4252,7 +4273,7 @@ fn render_externally_tagged_enum( default_value: None, alias: None, - is_partial: field.type_ref.name == "reflectapi::Option", + is_partial: is_partial_field(field), }); } @@ -4318,8 +4339,10 @@ fn render_externally_tagged_enum( used_type_vars, )?; - let (optional, default_value, final_field_type) = - resolve_field_optionality(field, field_type, field.required); + let (optional, default_value, final_field_type) = resolve_field_optionality( + schema_codegen::FieldWireFacts::of(field), + field_type, + ); let (sanitized, alias) = sanitize_field_name_with_alias(field.name(), field.serde_name()); @@ -4332,7 +4355,7 @@ fn render_externally_tagged_enum( default_value, alias, - is_partial: field.type_ref.name == "reflectapi::Option", + is_partial: is_partial_field(field), }); } @@ -4556,9 +4579,8 @@ fn render_internally_tagged_enum( let (optional, default_value, final_field_type) = resolve_field_optionality( - struct_field, + schema_codegen::FieldWireFacts::of(struct_field), field_type, - struct_field.required, ); let (sanitized, alias) = sanitize_field_name_with_alias( @@ -4574,7 +4596,7 @@ fn render_internally_tagged_enum( default_value, alias, - is_partial: struct_field.type_ref.name == "reflectapi::Option", + is_partial: is_partial_field(struct_field), }); } } @@ -4599,7 +4621,7 @@ fn render_internally_tagged_enum( default_value: None, alias: None, - is_partial: inner_field.type_ref.name == "reflectapi::Option", + is_partial: is_partial_field(inner_field), }); } } @@ -4616,8 +4638,10 @@ fn render_internally_tagged_enum( used_type_vars, )?; - let (optional, default_value, final_field_type) = - resolve_field_optionality(field, field_type, field.required); + let (optional, default_value, final_field_type) = resolve_field_optionality( + schema_codegen::FieldWireFacts::of(field), + field_type, + ); let (sanitized, alias) = sanitize_field_name_with_alias(field.name(), field.serde_name()); @@ -4630,7 +4654,7 @@ fn render_internally_tagged_enum( default_value, alias, - is_partial: field.type_ref.name == "reflectapi::Option", + is_partial: is_partial_field(field), }); } } @@ -4761,8 +4785,10 @@ fn render_untagged_enum( &generic_params, used_type_vars, )?; - let (optional, default_value, final_field_type) = - resolve_field_optionality(field, field_type, field.required); + let (optional, default_value, final_field_type) = resolve_field_optionality( + schema_codegen::FieldWireFacts::of(field), + field_type, + ); let (sanitized, alias) = sanitize_field_name_with_alias(field.name(), field.serde_name()); @@ -4775,7 +4801,7 @@ fn render_untagged_enum( default_value, alias, - is_partial: field.type_ref.name == "reflectapi::Option", + is_partial: is_partial_field(field), }); } } @@ -4790,8 +4816,10 @@ fn render_untagged_enum( &generic_params, used_type_vars, )?; - let (optional, default_value, final_field_type) = - resolve_field_optionality(field, field_type, field.required); + let (optional, default_value, final_field_type) = resolve_field_optionality( + schema_codegen::FieldWireFacts::of(field), + field_type, + ); fields.push(templates::Field { name: if unnamed_fields.len() == 1 { @@ -4806,7 +4834,7 @@ fn render_untagged_enum( default_value, alias: None, - is_partial: field.type_ref.name == "reflectapi::Option", + is_partial: is_partial_field(field), }); } } diff --git a/reflectapi/src/codegen/schema/mod.rs b/reflectapi/src/codegen/schema/mod.rs index 4e5308be..c98c2469 100644 --- a/reflectapi/src/codegen/schema/mod.rs +++ b/reflectapi/src/codegen/schema/mod.rs @@ -8,7 +8,7 @@ mod ids; mod normalize; -mod presence; +pub(crate) mod presence; mod semantic; mod symbol; @@ -17,7 +17,7 @@ mod symbol; // `self::ids::*`, `self::normalize::*`, etc. for the module's own tests. pub(crate) use self::ids::{build_schema_ids, SchemaIds}; pub(crate) use self::normalize::{Consolidation, Naming, Normalizer, PipelineBuilder}; -pub(crate) use self::presence::{resolve_field_wire_contract, KeyPresence}; +pub(crate) use self::presence::{resolve_field_wire_contract, FieldWireFacts, KeyPresence}; pub(crate) use self::semantic::{ FieldStyle, ResolvedTypeReference, SemanticEnum, SemanticField, SemanticFunction, SemanticOutputType, SemanticPrimitive, SemanticSchema, SemanticStruct, SemanticType, diff --git a/reflectapi/src/codegen/schema/presence.rs b/reflectapi/src/codegen/schema/presence.rs index 4fa33424..b7d575e9 100644 --- a/reflectapi/src/codegen/schema/presence.rs +++ b/reflectapi/src/codegen/schema/presence.rs @@ -2,32 +2,64 @@ //! //! A field's wire behavior decomposes into three orthogonal facts, resolved //! here once so that every backend renders from the same meaning instead of -//! re-deriving it from `(required, type name, custom_codec)` heuristics: +//! re-deriving it from `(required, type name, deserialize_with)` heuristics: //! //! - **key presence** — may the key be absent on the wire? -//! - **value nullability** — may the value be `null`? +//! - **declared nullability** — does the declared type admit `null`? //! - **absence semantics** — is "absent" distinct from "null"? //! -//! The resolution rules, with the serde behavior that justifies them: +//! The key-presence rules, with the serde deserialize behavior that +//! justifies them: //! //! | Field shape | Missing key behavior (deserialize) | //! |---|---| //! | `T` | rejected — key required | //! | `Option`, no attrs | accepted as `None` — `missing_field` special-cases `deserialize_option` | -//! | `reflectapi::Option`, no `default` | accepted as `None` (collapses the three-state type; a lint may catch this in future) | +//! | `reflectapi::Option`, no `default` | accepted as `None` (collapses the three-state type to two) | //! | `reflectapi::Option` + `default` | accepted as `Undefined` | //! | any + `serde(default)` | accepted — default value used | //! | option + `with`/`deserialize_with`, no `default` | **rejected** — `missing_field` cannot route through a custom deserializer | //! //! Only the deserialize side affects key presence: a custom *serializer* -//! never changes whether the key may be absent (`skip_serializing_if`, -//! folded into `required`, controls that), so `serialize_with` is -//! irrelevant here. On the serialize side the folded `required` flag -//! already captures whether the key can be absent, and option types -//! capture nullability. +//! never changes whether the key may be absent — `skip_serializing_if`, +//! already folded into `required`, controls that. use reflectapi_schema::Field; +/// The minimal facts key-presence resolution depends on. +/// +/// Constructed from a [`Field`] via [`FieldWireFacts::of`]; callers that +/// have no schema field (e.g. synthesized flattened-enum fields) construct +/// it directly, making explicit exactly which facts they assert. +#[derive(Debug, Clone, Copy)] +pub(crate) struct FieldWireFacts<'a> { + /// Fully qualified name of the field's declared type. + pub type_name: &'a str, + /// The field has a custom serde deserializer + /// (`#[serde(deserialize_with)]` or `#[serde(with)]`). + pub deserialize_with: bool, + /// Folded serde requiredness for the rendering context: absence of + /// `#[serde(default)]` on the deserialize side, absence of + /// `skip_serializing_if` on the serialize side. + pub required: bool, +} + +impl<'a> FieldWireFacts<'a> { + pub fn of(field: &'a Field) -> Self { + Self { + type_name: &field.type_ref.name, + deserialize_with: field.deserialize_with, + required: field.required, + } + } + + /// Override requiredness, e.g. when a flattened parent's own + /// requiredness folds into its expanded fields. + pub fn with_required(self, required: bool) -> Self { + Self { required, ..self } + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum KeyPresence { /// The key must be present on the wire. @@ -39,32 +71,26 @@ pub(crate) enum KeyPresence { #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) struct FieldWireContract { pub key: KeyPresence, - /// The value may be `null` on the wire (option-typed field). - pub nullable: bool, + /// The declared type admits `null` (option-typed field). Whether `null` + /// actually appears on the wire is direction-dependent: an output field + /// with `skip_serializing_if` omits the key instead of emitting `null`. + pub declared_nullable: bool, /// Absence carries distinct meaning from an explicit `null` /// (three-state `reflectapi::Option`). pub absent_distinct_from_null: bool, } -/// Resolve a field's wire contract. -/// -/// `effective_required` is the folded serde requiredness for the rendering -/// context — usually `field.required`, but callers expanding flattened -/// fields combine it with the parent's requiredness. -pub(crate) fn resolve_field_wire_contract( - field: &Field, - effective_required: bool, -) -> FieldWireContract { - let is_std_option = field.type_ref.name == "std::option::Option"; - let is_reflect_option = field.type_ref.name == "reflectapi::Option"; +pub(crate) fn resolve_field_wire_contract(facts: FieldWireFacts<'_>) -> FieldWireContract { + let is_std_option = facts.type_name == "std::option::Option"; + let is_reflect_option = facts.type_name == "reflectapi::Option"; let is_option = is_std_option || is_reflect_option; - let key = if !effective_required { + let key = if !facts.required { // serde(default) / skip_serializing_if: absence is always fine, // custom deserializer or not (a default is applied without // invoking it). KeyPresence::Optional - } else if is_option && !field.deserialize_with { + } else if is_option && !facts.deserialize_with { // serde accepts a missing key for plain option-typed fields even // without serde(default); a custom deserializer breaks that path, // so `required` stays authoritative there. @@ -75,7 +101,7 @@ pub(crate) fn resolve_field_wire_contract( FieldWireContract { key, - nullable: is_option, + declared_nullable: is_option, absent_distinct_from_null: is_reflect_option, } } @@ -83,71 +109,80 @@ pub(crate) fn resolve_field_wire_contract( #[cfg(test)] mod tests { use super::*; - use reflectapi_schema::TypeReference; - fn field(type_name: &str, required: bool) -> Field { - let argument = TypeReference::new("u8".to_string(), vec![]); - let mut f = Field::new( - "f".to_string(), - TypeReference::new(type_name.to_string(), vec![argument]), - ); - f.required = required; - f + fn facts(type_name: &str, required: bool) -> FieldWireFacts<'_> { + FieldWireFacts { + type_name, + deserialize_with: false, + required, + } } #[test] fn plain_required_field_is_required() { - let c = resolve_field_wire_contract(&field("u8", true), true); + let c = resolve_field_wire_contract(facts("u8", true)); assert_eq!(c.key, KeyPresence::Required); - assert!(!c.nullable); + assert!(!c.declared_nullable); assert!(!c.absent_distinct_from_null); } #[test] fn std_option_is_optional_even_when_required() { - let c = resolve_field_wire_contract(&field("std::option::Option", true), true); + let c = resolve_field_wire_contract(facts("std::option::Option", true)); assert_eq!(c.key, KeyPresence::Optional); - assert!(c.nullable); + assert!(c.declared_nullable); assert!(!c.absent_distinct_from_null); } #[test] fn reflectapi_option_is_optional_and_three_state() { - let c = resolve_field_wire_contract(&field("reflectapi::Option", true), true); + let c = resolve_field_wire_contract(facts("reflectapi::Option", true)); assert_eq!(c.key, KeyPresence::Optional); - assert!(c.nullable); + assert!(c.declared_nullable); assert!(c.absent_distinct_from_null); } #[test] fn custom_deserializer_keeps_required_authoritative() { - let mut f = field("std::option::Option", true); - f.deserialize_with = true; - let c = resolve_field_wire_contract(&f, true); + let c = resolve_field_wire_contract(FieldWireFacts { + deserialize_with: true, + ..facts("std::option::Option", true) + }); assert_eq!(c.key, KeyPresence::Required); - assert!(c.nullable); + assert!(c.declared_nullable); } #[test] - fn custom_serializer_alone_does_not_affect_key_presence() { - let mut f = field("std::option::Option", true); - f.serialize_with = true; - let c = resolve_field_wire_contract(&f, true); + fn custom_deserializer_with_serde_default_is_optional() { + let c = resolve_field_wire_contract(FieldWireFacts { + deserialize_with: true, + ..facts("std::option::Option", false) + }); assert_eq!(c.key, KeyPresence::Optional); } #[test] - fn custom_deserializer_with_serde_default_is_optional() { - let mut f = field("std::option::Option", false); - f.deserialize_with = true; - let c = resolve_field_wire_contract(&f, false); + fn non_required_non_option_is_optional_not_nullable() { + let c = resolve_field_wire_contract(facts("u8", false)); assert_eq!(c.key, KeyPresence::Optional); + assert!(!c.declared_nullable); } #[test] - fn non_required_non_option_is_optional_not_nullable() { - let c = resolve_field_wire_contract(&field("u8", false), false); + fn facts_of_field_carries_deserialize_with_only() { + // A custom serializer must not affect key presence; `of` therefore + // only reads the deserialize-side flag. + let mut field = reflectapi_schema::Field::new( + "f".to_string(), + reflectapi_schema::TypeReference::new("std::option::Option".to_string(), vec![]), + ); + field.required = true; + field.serialize_with = true; + let c = resolve_field_wire_contract(FieldWireFacts::of(&field)); assert_eq!(c.key, KeyPresence::Optional); - assert!(!c.nullable); + + field.deserialize_with = true; + let c = resolve_field_wire_contract(FieldWireFacts::of(&field)); + assert_eq!(c.key, KeyPresence::Required); } } diff --git a/reflectapi/src/codegen/typescript.rs b/reflectapi/src/codegen/typescript.rs index 7015c6e5..7b455747 100644 --- a/reflectapi/src/codegen/typescript.rs +++ b/reflectapi/src/codegen/typescript.rs @@ -1102,7 +1102,9 @@ fn field_to_ts_field( // key is accepted for plain option-typed fields, but not when a custom // serde deserializer is involved); nullability is already carried by // the type mapping (`T | null`, `T | null | undefined`). - let contract = crate::codegen::schema::resolve_field_wire_contract(field, field.required); + let contract = crate::codegen::schema::resolve_field_wire_contract( + crate::codegen::schema::FieldWireFacts::of(field), + ); templates::Field { name: field.serde_name().into(), description: doc_to_ts_comments(&field.description, field.deprecation_note.as_deref(), 4),