Allow additional trait bounds for #[pallet::constant] (#9050)

* Allow additional trait bounds for constants

* Add ui test for constants with additional trait bounds

* Update trait constant ui test

* Import syn::Error

* Use reference instead of cloning

* Add extra invalid bound ui test

* Out or order valid bounds

* Fix ui test

* Fix ui test

* Apply review suggestion about error message

substrate/frame/support/procedural/src/pallet/parse/config.rs

@@ -16,6 +16,7 @@
 // limitations under the License.
 
 use super::helper;
+use core::convert::TryFrom;
 use syn::spanned::Spanned;
 use quote::ToTokens;
 
@@ -25,7 +26,6 @@ mod keyword {
 	syn::custom_keyword!(From);
 	syn::custom_keyword!(T);
 	syn::custom_keyword!(I);
-	syn::custom_keyword!(Get);
 	syn::custom_keyword!(config);
 	syn::custom_keyword!(IsType);
 	syn::custom_keyword!(Event);
@@ -62,19 +62,41 @@ pub struct ConstMetadataDef {
 	pub doc: Vec<syn::Lit>,
 }
 
-impl syn::parse::Parse for ConstMetadataDef  {
-	fn parse(input: syn::parse::ParseStream) -> syn::Result<Self> {
-		let doc = helper::get_doc_literals(&syn::Attribute::parse_outer(input)?);
-		input.parse::<syn::Token![type]>()?;
-		let ident = input.parse::<syn::Ident>()?;
-		input.parse::<syn::Token![:]>()?;
-		input.parse::<keyword::Get>()?;
-		input.parse::<syn::Token![<]>()?;
-		let mut type_ = input.parse::<syn::Type>()?;
-		type_ = syn::parse2::<syn::Type>(replace_self_by_t(type_.to_token_stream()))
+impl TryFrom<&syn::TraitItemType> for ConstMetadataDef {
+	type Error = syn::Error;
+
+	fn try_from(trait_ty: &syn::TraitItemType) -> Result<Self, Self::Error> {
+		let err = |span, msg|
+			syn::Error::new(span, format!("Invalid usage of `#[pallet::constant]`: {}", msg));
+		let doc = helper::get_doc_literals(&trait_ty.attrs);
+		let ident = trait_ty.ident.clone();
+		let bound = trait_ty.bounds
+			.iter()
+			.find_map(|b|
+				if let syn::TypeParamBound::Trait(tb) = b {
+					tb.path.segments
+						.last()
+						.and_then(|s| if s.ident == "Get" { Some(s) } else { None } )
+				} else {
+					None
+				}
+			)
+			.ok_or_else(|| err(trait_ty.span(), "`Get<T>` trait bound not found"))?;
+		let type_arg = if let syn::PathArguments::AngleBracketed (ref ab) = bound.arguments {
+			if ab.args.len() == 1 {
+				if let syn::GenericArgument::Type(ref ty) = ab.args[0] {
+					Ok(ty)
+				} else {
+					Err(err(ab.args[0].span(), "Expected a type argument"))
+				}
+			} else {
+				Err(err(bound.span(), "Expected a single type argument"))
+			}
+		} else {
+			Err(err(bound.span(), "Expected trait generic args"))
+		}?;
+		let type_ = syn::parse2::<syn::Type>(replace_self_by_t(type_arg.to_token_stream()))
 			.expect("Internal error: replacing `Self` by `T` should result in valid type");
-		input.parse::<syn::Token![>]>()?;
-		input.parse::<syn::Token![;]>()?;
 
 		Ok(Self { ident, type_, doc })
 	}
@@ -322,16 +344,8 @@ impl ConfigDef {
 
 			if type_attrs_const.len() == 1 {
 				match trait_item {
-					syn::TraitItem::Type(type_) => {
-						let constant = syn::parse2::<ConstMetadataDef>(type_.to_token_stream())
-							.map_err(|e| {
-								let error_msg = "Invalid usage of `#[pallet::constant]`, syntax \
-									must be `type $SomeIdent: Get<$SomeType>;`";
-								let mut err = syn::Error::new(type_.span(), error_msg);
-								err.combine(e);
-								err
-							})?;
-
+					syn::TraitItem::Type(ref type_) => {
+						let constant = ConstMetadataDef::try_from(type_)?;
 						consts_metadata.push(constant);
 					},
 					_ => {

substrate/frame/support/test/tests/pallet_ui.rs

@@ -23,4 +23,5 @@ fn pallet_ui() {
 
 	let t = trybuild::TestCases::new();
 	t.compile_fail("tests/pallet_ui/*.rs");
+	t.pass("tests/pallet_ui/pass/*.rs");
 }

substrate/frame/support/test/tests/pallet_ui/pass/trait_constant_valid_bounds.rs

@@ -0,0 +1,29 @@
+#[frame_support::pallet]
+mod pallet {
+	use frame_support::pallet_prelude::*;
+	use frame_system::pallet_prelude::BlockNumberFor;
+
+	#[pallet::config]
+	pub trait Config: frame_system::Config {
+		#[pallet::constant]
+		type U: Get<u32>;
+
+		#[pallet::constant]
+		type V: Get<u32> + From<u16>;
+
+		#[pallet::constant]
+		type W: From<u16> + Get<u32>;
+	}
+
+	#[pallet::pallet]
+	pub struct Pallet<T>(core::marker::PhantomData<T>);
+
+	#[pallet::hooks]
+	impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {}
+
+	#[pallet::call]
+	impl<T: Config> Pallet<T> {}
+}
+
+fn main() {
+}

substrate/frame/support/test/tests/pallet_ui/trait_constant_invalid_bound.stderr

@@ -1,11 +1,5 @@
-error: Invalid usage of `#[pallet::constant]`, syntax must be `type $SomeIdent: Get<$SomeType>;`
+error: Invalid usage of `#[pallet::constant]`: `Get<T>` trait bound not found
  --> $DIR/trait_constant_invalid_bound.rs:9:3
   |
 9 |         type U;
   |         ^^^^
-
-error: expected `:`
- --> $DIR/trait_constant_invalid_bound.rs:9:9
-  |
-9 |         type U;
-  |               ^

substrate/frame/support/test/tests/pallet_ui/trait_constant_invalid_bound_lifetime.rs

@@ -0,0 +1,23 @@
+#[frame_support::pallet]
+mod pallet {
+	use frame_support::pallet_prelude::Hooks;
+	use frame_system::pallet_prelude::BlockNumberFor;
+
+	#[pallet::config]
+	pub trait Config: frame_system::Config {
+		#[pallet::constant]
+		type U: Get<'static>;
+	}
+
+	#[pallet::pallet]
+	pub struct Pallet<T>(core::marker::PhantomData<T>);
+
+	#[pallet::hooks]
+	impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {}
+
+	#[pallet::call]
+	impl<T: Config> Pallet<T> {}
+}
+
+fn main() {
+}

substrate/frame/support/test/tests/pallet_ui/trait_constant_invalid_bound_lifetime.stderr

@@ -0,0 +1,5 @@
+error: Invalid usage of `#[pallet::constant]`: Expected a type argument
+ --> $DIR/trait_constant_invalid_bound_lifetime.rs:9:15
+  |
+9 |         type U: Get<'static>;
+  |                     ^^^^^^^