From cec4c41695115f5ca7aecaee35c8b08aba4a988b Mon Sep 17 00:00:00 2001 From: Jeong Min Oh Date: Sun, 15 Nov 2026 10:11:07 +0900 Subject: [PATCH] fix(macro): round-7 parser findings - Query/Path: a #[serde(transparent)] struct reads serde's transparent field (never PhantomData / skipped / serde(default)); transparent and from/try_from tuple structs are followed too. - value_type generic substitution splits at the first comma outside <..> (HashMap, Result); a -> closes no bracket. - Request-view recursive generic instances are named `.Input`, so Tree never collides with Tree. - Two flattened maps: anyOf of their values in the response view (serde writes each map's own entries), allOf in the request view. - A NoContent error side is labelled "Successful response" (204). - A bare argument of a shapeless type keeps the {type: string} path placeholder instead of widening it to {}. - Docs: #[schema(any)] emits {}, custom typed headers are kebab-cased type names. - Tests now assert what they claim (exact response sets, content types, tag values per variant, innermost builtin idents, leaf conversion arms, more primitive arms, and more). Co-Authored-By: Claude Opus 5.5 --- .../changepack_log_macro-parser-round7.json | 1 + ...r_views_recursive_generic_definitions.snap | 4 +- ...n__tests__dedup_bare_custom_extractor.snap | 8 +- ...r__operation__tests__dedup_bare_typed.snap | 6 +- ...ion__tests__dedup_path_extractor_wins.snap | 6 +- ...est_shapes__request_shapes_known_enum.snap | 2 +- ...s__request_shapes_optional_query_enum.snap | 2 +- .../src/parser/operation/tests.rs | 88 ++++--- .../operation/tests_extractor_wrappers.rs | 5 +- .../parser/operation/tests_request_shapes.rs | 9 +- .../src/parser/parameters/header.rs | 3 +- .../src/parser/parameters/path.rs | 54 +++-- .../src/parser/parameters/query.rs | 15 +- .../src/parser/parameters/shared.rs | 44 ++-- ...meter_shapes_path_transparent_phantom.snap | 26 +++ ...rameter_shapes_path_transparent_tuple.snap | 26 +++ ...es__parameter_shapes_query_from_tuple.snap | 15 ++ ...eter_shapes_query_transparent_default.snap | 15 ++ ...eter_shapes_query_transparent_phantom.snap | 15 ++ ...ameter_shapes_query_transparent_tuple.snap | 15 ++ .../src/parser/parameters/tests_shapes.rs | 40 ++++ crates/vespera_macro/src/parser/response.rs | 13 +- ..._parts__response_parts_err_no_content.snap | 2 +- .../src/parser/response/tests.rs | 38 ++- .../src/parser/response/tests_parts.rs | 3 +- .../enum_schema/representations/tests.rs | 94 +++++--- .../schema/enum_schema/tests_validate.rs | 4 +- crates/vespera_macro/src/parser/schema/mod.rs | 2 +- .../src/parser/schema/schema_attrs.rs | 8 +- .../src/parser/schema/struct_schema.rs | 2 +- .../parser/schema/struct_schema/flatten.rs | 14 +- ...delity__struct_views_flatten_two_maps.snap | 4 +- ...atten_two_maps_before_optional_struct.snap | 2 +- .../src/parser/schema/struct_schema/tests.rs | 67 +++--- .../schema/struct_schema/tests_wire_names.rs | 4 +- .../parser/schema/type_schema/conversion.rs | 8 +- .../type_schema/conversion/schema_def.rs | 42 +++- .../schema/type_schema/conversion/tests.rs | 218 +++++++++--------- .../type_schema/conversion/tests_instances.rs | 117 ++++++++++ .../schema/type_schema/generic_instances.rs | 4 +- 40 files changed, 728 insertions(+), 317 deletions(-) create mode 100644 .changepacks/changepack_log_macro-parser-round7.json create mode 100644 crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_path_transparent_phantom.snap create mode 100644 crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_path_transparent_tuple.snap create mode 100644 crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_query_from_tuple.snap create mode 100644 crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_query_transparent_default.snap create mode 100644 crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_query_transparent_phantom.snap create mode 100644 crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_query_transparent_tuple.snap create mode 100644 crates/vespera_macro/src/parser/schema/type_schema/conversion/tests_instances.rs diff --git a/.changepacks/changepack_log_macro-parser-round7.json b/.changepacks/changepack_log_macro-parser-round7.json new file mode 100644 index 00000000..300ebe51 --- /dev/null +++ b/.changepacks/changepack_log_macro-parser-round7.json @@ -0,0 +1 @@ +{"changes":{"crates/vespera/Cargo.toml":"Patch","crates/vespera_core/Cargo.toml":"Patch","crates/vespera_macro/Cargo.toml":"Patch","crates/vespera_inprocess/Cargo.toml":"Patch","crates/vespera_jni/Cargo.toml":"Patch","libs/vespera-bridge/build.gradle.kts":"Patch","libs/vespera-bridge-gradle-plugin/build.gradle.kts":"Patch"},"note":"Fix round-7 macro parser findings: serde(transparent) Query/Path structs follow serde's transparent-field rule (PhantomData and serde(default) fields excluded) and tuple structs, generic value_type substitution keeps commas inside angle brackets (HashMap), request-view recursive generic instances use a .Input suffix that never collides with a type named *Input, two flattened maps write anyOf (not allOf) in the response view, a NoContent error side is labelled as a successful 204, a bare argument of unknown type keeps the string path placeholder, and doc and test assertion fixes","date":"2026-11-15T00:00:00.000Z"} diff --git a/crates/vespera_macro/src/openapi_generator/snapshots/vespera_macro__openapi_generator__tests_parser_views__parser_views_recursive_generic_definitions.snap b/crates/vespera_macro/src/openapi_generator/snapshots/vespera_macro__openapi_generator__tests_parser_views__parser_views_recursive_generic_definitions.snap index e74b9c8b..2764367e 100644 --- a/crates/vespera_macro/src/openapi_generator/snapshots/vespera_macro__openapi_generator__tests_parser_views__parser_views_recursive_generic_definitions.snap +++ b/crates/vespera_macro/src/openapi_generator/snapshots/vespera_macro__openapi_generator__tests_parser_views__parser_views_recursive_generic_definitions.snap @@ -100,13 +100,13 @@ expression: "serde_json::to_string_pretty(&components).unwrap()" "children" ] }, - "Tree_i32Input": { + "Tree_i32.Input": { "type": "object", "properties": { "children": { "type": "array", "items": { - "$ref": "#/components/schemas/Tree_i32Input" + "$ref": "#/components/schemas/Tree_i32.Input" } }, "value": { diff --git a/crates/vespera_macro/src/parser/operation/snapshots/vespera_macro__parser__operation__tests__dedup_bare_custom_extractor.snap b/crates/vespera_macro/src/parser/operation/snapshots/vespera_macro__parser__operation__tests__dedup_bare_custom_extractor.snap index 8fdeef00..b919db30 100644 --- a/crates/vespera_macro/src/parser/operation/snapshots/vespera_macro__parser__operation__tests__dedup_bare_custom_extractor.snap +++ b/crates/vespera_macro/src/parser/operation/snapshots/vespera_macro__parser__operation__tests__dedup_bare_custom_extractor.snap @@ -1,18 +1,16 @@ --- source: crates/vespera_macro/src/parser/operation/tests.rs -expression: "params.iter().map(|p|\n(p.name.as_str(), p.r#in, param_schema_type(p))).collect::>()" +expression: "params.iter().map(|p|\n{\n (p.name.as_str(), p.r#in, serde_json::to_string(&p.schema).unwrap(),)\n}).collect::>()" --- [ ( "org", Path, - None, + "{\"type\":\"string\"}", ), ( "repo", Path, - Some( - String, - ), + "{\"type\":\"string\"}", ), ] diff --git a/crates/vespera_macro/src/parser/operation/snapshots/vespera_macro__parser__operation__tests__dedup_bare_typed.snap b/crates/vespera_macro/src/parser/operation/snapshots/vespera_macro__parser__operation__tests__dedup_bare_typed.snap index 222b6ce6..49bdb0ba 100644 --- a/crates/vespera_macro/src/parser/operation/snapshots/vespera_macro__parser__operation__tests__dedup_bare_typed.snap +++ b/crates/vespera_macro/src/parser/operation/snapshots/vespera_macro__parser__operation__tests__dedup_bare_typed.snap @@ -1,13 +1,11 @@ --- source: crates/vespera_macro/src/parser/operation/tests.rs -expression: "params.iter().map(|p|\n(p.name.as_str(), p.r#in, param_schema_type(p))).collect::>()" +expression: "params.iter().map(|p|\n{\n (p.name.as_str(), p.r#in, serde_json::to_string(&p.schema).unwrap(),)\n}).collect::>()" --- [ ( "org", Path, - Some( - Integer, - ), + "{\"type\":\"integer\",\"format\":\"int64\"}", ), ] diff --git a/crates/vespera_macro/src/parser/operation/snapshots/vespera_macro__parser__operation__tests__dedup_path_extractor_wins.snap b/crates/vespera_macro/src/parser/operation/snapshots/vespera_macro__parser__operation__tests__dedup_path_extractor_wins.snap index 82c943b5..501d5098 100644 --- a/crates/vespera_macro/src/parser/operation/snapshots/vespera_macro__parser__operation__tests__dedup_path_extractor_wins.snap +++ b/crates/vespera_macro/src/parser/operation/snapshots/vespera_macro__parser__operation__tests__dedup_path_extractor_wins.snap @@ -1,13 +1,11 @@ --- source: crates/vespera_macro/src/parser/operation/tests.rs -expression: "params.iter().map(|p|\n(p.name.as_str(), p.r#in, param_schema_type(p))).collect::>()" +expression: "params.iter().map(|p|\n{\n (p.name.as_str(), p.r#in, serde_json::to_string(&p.schema).unwrap(),)\n}).collect::>()" --- [ ( "org", Path, - Some( - String, - ), + "{\"type\":\"string\"}", ), ] diff --git a/crates/vespera_macro/src/parser/operation/snapshots/vespera_macro__parser__operation__tests_request_shapes__request_shapes_known_enum.snap b/crates/vespera_macro/src/parser/operation/snapshots/vespera_macro__parser__operation__tests_request_shapes__request_shapes_known_enum.snap index 1033bd54..bf7c1afb 100644 --- a/crates/vespera_macro/src/parser/operation/snapshots/vespera_macro__parser__operation__tests_request_shapes__request_shapes_known_enum.snap +++ b/crates/vespera_macro/src/parser/operation/snapshots/vespera_macro__parser__operation__tests_request_shapes__request_shapes_known_enum.snap @@ -8,7 +8,7 @@ expression: "serde_json::to_string_pretty(&summary).unwrap()" "parameters": [ { "in": "query", - "name": "kind", + "name": "filter", "required": true, "schema": { "$ref": "#/components/schemas/Kind" diff --git a/crates/vespera_macro/src/parser/operation/snapshots/vespera_macro__parser__operation__tests_request_shapes__request_shapes_optional_query_enum.snap b/crates/vespera_macro/src/parser/operation/snapshots/vespera_macro__parser__operation__tests_request_shapes__request_shapes_optional_query_enum.snap index c8a7f4f6..3770b225 100644 --- a/crates/vespera_macro/src/parser/operation/snapshots/vespera_macro__parser__operation__tests_request_shapes__request_shapes_optional_query_enum.snap +++ b/crates/vespera_macro/src/parser/operation/snapshots/vespera_macro__parser__operation__tests_request_shapes__request_shapes_optional_query_enum.snap @@ -8,7 +8,7 @@ expression: "serde_json::to_string_pretty(&summary).unwrap()" "parameters": [ { "in": "query", - "name": "kind", + "name": "filter", "required": false, "schema": { "$ref": "#/components/schemas/Kind" diff --git a/crates/vespera_macro/src/parser/operation/tests.rs b/crates/vespera_macro/src/parser/operation/tests.rs index b56fc776..fe39df27 100644 --- a/crates/vespera_macro/src/parser/operation/tests.rs +++ b/crates/vespera_macro/src/parser/operation/tests.rs @@ -74,18 +74,13 @@ fn assert_body(op: &Operation, expected: Option<&ExpectedBody>) { None => assert!(op.request_body.is_none()), Some(exp) => { let body = op.request_body.as_ref().expect("request body expected"); - let media = body - .content - .get(exp.content_type) - .or_else(|| { - // allow fallback to the only available content type if expected is absent - if body.content.len() == 1 { - body.content.values().next() - } else { - None - } - }) - .expect("expected content type"); + let media = body.content.get(exp.content_type).unwrap_or_else(|| { + panic!( + "expected {}, got {:?}", + exp.content_type, + body.content.keys() + ) + }); if let Some(schema_ty) = &exp.schema { match media.schema.as_ref().expect("schema expected") { SchemaRef::Inline(schema) => { @@ -111,22 +106,30 @@ fn assert_params(op: &Operation, expected: &[ExpectedParam]) { } } +/// Exactly the `expected` statuses are documented (no stray inferred one). fn assert_responses(op: &Operation, expected: &[ExpectedResp]) { + assert_eq!( + op.responses.keys().collect::>(), + expected.iter().map(|exp| exp.status).collect::>() + ); for exp in expected { - let resp = op.responses.get(exp.status).expect("response missing"); + let resp = &op.responses[exp.status]; + // `schema: None` = a description-only response (no body). + let Some(schema_ty) = &exp.schema else { + assert!(resp.content.is_none(), "{}: {:?}", exp.status, resp.content); + continue; + }; let media = resp .content .as_ref() .and_then(|c| c.get("application/json")) .or_else(|| resp.content.as_ref().and_then(|c| c.get("text/plain"))) .expect("media type missing"); - if let Some(schema_ty) = &exp.schema { - match media.schema.as_ref().expect("schema expected") { - SchemaRef::Inline(schema) => { - assert_eq!(schema.schema_type, Some(*schema_ty)); - } - SchemaRef::Ref(_) => panic!("expected inline schema"), + match media.schema.as_ref().expect("schema expected") { + SchemaRef::Inline(schema) => { + assert_eq!(schema.schema_type, Some(*schema_ty)); } + SchemaRef::Ref(_) => panic!("expected inline schema"), } } } @@ -320,13 +323,18 @@ fn test_build_operation_summary_and_deprecated() { Some(ExpectedBody { content_type: "text/plain", schema: Some(SchemaType::String) }), vec![ExpectedResp { status: "200", schema: Some(SchemaType::String) }] )] +// A declared status on a handler without an error body is still documented, +// description only. #[case( "fn no_error_extra() -> String", "/plain", Some(&[500u16][..]), vec![], None, - vec![ExpectedResp { status: "200", schema: Some(SchemaType::String) }] + vec![ + ExpectedResp { status: "200", schema: Some(SchemaType::String) }, + ExpectedResp { status: "500", schema: None }, + ] )] #[case( "fn create() -> Result", @@ -391,6 +399,16 @@ fn typed_responses_use_schema_refs_and_override_error_status() { } SchemaRef::Inline(_) => panic!("typed response must use schema ref"), } + // Replaced, not merged with the inferred `text/plain` error body. + assert_eq!( + response + .content + .as_ref() + .unwrap() + .keys() + .collect::>(), + ["application/json"] + ); assert!(op.responses.contains_key("500")); } @@ -699,10 +717,10 @@ fn test_non_path_extractor_with_query() { None, ); - // Test: Query params should be extended to parameters - // But QueryParams is not in known_schemas/struct_definitions so it won't appear - // The key is that it doesn't treat Query as a Path extractor (line 85 returns false) - assert!(op.request_body.is_none()); // Query is not a body + // QueryParams is not in known_schemas/struct_definitions, so it documents + // no parameter (and `/search` has no segment to type); Query is no body. + assert!(op.parameters.is_none(), "{:?}", op.parameters); + assert!(op.request_body.is_none()); } #[test] @@ -825,14 +843,12 @@ fn test_non_path_extractor_generates_params_and_extends() { OperationRouteConfig::default(), ); - // Query is not Path (line 85 returns false) - // parse_function_parameter returns Some for Query - // Line 89: parameters.extend(params) - // TypedHeader also generates a header parameter - assert!(op.parameters.is_some()); - let params = op.parameters.unwrap(); - // Should have query param(s) and header param - assert!(!params.is_empty()); + // `Query` documents its field `q`. `TypedHeader` + // documents nothing: OpenAPI forbids an `Authorization` header parameter + // (it belongs to a security scheme). + let params = op.parameters.expect("query parameters"); + let params: Vec<_> = params.iter().map(|p| (p.name.as_str(), p.r#in)).collect(); + assert_eq!(params, [("q", ParameterLocation::Query)]); } #[test] @@ -922,7 +938,13 @@ fn path_parameters_are_deduplicated(#[case] snapshot: &str, #[case] sig: &str, # snapshot, params .iter() - .map(|p| (p.name.as_str(), p.r#in, param_schema_type(p))) + .map(|p| { + ( + p.name.as_str(), + p.r#in, + serde_json::to_string(&p.schema).unwrap(), + ) + }) .collect::>() ); } diff --git a/crates/vespera_macro/src/parser/operation/tests_extractor_wrappers.rs b/crates/vespera_macro/src/parser/operation/tests_extractor_wrappers.rs index 0e63b80b..7b0bc6ae 100644 --- a/crates/vespera_macro/src/parser/operation/tests_extractor_wrappers.rs +++ b/crates/vespera_macro/src/parser/operation/tests_extractor_wrappers.rs @@ -1,6 +1,7 @@ //! Request extractors behind `Result` (the handler takes the -//! rejection itself) document exactly like the bare extractor — still -//! required, unlike `Option` — and `Path<(..)>` tuples longer than the +//! rejection itself) document the same body / parameters as the bare +//! extractor — still required, unlike `Option`; no `422`, since the +//! handler owns the rejection — and `Path<(..)>` tuples longer than the //! route's own parameters (a `Router::nest("/{tenant}", ..)` prefix fills //! the leading elements) type the route's segments from the END. diff --git a/crates/vespera_macro/src/parser/operation/tests_request_shapes.rs b/crates/vespera_macro/src/parser/operation/tests_request_shapes.rs index 44605cb1..2d85ab92 100644 --- a/crates/vespera_macro/src/parser/operation/tests_request_shapes.rs +++ b/crates/vespera_macro/src/parser/operation/tests_request_shapes.rs @@ -76,8 +76,9 @@ fn operation(sig: &str, path: &str) -> Operation { "async fn f(OptionalQuery(q): axum_extra::extract::OptionalQuery)", "/x" )] -#[case::optional_query_enum("optional_query_enum", "async fn f(kind: OptionalQuery)", "/x")] -// 2: `WithRejection` / `Cached` document `E`. +#[case::optional_query_enum("optional_query_enum", "async fn f(filter: OptionalQuery)", "/x")] +// 2: `WithRejection` / `Cached` document `E`'s body / parameters +// (`WithRejection` remaps the rejection, so a `Validated` `E` adds no 422). #[case::with_rejection_json( "with_rejection_json", "async fn f(WithRejection(Json(b), _): WithRejection, ApiError>)", @@ -121,7 +122,9 @@ fn operation(sig: &str, path: &str) -> Operation { #[case::skipped_struct("skipped_struct", "async fn f(skipped: Query)", "/x")] #[case::flatten_map_only("flatten_map_only", "async fn f(rest: Query)", "/x")] #[case::tuple_struct("tuple_struct", "async fn f(w: Query)", "/x")] -#[case::known_enum("known_enum", "async fn f(kind: Query)", "/x")] +// A known non-struct (an internally tagged enum) is one parameter named +// after the binding (`filter`, not the enum's tag `kind`). +#[case::known_enum("known_enum", "async fn f(filter: Query)", "/x")] // 9: `Path>` / `Path>` type each segment. #[case::path_map("path_map", "async fn f(p: Path>)", "/x/{id}")] #[case::path_map_multi( diff --git a/crates/vespera_macro/src/parser/parameters/header.rs b/crates/vespera_macro/src/parser/parameters/header.rs index d3c4d981..991a8236 100644 --- a/crates/vespera_macro/src/parser/parameters/header.rs +++ b/crates/vespera_macro/src/parser/parameters/header.rs @@ -85,7 +85,8 @@ fn parse_header( } /// Wire names of the well-known `headers::*` typed headers. Any other type is -/// treated as a custom header and named after its Rust binding instead. +/// treated as a custom header named after its kebab-cased type name instead +/// (`XRequestId` → `x-request-id`; see [`typed_header_parameter`]). fn known_typed_header_name(ident: &str) -> Option<&'static str> { Some(match ident { "Authorization" => "Authorization", diff --git a/crates/vespera_macro/src/parser/parameters/path.rs b/crates/vespera_macro/src/parser/parameters/path.rs index bafff8e3..c9529510 100644 --- a/crates/vespera_macro/src/parser/parameters/path.rs +++ b/crates/vespera_macro/src/parser/parameters/path.rs @@ -8,7 +8,7 @@ use vespera_core::{ use super::shared::{ MAX_CONVERSION_DEPTH, deserialized_instead, input_rename_all, named_struct_fields, - serde_field_name, + serde_field_name, struct_definition, }; use crate::{ parser::schema::{ @@ -111,21 +111,25 @@ fn path_parameters_from( .collect(); } - if let Some(struct_item) = named_struct_fields(inner_ty, struct_definitions) { - // Container `#[serde(from = "X")]` / `try_from` (or `#[schema(value_type - // = X)]`), or `#[serde(transparent)]`: the segments are deserialized - // as `X` / the single field's type, not as these fields. - if depth < MAX_CONVERSION_DEPTH - && let Some(source) = deserialized_instead(&struct_item) - { - return path_parameters_from( - &source, - path_params, - known_schemas, - struct_definitions, - depth + 1, - ); - } + let struct_item = struct_definition(inner_ty, struct_definitions); + // Container `#[serde(from = "X")]` / `try_from` (or `#[schema(value_type + // = X)]`), or `#[serde(transparent)]` (named or tuple struct): the + // segments are deserialized as `X` / the transparent field's type, not + // as these fields. + if depth < MAX_CONVERSION_DEPTH + && let Some(source) = struct_item.as_ref().and_then(deserialized_instead) + { + return path_parameters_from( + &source, + path_params, + known_schemas, + struct_definitions, + depth + 1, + ); + } + if let Some(struct_item) = + struct_item.filter(|struct_item| matches!(struct_item.fields, syn::Fields::Named(_))) + { return path_params .iter() .map(|name| { @@ -260,12 +264,18 @@ pub(super) fn parse_bare_path_parameter( known_schemas: &HashSet<&str>, struct_definitions: &HashMap<&str, &str>, ) -> Option> { - path_param_set.contains(param_name).then(|| { - vec![path_parameter( - param_name, - parse_type_to_schema_ref(ty, known_schemas, struct_definitions), - )] - }) + if !path_param_set.contains(param_name) { + return None; + } + let schema = parse_type_to_schema_ref(ty, known_schemas, struct_definitions); + // A type with no known shape (a custom extractor such as `org: OrgCtx`) + // says nothing about the segment: keep its `{type: string}` placeholder + // rather than widening it to any value (`{}`). + if matches!(&schema, SchemaRef::Inline(inline) if serde_json::to_string(inline).is_ok_and(|json| json == "{}")) + { + return None; + } + Some(vec![path_parameter(param_name, schema)]) } #[cfg(test)] diff --git a/crates/vespera_macro/src/parser/parameters/query.rs b/crates/vespera_macro/src/parser/parameters/query.rs index f90bb8e0..adb5f1aa 100644 --- a/crates/vespera_macro/src/parser/parameters/query.rs +++ b/crates/vespera_macro/src/parser/parameters/query.rs @@ -9,6 +9,7 @@ use vespera_core::{ use super::shared::{ convert_to_inline_schema, deserialized_instead, deserialized_source, input_rename_all, is_defined_struct, is_known_type, is_primitive_or_like, named_struct_fields, serde_field_name, + struct_definition, }; use crate::{ parser::schema::{ @@ -163,7 +164,7 @@ fn collect_query_struct_parameters( visiting: &mut Vec, parameters: &mut Vec, ) { - let Some(struct_item) = named_struct_fields(ty, struct_definitions) else { + let Some(struct_item) = struct_definition(ty, struct_definitions) else { return; }; // A (boxed) struct that flattens itself would recurse forever. @@ -173,17 +174,21 @@ fn collect_query_struct_parameters( } visiting.push(struct_name); // Container `#[serde(from = "X")]` / `try_from` (or `#[schema(value_type = - // X)]`), or `#[serde(transparent)]`: serde reads `X` / the single field's - // type from the query string, never this struct's own fields. - if let Some(source) = deserialized_instead(&struct_item) { + // X)]`), or `#[serde(transparent)]` (named or tuple struct): serde reads + // `X` / the transparent field's type from the query string, never this + // struct's own fields. Any other tuple / unit struct has no named keys. + let source = deserialized_instead(&struct_item); + if let Some(source) = &source { collect_query_struct_parameters( - &source, + source, force_optional, known_schemas, struct_definitions, visiting, parameters, ); + } + if source.is_some() || !matches!(struct_item.fields, syn::Fields::Named(_)) { visiting.pop(); return; } diff --git a/crates/vespera_macro/src/parser/parameters/shared.rs b/crates/vespera_macro/src/parser/parameters/shared.rs index e65e93e4..35b200ed 100644 --- a/crates/vespera_macro/src/parser/parameters/shared.rs +++ b/crates/vespera_macro/src/parser/parameters/shared.rs @@ -6,8 +6,8 @@ use vespera_core::schema::SchemaRef; use crate::{ parser::schema::{ SerdeSide, definition_key, extract_field_rename_for, extract_rename_all_for, - extract_serde_skip_flags, extract_transparent, is_primitive_type, nullable_schema_ref, - rename_field, strip_raw_prefix_owned, substitute_type, + extract_transparent, is_primitive_type, nullable_schema_ref, rename_field, + single_emitted_field, strip_raw_prefix_owned, substitute_type, wire_override::container_override_type, }, schema_macro::type_utils::is_primitive_like as utils_is_primitive_like, @@ -61,13 +61,23 @@ pub(super) fn is_known_type( } /// The struct named by `ty`'s last path segment, when `struct_definitions` -/// holds it as a struct with **named** fields. +/// holds it as a struct with **named** fields (see [`struct_definition`]). +pub(super) fn named_struct_fields( + ty: &Type, + struct_definitions: &HashMap<&str, &str>, +) -> Option { + struct_definition(ty, struct_definitions) + .filter(|struct_item| matches!(struct_item.fields, syn::Fields::Named(_))) +} + +/// The struct named by `ty`'s last path segment, of any shape (named, tuple +/// or unit), when `struct_definitions` holds it. /// /// A generic struct's field types are instantiated with `ty`'s type /// arguments (`Paged` → `T` = `Filter`; a missing argument takes the /// parameter's default), so a flattened `T` or a `T`-typed field documents /// the concrete type. -pub(super) fn named_struct_fields( +pub(super) fn struct_definition( ty: &Type, struct_definitions: &HashMap<&str, &str>, ) -> Option { @@ -78,9 +88,6 @@ pub(super) fn named_struct_fields( let key = definition_key(&type_path.path, |key| struct_definitions.contains_key(key))?; let mut struct_item = syn::parse_str::(struct_definitions.get(key.as_str())?).ok()?; - if !matches!(struct_item.fields, syn::Fields::Named(_)) { - return None; - } let names: Vec = struct_item .generics .type_params() @@ -132,24 +139,17 @@ pub(super) fn named_struct_fields( /// (a cycle stops). pub(super) const MAX_CONVERSION_DEPTH: usize = 8; -/// The type serde reads INSTEAD of `struct_item`'s own fields: the container -/// `from` / `try_from` (or `#[schema(value_type)]`) source, or the single -/// field of a `#[serde(transparent)]` struct (serde requires every other -/// field to be skipped). +/// The type serde reads INSTEAD of `struct_item`'s own fields (named or +/// tuple): the container `from` / `try_from` (or `#[schema(value_type)]`) +/// source, or the transparent field of a `#[serde(transparent)]` struct — +/// serde's rule: the only field that is not `PhantomData`, not skipped and +/// not `#[serde(default)]` on the deserialize side. pub(super) fn deserialized_instead(struct_item: &syn::ItemStruct) -> Option { container_override_type(&struct_item.attrs, SerdeSide::Deserialize).or_else(|| { extract_transparent(&struct_item.attrs) - .then(|| { - struct_item - .fields - .iter() - .find(|field| { - let flags = extract_serde_skip_flags(&field.attrs); - !(flags.skip || flags.skip_deserializing) - }) - .map(|field| field.ty.clone()) - }) + .then(|| single_emitted_field(struct_item.fields.iter(), SerdeSide::Deserialize)) .flatten() + .map(|field| field.ty.clone()) }) } @@ -159,7 +159,7 @@ pub(super) fn deserialized_instead(struct_item: &syn::ItemStruct) -> Option) -> Type { let mut current = ty.clone(); for _ in 0..MAX_CONVERSION_DEPTH { - match named_struct_fields(¤t, struct_definitions) + match struct_definition(¤t, struct_definitions) .as_ref() .and_then(deserialized_instead) { diff --git a/crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_path_transparent_phantom.snap b/crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_path_transparent_phantom.snap new file mode 100644 index 00000000..500647e4 --- /dev/null +++ b/crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_path_transparent_phantom.snap @@ -0,0 +1,26 @@ +--- +source: crates/vespera_macro/src/parser/parameters/tests_shapes.rs +expression: "render(func_src, path_params)" +--- +fn f(p: Path>) {} +[ + { + "name": "a", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int64" + } + }, + { + "name": "b", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "uint64", + "minimum": 0 + } + } +] diff --git a/crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_path_transparent_tuple.snap b/crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_path_transparent_tuple.snap new file mode 100644 index 00000000..c663c26d --- /dev/null +++ b/crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_path_transparent_tuple.snap @@ -0,0 +1,26 @@ +--- +source: crates/vespera_macro/src/parser/parameters/tests_shapes.rs +expression: "render(func_src, path_params)" +--- +fn f(p: Path) {} +[ + { + "name": "a", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int64" + } + }, + { + "name": "b", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "uint64", + "minimum": 0 + } + } +] diff --git a/crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_query_from_tuple.snap b/crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_query_from_tuple.snap new file mode 100644 index 00000000..f273277f --- /dev/null +++ b/crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_query_from_tuple.snap @@ -0,0 +1,15 @@ +--- +source: crates/vespera_macro/src/parser/parameters/tests_shapes.rs +expression: "render(func_src, path_params)" +--- +fn f(q: Query) {} +[ + { + "name": "q", + "in": "query", + "required": true, + "schema": { + "type": "string" + } + } +] diff --git a/crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_query_transparent_default.snap b/crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_query_transparent_default.snap new file mode 100644 index 00000000..e14aae1b --- /dev/null +++ b/crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_query_transparent_default.snap @@ -0,0 +1,15 @@ +--- +source: crates/vespera_macro/src/parser/parameters/tests_shapes.rs +expression: "render(func_src, path_params)" +--- +fn f(q: Query) {} +[ + { + "name": "q", + "in": "query", + "required": true, + "schema": { + "type": "string" + } + } +] diff --git a/crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_query_transparent_phantom.snap b/crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_query_transparent_phantom.snap new file mode 100644 index 00000000..996750b6 --- /dev/null +++ b/crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_query_transparent_phantom.snap @@ -0,0 +1,15 @@ +--- +source: crates/vespera_macro/src/parser/parameters/tests_shapes.rs +expression: "render(func_src, path_params)" +--- +fn f(q: Query>) {} +[ + { + "name": "q", + "in": "query", + "required": true, + "schema": { + "type": "string" + } + } +] diff --git a/crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_query_transparent_tuple.snap b/crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_query_transparent_tuple.snap new file mode 100644 index 00000000..bf719746 --- /dev/null +++ b/crates/vespera_macro/src/parser/parameters/snapshots/vespera_macro__parser__parameters__tests_shapes__parameter_shapes_query_transparent_tuple.snap @@ -0,0 +1,15 @@ +--- +source: crates/vespera_macro/src/parser/parameters/tests_shapes.rs +expression: "render(func_src, path_params)" +--- +fn f(q: Query) {} +[ + { + "name": "q", + "in": "query", + "required": true, + "schema": { + "type": "string" + } + } +] diff --git a/crates/vespera_macro/src/parser/parameters/tests_shapes.rs b/crates/vespera_macro/src/parser/parameters/tests_shapes.rs index d12792df..172acf9d 100644 --- a/crates/vespera_macro/src/parser/parameters/tests_shapes.rs +++ b/crates/vespera_macro/src/parser/parameters/tests_shapes.rs @@ -21,6 +21,30 @@ const DEFINITIONS: &[(&str, &str)] = &[ ), ("RepoKey", "pub struct RepoKey { pub repo: i32 }"), ("Keyed", "pub struct Keyed { pub key: K }"), + // `#[serde(transparent)]` reads its ONE field that is neither + // `PhantomData` nor skipped / defaulted on the deserialize side. + ( + "PhantomQ", + "#[serde(transparent)] pub struct PhantomQ { pub _p: PhantomData, pub inner: Filter }", + ), + ( + "DefaultQ", + "#[serde(transparent)] pub struct DefaultQ { #[serde(default)] pub page: u32, pub inner: Filter }", + ), + ("WrapQ", "#[serde(transparent)] pub struct WrapQ(Filter);"), + ( + "FromTuple", + r#"#[serde(from = "Filter")] pub struct FromTuple(String);"#, + ), + ("TwoIds", "pub struct TwoIds { pub a: i64, pub b: u64 }"), + ( + "PhantomPath", + "#[serde(transparent)] pub struct PhantomPath { pub _p: std::marker::PhantomData, pub ids: TwoIds }", + ), + ( + "WrapPath", + "#[serde(transparent)] pub struct WrapPath(TwoIds);", + ), // Two `#[schema(name)]` renames sharing the ident `Foo` (`a::Foo` = `A`, // `b::Foo` = `B`); the bare ident aliases the first. ( @@ -95,6 +119,22 @@ fn render(func_src: &str, path_params: &[&str]) -> String { // module declares. #[case::query_scoped_rename("query_scoped_rename", "fn f(q: Query) {}", &[])] #[case::path_scoped_rename("path_scoped_rename", "fn f(p: Path) {}", &["b_field"])] +// Round 7: a transparent struct reads its single deserialized field (never +// a `PhantomData` / `#[serde(default)]` one); tuple structs are followed too. +#[case::query_transparent_phantom( + "query_transparent_phantom", + "fn f(q: Query>) {}", + &[] +)] +#[case::query_transparent_default("query_transparent_default", "fn f(q: Query) {}", &[])] +#[case::query_transparent_tuple("query_transparent_tuple", "fn f(q: Query) {}", &[])] +#[case::query_from_tuple("query_from_tuple", "fn f(q: Query) {}", &[])] +#[case::path_transparent_phantom( + "path_transparent_phantom", + "fn f(p: Path>) {}", + &["a", "b"] +)] +#[case::path_transparent_tuple("path_transparent_tuple", "fn f(p: Path) {}", &["a", "b"])] #[case::raw_identifier_bare("raw_identifier_bare", "fn f(r#type: i32) {}", &["type"])] fn parameter_shapes(#[case] name: &str, #[case] func_src: &str, #[case] path_params: &[&str]) { insta::assert_snapshot!( diff --git a/crates/vespera_macro/src/parser/response.rs b/crates/vespera_macro/src/parser/response.rs index e2e5b116..ad67df4c 100644 --- a/crates/vespera_macro/src/parser/response.rs +++ b/crates/vespera_macro/src/parser/response.rs @@ -414,11 +414,14 @@ pub fn parse_return_type( responses .entry(key.to_string()) .or_insert_with(|| Response { - description: if key == "3XX" { - "Redirect".to_string() - } else { - "Error response".to_string() - }, + // Labelled by the status it is keyed under: a `NoContent` + // error is still a 2xx. + description: match key { + "3XX" => "Redirect", + "204" => "Successful response", + _ => "Error response", + } + .to_string(), headers: None, content: response_content(&err_ty, known_schemas, struct_definitions), ..Default::default() diff --git a/crates/vespera_macro/src/parser/response/snapshots/vespera_macro__parser__response__tests_parts__response_parts_err_no_content.snap b/crates/vespera_macro/src/parser/response/snapshots/vespera_macro__parser__response__tests_parts__response_parts_err_no_content.snap index fa59d2e7..67e815df 100644 --- a/crates/vespera_macro/src/parser/response/snapshots/vespera_macro__parser__response__tests_parts__response_parts_err_no_content.snap +++ b/crates/vespera_macro/src/parser/response/snapshots/vespera_macro__parser__response__tests_parts__response_parts_err_no_content.snap @@ -14,6 +14,6 @@ expression: responses_json(return_type) } }, "204": { - "description": "Error response" + "description": "Successful response" } } diff --git a/crates/vespera_macro/src/parser/response/tests.rs b/crates/vespera_macro/src/parser/response/tests.rs index f8630da8..b7bf935e 100644 --- a/crates/vespera_macro/src/parser/response/tests.rs +++ b/crates/vespera_macro/src/parser/response/tests.rs @@ -397,9 +397,8 @@ fn test_extract_result_types_with_reference() { fn test_unwrap_json_non_json() { // Test unwrap_json with non-Json type returns original let ty: syn::Type = syn::parse_str("String").unwrap(); - let unwrapped = unwrap_json(&ty); - // Should return the same type - assert!(matches!(unwrapped, syn::Type::Path(_))); + // The very same type, not a rebuilt one. + assert!(std::ptr::eq(unwrap_json(&ty), &raw const ty)); } #[test] @@ -440,12 +439,13 @@ fn test_extract_ok_payload_and_headers_tuple_without_headermap() { let payload = payload.expect("body payload"); // Payload should be String (last element unwrapped) - if let syn::Type::Path(type_path) = &payload { - assert_eq!( - type_path.path.segments.last().unwrap().ident.to_string(), - "String" - ); - } + let syn::Type::Path(type_path) = &payload else { + panic!("expected path payload") + }; + assert_eq!( + type_path.path.segments.last().unwrap().ident.to_string(), + "String" + ); // Headers should be None (no HeaderMap in tuple) - this is line 95 assert!(headers.is_none()); } @@ -460,10 +460,16 @@ fn test_parse_return_type_result_with_ok_tuple_no_headermap() { let responses = parse_return_type(&return_type, &known_schemas, &struct_definitions); // Should have 200 and 400 responses - assert!(responses.contains_key("200")); + assert_eq!(responses.keys().collect::>(), ["200", "400"]); let ok_response = responses.get("200").unwrap(); // Headers should be None assert!(ok_response.headers.is_none()); + // The body is the tuple's `Json` element. + let media = &ok_response.content.as_ref().unwrap()["application/json"]; + let Some(SchemaRef::Inline(schema)) = &media.schema else { + panic!("expected inline schema") + }; + assert_eq!(schema.schema_type, Some(SchemaType::String)); } // ======== CookieJar tuple extraction tests ======== @@ -592,15 +598,7 @@ fn test_unwrap_json_qualified_path() { fn test_unwrap_json_non_generic_path() { // Type with segments but no angle brackets → returns original let ty: syn::Type = syn::parse_str("std::string::String").unwrap(); - let unwrapped = unwrap_json(&ty); - if let syn::Type::Path(type_path) = unwrapped { - assert_eq!( - type_path.path.segments.last().unwrap().ident.to_string(), - "String" - ); - } else { - panic!("Expected Path type"); - } + assert!(std::ptr::eq(unwrap_json(&ty), &raw const ty)); } #[test] @@ -727,7 +725,7 @@ fn media_shape( #[case::unknown_named_type("-> CustomResponse", "200", None)] #[case::result_lifetime_arg("-> Result<'static, Json>", "200", Some(("application/json", SchemaType::Integer, Some("int32"))))] #[case::header_array_in_tuple("-> (StatusCode, [(HeaderName, &'static str); 1])", "200", None)] -#[case::reference_to_non_path("-> &'static [i32]", "200", Some(("application/json", SchemaType::Array, None)))] +#[case::reference_to_non_path("-> &'static (i32, i32)", "200", Some(("application/json", SchemaType::Array, None)))] fn opaque_and_raw_bodies( #[case] return_type: &str, #[case] status: &str, diff --git a/crates/vespera_macro/src/parser/response/tests_parts.rs b/crates/vespera_macro/src/parser/response/tests_parts.rs index fe268734..ab127ef0 100644 --- a/crates/vespera_macro/src/parser/response/tests_parts.rs +++ b/crates/vespera_macro/src/parser/response/tests_parts.rs @@ -57,7 +57,8 @@ fn responses_json(return_type: &str) -> String { #[case::err_status_unresolved("err_status_unresolved", "-> Result")] #[case::err_json_unknown("err_json_unknown", "-> Result>")] // 13: the error side is keyed like the success one (`Redirect` → `3XX`, -// `NoContent` → `204`); an `Infallible` / `!` error never happens. +// `NoContent` → `204`, labelled by that status: "Redirect" / "Successful +// response"); an `Infallible` / `!` error never happens. #[case::err_redirect("err_redirect", "-> Result, Redirect>")] #[case::err_no_content("err_no_content", "-> Result, (HeaderMap, NoContent)>")] #[case::redirect_both("redirect_both", "-> Result")] diff --git a/crates/vespera_macro/src/parser/schema/enum_schema/representations/tests.rs b/crates/vespera_macro/src/parser/schema/enum_schema/representations/tests.rs index e7eafba2..6260a5e5 100644 --- a/crates/vespera_macro/src/parser/schema/enum_schema/representations/tests.rs +++ b/crates/vespera_macro/src/parser/schema/enum_schema/representations/tests.rs @@ -12,6 +12,23 @@ fn inline(schema_ref: &SchemaRef) -> &Schema { } } +/// The required, single-value `enum` of `variant`'s `tag` property. +fn tag_value(variant: &SchemaRef, tag: &str) -> String { + let variant = inline(variant); + assert!( + variant + .required + .as_ref() + .unwrap() + .iter() + .any(|key| key == tag) + ); + let tag_schema = inline(&variant.properties.as_ref().unwrap()[tag]); + let values = tag_schema.r#enum.as_ref().expect("tag enum missing"); + assert_eq!(values.len(), 1); + values[0].as_str().unwrap().to_string() +} + // Internally tagged enum tests #[test] fn test_internally_tagged_enum_unit_variants() { @@ -43,14 +60,9 @@ fn test_internally_tagged_enum_unit_variants() { let one_of = schema.one_of.expect("one_of missing"); assert_eq!(one_of.len(), 2); - // Each variant should be an object with "type" property - { - let ping = inline(&one_of[0]); - let props = ping.properties.as_ref().expect("properties missing"); - assert!(props.contains_key("type")); - let required = ping.required.as_ref().expect("required missing"); - assert!(required.contains(&"type".to_string())); - } + // Each variant is an object whose required "type" names it + let tags: Vec = one_of.iter().map(|v| tag_value(v, "type")).collect(); + assert_eq!(tags, ["Ping", "Pong"]); } #[test] @@ -82,14 +94,20 @@ fn test_internally_tagged_enum_struct_variants() { let one_of = schema.one_of.expect("one_of missing"); assert_eq!(one_of.len(), 2); - // Created variant should have kind, id, and name - { - let created = inline(&one_of[0]); - let props = created.properties.as_ref().expect("properties missing"); - assert!(props.contains_key("kind")); - assert!(props.contains_key("id")); - assert!(props.contains_key("name")); - } + // Each variant carries its tag next to its own fields + let tags: Vec = one_of.iter().map(|v| tag_value(v, "kind")).collect(); + assert_eq!(tags, ["Created", "Updated"]); + let keys = |variant: &SchemaRef| -> Vec { + inline(variant) + .properties + .as_ref() + .unwrap() + .keys() + .cloned() + .collect() + }; + assert_eq!(keys(&one_of[0]), ["id", "kind", "name"]); + assert_eq!(keys(&one_of[1]), ["id", "kind"]); } #[test] @@ -153,16 +171,20 @@ fn test_adjacently_tagged_enum_basic() { let one_of = schema.one_of.expect("one_of missing"); assert_eq!(one_of.len(), 2); - // Each variant should have "type" and "data" properties - { - let success = inline(&one_of[0]); - let props = success.properties.as_ref().expect("properties missing"); - assert!(props.contains_key("type")); - assert!(props.contains_key("data")); - - let required = success.required.as_ref().expect("required missing"); - assert!(required.contains(&"type".to_string())); - assert!(required.contains(&"data".to_string())); + // Each variant has its required "type" tag and its required "data" + let tags: Vec = one_of.iter().map(|v| tag_value(v, "type")).collect(); + assert_eq!(tags, ["Success", "Error"]); + for variant in &one_of { + let variant = inline(variant); + assert!(variant.properties.as_ref().unwrap().contains_key("data")); + assert!( + variant + .required + .as_ref() + .unwrap() + .iter() + .any(|key| key == "data") + ); } } @@ -541,13 +563,25 @@ fn internally_tagged_scalar_newtype_is_skipped_by_generator() { &HashMap::::new(), ); // `Unit(())` → tag only; `Empty` → tag only. - assert_eq!(schema.one_of.expect("one_of").len(), 2); + let one_of = schema.one_of.expect("one_of"); + let tags: Vec = one_of.iter().map(|v| tag_value(v, "type")).collect(); + assert_eq!(tags, ["Unit", "Empty"]); + for variant in &one_of { + let keys: Vec<&String> = inline(variant) + .properties + .as_ref() + .unwrap() + .keys() + .collect(); + assert_eq!(keys, ["type"]); + } } /// Struct-variant fields share the struct field walker: `skip`, `flatten`, /// `rename`, `skip_serializing_if`, doc comments and `#[schema]` constraints /// all apply. Variant-level `skip` / `skip_serializing` drop the variant; -/// `#[serde(other)]` accepts any tag except the known ones. +/// `#[serde(other)]` writes its own name (this is the response view; the +/// request view, which accepts any unknown tag, is in `tests_fidelity.rs`). #[rstest::rstest] #[case::external_field_attrs( "external_field_attrs", @@ -662,13 +696,15 @@ fn adjacently_tagged_empty_enum_has_no_one_of() { } #[test] -fn untagged_empty_enum_has_no_one_of_or_discriminator() { +fn untagged_empty_enum_has_no_union_or_discriminator() { let enum_item: syn::ItemEnum = syn::parse_str("#[serde(untagged)] enum Empty {}").unwrap(); let schema = parse_enum_to_schema( &enum_item, &HashSet::::new(), &HashMap::::new(), ); + // Untagged enums emit `anyOf`; an empty one emits no union at all. + assert!(schema.any_of.is_none()); assert!(schema.one_of.is_none()); assert!(schema.discriminator.is_none()); } diff --git a/crates/vespera_macro/src/parser/schema/enum_schema/tests_validate.rs b/crates/vespera_macro/src/parser/schema/enum_schema/tests_validate.rs index 973c77f9..33f14ba5 100644 --- a/crates/vespera_macro/src/parser/schema/enum_schema/tests_validate.rs +++ b/crates/vespera_macro/src/parser/schema/enum_schema/tests_validate.rs @@ -59,7 +59,9 @@ fn codec_and_value_type_payloads(#[case] variant: &str, #[case] is_error: bool) #[case::option_array("Option<[u8; 2]>", false)] #[case::qualified_self("::Output", false)] #[case::fn_pointer("fn() -> i32", false)] -#[case::cow_lifetime_first("std::borrow::Cow<'static, str>", false)] +// `Cow` is not a wrapper the classifier looks through (a `Cow` may be an +// object), so it is left to serde. +#[case::cow_is_not_classified("std::borrow::Cow<'static, str>", false)] fn internally_tagged_newtype_payload_classification(#[case] inner: &str, #[case] is_error: bool) { let inner: syn::Type = syn::parse_str(inner).unwrap(); let enum_item: syn::ItemEnum = syn::parse_quote! { diff --git a/crates/vespera_macro/src/parser/schema/mod.rs b/crates/vespera_macro/src/parser/schema/mod.rs index 4cdeed47..1f03f82c 100644 --- a/crates/vespera_macro/src/parser/schema/mod.rs +++ b/crates/vespera_macro/src/parser/schema/mod.rs @@ -52,7 +52,7 @@ pub use serde_attrs::{ extract_rename_all, extract_rename_all_for, extract_schema_default, extract_serde_skip_flags, extract_skip, extract_transparent, rename_field, strip_raw_prefix_owned, }; -pub use struct_schema::{apply_field_rules, parse_struct_to_schema}; +pub use struct_schema::{apply_field_rules, parse_struct_to_schema, single_emitted_field}; // Re-export for internal use within the parser module. `parse_type_to_schema_ref` // is reached directly via the `type_schema` submodule path where needed. pub use type_schema::generic_instances; diff --git a/crates/vespera_macro/src/parser/schema/schema_attrs.rs b/crates/vespera_macro/src/parser/schema/schema_attrs.rs index ab4170a1..a1bed592 100644 --- a/crates/vespera_macro/src/parser/schema/schema_attrs.rs +++ b/crates/vespera_macro/src/parser/schema/schema_attrs.rs @@ -85,7 +85,7 @@ pub struct SchemaConstraints { // ── schema-generation override ─────────────────────────────────── /// When `Some(true)`, the field's OpenAPI schema is forced to the - /// generic `{type: "object"}` regardless of its Rust type, AND the + /// "any JSON value" schema `{}` regardless of its Rust type, AND the /// compile-time `#[derive(Schema)]` leaf-type assertion is skipped /// for this field. Use this to opt a field out of strict /// "every-custom-type-must-derive-Schema" checking when the field @@ -154,8 +154,8 @@ impl SchemaConstraints { } /// `true` when the field is marked `#[schema(any)]` (intentionally - /// opaque object — skips the derive-Schema leaf-type assertion AND - /// overrides the emitted schema to `{type: "object"}`). + /// opaque value — skips the derive-Schema leaf-type assertion AND + /// overrides the emitted schema to the any-value `{}`). #[must_use] pub fn is_any(&self) -> bool { self.any == Some(true) @@ -260,7 +260,7 @@ pub fn try_extract_schema_constraints(attrs: &[Attribute]) -> syn::Result( +pub fn single_emitted_field<'a>( fields: impl Iterator, side: SerdeSide, ) -> Option<&'a syn::Field> { diff --git a/crates/vespera_macro/src/parser/schema/struct_schema/flatten.rs b/crates/vespera_macro/src/parser/schema/struct_schema/flatten.rs index 71283db1..5d476117 100644 --- a/crates/vespera_macro/src/parser/schema/struct_schema/flatten.rs +++ b/crates/vespera_macro/src/parser/schema/struct_schema/flatten.rs @@ -370,8 +370,12 @@ fn merge(target: &mut ObjectParts, parts: ObjectParts, optional: bool) -> bool { target.opaque.extend(parts.opaque); } target.closed |= parts.closed; + // Two flattened maps: serde READS every buffered entry into each map + // (the value must fit both), but WRITES each map's own entries (the + // value fits one of them). target.additional = match (target.additional.take(), parts.additional) { - (Some(first), Some(second)) => Some(both(first, second)), + (Some(first), Some(second)) if request_view::in_request_view() => Some(both(first, second)), + (Some(first), Some(second)) => Some(either(first, second)), (first, second) => first.or(second), }; open @@ -385,6 +389,14 @@ fn both(first: SchemaRef, second: SchemaRef) -> SchemaRef { })) } +/// `anyOf: [first, second]` — a value one of the schemas accepts. +fn either(first: SchemaRef, second: SchemaRef) -> SchemaRef { + SchemaRef::Inline(Box::new(Schema { + any_of: Some(vec![first, second]), + ..Schema::default() + })) +} + /// `anyOf: [schema, {}]` — documents `schema` without rejecting anything /// (a flattened `Option` is `None` when its value fails). fn lenient(schema: SchemaRef) -> SchemaRef { diff --git a/crates/vespera_macro/src/parser/schema/struct_schema/snapshots/vespera_macro__parser__schema__struct_schema__tests_serde_fidelity__struct_views_flatten_two_maps.snap b/crates/vespera_macro/src/parser/schema/struct_schema/snapshots/vespera_macro__parser__schema__struct_schema__tests_serde_fidelity__struct_views_flatten_two_maps.snap index 0fc074d8..6f4e4713 100644 --- a/crates/vespera_macro/src/parser/schema/struct_schema/snapshots/vespera_macro__parser__schema__struct_schema__tests_serde_fidelity__struct_views_flatten_two_maps.snap +++ b/crates/vespera_macro/src/parser/schema/struct_schema/snapshots/vespera_macro__parser__schema__struct_schema__tests_serde_fidelity__struct_views_flatten_two_maps.snap @@ -1,13 +1,13 @@ --- source: crates/vespera_macro/src/parser/schema/struct_schema/tests_serde_fidelity.rs -assertion_line: 209 +assertion_line: 235 expression: both_views(src) --- struct Req { #[serde(flatten)] a: HashMap, #[serde(flatten)] b: HashMap } response: { "type": "object", "additionalProperties": { - "allOf": [ + "anyOf": [ { "type": "integer", "format": "uint8", diff --git a/crates/vespera_macro/src/parser/schema/struct_schema/snapshots/vespera_macro__parser__schema__struct_schema__tests_serde_fidelity__struct_views_flatten_two_maps_before_optional_struct.snap b/crates/vespera_macro/src/parser/schema/struct_schema/snapshots/vespera_macro__parser__schema__struct_schema__tests_serde_fidelity__struct_views_flatten_two_maps_before_optional_struct.snap index fb715e3e..a6651d44 100644 --- a/crates/vespera_macro/src/parser/schema/struct_schema/snapshots/vespera_macro__parser__schema__struct_schema__tests_serde_fidelity__struct_views_flatten_two_maps_before_optional_struct.snap +++ b/crates/vespera_macro/src/parser/schema/struct_schema/snapshots/vespera_macro__parser__schema__struct_schema__tests_serde_fidelity__struct_views_flatten_two_maps_before_optional_struct.snap @@ -20,7 +20,7 @@ response: { } }, "additionalProperties": { - "allOf": [ + "anyOf": [ { "type": "integer", "format": "uint8", diff --git a/crates/vespera_macro/src/parser/schema/struct_schema/tests.rs b/crates/vespera_macro/src/parser/schema/struct_schema/tests.rs index fbdd70a3..277de16f 100644 --- a/crates/vespera_macro/src/parser/schema/struct_schema/tests.rs +++ b/crates/vespera_macro/src/parser/schema/struct_schema/tests.rs @@ -65,8 +65,8 @@ fn test_parse_struct_to_schema_rename_all_and_field_rename() { } /// Tuple structs follow serde's wire shape: newtype → inner schema, -/// multi-field → fixed-length `prefixItems` array; unit structs stay an -/// empty object. +/// multi-field → fixed-length `prefixItems` array; unit structs are `null` +/// (serde writes a unit struct as `null`). #[rstest] #[case::newtype_int("newtype_int", "struct Wrapper(i32);")] #[case::newtype_doc("newtype_doc", "/// An id\nstruct UserId(u64);")] @@ -317,13 +317,14 @@ fn test_parse_struct_to_schema_with_description() { Some("User struct description".to_string()) ); // Check field descriptions - let props = schema.properties.unwrap(); - if let SchemaRef::Inline(id_schema) = props.get("id").unwrap() { - assert_eq!(id_schema.description, Some("User ID".to_string())); - } - if let SchemaRef::Inline(name_schema) = props.get("name").unwrap() { - assert_eq!(name_schema.description, Some("User name".to_string())); - } + assert_eq!( + field_schema(&schema, "id").description.as_deref(), + Some("User ID") + ); + assert_eq!( + field_schema(&schema, "name").description.as_deref(), + Some("User name") + ); } #[test] @@ -340,15 +341,16 @@ fn test_parse_struct_to_schema_field_with_ref_and_description() { let mut known = HashSet::new(); known.insert("User".to_string()); let schema = parse_struct_to_schema(&struct_item, &known, &struct_defs); - let props = schema.properties.unwrap(); // Field with $ref and description should use allOf - if let SchemaRef::Inline(user_schema) = props.get("user").unwrap() { - assert_eq!( - user_schema.description, - Some("The user reference".to_string()) - ); - assert!(user_schema.all_of.is_some()); - } + let user_schema = field_schema(&schema, "user"); + assert_eq!( + user_schema.description.as_deref(), + Some("The user reference") + ); + assert_eq!( + serde_json::to_value(&user_schema.all_of).unwrap(), + serde_json::json!([{"$ref": "#/components/schemas/User"}]) + ); } #[test] @@ -371,10 +373,10 @@ fn test_parse_struct_to_schema_description_strips_slash_prefix() { &HashMap::::new(), ); assert_eq!(schema.description, Some("Struct description".to_string())); - let props = schema.properties.unwrap(); - if let SchemaRef::Inline(id_schema) = props.get("id").unwrap() { - assert_eq!(id_schema.description, Some("Field description".to_string())); - } + assert_eq!( + field_schema(&schema, "id").description.as_deref(), + Some("Field description") + ); } #[test] @@ -532,8 +534,10 @@ fn test_parse_struct_to_schema_transparent_tuple_wrapper_uses_ref_schema() { } } +/// `transparent` with two emitted fields (which serde rejects) falls back to +/// the plain tuple shape, a fixed-length array. #[test] -fn test_parse_struct_to_schema_multi_field_tuple_is_fixed_array() { +fn transparent_with_two_fields_falls_back_to_tuple_array() { let struct_item: syn::ItemStruct = syn::parse_str( r" #[serde(transparent)] @@ -739,10 +743,18 @@ fn schema_constraints_coexist_with_doc_comment_on_ref_field() { .unwrap(); let schema = parse_struct_to_schema(&s, &known, &HashMap::::new()); let field = field_schema(&schema, "shipping"); - assert!(field.description.is_some(), "doc comment lost"); + assert_eq!( + field.description.as_deref(), + Some("Shipping address — must be present."), + "doc comment lost" + ); assert_eq!(field.read_only, Some(true)); assert_eq!(field.write_only, Some(false)); - assert!(field.all_of.is_some(), "allOf wrap lost"); + assert_eq!( + serde_json::to_value(&field.all_of).unwrap(), + serde_json::json!([{"$ref": "#/components/schemas/Address"}]), + "allOf wrap lost" + ); } #[test] @@ -763,8 +775,11 @@ fn schema_constraints_unknown_keys_on_field_are_silently_ignored() { &HashSet::::new(), &HashMap::::new(), ); - let field = field_schema(&schema, "pin"); - assert_eq!(field.min_length, Some(4)); + // Exactly the string schema plus `minLength`: `name` added nothing. + assert_eq!( + serde_json::to_value(field_schema(&schema, "pin")).unwrap(), + serde_json::json!({"type": "string", "minLength": 4}) + ); } #[test] diff --git a/crates/vespera_macro/src/parser/schema/struct_schema/tests_wire_names.rs b/crates/vespera_macro/src/parser/schema/struct_schema/tests_wire_names.rs index 12b5b332..dbb215e8 100644 --- a/crates/vespera_macro/src/parser/schema/struct_schema/tests_wire_names.rs +++ b/crates/vespera_macro/src/parser/schema/struct_schema/tests_wire_names.rs @@ -62,7 +62,9 @@ fn struct_tag(#[case] name: &str, #[case] src: &str) { /// Multipart form field names: the request view uses the `Multipart` /// derive's order (`form_data(field_name)` > deserialize-side serde rename > -/// deserialize-side `rename_all`); the response view keeps serde's names. +/// deserialize-side `rename_all`); the response view keeps serde's +/// serialize-side rename, falling back to `form_data(field_name)` before +/// `rename_all` (`doc`, not `DOC_FILE`). #[rstest] #[case::form_data_over_serde_rename( "form_data_over_serde_rename", diff --git a/crates/vespera_macro/src/parser/schema/type_schema/conversion.rs b/crates/vespera_macro/src/parser/schema/type_schema/conversion.rs index fb10a8c3..b3e75b48 100644 --- a/crates/vespera_macro/src/parser/schema/type_schema/conversion.rs +++ b/crates/vespera_macro/src/parser/schema/type_schema/conversion.rs @@ -662,7 +662,9 @@ fn parse_type_impl( &scoped, ); // A request-view instantiation differs from the - // response one when it names an input type. + // response one when it names an input type: `.Input` + // (no identifier holds a `.`, so `Tree` never + // shares a name with `Tree`). if request_view::in_request_view() && (request_view::has_input(&base) || request_view::mentions_input(concrete_types.iter().copied()) @@ -670,7 +672,7 @@ fn parse_type_impl( .iter() .any(|ty| request_view::type_view_differs(ty))) { - name.push_str("Input"); + name.push_str(".Input"); } return super::generic_instances::inline_or_reference(&name, || { match &parsed { @@ -951,3 +953,5 @@ fn non_zero_unsigned(format: Option<&str>, maximum: Option) -> SchemaRef { #[cfg(test)] mod tests; +#[cfg(test)] +mod tests_instances; diff --git a/crates/vespera_macro/src/parser/schema/type_schema/conversion/schema_def.rs b/crates/vespera_macro/src/parser/schema/type_schema/conversion/schema_def.rs index 84437f5e..c4efc816 100644 --- a/crates/vespera_macro/src/parser/schema/type_schema/conversion/schema_def.rs +++ b/crates/vespera_macro/src/parser/schema/type_schema/conversion/schema_def.rs @@ -110,10 +110,7 @@ fn substitute_tokens(tokens: TokenStream, params: &[String], concrete: &[&Type]) } out.extend(trees[index..=index].iter().cloned()); index += 1; - let end = trees[index..] - .iter() - .position(|tree| matches!(tree, TokenTree::Punct(p) if p.as_char() == ',')) - .map_or(trees.len(), |offset| index + offset); + let end = value_end(&trees, index); let value: TokenStream = trees[index..end].iter().cloned().collect(); out.extend(substitute_value(&key, value, params, concrete)); index = end; @@ -121,6 +118,29 @@ fn substitute_tokens(tokens: TokenStream, params: &[String], concrete: &[&Type]) out } +/// The index of the `,` ending the value that starts at `start` (or +/// `trees.len()`): the first comma outside every `<..>` pair, since `<` / +/// `>` are plain puncts (`HashMap` is one value). The `>` of a +/// `->` closes nothing. +fn value_end(trees: &[TokenTree], start: usize) -> usize { + let mut depth = 0usize; + let mut after_dash = false; + for (offset, tree) in trees[start..].iter().enumerate() { + let TokenTree::Punct(punct) = tree else { + after_dash = false; + continue; + }; + match punct.as_char() { + ',' if depth == 0 => return start + offset, + '<' => depth += 1, + '>' if !after_dash => depth = depth.saturating_sub(1), + _ => {} + } + after_dash = punct.as_char() == '-'; + } + trees.len() +} + /// The substituted `value` of `key` (unchanged for other keys or a value /// that is not a type / type string). fn substitute_value( @@ -167,6 +187,20 @@ mod tests { "#[serde(from = \"T\")] enum E { #[serde(into = \"T\")] A(#[schema(value_type = T)] T), B { b: T } }", "# [serde (from = \"String\")] enum E < T > { # [serde (into = \"String\")] A (# [schema (value_type = String)] String) , B { b : String } }" )] + // A comma inside `<..>` belongs to the value (`HashMap`); a + // `->`'s `>` closes no bracket. + #[case::value_type_map( + "struct W { #[schema(value_type = HashMap, format = \"x\")] a: T }", + "struct W < T > { # [schema (value_type = HashMap < String , String > , format = \"x\")] a : String }" + )] + #[case::value_type_result( + "struct W { #[schema(value_type = Result>)] a: T }", + "struct W < T > { # [schema (value_type = Result < String , Vec < String > >)] a : String }" + )] + #[case::value_type_fn_arrow( + "struct W { #[schema(value_type = Pair u8, T>, nullable)] a: T }", + "struct W < T > { # [schema (value_type = Pair < fn () -> u8 , String > , nullable)] a : String }" + )] #[case::unparsable_values( "struct W { #[schema(value_type = +, into = 1, flag)] #[serde(from = \"<\")] a: T }", "struct W < T > { # [schema (value_type = +, into = 1 , flag)] # [serde (from = \"<\")] a : String }" diff --git a/crates/vespera_macro/src/parser/schema/type_schema/conversion/tests.rs b/crates/vespera_macro/src/parser/schema/type_schema/conversion/tests.rs index 1d2efc54..172b4f7b 100644 --- a/crates/vespera_macro/src/parser/schema/type_schema/conversion/tests.rs +++ b/crates/vespera_macro/src/parser/schema/type_schema/conversion/tests.rs @@ -98,82 +98,90 @@ fn generic_value_type_is_substituted() { assert_eq!(schema["properties"]["items"]["items"]["format"], "uint8"); } -/// Recursive instantiations over two same-ident types renamed apart -/// (`a::Foo` = `AFoo`, `b::Foo` = `BFoo`) get distinct components. -#[test] -fn recursive_instances_over_scoped_arguments_do_not_collide() { - super::super::generic_instances::take_materialized(); - let known: HashSet = ["Tree", "Foo", "a::Foo", "b::Foo"] - .into_iter() - .map(String::from) - .collect(); - let defs = HashMap::from([ - ( - "Tree".to_string(), - "struct Tree { value: T, children: Vec> }".to_string(), - ), - ( - "a::Foo".to_string(), - r#"#[schema(name = "AFoo")] struct Foo { a: u8 }"#.to_string(), - ), - ( - "b::Foo".to_string(), - r#"#[schema(name = "BFoo")] struct Foo { b: u8 }"#.to_string(), - ), - ]); - let reference = |src: &str| { - let ty: Type = syn::parse_str(src).unwrap(); - match parse_type_to_schema_ref(&ty, &known, &defs) { - SchemaRef::Ref(reference) => reference.ref_path, - SchemaRef::Inline(schema) => panic!("expected a $ref: {schema:?}"), - } - }; - assert_eq!(reference("Tree"), "#/components/schemas/Tree_AFoo"); - assert_eq!(reference("Tree"), "#/components/schemas/Tree_BFoo"); - // An unscoped argument keeps its ident. - assert_eq!(reference("Tree"), "#/components/schemas/Tree_u8"); - let materialized = super::super::generic_instances::take_materialized(); - assert_eq!( - materialized.keys().collect::>(), - ["Tree_AFoo", "Tree_BFoo", "Tree_u8"] - ); -} - /// Every ident [`is_builtin_openapi_type`] lists has its own arm in -/// `parse_type_impl` (a listed ident without one skips the `T: Schema` -/// assertion yet maps to the unknown-type `{}`). +/// `parse_type_impl`'s leaf `primitive_result` match, or — for the few +/// generic-only builtins — in its generic match (a listed ident without one +/// skips the `T: Schema` assertion yet maps to the unknown-type `{}`). #[test] fn is_builtin_openapi_type_matches_conversion_arms() { - fn literals(tokens: proc_macro2::TokenStream, out: &mut Vec) { + use proc_macro2::{Delimiter, TokenStream, TokenTree}; + + fn literals(tokens: TokenStream, out: &mut Vec) { for tree in tokens { match tree { - proc_macro2::TokenTree::Group(group) => literals(group.stream(), out), - proc_macro2::TokenTree::Literal(lit) => out.push(lit.to_string()), + TokenTree::Group(group) => literals(group.stream(), out), + TokenTree::Literal(lit) => out.push(lit.to_string()), _ => {} } } } - let file = syn::parse_file(include_str!("../conversion.rs")).unwrap(); - let body = |name: &str| { - let mut out = Vec::new(); - for item in &file.items { - if let syn::Item::Fn(func) = item - && func.sig.ident == name - { - literals(quote::ToTokens::to_token_stream(&func.block), &mut out); - } + /// The brace group (match arms) of the first `match` after the ident + /// `binding`, searched through every nested group. + fn match_arms_after(tokens: TokenStream, binding: &str) -> Option { + let trees: Vec = tokens.into_iter().collect(); + if let Some(start) = trees + .iter() + .position(|tree| matches!(tree, TokenTree::Ident(ident) if ident == binding)) + { + let after_match = trees[start..] + .iter() + .position(|tree| matches!(tree, TokenTree::Ident(ident) if ident == "match"))?; + return trees[start + after_match..] + .iter() + .find_map(|tree| match tree { + TokenTree::Group(group) if group.delimiter() == Delimiter::Brace => { + Some(group.stream()) + } + _ => None, + }); } - out + trees.into_iter().find_map(|tree| match tree { + TokenTree::Group(group) => match_arms_after(group.stream(), binding), + _ => None, + }) + } + let file = syn::parse_file(include_str!("../conversion.rs")).unwrap(); + let block = |name: &str| { + file.items + .iter() + .find_map(|item| match item { + syn::Item::Fn(func) if func.sig.ident == name => { + Some(quote::ToTokens::to_token_stream(&func.block)) + } + _ => None, + }) + .unwrap() }; - let builtins: Vec = body("is_builtin_openapi_type") + let mut builtins = Vec::new(); + literals(block("is_builtin_openapi_type"), &mut builtins); + let builtins: Vec = builtins .into_iter() .filter_map(|lit| lit.strip_prefix('b').map(str::to_string)) .collect(); - let arms = body("parse_type_impl"); + let mut leaf_arms = Vec::new(); + literals( + match_arms_after(block("parse_type_impl"), "primitive_result").unwrap(), + &mut leaf_arms, + ); + let mut all_arms = Vec::new(); + literals(block("parse_type_impl"), &mut all_arms); + // Builtins only meaningful with type arguments: their arm is in the + // generic (`AngleBracketed`) match, not the leaf one. + let generic_only = ["HashMap", "BTreeMap", "HasOne", "HasMany", "BelongsTo"]; assert!(builtins.len() > 50, "{builtins:?}"); + assert!(leaf_arms.len() > 50, "{leaf_arms:?}"); for builtin in &builtins { - assert!(arms.contains(builtin), "no conversion arm for {builtin}"); - assert!(is_builtin_openapi_type(builtin.trim_matches('"'))); + let ident = builtin.trim_matches('"'); + if generic_only.contains(&ident) { + assert!(!leaf_arms.contains(builtin), "{builtin} has a leaf arm"); + assert!(all_arms.contains(builtin), "no generic arm for {builtin}"); + } else { + assert!( + leaf_arms.contains(builtin), + "no leaf conversion arm for {builtin}" + ); + } + assert!(is_builtin_openapi_type(ident)); } } @@ -375,21 +383,6 @@ fn test_vec_does_not_have_unique_items() { } } -#[test] -fn test_bare_hashset_without_generics() { - // HashSet without angle brackets → falls through to bare-name match - let ty: Type = syn::parse_str("HashSet").unwrap(); - let schema_ref = parse_type_to_schema_ref(&ty, &empty_known(), &empty_struct_definitions()); - assert!(matches!(schema_ref, SchemaRef::Inline(_))); -} - -#[test] -fn test_bare_btreeset_without_generics() { - let ty: Type = syn::parse_str("BTreeSet").unwrap(); - let schema_ref = parse_type_to_schema_ref(&ty, &empty_known(), &empty_struct_definitions()); - assert!(matches!(schema_ref, SchemaRef::Inline(_))); -} - #[test] fn test_known_schema_ref_override_returns_inline_ref_schema() { let mut known = HashSet::new(); @@ -451,6 +444,16 @@ fn generic_definition_ignores_lifetime_parameters() { panic!("matching type arguments should inline"); }; assert_eq!(schema.schema_type, Some(SchemaType::Object)); + // `T` (not the lifetime) is bound to `String`. + let schema = serde_json::to_value(&schema).unwrap(); + assert_eq!( + schema["properties"]["value"], + serde_json::json!({"type": "string"}) + ); + assert_eq!( + schema["properties"]["marker"], + serde_json::json!({"type": "string"}) + ); } #[test] @@ -599,6 +602,8 @@ fn invisible_type_group_maps_like_its_contents() { #[case::generic_param("T")] #[case::qualified_unknown("other::Thing")] #[case::bare_wrapper("Vec")] +#[case::bare_hash_set("HashSet")] +#[case::bare_btree_set("BTreeSet")] #[case::option_unknown("Option")] fn shapeless_types_are_any_value(#[case] ty_src: &str) { let ty: Type = syn::parse_str(ty_src).unwrap(); @@ -630,12 +635,24 @@ fn shapeless_types_are_any_value(#[case] ty_src: &str) { #[case::option_url("option_url", "Option")] #[case::vec_ip("vec_ip", "Vec")] fn string_serialized_std_types(#[case] name: &str, #[case] ty_src: &str) { + /// The innermost type's last ident (`Option` → `Url`). + fn innermost(ty: &Type) -> String { + let Type::Path(type_path) = ty else { + unreachable!("every case is a path type") + }; + let segment = type_path.path.segments.last().unwrap(); + match &segment.arguments { + syn::PathArguments::AngleBracketed(args) => match args.args.first() { + Some(syn::GenericArgument::Type(inner)) => innermost(inner), + _ => unreachable!("every wrapper case has a type argument"), + }, + _ => segment.ident.to_string(), + } + } let ty: Type = syn::parse_str(ty_src).unwrap(); - let last = match &ty { - Type::Path(type_path) => type_path.path.segments.last().unwrap().ident.to_string(), - _ => unreachable!("every case is a path type"), - }; - assert!(is_builtin_openapi_type(&last)); + let leaf = innermost(&ty); + assert!(!["Option", "Vec"].contains(&leaf.as_str()), "{leaf}"); + assert!(is_builtin_openapi_type(&leaf), "{leaf}"); let schema_ref = parse_type_to_schema_ref(&ty, &empty_known(), &empty_struct_definitions()); let json = serde_json::to_string(&schema_ref).unwrap(); insta::assert_snapshot!( @@ -814,6 +831,17 @@ fn option_unit_is_single_null_type() { )] #[case::non_zero_u64("NonZeroU64", SchemaType::Integer, Some("uint64"), Some(1.0), None)] #[case::non_zero_u128("NonZeroU128", SchemaType::Integer, None, Some(1.0), None)] +#[case::i128("i128", SchemaType::Integer, None, None, None)] +#[case::isize("isize", SchemaType::Integer, None, None, None)] +#[case::status_code("StatusCode", SchemaType::Integer, None, None, None)] +#[case::non_zero_i8("NonZeroI8", SchemaType::Integer, Some("int8"), None, None)] +#[case::non_zero_i32("NonZeroI32", SchemaType::Integer, Some("int32"), None, None)] +#[case::non_zero_i64("NonZeroI64", SchemaType::Integer, Some("int64"), None, None)] +#[case::non_zero_i128("NonZeroI128", SchemaType::Integer, None, None, None)] +#[case::non_zero_isize("NonZeroIsize", SchemaType::Integer, None, None, None)] +#[case::bool("bool", SchemaType::Boolean, None, None, None)] +#[case::string("String", SchemaType::String, None, None, None)] +#[case::str("str", SchemaType::String, None, None, None)] fn primitive_wire_types( #[case] source: &str, #[case] schema_type: SchemaType, @@ -939,33 +967,3 @@ fn generic_missing_argument_without_default_is_a_reference() { SchemaRef::Ref(_) )); } - -/// A recursive generic instance whose argument reads differently than it -/// writes (a set: `uniqueItems` on output only) is a separate `...Input` -/// component in the request view. -#[test] -fn recursive_generic_instance_over_a_set_has_an_input_name() { - use super::super::generic_instances::take_materialized; - let known: HashSet = HashSet::from(["Tree".to_string()]); - let defs = HashMap::from([( - "Tree".to_string(), - "struct Tree { v: T, kids: Vec> }".to_string(), - )]); - let ty: Type = syn::parse_str("Tree>").unwrap(); - let response = parse_type_to_schema_ref(&ty, &known, &defs); - let response_names: Vec = take_materialized().into_keys().collect(); - let request = crate::parser::schema::request_view::with_request_view(|| { - parse_type_to_schema_ref(&ty, &known, &defs) - }); - let request_names: Vec = take_materialized().into_keys().collect(); - assert_eq!(response_names, ["Tree_HashSet_u8"]); - assert_eq!(request_names, ["Tree_HashSet_u8Input"]); - let (SchemaRef::Ref(response), SchemaRef::Ref(request)) = (response, request) else { - panic!("a recursive instance is a $ref"); - }; - assert_eq!(response.ref_path, "#/components/schemas/Tree_HashSet_u8"); - assert_eq!( - request.ref_path, - "#/components/schemas/Tree_HashSet_u8Input" - ); -} diff --git a/crates/vespera_macro/src/parser/schema/type_schema/conversion/tests_instances.rs b/crates/vespera_macro/src/parser/schema/type_schema/conversion/tests_instances.rs new file mode 100644 index 00000000..5ac630d6 --- /dev/null +++ b/crates/vespera_macro/src/parser/schema/type_schema/conversion/tests_instances.rs @@ -0,0 +1,117 @@ +//! Component names of recursive generic instantiations (sidecar module; +//! `use super::*` semantics preserved). + +use super::*; +use crate::parser::schema::{ + generic_instances::take_materialized, request_view::with_request_view, +}; + +fn reference(schema_ref: SchemaRef) -> String { + match schema_ref { + SchemaRef::Ref(reference) => reference.ref_path, + SchemaRef::Inline(schema) => panic!("expected a $ref: {schema:?}"), + } +} + +/// Recursive instantiations over two same-ident types renamed apart +/// (`a::Foo` = `AFoo`, `b::Foo` = `BFoo`) get distinct components. +#[test] +fn recursive_instances_over_scoped_arguments_do_not_collide() { + take_materialized(); + let known: HashSet = ["Tree", "Foo", "a::Foo", "b::Foo"] + .into_iter() + .map(String::from) + .collect(); + let defs = HashMap::from([ + ( + "Tree".to_string(), + "struct Tree { value: T, children: Vec> }".to_string(), + ), + ( + "a::Foo".to_string(), + r#"#[schema(name = "AFoo")] struct Foo { a: u8 }"#.to_string(), + ), + ( + "b::Foo".to_string(), + r#"#[schema(name = "BFoo")] struct Foo { b: u8 }"#.to_string(), + ), + ]); + let reference_of = |src: &str| { + let ty: Type = syn::parse_str(src).unwrap(); + reference(parse_type_to_schema_ref(&ty, &known, &defs)) + }; + assert_eq!( + reference_of("Tree"), + "#/components/schemas/Tree_AFoo" + ); + assert_eq!( + reference_of("Tree"), + "#/components/schemas/Tree_BFoo" + ); + // An unscoped argument keeps its ident. + assert_eq!(reference_of("Tree"), "#/components/schemas/Tree_u8"); + let materialized = take_materialized(); + assert_eq!( + materialized.keys().collect::>(), + ["Tree_AFoo", "Tree_BFoo", "Tree_u8"] + ); +} + +/// A recursive generic instance whose argument reads differently than it +/// writes (a set: `uniqueItems` on output only) is a separate `.Input` +/// component in the request view. +#[test] +fn recursive_generic_instance_over_a_set_has_an_input_name() { + take_materialized(); + let known: HashSet = HashSet::from(["Tree".to_string()]); + let defs = HashMap::from([( + "Tree".to_string(), + "struct Tree { v: T, kids: Vec> }".to_string(), + )]); + let ty: Type = syn::parse_str("Tree>").unwrap(); + let response = parse_type_to_schema_ref(&ty, &known, &defs); + let response_names: Vec = take_materialized().into_keys().collect(); + let request = with_request_view(|| parse_type_to_schema_ref(&ty, &known, &defs)); + let request_names: Vec = take_materialized().into_keys().collect(); + assert_eq!(response_names, ["Tree_HashSet_u8"]); + assert_eq!(request_names, ["Tree_HashSet_u8.Input"]); + assert_eq!(reference(response), "#/components/schemas/Tree_HashSet_u8"); + assert_eq!( + reference(request), + "#/components/schemas/Tree_HashSet_u8.Input" + ); +} + +/// The request view's `Tree>` and the response view's +/// `Tree>` (a real type named `ItemInput`) are different +/// schemas, so they never share a component name. +#[test] +fn request_instance_name_never_collides_with_an_input_named_argument() { + take_materialized(); + let known: HashSet = ["Tree", "Item", "ItemInput"] + .into_iter() + .map(String::from) + .collect(); + let defs = HashMap::from([ + ( + "Tree".to_string(), + "struct Tree { v: T, kids: Vec> }".to_string(), + ), + ("Item".to_string(), "struct Item { a: u8 }".to_string()), + ( + "ItemInput".to_string(), + "struct ItemInput { b: String }".to_string(), + ), + ]); + let request_ty: Type = syn::parse_str("Tree>").unwrap(); + let response_ty: Type = syn::parse_str("Tree>").unwrap(); + let request = reference(with_request_view(|| { + parse_type_to_schema_ref(&request_ty, &known, &defs) + })); + let response = reference(parse_type_to_schema_ref(&response_ty, &known, &defs)); + assert_ne!(request, response); + assert_eq!( + take_materialized().keys().collect::>(), + ["Tree_HashSet_Item.Input", "Tree_HashSet_ItemInput"] + ); +} diff --git a/crates/vespera_macro/src/parser/schema/type_schema/generic_instances.rs b/crates/vespera_macro/src/parser/schema/type_schema/generic_instances.rs index b5b74bda..97a1c6f0 100644 --- a/crates/vespera_macro/src/parser/schema/type_schema/generic_instances.rs +++ b/crates/vespera_macro/src/parser/schema/type_schema/generic_instances.rs @@ -130,7 +130,7 @@ fn push_array_len(name: &mut String, len: &syn::Expr) { // Inline the generic instantiation `name`, built by `build`, unless that /// would recurse forever — then a `$ref` to the materialised component. pub fn inline_or_reference(name: &str, build: impl FnOnce() -> Schema) -> SchemaRef { - // In the request view the instantiation carries an `Input` suffix when + // In the request view the instantiation carries a `.Input` suffix when // the definition has an input view; it then refers to that input. let request = super::super::request_view::in_request_view(); let definition = DEFINITIONS.with(|stack| { @@ -140,7 +140,7 @@ pub fn inline_or_reference(name: &str, build: impl FnOnce() -> Schema) -> Schema .rev() .find(|(instance, _)| { instance == name - || (request && name.strip_suffix("Input") == Some(instance.as_str())) + || (request && name.strip_suffix(".Input") == Some(instance.as_str())) }) .map(|(_, component)| component.clone()) });