extern/nushell
1
0
Fork 1
mirror of https://github.com/nushell/nushell.git synced 2024-11-28 11:24:09 +01:00
Code Issues Packages Projects Releases Wiki Activity

Change expected type for derived FromValue implementations via attribute (#13647)

Browse Source 
<!--
if this PR closes one or more issues, you can automatically link the PR
with
them by using one of the [*linking
keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword),
e.g.
- this PR should close #xxxx
- fixes #xxxx

you can also mention related issues, PRs or discussions!
-->

# Description
<!--
Thank you for improving Nushell. Please, check our [contributing
guide](../CONTRIBUTING.md) and talk to the core team before making major
changes.

Description of your pull request goes here. **Provide examples and/or
screenshots** if your changes affect the user experience.
-->
In this PR I expanded the helper attribute `#[nu_value]` on
`#[derive(FromValue)]`. It now allows the usage of `#[nu_value(type_name
= "...")]` to set a type name for the `FromValue::expected_type`
implementation. Currently it only uses the default implementation but
I'd like to change that without having to manually implement the entire
trait on my own.

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->
Users that derive `FromValue` may now change the name of the expected
type.

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use toolkit.nu; toolkit test stdlib"` to run the
tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->
I added some tests that check if this feature work and updated the
documentation about the derive macro.

- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->
This commit is contained in:
Piepmatz 2024-08-23 13:47:15 +02:00 committed by GitHub
parent 712fec166d
commit 39b0f3bdda
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 117 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
crates/nu-derive-value/src/attributes.rs
View File

@ -6,12 +6,14 @@ use crate::{error::DeriveError, HELPER_ATTRIBUTE};
#[derive(Debug)]
pub struct ContainerAttributes {
pub rename_all: Case,
pub type_name: Option<String>,
}
impl Default for ContainerAttributes {
fn default() -> Self {
Self {
rename_all: Case::Snake,
type_name: None,
}
}
}
@ -59,6 +61,11 @@ impl ContainerAttributes {
};
container_attrs.rename_all = case;
}
"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(),

96
crates/nu-derive-value/src/from.rs
View File

