From 2af6f599784f4b077b0dce0df04b53e35688dd14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Wo=C5=BAniak?= Date: Wed, 18 Sep 2024 12:27:43 +0200 Subject: [PATCH] chore: Review fixes --- .../src/contract/communication/reply.rs | 161 ++++++++++-------- .../msg/overlapping_reply_handlers.rs | 6 +- .../msg/overlapping_reply_handlers.stderr | 10 +- 3 files changed, 98 insertions(+), 79 deletions(-) diff --git a/sylvia-derive/src/contract/communication/reply.rs b/sylvia-derive/src/contract/communication/reply.rs index 3f0b98f1..78dadd86 100644 --- a/sylvia-derive/src/contract/communication/reply.rs +++ b/sylvia-derive/src/contract/communication/reply.rs @@ -79,7 +79,6 @@ impl<'a> Reply<'a> { match id { #(#match_arms,)* - // TODO: Add proper result forwarding. _ => { let err_msg = format!("Unknown reply id: {}.", id); Err( #sylvia ::cw_std::StdError::generic_err(err_msg)).map_err(Into::into) @@ -114,34 +113,37 @@ impl<'a> ReplyVariants<'a> for MsgVariants<'a, GenericParam> { fn as_reply_data(&self) -> Vec { let mut reply_data: Vec = vec![]; - self - .variants() + self.variants() .flat_map(ReplyVariant::as_reply_data) .for_each(|(reply_id, function_name, reply_on)| { - match reply_data.iter_mut().find(|existing_data| existing_data.reply_id == reply_id) { + match reply_data + .iter_mut() + .find(|existing_data| existing_data.reply_id == reply_id) + { Some(existing_data) - if existing_data.handlers + if existing_data + .handlers .iter() - .any(|(_, prev_reply_on)| { - prev_reply_on.excludes(&reply_on) - }) => + .any(|(_, prev_reply_on)| prev_reply_on.excludes(&reply_on)) => { - existing_data.handlers.iter().for_each(|(prev_function_name, prev_reply_on)|{ - emit_error!(reply_id.span(), "Duplicated reply handler."; - note = prev_function_name.span() => format!("Previous definition of handler={} for reply_on={}", existing_data.reply_id.to_string(), prev_reply_on) - ) - }) - }, - Some(existing_data) => { - existing_data.handlers.push((function_name, reply_on)) - }, - None => reply_data.push(ReplyData::new (reply_id, function_name, reply_on)), - }}); + existing_data.handlers.iter().for_each( + |(prev_function_name, prev_reply_on)| { + emit_error!(reply_id.span(), "Duplicated reply handler."; + note = existing_data.reply_id.span() => format!("Previous definition of handler={} for reply_on={} defined on `fn {}()`", existing_data.reply_id.to_string(), prev_reply_on, prev_function_name); + ) + }, + ) + } + Some(existing_data) => existing_data.handlers.push((function_name, reply_on)), + None => reply_data.push(ReplyData::new(reply_id, function_name, reply_on)), + } + }); reply_data } } +/// Maps single reply id with its handlers. struct ReplyData<'a> { pub reply_id: Ident, pub handlers: Vec<(&'a Ident, ReplyOn)>, @@ -155,10 +157,10 @@ impl<'a> ReplyData<'a> { } } + /// Emits success and failure match arms for a single `ReplyId`. fn emit_match_arms(&self, contract: &Type, generics: &[&GenericParam]) -> TokenStream { let Self { reply_id, handlers } = self; - let sylvia = crate_module(); let contract_turbo: Type = if !generics.is_empty() { let contract_name = StripGenerics.fold_type((contract.clone()).clone()); parse_quote! { #contract_name :: < #(#generics),* > } @@ -166,56 +168,8 @@ impl<'a> ReplyData<'a> { parse_quote! { #contract } }; - let success_match_arm = match handlers - .iter() - .find(|(_, reply_on)| reply_on == &ReplyOn::Success || reply_on == &ReplyOn::Always) - { - Some((function_name, reply_on)) if reply_on == &ReplyOn::Success => quote! { - #sylvia ::cw_std::SubMsgResult::Ok(sub_msg_resp) => { - #[allow(deprecated)] - let #sylvia ::cw_std::SubMsgResponse { events, data, msg_responses} = sub_msg_resp; - #contract_turbo ::new(). #function_name ((deps, env, gas_used, events, msg_responses).into(), data, payload) - } - }, - Some((function_name, reply_on)) if reply_on == &ReplyOn::Always => quote! { - #sylvia ::cw_std::SubMsgResult::Ok(_) => { - #contract_turbo ::new(). #function_name ((deps, env, gas_used, vec![], vec![]).into(), result, payload) - } - }, - _ => quote! { - #sylvia ::cw_std::SubMsgResult::Ok(sub_msg_resp) => { - let mut resp = sylvia::cw_std::Response::new().add_events(sub_msg_resp.events); - - #[allow(deprecated)] - if sub_msg_resp.data.is_some() { - resp = resp.set_data(sub_msg_resp.data.unwrap()); - } - - Ok(resp) - } - }, - }; - - let failure_match_arm = match handlers - .iter() - .find(|(_, reply_on)| reply_on == &ReplyOn::Failure || reply_on == &ReplyOn::Always) - { - Some((function_name, reply_on)) if reply_on == &ReplyOn::Failure => quote! { - #sylvia ::cw_std::SubMsgResult::Err(error) => { - #contract_turbo ::new(). #function_name ((deps, env, gas_used, vec![], vec![]).into(), error, payload) - } - }, - Some((function_name, reply_on)) if reply_on == &ReplyOn::Always => quote! { - #sylvia ::cw_std::SubMsgResult::Err(_) => { - #contract_turbo ::new(). #function_name ((deps, env, gas_used, vec![], vec![]).into(), result, payload) - } - }, - _ => quote! { - #sylvia ::cw_std::SubMsgResult::Err(error) => { - Err(sylvia::cw_std::StdError::generic_err(error)).map_err(Into::into) - } - }, - }; + let success_match_arm = emit_success_match_arm(handlers, &contract_turbo); + let failure_match_arm = emit_failure_match_arm(handlers, &contract_turbo); quote! { #reply_id => { @@ -228,6 +182,71 @@ impl<'a> ReplyData<'a> { } } +/// Emits match arm for [ReplyOn::Success]. +/// In case neither [ReplyOn::Success] nor [ReplyOn::Always] is present, `Response::events` +/// and `Response::data` are forwarded in the `Response` +fn emit_success_match_arm(handlers: &[(&Ident, ReplyOn)], contract_turbo: &Type) -> TokenStream { + let sylvia = crate_module(); + + match handlers + .iter() + .find(|(_, reply_on)| reply_on == &ReplyOn::Success || reply_on == &ReplyOn::Always) + { + Some((function_name, reply_on)) if reply_on == &ReplyOn::Success => quote! { + #sylvia ::cw_std::SubMsgResult::Ok(sub_msg_resp) => { + #[allow(deprecated)] + let #sylvia ::cw_std::SubMsgResponse { events, data, msg_responses} = sub_msg_resp; + #contract_turbo ::new(). #function_name ((deps, env, gas_used, events, msg_responses).into(), data, payload) + } + }, + Some((function_name, reply_on)) if reply_on == &ReplyOn::Always => quote! { + #sylvia ::cw_std::SubMsgResult::Ok(_) => { + #contract_turbo ::new(). #function_name ((deps, env, gas_used, vec![], vec![]).into(), result, payload) + } + }, + _ => quote! { + #sylvia ::cw_std::SubMsgResult::Ok(sub_msg_resp) => { + let mut resp = sylvia::cw_std::Response::new().add_events(sub_msg_resp.events); + + #[allow(deprecated)] + if sub_msg_resp.data.is_some() { + resp = resp.set_data(sub_msg_resp.data.unwrap()); + } + + Ok(resp) + } + }, + } +} + +/// Emits match arm for [ReplyOn::Failure]. +/// In case neither [ReplyOn::Failure] nor [ReplyOn::Always] is present, +/// the error is forwarded. +fn emit_failure_match_arm(handlers: &[(&Ident, ReplyOn)], contract_turbo: &Type) -> TokenStream { + let sylvia = crate_module(); + + match handlers + .iter() + .find(|(_, reply_on)| reply_on == &ReplyOn::Failure || reply_on == &ReplyOn::Always) + { + Some((function_name, reply_on)) if reply_on == &ReplyOn::Failure => quote! { + #sylvia ::cw_std::SubMsgResult::Err(error) => { + #contract_turbo ::new(). #function_name ((deps, env, gas_used, vec![], vec![]).into(), error, payload) + } + }, + Some((function_name, reply_on)) if reply_on == &ReplyOn::Always => quote! { + #sylvia ::cw_std::SubMsgResult::Err(_) => { + #contract_turbo ::new(). #function_name ((deps, env, gas_used, vec![], vec![]).into(), result, payload) + } + }, + _ => quote! { + #sylvia ::cw_std::SubMsgResult::Err(error) => { + Err(sylvia::cw_std::StdError::generic_err(error)).map_err(Into::into) + } + }, + } +} + trait ReplyVariant<'a> { fn as_handlers(&'a self) -> Vec<&'a Ident>; fn as_reply_data(&self) -> Vec<(Ident, &Ident, ReplyOn)>; @@ -255,7 +274,7 @@ impl<'a> ReplyVariant<'a> for MsgVariant<'a> { } } -/// Maps self to a reply id +/// Maps self to an [Ident] reply id. trait AsReplyId { fn as_reply_id(&self) -> Ident; } diff --git a/sylvia/tests/ui/attributes/msg/overlapping_reply_handlers.rs b/sylvia/tests/ui/attributes/msg/overlapping_reply_handlers.rs index 9b33f59c..0a18cd84 100644 --- a/sylvia/tests/ui/attributes/msg/overlapping_reply_handlers.rs +++ b/sylvia/tests/ui/attributes/msg/overlapping_reply_handlers.rs @@ -18,12 +18,12 @@ impl Contract { Ok(Response::new()) } - #[sv::msg(reply, handlers(handler1))] + #[sv::msg(reply, handlers=[handler1])] fn reply_always(&self, _ctx: ReplyCtx, _reply: Reply) -> StdResult { Ok(Response::new()) } - #[sv::msg(reply, handlers(handler1), reply_on=success)] + #[sv::msg(reply, handlers=[handler1], reply_on=success)] fn duplicated_success_for_reply_always( &self, _ctx: ReplyCtx, @@ -32,7 +32,7 @@ impl Contract { Ok(Response::new()) } - #[sv::msg(reply, handlers(handler2), reply_on=failure)] + #[sv::msg(reply, handlers=[handler2], reply_on=failure)] fn some_reply(&self, _ctx: ReplyCtx, _reply: Reply) -> StdResult { Ok(Response::new()) } diff --git a/sylvia/tests/ui/attributes/msg/overlapping_reply_handlers.stderr b/sylvia/tests/ui/attributes/msg/overlapping_reply_handlers.stderr index e02e84bd..80c211b6 100644 --- a/sylvia/tests/ui/attributes/msg/overlapping_reply_handlers.stderr +++ b/sylvia/tests/ui/attributes/msg/overlapping_reply_handlers.stderr @@ -1,15 +1,15 @@ error: Duplicated reply handler. - = note: Previous definition of handler=HANDLER_1_REPLY_ID for reply_on=always + = note: Previous definition of handler=HANDLER_1_REPLY_ID for reply_on=always defined on `fn reply_always()` - --> tests/ui/attributes/msg/overlapping_reply_handlers.rs:26:31 + --> tests/ui/attributes/msg/overlapping_reply_handlers.rs:26:32 | -26 | #[sv::msg(reply, handlers(handler1), reply_on=success)] - | ^^^^^^^^ +26 | #[sv::msg(reply, handlers=[handler1], reply_on=success)] + | ^^^^^^^^ error: Duplicated reply handler. - = note: Previous definition of handler=HANDLER_2_REPLY_ID for reply_on=failure + = note: Previous definition of handler=HANDLER_2_REPLY_ID for reply_on=failure defined on `fn some_reply()` --> tests/ui/attributes/msg/overlapping_reply_handlers.rs:41:8 |