From 86e6fe030bba7211983914748b650488e83cb673 Mon Sep 17 00:00:00 2001 From: benluelo Date: Wed, 16 Nov 2022 23:53:52 -0500 Subject: [PATCH] update DefaultNoBound derive macro MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix derive for empty enums Update derive & ui tests clean up Apply suggestions from code review Co-authored-by: Bastian Köcher rename variable formatting & clippy formatting --- .../procedural/src/default_no_bound.rs | 174 ++++++++++++------ frame/support/procedural/src/lib.rs | 2 +- frame/support/src/lib.rs | 13 +- frame/support/test/tests/derive_no_bound.rs | 39 +++- .../derive_no_bound_ui/default_empty_enum.rs | 4 + .../default_empty_enum.stderr | 5 + .../default_no_attribute.rs | 11 ++ .../default_no_attribute.stderr | 5 + .../default_too_many_attributes.rs | 13 ++ .../default_too_many_attributes.stderr | 21 +++ .../tests/derive_no_bound_ui/default_union.rs | 7 + .../derive_no_bound_ui/default_union.stderr | 5 + .../asset-tx-payment/src/lib.rs | 1 + 13 files changed, 237 insertions(+), 63 deletions(-) create mode 100644 frame/support/test/tests/derive_no_bound_ui/default_empty_enum.rs create mode 100644 frame/support/test/tests/derive_no_bound_ui/default_empty_enum.stderr create mode 100644 frame/support/test/tests/derive_no_bound_ui/default_no_attribute.rs create mode 100644 frame/support/test/tests/derive_no_bound_ui/default_no_attribute.stderr create mode 100644 frame/support/test/tests/derive_no_bound_ui/default_too_many_attributes.rs create mode 100644 frame/support/test/tests/derive_no_bound_ui/default_too_many_attributes.stderr create mode 100644 frame/support/test/tests/derive_no_bound_ui/default_union.rs create mode 100644 frame/support/test/tests/derive_no_bound_ui/default_union.stderr diff --git a/frame/support/procedural/src/default_no_bound.rs b/frame/support/procedural/src/default_no_bound.rs index 192be0786d96b..b174a49e91741 100644 --- a/frame/support/procedural/src/default_no_bound.rs +++ b/frame/support/procedural/src/default_no_bound.rs @@ -15,82 +15,142 @@ // See the License for the specific language governing permissions and // limitations under the License. -use syn::spanned::Spanned; +use proc_macro2::Span; +use quote::{quote, quote_spanned}; +use syn::{spanned::Spanned, Data, DeriveInput, Fields}; -/// Derive Clone but do not bound any generic. +/// Derive Default but do not bound any generic. pub fn derive_default_no_bound(input: proc_macro::TokenStream) -> proc_macro::TokenStream { - let input: syn::DeriveInput = match syn::parse(input) { - Ok(input) => input, - Err(e) => return e.to_compile_error().into(), - }; + let input = syn::parse_macro_input!(input as DeriveInput); let name = &input.ident; + let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); let impl_ = match input.data { - syn::Data::Struct(struct_) => match struct_.fields { - syn::Fields::Named(named) => { - let fields = named.named.iter().map(|i| &i.ident).map(|i| { - quote::quote_spanned!(i.span() => - #i: core::default::Default::default() - ) + Data::Struct(struct_) => match struct_.fields { + Fields::Named(named) => { + let fields = named.named.iter().map(|field| &field.ident).map(|ident| { + quote_spanned! {ident.span() => + #ident: core::default::Default::default() + } }); - quote::quote!( Self { #( #fields, )* } ) + quote!(Self { #( #fields, )* }) }, - syn::Fields::Unnamed(unnamed) => { - let fields = - unnamed.unnamed.iter().enumerate().map(|(i, _)| syn::Index::from(i)).map(|i| { - quote::quote_spanned!(i.span() => - core::default::Default::default() - ) - }); - - quote::quote!( Self ( #( #fields, )* ) ) + Fields::Unnamed(unnamed) => { + let fields = unnamed.unnamed.iter().map(|field| { + quote_spanned! {field.span()=> + core::default::Default::default() + } + }); + + quote!(Self( #( #fields, )* )) }, - syn::Fields::Unit => { - quote::quote!(Self) + Fields::Unit => { + quote!(Self) }, }, - syn::Data::Enum(enum_) => - if let Some(first_variant) = enum_.variants.first() { - let variant_ident = &first_variant.ident; - match &first_variant.fields { - syn::Fields::Named(named) => { - let fields = named.named.iter().map(|i| &i.ident).map(|i| { - quote::quote_spanned!(i.span() => - #i: core::default::Default::default() - ) - }); - - quote::quote!( #name :: #ty_generics :: #variant_ident { #( #fields, )* } ) - }, - syn::Fields::Unnamed(unnamed) => { - let fields = unnamed - .unnamed - .iter() - .enumerate() - .map(|(i, _)| syn::Index::from(i)) - .map(|i| { - quote::quote_spanned!(i.span() => + Data::Enum(enum_) => { + if enum_.variants.is_empty() { + return syn::Error::new_spanned(name, "cannot derive Default for an empty enum") + .to_compile_error() + .into() + } + + // all #[default] attrs with the variant they're on; i.e. a var + let default_variants = enum_ + .variants + .into_iter() + .filter(|variant| variant.attrs.iter().any(|attr| attr.path.is_ident("default"))) + .collect::>(); + + match &*default_variants { + [] => { + return syn::Error::new( + name.clone().span(), + // writing this as a regular string breaks rustfmt for some reason + r#"no default declared, make a variant default by placing `#[default]` above it"#, + ) + .into_compile_error() + .into() + }, + // only one variant with the #[default] attribute set + [default_variant] => { + let variant_attrs = default_variant + .attrs + .iter() + .filter(|a| a.path.is_ident("default")) + .collect::>(); + + // check that there is only one #[default] attribute on the variant + if let [first_attr, second_attr, additional_attrs @ ..] = &*variant_attrs { + let mut err = + syn::Error::new(Span::call_site(), "multiple `#[default]` attributes"); + + err.combine(syn::Error::new_spanned(first_attr, "`#[default]` used here")); + + err.extend([second_attr].into_iter().chain(additional_attrs).map( + |variant| { + syn::Error::new_spanned(variant, "`#[default]` used again here") + }, + )); + + return err.into_compile_error().into() + } + + let variant_ident = &default_variant.ident; + + let fully_qualified_variant_path = quote!(Self::#variant_ident); + + match &default_variant.fields { + Fields::Named(named) => { + let fields = + named.named.iter().map(|field| &field.ident).map(|ident| { + quote_spanned! {ident.span()=> + #ident: core::default::Default::default() + } + }); + + quote!(#fully_qualified_variant_path { #( #fields, )* }) + }, + Fields::Unnamed(unnamed) => { + let fields = unnamed.unnamed.iter().map(|field| { + quote_spanned! {field.span()=> core::default::Default::default() - ) + } }); - quote::quote!( #name :: #ty_generics :: #variant_ident ( #( #fields, )* ) ) - }, - syn::Fields::Unit => quote::quote!( #name :: #ty_generics :: #variant_ident ), - } - } else { - quote::quote!(Self) - }, - syn::Data::Union(_) => { - let msg = "Union type not supported by `derive(CloneNoBound)`"; - return syn::Error::new(input.span(), msg).to_compile_error().into() + quote!(#fully_qualified_variant_path( #( #fields, )* )) + }, + Fields::Unit => fully_qualified_variant_path, + } + }, + [first, additional @ ..] => { + let mut err = syn::Error::new(Span::call_site(), "multiple declared defaults"); + + err.combine(syn::Error::new_spanned(first, "first default")); + + err.extend( + additional + .into_iter() + .map(|variant| syn::Error::new_spanned(variant, "additional default")), + ); + + return err.into_compile_error().into() + }, + } }, + Data::Union(union_) => + return syn::Error::new_spanned( + union_.union_token, + "Union type not supported by `derive(DefaultNoBound)`", + ) + .to_compile_error() + .into(), }; - quote::quote!( + quote!( const _: () = { impl #impl_generics core::default::Default for #name #ty_generics #where_clause { fn default() -> Self { diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index cd30946ae7855..41dbc4ee9592c 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -582,7 +582,7 @@ pub fn derive_eq_no_bound(input: TokenStream) -> TokenStream { } /// derive `Default` but do no bound any generic. Docs are at `frame_support::DefaultNoBound`. -#[proc_macro_derive(DefaultNoBound)] +#[proc_macro_derive(DefaultNoBound, attributes(default))] pub fn derive_default_no_bound(input: TokenStream) -> TokenStream { default_no_bound::derive_default_no_bound(input) } diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 84e416e50544d..efecbb75f9c62 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -621,14 +621,23 @@ pub use frame_support_procedural::DebugNoBound; /// # use frame_support::DefaultNoBound; /// # use core::default::Default; /// trait Config { -/// type C: Default; +/// type C: Default; /// } /// /// // Foo implements [`Default`] because `C` bounds [`Default`]. /// // Otherwise compilation will fail with an output telling `c` doesn't implement [`Default`]. /// #[derive(DefaultNoBound)] /// struct Foo { -/// c: T::C, +/// c: T::C, +/// } +/// +/// // Also works with enums, by specifying the default with #[default]: +/// #[derive(DefaultNoBound)] +/// enum Bar { +/// // Bar will implement Default as long as all of the types within Baz also implement default. +/// #[default] +/// Baz(T::C), +/// Quxx, /// } /// ``` pub use frame_support_procedural::DefaultNoBound; diff --git a/frame/support/test/tests/derive_no_bound.rs b/frame/support/test/tests/derive_no_bound.rs index e1cf539f2ac58..f891b3a2d2db8 100644 --- a/frame/support/test/tests/derive_no_bound.rs +++ b/frame/support/test/tests/derive_no_bound.rs @@ -110,10 +110,33 @@ fn test_struct_unnamed() { assert!(b != a_1); } +#[derive(DebugNoBound, CloneNoBound, EqNoBound, PartialEqNoBound, DefaultNoBound)] +struct StructNoGenerics { + field1: u32, + field2: u64, +} + +#[derive(DebugNoBound, CloneNoBound, EqNoBound, PartialEqNoBound, DefaultNoBound)] +enum EnumNoGenerics { + #[default] + VariantUnnamed(u32, u64), + VariantNamed { + a: u32, + b: u64, + }, + VariantUnit, +} + #[derive(DebugNoBound, CloneNoBound, EqNoBound, PartialEqNoBound, DefaultNoBound)] enum Enum { + #[default] VariantUnnamed(u32, u64, T::C, core::marker::PhantomData<(U, V)>), - VariantNamed { a: u32, b: u64, c: T::C, phantom: core::marker::PhantomData<(U, V)> }, + VariantNamed { + a: u32, + b: u64, + c: T::C, + phantom: core::marker::PhantomData<(U, V)>, + }, VariantUnit, VariantUnit2, } @@ -121,7 +144,12 @@ enum Enum { // enum that will have a named default. #[derive(DebugNoBound, CloneNoBound, EqNoBound, PartialEqNoBound, DefaultNoBound)] enum Enum2 { - VariantNamed { a: u32, b: u64, c: T::C }, + #[default] + VariantNamed { + a: u32, + b: u64, + c: T::C, + }, VariantUnnamed(u32, u64, T::C), VariantUnit, VariantUnit2, @@ -130,8 +158,13 @@ enum Enum2 { // enum that will have a unit default. #[derive(DebugNoBound, CloneNoBound, EqNoBound, PartialEqNoBound, DefaultNoBound)] enum Enum3 { + #[default] VariantUnit, - VariantNamed { a: u32, b: u64, c: T::C }, + VariantNamed { + a: u32, + b: u64, + c: T::C, + }, VariantUnnamed(u32, u64, T::C), VariantUnit2, } diff --git a/frame/support/test/tests/derive_no_bound_ui/default_empty_enum.rs b/frame/support/test/tests/derive_no_bound_ui/default_empty_enum.rs new file mode 100644 index 0000000000000..51b6137c00755 --- /dev/null +++ b/frame/support/test/tests/derive_no_bound_ui/default_empty_enum.rs @@ -0,0 +1,4 @@ +#[derive(frame_support::DefaultNoBound)] +enum Empty {} + +fn main() {} diff --git a/frame/support/test/tests/derive_no_bound_ui/default_empty_enum.stderr b/frame/support/test/tests/derive_no_bound_ui/default_empty_enum.stderr new file mode 100644 index 0000000000000..9c93b515adce5 --- /dev/null +++ b/frame/support/test/tests/derive_no_bound_ui/default_empty_enum.stderr @@ -0,0 +1,5 @@ +error: cannot derive Default for an empty enum + --> tests/derive_no_bound_ui/default_empty_enum.rs:2:6 + | +2 | enum Empty {} + | ^^^^^ diff --git a/frame/support/test/tests/derive_no_bound_ui/default_no_attribute.rs b/frame/support/test/tests/derive_no_bound_ui/default_no_attribute.rs new file mode 100644 index 0000000000000..185df01fe2b84 --- /dev/null +++ b/frame/support/test/tests/derive_no_bound_ui/default_no_attribute.rs @@ -0,0 +1,11 @@ +trait Config { + type C; +} + +#[derive(frame_support::DefaultNoBound)] +enum Foo { + Bar(T::C), + Baz, +} + +fn main() {} diff --git a/frame/support/test/tests/derive_no_bound_ui/default_no_attribute.stderr b/frame/support/test/tests/derive_no_bound_ui/default_no_attribute.stderr new file mode 100644 index 0000000000000..12e0023671587 --- /dev/null +++ b/frame/support/test/tests/derive_no_bound_ui/default_no_attribute.stderr @@ -0,0 +1,5 @@ +error: no default declared, make a variant default by placing `#[default]` above it + --> tests/derive_no_bound_ui/default_no_attribute.rs:6:6 + | +6 | enum Foo { + | ^^^ diff --git a/frame/support/test/tests/derive_no_bound_ui/default_too_many_attributes.rs b/frame/support/test/tests/derive_no_bound_ui/default_too_many_attributes.rs new file mode 100644 index 0000000000000..c3d175da6c056 --- /dev/null +++ b/frame/support/test/tests/derive_no_bound_ui/default_too_many_attributes.rs @@ -0,0 +1,13 @@ +trait Config { + type C; +} + +#[derive(frame_support::DefaultNoBound)] +enum Foo { + #[default] + Bar(T::C), + #[default] + Baz, +} + +fn main() {} diff --git a/frame/support/test/tests/derive_no_bound_ui/default_too_many_attributes.stderr b/frame/support/test/tests/derive_no_bound_ui/default_too_many_attributes.stderr new file mode 100644 index 0000000000000..5430ef142c5c8 --- /dev/null +++ b/frame/support/test/tests/derive_no_bound_ui/default_too_many_attributes.stderr @@ -0,0 +1,21 @@ +error: multiple declared defaults + --> tests/derive_no_bound_ui/default_too_many_attributes.rs:5:10 + | +5 | #[derive(frame_support::DefaultNoBound)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the derive macro `frame_support::DefaultNoBound` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: first default + --> tests/derive_no_bound_ui/default_too_many_attributes.rs:7:2 + | +7 | / #[default] +8 | | Bar(T::C), + | |_____________^ + +error: additional default + --> tests/derive_no_bound_ui/default_too_many_attributes.rs:9:2 + | +9 | / #[default] +10 | | Baz, + | |_______^ diff --git a/frame/support/test/tests/derive_no_bound_ui/default_union.rs b/frame/support/test/tests/derive_no_bound_ui/default_union.rs new file mode 100644 index 0000000000000..5822cda1aa64d --- /dev/null +++ b/frame/support/test/tests/derive_no_bound_ui/default_union.rs @@ -0,0 +1,7 @@ +#[derive(frame_support::DefaultNoBound)] +union Foo { + field1: u32, + field2: (), +} + +fn main() {} diff --git a/frame/support/test/tests/derive_no_bound_ui/default_union.stderr b/frame/support/test/tests/derive_no_bound_ui/default_union.stderr new file mode 100644 index 0000000000000..1e01e1baaf8ac --- /dev/null +++ b/frame/support/test/tests/derive_no_bound_ui/default_union.stderr @@ -0,0 +1,5 @@ +error: Union type not supported by `derive(DefaultNoBound)` + --> tests/derive_no_bound_ui/default_union.rs:2:1 + | +2 | union Foo { + | ^^^^^ diff --git a/frame/transaction-payment/asset-tx-payment/src/lib.rs b/frame/transaction-payment/asset-tx-payment/src/lib.rs index e136da8b0bb75..43cc1efa0858e 100644 --- a/frame/transaction-payment/asset-tx-payment/src/lib.rs +++ b/frame/transaction-payment/asset-tx-payment/src/lib.rs @@ -97,6 +97,7 @@ pub(crate) type ChargeAssetLiquidityOf = #[derive(Encode, Decode, DefaultNoBound, TypeInfo)] pub enum InitialPayment { /// No initial fee was payed. + #[default] Nothing, /// The initial fee was payed in the native currency. Native(LiquidityInfoOf),