Allow pallet::call to return DispatchResult (#8241)

* allow dispatch result

* remove custom error message

* format

* add forgotten UI test

* fix test

* fix tests

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

@@ -21,7 +21,6 @@ use syn::spanned::Spanned;
 
 /// List of additional token to be used for parsing.
 mod keyword {
-	syn::custom_keyword!(DispatchResultWithPostInfo);
 	syn::custom_keyword!(Call);
 	syn::custom_keyword!(OriginFor);
 	syn::custom_keyword!(weight);
@@ -163,7 +162,7 @@ impl CallDef {
 				}
 
 				if let syn::ReturnType::Type(_, type_) = &method.sig.output {
-					syn::parse2::<keyword::DispatchResultWithPostInfo>(type_.to_token_stream())?;
+					helper::check_pallet_call_return_type(type_)?;
 				} else {
 					let msg = "Invalid pallet::call, require return type \
 						DispatchResultWithPostInfo";

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

@@ -27,6 +27,8 @@ mod keyword {
 	syn::custom_keyword!(T);
 	syn::custom_keyword!(Pallet);
 	syn::custom_keyword!(origin);
+	syn::custom_keyword!(DispatchResult);
+	syn::custom_keyword!(DispatchResultWithPostInfo);
 }
 
 /// A usage of instance, either the trait `Config` has been used with instance or without instance.
@@ -596,3 +598,26 @@ pub fn check_type_value_gen(
 
 	Ok(i)
 }
+
+/// Check the keyword `DispatchResultWithPostInfo` or `DispatchResult`.
+pub fn check_pallet_call_return_type(
+	type_: &syn::Type,
+) -> syn::Result<()> {
+	pub struct Checker;
+	impl syn::parse::Parse for Checker {
+		fn parse(input: syn::parse::ParseStream) -> syn::Result<Self> {
+			let lookahead = input.lookahead1();
+			if lookahead.peek(keyword::DispatchResultWithPostInfo) {
+				input.parse::<keyword::DispatchResultWithPostInfo>()?;
+				Ok(Self)
+			} else if lookahead.peek(keyword::DispatchResult) {
+				input.parse::<keyword::DispatchResult>()?;
+				Ok(Self)
+			} else {
+				Err(lookahead.error())
+			}
+		}
+	}
+
+	syn::parse2::<Checker>(type_.to_token_stream()).map(|_| ())
+}

substrate/frame/support/src/lib.rs

@@ -1078,7 +1078,7 @@ pub mod pallet_prelude {
 		Twox128, Blake2_256, Blake2_128, Identity, Twox64Concat, Blake2_128Concat, ensure,
 		RuntimeDebug, storage,
 		traits::{Get, Hooks, IsType, GetPalletVersion, EnsureOrigin},
-		dispatch::{DispatchResultWithPostInfo, Parameter, DispatchError},
+		dispatch::{DispatchResultWithPostInfo, Parameter, DispatchError, DispatchResult},
 		weights::{DispatchClass, Pays, Weight},
 		storage::types::{StorageValue, StorageMap, StorageDoubleMap, ValueQuery, OptionQuery},
 	};
@@ -1244,7 +1244,7 @@ pub mod pallet_prelude {
 /// 		$some_arg: $some_type,
 /// 		// or with compact attribute: #[pallet::compact] $some_arg: $some_type,
 /// 		...
-/// 	) -> DispatchResultWithPostInfo {
+/// 	) -> DispatchResultWithPostInfo { // or `-> DispatchResult`
 /// 		...
 /// 	}
 /// 	...
@@ -1255,7 +1255,8 @@ pub mod pallet_prelude {
 ///
 /// Each dispatchable needs to define a weight with `#[pallet::weight($expr)]` attribute,
 /// the first argument must be `origin: OriginFor<T>`, compact encoding for argument can be used
-/// using `#[pallet::compact]`, function must return DispatchResultWithPostInfo.
+/// using `#[pallet::compact]`, function must return `DispatchResultWithPostInfo` or
+/// `DispatchResult`.
 ///
 /// All arguments must implement `Debug`, `PartialEq`, `Eq`, `Decode`, `Encode`, `Clone`. For ease
 /// of use, bound the trait `Member` available in frame_support::pallet_prelude.

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

@@ -161,6 +161,14 @@ pub mod pallet {
 
 			Ok(().into())
 		}
+
+		// Test for DispatchResult return type
+		#[pallet::weight(1)]
+		fn foo_no_post_info(
+			_origin: OriginFor<T>,
+		) -> DispatchResult {
+			Ok(())
+		}
 	}
 
 	#[pallet::error]
@@ -425,7 +433,7 @@ fn call_expand() {
 	assert_eq!(call_foo.get_call_name(), "foo");
 	assert_eq!(
 		pallet::Call::<Runtime>::get_call_names(),
-		&["foo", "foo_transactional"],
+		&["foo", "foo_transactional", "foo_no_post_info"],
 	);
 }
 
@@ -669,6 +677,11 @@ fn metadata() {
 					" Doc comment put in metadata".to_string(),
 				]),
 			},
+			FunctionMetadata {
+				name: DecodeDifferent::Decoded("foo_no_post_info".to_string()),
+				arguments: DecodeDifferent::Decoded(vec![]),
+				documentation: DecodeDifferent::Decoded(vec![]),
+			},
 		])),
 		event: Some(DecodeDifferent::Decoded(vec![
 			EventMetadata {

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

@@ -0,0 +1,22 @@
+#[frame_support::pallet]
+mod pallet {
+	use frame_support::pallet_prelude::Hooks;
+	use frame_system::pallet_prelude::{BlockNumberFor, OriginFor};
+
+	#[pallet::config]
+	pub trait Config: frame_system::Config {}
+
+	#[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 foo(origin: OriginFor<T>) -> ::DispatchResult { todo!() }
+	}
+}
+
+fn main() {
+}

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

@@ -0,0 +1,5 @@
+error: expected `DispatchResultWithPostInfo` or `DispatchResult`
+  --> $DIR/call_invalid_return.rs:17:35
+   |
+17 |         fn foo(origin: OriginFor<T>) -> ::DispatchResult { todo!() }
+   |                                         ^^