update DefaultNoBound derive macro (#12723)

fix derive for empty enums

Update derive & ui tests

clean up

Apply suggestions from code review

Co-authored-by: Bastian Köcher <git@kchr.de>

rename variable

formatting & clippy

formatting

Co-authored-by: parity-processbot <>

substrate/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()
-						)
-					});
+			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()
-							)
-						});
+		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()
+			}
 
-						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() =>
+			// 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()
+				},
+				// 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::<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::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 {

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

substrate/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;

substrate/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<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,
 }
@@ -121,7 +144,12 @@ enum Enum<T: Config, U, V> {
 // 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,
 }

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

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

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

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

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

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

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

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

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

substrate/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>),