@ -42,9 +42,7 @@ pub fn derive_from_value(input: TokenStream2) -> Result<TokenStream2, DeriveErro
/// Implements the `#[derive(FromValue)]` macro for structs.
///
/// This function ensures that the helper attribute is not used anywhere, as it is not supported for
/// structs.
/// Other than this, this function provides the impl signature for `FromValue`.
/// This function provides the impl signature for `FromValue`.
/// The implementation for `FromValue::from_value` is handled by [`struct_from_value`] and the
/// `FromValue::expected_type` is handled by [`struct_expected_type`].
fn derive_struct_from_value(
@ -53,11 +51,12 @@ fn derive_struct_from_value(
generics: Generics,
attrs: Vec<Attribute>,
) -> Result<TokenStream2, DeriveError> {
attributes::deny(&attrs)?;
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 expected_type_impl = struct_expected_type(&data.fields);
let expected_type_impl =
struct_expected_type(&data.fields, container_attrs.type_name.as_deref());
Ok(quote! {
#[automatically_derived]
impl #impl_generics nu_protocol::FromValue for #ident #ty_generics #where_clause {
@ -219,13 +218,16 @@ fn struct_from_value(data: &DataStruct) -> TokenStream2 {
/// `list[type0, type1, type2]`.
/// No fields expect the `Type::Nothing`.
///
/// If `#[nu_value(type_name = "...")]` is used, the output type will be `Type::Custom` with that
/// passed name.
///
/// # Examples
///
/// These examples show what the macro would generate.
///
/// Struct with named fields:
/// ```rust
/// #[derive(IntoValue)]
/// #[derive(FromValue)]
/// struct Pet {
/// name: String,
/// age: u8,
@ -256,7 +258,7 @@ fn struct_from_value(data: &DataStruct) -> TokenStream2 {
///
/// Struct with unnamed fields:
/// ```rust
/// #[derive(IntoValue)]
/// #[derive(FromValue)]
/// struct Color(u8, u8, u8);
///
/// impl nu_protocol::FromValue for Color {
@ -276,7 +278,7 @@ fn struct_from_value(data: &DataStruct) -> TokenStream2 {
///
/// Unit struct:
/// ```rust
/// #[derive(IntoValue)]
/// #[derive(FromValue)]
/// struct Unicorn;
///
/// impl nu_protocol::FromValue for Color {
@ -285,9 +287,30 @@ fn struct_from_value(data: &DataStruct) -> TokenStream2 {
/// }
/// }
/// ```
fn struct_expected_type(fields: &Fields) -> TokenStream2 {
let ty = match fields {
Fields::Named(fields) => {
///
/// Struct with passed type name:
/// ```rust
/// #[derive(FromValue)]
/// #[nu_value(type_name = "bird")]
/// struct Parrot;
///
/// impl nu_protocol::FromValue for Parrot {
/// fn expected_type() -> nu_protocol::Type {
/// nu_protocol::Type::Custom(
/// <std::string::String as std::convert::From::<&str>>::from("bird")
/// .into_boxed_str()
/// )
/// }
/// }
/// ```
fn struct_expected_type(fields: &Fields, attr_type_name: Option<&str>) -> TokenStream2 {
let ty = match (fields, attr_type_name) {
(_, Some(type_name)) => {
quote!(nu_protocol::Type::Custom(
<std::string::String as std::convert::From::<&str>>::from(#type_name).into_boxed_str()
))
}
(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();
@ -301,7 +324,7 @@ fn struct_expected_type(fields: &Fields) -> TokenStream2 {
std::vec![#(#fields),*].into_boxed_slice()
))
}
Fields::Unnamed(fields) => {
(Fields::Unnamed(fields), _) => {
let mut iter = fields.unnamed.iter();
let fields = fields.unnamed.iter().map(|field| {
let ty = &field.ty;
@ -324,7 +347,7 @@ fn struct_expected_type(fields: &Fields) -> TokenStream2 {
)
}
}
Fields::Unit => quote!(nu_protocol::Type::Nothing),
(Fields::Unit, _) => quote!(nu_protocol::Type::Nothing),
};
quote! {
@ -338,23 +361,25 @@ fn struct_expected_type(fields: &Fields) -> TokenStream2 {
///
/// This function constructs the implementation of the `FromValue` trait for enums.
/// It is designed to be on the same level as [`derive_struct_from_value`], even though this
/// function only provides the impl signature for `FromValue`.
/// The actual implementation for `FromValue::from_value` is handled by [`enum_from_value`].
///
/// Since variants are difficult to type with the current type system, this function uses the
/// default implementation for `expected_type`.
/// implementation is a lot simpler.
/// The main `FromValue::from_value` implementation is handled by [`enum_from_value`].
/// The `FromValue::expected_type` implementation is usually kept empty to use the default
/// implementation, but if `#[nu_value(type_name = "...")]` if given, we use that.
fn derive_enum_from_value(
ident: Ident,
data: DataEnum,
generics: Generics,
attrs: Vec<Attribute>,
) -> Result<TokenStream2, DeriveError> {
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)?;
let expected_type_impl = enum_expected_type(container_attrs.type_name.as_deref());
Ok(quote! {
#[automatically_derived]
impl #impl_generics nu_protocol::FromValue for #ident #ty_generics #where_clause {
#from_value_impl
#expected_type_impl
}
})
}
@ -448,6 +473,41 @@ fn enum_from_value(data: &DataEnum, attrs: &[Attribute]) -> Result<TokenStream2,
})
}
/// Implements `FromValue::expected_type` for enums.
///
/// Since it's difficult to name the type of an enum in the current type system, we want to use the
/// default implementation if `#[nu_value(type_name = "...")]` was *not* given.
/// For that, a `None` value is returned, for a passed type name we return something like this:
/// ```rust
/// #[derive(IntoValue)]
/// #[nu_value(type_name = "sunny | cloudy | raining")]
/// enum Weather {
/// Sunny,
/// Cloudy,
/// Raining
/// }
///
/// impl nu_protocol::FromValue for Weather {
/// fn expected_type() -> nu_protocol::Type {
/// nu_protocol::Type::Custom(
/// <std::string::String as std::convert::From::<&str>>::from("sunny | cloudy | raining")
/// .into_boxed_str()
/// )
/// }
/// }
/// ```
fn enum_expected_type(attr_type_name: Option<&str>) -> Option<TokenStream2> {
let type_name = attr_type_name?;
Some(quote! {
fn expected_type() -> nu_protocol::Type {
nu_protocol::Type::Custom(
<std::string::String as std::convert::From::<&str>>::from(#type_name)
.into_boxed_str()
)
}
})
}
/// Parses a `Value` into self.
///
/// This function handles the actual parsing of a `Value` into self.

4
crates/nu-derive-value/src/into.rs
View File

@ -50,8 +50,6 @@ 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.
///
/// Note: The helper attribute `#[nu_value(...)]` is currently not allowed on structs.
///
/// # Examples
///
/// These examples show what the macro would generate.
@ -109,7 +107,7 @@ fn struct_into_value(
generics: Generics,
attrs: Vec<Attribute>,
) -> Result<TokenStream2, DeriveError> {
attributes::deny(&attrs)?;
let _ = ContainerAttributes::parse_attrs(attrs.iter())?;
attributes::deny_fields(&data.fields)?;
let record = match &data.fields {
Fields::Named(fields) => {

22
crates/nu-derive-value/src/tests.rs
View File

@ -85,28 +85,6 @@ fn unexpected_attribute() {
);
}
#[test]
fn deny_attribute_on_structs() {
let input = quote! {
#[nu_value]
struct SomeStruct;
};
let from_res = derive_from_value(input.clone());
assert!(
matches!(from_res, Err(DeriveError::InvalidAttributePosition { .. })),
"expected `DeriveError::InvalidAttributePosition`, got {:?}",
from_res
);
let into_res = derive_into_value(input);
assert!(
matches!(into_res, Err(DeriveError::InvalidAttributePosition { .. })),
"expected `DeriveError::InvalidAttributePosition`, got {:?}",
into_res
);
}
#[test]
fn deny_attribute_on_fields() {
let input = quote! {

11
crates/nu-protocol/src/value/from_value.rs
View File

@ -34,10 +34,14 @@ use std::{
/// [`#[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};
/// #[derive(FromValue, Debug, PartialEq)]
/// #[nu_value(rename_all = "COBOL-CASE")]
/// #[nu_value(rename_all = "COBOL-CASE", type_name = "birb")]
/// enum Bird {
/// MountainEagle,
/// ForestOwl,
@ -48,6 +52,11 @@ use std::{
/// Bird::from_value(Value::test_string("RIVER-DUCK")).unwrap(),
/// Bird::RiverDuck
/// );
///
/// assert_eq!(
/// &Bird::expected_type().to_string(),
/// "birb"
/// );
/// ```
pub trait FromValue: Sized {
// TODO: instead of ShellError, maybe we could have a FromValueError that implements Into<ShellError>

21
crates/nu-protocol/src/value/test_derive.rs
View File

@ -494,3 +494,24 @@ fn bytes_roundtrip() {
.into_test_value();
assert_eq!(expected, actual);
}
#[test]
fn struct_type_name_attr() {
#[derive(FromValue, Debug)]
#[nu_value(type_name = "struct")]
struct TypeNameStruct;
assert_eq!(
TypeNameStruct::expected_type().to_string().as_str(),
"struct"
);
}
#[test]
fn enum_type_name_attr() {
#[derive(FromValue, Debug)]
#[nu_value(type_name = "enum")]
struct TypeNameEnum;
assert_eq!(TypeNameEnum::expected_type().to_string().as_str(), "enum");
}