From 50f338e1ea6038f2f5cd1699e9aeae6e799b31dc Mon Sep 17 00:00:00 2001
From: benluelo <57334811+benluelo@users.noreply.github.com>
Date: Fri, 25 Nov 2022 05:11:19 -0500
Subject: [PATCH] update DefaultNoBound derive macro (#12723)
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 <git@kchr.de>
rename variable
formatting & clippy
formatting
Co-authored-by: parity-processbot <>
---
.../procedural/src/default_no_bound.rs | 174 ++++++++++++------
substrate/frame/support/procedural/src/lib.rs | 2 +-
substrate/frame/support/src/lib.rs | 13 +-
.../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 substrate/frame/support/test/tests/derive_no_bound_ui/default_empty_enum.rs
create mode 100644 substrate/frame/support/test/tests/derive_no_bound_ui/default_empty_enum.stderr
create mode 100644 substrate/frame/support/test/tests/derive_no_bound_ui/default_no_attribute.rs
create mode 100644 substrate/frame/support/test/tests/derive_no_bound_ui/default_no_attribute.stderr
create mode 100644 substrate/frame/support/test/tests/derive_no_bound_ui/default_too_many_attributes.rs
create mode 100644 substrate/frame/support/test/tests/derive_no_bound_ui/default_too_many_attributes.stderr
create mode 100644 substrate/frame/support/test/tests/derive_no_bound_ui/default_union.rs
create mode 100644 substrate/frame/support/test/tests/derive_no_bound_ui/default_union.stderr
diff --git a/substrate/frame/support/procedural/src/default_no_bound.rs b/substrate/frame/support/procedural/src/default_no_bound.rs
index 192be0786d9..b174a49e917 100644
--- a/substrate/frame/support/procedural/src/default_no_bound.rs
+++ b/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()
- )
- });
-
- 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::<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 {
diff --git a/substrate/frame/support/procedural/src/lib.rs b/substrate/frame/support/procedural/src/lib.rs
index cd30946ae78..41dbc4ee959 100644
--- a/substrate/frame/support/procedural/src/lib.rs
+++ b/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)
}
diff --git a/substrate/frame/support/src/lib.rs b/substrate/frame/support/src/lib.rs
index 84e416e5054..efecbb75f9c 100644
--- a/substrate/frame/support/src/lib.rs
+++ b/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;
diff --git a/substrate/frame/support/test/tests/derive_no_bound.rs b/substrate/frame/support/test/tests/derive_no_bound.rs
index e1cf539f2ac..f891b3a2d2d 100644
--- a/substrate/frame/support/test/tests/derive_no_bound.rs
+++ b/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,
}
diff --git a/substrate/frame/support/test/tests/derive_no_bound_ui/default_empty_enum.rs b/substrate/frame/support/test/tests/derive_no_bound_ui/default_empty_enum.rs
new file mode 100644
index 00000000000..51b6137c007
--- /dev/null
+++ b/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() {}
diff --git a/substrate/frame/support/test/tests/derive_no_bound_ui/default_empty_enum.stderr b/substrate/frame/support/test/tests/derive_no_bound_ui/default_empty_enum.stderr
new file mode 100644
index 00000000000..9c93b515adc
--- /dev/null
+++ b/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 {}
+ | ^^^^^
diff --git a/substrate/frame/support/test/tests/derive_no_bound_ui/default_no_attribute.rs b/substrate/frame/support/test/tests/derive_no_bound_ui/default_no_attribute.rs
new file mode 100644
index 00000000000..185df01fe2b
--- /dev/null
+++ b/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() {}
diff --git a/substrate/frame/support/test/tests/derive_no_bound_ui/default_no_attribute.stderr b/substrate/frame/support/test/tests/derive_no_bound_ui/default_no_attribute.stderr
new file mode 100644
index 00000000000..12e00236715
--- /dev/null
+++ b/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> {
+ | ^^^
diff --git a/substrate/frame/support/test/tests/derive_no_bound_ui/default_too_many_attributes.rs b/substrate/frame/support/test/tests/derive_no_bound_ui/default_too_many_attributes.rs
new file mode 100644
index 00000000000..c3d175da6c0
--- /dev/null
+++ b/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() {}
diff --git a/substrate/frame/support/test/tests/derive_no_bound_ui/default_too_many_attributes.stderr b/substrate/frame/support/test/tests/derive_no_bound_ui/default_too_many_attributes.stderr
new file mode 100644
index 00000000000..5430ef142c5
--- /dev/null
+++ b/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,
+ | |_______^
diff --git a/substrate/frame/support/test/tests/derive_no_bound_ui/default_union.rs b/substrate/frame/support/test/tests/derive_no_bound_ui/default_union.rs
new file mode 100644
index 00000000000..5822cda1aa6
--- /dev/null
+++ b/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() {}
diff --git a/substrate/frame/support/test/tests/derive_no_bound_ui/default_union.stderr b/substrate/frame/support/test/tests/derive_no_bound_ui/default_union.stderr
new file mode 100644
index 00000000000..1e01e1baaf8
--- /dev/null
+++ b/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 {
+ | ^^^^^
diff --git a/substrate/frame/transaction-payment/asset-tx-payment/src/lib.rs b/substrate/frame/transaction-payment/asset-tx-payment/src/lib.rs
index e136da8b0bb..43cc1efa085 100644
--- a/substrate/frame/transaction-payment/asset-tx-payment/src/lib.rs
+++ b/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>),
--
GitLab