Added record key renaming for derive macros IntoValue and FromValue (#13699)

# Description

Using derived `IntoValue` and `FromValue` implementations on structs
with named fields currently produce `Value::Record`s where each key is
the key of the Rust struct. For records like the `$nu` constant, that
won't work as this record uses `kebab-case` for it's keys. To accomodate
this, I upgraded the `#[nu_value(rename_all = "...")]` helper attribute
to also work on structs with named fields which will rename the keys via
the same case conversion as the enums already have.

# User-Facing Changes
Users of these macros may choose different key styles for their in
`Value` representation.

# Tests + Formatting
I added the same test suite as enums already have and updated the traits
documentation with more examples that also pass the doc test.

# After Submitting
I played around with the `$nu` constant but got stuck at the point that
these keys are kebab-cased, with this, I can play around more with it.

crates/nu-derive-value/src/attributes.rs

@@ -3,21 +3,12 @@ use syn::{spanned::Spanned, Attribute, Fields, LitStr};
 
 use crate::{error::DeriveError, HELPER_ATTRIBUTE};
 
-#[derive(Debug)]
+#[derive(Debug, Default)]
 pub struct ContainerAttributes {
-    pub rename_all: Case,
+    pub rename_all: Option<Case>,
     pub type_name: Option<String>,
 }
 
-impl Default for ContainerAttributes {
-    fn default() -> Self {
-        Self {
-            rename_all: Case::Snake,
-            type_name: None,
-        }
-    }
-}
-
 impl ContainerAttributes {
     pub fn parse_attrs<'a, M>(
         iter: impl Iterator<Item = &'a Attribute>,
@@ -59,7 +50,7 @@ impl ContainerAttributes {
                                 return Ok(()); // We stored the err in `err`.
                             }
                         };
-                        container_attrs.rename_all = case;
+                        container_attrs.rename_all = Some(case);
                     }
                     "type_name" => {
                         let type_name: LitStr = meta.value()?.parse()?;

crates/nu-derive-value/src/from.rs

@@ -1,4 +1,4 @@
-use convert_case::Casing;
+use convert_case::{Case, Casing};
 use proc_macro2::TokenStream as TokenStream2;
 use quote::{quote, ToTokens};
 use syn::{
@@ -54,7 +54,7 @@ fn derive_struct_from_value(
     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);
+    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());
     Ok(quote! {
@@ -70,7 +70,7 @@ fn derive_struct_from_value(
 ///
 /// This function constructs the `from_value` function for structs.
 /// The implementation is straightforward as most of the heavy lifting is handled by
-/// `parse_value_via_fields`, and this function only needs to construct the signature around it.
+/// [`parse_value_via_fields`], and this function only needs to construct the signature around it.
 ///
 /// For structs with named fields, this constructs a large return type where each field
 /// contains the implementation for that specific field.
@@ -198,8 +198,8 @@ fn derive_struct_from_value(
 ///     }
 /// }
 /// ```
-fn struct_from_value(data: &DataStruct) -> TokenStream2 {
+fn struct_from_value(data: &DataStruct, rename_all: Option<Case>) -> TokenStream2 {
-    let body = parse_value_via_fields(&data.fields, quote!(Self));
+    let body = parse_value_via_fields(&data.fields, quote!(Self), rename_all);
     quote! {
         fn from_value(
             v: nu_protocol::Value
@@ -437,7 +437,7 @@ fn enum_from_value(data: &DataEnum, attrs: &[Attribute]) -> Result<TokenStream2,
             let ident = &variant.ident;
             let ident_s = format!("{ident}")
                 .as_str()
-                .to_case(container_attrs.rename_all);
+                .to_case(container_attrs.rename_all.unwrap_or(Case::Snake));
             match &variant.fields {
                 Fields::Named(fields) => Err(DeriveError::UnsupportedEnums {
                     fields_span: fields.span(),
@@ -511,7 +511,7 @@ fn enum_expected_type(attr_type_name: Option<&str>) -> Option<TokenStream2> {
 /// Parses a `Value` into self.
 ///
 /// This function handles the actual parsing of a `Value` into self.
-/// It takes two parameters: `fields` and `self_ident`.
+/// 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`.
 ///
@@ -525,6 +525,10 @@ fn enum_expected_type(attr_type_name: Option<&str>) -> Option<TokenStream2> {
 /// For structs, `Self` is usually sufficient, but for enums, `Self::Variant` may be needed in the
 /// future.
 ///
+/// 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.
+///
 /// 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,
@@ -533,12 +537,19 @@ fn enum_expected_type(attr_type_name: Option<&str>) -> Option<TokenStream2> {
 /// that poorly named fields don't cause issues.
 /// While this style is not typically recommended in handwritten Rust, it is acceptable for code
 /// generation.
-fn parse_value_via_fields(fields: &Fields, self_ident: impl ToTokens) -> TokenStream2 {
+fn parse_value_via_fields(
+    fields: &Fields,
+    self_ident: impl ToTokens,
+    rename_all: Option<Case>,
+) -> TokenStream2 {
     match fields {
         Fields::Named(fields) => {
             let fields = fields.named.iter().map(|field| {
                 let ident = field.ident.as_ref().expect("named has idents");
-                let ident_s = ident.to_string();
+                let mut ident_s = ident.to_string();
+                if let Some(rename_all) = rename_all {
+                    ident_s = ident_s.to_case(rename_all);
+                }
                 let ty = &field.ty;
                 match type_is_option(ty) {
                     true => quote! {

crates/nu-derive-value/src/into.rs

@@ -1,4 +1,4 @@
-use convert_case::Casing;
+use convert_case::{Case, Casing};
 use proc_macro2::TokenStream as TokenStream2;
 use quote::{quote, ToTokens};
 use syn::{
@@ -50,6 +50,9 @@ pub fn derive_into_value(input: TokenStream2) -> Result<TokenStream2, DeriveErro
 /// For structs with unnamed fields, this generates a `Value::List` with each field in the list.
 /// For unit structs, this generates `Value::Nothing`, because there is no data.
 ///
+/// This function provides the signature and prepares the call to the [`fields_return_value`]
+/// function which does the heavy lifting of creating the `Value` calls.
+///
 /// # Examples
 ///
 /// These examples show what the macro would generate.
@@ -107,7 +110,7 @@ fn struct_into_value(
     generics: Generics,
     attrs: Vec<Attribute>,
 ) -> Result<TokenStream2, DeriveError> {
-    let _ = ContainerAttributes::parse_attrs(attrs.iter())?;
+    let rename_all = ContainerAttributes::parse_attrs(attrs.iter())?.rename_all;
     attributes::deny_fields(&data.fields)?;
     let record = match &data.fields {
         Fields::Named(fields) => {
@@ -116,7 +119,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)
+            fields_return_value(&data.fields, accessor, rename_all)
         }
         Fields::Unnamed(fields) => {
             let accessor = fields
@@ -125,7 +128,7 @@ fn struct_into_value(
                 .enumerate()
                 .map(|(n, _)| Index::from(n))
                 .map(|index| quote!(self.#index));
-            fields_return_value(&data.fields, accessor)
+            fields_return_value(&data.fields, accessor, rename_all)
         }
         Fields::Unit => quote!(nu_protocol::Value::nothing(span)),
     };
@@ -184,7 +187,7 @@ fn enum_into_value(
             let ident = variant.ident;
             let ident_s = format!("{ident}")
                 .as_str()
-                .to_case(container_attrs.rename_all);
+                .to_case(container_attrs.rename_all.unwrap_or(Case::Snake));
             match &variant.fields {
                 // In the future we can implement more complexe enums here.
                 Fields::Named(fields) => Err(DeriveError::UnsupportedEnums {
@@ -216,11 +219,13 @@ fn enum_into_value(
 ///
 /// 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 two parameters: the `fields`, which allow iterating over each field of a data
+/// The function takes three parameters: the `fields`, which allow iterating over each field of a
-/// type, and the `accessor`.
+/// 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.
 ///
 /// 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
@@ -233,6 +238,7 @@ fn enum_into_value(
 fn fields_return_value(
     fields: &Fields,
     accessor: impl Iterator<Item = impl ToTokens>,
+    rename_all: Option<Case>,
 ) -> TokenStream2 {
     match fields {
         Fields::Named(fields) => {
@@ -242,7 +248,10 @@ fn fields_return_value(
                 .zip(accessor)
                 .map(|(field, accessor)| {
                     let ident = field.ident.as_ref().expect("named has idents");
-                    let field = ident.to_string();
+                    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();

crates/nu-protocol/src/value/from_value.rs

@@ -17,29 +17,36 @@ use std::{
 ///
 /// # Derivable
 /// This trait can be used with `#[derive]`.
+///
 /// 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.
+/// Supported case conversions include those provided by [`convert_case::Case`], 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.
-/// Unit structs expect  a [`Value::Nothing`], as they contain no data.
+/// Unit structs expect a [`Value::Nothing`], as they contain no data.
 /// Attempting to convert from a non-matching `Value` type will result in an error.
 ///
 /// 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"](convert_case::Case::Snake).
 /// You can customize the case conversion using `#[nu_value(rename_all = "kebab-case")]` on the enum.
-/// All deterministic and useful case conversions provided by [`convert_case::Case`] are supported
-/// by specifying the case name followed by "case".
-/// Also all values for
-/// [`#[serde(rename_all = "...")]`](https://serde.rs/container-attrs.html#rename_all) are valid
-/// here.
 ///
 /// 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.
 ///
 /// ```
-/// # use nu_protocol::{FromValue, Value, ShellError};
+/// # use nu_protocol::{FromValue, Value, ShellError, record, Span};
+/// #
+/// # let span = Span::unknown();
+/// #
 /// #[derive(FromValue, Debug, PartialEq)]
 /// #[nu_value(rename_all = "COBOL-CASE", type_name = "birb")]
 /// enum Bird {
@@ -57,6 +64,30 @@ use std::{
 ///     &Bird::expected_type().to_string(),
 ///     "birb"
 /// );
+///
+///
+/// #[derive(FromValue, PartialEq, Eq, Debug)]
+/// #[nu_value(rename_all = "kebab-case")]
+/// struct Person {
+///     first_name: String,
+///     last_name: String,
+///     age: u32,
+/// }
+///
+/// let value = Value::record(record! {
+///     "first-name" => Value::string("John", span),
+///     "last-name" => Value::string("Doe", span),
+///     "age" => Value::int(42, span),
+/// }, span);
+///
+/// assert_eq!(
+///     Person::from_value(value).unwrap(),
+///     Person {
+///         first_name: "John".into(),
+///         last_name: "Doe".into(),
+///         age: 42,
+///     }
+/// );
 /// ```
 pub trait FromValue: Sized {
     // TODO: instead of ShellError, maybe we could have a FromValueError that implements Into<ShellError>

crates/nu-protocol/src/value/into_value.rs

@@ -10,6 +10,11 @@ use crate::{Record, ShellError, Span, Value};
 /// 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 useful case options from [`convert_case::Case`] are supported, as well as the
+/// values allowed by [`#[serde(rename_all)]`](https://serde.rs/container-attrs.html#rename_all).
+///
 /// 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.
@@ -18,14 +23,12 @@ use crate::{Record, ShellError, Span, Value};
 /// 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"](convert_case::Case::Snake).
 /// You can customize the case conversion using `#[nu_value(rename_all = "kebab-case")]` on the enum.
-/// All deterministic and useful case conversions provided by [`convert_case::Case`] are supported
-/// by specifying the case name followed by "case".
-/// Also all values for
-/// [`#[serde(rename_all = "...")]`](https://serde.rs/container-attrs.html#rename_all) are valid
-/// here.
 ///
 /// ```
-/// # use nu_protocol::{IntoValue, Value, Span};
+/// # use nu_protocol::{IntoValue, Value, Span, record};
+/// #
+/// # let span = Span::unknown();
+/// #
 /// #[derive(IntoValue)]
 /// #[nu_value(rename_all = "COBOL-CASE")]
 /// enum Bird {
@@ -35,9 +38,31 @@ use crate::{Record, ShellError, Span, Value};
 /// }
 ///
 /// assert_eq!(
-///     Bird::RiverDuck.into_value(Span::unknown()),
+///     Bird::RiverDuck.into_value(span),
 ///     Value::test_string("RIVER-DUCK")
 /// );
+///
+///
+/// #[derive(IntoValue)]
+/// #[nu_value(rename_all = "kebab-case")]
+/// struct Person {
+///     first_name: String,
+///     last_name: String,
+///     age: u32,
+/// }
+///
+/// assert_eq!(
+///     Person {
+///         first_name: "John".into(),
+///         last_name: "Doe".into(),
+///         age: 42,
+///     }.into_value(span),
+///     Value::record(record! {
+///         "first-name" => Value::string("John", span),
+///         "last-name" => Value::string("Doe", span),
+///         "age" => Value::int(42, span),
+///     }, span)
+/// );
 /// ```
 pub trait IntoValue: Sized {
     /// Converts the given value to a [`Value`].

crates/nu-protocol/src/value/test_derive.rs

@@ -384,62 +384,133 @@ fn enum_incorrect_type() {
     assert!(res.is_err());
 }
 
-// Generate the `Enum` from before but with all possible `rename_all` variants.
+mod enum_rename_all {
-macro_rules! enum_rename_all {
+    use super::*;
-    ($($ident:ident: $case:literal => [$a1:literal, $b2:literal, $c3:literal]),*) => {
+    use crate as nu_protocol;
-        $(
-            #[derive(Debug, PartialEq, IntoValue, FromValue)]
-            #[nu_value(rename_all = $case)]
-            enum $ident {
-                AlphaOne,
-                BetaTwo,
-                CharlieThree
-            }
 
-            impl $ident {
+    // Generate the `Enum` from before but with all possible `rename_all` variants.
-                fn make() -> [Self; 3] {
+    macro_rules! enum_rename_all {
-                    [Self::AlphaOne, Self::BetaTwo, Self::CharlieThree]
+        ($($ident:ident: $case:literal => [$a1:literal, $b2:literal, $c3:literal]),*) => {
+            $(
+                #[derive(Debug, PartialEq, IntoValue, FromValue)]
+                #[nu_value(rename_all = $case)]
+                enum $ident {
+                    AlphaOne,
+                    BetaTwo,
+                    CharlieThree
                 }
 
-                fn value() -> Value {
+                impl $ident {
-                    Value::test_list(vec![
+                    fn make() -> [Self; 3] {
-                        Value::test_string($a1),
+                        [Self::AlphaOne, Self::BetaTwo, Self::CharlieThree]
-                        Value::test_string($b2),
+                    }
-                        Value::test_string($c3),
+
-                    ])
+                    fn value() -> Value {
+                        Value::test_list(vec![
+                            Value::test_string($a1),
+                            Value::test_string($b2),
+                            Value::test_string($c3),
+                        ])
+                    }
                 }
-            }
+            )*
-        )*
 
-        #[test]
+            #[test]
-        fn enum_rename_all_into_value() {$({
+            fn into_value() {$({
-            let expected = $ident::value();
+                let expected = $ident::value();
-            let actual = $ident::make().into_test_value();
+                let actual = $ident::make().into_test_value();
-            assert_eq!(expected, actual);
+                assert_eq!(expected, actual);
-        })*}
+            })*}
 
-        #[test]
+            #[test]
-        fn enum_rename_all_from_value() {$({
+            fn from_value() {$({
-            let expected = $ident::make();
+                let expected = $ident::make();
-            let actual = <[$ident; 3]>::from_value($ident::value()).unwrap();
+                let actual = <[$ident; 3]>::from_value($ident::value()).unwrap();
-            assert_eq!(expected, actual);
+                assert_eq!(expected, actual);
-        })*}
+            })*}
+        }
+    }
+
+    enum_rename_all! {
+        Upper: "UPPER CASE" => ["ALPHA ONE", "BETA TWO", "CHARLIE THREE"],
+        Lower: "lower case" => ["alpha one", "beta two", "charlie three"],
+        Title: "Title Case" => ["Alpha One", "Beta Two", "Charlie Three"],
+        Camel: "camelCase" => ["alphaOne", "betaTwo", "charlieThree"],
+        Pascal: "PascalCase" => ["AlphaOne", "BetaTwo", "CharlieThree"],
+        Snake: "snake_case" => ["alpha_one", "beta_two", "charlie_three"],
+        UpperSnake: "UPPER_SNAKE_CASE" => ["ALPHA_ONE", "BETA_TWO", "CHARLIE_THREE"],
+        Kebab: "kebab-case" => ["alpha-one", "beta-two", "charlie-three"],
+        Cobol: "COBOL-CASE" => ["ALPHA-ONE", "BETA-TWO", "CHARLIE-THREE"],
+        Train: "Train-Case" => ["Alpha-One", "Beta-Two", "Charlie-Three"],
+        Flat: "flatcase" => ["alphaone", "betatwo", "charliethree"],
+        UpperFlat: "UPPERFLATCASE" => ["ALPHAONE", "BETATWO", "CHARLIETHREE"]
     }
 }
 
-enum_rename_all! {
+mod named_fields_struct_rename_all {
-    Upper: "UPPER CASE" => ["ALPHA ONE", "BETA TWO", "CHARLIE THREE"],
+    use super::*;
-    Lower: "lower case" => ["alpha one", "beta two", "charlie three"],
+    use crate as nu_protocol;
-    Title: "Title Case" => ["Alpha One", "Beta Two", "Charlie Three"],
+
-    Camel: "camelCase" => ["alphaOne", "betaTwo", "charlieThree"],
+    macro_rules! named_fields_struct_rename_all {
-    Pascal: "PascalCase" => ["AlphaOne", "BetaTwo", "CharlieThree"],
+        ($($ident:ident: $case:literal => [$a1:literal, $b2:literal, $c3:literal]),*) => {
-    Snake: "snake_case" => ["alpha_one", "beta_two", "charlie_three"],
+            $(
-    UpperSnake: "UPPER_SNAKE_CASE" => ["ALPHA_ONE", "BETA_TWO", "CHARLIE_THREE"],
+                #[derive(Debug, PartialEq, IntoValue, FromValue)]
-    Kebab: "kebab-case" => ["alpha-one", "beta-two", "charlie-three"],
+                #[nu_value(rename_all = $case)]
-    Cobol: "COBOL-CASE" => ["ALPHA-ONE", "BETA-TWO", "CHARLIE-THREE"],
+                struct $ident {
-    Train: "Train-Case" => ["Alpha-One", "Beta-Two", "Charlie-Three"],
+                    alpha_one: (),
-    Flat: "flatcase" => ["alphaone", "betatwo", "charliethree"],
+                    beta_two: (),
-    UpperFlat: "UPPERFLATCASE" => ["ALPHAONE", "BETATWO", "CHARLIETHREE"]
+                    charlie_three: (),
+                }
+
+                impl $ident {
+                    fn make() -> Self {
+                        Self {
+                            alpha_one: (),
+                            beta_two: (),
+                            charlie_three: (),
+                        }
+                    }
+
+                    fn value() -> Value {
+                        Value::test_record(record! {
+                            $a1 => Value::test_nothing(),
+                            $b2 => Value::test_nothing(),
+                            $c3 => Value::test_nothing(),
+                        })
+                    }
+                }
+            )*
+
+            #[test]
+            fn into_value() {$({
+                let expected = $ident::value();
+                let actual = $ident::make().into_test_value();
+                assert_eq!(expected, actual);
+            })*}
+
+            #[test]
+            fn from_value() {$({
+                let expected = $ident::make();
+                let actual = $ident::from_value($ident::value()).unwrap();
+                assert_eq!(expected, actual);
+            })*}
+        }
+    }
+
+    named_fields_struct_rename_all! {
+        Upper: "UPPER CASE" => ["ALPHA ONE", "BETA TWO", "CHARLIE THREE"],
+        Lower: "lower case" => ["alpha one", "beta two", "charlie three"],
+        Title: "Title Case" => ["Alpha One", "Beta Two", "Charlie Three"],
+        Camel: "camelCase" => ["alphaOne", "betaTwo", "charlieThree"],
+        Pascal: "PascalCase" => ["AlphaOne", "BetaTwo", "CharlieThree"],
+        Snake: "snake_case" => ["alpha_one", "beta_two", "charlie_three"],
+        UpperSnake: "UPPER_SNAKE_CASE" => ["ALPHA_ONE", "BETA_TWO", "CHARLIE_THREE"],
+        Kebab: "kebab-case" => ["alpha-one", "beta-two", "charlie-three"],
+        Cobol: "COBOL-CASE" => ["ALPHA-ONE", "BETA-TWO", "CHARLIE-THREE"],
+        Train: "Train-Case" => ["Alpha-One", "Beta-Two", "Charlie-Three"],
+        Flat: "flatcase" => ["alphaone", "betatwo", "charliethree"],
+        UpperFlat: "UPPERFLATCASE" => ["ALPHAONE", "BETATWO", "CHARLIETHREE"]
+    }
 }
 
 #[derive(IntoValue, FromValue, Debug, PartialEq)]