Skip to content

Commit

Permalink
Fixes error description and span for closure args
Browse files Browse the repository at this point in the history
  • Loading branch information
gupnik committed Nov 11, 2023
1 parent d79cc22 commit 0ab5492
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 41 deletions.
70 changes: 56 additions & 14 deletions substrate/frame/support/procedural/src/pallet/parse/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ impl syn::parse::Parse for FunctionAttr {
let mut err = syn::Error::new(closure_content.span(), msg);
err.combine(e);
err
})?
})?,
))
} else {
Err(lookahead.error())
Expand All @@ -158,28 +158,33 @@ impl syn::parse::Parse for ArgAttrIsCompact {
}
}

/// Check the syntax is `OriginFor<T>`
pub fn check_dispatchable_first_arg_type(ty: &syn::Type) -> syn::Result<()> {
pub struct CheckDispatchableFirstArg;
/// Check the syntax is `OriginFor<T>` or `&OriginFor<T>`.
pub fn check_dispatchable_first_arg_type(ty: &syn::Type, is_ref: bool) -> syn::Result<()> {
pub struct CheckDispatchableFirstArg(bool);
impl syn::parse::Parse for CheckDispatchableFirstArg {
fn parse(input: syn::parse::ParseStream) -> syn::Result<Self> {
let is_ref = input.parse::<syn::Token![&]>().is_ok();
input.parse::<keyword::OriginFor>()?;
input.parse::<syn::Token![<]>()?;
input.parse::<keyword::T>()?;
input.parse::<syn::Token![>]>()?;

Ok(Self)
Ok(Self(is_ref))
}
}

syn::parse2::<CheckDispatchableFirstArg>(ty.to_token_stream()).map_err(|e| {
let msg = "Invalid type: expected `OriginFor<T>`";
let mut err = syn::Error::new(ty.span(), msg);
err.combine(e);
err
})?;

Ok(())
let result = syn::parse2::<CheckDispatchableFirstArg>(ty.to_token_stream());
return match result {
Ok(CheckDispatchableFirstArg(has_ref)) if is_ref == has_ref => Ok(()),
_ => {
let msg = if is_ref {
"Invalid type: expected `&OriginFor<T>`"
} else {
"Invalid type: expected `OriginFor<T>`"
};
return Err(syn::Error::new(ty.span(), msg))
},
}
}

impl CallDef {
Expand Down Expand Up @@ -235,7 +240,7 @@ impl CallDef {
return Err(syn::Error::new(method.sig.span(), msg))
},
Some(syn::FnArg::Typed(arg)) => {
check_dispatchable_first_arg_type(&arg.ty)?;
check_dispatchable_first_arg_type(&arg.ty, false)?;
},
}

Expand Down Expand Up @@ -366,6 +371,43 @@ impl CallDef {
return Err(syn::Error::new(feeless_check.span(), msg))
}

match feeless_check.inputs.first() {
None => {
let msg = "Invalid pallet::call, feeless_if closure must have at least origin arg";
return Err(syn::Error::new(feeless_check.span(), msg))
},
Some(syn::Pat::Type(arg)) => {
check_dispatchable_first_arg_type(&arg.ty, true)?;
},
_ => {
let msg = "Invalid pallet::call, feeless_if closure first argument must be a typed argument, \
e.g. `origin: OriginFor<T>`";
return Err(syn::Error::new(feeless_check.span(), msg))
},
}

for (feeless_arg, arg) in feeless_check.inputs.iter().skip(1).zip(args.iter()) {
let feeless_arg_type =
if let syn::Pat::Type(syn::PatType { ty, .. }) = feeless_arg.clone() {
if let syn::Type::Reference(pat) = *ty {
pat.elem.clone()
} else {
let msg = "Invalid pallet::call, feeless_if closure argument must be a reference";
return Err(syn::Error::new(ty.span(), msg))
}
} else {
let msg = "Invalid pallet::call, feeless_if closure argument must be a type ascription pattern";
return Err(syn::Error::new(feeless_arg.span(), msg))
};

if feeless_arg_type != arg.2 {
let msg =
"Invalid pallet::call, feeless_if closure argument must have \
a reference to the same type as the dispatchable function argument";
return Err(syn::Error::new(feeless_arg.span(), msg))
}
}

let valid_return = match &feeless_check.output {
syn::ReturnType::Type(_, type_) => match *(type_.clone()) {
syn::Type::Path(syn::TypePath { path, .. }) => path.is_ident("bool"),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,5 @@
error[E0308]: mismatched types
--> tests/pallet_ui/call_feeless_invalid_closure_arg2.rs:18:1
error: Invalid type: expected `&OriginFor<T>`
--> tests/pallet_ui/call_feeless_invalid_closure_arg2.rs:31:28
|
18 | #[frame_support::pallet(dev_mode)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `bool`, found `&<T as Config>::RuntimeOrigin`
...
31 | #[pallet::feeless_if(|_: bool| -> bool { true })]
| -------------------------- arguments to this function are incorrect
|
= note: expected type `bool`
found reference `&<T as frame_system::Config>::RuntimeOrigin`
note: closure parameter defined here
--> tests/pallet_ui/call_feeless_invalid_closure_arg2.rs:31:25
|
31 | #[pallet::feeless_if(|_: bool| -> bool { true })]
| ^^^^^^^
= note: this error originates in the attribute macro `frame_support::pallet` (in Nightly builds, run with -Z macro-backtrace for more info)
| ^^^^
Original file line number Diff line number Diff line change
@@ -1,15 +1,5 @@
error[E0308]: mismatched types
--> tests/pallet_ui/call_feeless_invalid_closure_arg3.rs:32:31
|
31 | #[pallet::feeless_if(|_: &OriginFor<T>, _s: &u32| -> bool { true })]
| --------------------------------------------- arguments to this function are incorrect
32 | pub fn foo(_: OriginFor<T>, _something: u64) -> DispatchResult { Ok(()) }
| ^^^^^^^^^^ expected `&u32`, found `&u64`
|
= note: expected reference `&u32`
found reference `&u64`
note: closure parameter defined here
error: Invalid pallet::call, feeless_if closure argument must have a reference to the same type as the dispatchable function argument
--> tests/pallet_ui/call_feeless_invalid_closure_arg3.rs:31:43
|
31 | #[pallet::feeless_if(|_: &OriginFor<T>, _s: &u32| -> bool { true })]
| ^^^^^^^^
| ^^

0 comments on commit 0ab5492

Please sign in to comment.