update DefaultNoBound derive macro

fix derive for empty enums

Update derive & ui tests

clean up

Apply suggestions from code review

Co-authored-by: Bastian Köcher <[email protected]>

rename variable

frame/support/procedural/src/default_no_bound.rs

@@ -15,82 +15,151 @@
 // 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()
-						)
-					});
+			Fields::Unnamed(unnamed) => {
+				let fields = unnamed.unnamed.iter().map(|field| {
+					quote_spanned! {field.span()=>
+						core::default::Default::default()
+					}
+				});
 
-				quote::quote!( Self ( #( #fields, )* ) )
+				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, )* } )
+		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();
+			} else {
+				// 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::<Vec<_>>();
+
+				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();
 					},
-					syn::Fields::Unnamed(unnamed) => {
-						let fields = unnamed
-							.unnamed
+					// only one variant with the #[default] attribute set
+					[default_variant] => {
+						let variant_attrs = default_variant
+							.attrs
 							.iter()
-							.enumerate()
-							.map(|(i, _)| syn::Index::from(i))
-							.map(|i| {
-								quote::quote_spanned!(i.span() =>
-									core::default::Default::default()
-								)
-							});
-
-						quote::quote!( #name :: #ty_generics :: #variant_ident ( #( #fields, )* ) )
+							.filter(|a| a.path.is_ident("default"))
+							.collect::<Vec<_>>();
+
+						// 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!(#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();
 					},
-					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()
+			}
+		},
+		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 {

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)
 }

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<T: Config> {
-/// 		c: T::C,
+/// 	c: T::C,
+/// }
+///
+/// // Also works with enums, by specifying the default with #[default]:
+/// #[derive(DefaultNoBound)]
+/// enum Bar<T: Config> {
+/// 	// 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;

frame/support/test/tests/derive_no_bound.rs

@@ -110,18 +110,46 @@ 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<T: Config, U, V> {
+	#[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,
 }
 
 // enum that will have a named default.
 #[derive(DebugNoBound, CloneNoBound, EqNoBound, PartialEqNoBound, DefaultNoBound)]
 enum Enum2<T: Config> {
-	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<T: Config> {
 // enum that will have a unit default.
 #[derive(DebugNoBound, CloneNoBound, EqNoBound, PartialEqNoBound, DefaultNoBound)]
 enum Enum3<T: Config> {
+	#[default]
 	VariantUnit,
-	VariantNamed { a: u32, b: u64, c: T::C },
+	VariantNamed {
+		a: u32,
+		b: u64,
+		c: T::C,
+	},
 	VariantUnnamed(u32, u64, T::C),
 	VariantUnit2,
 }

frame/support/test/tests/derive_no_bound_ui/default_empty_enum.rs

@@ -0,0 +1,4 @@
+#[derive(frame_support::DefaultNoBound)]
+enum Empty {}
+
+fn main() {}

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 {}
+  |      ^^^^^

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<T: Config> {
+	Bar(T::C),
+	Baz,
+}
+
+fn main() {}

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<T: Config> {
+  |      ^^^

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<T: Config> {
+	#[default]
+	Bar(T::C),
+	#[default]
+	Baz,
+}
+
+fn main() {}

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,
+   | |_______^

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() {}

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 {
+  | ^^^^^

frame/transaction-payment/asset-tx-payment/src/lib.rs

@@ -97,6 +97,7 @@ pub(crate) type ChargeAssetLiquidityOf<T> =
 #[derive(Encode, Decode, DefaultNoBound, TypeInfo)]
 pub enum InitialPayment<T: Config> {
 	/// No initial fee was payed.
+	#[default]
 	Nothing,
 	/// The initial fee was payed in the native currency.
 	Native(LiquidityInfoOf<T>),