From d2e114f80d37295b40a0d2bec07e047776735ac8 Mon Sep 17 00:00:00 2001 From: realbigsean Date: Tue, 30 Jan 2024 16:39:58 -0500 Subject: [PATCH 1/8] add flatten --- src/lib.rs | 64 +++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 54 insertions(+), 10 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index d6b675a7..a736984b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,8 +12,8 @@ use quote::{format_ident, quote, ToTokens}; use std::collections::HashMap; use std::iter::{self, FromIterator}; use syn::{ - parse_macro_input, Attribute, AttributeArgs, Expr, Field, GenericParam, Ident, ItemStruct, - Lifetime, LifetimeDef, Type, TypeGenerics, TypeParamBound, + parse2, parse_macro_input, Attribute, AttributeArgs, Expr, Field, GenericParam, Ident, + ItemEnum, ItemStruct, Lifetime, LifetimeDef, Type, TypeGenerics, TypeParamBound, }; mod attributes; @@ -65,6 +65,8 @@ struct StructOpts { /// Field-level configuration. #[derive(Debug, Default, FromMeta)] struct FieldOpts { + #[darling(default)] + flatten: bool, #[darling(default)] only: Option>, #[darling(default)] @@ -193,20 +195,62 @@ pub fn superstruct(args: TokenStream, input: TokenStream) -> TokenStream { panic!("can't configure `only` and `getter` on the same field"); } else if field_opts.only.is_none() && field_opts.partial_getter.is_some() { panic!("can't set `partial_getter` options on common field"); + } else if field_opts.flatten && field_opts.only.is_some() { + panic!("can't set `flatten` and `only` on the same field"); + } else if field_opts.flatten && field_opts.getter.is_some() { + panic!("can't set `flatten` and `getter` on the same field"); + } else if field_opts.flatten && field_opts.partial_getter.is_some() { + panic!("can't set `flatten` and `partial_getter` on the same field"); } let only = field_opts.only.map(|only| only.keys().cloned().collect()); let getter_opts = field_opts.getter.unwrap_or_default(); let partial_getter_opts = field_opts.partial_getter.unwrap_or_default(); - // Add to list of all fields - fields.push(FieldData { - name, - field: output_field, - only, - getter_opts, - partial_getter_opts, - }); + if field_opts.flatten { + // Parse the inner type, making sure it's an enum. + let ty = &output_field.ty; + + let inner_enum: ItemEnum = parse2(output_field.ty.to_token_stream()) + .expect(format!("inner type must be an enum {ty:?}").as_str()); + + // Extract the names of the variants for the inner enum. + let variant_names_inner: Vec<_> = inner_enum + .variants + .iter() + .map(|v| v.ident.clone()) + .collect(); + + // Compare the sets of variant names. + assert_eq!( + variant_names, &variant_names_inner, + "variant names must match" + ); + + for variant in inner_enum.variants { + assert!( + variant.fields.len() == 1, + "only one field allowed in flattened enum variants" + ); + let inner_field = variant.fields.into_iter().next().unwrap(); + // Add the variant name as a suffix to the getter name. + fields.push(FieldData { + name: format_ident!("{}_{}", name, variant.ident.to_string().to_lowercase()), + field: inner_field, + only: None, + getter_opts: <_>::default(), + partial_getter_opts: <_>::default(), + }); + } + } else { + fields.push(FieldData { + name, + field: output_field, + only, + getter_opts, + partial_getter_opts, + }); + } } // Generate structs for all of the variants. From 0f69f8e2b4638487a2ffb09e2b736b1b83b8f333 Mon Sep 17 00:00:00 2001 From: realbigsean Date: Tue, 30 Jan 2024 19:57:16 -0500 Subject: [PATCH 2/8] add tests,docs,fix the feature --- book/src/config/field.md | 42 +++++++++++++++++++++++++ src/lib.rs | 68 ++++++++++++++++++++++------------------ tests/flatten.rs | 62 ++++++++++++++++++++++++++++++++++++ 3 files changed, 141 insertions(+), 31 deletions(-) create mode 100644 tests/flatten.rs diff --git a/book/src/config/field.md b/book/src/config/field.md index 142cbc5f..8e499981 100644 --- a/book/src/config/field.md +++ b/book/src/config/field.md @@ -68,3 +68,45 @@ may be applied in a single attribute, e.g. `#[superstruct(partial_getter(copy, n The error type for partial getters can currently only be configured on a per-struct basis via the [`partial_getter_error`](./struct.md#partial-getter-error) attribute, although this may change in a future release. + +## Flatten + +``` +#[superstruct(flatten)] +``` + +This attribute can only be applied to enum fields that whose variants match each variant of the +superstruct. This is useful for nesting superstructs whose variant types should be linked. + +This will automatically create a partial getter for each variant. The following two examples are equivalent. + +Using `flatten`: +``` +#[superstruct(variants(A, B))] +struct InnerMessage { + pub x: u64, + pub y: u64, +} + +#[superstruct(variants(A, B))] +struct Message { + #[superstruct(flatten)] + pub inner: InnerMessage, +} +``` +Equivalent without `flatten`: +``` +#[superstruct(variants(A, B))] +struct InnerMessage { + pub x: u64, + pub y: u64, +} + +#[superstruct(variants(A, B))] +struct Message { + #[superstruct(only(A), partial_getter(rename = "inner_a"))] + pub inner: InnerMessageA, + #[superstruct(only(B), partial_getter(rename = "inner_b"))] + pub inner: InnerMessageB, +} +``` diff --git a/src/lib.rs b/src/lib.rs index a736984b..705735ae 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,8 +12,8 @@ use quote::{format_ident, quote, ToTokens}; use std::collections::HashMap; use std::iter::{self, FromIterator}; use syn::{ - parse2, parse_macro_input, Attribute, AttributeArgs, Expr, Field, GenericParam, Ident, - ItemEnum, ItemStruct, Lifetime, LifetimeDef, Type, TypeGenerics, TypeParamBound, + parse_macro_input, Attribute, AttributeArgs, Expr, Field, GenericParam, Ident, ItemStruct, + Lifetime, LifetimeDef, Type, TypeGenerics, TypeParamBound, }; mod attributes; @@ -76,7 +76,7 @@ struct FieldOpts { } /// Getter configuration for a specific field -#[derive(Debug, Default, FromMeta)] +#[derive(Clone, Debug, Default, FromMeta)] struct GetterOpts { #[darling(default)] copy: bool, @@ -208,39 +208,45 @@ pub fn superstruct(args: TokenStream, input: TokenStream) -> TokenStream { let partial_getter_opts = field_opts.partial_getter.unwrap_or_default(); if field_opts.flatten { - // Parse the inner type, making sure it's an enum. - let ty = &output_field.ty; - - let inner_enum: ItemEnum = parse2(output_field.ty.to_token_stream()) - .expect(format!("inner type must be an enum {ty:?}").as_str()); - - // Extract the names of the variants for the inner enum. - let variant_names_inner: Vec<_> = inner_enum - .variants - .iter() - .map(|v| v.ident.clone()) - .collect(); + for variant in variant_names { + // Update the struct name for this variant. + let mut next_variant_field = output_field.clone(); + match &mut next_variant_field.ty { + Type::Path(ref mut p) => { + let first_segment = &mut p + .path + .segments + .first_mut() + .expect("path should have at least one segment"); + let inner_ty_name = first_segment.ident.clone(); + let next_variant_ty_name = format_ident!("{}{}", inner_ty_name, variant); + first_segment.ident = next_variant_ty_name; + } + _ => panic!("field must be a path"), + }; - // Compare the sets of variant names. - assert_eq!( - variant_names, &variant_names_inner, - "variant names must match" - ); + // Create a partial getter for the field. + let partial_getter_rename = + format_ident!("{}_{}", name, variant.to_string().to_lowercase()); + let partial_getter_opts = GetterOpts { + rename: Some(partial_getter_rename), + ..<_>::default() + }; - for variant in inner_enum.variants { - assert!( - variant.fields.len() == 1, - "only one field allowed in flattened enum variants" - ); - let inner_field = variant.fields.into_iter().next().unwrap(); - // Add the variant name as a suffix to the getter name. fields.push(FieldData { - name: format_ident!("{}_{}", name, variant.ident.to_string().to_lowercase()), - field: inner_field, - only: None, + name: name.clone(), + field: next_variant_field.clone(), + // Make sure the field is only accessible from this variant. + only: Some(vec![variant.clone()]), getter_opts: <_>::default(), - partial_getter_opts: <_>::default(), + partial_getter_opts, }); + + // Update the variant field map + let fields = variant_fields + .get_mut(variant) + .expect("invalid variant name"); + *fields.get_mut(0).expect("no fields for variant") = next_variant_field; } } else { fields.push(FieldData { diff --git a/tests/flatten.rs b/tests/flatten.rs new file mode 100644 index 00000000..93eaaf4b --- /dev/null +++ b/tests/flatten.rs @@ -0,0 +1,62 @@ +use superstruct::superstruct; + +#[test] +fn flatten() { + #[superstruct(variants(A, B), variant_attributes(derive(Debug, PartialEq, Eq)))] + #[derive(Debug, PartialEq, Eq)] + struct InnerMessage { + pub x: u64, + #[superstruct(only(B))] + pub y: u64, + } + + #[superstruct(variants(A, B), variant_attributes(derive(Debug, PartialEq, Eq)))] + #[derive(Debug, PartialEq, Eq)] + struct Message { + #[superstruct(flatten)] + pub inner: InnerMessage, + } + + let message_a = Message::A(MessageA { + inner: InnerMessageA { x: 1 }, + }); + let message_b = Message::B(MessageB { + inner: InnerMessageB { x: 3, y: 4 }, + }); + assert_eq!(message_a.inner_a().unwrap().x, 1); + assert!(message_a.inner_b().is_err()); + assert_eq!(message_b.inner_b().unwrap().x, 3); + assert_eq!(message_b.inner_b().unwrap().y, 4); + assert!(message_b.inner_a().is_err()); + + let message_a_ref = MessageRef::A(&MessageA { + inner: InnerMessageA { x: 1 }, + }); + let message_b_ref = MessageRef::B(&MessageB { + inner: InnerMessageB { x: 3, y: 4 }, + }); + assert_eq!(message_a_ref.inner_a().unwrap().x, 1); + assert!(message_a_ref.inner_b().is_err()); + assert_eq!(message_b_ref.inner_b().unwrap().x, 3); + assert_eq!(message_b_ref.inner_b().unwrap().y, 4); + assert!(message_b_ref.inner_a().is_err()); + + let mut inner_a = MessageA { + inner: InnerMessageA { x: 1 }, + }; + let mut inner_b = MessageB { + inner: InnerMessageB { x: 3, y: 4 }, + }; + + // Re-initialize the struct to avoid borrow checker errors. + let mut message_a_ref_mut = MessageRefMut::A(&mut inner_a); + assert_eq!(message_a_ref_mut.inner_a_mut().map(|inner| inner.x), Ok(1)); + let mut message_a_ref_mut = MessageRefMut::A(&mut inner_a); + assert!(message_a_ref_mut.inner_b_mut().is_err()); + let mut message_b_ref_mut = MessageRefMut::B(&mut inner_b); + assert_eq!(message_b_ref_mut.inner_b_mut().unwrap().x, 3); + let mut message_b_ref_mut = MessageRefMut::B(&mut inner_b); + assert_eq!(message_b_ref_mut.inner_b_mut().unwrap().y, 4); + let mut message_b_ref_mut = MessageRefMut::B(&mut inner_b); + assert!(message_b_ref_mut.inner_a_mut().is_err()); +} From 107b063980a60407d82684674d45891ca47da59e Mon Sep 17 00:00:00 2001 From: realbigsean Date: Tue, 30 Jan 2024 19:58:37 -0500 Subject: [PATCH 3/8] remove clone from getter opt --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 705735ae..89b59508 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -76,7 +76,7 @@ struct FieldOpts { } /// Getter configuration for a specific field -#[derive(Clone, Debug, Default, FromMeta)] +#[derive(Debug, Default, FromMeta)] struct GetterOpts { #[darling(default)] copy: bool, From 748e0a5dbb824db48beacb2471aec1c62eaa1311 Mon Sep 17 00:00:00 2001 From: realbigsean Date: Wed, 31 Jan 2024 09:22:22 -0500 Subject: [PATCH 4/8] fix bug in generated struct field placement --- src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 89b59508..19711093 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -161,7 +161,7 @@ pub fn superstruct(args: TokenStream, input: TokenStream) -> TokenStream { let mut variant_fields = HashMap::<_, _>::from_iter(variant_names.iter().zip(iter::repeat(vec![]))); - for field in item.fields.iter() { + for (field_index, field) in item.fields.iter().enumerate() { let name = field.ident.clone().expect("named fields only"); let field_opts = field .attrs @@ -246,7 +246,7 @@ pub fn superstruct(args: TokenStream, input: TokenStream) -> TokenStream { let fields = variant_fields .get_mut(variant) .expect("invalid variant name"); - *fields.get_mut(0).expect("no fields for variant") = next_variant_field; + *fields.get_mut(field_index).expect("invalid field index") = next_variant_field; } } else { fields.push(FieldData { From 76b67a19fdea613b5f7050e43644544f115aa517 Mon Sep 17 00:00:00 2001 From: realbigsean Date: Wed, 31 Jan 2024 12:30:05 -0500 Subject: [PATCH 5/8] flatten subset --- book/src/config/field.md | 16 ++++++++++++++++ src/lib.rs | 27 ++++++++++++++++++++++----- tests/flatten.rs | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 5 deletions(-) diff --git a/book/src/config/field.md b/book/src/config/field.md index 8e499981..9bc0ffc5 100644 --- a/book/src/config/field.md +++ b/book/src/config/field.md @@ -110,3 +110,19 @@ struct Message { pub inner: InnerMessageB, } ``` + +If you wish to only flatten into only a subset of variants, you can define them like so: + +``` +#[superstruct(variants(A, B))] +struct InnerMessage { + pub x: u64, + pub y: u64, +} + +#[superstruct(variants(A, B, C))] +struct Message { + #[superstruct(flatten(A,B))] + pub inner: InnerMessage, +} +``` diff --git a/src/lib.rs b/src/lib.rs index 19711093..028022c6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,5 @@ use attributes::{IdentList, NestedMetaList}; +use darling::util::Override; use darling::FromMeta; use from::{ generate_from_enum_trait_impl_for_ref, generate_from_variant_trait_impl, @@ -66,7 +67,7 @@ struct StructOpts { #[derive(Debug, Default, FromMeta)] struct FieldOpts { #[darling(default)] - flatten: bool, + flatten: Option>>, #[darling(default)] only: Option>, #[darling(default)] @@ -75,6 +76,13 @@ struct FieldOpts { partial_getter: Option, } +fn should_skip(flatten: &Override>, key: &Ident) -> bool { + match flatten { + Override::Inherit => false, + Override::Explicit(map) => !map.is_empty() && !map.contains_key(key), + } +} + /// Getter configuration for a specific field #[derive(Debug, Default, FromMeta)] struct GetterOpts { @@ -195,11 +203,11 @@ pub fn superstruct(args: TokenStream, input: TokenStream) -> TokenStream { panic!("can't configure `only` and `getter` on the same field"); } else if field_opts.only.is_none() && field_opts.partial_getter.is_some() { panic!("can't set `partial_getter` options on common field"); - } else if field_opts.flatten && field_opts.only.is_some() { + } else if field_opts.flatten.is_some() && field_opts.only.is_some() { panic!("can't set `flatten` and `only` on the same field"); - } else if field_opts.flatten && field_opts.getter.is_some() { + } else if field_opts.flatten.is_some() && field_opts.getter.is_some() { panic!("can't set `flatten` and `getter` on the same field"); - } else if field_opts.flatten && field_opts.partial_getter.is_some() { + } else if field_opts.flatten.is_some() && field_opts.partial_getter.is_some() { panic!("can't set `flatten` and `partial_getter` on the same field"); } @@ -207,8 +215,17 @@ pub fn superstruct(args: TokenStream, input: TokenStream) -> TokenStream { let getter_opts = field_opts.getter.unwrap_or_default(); let partial_getter_opts = field_opts.partial_getter.unwrap_or_default(); - if field_opts.flatten { + if let Some(flatten_opts) = field_opts.flatten { for variant in variant_names { + if should_skip(&flatten_opts, variant) { + // Remove the field from the field map + let fields = variant_fields + .get_mut(variant) + .expect("invalid variant name"); + fields.remove(field_index); + continue; + } + // Update the struct name for this variant. let mut next_variant_field = output_field.clone(); match &mut next_variant_field.ty { diff --git a/tests/flatten.rs b/tests/flatten.rs index 93eaaf4b..46f2c613 100644 --- a/tests/flatten.rs +++ b/tests/flatten.rs @@ -60,3 +60,36 @@ fn flatten() { let mut message_b_ref_mut = MessageRefMut::B(&mut inner_b); assert!(message_b_ref_mut.inner_a_mut().is_err()); } + +#[test] +fn flatten_subset() { + #[superstruct(variants(A, B), variant_attributes(derive(Debug, PartialEq, Eq)))] + #[derive(Debug, PartialEq, Eq)] + struct InnerMessageSubset { + pub x: u64, + #[superstruct(only(B))] + pub y: u64, + } + + #[superstruct(variants(A, B, C), variant_attributes(derive(Debug, PartialEq, Eq)))] + #[derive(Debug, PartialEq, Eq)] + struct MessageSubset { + #[superstruct(flatten(A, B))] + pub inner: InnerMessageSubset, + } + + let message_a = MessageSubset::A(MessageSubsetA { + inner: InnerMessageSubsetA { x: 1 }, + }); + let message_b = MessageSubset::B(MessageSubsetB { + inner: InnerMessageSubsetB { x: 3, y: 4 }, + }); + let message_c = MessageSubset::C(MessageSubsetC {}); + assert_eq!(message_a.inner_a().unwrap().x, 1); + assert!(message_a.inner_b().is_err()); + assert_eq!(message_b.inner_b().unwrap().x, 3); + assert_eq!(message_b.inner_b().unwrap().y, 4); + assert!(message_b.inner_a().is_err()); + assert!(message_c.inner_a().is_err()); + assert!(message_c.inner_b().is_err()); +} From 1894a1b1396ba602610fd07ff42618f67a58a1bd Mon Sep 17 00:00:00 2001 From: realbigsean Date: Wed, 31 Jan 2024 12:42:02 -0500 Subject: [PATCH 6/8] add comment --- src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 028022c6..06e01784 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -66,6 +66,9 @@ struct StructOpts { /// Field-level configuration. #[derive(Debug, Default, FromMeta)] struct FieldOpts { + // TODO: When we update darling, we can replace `Override` + // with a custom enum and use `#[darling(word)]` on the variant + // we want to use for `#[superstruct(flatten)]` (no variants specified). #[darling(default)] flatten: Option>>, #[darling(default)] From 52eba9fb1df8bc09628c615dc6117aea01bf1e3a Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 1 Feb 2024 12:46:43 +1100 Subject: [PATCH 7/8] Fix bug in field_index usage --- book/src/config/field.md | 8 ++--- src/lib.rs | 15 +++++++-- tests/flatten.rs | 68 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 7 deletions(-) diff --git a/book/src/config/field.md b/book/src/config/field.md index 9bc0ffc5..1acae8d8 100644 --- a/book/src/config/field.md +++ b/book/src/config/field.md @@ -75,13 +75,13 @@ change in a future release. #[superstruct(flatten)] ``` -This attribute can only be applied to enum fields that whose variants match each variant of the +This attribute can only be applied to enum fields with variants that match each variant of the superstruct. This is useful for nesting superstructs whose variant types should be linked. This will automatically create a partial getter for each variant. The following two examples are equivalent. Using `flatten`: -``` +```rust #[superstruct(variants(A, B))] struct InnerMessage { pub x: u64, @@ -95,7 +95,7 @@ struct Message { } ``` Equivalent without `flatten`: -``` +```rust #[superstruct(variants(A, B))] struct InnerMessage { pub x: u64, @@ -113,7 +113,7 @@ struct Message { If you wish to only flatten into only a subset of variants, you can define them like so: -``` +```rust #[superstruct(variants(A, B))] struct InnerMessage { pub x: u64, diff --git a/src/lib.rs b/src/lib.rs index 06e01784..c7464ae5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -172,7 +172,7 @@ pub fn superstruct(args: TokenStream, input: TokenStream) -> TokenStream { let mut variant_fields = HashMap::<_, _>::from_iter(variant_names.iter().zip(iter::repeat(vec![]))); - for (field_index, field) in item.fields.iter().enumerate() { + for field in &item.fields { let name = field.ident.clone().expect("named fields only"); let field_opts = field .attrs @@ -220,12 +220,19 @@ pub fn superstruct(args: TokenStream, input: TokenStream) -> TokenStream { if let Some(flatten_opts) = field_opts.flatten { for variant in variant_names { + let variant_field_index = variant_fields + .get(variant) + .expect("invalid variant name") + .iter() + .position(|f| f.ident.as_ref() == Some(&name)) + .expect("flattened fields are present on all variants"); + if should_skip(&flatten_opts, variant) { // Remove the field from the field map let fields = variant_fields .get_mut(variant) .expect("invalid variant name"); - fields.remove(field_index); + fields.remove(variant_field_index); continue; } @@ -266,7 +273,9 @@ pub fn superstruct(args: TokenStream, input: TokenStream) -> TokenStream { let fields = variant_fields .get_mut(variant) .expect("invalid variant name"); - *fields.get_mut(field_index).expect("invalid field index") = next_variant_field; + *fields + .get_mut(variant_field_index) + .expect("invalid field index") = next_variant_field; } } else { fields.push(FieldData { diff --git a/tests/flatten.rs b/tests/flatten.rs index 46f2c613..68311ab7 100644 --- a/tests/flatten.rs +++ b/tests/flatten.rs @@ -93,3 +93,71 @@ fn flatten_subset() { assert!(message_c.inner_a().is_err()); assert!(message_c.inner_b().is_err()); } + +#[test] +fn flatten_not_first_field() { + #[superstruct(variants(A, B), variant_attributes(derive(Debug, PartialEq, Eq)))] + #[derive(Debug, PartialEq, Eq)] + struct InnerMessageTwo { + pub x: u64, + #[superstruct(only(B))] + pub y: u64, + } + + #[superstruct(variants(A, B), variant_attributes(derive(Debug, PartialEq, Eq)))] + #[derive(Debug, PartialEq, Eq)] + struct MessageTwo { + #[superstruct(only(A), partial_getter(copy))] + pub other: u64, + #[superstruct(flatten)] + pub inner: InnerMessageTwo, + } + + let message_a = MessageTwo::A(MessageTwoA { + other: 21, + inner: InnerMessageTwoA { x: 1 }, + }); + let message_b = MessageTwo::B(MessageTwoB { + inner: InnerMessageTwoB { x: 3, y: 4 }, + }); + assert_eq!(message_a.other().unwrap(), 21); + assert_eq!(message_a.inner_a().unwrap().x, 1); + assert!(message_a.inner_b().is_err()); + assert_eq!(message_b.inner_b().unwrap().x, 3); + assert_eq!(message_b.inner_b().unwrap().y, 4); + assert!(message_b.inner_a().is_err()); + + let message_a_ref = MessageTwoRef::A(&MessageTwoA { + other: 21, + inner: InnerMessageTwoA { x: 1 }, + }); + let message_b_ref = MessageTwoRef::B(&MessageTwoB { + inner: InnerMessageTwoB { x: 3, y: 4 }, + }); + assert_eq!(message_a.other().unwrap(), 21); + assert_eq!(message_a_ref.inner_a().unwrap().x, 1); + assert!(message_a_ref.inner_b().is_err()); + assert_eq!(message_b_ref.inner_b().unwrap().x, 3); + assert_eq!(message_b_ref.inner_b().unwrap().y, 4); + assert!(message_b_ref.inner_a().is_err()); + + let mut inner_a = MessageTwoA { + other: 21, + inner: InnerMessageTwoA { x: 1 }, + }; + let mut inner_b = MessageTwoB { + inner: InnerMessageTwoB { x: 3, y: 4 }, + }; + + // Re-initialize the struct to avoid borrow checker errors. + let mut message_a_ref_mut = MessageTwoRefMut::A(&mut inner_a); + assert_eq!(message_a_ref_mut.inner_a_mut().map(|inner| inner.x), Ok(1)); + let mut message_a_ref_mut = MessageTwoRefMut::A(&mut inner_a); + assert!(message_a_ref_mut.inner_b_mut().is_err()); + let mut message_b_ref_mut = MessageTwoRefMut::B(&mut inner_b); + assert_eq!(message_b_ref_mut.inner_b_mut().unwrap().x, 3); + let mut message_b_ref_mut = MessageTwoRefMut::B(&mut inner_b); + assert_eq!(message_b_ref_mut.inner_b_mut().unwrap().y, 4); + let mut message_b_ref_mut = MessageTwoRefMut::B(&mut inner_b); + assert!(message_b_ref_mut.inner_a_mut().is_err()); +} From 30f768f5206858f090b89a93ba02e4038ce0307f Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 1 Feb 2024 12:54:08 +1100 Subject: [PATCH 8/8] Use *last* path segment not first --- src/lib.rs | 8 ++++---- tests/flatten.rs | 21 ++++++++++++++------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c7464ae5..fa836505 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -240,14 +240,14 @@ pub fn superstruct(args: TokenStream, input: TokenStream) -> TokenStream { let mut next_variant_field = output_field.clone(); match &mut next_variant_field.ty { Type::Path(ref mut p) => { - let first_segment = &mut p + let last_segment = &mut p .path .segments - .first_mut() + .last_mut() .expect("path should have at least one segment"); - let inner_ty_name = first_segment.ident.clone(); + let inner_ty_name = last_segment.ident.clone(); let next_variant_ty_name = format_ident!("{}{}", inner_ty_name, variant); - first_segment.ident = next_variant_ty_name; + last_segment.ident = next_variant_ty_name; } _ => panic!("field must be a path"), }; diff --git a/tests/flatten.rs b/tests/flatten.rs index 68311ab7..21fafbb8 100644 --- a/tests/flatten.rs +++ b/tests/flatten.rs @@ -96,12 +96,19 @@ fn flatten_subset() { #[test] fn flatten_not_first_field() { - #[superstruct(variants(A, B), variant_attributes(derive(Debug, PartialEq, Eq)))] - #[derive(Debug, PartialEq, Eq)] - struct InnerMessageTwo { - pub x: u64, - #[superstruct(only(B))] - pub y: u64, + use test_mod::*; + + // Put this type in a submodule to test path parsing in `flatten`. + pub mod test_mod { + use superstruct::superstruct; + + #[superstruct(variants(A, B), variant_attributes(derive(Debug, PartialEq, Eq)))] + #[derive(Debug, PartialEq, Eq)] + pub struct InnerMessageTwo { + pub x: u64, + #[superstruct(only(B))] + pub y: u64, + } } #[superstruct(variants(A, B), variant_attributes(derive(Debug, PartialEq, Eq)))] @@ -110,7 +117,7 @@ fn flatten_not_first_field() { #[superstruct(only(A), partial_getter(copy))] pub other: u64, #[superstruct(flatten)] - pub inner: InnerMessageTwo, + pub inner: test_mod::InnerMessageTwo, } let message_a = MessageTwo::A(MessageTwoA {