diff --git a/crates/nu-derive-value/src/attributes.rs b/crates/nu-derive-value/src/attributes.rs index ef353685ab..f13ac51ce6 100644 --- a/crates/nu-derive-value/src/attributes.rs +++ b/crates/nu-derive-value/src/attributes.rs @@ -1,60 +1,92 @@ -use syn::{spanned::Spanned, Attribute, Fields, LitStr}; +use syn::{meta::ParseNestedMeta, spanned::Spanned, Attribute, Fields, LitStr}; use crate::{case::Case, error::DeriveError, HELPER_ATTRIBUTE}; +pub trait ParseAttrs: Default { + fn parse_attrs<'a, M>( + iter: impl IntoIterator, + ) -> Result> { + let mut attrs = Self::default(); + for attr in filter(iter.into_iter()) { + // This is a container to allow returning derive errors inside the parse_nested_meta fn. + let mut err = Ok(()); + let _ = attr.parse_nested_meta(|meta| { + attrs.parse_attr(meta).or_else(|e| { + err = Err(e); + Ok(()) // parse_nested_meta requires another error type, so we escape it here + }) + }); + err?; // Shortcircuit here if `err` is holding some error. + } + + Ok(attrs) + } + + fn parse_attr(&mut self, attr_meta: ParseNestedMeta<'_>) -> Result<(), DeriveError>; +} + #[derive(Debug, Default)] pub struct ContainerAttributes { pub rename_all: Option, pub type_name: Option, } -impl ContainerAttributes { - pub fn parse_attrs<'a, M>( - iter: impl Iterator, - ) -> Result> { - let mut container_attrs = ContainerAttributes::default(); - for attr in filter(iter) { - // This is a container to allow returning derive errors inside the parse_nested_meta fn. - let mut err = Ok(()); - - attr.parse_nested_meta(|meta| { - let ident = meta.path.require_ident()?; - match ident.to_string().as_str() { - "rename_all" => { - let case: LitStr = meta.value()?.parse()?; - let value_span = case.span(); - let case = case.value(); - match Case::from_str(&case) { - Some(case) => container_attrs.rename_all = Some(case), - None => { - err = Err(DeriveError::InvalidAttributeValue { - value_span, - value: Box::new(case), - }); - return Ok(()); // We stored the err in `err`. - } - } - } - "type_name" => { - let type_name: LitStr = meta.value()?.parse()?; - let type_name = type_name.value(); - container_attrs.type_name = Some(type_name); - } - ident => { - err = Err(DeriveError::UnexpectedAttribute { - meta_span: ident.span(), +impl ParseAttrs for ContainerAttributes { + fn parse_attr(&mut self, attr_meta: ParseNestedMeta<'_>) -> Result<(), DeriveError> { + let ident = attr_meta.path.require_ident()?; + match ident.to_string().as_str() { + "rename_all" => { + let case: LitStr = attr_meta.value()?.parse()?; + let value_span = case.span(); + let case = case.value(); + match Case::from_str(&case) { + Some(case) => self.rename_all = Some(case), + None => { + return Err(DeriveError::InvalidAttributeValue { + value_span, + value: Box::new(case), }); } } - - Ok(()) - }) - .map_err(DeriveError::Syn)?; - - err?; // Shortcircuit here if `err` is holding some error. + } + "type_name" => { + let type_name: LitStr = attr_meta.value()?.parse()?; + let type_name = type_name.value(); + self.type_name = Some(type_name); + } + ident => { + return Err(DeriveError::UnexpectedAttribute { + meta_span: ident.span(), + }); + } } - Ok(container_attrs) + Ok(()) + } +} + +#[derive(Debug, Default)] +pub struct MemberAttributes { + pub rename: Option, +} + +impl ParseAttrs for MemberAttributes { + fn parse_attr(&mut self, attr_meta: ParseNestedMeta<'_>) -> Result<(), DeriveError> { + let ident = attr_meta.path.require_ident()?; + match ident.to_string().as_str() { + "rename" => { + let rename: LitStr = attr_meta.value()?.parse()?; + let rename = rename.value(); + self.rename = Some(rename); + } + ident => { + return Err(DeriveError::UnexpectedAttribute { + meta_span: ident.span(), + }); + } + } + + Ok(()) } } diff --git a/crates/nu-derive-value/src/case.rs b/crates/nu-derive-value/src/case.rs index 02ca5ba255..5962a29b91 100644 --- a/crates/nu-derive-value/src/case.rs +++ b/crates/nu-derive-value/src/case.rs @@ -47,12 +47,16 @@ impl Case { } pub trait Casing { - fn to_case(&self, case: Case) -> String; + fn to_case(&self, case: impl Into>) -> String; } -impl> Casing for T { - fn to_case(&self, case: Case) -> String { - let s = self.as_ref(); +impl Casing for T { + fn to_case(&self, case: impl Into>) -> String { + let s = self.to_string(); + let Some(case) = case.into() else { + return s.to_string(); + }; + match case { Case::Pascal => s.to_upper_camel_case(), Case::Camel => s.to_lower_camel_case(), diff --git a/crates/nu-derive-value/src/error.rs b/crates/nu-derive-value/src/error.rs index b7fd3e2f91..cde155044e 100644 --- a/crates/nu-derive-value/src/error.rs +++ b/crates/nu-derive-value/src/error.rs @@ -28,6 +28,19 @@ pub enum DeriveError { value_span: Span, value: Box, }, + + /// Two keys or variants are called the same name breaking bidirectionality. + NonUniqueName { + name: String, + first: Span, + second: Span, + }, +} + +impl From for DeriveError { + fn from(value: syn::parse::Error) -> Self { + Self::Syn(value) + } } impl From> for Diagnostic { @@ -77,6 +90,15 @@ impl From> for Diagnostic { "check documentation for `{derive_name}` for valid attribute values" )) } + + DeriveError::NonUniqueName { + name, + first, + second, + } => Diagnostic::new(Level::Error, format!("non-unique name {name:?} found")) + .span_error(first, "first occurrence found here".to_string()) + .span_error(second, "second occurrence found here".to_string()) + .help("use `#[nu_value(rename = \"...\")]` to ensure unique names".to_string()), } } } diff --git a/crates/nu-derive-value/src/from.rs b/crates/nu-derive-value/src/from.rs index 820fa46bc6..7c929ffca3 100644 --- a/crates/nu-derive-value/src/from.rs +++ b/crates/nu-derive-value/src/from.rs @@ -6,13 +6,15 @@ use syn::{ }; use crate::{ - attributes::{self, ContainerAttributes}, - case::{Case, Casing}, + attributes::{self, ContainerAttributes, MemberAttributes, ParseAttrs}, + case::Case, + names::NameResolver, }; #[derive(Debug)] pub struct FromValue; type DeriveError = super::error::DeriveError; +type Result = std::result::Result; /// Inner implementation of the `#[derive(FromValue)]` macro for structs and enums. /// @@ -23,7 +25,7 @@ type DeriveError = super::error::DeriveError; /// - For structs: [`derive_struct_from_value`] /// - For enums: [`derive_enum_from_value`] /// - Unions are not supported and will return an error. -pub fn derive_from_value(input: TokenStream2) -> Result { +pub fn derive_from_value(input: TokenStream2) -> Result { let input: DeriveInput = syn::parse2(input).map_err(DeriveError::Syn)?; match input.data { Data::Struct(data_struct) => Ok(derive_struct_from_value( @@ -52,13 +54,15 @@ fn derive_struct_from_value( data: DataStruct, generics: Generics, attrs: Vec, -) -> Result { +) -> Result { let container_attrs = ContainerAttributes::parse_attrs(attrs.iter())?; - attributes::deny_fields(&data.fields)?; let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); - let from_value_impl = struct_from_value(&data, container_attrs.rename_all); - let expected_type_impl = - struct_expected_type(&data.fields, container_attrs.type_name.as_deref()); + let from_value_impl = struct_from_value(&data, &container_attrs)?; + let expected_type_impl = struct_expected_type( + &data.fields, + container_attrs.type_name.as_deref(), + &container_attrs, + )?; Ok(quote! { #[automatically_derived] impl #impl_generics nu_protocol::FromValue for #ident #ty_generics #where_clause { @@ -200,28 +204,28 @@ fn derive_struct_from_value( /// } /// } /// ``` -fn struct_from_value(data: &DataStruct, rename_all: Option) -> TokenStream2 { - let body = parse_value_via_fields(&data.fields, quote!(Self), rename_all); - quote! { +fn struct_from_value(data: &DataStruct, container_attrs: &ContainerAttributes) -> Result { + let body = parse_value_via_fields(&data.fields, quote!(Self), container_attrs)?; + Ok(quote! { fn from_value( v: nu_protocol::Value ) -> std::result::Result { #body } - } + }) } /// Implements `FromValue::expected_type` for structs. /// -/// This function constructs the `expected_type` function for structs. -/// The type depends on the `fields`: named fields construct a record type with every key and type -/// laid out. -/// Unnamed fields construct a custom type with the name in the format like -/// `list[type0, type1, type2]`. -/// No fields expect the `Type::Nothing`. +/// This function constructs the `expected_type` function for structs based on the provided fields. +/// The type depends on the `fields`: +/// - Named fields construct a record type where each key corresponds to a field name. +/// The specific keys are resolved by [`NameResolver::resolve_ident`]. +/// - Unnamed fields construct a custom type with the format `list[type0, type1, type2]`. +/// - Unit structs expect `Type::Nothing`. /// -/// If `#[nu_value(type_name = "...")]` is used, the output type will be `Type::Custom` with that -/// passed name. +/// If the `#[nu_value(type_name = "...")]` attribute is used, the output type will be +/// `Type::Custom` with the provided name. /// /// # Examples /// @@ -233,6 +237,7 @@ fn struct_from_value(data: &DataStruct, rename_all: Option) -> TokenStream /// struct Pet { /// name: String, /// age: u8, +/// #[nu_value(rename = "toy")] /// favorite_toy: Option, /// } /// @@ -249,7 +254,7 @@ fn struct_from_value(data: &DataStruct, rename_all: Option) -> TokenStream /// ::expected_type(), /// ), /// ( -/// std::string::ToString::to_string("favorite_toy"), +/// std::string::ToString::to_string("toy"), /// as nu_protocol::FromValue>::expected_type(), /// ) /// ].into_boxed_slice() @@ -305,7 +310,11 @@ fn struct_from_value(data: &DataStruct, rename_all: Option) -> TokenStream /// } /// } /// ``` -fn struct_expected_type(fields: &Fields, attr_type_name: Option<&str>) -> TokenStream2 { +fn struct_expected_type( + fields: &Fields, + attr_type_name: Option<&str>, + container_attrs: &ContainerAttributes, +) -> Result { let ty = match (fields, attr_type_name) { (_, Some(type_name)) => { quote!(nu_protocol::Type::Custom( @@ -313,20 +322,25 @@ fn struct_expected_type(fields: &Fields, attr_type_name: Option<&str>) -> TokenS )) } (Fields::Named(fields), _) => { - let fields = fields.named.iter().map(|field| { + let mut name_resolver = NameResolver::new(); + let mut fields_ts = Vec::with_capacity(fields.named.len()); + for field in fields.named.iter() { + let member_attrs = MemberAttributes::parse_attrs(&field.attrs)?; let ident = field.ident.as_ref().expect("named has idents"); - let ident_s = ident.to_string(); + let ident_s = + name_resolver.resolve_ident(ident, container_attrs, &member_attrs, None)?; let ty = &field.ty; - quote! {( + fields_ts.push(quote! {( std::string::ToString::to_string(#ident_s), <#ty as nu_protocol::FromValue>::expected_type(), - )} - }); + )}); + } quote!(nu_protocol::Type::Record( - std::vec![#(#fields),*].into_boxed_slice() + std::vec![#(#fields_ts),*].into_boxed_slice() )) } - (Fields::Unnamed(fields), _) => { + (f @ Fields::Unnamed(fields), _) => { + attributes::deny_fields(f)?; let mut iter = fields.unnamed.iter(); let fields = fields.unnamed.iter().map(|field| { let ty = &field.ty; @@ -352,11 +366,11 @@ fn struct_expected_type(fields: &Fields, attr_type_name: Option<&str>) -> TokenS (Fields::Unit, _) => quote!(nu_protocol::Type::Nothing), }; - quote! { + Ok(quote! { fn expected_type() -> nu_protocol::Type { #ty } - } + }) } /// Implements the `#[derive(FromValue)]` macro for enums. @@ -372,7 +386,7 @@ fn derive_enum_from_value( data: DataEnum, generics: Generics, attrs: Vec, -) -> Result { +) -> Result { let container_attrs = ContainerAttributes::parse_attrs(attrs.iter())?; let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); let from_value_impl = enum_from_value(&data, &attrs)?; @@ -393,9 +407,9 @@ fn derive_enum_from_value( /// should be represented via a `Value`. /// This function checks that every field is a unit variant and constructs a match statement over /// all possible variants. -/// The input value is expected to be a `Value::String` containing the name of the variant formatted -/// as defined by the `#[nu_value(rename_all = "...")]` attribute. -/// If no attribute is given, [`snake_case`](heck::ToSnakeCase) is expected. +/// The input value is expected to be a `Value::String` containing the name of the variant. +/// That string is defined by the [`NameResolver::resolve_ident`] method with the `default` value +/// being [`Case::Snake`]. /// /// If no matching variant is found, `ShellError::CantConvert` is returned. /// @@ -405,6 +419,7 @@ fn derive_enum_from_value( /// enum Weather { /// Sunny, /// Cloudy, +/// #[nu_value(rename = "rain")] /// Raining /// } /// @@ -417,7 +432,7 @@ fn derive_enum_from_value( /// match s.as_str() { /// "sunny" => std::result::Ok(Self::Sunny), /// "cloudy" => std::result::Ok(Self::Cloudy), -/// "raining" => std::result::Ok(Self::Raining), +/// "rain" => std::result::Ok(Self::Raining), /// _ => std::result::Result::Err(nu_protocol::ShellError::CantConvert { /// to_type: std::string::ToString::to_string( /// &::expected_type() @@ -429,17 +444,17 @@ fn derive_enum_from_value( /// } /// } /// ``` -fn enum_from_value(data: &DataEnum, attrs: &[Attribute]) -> Result { +fn enum_from_value(data: &DataEnum, attrs: &[Attribute]) -> Result { let container_attrs = ContainerAttributes::parse_attrs(attrs.iter())?; + let mut name_resolver = NameResolver::new(); let arms: Vec = data .variants .iter() .map(|variant| { - attributes::deny(&variant.attrs)?; + let member_attrs = MemberAttributes::parse_attrs(&variant.attrs)?; let ident = &variant.ident; - let ident_s = format!("{ident}") - .as_str() - .to_case(container_attrs.rename_all.unwrap_or(Case::Snake)); + let ident_s = + name_resolver.resolve_ident(ident, &container_attrs, &member_attrs, Case::Snake)?; match &variant.fields { Fields::Named(fields) => Err(DeriveError::UnsupportedEnums { fields_span: fields.span(), @@ -450,7 +465,7 @@ fn enum_from_value(data: &DataEnum, attrs: &[Attribute]) -> Result Ok(quote!(#ident_s => std::result::Result::Ok(Self::#ident))), } }) - .collect::>()?; + .collect::>()?; Ok(quote! { fn from_value( @@ -512,48 +527,50 @@ fn enum_expected_type(attr_type_name: Option<&str>) -> Option { /// Parses a `Value` into self. /// -/// This function handles the actual parsing of a `Value` into self. -/// It takes three parameters: `fields`, `self_ident` and `rename_all`. -/// The `fields` parameter determines the expected type of `Value`: named fields expect a -/// `Value::Record`, unnamed fields expect a `Value::List`, and a unit expects `Value::Nothing`. +/// This function handles parsing a `Value` into the corresponding struct or enum variant (`self`). +/// It takes three parameters: `fields`, `self_ident`, and `rename_all`. /// -/// For named fields, the `fields` parameter indicates which field in the record corresponds to -/// which struct field. -/// For both named and unnamed fields, it also helps cast the type into a `FromValue` type. -/// This approach maintains -/// [hygiene](https://doc.rust-lang.org/reference/macros-by-example.html#hygiene). +/// - The `fields` parameter specifies the expected structure of the `Value`: +/// - Named fields expect a `Value::Record`. +/// - Unnamed fields expect a `Value::List`. +/// - A unit struct expects `Value::Nothing`. /// -/// The `self_ident` parameter is used to describe the identifier of the returned value. -/// For structs, `Self` is usually sufficient, but for enums, `Self::Variant` may be needed in the -/// future. +/// For named fields, each field in the record is matched to a struct field. +/// The name matching uses the identifiers resolved by +/// [`NameResolver`](NameResolver::resolve_ident) with `default` being `None`. /// -/// The `rename_all` parameter is provided through `#[nu_value(rename_all = "...")]` and describes -/// how, if passed, the field keys in the `Value` should be named. -/// If this is `None`, we keep the names as they are in the struct. +/// The `self_ident` parameter is used to specify the identifier for the returned value. +/// For most structs, `Self` is sufficient, but `Self::Variant` may be needed for enum variants. +/// +/// The `container_attrs` parameters, provided through `#[nu_value]` on the container, defines +/// global rules for the `FromValue` implementation. +/// This is used for the [`NameResolver`] to resolve the correct ident in the `Value`. +/// +/// This function is more complex than the equivalent for `IntoValue` due to additional error +/// handling: +/// - If a named field is missing in the `Value`, `ShellError::CantFindColumn` is returned. +/// - For unit structs, if the value is not `Value::Nothing`, `ShellError::CantConvert` is returned. /// -/// This function is more complex than the equivalent for `IntoValue` due to error handling -/// requirements. -/// For missing fields, `ShellError::CantFindColumn` is used, and for unit structs, -/// `ShellError::CantConvert` is used. /// The implementation avoids local variables for fields to prevent accidental shadowing, ensuring -/// that poorly named fields don't cause issues. -/// While this style is not typically recommended in handwritten Rust, it is acceptable for code +/// that fields with similar names do not cause unexpected behavior. +/// This approach is not typically recommended in handwritten Rust, but it is acceptable for code /// generation. fn parse_value_via_fields( fields: &Fields, self_ident: impl ToTokens, - rename_all: Option, -) -> TokenStream2 { + container_attrs: &ContainerAttributes, +) -> Result { match fields { Fields::Named(fields) => { - let fields = fields.named.iter().map(|field| { + let mut name_resolver = NameResolver::new(); + let mut fields_ts: Vec = Vec::with_capacity(fields.named.len()); + for field in fields.named.iter() { + let member_attrs = MemberAttributes::parse_attrs(&field.attrs)?; let ident = field.ident.as_ref().expect("named has idents"); - let mut ident_s = ident.to_string(); - if let Some(rename_all) = rename_all { - ident_s = ident_s.to_case(rename_all); - } + let ident_s = + name_resolver.resolve_ident(ident, container_attrs, &member_attrs, None)?; let ty = &field.ty; - match type_is_option(ty) { + fields_ts.push(match type_is_option(ty) { true => quote! { #ident: record .remove(#ident_s) @@ -573,15 +590,16 @@ fn parse_value_via_fields( })?, )? }, - } - }); - quote! { + }); + } + Ok(quote! { let span = v.span(); let mut record = v.into_record()?; - std::result::Result::Ok(#self_ident {#(#fields),*}) - } + std::result::Result::Ok(#self_ident {#(#fields_ts),*}) + }) } - Fields::Unnamed(fields) => { + f @ Fields::Unnamed(fields) => { + attributes::deny_fields(f)?; let fields = fields.unnamed.iter().enumerate().map(|(i, field)| { let ty = &field.ty; quote! {{ @@ -596,14 +614,14 @@ fn parse_value_via_fields( )? }} }); - quote! { + Ok(quote! { let span = v.span(); let list = v.into_list()?; let mut deque: std::collections::VecDeque<_> = std::convert::From::from(list); std::result::Result::Ok(#self_ident(#(#fields),*)) - } + }) } - Fields::Unit => quote! { + Fields::Unit => Ok(quote! { match v { nu_protocol::Value::Nothing {..} => Ok(#self_ident), v => std::result::Result::Err(nu_protocol::ShellError::CantConvert { @@ -613,7 +631,7 @@ fn parse_value_via_fields( help: std::option::Option::None }) } - }, + }), } } diff --git a/crates/nu-derive-value/src/into.rs b/crates/nu-derive-value/src/into.rs index 370558005a..4a52119097 100644 --- a/crates/nu-derive-value/src/into.rs +++ b/crates/nu-derive-value/src/into.rs @@ -6,13 +6,15 @@ use syn::{ }; use crate::{ - attributes::{self, ContainerAttributes}, - case::{Case, Casing}, + attributes::{self, ContainerAttributes, MemberAttributes, ParseAttrs}, + case::Case, + names::NameResolver, }; #[derive(Debug)] pub struct IntoValue; type DeriveError = super::error::DeriveError; +type Result = std::result::Result; /// Inner implementation of the `#[derive(IntoValue)]` macro for structs and enums. /// @@ -23,7 +25,7 @@ type DeriveError = super::error::DeriveError; /// - For structs: [`struct_into_value`] /// - For enums: [`enum_into_value`] /// - Unions are not supported and will return an error. -pub fn derive_into_value(input: TokenStream2) -> Result { +pub fn derive_into_value(input: TokenStream2) -> Result { let input: DeriveInput = syn::parse2(input).map_err(DeriveError::Syn)?; match input.data { Data::Struct(data_struct) => Ok(struct_into_value( @@ -48,6 +50,7 @@ pub fn derive_into_value(input: TokenStream2) -> Result, -) -> Result { - let rename_all = ContainerAttributes::parse_attrs(attrs.iter())?.rename_all; - attributes::deny_fields(&data.fields)?; +) -> Result { + let container_attrs = ContainerAttributes::parse_attrs(attrs.iter())?; let record = match &data.fields { Fields::Named(fields) => { let accessor = fields @@ -121,7 +123,7 @@ fn struct_into_value( .iter() .map(|field| field.ident.as_ref().expect("named has idents")) .map(|ident| quote!(self.#ident)); - fields_return_value(&data.fields, accessor, rename_all) + fields_return_value(&data.fields, accessor, &container_attrs)? } Fields::Unnamed(fields) => { let accessor = fields @@ -130,7 +132,7 @@ fn struct_into_value( .enumerate() .map(|(n, _)| Index::from(n)) .map(|index| quote!(self.#index)); - fields_return_value(&data.fields, accessor, rename_all) + fields_return_value(&data.fields, accessor, &container_attrs)? } Fields::Unit => quote!(nu_protocol::Value::nothing(span)), }; @@ -150,10 +152,13 @@ fn struct_into_value( /// This function implements the derive macro `IntoValue` for enums. /// Currently, only unit enum variants are supported as it is not clear how other types of enums /// should be represented in a `Value`. -/// For simple enums, we represent the enum as a `Value::String`. For other types of variants, we return an error. -/// The variant name will be case-converted as described by the `#[nu_value(rename_all = "...")]` helper attribute. -/// If no attribute is used, the default is `case_convert::Case::Snake`. -/// The implementation matches over all variants, uses the appropriate variant name, and constructs a `Value::String`. +/// For simple enums, we represent the enum as a `Value::String`. +/// For other types of variants, we return an error. +/// +/// The variant name used in the `Value::String` is resolved by the +/// [`NameResolver`](NameResolver::resolve_ident) with the `default` being [`Case::Snake`]. +/// The implementation matches over all variants, uses the appropriate variant name, and constructs +/// a `Value::String`. /// /// This is how such a derived implementation looks: /// ```rust @@ -161,6 +166,7 @@ fn struct_into_value( /// enum Weather { /// Sunny, /// Cloudy, +/// #[nu_value(rename = "rain")] /// Raining /// } /// @@ -169,7 +175,7 @@ fn struct_into_value( /// match self { /// Self::Sunny => nu_protocol::Value::string("sunny", span), /// Self::Cloudy => nu_protocol::Value::string("cloudy", span), -/// Self::Raining => nu_protocol::Value::string("raining", span), +/// Self::Raining => nu_protocol::Value::string("rain", span), /// } /// } /// } @@ -179,17 +185,21 @@ fn enum_into_value( data: DataEnum, generics: Generics, attrs: Vec, -) -> Result { +) -> Result { let container_attrs = ContainerAttributes::parse_attrs(attrs.iter())?; + let mut name_resolver = NameResolver::new(); let arms: Vec = data .variants .into_iter() .map(|variant| { - attributes::deny(&variant.attrs)?; + let member_attrs = MemberAttributes::parse_attrs(variant.attrs.iter())?; let ident = variant.ident; - let ident_s = format!("{ident}") - .as_str() - .to_case(container_attrs.rename_all.unwrap_or(Case::Snake)); + let ident_s = name_resolver.resolve_ident( + &ident, + &container_attrs, + &member_attrs, + Case::Snake, + )?; match &variant.fields { // In the future we can implement more complex enums here. Fields::Named(fields) => Err(DeriveError::UnsupportedEnums { @@ -203,7 +213,7 @@ fn enum_into_value( } } }) - .collect::>()?; + .collect::>()?; let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); Ok(quote! { @@ -219,57 +229,63 @@ fn enum_into_value( /// Constructs the final `Value` that the macro generates. /// -/// This function handles the construction of the final `Value` that the macro generates. -/// It is currently only used for structs but may be used for enums in the future. -/// The function takes three parameters: the `fields`, which allow iterating over each field of a -/// data type, the `accessor` and `rename_all`. -/// The fields determine whether we need to generate a `Value::Record`, `Value::List`, or -/// `Value::Nothing`. -/// For named fields, they are also directly used to generate the record key. -/// If `#[nu_value(rename_all = "...")]` is used and then passed in here via `rename_all`, the -/// named fields will be converted to the given case and then uses as the record key. +/// This function handles the construction of the final `Value` that the macro generates, primarily +/// for structs. +/// It takes three parameters: `fields`, which allows iterating over each field of a data type, +/// `accessor`, which generalizes data access, and `container_attrs`, which is used for the +/// [`NameResolver`]. /// -/// The `accessor` parameter generalizes how the data is accessed. -/// For named fields, this is usually the name of the fields preceded by `self` in a struct, and -/// maybe something else for enums. -/// For unnamed fields, this should be an iterator similar to the one with named fields, but -/// accessing tuple fields, so we get `self.n`. -/// For unit structs, this parameter is ignored. -/// By using the accessor like this, we can have the same code for structs and enums with data +/// - **Field Keys**: +/// The field key is field name of the input struct and resolved the +/// [`NameResolver`](NameResolver::resolve_ident). +/// +/// - **Fields Type**: +/// - Determines whether to generate a `Value::Record`, `Value::List`, or `Value::Nothing` based +/// on the nature of the fields. +/// - Named fields are directly used to generate the record key, as described above. +/// +/// - **Accessor**: +/// - Generalizes how data is accessed for different data types. +/// - For named fields in structs, this is typically `self.field_name`. +/// - For unnamed fields (e.g., tuple structs), it should be an iterator similar to named fields +/// but accessing fields like `self.0`. +/// - For unit structs, this parameter is ignored. +/// +/// This design allows the same function to potentially handle both structs and enums with data /// variants in the future. fn fields_return_value( fields: &Fields, accessor: impl Iterator, - rename_all: Option, -) -> TokenStream2 { + container_attrs: &ContainerAttributes, +) -> Result { match fields { Fields::Named(fields) => { - let items: Vec = fields - .named - .iter() - .zip(accessor) - .map(|(field, accessor)| { - let ident = field.ident.as_ref().expect("named has idents"); - let mut field = ident.to_string(); - if let Some(rename_all) = rename_all { - field = field.to_case(rename_all); - } - quote!(#field => nu_protocol::IntoValue::into_value(#accessor, span)) - }) - .collect(); - quote! { + let mut name_resolver = NameResolver::new(); + let mut items: Vec = Vec::with_capacity(fields.named.len()); + for (field, accessor) in fields.named.iter().zip(accessor) { + let member_attrs = MemberAttributes::parse_attrs(field.attrs.iter())?; + let ident = field.ident.as_ref().expect("named has idents"); + let field = + name_resolver.resolve_ident(ident, container_attrs, &member_attrs, None)?; + items.push(quote!(#field => nu_protocol::IntoValue::into_value(#accessor, span))); + } + Ok(quote! { nu_protocol::Value::record(nu_protocol::record! { #(#items),* }, span) - } + }) } - Fields::Unnamed(fields) => { + f @ Fields::Unnamed(fields) => { + attributes::deny_fields(f)?; let items = fields.unnamed.iter().zip(accessor).map( |(_, accessor)| quote!(nu_protocol::IntoValue::into_value(#accessor, span)), ); - quote!(nu_protocol::Value::list(std::vec![#(#items),*], span)) + Ok(quote!(nu_protocol::Value::list( + std::vec![#(#items),*], + span + ))) } - Fields::Unit => quote!(nu_protocol::Value::nothing(span)), + Fields::Unit => Ok(quote!(nu_protocol::Value::nothing(span))), } } diff --git a/crates/nu-derive-value/src/lib.rs b/crates/nu-derive-value/src/lib.rs index 4fa038f2fd..5d87951927 100644 --- a/crates/nu-derive-value/src/lib.rs +++ b/crates/nu-derive-value/src/lib.rs @@ -36,6 +36,7 @@ mod case; mod error; mod from; mod into; +mod names; #[cfg(test)] mod tests; diff --git a/crates/nu-derive-value/src/names.rs b/crates/nu-derive-value/src/names.rs new file mode 100644 index 0000000000..3798395b7f --- /dev/null +++ b/crates/nu-derive-value/src/names.rs @@ -0,0 +1,61 @@ +use proc_macro2::Span; +use std::collections::HashMap; +use syn::Ident; + +use crate::attributes::{ContainerAttributes, MemberAttributes}; +use crate::case::{Case, Casing}; +use crate::error::DeriveError; + +#[derive(Debug, Default)] +pub struct NameResolver { + seen_names: HashMap, +} + +impl NameResolver { + pub fn new() -> Self { + Self::default() + } + + /// Resolves an identifier using attributes and ensures its uniqueness. + /// + /// The identifier is transformed according to these rules: + /// - If [`MemberAttributes::rename`] is set, this explicitly renamed value is used. + /// The value is defined by the helper attribute `#[nu_value(rename = "...")]` on a member. + /// - If the above is not set but [`ContainerAttributes::rename_all`] is, the identifier + /// undergoes case conversion as specified by the helper attribute + /// `#[nu_value(rename_all = "...")]` on the container (struct or enum). + /// - If neither renaming attribute is set, the function applies the case conversion provided + /// by the `default` parameter. + /// If `default` is `None`, the identifier remains unchanged. + /// + /// This function checks the transformed identifier against previously seen identifiers to + /// ensure it is unique. + /// If a duplicate identifier is detected, it returns [`DeriveError::NonUniqueName`]. + pub fn resolve_ident( + &mut self, + ident: &'_ Ident, + container_attrs: &'_ ContainerAttributes, + member_attrs: &'_ MemberAttributes, + default: impl Into>, + ) -> Result> { + let span = ident.span(); + let rename_all = container_attrs.rename_all; + let rename = member_attrs.rename.as_ref(); + let ident = match (rename, rename_all) { + (Some(rename), _) => rename.to_string(), + (None, Some(case)) => ident.to_case(case), + (None, None) => ident.to_case(default), + }; + + if let Some(seen) = self.seen_names.get(&ident) { + return Err(DeriveError::NonUniqueName { + name: ident.to_string(), + first: *seen, + second: span, + }); + } + + self.seen_names.insert(ident.clone(), span); + Ok(ident) + } +} diff --git a/crates/nu-derive-value/src/tests.rs b/crates/nu-derive-value/src/tests.rs index ca51ed5970..87a4b7c24b 100644 --- a/crates/nu-derive-value/src/tests.rs +++ b/crates/nu-derive-value/src/tests.rs @@ -86,24 +86,81 @@ fn unexpected_attribute() { } #[test] -fn deny_attribute_on_fields() { +fn unexpected_attribute_on_struct_field() { let input = quote! { - struct SomeStruct { - #[nu_value] - field: () + struct SimpleStruct { + #[nu_value(what)] + field_a: i32, + field_b: String, } }; let from_res = derive_from_value(input.clone()); assert!( - matches!(from_res, Err(DeriveError::InvalidAttributePosition { .. })), + matches!(from_res, Err(DeriveError::UnexpectedAttribute { .. })), + "expected `DeriveError::UnexpectedAttribute`, got {:?}", + from_res + ); + + let into_res = derive_into_value(input); + assert!( + matches!(into_res, Err(DeriveError::UnexpectedAttribute { .. })), + "expected `DeriveError::UnexpectedAttribute`, got {:?}", + into_res + ); +} + +#[test] +fn unexpected_attribute_on_enum_variant() { + let input = quote! { + enum SimpleEnum { + #[nu_value(what)] + A, + B, + } + }; + + let from_res = derive_from_value(input.clone()); + assert!( + matches!(from_res, Err(DeriveError::UnexpectedAttribute { .. })), + "expected `DeriveError::UnexpectedAttribute`, got {:?}", + from_res + ); + + let into_res = derive_into_value(input); + assert!( + matches!(into_res, Err(DeriveError::UnexpectedAttribute { .. })), + "expected `DeriveError::UnexpectedAttribute`, got {:?}", + into_res + ); +} + +#[test] +fn invalid_attribute_position_in_tuple_struct() { + let input = quote! { + struct SimpleTupleStruct( + #[nu_value(what)] + i32, + String, + ); + }; + + let from_res = derive_from_value(input.clone()); + assert!( + matches!( + from_res, + Err(DeriveError::InvalidAttributePosition { attribute_span: _ }) + ), "expected `DeriveError::InvalidAttributePosition`, got {:?}", from_res ); let into_res = derive_into_value(input); assert!( - matches!(into_res, Err(DeriveError::InvalidAttributePosition { .. })), + matches!( + into_res, + Err(DeriveError::InvalidAttributePosition { attribute_span: _ }) + ), "expected `DeriveError::InvalidAttributePosition`, got {:?}", into_res ); @@ -133,3 +190,53 @@ fn invalid_attribute_value() { into_res ); } + +#[test] +fn non_unique_struct_keys() { + let input = quote! { + struct DuplicateStruct { + #[nu_value(rename = "field")] + some_field: (), + field: (), + } + }; + + let from_res = derive_from_value(input.clone()); + assert!( + matches!(from_res, Err(DeriveError::NonUniqueName { .. })), + "expected `DeriveError::NonUniqueName`, got {:?}", + from_res + ); + + let into_res = derive_into_value(input); + assert!( + matches!(into_res, Err(DeriveError::NonUniqueName { .. })), + "expected `DeriveError::NonUniqueName`, got {:?}", + into_res + ); +} + +#[test] +fn non_unique_enum_variants() { + let input = quote! { + enum DuplicateEnum { + #[nu_value(rename = "variant")] + SomeVariant, + Variant + } + }; + + let from_res = derive_from_value(input.clone()); + assert!( + matches!(from_res, Err(DeriveError::NonUniqueName { .. })), + "expected `DeriveError::NonUniqueName`, got {:?}", + from_res + ); + + let into_res = derive_into_value(input); + assert!( + matches!(into_res, Err(DeriveError::NonUniqueName { .. })), + "expected `DeriveError::NonUniqueName`, got {:?}", + into_res + ); +} diff --git a/crates/nu-protocol/src/value/from_value.rs b/crates/nu-protocol/src/value/from_value.rs index 1b8e9536d7..624d7dae39 100644 --- a/crates/nu-protocol/src/value/from_value.rs +++ b/crates/nu-protocol/src/value/from_value.rs @@ -20,13 +20,17 @@ use std::{ /// /// When derived on structs with named fields, it expects a [`Value::Record`] where each field of /// the struct maps to a corresponding field in the record. -/// You can customize the case conversion of these field names by using -/// `#[nu_value(rename_all = "...")]` on the struct. +/// +/// - If `#[nu_value(rename = "...")]` is applied to a field, that name will be used as the key in +/// the record. +/// - If `#[nu_value(rename_all = "...")]` is applied on the container (struct) the key of the +/// field will be case-converted accordingly. +/// - If neither attribute is applied, the field name is used as is. +/// /// Supported case conversions include those provided by [`heck`], such as /// "snake_case", "kebab-case", "PascalCase", and others. /// Additionally, all values accepted by /// [`#[serde(rename_all = "...")]`](https://serde.rs/container-attrs.html#rename_all) are valid here. -/// If not set, the field names will match the original Rust field names as-is. /// /// For structs with unnamed fields, it expects a [`Value::List`], and the fields are populated in /// the order they appear in the list. @@ -35,13 +39,18 @@ use std::{ /// /// Only enums with no fields may derive this trait. /// The expected value representation will be the name of the variant as a [`Value::String`]. -/// By default, variant names will be expected in ["snake_case"](heck::ToSnakeCase). -/// You can customize the case conversion using `#[nu_value(rename_all = "kebab-case")]` on the enum. +/// +/// - If `#[nu_value(rename = "...")]` is applied to a variant, that name will be used. +/// - If `#[nu_value(rename_all = "...")]` is applied on the enum container, the name of variant +/// will be case-converted accordingly. +/// - If neither attribute is applied, the variant name will default to +/// ["snake_case"](heck::ToSnakeCase). /// /// Additionally, you can use `#[nu_value(type_name = "...")]` in the derive macro to set a custom type name /// for `FromValue::expected_type`. This will result in a `Type::Custom` with the specified type name. /// This can be useful in situations where the default type name is not desired. /// +/// # Enum Example /// ``` /// # use nu_protocol::{FromValue, Value, ShellError, record, Span}; /// # @@ -52,11 +61,17 @@ use std::{ /// enum Bird { /// MountainEagle, /// ForestOwl, +/// #[nu_value(rename = "RIVER-QUACK")] /// RiverDuck, /// } /// /// assert_eq!( -/// Bird::from_value(Value::test_string("RIVER-DUCK")).unwrap(), +/// Bird::from_value(Value::string("FOREST-OWL", span)).unwrap(), +/// Bird::ForestOwl +/// ); +/// +/// assert_eq!( +/// Bird::from_value(Value::string("RIVER-QUACK", span)).unwrap(), /// Bird::RiverDuck /// ); /// @@ -64,14 +79,21 @@ use std::{ /// &Bird::expected_type().to_string(), /// "birb" /// ); +/// ``` /// -/// +/// # Struct Example +/// ``` +/// # use nu_protocol::{FromValue, Value, ShellError, record, Span}; +/// # +/// # let span = Span::unknown(); +/// # /// #[derive(FromValue, PartialEq, Eq, Debug)] /// #[nu_value(rename_all = "kebab-case")] /// struct Person { /// first_name: String, /// last_name: String, -/// age: u32, +/// #[nu_value(rename = "age")] +/// age_years: u32, /// } /// /// let value = Value::record(record! { @@ -85,7 +107,7 @@ use std::{ /// Person { /// first_name: "John".into(), /// last_name: "Doe".into(), -/// age: 42, +/// age_years: 42, /// } /// ); /// ``` diff --git a/crates/nu-protocol/src/value/into_value.rs b/crates/nu-protocol/src/value/into_value.rs index c7b776071a..0cebd2e481 100644 --- a/crates/nu-protocol/src/value/into_value.rs +++ b/crates/nu-protocol/src/value/into_value.rs @@ -10,20 +10,30 @@ use std::{borrow::Borrow, collections::HashMap}; /// This trait can be used with `#[derive]`. /// When derived on structs with named fields, the resulting value representation will use /// [`Value::Record`], where each field of the record corresponds to a field of the struct. -/// By default, field names will be kept as-is, but you can customize the case conversion -/// for all fields in a struct by using `#[nu_value(rename_all = "kebab-case")]` on the struct. -/// All case options from [`heck`] are supported, as well as the values allowed by -/// [`#[serde(rename_all)]`](https://serde.rs/container-attrs.html#rename_all). +/// +/// By default, field names will be used as-is unless specified otherwise: +/// - If `#[nu_value(rename = "...")]` is applied to a specific field, that name is used. +/// - If `#[nu_value(rename_all = "...")]` is applied to the struct, field names will be +/// case-converted accordingly. +/// - If neither attribute is used, the original field name will be retained. /// /// For structs with unnamed fields, the value representation will be [`Value::List`], with all /// fields inserted into a list. /// Unit structs will be represented as [`Value::Nothing`] since they contain no data. /// -/// Only enums with no fields may derive this trait. -/// The resulting value representation will be the name of the variant as a [`Value::String`]. -/// By default, variant names will be converted to ["snake_case"](heck::ToSnakeCase). -/// You can customize the case conversion using `#[nu_value(rename_all = "kebab-case")]` on the enum. +/// For enums, the resulting value representation depends on the variant name: +/// - If `#[nu_value(rename = "...")]` is applied to a specific variant, that name is used. +/// - If `#[nu_value(rename_all = "...")]` is applied to the enum, variant names will be +/// case-converted accordingly. +/// - If neither attribute is used, variant names will default to snake_case. /// +/// Only enums with no fields may derive this trait. +/// The resulting value will be the name of the variant as a [`Value::String`]. +/// +/// All case options from [`heck`] are supported, as well as the values allowed by +/// [`#[serde(rename_all)]`](https://serde.rs/container-attrs.html#rename_all). +/// +/// # Enum Example /// ``` /// # use nu_protocol::{IntoValue, Value, Span, record}; /// # @@ -34,28 +44,41 @@ use std::{borrow::Borrow, collections::HashMap}; /// enum Bird { /// MountainEagle, /// ForestOwl, +/// #[nu_value(rename = "RIVER-QUACK")] /// RiverDuck, /// } /// /// assert_eq!( -/// Bird::RiverDuck.into_value(span), -/// Value::test_string("RIVER-DUCK") +/// Bird::ForestOwl.into_value(span), +/// Value::string("FOREST-OWL", span) /// ); /// +/// assert_eq!( +/// Bird::RiverDuck.into_value(span), +/// Value::string("RIVER-QUACK", span) +/// ); +/// ``` /// +/// # Struct Example +/// ``` +/// # use nu_protocol::{IntoValue, Value, Span, record}; +/// # +/// # let span = Span::unknown(); +/// # /// #[derive(IntoValue)] /// #[nu_value(rename_all = "kebab-case")] /// struct Person { /// first_name: String, /// last_name: String, -/// age: u32, +/// #[nu_value(rename = "age")] +/// age_years: u32, /// } /// /// assert_eq!( /// Person { /// first_name: "John".into(), /// last_name: "Doe".into(), -/// age: 42, +/// age_years: 42, /// }.into_value(span), /// Value::record(record! { /// "first-name" => Value::string("John", span), diff --git a/crates/nu-protocol/src/value/test_derive.rs b/crates/nu-protocol/src/value/test_derive.rs index 4209b872b3..cbb9ccbadf 100644 --- a/crates/nu-protocol/src/value/test_derive.rs +++ b/crates/nu-protocol/src/value/test_derive.rs @@ -586,3 +586,86 @@ fn enum_type_name_attr() { assert_eq!(TypeNameEnum::expected_type().to_string().as_str(), "enum"); } + +#[derive(IntoValue, FromValue, Default, Debug, PartialEq)] +struct RenamedFieldStruct { + #[nu_value(rename = "renamed")] + field: (), +} + +impl RenamedFieldStruct { + fn value() -> Value { + Value::test_record(record! { + "renamed" => Value::test_nothing(), + }) + } +} + +#[test] +fn renamed_field_struct_into_value() { + let expected = RenamedFieldStruct::value(); + let actual = RenamedFieldStruct::default().into_test_value(); + assert_eq!(expected, actual); +} + +#[test] +fn renamed_field_struct_from_value() { + let expected = RenamedFieldStruct::default(); + let actual = RenamedFieldStruct::from_value(RenamedFieldStruct::value()).unwrap(); + assert_eq!(expected, actual); +} + +#[test] +fn renamed_field_struct_roundtrip() { + let expected = RenamedFieldStruct::default(); + let actual = + RenamedFieldStruct::from_value(RenamedFieldStruct::default().into_test_value()).unwrap(); + assert_eq!(expected, actual); + + let expected = RenamedFieldStruct::value(); + let actual = RenamedFieldStruct::from_value(RenamedFieldStruct::value()) + .unwrap() + .into_test_value(); + assert_eq!(expected, actual); +} + +#[derive(IntoValue, FromValue, Default, Debug, PartialEq)] +enum RenamedVariantEnum { + #[default] + #[nu_value(rename = "renamed")] + Variant, +} + +impl RenamedVariantEnum { + fn value() -> Value { + Value::test_string("renamed") + } +} + +#[test] +fn renamed_variant_enum_into_value() { + let expected = RenamedVariantEnum::value(); + let actual = RenamedVariantEnum::default().into_test_value(); + assert_eq!(expected, actual); +} + +#[test] +fn renamed_variant_enum_from_value() { + let expected = RenamedVariantEnum::default(); + let actual = RenamedVariantEnum::from_value(RenamedVariantEnum::value()).unwrap(); + assert_eq!(expected, actual); +} + +#[test] +fn renamed_variant_enum_roundtrip() { + let expected = RenamedVariantEnum::default(); + let actual = + RenamedVariantEnum::from_value(RenamedVariantEnum::default().into_test_value()).unwrap(); + assert_eq!(expected, actual); + + let expected = RenamedVariantEnum::value(); + let actual = RenamedVariantEnum::from_value(RenamedVariantEnum::value()) + .unwrap() + .into_test_value(); + assert_eq!(expected, actual); +}