From 3c89ab705a5d7cc0128c448fe1f7b5ef242d7836 Mon Sep 17 00:00:00 2001 From: Anton Zherdev Date: Thu, 5 Nov 2026 10:28:23 +0000 Subject: [PATCH 1/5] fix(codegen): responses may omit deprecated fields A server removes a deprecated field once its sunset passes, and a client generated while the field was deprecated then failed to decode every response, although it never read the field. A schema transform in codegen/schema now makes deprecated fields of response-only types optional, and every generator runs it: Python reads such a field as None and types a non-Option field T | None, TypeScript marks it optional, the Rust client gives it a serde default, and OpenAPI leaves it out of required. Types also used in requests keep the field required, since the server may still require it, and so do untagged enum variants, where an optional field could change which variant matches. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Q7zEtTUpmxtYWDkuq6uwpY --- CHANGELOG.md | 4 + ...asic__reflectapi_deprecated_headers-2.snap | 2 +- ...asic__reflectapi_deprecated_headers-3.snap | 6 +- ...asic__reflectapi_deprecated_headers-5.snap | 9 +- ...basic__reflectapi_deprecation_dates-2.snap | 12 +- ...basic__reflectapi_deprecation_dates-3.snap | 6 + ...basic__reflectapi_deprecation_dates-4.snap | 8 +- ...basic__reflectapi_deprecation_dates-5.snap | 39 +++++-- reflectapi-demo/tests/field_presence.rs | 52 +++++++++ .../tests/python/test_field_presence.py | 38 +++++++ .../rust/field_presence/tests/presence.rs | 12 +- .../tests/typescript/field-presence.ts | 6 + reflectapi/src/codegen/openapi.rs | 5 +- reflectapi/src/codegen/python.rs | 1 + reflectapi/src/codegen/rust.rs | 1 + reflectapi/src/codegen/schema/deprecation.rs | 107 ++++++++++++++++++ reflectapi/src/codegen/schema/mod.rs | 2 + reflectapi/src/codegen/typescript.rs | 1 + 18 files changed, 285 insertions(+), 26 deletions(-) create mode 100644 reflectapi/src/codegen/schema/deprecation.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index ac097ea8..d2a724a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ - A generated Python module with an omittable field of a unit type, `()` or a unit struct, imports again. The field was annotated `None | None`, which Python can't evaluate, so importing the module raised `TypeError`. It is now `None`. - A generated Rust client compiles when an optional field's type is a generated struct or enum without `Default`, such as a `#[serde(default)] memo: Memo` whose `Memo` implements `Default` only on the server. The field got `#[serde(default)]`, which needs `Default` on the generated type, and generated types derive it only when configured to. Such a field is now `Option`: `None` leaves the key out and the server applies its default. Fields whose generated type derives `Default`, through `base_derives` or the type's `additional_derives`, are unchanged. +### Changed + +- Generated clients decode responses after the server removes a deprecated field. Every generator now treats a deprecated field of a response-only type as optional: Python reads it as `None` and types a non-`Option` field `T | None`, TypeScript marks it `?:`, the Rust client gives it `#[serde(default)]` (an `Option` when its type has no `Default`), and OpenAPI leaves it out of `required`. Before, a client generated while the field was deprecated failed to decode every response once the server dropped it, although the client never read it. TypeScript and Rust code that reads such a field must now handle its absence. Types also used in requests keep deprecated fields required, because the server may still require them until it removes them, and so do variants of untagged enums, where an optional field could make the client pick a different variant than serde. + ## 0.23.0 Keep idle stream routes open through proxies and load balancers: a server can now write a Server-Sent Events comment whenever a stream has sent nothing for an interval, set for the whole server or per route. See [Keep idle streams open](https://thepartly.github.io/reflectapi/serving/axum.html#keep-idle-streams-open). Generated Python models also describe their wire shape in their JSON schemas. `reflectapi::axum::into_router` takes a new settings argument, and Python clients need the matching runtime; see Breaking. diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecated_headers-2.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecated_headers-2.snap index cc4c2535..f3e6d6dc 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecated_headers-2.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecated_headers-2.snap @@ -52,7 +52,7 @@ export namespace reflectapi_demo { * @deprecated Deprecated since 2026-10-01; removal after 2026-11-01. Read `x-request-id` instead. * The old request id */ - "x-legacy-request-id": string | null; + "x-legacy-request-id"?: string | null; "x-request-id": string | null; } } diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecated_headers-3.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecated_headers-3.snap index 739cbe0d..13466214 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecated_headers-3.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecated_headers-3.snap @@ -91,7 +91,11 @@ pub mod types { #[derive(Debug, serde::Deserialize)] pub struct ResponseHeaders { /// The old request id - #[serde(rename = "x-legacy-request-id")] + #[serde( + rename = "x-legacy-request-id", + default = "Default::default", + skip_serializing_if = "std::option::Option::is_none" + )] #[deprecated( note = "Deprecated since 2026-10-01; removal after 2026-11-01. Read `x-request-id` instead." )] diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecated_headers-5.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecated_headers-5.snap index 049d7e4f..db4edc6a 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecated_headers-5.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecated_headers-5.snap @@ -45,12 +45,17 @@ class ReflectapiDemoTestsBasicRequestHeaders(ReflectapiPartialModel): ) -class ReflectapiDemoTestsBasicResponseHeaders(BaseModel): +class ReflectapiDemoTestsBasicResponseHeaders(ReflectapiPartialModel): model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=(), defer_build=True + extra="ignore", + populate_by_name=True, + validate_assignment=True, + protected_namespaces=(), + defer_build=True, ) x_legacy_request_id: str | None = Field( + default=None, serialization_alias="x-legacy-request-id", validation_alias="x-legacy-request-id", description="The old request id", diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecation_dates-2.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecation_dates-2.snap index ffe93326..8fa16f6b 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecation_dates-2.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecation_dates-2.snap @@ -106,29 +106,29 @@ export namespace reflectapi_demo { /** * @deprecated Deprecated since 2026-10-01; removal after 2026-11-01. Use `current` instead. */ - _since_and_sunset: number /* u8 */; + _since_and_sunset?: number /* u8 */; /** * @deprecated Deprecated since 2026-10-01. */ - _since_only: number /* u8 */; + _since_only?: number /* u8 */; /** * @deprecated Removal after 2026-11-01. */ - _sunset_only: number /* u8 */ | null; + _sunset_only?: number /* u8 */ | null; /** * @deprecated Deprecated in 1.2.0. Versioned. * rustc's `since` is the release, kept verbatim. */ - _release_only: number /* u8 */; + _release_only?: number /* u8 */; /** * @deprecated Deprecated in 2026-10-01. * It is never read as a date, even when it looks like one. */ - _release_like_a_date: number /* u8 */; + _release_like_a_date?: number /* u8 */; /** * @deprecated Deprecated in 2.3.0, since 2026-10-01; removal after 2026-11-01. Use `current` instead. */ - _release_and_dates: number /* u8 */; + _release_and_dates?: number /* u8 */; _current: reflectapi_demo.tests.basic.EnumWithDeprecatedVariants; } } diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecation_dates-3.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecation_dates-3.snap index a07b187a..a633414c 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecation_dates-3.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecation_dates-3.snap @@ -100,18 +100,24 @@ pub mod types { #[derive(Debug, serde::Deserialize)] pub struct StructWithDeprecationDates { + #[serde(default = "Default::default")] #[deprecated(note = "Deprecated since 2026-10-01; removal after 2026-11-01. Use `current` instead.")] pub _since_and_sunset: u8, + #[serde(default = "Default::default")] #[deprecated(note = "Deprecated since 2026-10-01.")] pub _since_only: u8, + #[serde(default = "Default::default", skip_serializing_if = "std::option::Option::is_none")] #[deprecated(note = "Removal after 2026-11-01.")] pub _sunset_only: std::option::Option, /// rustc's `since` is the release, kept verbatim. + #[serde(default = "Default::default")] #[deprecated(note = "Deprecated in 1.2.0. Versioned.")] pub _release_only: u8, /// It is never read as a date, even when it looks like one. + #[serde(default = "Default::default")] #[deprecated(note = "Deprecated in 2026-10-01.")] pub _release_like_a_date: u8, + #[serde(default = "Default::default")] #[deprecated(note = "Deprecated in 2.3.0, since 2026-10-01; removal after 2026-11-01. Use `current` instead.")] pub _release_and_dates: u8, pub _current: super::super::super::super::reflectapi_demo::tests::basic::EnumWithDeprecatedVariants, diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecation_dates-4.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecation_dates-4.snap index a621d38f..ff8d664a 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecation_dates-4.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecation_dates-4.snap @@ -159,13 +159,7 @@ expression: "reflectapi :: codegen :: openapi :: Spec :: from(& schema)" "type": "object", "title": "reflectapi_demo.tests.basic.output.StructWithDeprecationDates", "required": [ - "_current", - "_release_and_dates", - "_release_like_a_date", - "_release_only", - "_since_and_sunset", - "_since_only", - "_sunset_only" + "_current" ], "properties": { "_current": { diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecation_dates-5.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecation_dates-5.snap index 75002897..6cee51e1 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecation_dates-5.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecation_dates-5.snap @@ -22,6 +22,7 @@ from pydantic import ( ConfigDict, Field, RootModel, + field_validator, model_serializer, model_validator, ) @@ -186,39 +187,49 @@ class ReflectapiDemoTestsBasicInputStructWithDeprecationDates(ReflectapiPartialM ) -class ReflectapiDemoTestsBasicOutputStructWithDeprecationDates(BaseModel): +class ReflectapiDemoTestsBasicOutputStructWithDeprecationDates(ReflectapiPartialModel): model_config = ConfigDict( - extra="ignore", populate_by_name=True, protected_namespaces=(), defer_build=True + extra="ignore", + populate_by_name=True, + validate_assignment=True, + protected_namespaces=(), + defer_build=True, ) - since_and_sunset: int = Field( + since_and_sunset: int | None = Field( + default=None, serialization_alias="_since_and_sunset", validation_alias="_since_and_sunset", deprecated="Deprecated since 2026-10-01; removal after 2026-11-01. Use `current` instead.", ) - since_only: int = Field( + since_only: int | None = Field( + default=None, serialization_alias="_since_only", validation_alias="_since_only", deprecated="Deprecated since 2026-10-01.", ) sunset_only: int | None = Field( + default=None, serialization_alias="_sunset_only", validation_alias="_sunset_only", deprecated="Removal after 2026-11-01.", ) - release_only: int = Field( + release_only: int | None = Field( + default=None, serialization_alias="_release_only", validation_alias="_release_only", description="rustc's `since` is the release, kept verbatim.", deprecated="Deprecated in 1.2.0. Versioned.", ) - release_like_a_date: int = Field( + release_like_a_date: int | None = Field( + default=None, serialization_alias="_release_like_a_date", validation_alias="_release_like_a_date", description="It is never read as a date, even when it looks like one.", deprecated="Deprecated in 2026-10-01.", ) - release_and_dates: int = Field( + release_and_dates: int | None = Field( + default=None, serialization_alias="_release_and_dates", validation_alias="_release_and_dates", deprecated="Deprecated in 2.3.0, since 2026-10-01; removal after 2026-11-01. Use `current` instead.", @@ -227,6 +238,20 @@ class ReflectapiDemoTestsBasicOutputStructWithDeprecationDates(BaseModel): serialization_alias="_current", validation_alias="_current" ) + @field_validator( + "since_and_sunset", + "since_only", + "release_only", + "release_like_a_date", + "release_and_dates", + mode="before", + ) + @classmethod + def _reflectapi_reject_null(cls, value): + if value is None: + raise ValueError("Field may be omitted but cannot be null") + return value + # Namespace classes for dotted access to types class reflectapi_demo: diff --git a/reflectapi-demo/tests/field_presence.rs b/reflectapi-demo/tests/field_presence.rs index 06805435..ebac6271 100644 --- a/reflectapi-demo/tests/field_presence.rs +++ b/reflectapi-demo/tests/field_presence.rs @@ -1,5 +1,6 @@ //! Directional field-presence fixtures shared by the generated SDK tests. +#[allow(deprecated)] pub mod fixtures { fn deserialize_nullable<'de, D: serde::Deserializer<'de>>( deserializer: D, @@ -155,6 +156,31 @@ pub mod fixtures { pub text: String, } + #[derive(serde::Serialize, serde::Deserialize, reflectapi::Output)] + pub struct Config { + pub level: u32, + } + + // Response-only: a newer server may have removed the deprecated fields. + #[derive(serde::Serialize, serde::Deserialize, reflectapi::Output)] + pub struct Profile { + pub id: u32, + #[deprecated(note = "use id")] + pub legacy_tags: Option>, + #[deprecated(note = "use id")] + pub legacy_code: String, + #[deprecated(note = "use id")] + pub legacy_config: Config, + } + + // Used in both directions, so a deprecated field stays required. + #[derive(serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output)] + pub struct Shared { + pub id: u32, + #[deprecated(note = "use id")] + pub old: String, + } + // Both directions agree when the serializer also omits absent values. #[derive(serde::Serialize, serde::Deserialize, reflectapi::Input, reflectapi::Output)] pub struct Equal { @@ -163,6 +189,10 @@ pub mod fixtures { } } +async fn profile(_: (), _: reflectapi::Empty, _: reflectapi::Empty) -> fixtures::Profile { + unimplemented!("schema-only route") +} + fn schema() -> reflectapi::Schema { reflectapi::Builder::<()>::new() .name("Field presence") @@ -206,6 +236,11 @@ fn schema() -> reflectapi::Schema { |_: (), input: fixtures::Defaulted, _: reflectapi::Empty| async { input }, |b| b.name("fields.defaulted"), ) + .route(profile, |b| b.name("fields.profile")) + .route( + |_: (), input: fixtures::Shared, _: reflectapi::Empty| async { input }, + |b| b.name("fields.shared"), + ) .build() .unwrap() .0 @@ -413,3 +448,20 @@ fn openapi_uses_the_same_directional_required_keys() { && !keys.contains(&serde_json::json!("skipped")) })); } + +#[test] +fn openapi_lets_responses_omit_deprecated_fields() { + let spec = reflectapi::codegen::openapi::generate(&schema(), &Default::default()).unwrap(); + let spec: serde_json::Value = serde_json::from_str(&spec).unwrap(); + let required = |suffix: &str| { + let (_, definition) = spec["components"]["schemas"] + .as_object() + .unwrap() + .iter() + .find(|(name, _)| name.ends_with(suffix)) + .unwrap(); + definition["required"].as_array().unwrap().clone() + }; + assert_eq!(required(".Profile"), vec![serde_json::json!("id")]); + assert!(required(".Shared").contains(&serde_json::json!("old"))); +} diff --git a/reflectapi-demo/tests/python/test_field_presence.py b/reflectapi-demo/tests/python/test_field_presence.py index 439e7079..c2918fa3 100644 --- a/reflectapi-demo/tests/python/test_field_presence.py +++ b/reflectapi-demo/tests/python/test_field_presence.py @@ -18,6 +18,8 @@ from null_contract_client.field_presence.fixtures import Context, Create # noqa: E402 from presence_client.field_presence.fixtures import ( # noqa: E402 Equal, + Profile, + Shared, Tuple, input, output, @@ -258,3 +260,39 @@ def test_omittable_field_of_a_nullable_type_accepts_null(field: str) -> None: # an Option, JSON values and custom deserializers may all read null. create = Create.model_validate({field: None}) assert create.model_dump(by_alias=True, exclude_unset=True) == {field: None} + + +def test_response_may_omit_deprecated_fields() -> None: + # A server that removed its deprecated fields still decodes in a client + # generated before the removal. + profile = Profile.model_validate({"id": 1}) + with pytest.warns(DeprecationWarning): + assert profile.legacy_tags is None + with pytest.warns(DeprecationWarning): + assert profile.legacy_code is None + with pytest.warns(DeprecationWarning): + assert profile.legacy_config is None + # Re-serializing keeps the server's shape: what it omitted stays omitted. + assert profile.model_dump(by_alias=True) == {"id": 1} + full = Profile.model_validate( + { + "id": 1, + "legacy_tags": {"a": 1}, + "legacy_code": "x", + "legacy_config": {"level": 2}, + } + ) + assert full.model_dump(by_alias=True) == { + "id": 1, + "legacy_tags": {"a": 1}, + "legacy_code": "x", + "legacy_config": {"level": 2}, + } + + +def test_request_still_requires_deprecated_fields() -> None: + # The server may still require the field, so a model sent as a request + # keeps it required. + with pytest.raises(ValidationError): + Shared.model_validate({"id": 1}) + Shared.model_validate({"id": 1, "old": "x"}) diff --git a/reflectapi-demo/tests/rust/field_presence/tests/presence.rs b/reflectapi-demo/tests/rust/field_presence/tests/presence.rs index 48ad75a3..8be47991 100644 --- a/reflectapi-demo/tests/rust/field_presence/tests/presence.rs +++ b/reflectapi-demo/tests/rust/field_presence/tests/presence.rs @@ -1,4 +1,4 @@ -use reflectapi_field_presence_rust::types::field_presence::fixtures::{input, output, Tuple}; +use reflectapi_field_presence_rust::types::field_presence::fixtures::{input, output, Profile, Shared, Tuple}; use serde_json::json; #[test] @@ -60,3 +60,13 @@ fn defaulted_fields_of_generated_types_may_be_omitted() { assert!(empty.memo.is_none() && empty.boxed.is_none()); assert_eq!(serde_json::to_value(empty).unwrap(), json!({})); } + +#[test] +#[allow(deprecated)] +fn generated_responses_may_omit_deprecated_fields() { + let profile: Profile = serde_json::from_value(json!({"id": 1})).unwrap(); + assert_eq!(profile.legacy_tags, None); + assert_eq!(profile.legacy_code, ""); + assert!(profile.legacy_config.is_none()); + assert!(serde_json::from_value::(json!({"id": 1})).is_err()); +} diff --git a/reflectapi-demo/tests/typescript/field-presence.ts b/reflectapi-demo/tests/typescript/field-presence.ts index dd7b9e38..ecb42dea 100644 --- a/reflectapi-demo/tests/typescript/field-presence.ts +++ b/reflectapi-demo/tests/typescript/field-presence.ts @@ -46,3 +46,9 @@ const nullDefault: field_presence.fixtures.input.Defaults = { count: null }; // @ts-expect-error a default on deserialization does not omit its output key. const missingDefault: field_presence.fixtures.output.Defaults = {}; void [defaults, nullDefault, missingDefault]; + +// A response may leave out a deprecated field once the server removes it. +const profile: field_presence.fixtures.Profile = { id: 1 }; +// @ts-expect-error a deprecated field of a type also sent as a request stays required. +const missingOld: field_presence.fixtures.Shared = { id: 1 }; +void [profile, missingOld]; diff --git a/reflectapi/src/codegen/openapi.rs b/reflectapi/src/codegen/openapi.rs index fbf98459..36a86ff8 100644 --- a/reflectapi/src/codegen/openapi.rs +++ b/reflectapi/src/codegen/openapi.rs @@ -56,12 +56,15 @@ pub fn generate(schema: &crate::Schema, config: &Config) -> anyhow::Result Spec { let mut schema = schema.clone(); schema.strip_hidden_fields(); + super::schema::relax_deprecated_response_fields(&mut schema); Converter::new(config).convert(&schema) } impl From<&crate::Schema> for Spec { fn from(schema: &crate::Schema) -> Self { - Converter::new(&Config::default()).convert(schema) + let mut schema = schema.clone(); + super::schema::relax_deprecated_response_fields(&mut schema); + Converter::new(&Config::default()).convert(&schema) } } diff --git a/reflectapi/src/codegen/python.rs b/reflectapi/src/codegen/python.rs index 8f2749dc..7cf5efd2 100644 --- a/reflectapi/src/codegen/python.rs +++ b/reflectapi/src/codegen/python.rs @@ -1994,6 +1994,7 @@ fn build_python_generation( // Python model doesn't reference a non-existent // `std.marker.PhantomData`. strip_phantom_data_fields(&mut schema); + schema_codegen::relax_deprecated_response_fields(&mut schema); // Consolidate input/output types FIRST so both the SemanticSchema and // the raw Schema share the same unified type names. diff --git a/reflectapi/src/codegen/rust.rs b/reflectapi/src/codegen/rust.rs index c0e3053a..43878c5b 100644 --- a/reflectapi/src/codegen/rust.rs +++ b/reflectapi/src/codegen/rust.rs @@ -140,6 +140,7 @@ fn types_referenced_by( pub fn generate(mut schema: crate::Schema, config: &Config) -> anyhow::Result { super::validate_response_enums(&schema)?; schema.strip_hidden_fields(); + super::schema::relax_deprecated_response_fields(&mut schema); let mut implemented_types = __build_implemented_types(); for type_def in schema .input_types() diff --git a/reflectapi/src/codegen/schema/deprecation.rs b/reflectapi/src/codegen/schema/deprecation.rs new file mode 100644 index 00000000..91fbc721 --- /dev/null +++ b/reflectapi/src/codegen/schema/deprecation.rs @@ -0,0 +1,107 @@ +//! Deprecated fields that a response may leave out. + +use reflectapi_schema::{Field, Representation, Schema, Type}; + +/// A server removes a deprecated field once its sunset passes, so a client +/// generated before that must decode responses without it. Only types that +/// appear in responses alone are relaxed: a request type keeps the field +/// required, because the server may still require it until it is removed. +/// Every generator runs this before consolidating the schema, so a type used +/// in both directions is never split by it, and all of them agree on which +/// keys a response may leave out. +pub(crate) fn relax_deprecated_response_fields(schema: &mut Schema) { + let response_only: Vec = schema + .output_types + .types() + .map(|t| t.name().to_string()) + .filter(|name| !schema.input_types.has_type(name)) + .collect(); + let relax = |field: &mut Field| { + if field.deprecation().is_some() + && field.required + && field.is_named() + && !field.flattened + && !field.deserialize_with + { + field.required = false; + } + }; + for name in response_only { + match schema.output_types.get_type_mut(&name) { + Some(Type::Struct(s)) => s.fields.iter_mut().for_each(relax), + // An untagged enum picks the first variant that matches, so an + // optional field could make an earlier variant match where serde + // would pick a later one. + Some(Type::Enum(e)) if e.representation != Representation::None => { + for variant in &mut e.variants { + variant.fields.iter_mut().for_each(relax); + } + } + _ => {} + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn deprecated_fields_of_response_only_types_may_be_absent() { + let field = |name: &str, deprecated: bool| { + let mut field = + serde_json::json!({"name": name, "required": true, "type": {"name": "u32"}}); + if deprecated { + field["deprecation_note"] = "gone soon".into(); + } + field + }; + let mut schema: Schema = serde_json::from_value(serde_json::json!({ + "name": "deprecated-responses", + "input_types": {"types": [ + {"kind": "primitive", "name": "u32"}, + {"kind": "struct", "name": "Both", "fields": {"named": [field("old", true)]}} + ]}, + "output_types": {"types": [ + {"kind": "primitive", "name": "u32"}, + {"kind": "struct", "name": "Both", "fields": {"named": [field("old", true)]}}, + {"kind": "struct", "name": "Reply", "fields": {"named": [field("old", true), field("kept", false)]}}, + {"kind": "enum", "name": "Event", "variants": [ + {"name": "Changed", "fields": {"named": [field("old", true), field("kept", false)]}} + ]}, + {"kind": "enum", "name": "Untagged", "representation": "none", "variants": [ + {"name": "Changed", "fields": {"named": [field("old", true), field("kept", false)]}} + ]} + ]} + })) + .unwrap(); + super::relax_deprecated_response_fields(&mut schema); + let required = |ts: &reflectapi_schema::Typespace, ty: &str| -> Vec<(String, bool)> { + let fields = match ts.get_type(ty).unwrap() { + Type::Struct(s) => s.fields.iter().collect::>(), + Type::Enum(e) => e.variants[0].fields.iter().collect(), + _ => unreachable!(), + }; + fields + .iter() + .map(|f| (f.name.clone(), f.required)) + .collect() + }; + assert_eq!( + required(&schema.output_types, "Reply"), + [("old".into(), false), ("kept".into(), true)] + ); + assert_eq!( + required(&schema.output_types, "Event"), + [("old".into(), false), ("kept".into(), true)] + ); + assert_eq!( + required(&schema.output_types, "Both"), + [("old".into(), true)] + ); + assert_eq!( + required(&schema.input_types, "Both"), + [("old".into(), true)] + ); + } +} diff --git a/reflectapi/src/codegen/schema/mod.rs b/reflectapi/src/codegen/schema/mod.rs index 9583d03a..9f338723 100644 --- a/reflectapi/src/codegen/schema/mod.rs +++ b/reflectapi/src/codegen/schema/mod.rs @@ -6,6 +6,7 @@ // short term. #![allow(dead_code)] +mod deprecation; mod ids; mod layout; mod normalize; @@ -18,6 +19,7 @@ mod symbol; // Public-to-the-crate surface. Only what backends (currently just Python) // actually consume is re-exported; everything else stays accessible as // `self::ids::*`, `self::normalize::*`, etc. for the module's own tests. +pub(crate) use self::deprecation::relax_deprecated_response_fields; pub(crate) use self::ids::{build_schema_ids, SchemaIds}; pub(crate) use self::layout::{FlattenedGroup, Layout, Member, Opaque}; pub(crate) use self::normalize::{Consolidation, Naming, Normalizer, PipelineBuilder}; diff --git a/reflectapi/src/codegen/typescript.rs b/reflectapi/src/codegen/typescript.rs index cfb94410..2d549274 100644 --- a/reflectapi/src/codegen/typescript.rs +++ b/reflectapi/src/codegen/typescript.rs @@ -48,6 +48,7 @@ pub fn generate( config: &Config, ) -> anyhow::Result> { schema.strip_hidden_fields(); + super::schema::relax_deprecated_response_fields(&mut schema); super::validate_response_enums(&schema)?; let implemented_types = build_implemented_types(); From ebf649c0090ccda7d2596b6ca7fd7b7079da8c2b Mon Sep 17 00:00:00 2001 From: Anton Zherdev Date: Thu, 5 Nov 2026 20:44:35 +0000 Subject: [PATCH 2/5] fix(codegen): keep deprecated fields required in types an untagged enum reaches An untagged enum picks the first variant that matches. Only fields declared on its variants were kept required, so a response-only struct a variant wraps was still relaxed, and a payload serde gives to a later variant could match the earlier one. Every type reachable from an untagged enum's variants, through fields and generic arguments, now keeps its deprecated fields required. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Q7zEtTUpmxtYWDkuq6uwpY --- CHANGELOG.md | 2 +- .../rust/field_presence/tests/presence.rs | 4 +- reflectapi/src/codegen/schema/deprecation.rs | 79 ++++++++++++++++--- 3 files changed, 72 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d2a724a6..7c211e8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ ### Changed -- Generated clients decode responses after the server removes a deprecated field. Every generator now treats a deprecated field of a response-only type as optional: Python reads it as `None` and types a non-`Option` field `T | None`, TypeScript marks it `?:`, the Rust client gives it `#[serde(default)]` (an `Option` when its type has no `Default`), and OpenAPI leaves it out of `required`. Before, a client generated while the field was deprecated failed to decode every response once the server dropped it, although the client never read it. TypeScript and Rust code that reads such a field must now handle its absence. Types also used in requests keep deprecated fields required, because the server may still require them until it removes them, and so do variants of untagged enums, where an optional field could make the client pick a different variant than serde. +- Generated clients decode responses after the server removes a deprecated field. Every generator now treats a deprecated field of a response-only type as optional: Python reads it as `None` and types a non-`Option` field `T | None`, TypeScript marks it `?:`, the Rust client gives it `#[serde(default)]` (an `Option` when its type has no `Default`), and OpenAPI leaves it out of `required`. Before, a client generated while the field was deprecated failed to decode every response once the server dropped it, although the client never read it. TypeScript and Rust code that reads such a field must now handle its absence. Types also used in requests keep deprecated fields required, because the server may still require them until it removes them, and so do untagged enums and every type their variants reach, where an optional field could make the client pick a different variant than serde. ## 0.23.0 diff --git a/reflectapi-demo/tests/rust/field_presence/tests/presence.rs b/reflectapi-demo/tests/rust/field_presence/tests/presence.rs index 8be47991..d0aad366 100644 --- a/reflectapi-demo/tests/rust/field_presence/tests/presence.rs +++ b/reflectapi-demo/tests/rust/field_presence/tests/presence.rs @@ -1,4 +1,6 @@ -use reflectapi_field_presence_rust::types::field_presence::fixtures::{input, output, Profile, Shared, Tuple}; +use reflectapi_field_presence_rust::types::field_presence::fixtures::{ + input, output, Profile, Shared, Tuple, +}; use serde_json::json; #[test] diff --git a/reflectapi/src/codegen/schema/deprecation.rs b/reflectapi/src/codegen/schema/deprecation.rs index 91fbc721..d332eda2 100644 --- a/reflectapi/src/codegen/schema/deprecation.rs +++ b/reflectapi/src/codegen/schema/deprecation.rs @@ -1,20 +1,24 @@ //! Deprecated fields that a response may leave out. -use reflectapi_schema::{Field, Representation, Schema, Type}; +use std::collections::HashSet; + +use reflectapi_schema::{Field, Representation, Schema, Type, TypeReference, Typespace}; /// A server removes a deprecated field once its sunset passes, so a client /// generated before that must decode responses without it. Only types that /// appear in responses alone are relaxed: a request type keeps the field -/// required, because the server may still require it until it is removed. +/// required, because the server may still require it until it is removed, +/// and so does every type an untagged enum's variants reach. /// Every generator runs this before consolidating the schema, so a type used /// in both directions is never split by it, and all of them agree on which /// keys a response may leave out. pub(crate) fn relax_deprecated_response_fields(schema: &mut Schema) { - let response_only: Vec = schema + let matched_untagged = types_matched_untagged(&schema.output_types); + let relaxed: Vec = schema .output_types .types() .map(|t| t.name().to_string()) - .filter(|name| !schema.input_types.has_type(name)) + .filter(|name| !schema.input_types.has_type(name) && !matched_untagged.contains(name)) .collect(); let relax = |field: &mut Field| { if field.deprecation().is_some() @@ -26,13 +30,10 @@ pub(crate) fn relax_deprecated_response_fields(schema: &mut Schema) { field.required = false; } }; - for name in response_only { + for name in relaxed { match schema.output_types.get_type_mut(&name) { Some(Type::Struct(s)) => s.fields.iter_mut().for_each(relax), - // An untagged enum picks the first variant that matches, so an - // optional field could make an earlier variant match where serde - // would pick a later one. - Some(Type::Enum(e)) if e.representation != Representation::None => { + Some(Type::Enum(e)) => { for variant in &mut e.variants { variant.fields.iter_mut().for_each(relax); } @@ -42,6 +43,43 @@ pub(crate) fn relax_deprecated_response_fields(schema: &mut Schema) { } } +/// Untagged enums and every type their variants reach. An untagged enum +/// picks the first variant that matches, so an optional field anywhere +/// inside a variant could make it match a payload that serde gives to a +/// later variant. +fn types_matched_untagged(types: &Typespace) -> HashSet { + let mut pending: Vec<&TypeReference> = Vec::new(); + let mut seen = HashSet::new(); + for ty in types.types() { + if let Type::Enum(e) = ty { + if e.representation == Representation::None { + seen.insert(e.name.clone()); + pending.extend( + e.variants + .iter() + .flat_map(|v| v.fields.iter().map(|f| &f.type_ref)), + ); + } + } + } + while let Some(type_ref) = pending.pop() { + pending.extend(type_ref.arguments.iter()); + if !seen.insert(type_ref.name.clone()) { + continue; + } + match types.get_type(&type_ref.name) { + Some(Type::Struct(s)) => pending.extend(s.fields.iter().map(|f| &f.type_ref)), + Some(Type::Enum(e)) => pending.extend( + e.variants + .iter() + .flat_map(|v| v.fields.iter().map(|f| &f.type_ref)), + ), + _ => {} + } + } + seen +} + #[cfg(test)] mod tests { use super::*; @@ -70,8 +108,14 @@ mod tests { {"name": "Changed", "fields": {"named": [field("old", true), field("kept", false)]}} ]}, {"kind": "enum", "name": "Untagged", "representation": "none", "variants": [ - {"name": "Changed", "fields": {"named": [field("old", true), field("kept", false)]}} - ]} + {"name": "Changed", "fields": {"named": [field("old", true), field("kept", false)]}}, + {"name": "Wrapped", "fields": {"unnamed": [{"name": "0", "type": {"name": "Inner"}}]}} + ]}, + {"kind": "struct", "name": "Inner", "fields": {"named": [ + field("old", true), + {"name": "nested", "required": true, "type": {"name": "Nested"}} + ]}}, + {"kind": "struct", "name": "Nested", "fields": {"named": [field("old", true)]}} ]} })) .unwrap(); @@ -95,6 +139,19 @@ mod tests { required(&schema.output_types, "Event"), [("old".into(), false), ("kept".into(), true)] ); + // Untagged variants match as before, down to the types they reach. + assert_eq!( + required(&schema.output_types, "Untagged"), + [("old".into(), true), ("kept".into(), true)] + ); + assert_eq!( + required(&schema.output_types, "Inner"), + [("old".into(), true), ("nested".into(), true)] + ); + assert_eq!( + required(&schema.output_types, "Nested"), + [("old".into(), true)] + ); assert_eq!( required(&schema.output_types, "Both"), [("old".into(), true)] From 8cd66b36b51fe681490d92707fae29a04234c382 Mon Sep 17 00:00:00 2001 From: Anton Zherdev Date: Thu, 5 Nov 2026 21:12:27 +0000 Subject: [PATCH 3/5] fix(codegen): keep fallback decodes exact and relaxed Rust fields compiling A tagged enum's untagged variants and the type behind a flattened Option are decoded by trying them, like untagged enums, so the types they reach keep their deprecated fields required too. Otherwise a payload could match an earlier variant, or a flattened group the server left out could decode as Some with defaults. A relaxed deprecated field of the Rust client keeps its type only when the type is known to implement Default. A type parameter or a primitive such as NonZeroU32 or url::Url becomes Option, where a serde default would not compile. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Q7zEtTUpmxtYWDkuq6uwpY --- CHANGELOG.md | 2 +- reflectapi-demo/tests/field_presence.rs | 15 ++++ .../rust/field_presence/tests/presence.rs | 5 +- reflectapi/src/codegen/rust.rs | 80 ++++++++++++++++++- reflectapi/src/codegen/schema/deprecation.rs | 67 ++++++++++++---- 5 files changed, 149 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c211e8d..28846b9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ ### Changed -- Generated clients decode responses after the server removes a deprecated field. Every generator now treats a deprecated field of a response-only type as optional: Python reads it as `None` and types a non-`Option` field `T | None`, TypeScript marks it `?:`, the Rust client gives it `#[serde(default)]` (an `Option` when its type has no `Default`), and OpenAPI leaves it out of `required`. Before, a client generated while the field was deprecated failed to decode every response once the server dropped it, although the client never read it. TypeScript and Rust code that reads such a field must now handle its absence. Types also used in requests keep deprecated fields required, because the server may still require them until it removes them, and so do untagged enums and every type their variants reach, where an optional field could make the client pick a different variant than serde. +- Generated clients decode responses after the server removes a deprecated field. Every generator now treats a deprecated field of a response-only type as optional: Python reads it as `None` and types a non-`Option` field `T | None`, TypeScript marks it `?:`, the Rust client gives it `#[serde(default)]`, or makes it an `Option` unless its type is known to implement `Default`, and OpenAPI leaves it out of `required`. Before, a client generated while the field was deprecated failed to decode every response once the server dropped it, although the client never read it. TypeScript and Rust code that reads such a field must now handle its absence. Types also used in requests keep deprecated fields required, because the server may still require them until it removes them, and so do types the client decodes by trying them in turn: untagged enums and untagged variants, the type behind a flattened `Option`, and every type these reach. There, an optional field could change what a payload decodes as. ## 0.23.0 diff --git a/reflectapi-demo/tests/field_presence.rs b/reflectapi-demo/tests/field_presence.rs index ebac6271..ada9b3d1 100644 --- a/reflectapi-demo/tests/field_presence.rs +++ b/reflectapi-demo/tests/field_presence.rs @@ -171,6 +171,16 @@ pub mod fixtures { pub legacy_code: String, #[deprecated(note = "use id")] pub legacy_config: Config, + #[deprecated(note = "use id")] + pub legacy_rank: std::num::NonZeroU32, + } + + // A deprecated field typed by a type parameter, which may lack `Default`. + #[derive(serde::Serialize, serde::Deserialize, reflectapi::Output)] + pub struct Envelope { + pub data: T, + #[deprecated(note = "use data")] + pub result: T, } // Used in both directions, so a deprecated field stays required. @@ -193,6 +203,10 @@ async fn profile(_: (), _: reflectapi::Empty, _: reflectapi::Empty) -> fixtures: unimplemented!("schema-only route") } +async fn envelope(_: (), _: reflectapi::Empty, _: reflectapi::Empty) -> fixtures::Envelope { + unimplemented!("schema-only route") +} + fn schema() -> reflectapi::Schema { reflectapi::Builder::<()>::new() .name("Field presence") @@ -237,6 +251,7 @@ fn schema() -> reflectapi::Schema { |b| b.name("fields.defaulted"), ) .route(profile, |b| b.name("fields.profile")) + .route(envelope, |b| b.name("fields.envelope")) .route( |_: (), input: fixtures::Shared, _: reflectapi::Empty| async { input }, |b| b.name("fields.shared"), diff --git a/reflectapi-demo/tests/rust/field_presence/tests/presence.rs b/reflectapi-demo/tests/rust/field_presence/tests/presence.rs index d0aad366..f7dd07fc 100644 --- a/reflectapi-demo/tests/rust/field_presence/tests/presence.rs +++ b/reflectapi-demo/tests/rust/field_presence/tests/presence.rs @@ -1,5 +1,5 @@ use reflectapi_field_presence_rust::types::field_presence::fixtures::{ - input, output, Profile, Shared, Tuple, + input, output, Envelope, Profile, Shared, Tuple, }; use serde_json::json; @@ -70,5 +70,8 @@ fn generated_responses_may_omit_deprecated_fields() { assert_eq!(profile.legacy_tags, None); assert_eq!(profile.legacy_code, ""); assert!(profile.legacy_config.is_none()); + assert!(profile.legacy_rank.is_none()); + let envelope: Envelope = serde_json::from_value(json!({"data": 1})).unwrap(); + assert_eq!(envelope.result, None); assert!(serde_json::from_value::(json!({"id": 1})).is_err()); } diff --git a/reflectapi/src/codegen/rust.rs b/reflectapi/src/codegen/rust.rs index 43878c5b..d2dbdf96 100644 --- a/reflectapi/src/codegen/rust.rs +++ b/reflectapi/src/codegen/rust.rs @@ -1704,20 +1704,90 @@ fn type_implements_default( } } +/// Like `type_implements_default`, but assumes nothing: a type parameter, a +/// mapped type or a primitive outside this list may lack `Default`. +fn type_known_to_implement_default( + type_ref: &crate::TypeReference, + schema: &crate::Schema, + base_derives: &BTreeSet, +) -> bool { + let mut type_ref = type_ref; + while matches!( + type_ref.name.as_str(), + "std::boxed::Box" | "std::rc::Rc" | "std::sync::Arc" + ) { + match type_ref.arguments.first() { + Some(inner) => type_ref = inner, + None => return false, + } + } + if matches!( + type_ref.name.as_str(), + "u8" | "u16" + | "u32" + | "u64" + | "u128" + | "usize" + | "i8" + | "i16" + | "i32" + | "i64" + | "i128" + | "isize" + | "f32" + | "f64" + | "bool" + | "char" + | "std::string::String" + | "std::vec::Vec" + | "std::collections::HashMap" + | "std::collections::BTreeMap" + | "std::collections::HashSet" + | "std::collections::BTreeSet" + | "std::collections::VecDeque" + | "std::option::Option" + | "reflectapi::Option" + | "std::time::Duration" + | "std::tuple::Tuple0" + | "serde_json::Value" + ) { + return true; + } + let derives = |config: &reflectapi_schema::LanguageSpecificTypeCodegenConfig| { + base_derives.contains("Default") || config.rust.additional_derives.contains("Default") + }; + match schema + .input_types + .get_type(&type_ref.name) + .or_else(|| schema.output_types.get_type(&type_ref.name)) + { + Some(crate::Type::Struct(s)) => derives(&s.codegen_config), + Some(crate::Type::Enum(e)) => derives(&e.codegen_config), + _ => false, + } +} + /// A field the server fills with `Default` when its key is missing. When the /// client's own type can't do the same, it becomes `Option`: `None` leaves /// the key out and lets the server apply its default. +/// +/// A deprecated response field may be optional only because the client must +/// survive its removal, so nothing on the server vouches for `Default`. It +/// keeps its type only when the type is known to implement `Default`. fn optional_field_type( type_: String, field: &crate::Field, + is_input_type: bool, schema: &crate::Schema, implemented_types: &HashMap, base_derives: &BTreeSet, ) -> String { - if field.required - || field.flattened - || type_implements_default(&field.type_ref, schema, implemented_types, base_derives) - { + let has_default = if !is_input_type && field.deprecation().is_some() { + type_known_to_implement_default(&field.type_ref, schema, base_derives) + } else { + type_implements_default(&field.type_ref, schema, implemented_types, base_derives) + }; + if field.required || field.flattened || has_default { type_ } else { format!("std::option::Option<{type_}>") @@ -1813,6 +1883,7 @@ fn __render_type( Some(type_def), ), field, + is_input_type, schema, implemented_types, &config.base_derives, @@ -1869,6 +1940,7 @@ fn __render_type( Some(type_def), ), field, + is_input_type, schema, implemented_types, &config.base_derives, diff --git a/reflectapi/src/codegen/schema/deprecation.rs b/reflectapi/src/codegen/schema/deprecation.rs index d332eda2..cfe1ba62 100644 --- a/reflectapi/src/codegen/schema/deprecation.rs +++ b/reflectapi/src/codegen/schema/deprecation.rs @@ -8,7 +8,8 @@ use reflectapi_schema::{Field, Representation, Schema, Type, TypeReference, Type /// generated before that must decode responses without it. Only types that /// appear in responses alone are relaxed: a request type keeps the field /// required, because the server may still require it until it is removed, -/// and so does every type an untagged enum's variants reach. +/// and so does every type a client decodes by trying it (see +/// `types_matched_untagged`). /// Every generator runs this before consolidating the schema, so a type used /// in both directions is never split by it, and all of them agree on which /// keys a response may leave out. @@ -43,23 +44,28 @@ pub(crate) fn relax_deprecated_response_fields(schema: &mut Schema) { } } -/// Untagged enums and every type their variants reach. An untagged enum -/// picks the first variant that matches, so an optional field anywhere -/// inside a variant could make it match a payload that serde gives to a -/// later variant. +/// Types a client decodes by trying them in turn: untagged enums, enums with +/// untagged variants, the type behind a flattened `Option`, and every type +/// these reach. A first-match decoder takes the first variant that matches, +/// and a flattened `Option` is `None` only when its type fails to decode, so +/// an optional field anywhere inside could change what a payload decodes as. fn types_matched_untagged(types: &Typespace) -> HashSet { let mut pending: Vec<&TypeReference> = Vec::new(); let mut seen = HashSet::new(); for ty in types.types() { - if let Type::Enum(e) = ty { - if e.representation == Representation::None { - seen.insert(e.name.clone()); - pending.extend( - e.variants - .iter() - .flat_map(|v| v.fields.iter().map(|f| &f.type_ref)), - ); + match ty { + Type::Enum(e) => { + let untagged = e.representation == Representation::None; + for variant in &e.variants { + if untagged || variant.untagged { + seen.insert(e.name.clone()); + pending.extend(variant.fields.iter().map(|f| &f.type_ref)); + } + pending.extend(flattened_options(variant.fields.iter())); + } } + Type::Struct(s) => pending.extend(flattened_options(s.fields.iter())), + Type::Primitive(_) => {} } } while let Some(type_ref) = pending.pop() { @@ -80,6 +86,21 @@ fn types_matched_untagged(types: &Typespace) -> HashSet { seen } +/// The types behind flattened `Option` fields. +fn flattened_options<'a>( + fields: impl Iterator, +) -> impl Iterator { + fields + .filter(|f| { + f.flattened + && matches!( + f.type_ref.name.as_str(), + "std::option::Option" | "reflectapi::Option" + ) + }) + .flat_map(|f| f.type_ref.arguments.iter()) +} + #[cfg(test)] mod tests { use super::*; @@ -115,7 +136,16 @@ mod tests { field("old", true), {"name": "nested", "required": true, "type": {"name": "Nested"}} ]}}, - {"kind": "struct", "name": "Nested", "fields": {"named": [field("old", true)]}} + {"kind": "struct", "name": "Nested", "fields": {"named": [field("old", true)]}}, + {"kind": "enum", "name": "Fallback", "variants": [ + {"name": "Known", "fields": {"named": [field("old", true)]}}, + {"name": "Other", "untagged": true, "fields": {"unnamed": [{"name": "0", "type": {"name": "Unmatched"}}]}} + ]}, + {"kind": "struct", "name": "Unmatched", "fields": {"named": [field("old", true)]}}, + {"kind": "struct", "name": "Holder", "fields": {"named": [ + {"name": "group", "required": true, "flattened": true, "type": {"name": "std::option::Option", "arguments": [{"name": "Group"}]}} + ]}}, + {"kind": "struct", "name": "Group", "fields": {"named": [field("old", true)]}} ]} })) .unwrap(); @@ -152,6 +182,15 @@ mod tests { required(&schema.output_types, "Nested"), [("old".into(), true)] ); + // A tagged enum's untagged variants, and the type behind a flattened + // `Option`, are decoded by trying them too. + for name in ["Fallback", "Unmatched", "Group"] { + assert_eq!( + required(&schema.output_types, name), + [("old".into(), true)], + "{name}" + ); + } assert_eq!( required(&schema.output_types, "Both"), [("old".into(), true)] From 7b4cccf7ebc4e8f6f515a0992cc684b845b696a5 Mon Sep 17 00:00:00 2001 From: Anton Zherdev Date: Thu, 5 Nov 2026 21:49:42 +0000 Subject: [PATCH 4/5] fix(codegen): relaxed Rust fields become Option, and generic fallbacks stay exact The transform now returns the fields it relaxed. The Rust client makes exactly those Option unless they already are, instead of guessing whether their type implements Default: Option always does, so error types, generic and aliased types and primitives without Default all compile, and deprecated fields with a server default keep their type. Types decoded by trying them now include every variant of an enum with an untagged fallback, and the type arguments wherever a reached generic type is used, such as Either or a generic type flattening an Option of its parameter. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Q7zEtTUpmxtYWDkuq6uwpY --- CHANGELOG.md | 2 +- ...basic__reflectapi_deprecation_dates-3.snap | 20 +- .../rust/field_presence/tests/presence.rs | 2 +- reflectapi/src/codegen/rust.rs | 100 +++------- reflectapi/src/codegen/schema/deprecation.rs | 186 ++++++++++++++---- reflectapi/src/codegen/schema/mod.rs | 2 +- 6 files changed, 184 insertions(+), 128 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28846b9d..a15715db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ ### Changed -- Generated clients decode responses after the server removes a deprecated field. Every generator now treats a deprecated field of a response-only type as optional: Python reads it as `None` and types a non-`Option` field `T | None`, TypeScript marks it `?:`, the Rust client gives it `#[serde(default)]`, or makes it an `Option` unless its type is known to implement `Default`, and OpenAPI leaves it out of `required`. Before, a client generated while the field was deprecated failed to decode every response once the server dropped it, although the client never read it. TypeScript and Rust code that reads such a field must now handle its absence. Types also used in requests keep deprecated fields required, because the server may still require them until it removes them, and so do types the client decodes by trying them in turn: untagged enums and untagged variants, the type behind a flattened `Option`, and every type these reach. There, an optional field could change what a payload decodes as. +- Generated clients decode responses after the server removes a deprecated field. Every generator now treats a deprecated field of a response-only type as optional: Python reads it as `None` and types a non-`Option` field `T | None`, TypeScript marks it `?:`, the Rust client types it `Option`, unless it already is one, and OpenAPI leaves it out of `required`. Before, a client generated while the field was deprecated failed to decode every response once the server dropped it, although the client never read it. TypeScript and Rust code that reads such a field must now handle its absence. Types also used in requests keep deprecated fields required, because the server may still require them until it removes them, and so do types the client decodes by trying them in turn: untagged enums and untagged variants, the type behind a flattened `Option`, and every type these reach. There, an optional field could change what a payload decodes as. ## 0.23.0 diff --git a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecation_dates-3.snap b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecation_dates-3.snap index a633414c..00df7c22 100644 --- a/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecation_dates-3.snap +++ b/reflectapi-demo/src/tests/snapshots/reflectapi_demo__tests__basic__reflectapi_deprecation_dates-3.snap @@ -100,26 +100,26 @@ pub mod types { #[derive(Debug, serde::Deserialize)] pub struct StructWithDeprecationDates { - #[serde(default = "Default::default")] + #[serde(default = "Default::default", skip_serializing_if = "std::option::Option::is_none")] #[deprecated(note = "Deprecated since 2026-10-01; removal after 2026-11-01. Use `current` instead.")] - pub _since_and_sunset: u8, - #[serde(default = "Default::default")] + pub _since_and_sunset: std::option::Option, + #[serde(default = "Default::default", skip_serializing_if = "std::option::Option::is_none")] #[deprecated(note = "Deprecated since 2026-10-01.")] - pub _since_only: u8, + pub _since_only: std::option::Option, #[serde(default = "Default::default", skip_serializing_if = "std::option::Option::is_none")] #[deprecated(note = "Removal after 2026-11-01.")] pub _sunset_only: std::option::Option, /// rustc's `since` is the release, kept verbatim. - #[serde(default = "Default::default")] + #[serde(default = "Default::default", skip_serializing_if = "std::option::Option::is_none")] #[deprecated(note = "Deprecated in 1.2.0. Versioned.")] - pub _release_only: u8, + pub _release_only: std::option::Option, /// It is never read as a date, even when it looks like one. - #[serde(default = "Default::default")] + #[serde(default = "Default::default", skip_serializing_if = "std::option::Option::is_none")] #[deprecated(note = "Deprecated in 2026-10-01.")] - pub _release_like_a_date: u8, - #[serde(default = "Default::default")] + pub _release_like_a_date: std::option::Option, + #[serde(default = "Default::default", skip_serializing_if = "std::option::Option::is_none")] #[deprecated(note = "Deprecated in 2.3.0, since 2026-10-01; removal after 2026-11-01. Use `current` instead.")] - pub _release_and_dates: u8, + pub _release_and_dates: std::option::Option, pub _current: super::super::super::super::reflectapi_demo::tests::basic::EnumWithDeprecatedVariants, } } diff --git a/reflectapi-demo/tests/rust/field_presence/tests/presence.rs b/reflectapi-demo/tests/rust/field_presence/tests/presence.rs index f7dd07fc..3db0e229 100644 --- a/reflectapi-demo/tests/rust/field_presence/tests/presence.rs +++ b/reflectapi-demo/tests/rust/field_presence/tests/presence.rs @@ -68,7 +68,7 @@ fn defaulted_fields_of_generated_types_may_be_omitted() { fn generated_responses_may_omit_deprecated_fields() { let profile: Profile = serde_json::from_value(json!({"id": 1})).unwrap(); assert_eq!(profile.legacy_tags, None); - assert_eq!(profile.legacy_code, ""); + assert_eq!(profile.legacy_code, None); assert!(profile.legacy_config.is_none()); assert!(profile.legacy_rank.is_none()); let envelope: Envelope = serde_json::from_value(json!({"data": 1})).unwrap(); diff --git a/reflectapi/src/codegen/rust.rs b/reflectapi/src/codegen/rust.rs index d2dbdf96..f3ec1632 100644 --- a/reflectapi/src/codegen/rust.rs +++ b/reflectapi/src/codegen/rust.rs @@ -140,7 +140,7 @@ fn types_referenced_by( pub fn generate(mut schema: crate::Schema, config: &Config) -> anyhow::Result { super::validate_response_enums(&schema)?; schema.strip_hidden_fields(); - super::schema::relax_deprecated_response_fields(&mut schema); + let relaxed_fields = super::schema::relax_deprecated_response_fields(&mut schema); let mut implemented_types = __build_implemented_types(); for type_def in schema .input_types() @@ -201,6 +201,7 @@ pub fn generate(mut schema: crate::Schema, config: &Config) -> anyhow::Result, -) -> bool { - let mut type_ref = type_ref; - while matches!( - type_ref.name.as_str(), - "std::boxed::Box" | "std::rc::Rc" | "std::sync::Arc" - ) { - match type_ref.arguments.first() { - Some(inner) => type_ref = inner, - None => return false, - } - } - if matches!( - type_ref.name.as_str(), - "u8" | "u16" - | "u32" - | "u64" - | "u128" - | "usize" - | "i8" - | "i16" - | "i32" - | "i64" - | "i128" - | "isize" - | "f32" - | "f64" - | "bool" - | "char" - | "std::string::String" - | "std::vec::Vec" - | "std::collections::HashMap" - | "std::collections::BTreeMap" - | "std::collections::HashSet" - | "std::collections::BTreeSet" - | "std::collections::VecDeque" - | "std::option::Option" - | "reflectapi::Option" - | "std::time::Duration" - | "std::tuple::Tuple0" - | "serde_json::Value" - ) { - return true; - } - let derives = |config: &reflectapi_schema::LanguageSpecificTypeCodegenConfig| { - base_derives.contains("Default") || config.rust.additional_derives.contains("Default") - }; - match schema - .input_types - .get_type(&type_ref.name) - .or_else(|| schema.output_types.get_type(&type_ref.name)) - { - Some(crate::Type::Struct(s)) => derives(&s.codegen_config), - Some(crate::Type::Enum(e)) => derives(&e.codegen_config), - _ => false, - } -} - /// A field the server fills with `Default` when its key is missing. When the /// client's own type can't do the same, it becomes `Option`: `None` leaves /// the key out and lets the server apply its default. /// -/// A deprecated response field may be optional only because the client must -/// survive its removal, so nothing on the server vouches for `Default`. It -/// keeps its type only when the type is known to implement `Default`. +/// A deprecated response field that was relaxed so the client survives its +/// removal has no server default behind it, so it becomes `Option` unless +/// it already is one: `Option` implements `Default` whatever `T` is. fn optional_field_type( type_: String, field: &crate::Field, - is_input_type: bool, + relaxed: bool, schema: &crate::Schema, implemented_types: &HashMap, base_derives: &BTreeSet, ) -> String { - let has_default = if !is_input_type && field.deprecation().is_some() { - type_known_to_implement_default(&field.type_ref, schema, base_derives) + let keeps_type = if relaxed { + matches!( + field.type_ref.name.as_str(), + "std::option::Option" | "reflectapi::Option" + ) } else { - type_implements_default(&field.type_ref, schema, implemented_types, base_derives) + field.required + || field.flattened + || type_implements_default(&field.type_ref, schema, implemented_types, base_derives) }; - if field.required || field.flattened || has_default { + if keeps_type { type_ } else { format!("std::option::Option<{type_}>") @@ -1817,6 +1760,7 @@ fn request_field_type_ref<'a>( } } +#[allow(clippy::too_many_arguments)] fn __render_type( config: &Config, type_def: &crate::Type, @@ -1825,8 +1769,16 @@ fn __render_type( is_input_type: bool, is_output_type: bool, is_error_type: bool, + relaxed_fields: &HashSet, ) -> Result { let type_name = __type_to_ts_name(type_def); + let relaxed = |variant: Option<&str>, field: &crate::Field| { + relaxed_fields.contains(&crate::codegen::schema::RelaxedField { + type_name: type_def.name().to_owned(), + variant: variant.map(str::to_owned), + field: field.name.clone(), + }) + }; let type_name_depth = type_def.name().split("::").count() - 1; Ok(match type_def { @@ -1883,7 +1835,7 @@ fn __render_type( Some(type_def), ), field, - is_input_type, + relaxed(None, field), schema, implemented_types, &config.base_derives, @@ -1940,7 +1892,7 @@ fn __render_type( Some(type_def), ), field, - is_input_type, + relaxed(Some(variant.name()), field), schema, implemented_types, &config.base_derives, diff --git a/reflectapi/src/codegen/schema/deprecation.rs b/reflectapi/src/codegen/schema/deprecation.rs index cfe1ba62..33a561db 100644 --- a/reflectapi/src/codegen/schema/deprecation.rs +++ b/reflectapi/src/codegen/schema/deprecation.rs @@ -4,24 +4,35 @@ use std::collections::HashSet; use reflectapi_schema::{Field, Representation, Schema, Type, TypeReference, Typespace}; +/// A field the transform made optional: its type, its variant if the type +/// is an enum, and its name. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub(crate) struct RelaxedField { + pub type_name: String, + pub variant: Option, + pub field: String, +} + /// A server removes a deprecated field once its sunset passes, so a client /// generated before that must decode responses without it. Only types that /// appear in responses alone are relaxed: a request type keeps the field /// required, because the server may still require it until it is removed, /// and so does every type a client decodes by trying it (see -/// `types_matched_untagged`). -/// Every generator runs this before consolidating the schema, so a type used -/// in both directions is never split by it, and all of them agree on which -/// keys a response may leave out. -pub(crate) fn relax_deprecated_response_fields(schema: &mut Schema) { - let matched_untagged = types_matched_untagged(&schema.output_types); - let relaxed: Vec = schema +/// `types_decoded_by_trying`). Every generator runs this before +/// consolidating the schema, so a type used in both directions is never +/// split by it, and all of them agree on which keys a response may leave +/// out. A relaxed type keeps its name through consolidation, so the returned +/// fields can be looked up in the consolidated schema. +pub(crate) fn relax_deprecated_response_fields(schema: &mut Schema) -> HashSet { + let tried = types_decoded_by_trying(&schema.output_types); + let candidates: Vec = schema .output_types .types() .map(|t| t.name().to_string()) - .filter(|name| !schema.input_types.has_type(name) && !matched_untagged.contains(name)) + .filter(|name| !schema.input_types.has_type(name) && !tried.contains(name)) .collect(); - let relax = |field: &mut Field| { + let mut relaxed = HashSet::new(); + let mut relax = |type_name: &str, variant: Option<&str>, field: &mut Field| { if field.deprecation().is_some() && field.required && field.is_named() @@ -29,61 +40,117 @@ pub(crate) fn relax_deprecated_response_fields(schema: &mut Schema) { && !field.deserialize_with { field.required = false; + relaxed.insert(RelaxedField { + type_name: type_name.to_owned(), + variant: variant.map(str::to_owned), + field: field.name.clone(), + }); } }; - for name in relaxed { + for name in candidates { match schema.output_types.get_type_mut(&name) { - Some(Type::Struct(s)) => s.fields.iter_mut().for_each(relax), + Some(Type::Struct(s)) => s.fields.iter_mut().for_each(|f| relax(&name, None, f)), Some(Type::Enum(e)) => { for variant in &mut e.variants { - variant.fields.iter_mut().for_each(relax); + let variant_name = variant.name.clone(); + variant + .fields + .iter_mut() + .for_each(|f| relax(&name, Some(&variant_name), f)); } } _ => {} } } + relaxed } -/// Types a client decodes by trying them in turn: untagged enums, enums with -/// untagged variants, the type behind a flattened `Option`, and every type -/// these reach. A first-match decoder takes the first variant that matches, -/// and a flattened `Option` is `None` only when its type fails to decode, so -/// an optional field anywhere inside could change what a payload decodes as. -fn types_matched_untagged(types: &Typespace) -> HashSet { - let mut pending: Vec<&TypeReference> = Vec::new(); +/// Types a client decodes by trying them in turn, and every type these +/// reach. A first-match decoder takes the first variant that matches, and a +/// flattened `Option` is `None` only when its type fails to decode, so an +/// optional field anywhere inside could change what a payload decodes as. +/// The seeds are untagged enums, enums with an untagged variant (a tagged +/// variant that fails falls back to it), and the type behind a flattened +/// `Option`. A generic type that is reached, or that flattens an `Option` of +/// its own parameter, reaches the type arguments of every use of it. +fn types_decoded_by_trying(types: &Typespace) -> HashSet { let mut seen = HashSet::new(); + let mut pending: Vec<&TypeReference> = Vec::new(); for ty in types.types() { + let parameters: HashSet<&str> = ty.parameters().map(|p| p.name.as_str()).collect(); + let seed_flattened = |fields: &'_ reflectapi_schema::Fields, seen: &mut HashSet| { + for inner in flattened_options(fields.iter()) { + if parameters.contains(inner.name.as_str()) { + seen.insert(ty.name().to_owned()); + } + } + }; match ty { Type::Enum(e) => { - let untagged = e.representation == Representation::None; + if e.representation == Representation::None || e.variants.iter().any(|v| v.untagged) + { + seen.insert(e.name.clone()); + pending.extend( + e.variants + .iter() + .flat_map(|v| v.fields.iter().map(|f| &f.type_ref)), + ); + } for variant in &e.variants { - if untagged || variant.untagged { - seen.insert(e.name.clone()); - pending.extend(variant.fields.iter().map(|f| &f.type_ref)); - } pending.extend(flattened_options(variant.fields.iter())); + seed_flattened(&variant.fields, &mut seen); } } - Type::Struct(s) => pending.extend(flattened_options(s.fields.iter())), + Type::Struct(s) => { + pending.extend(flattened_options(s.fields.iter())); + seed_flattened(&s.fields, &mut seen); + } Type::Primitive(_) => {} } } - while let Some(type_ref) = pending.pop() { - pending.extend(type_ref.arguments.iter()); - if !seen.insert(type_ref.name.clone()) { - continue; + let uses: Vec<&TypeReference> = types + .types() + .flat_map(|ty| match ty { + Type::Struct(s) => s.fields.iter().collect::>(), + Type::Enum(e) => e.variants.iter().flat_map(|v| v.fields.iter()).collect(), + Type::Primitive(_) => Vec::new(), + }) + .map(|f| &f.type_ref) + .collect(); + loop { + while let Some(type_ref) = pending.pop() { + pending.extend(type_ref.arguments.iter()); + if !seen.insert(type_ref.name.clone()) { + continue; + } + match types.get_type(&type_ref.name) { + Some(Type::Struct(s)) => pending.extend(s.fields.iter().map(|f| &f.type_ref)), + Some(Type::Enum(e)) => pending.extend( + e.variants + .iter() + .flat_map(|v| v.fields.iter().map(|f| &f.type_ref)), + ), + _ => {} + } } - match types.get_type(&type_ref.name) { - Some(Type::Struct(s)) => pending.extend(s.fields.iter().map(|f| &f.type_ref)), - Some(Type::Enum(e)) => pending.extend( - e.variants - .iter() - .flat_map(|v| v.fields.iter().map(|f| &f.type_ref)), - ), - _ => {} + // A use of a reached generic type, possibly nested in another + // type's arguments, reaches its arguments. + let mut stack = uses.clone(); + while let Some(type_ref) = stack.pop() { + if seen.contains(&type_ref.name) { + pending.extend( + type_ref + .arguments + .iter() + .filter(|argument| !seen.contains(&argument.name)), + ); + } + stack.extend(type_ref.arguments.iter()); + } + if pending.is_empty() { + return seen; } } - seen } /// The types behind flattened `Option` fields. @@ -145,11 +212,38 @@ mod tests { {"kind": "struct", "name": "Holder", "fields": {"named": [ {"name": "group", "required": true, "flattened": true, "type": {"name": "std::option::Option", "arguments": [{"name": "Group"}]}} ]}}, - {"kind": "struct", "name": "Group", "fields": {"named": [field("old", true)]}} + {"kind": "struct", "name": "Group", "fields": {"named": [field("old", true)]}}, + {"kind": "enum", "name": "WithFallback", "variants": [ + {"name": "Tagged", "fields": {"unnamed": [{"name": "0", "type": {"name": "TaggedPayload"}}]}}, + {"name": "Rest", "untagged": true, "fields": {"unnamed": [{"name": "0", "type": {"name": "u32"}}]}} + ]}, + {"kind": "struct", "name": "TaggedPayload", "fields": {"named": [field("old", true)]}}, + {"kind": "enum", "name": "Either", "representation": "none", + "parameters": [{"name": "L"}, {"name": "R"}], + "variants": [ + {"name": "Left", "fields": {"unnamed": [{"name": "0", "type": {"name": "L"}}]}}, + {"name": "Right", "fields": {"unnamed": [{"name": "0", "type": {"name": "R"}}]}} + ]}, + {"kind": "struct", "name": "Lefty", "fields": {"named": [field("old", true)]}}, + {"kind": "struct", "name": "Holds", "parameters": [{"name": "T"}], "fields": {"named": [ + {"name": "group", "required": true, "flattened": true, "type": {"name": "std::option::Option", "arguments": [{"name": "T"}]}} + ]}}, + {"kind": "struct", "name": "Held", "fields": {"named": [field("old", true)]}}, + {"kind": "struct", "name": "User", "fields": {"named": [ + {"name": "either", "required": true, "type": {"name": "std::vec::Vec", "arguments": [ + {"name": "Either", "arguments": [{"name": "Lefty"}, {"name": "u32"}]} + ]}}, + {"name": "holds", "required": true, "type": {"name": "Holds", "arguments": [{"name": "Held"}]}} + ]}} ]} })) .unwrap(); - super::relax_deprecated_response_fields(&mut schema); + let relaxed = super::relax_deprecated_response_fields(&mut schema); + assert!(relaxed.contains(&RelaxedField { + type_name: "Event".into(), + variant: Some("Changed".into()), + field: "old".into(), + })); let required = |ts: &reflectapi_schema::Typespace, ty: &str| -> Vec<(String, bool)> { let fields = match ts.get_type(ty).unwrap() { Type::Struct(s) => s.fields.iter().collect::>(), @@ -184,7 +278,17 @@ mod tests { ); // A tagged enum's untagged variants, and the type behind a flattened // `Option`, are decoded by trying them too. - for name in ["Fallback", "Unmatched", "Group"] { + // So are a tagged variant of an enum with an untagged fallback, and + // the arguments of a generic untagged enum or of a generic type + // flattening an `Option` of its parameter, wherever it is used. + for name in [ + "Fallback", + "Unmatched", + "Group", + "TaggedPayload", + "Lefty", + "Held", + ] { assert_eq!( required(&schema.output_types, name), [("old".into(), true)], diff --git a/reflectapi/src/codegen/schema/mod.rs b/reflectapi/src/codegen/schema/mod.rs index 9f338723..8b664c2b 100644 --- a/reflectapi/src/codegen/schema/mod.rs +++ b/reflectapi/src/codegen/schema/mod.rs @@ -19,7 +19,7 @@ mod symbol; // Public-to-the-crate surface. Only what backends (currently just Python) // actually consume is re-exported; everything else stays accessible as // `self::ids::*`, `self::normalize::*`, etc. for the module's own tests. -pub(crate) use self::deprecation::relax_deprecated_response_fields; +pub(crate) use self::deprecation::{relax_deprecated_response_fields, RelaxedField}; pub(crate) use self::ids::{build_schema_ids, SchemaIds}; pub(crate) use self::layout::{FlattenedGroup, Layout, Member, Opaque}; pub(crate) use self::normalize::{Consolidation, Naming, Normalizer, PipelineBuilder}; From 128a07064719435a89a66d58ee05def670c8e599 Mon Sep 17 00:00:00 2001 From: Anton Zherdev Date: Fri, 6 Nov 2026 01:37:35 +0000 Subject: [PATCH 5/5] fix(codegen): walk generic instances when finding types decoded by trying Substitute each use's type arguments into a generic type's fields, starting from every response type and every function's output and error types, so a generic untagged enum returned directly, or reached through a generic type forwarding its parameter, keeps its arguments required. `Option` is no longer treated as an untagged enum, so a type behind an `Option` field is relaxed. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Q7zEtTUpmxtYWDkuq6uwpY --- reflectapi/src/codegen/schema/deprecation.rs | 217 +++++++++++-------- 1 file changed, 132 insertions(+), 85 deletions(-) diff --git a/reflectapi/src/codegen/schema/deprecation.rs b/reflectapi/src/codegen/schema/deprecation.rs index 33a561db..103fcd25 100644 --- a/reflectapi/src/codegen/schema/deprecation.rs +++ b/reflectapi/src/codegen/schema/deprecation.rs @@ -2,7 +2,9 @@ use std::collections::HashSet; -use reflectapi_schema::{Field, Representation, Schema, Type, TypeReference, Typespace}; +use reflectapi_schema::{ + Field, Instantiate, Representation, Schema, Type, TypeReference, Typespace, +}; /// A field the transform made optional: its type, its variant if the type /// is an enum, and its name. @@ -24,7 +26,7 @@ pub(crate) struct RelaxedField { /// out. A relaxed type keeps its name through consolidation, so the returned /// fields can be looked up in the consolidated schema. pub(crate) fn relax_deprecated_response_fields(schema: &mut Schema) -> HashSet { - let tried = types_decoded_by_trying(&schema.output_types); + let tried = types_decoded_by_trying(schema); let candidates: Vec = schema .output_types .types() @@ -71,100 +73,118 @@ pub(crate) fn relax_deprecated_response_fields(schema: &mut Schema) -> HashSet HashSet { - let mut seen = HashSet::new(); - let mut pending: Vec<&TypeReference> = Vec::new(); - for ty in types.types() { - let parameters: HashSet<&str> = ty.parameters().map(|p| p.name.as_str()).collect(); - let seed_flattened = |fields: &'_ reflectapi_schema::Fields, seen: &mut HashSet| { - for inner in flattened_options(fields.iter()) { - if parameters.contains(inner.name.as_str()) { - seen.insert(ty.name().to_owned()); - } - } +/// `Option`. `Option` itself reads null or its argument, so it is no seed. +/// +/// Generic types are walked as instances, with the arguments of each use +/// substituted into their fields, so `Either` reaches `Old` and a +/// generic struct flattening `Option` reaches the `T` of each use. +fn types_decoded_by_trying(schema: &Schema) -> HashSet { + let types = &schema.output_types; + let mut pending: Vec = types + .types() + .filter(|ty| ty.parameters().next().is_none()) + .map(|ty| TypeReference::new(ty.name(), vec![])) + .collect(); + for function in schema.functions() { + pending.extend(function.output_type.type_refs().into_iter().cloned()); + pending.extend(function.error_type.iter().cloned()); + } + + // Every instance a response may contain, and the seeds among them. + let mut instances = HashSet::new(); + let mut seeds = Vec::new(); + while let Some(type_ref) = pending.pop() { + if !within_depth(&type_ref) || !instances.insert(type_ref.clone()) { + continue; + } + pending.extend(type_ref.arguments.iter().cloned()); + let Some(instance) = Instance::of(types, &type_ref) else { + continue; }; - match ty { - Type::Enum(e) => { - if e.representation == Representation::None || e.variants.iter().any(|v| v.untagged) - { - seen.insert(e.name.clone()); - pending.extend( - e.variants - .iter() - .flat_map(|v| v.fields.iter().map(|f| &f.type_ref)), - ); - } - for variant in &e.variants { - pending.extend(flattened_options(variant.fields.iter())); - seed_flattened(&variant.fields, &mut seen); - } - } - Type::Struct(s) => { - pending.extend(flattened_options(s.fields.iter())); - seed_flattened(&s.fields, &mut seen); - } - Type::Primitive(_) => {} + if instance.tried_in_turn { + seeds.push(type_ref.clone()); } + seeds.extend(flattened_options(instance.fields.iter()).cloned()); + pending.extend(instance.fields.into_iter().map(|f| f.type_ref)); } - let uses: Vec<&TypeReference> = types - .types() - .flat_map(|ty| match ty { - Type::Struct(s) => s.fields.iter().collect::>(), - Type::Enum(e) => e.variants.iter().flat_map(|v| v.fields.iter()).collect(), - Type::Primitive(_) => Vec::new(), - }) - .map(|f| &f.type_ref) - .collect(); - loop { - while let Some(type_ref) = pending.pop() { - pending.extend(type_ref.arguments.iter()); - if !seen.insert(type_ref.name.clone()) { - continue; - } - match types.get_type(&type_ref.name) { - Some(Type::Struct(s)) => pending.extend(s.fields.iter().map(|f| &f.type_ref)), - Some(Type::Enum(e)) => pending.extend( - e.variants - .iter() - .flat_map(|v| v.fields.iter().map(|f| &f.type_ref)), - ), - _ => {} - } + + let mut tried = HashSet::new(); + let mut visited = HashSet::new(); + let mut pending = seeds; + while let Some(type_ref) = pending.pop() { + if !within_depth(&type_ref) || !visited.insert(type_ref.clone()) { + continue; } - // A use of a reached generic type, possibly nested in another - // type's arguments, reaches its arguments. - let mut stack = uses.clone(); - while let Some(type_ref) = stack.pop() { - if seen.contains(&type_ref.name) { - pending.extend( - type_ref - .arguments + tried.insert(type_ref.name.clone()); + pending.extend(type_ref.arguments.iter().cloned()); + if let Some(instance) = Instance::of(types, &type_ref) { + pending.extend(instance.fields.into_iter().map(|f| f.type_ref)); + } + } + tried +} + +/// A type whose arguments nest its own name deeper on each step, such as +/// `struct Tree { children: Vec>> }`, has no finite set of +/// instances; the walk stops at this depth. +fn within_depth(type_ref: &TypeReference) -> bool { + fn depth(type_ref: &TypeReference) -> usize { + 1 + type_ref.arguments.iter().map(depth).max().unwrap_or(0) + } + depth(type_ref) <= 16 +} + +/// The fields of a struct or enum with a use's arguments substituted in. +struct Instance { + tried_in_turn: bool, + fields: Vec, +} + +impl Instance { + fn of(types: &Typespace, type_ref: &TypeReference) -> Option { + let ty = types.get_type(&type_ref.name)?; + if ty.parameters().len() != type_ref.arguments.len() { + return None; + } + match ty.clone() { + Type::Struct(s) => Some(Instance { + tried_in_turn: false, + fields: s + .instantiate(&type_ref.arguments) + .fields + .iter() + .cloned() + .collect(), + }), + Type::Enum(e) => { + let tried_in_turn = !is_option(&e.name) + && (e.representation == Representation::None + || e.variants.iter().any(|v| v.untagged)); + let e = e.instantiate(&type_ref.arguments); + Some(Instance { + tried_in_turn, + fields: e + .variants .iter() - .filter(|argument| !seen.contains(&argument.name)), - ); + .flat_map(|v| v.fields.iter().cloned()) + .collect(), + }) } - stack.extend(type_ref.arguments.iter()); - } - if pending.is_empty() { - return seen; + Type::Primitive(_) => None, } } } +fn is_option(name: &str) -> bool { + matches!(name, "std::option::Option" | "reflectapi::Option") +} + /// The types behind flattened `Option` fields. fn flattened_options<'a>( fields: impl Iterator, ) -> impl Iterator { fields - .filter(|f| { - f.flattened - && matches!( - f.type_ref.name.as_str(), - "std::option::Option" | "reflectapi::Option" - ) - }) + .filter(|f| f.flattened && is_option(&f.type_ref.name)) .flat_map(|f| f.type_ref.arguments.iter()) } @@ -233,9 +253,27 @@ mod tests { {"name": "either", "required": true, "type": {"name": "std::vec::Vec", "arguments": [ {"name": "Either", "arguments": [{"name": "Lefty"}, {"name": "u32"}]} ]}}, - {"name": "holds", "required": true, "type": {"name": "Holds", "arguments": [{"name": "Held"}]}} - ]}} - ]} + {"name": "holds", "required": true, "type": {"name": "Holds", "arguments": [{"name": "Held"}]}}, + {"name": "config", "required": true, "type": {"name": "std::option::Option", "arguments": [{"name": "Configured"}]}} + ]}}, + {"kind": "enum", "name": "std::option::Option", "representation": "none", + "parameters": [{"name": "T"}], + "variants": [ + {"name": "None", "fields": "none"}, + {"name": "Some", "fields": {"unnamed": [{"name": "0", "type": {"name": "T"}}]}} + ]}, + {"kind": "struct", "name": "Configured", "fields": {"named": [field("old", true)]}}, + {"kind": "struct", "name": "FnLeft", "fields": {"named": [field("old", true)]}}, + {"kind": "struct", "name": "Forward", "parameters": [{"name": "T"}], "fields": {"named": [ + {"name": "either", "required": true, "type": {"name": "Either", "arguments": [{"name": "T"}, {"name": "u32"}]}} + ]}}, + {"kind": "struct", "name": "Forwarded", "fields": {"named": [field("old", true)]}} + ]}, + "functions": [ + {"name": "direct", "path": "", "output_kind": "complete", + "output_type": {"name": "Either", "arguments": [{"name": "FnLeft"}, {"name": "u32"}]}, + "error_type": {"name": "Forward", "arguments": [{"name": "Forwarded"}]}} + ] })) .unwrap(); let relaxed = super::relax_deprecated_response_fields(&mut schema); @@ -259,6 +297,11 @@ mod tests { required(&schema.output_types, "Reply"), [("old".into(), false), ("kept".into(), true)] ); + // `Option` reads null or its argument, not by trying. + assert_eq!( + required(&schema.output_types, "Configured"), + [("old".into(), false)] + ); assert_eq!( required(&schema.output_types, "Event"), [("old".into(), false), ("kept".into(), true)] @@ -280,8 +323,12 @@ mod tests { // `Option`, are decoded by trying them too. // So are a tagged variant of an enum with an untagged fallback, and // the arguments of a generic untagged enum or of a generic type - // flattening an `Option` of its parameter, wherever it is used. + // flattening an `Option` of its parameter, wherever it is used, + // including a function's response and error types and a generic + // type forwarding its parameter into one. for name in [ + "FnLeft", + "Forwarded", "Fallback", "Unmatched", "Group",