From 5d6a7c8ea0c4c074423ab0ca483d7358e5d47fc2 Mon Sep 17 00:00:00 2001 From: Daniyar Itegulov Date: Thu, 3 Mar 2022 10:06:31 +1100 Subject: [PATCH 1/6] add an optional way to handle contract errors with Result --- .../core_impl/code_generator/attr_sig_info.rs | 36 +--------------- .../code_generator/impl_item_method_info.rs | 36 +++++++++++++--- .../core_impl/info_extractor/attr_sig_info.rs | 7 ++++ near-sdk-macros/src/core_impl/mod.rs | 1 + near-sdk-macros/src/core_impl/utils/mod.rs | 41 +++++++++++++++++++ near-sdk-macros/src/lib.rs | 27 ++++++++++++ near-sdk/src/lib.rs | 2 +- near-sdk/src/types/error.rs | 36 ++++++++++++++++ near-sdk/src/types/mod.rs | 3 ++ 9 files changed, 149 insertions(+), 40 deletions(-) create mode 100644 near-sdk-macros/src/core_impl/utils/mod.rs create mode 100644 near-sdk/src/types/error.rs diff --git a/near-sdk-macros/src/core_impl/code_generator/attr_sig_info.rs b/near-sdk-macros/src/core_impl/code_generator/attr_sig_info.rs index 39a56277f..5c99cc4cf 100644 --- a/near-sdk-macros/src/core_impl/code_generator/attr_sig_info.rs +++ b/near-sdk-macros/src/core_impl/code_generator/attr_sig_info.rs @@ -3,8 +3,8 @@ use proc_macro2::TokenStream as TokenStream2; use crate::core_impl::info_extractor::{ ArgInfo, AttrSigInfo, BindgenArgType, InputStructType, SerializerType, }; +use crate::core_impl::utils; use quote::quote; -use syn::{GenericArgument, Path, PathArguments, Type}; impl AttrSigInfo { /// Create struct representing input arguments. @@ -192,7 +192,7 @@ impl AttrSigInfo { } } BindgenArgType::CallbackResultArg => { - let ok_type = if let Some(ok_type) = extract_ok_type(ty) { + let ok_type = if let Some(ok_type) = utils::extract_ok_type(ty) { ok_type } else { return syn::Error::new_spanned(ty, "Function parameters marked with \ @@ -262,38 +262,6 @@ impl AttrSigInfo { } } -/// Checks whether the given path is literally "Result". -/// Note that it won't match a fully qualified name `core::result::Result` or a type alias like -/// `type StringResult = Result`. -fn path_is_result(path: &Path) -> bool { - path.leading_colon.is_none() - && path.segments.len() == 1 - && path.segments.iter().next().unwrap().ident == "Result" -} - -/// Extracts the Ok type from a `Result` type. -/// -/// For example, given `Result` type it will return `String` type. -fn extract_ok_type(ty: &Type) -> Option<&Type> { - match ty { - Type::Path(type_path) if type_path.qself.is_none() && path_is_result(&type_path.path) => { - // Get the first segment of the path (there should be only one, in fact: "Result"): - let type_params = &type_path.path.segments.first()?.arguments; - // We are interested in the first angle-bracketed param responsible for Ok type (""): - let generic_arg = match type_params { - PathArguments::AngleBracketed(params) => Some(params.args.first()?), - _ => None, - }?; - // This argument must be a type: - match generic_arg { - GenericArgument::Type(ty) => Some(ty), - _ => None, - } - } - _ => None, - } -} - pub fn deserialize_data(ty: &SerializerType) -> TokenStream2 { match ty { SerializerType::JSON => quote! { diff --git a/near-sdk-macros/src/core_impl/code_generator/impl_item_method_info.rs b/near-sdk-macros/src/core_impl/code_generator/impl_item_method_info.rs index 7fbab28b4..e31a33de0 100644 --- a/near-sdk-macros/src/core_impl/code_generator/impl_item_method_info.rs +++ b/near-sdk-macros/src/core_impl/code_generator/impl_item_method_info.rs @@ -1,6 +1,7 @@ use crate::core_impl::info_extractor::{ AttrSigInfo, ImplItemMethodInfo, InputStructType, MethodType, SerializerType, }; +use crate::core_impl::utils; use proc_macro2::TokenStream as TokenStream2; use quote::quote; use syn::{ReturnType, Signature}; @@ -53,6 +54,7 @@ impl ImplItemMethodInfo { method_type, is_payable, is_private, + is_returns_result, .. } = attr_signature_info; let deposit_check = if *is_payable || matches!(method_type, &MethodType::View) { @@ -136,6 +138,30 @@ impl ImplItemMethodInfo { #method_invocation; #contract_ser }, + ReturnType::Type(_, return_type) + if utils::type_is_result(&return_type) && *is_returns_result => + { + let value_ser = match result_serializer { + SerializerType::JSON => quote! { + let result = near_sdk::serde_json::to_vec(&result).expect("Failed to serialize the return value using JSON."); + }, + SerializerType::Borsh => quote! { + let result = near_sdk::borsh::BorshSerialize::try_to_vec(&result).expect("Failed to serialize the return value using Borsh."); + }, + }; + quote! { + #contract_deser + let result = #method_invocation; + match result { + Ok(result) => { + #value_ser + near_sdk::env::value_return(&result); + #contract_ser + } + Err(err) => near_sdk::FunctionError::panic(&err) + } + } + } ReturnType::Type(_, _) => { let value_ser = match result_serializer { SerializerType::JSON => quote! { @@ -146,11 +172,11 @@ impl ImplItemMethodInfo { }, }; quote! { - #contract_deser - let result = #method_invocation; - #value_ser - near_sdk::env::value_return(&result); - #contract_ser + #contract_deser + let result = #method_invocation; + #value_ser + near_sdk::env::value_return(&result); + #contract_ser } } } diff --git a/near-sdk-macros/src/core_impl/info_extractor/attr_sig_info.rs b/near-sdk-macros/src/core_impl/info_extractor/attr_sig_info.rs index e670eefd6..cb71da366 100644 --- a/near-sdk-macros/src/core_impl/info_extractor/attr_sig_info.rs +++ b/near-sdk-macros/src/core_impl/info_extractor/attr_sig_info.rs @@ -18,6 +18,8 @@ pub struct AttrSigInfo { pub is_payable: bool, /// Whether method can accept calls from self (current account) pub is_private: bool, + /// Whether method returns Result type where only Ok type is serialized + pub is_returns_result: bool, /// The serializer that we use for `env::input()`. pub input_serializer: SerializerType, /// The serializer that we use for the return type. @@ -61,6 +63,7 @@ impl AttrSigInfo { let mut method_type = MethodType::Regular; let mut is_payable = false; let mut is_private = false; + let mut is_returns_result = false; // By the default we serialize the result with JSON. let mut result_serializer = SerializerType::JSON; @@ -87,6 +90,9 @@ impl AttrSigInfo { let serializer: SerializerAttr = syn::parse2(attr.tokens.clone())?; result_serializer = serializer.serializer_type; } + "return_result" => { + is_returns_result = true; + } _ => { non_bindgen_attrs.push((*attr).clone()); } @@ -136,6 +142,7 @@ impl AttrSigInfo { method_type, is_payable, is_private, + is_returns_result, result_serializer, receiver, returns, diff --git a/near-sdk-macros/src/core_impl/mod.rs b/near-sdk-macros/src/core_impl/mod.rs index 7a0418f6b..ff6047706 100644 --- a/near-sdk-macros/src/core_impl/mod.rs +++ b/near-sdk-macros/src/core_impl/mod.rs @@ -1,6 +1,7 @@ mod code_generator; mod info_extractor; mod metadata; +mod utils; pub use code_generator::*; pub use info_extractor::*; pub use metadata::metadata_visitor::MetadataVisitor; diff --git a/near-sdk-macros/src/core_impl/utils/mod.rs b/near-sdk-macros/src/core_impl/utils/mod.rs new file mode 100644 index 000000000..804cb6b7e --- /dev/null +++ b/near-sdk-macros/src/core_impl/utils/mod.rs @@ -0,0 +1,41 @@ +use syn::{Type, Path, PathArguments, GenericArgument}; + +/// Checks whether the given path is literally "Result". +/// Note that it won't match a fully qualified name `core::result::Result` or a type alias like +/// `type StringResult = Result`. +pub(crate) fn path_is_result(path: &Path) -> bool { + path.leading_colon.is_none() + && path.segments.len() == 1 + && path.segments.iter().next().unwrap().ident == "Result" +} + +/// Equivalent to `path_is_result` except that it works on `Type` values. +pub(crate) fn type_is_result(ty: &Type) -> bool { + match ty { + Type::Path(type_path) if type_path.qself.is_none() => path_is_result(&type_path.path), + _ => false, + } +} + +/// Extracts the Ok type from a `Result` type. +/// +/// For example, given `Result` type it will return `String` type. +pub(crate) fn extract_ok_type(ty: &Type) -> Option<&Type> { + match ty { + Type::Path(type_path) if type_path.qself.is_none() && path_is_result(&type_path.path) => { + // Get the first segment of the path (there should be only one, in fact: "Result"): + let type_params = &type_path.path.segments.first()?.arguments; + // We are interested in the first angle-bracketed param responsible for Ok type (""): + let generic_arg = match type_params { + PathArguments::AngleBracketed(params) => Some(params.args.first()?), + _ => None, + }?; + // This argument must be a type: + match generic_arg { + GenericArgument::Type(ty) => Some(ty), + _ => None, + } + } + _ => None, + } +} diff --git a/near-sdk-macros/src/lib.rs b/near-sdk-macros/src/lib.rs index bac9ce127..b95ab0ba7 100644 --- a/near-sdk-macros/src/lib.rs +++ b/near-sdk-macros/src/lib.rs @@ -184,3 +184,30 @@ pub fn borsh_storage_key(item: TokenStream) -> TokenStream { impl near_sdk::BorshIntoStorageKey for #name {} }) } + +/// `FunctionError` generates implementation for `near_sdk::FunctionError` trait. +/// It allows contract runtime to panic with the type using its `ToString` implementation +/// as the message. +#[proc_macro_derive(FunctionError)] +pub fn function_error(item: TokenStream) -> TokenStream { + let name = if let Ok(input) = syn::parse::(item.clone()) { + input.ident + } else if let Ok(input) = syn::parse::(item) { + input.ident + } else { + return TokenStream::from( + syn::Error::new( + Span::call_site(), + "FunctionError can only be used as a derive on enums or structs.", + ) + .to_compile_error(), + ); + }; + TokenStream::from(quote! { + impl near_sdk::FunctionError for #name { + fn panic(&self) { + near_sdk::env::panic_str(&std::string::ToString::to_string(&self)) + } + } + }) +} diff --git a/near-sdk/src/lib.rs b/near-sdk/src/lib.rs index bda4697e0..2055a1c88 100644 --- a/near-sdk/src/lib.rs +++ b/near-sdk/src/lib.rs @@ -7,7 +7,7 @@ extern crate quickcheck; pub use near_sdk_macros::{ callback, callback_vec, ext_contract, init, metadata, near_bindgen, result_serializer, - serializer, BorshStorageKey, PanicOnDefault, + serializer, BorshStorageKey, PanicOnDefault, FunctionError }; #[cfg(feature = "unstable")] diff --git a/near-sdk/src/types/error.rs b/near-sdk/src/types/error.rs new file mode 100644 index 000000000..c83bb967d --- /dev/null +++ b/near-sdk/src/types/error.rs @@ -0,0 +1,36 @@ +use std::borrow::Borrow; + +/// Enables contract runtime to panic with the given type. Any error type used in conjunction +/// with `#[return_result]` has to implement this trait. +/// +/// ``` +/// use near_sdk::FunctionError; +/// +/// enum Error { +/// NotFound, +/// Unexpected { message: String }, +/// } +/// +/// impl FunctionError for Error { +/// fn panic(&self) { +/// match self { +/// Error::NotFound => +/// near_sdk::env::panic_str("not found"), +/// Error::Unexpected { message } => +/// near_sdk::env::panic_str(&format!("unexpected error: {}", message)) +/// } +/// } +/// } +/// ``` +pub trait FunctionError { + fn panic(&self); +} + +impl FunctionError for T +where + T: Borrow +{ + fn panic(&self) { + crate::env::panic_str(self.borrow()) + } +} diff --git a/near-sdk/src/types/mod.rs b/near-sdk/src/types/mod.rs index 37fc4cc9c..100862316 100644 --- a/near-sdk/src/types/mod.rs +++ b/near-sdk/src/types/mod.rs @@ -13,6 +13,9 @@ pub use self::account_id::{AccountId, ParseAccountIdError}; mod gas; pub use self::gas::Gas; +mod error; +pub use self::error::FunctionError; + /// Raw type for duration in nanoseconds pub type Duration = u64; From f6450243dcf32d22c2996d4ea8e363b3f8ff1888 Mon Sep 17 00:00:00 2001 From: Daniyar Itegulov Date: Tue, 8 Mar 2022 14:58:59 +1100 Subject: [PATCH 2/6] add compilation tests for FunctionError derivation macro --- near-sdk/compilation_tests/all.rs | 1 + near-sdk/compilation_tests/function_error.rs | 50 ++++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 near-sdk/compilation_tests/function_error.rs diff --git a/near-sdk/compilation_tests/all.rs b/near-sdk/compilation_tests/all.rs index be3cf3b4d..f9a49ffc1 100644 --- a/near-sdk/compilation_tests/all.rs +++ b/near-sdk/compilation_tests/all.rs @@ -19,4 +19,5 @@ fn compilation_tests() { t.pass("compilation_tests/cond_compilation.rs"); t.compile_fail("compilation_tests/payable_view.rs"); t.pass("compilation_tests/borsh_storage_key.rs"); + t.pass("compilation_tests/function_error.rs"); } diff --git a/near-sdk/compilation_tests/function_error.rs b/near-sdk/compilation_tests/function_error.rs new file mode 100644 index 000000000..6636b5b49 --- /dev/null +++ b/near-sdk/compilation_tests/function_error.rs @@ -0,0 +1,50 @@ +//! Testing FunctionError macro. + +use borsh::{BorshDeserialize, BorshSerialize}; +use near_sdk::{near_bindgen, FunctionError}; +use std::fmt; + +#[derive(FunctionError, BorshSerialize)] +struct ErrorStruct { + message: String, +} + +impl fmt::Display for ErrorStruct { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "error ocurred: {}", self.message) + } +} + +#[derive(FunctionError, BorshSerialize)] +enum ErrorEnum { + NotFound, + Banned { account_id: String }, +} + +impl fmt::Display for ErrorEnum { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ErrorEnum::NotFound => write!(f, "not found"), + ErrorEnum::Banned { account_id } => write!(f, "account {} is banned", account_id) + } + } +} + +#[near_bindgen] +#[derive(BorshDeserialize, BorshSerialize, Default)] +struct Contract {} + +#[near_bindgen] +impl Contract { + #[return_result] + pub fn set(&self, value: String) -> Result { + Err(ErrorStruct { message: format!("Could not set to {}", value) }) + } + + #[return_result] + pub fn get(&self) -> Result { + Err(ErrorEnum::NotFound) + } +} + +fn main() {} From e6425e665c9f9b51d8c81886ab767d8a042b7549 Mon Sep 17 00:00:00 2001 From: Daniyar Itegulov Date: Tue, 8 Mar 2022 22:43:50 +1100 Subject: [PATCH 3/6] fmt and clippy --- .../src/core_impl/code_generator/impl_item_method_info.rs | 2 +- near-sdk-macros/src/core_impl/utils/mod.rs | 2 +- near-sdk/src/lib.rs | 2 +- near-sdk/src/types/error.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/near-sdk-macros/src/core_impl/code_generator/impl_item_method_info.rs b/near-sdk-macros/src/core_impl/code_generator/impl_item_method_info.rs index e31a33de0..d03c9360d 100644 --- a/near-sdk-macros/src/core_impl/code_generator/impl_item_method_info.rs +++ b/near-sdk-macros/src/core_impl/code_generator/impl_item_method_info.rs @@ -139,7 +139,7 @@ impl ImplItemMethodInfo { #contract_ser }, ReturnType::Type(_, return_type) - if utils::type_is_result(&return_type) && *is_returns_result => + if utils::type_is_result(return_type) && *is_returns_result => { let value_ser = match result_serializer { SerializerType::JSON => quote! { diff --git a/near-sdk-macros/src/core_impl/utils/mod.rs b/near-sdk-macros/src/core_impl/utils/mod.rs index 804cb6b7e..20c39633d 100644 --- a/near-sdk-macros/src/core_impl/utils/mod.rs +++ b/near-sdk-macros/src/core_impl/utils/mod.rs @@ -1,4 +1,4 @@ -use syn::{Type, Path, PathArguments, GenericArgument}; +use syn::{GenericArgument, Path, PathArguments, Type}; /// Checks whether the given path is literally "Result". /// Note that it won't match a fully qualified name `core::result::Result` or a type alias like diff --git a/near-sdk/src/lib.rs b/near-sdk/src/lib.rs index 2055a1c88..7c10937c4 100644 --- a/near-sdk/src/lib.rs +++ b/near-sdk/src/lib.rs @@ -7,7 +7,7 @@ extern crate quickcheck; pub use near_sdk_macros::{ callback, callback_vec, ext_contract, init, metadata, near_bindgen, result_serializer, - serializer, BorshStorageKey, PanicOnDefault, FunctionError + serializer, BorshStorageKey, FunctionError, PanicOnDefault, }; #[cfg(feature = "unstable")] diff --git a/near-sdk/src/types/error.rs b/near-sdk/src/types/error.rs index c83bb967d..6af66343d 100644 --- a/near-sdk/src/types/error.rs +++ b/near-sdk/src/types/error.rs @@ -28,7 +28,7 @@ pub trait FunctionError { impl FunctionError for T where - T: Borrow + T: Borrow, { fn panic(&self) { crate::env::panic_str(self.borrow()) From bf4c468d72199b317b0d61e202d6fad233a25d5a Mon Sep 17 00:00:00 2001 From: Daniyar Itegulov Date: Wed, 9 Mar 2022 10:50:30 +1100 Subject: [PATCH 4/6] address comments --- near-sdk-macros/src/lib.rs | 4 ++-- near-sdk/src/types/error.rs | 12 +++++------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/near-sdk-macros/src/lib.rs b/near-sdk-macros/src/lib.rs index b95ab0ba7..b0c215949 100644 --- a/near-sdk-macros/src/lib.rs +++ b/near-sdk-macros/src/lib.rs @@ -205,8 +205,8 @@ pub fn function_error(item: TokenStream) -> TokenStream { }; TokenStream::from(quote! { impl near_sdk::FunctionError for #name { - fn panic(&self) { - near_sdk::env::panic_str(&std::string::ToString::to_string(&self)) + fn panic(&self) -> ! { + near_sdk::env::panic_str(&::std::string::ToString::to_string(&self)) } } }) diff --git a/near-sdk/src/types/error.rs b/near-sdk/src/types/error.rs index 6af66343d..6b831d939 100644 --- a/near-sdk/src/types/error.rs +++ b/near-sdk/src/types/error.rs @@ -1,5 +1,3 @@ -use std::borrow::Borrow; - /// Enables contract runtime to panic with the given type. Any error type used in conjunction /// with `#[return_result]` has to implement this trait. /// @@ -12,7 +10,7 @@ use std::borrow::Borrow; /// } /// /// impl FunctionError for Error { -/// fn panic(&self) { +/// fn panic(&self) -> ! { /// match self { /// Error::NotFound => /// near_sdk::env::panic_str("not found"), @@ -23,14 +21,14 @@ use std::borrow::Borrow; /// } /// ``` pub trait FunctionError { - fn panic(&self); + fn panic(&self) -> !; } impl FunctionError for T where - T: Borrow, + T: AsRef, { - fn panic(&self) { - crate::env::panic_str(self.borrow()) + fn panic(&self) -> ! { + crate::env::panic_str(self.as_ref()) } } From 43b65d8e3fa098c72f2c4f5db952707f27979dbe Mon Sep 17 00:00:00 2001 From: Daniyar Itegulov Date: Wed, 9 Mar 2022 19:14:11 +1100 Subject: [PATCH 5/6] add code generation tests --- .../code_generator/item_impl_info.rs | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/near-sdk-macros/src/core_impl/code_generator/item_impl_info.rs b/near-sdk-macros/src/core_impl/code_generator/item_impl_info.rs index 430e0b9e7..a315ee32f 100644 --- a/near-sdk-macros/src/core_impl/code_generator/item_impl_info.rs +++ b/near-sdk-macros/src/core_impl/code_generator/item_impl_info.rs @@ -751,4 +751,63 @@ mod tests { ); assert_eq!(expected.to_string(), actual.to_string()); } + + #[test] + fn return_result_json() { + let impl_type: Type = syn::parse_str("Hello").unwrap(); + let mut method: ImplItemMethod = parse_quote! { + #[return_result] + pub fn method(&self) -> Result { } + }; + let method_info = ImplItemMethodInfo::new(&mut method, impl_type).unwrap(); + let actual = method_info.method_wrapper(); + let expected = quote!( + #[cfg(target_arch = "wasm32")] + #[no_mangle] + pub extern "C" fn method() { + near_sdk::env::setup_panic_hook(); + let contract: Hello = near_sdk::env::state_read().unwrap_or_default(); + let result = contract.method(); + match result { + Ok(result) => { + let result = + near_sdk::serde_json::to_vec(&result).expect("Failed to serialize the return value using JSON."); + near_sdk::env::value_return(&result); + } + Err(err) => near_sdk::FunctionError::panic(&err) + } + } + ); + assert_eq!(expected.to_string(), actual.to_string()); + } + + #[test] + fn return_result_borsh() { + let impl_type: Type = syn::parse_str("Hello").unwrap(); + let mut method: ImplItemMethod = parse_quote! { + #[return_result] + #[result_serializer(borsh)] + pub fn method(&self) -> Result { } + }; + let method_info = ImplItemMethodInfo::new(&mut method, impl_type).unwrap(); + let actual = method_info.method_wrapper(); + let expected = quote!( + #[cfg(target_arch = "wasm32")] + #[no_mangle] + pub extern "C" fn method() { + near_sdk::env::setup_panic_hook(); + let contract: Hello = near_sdk::env::state_read().unwrap_or_default(); + let result = contract.method(); + match result { + Ok(result) => { + let result = + near_sdk::borsh::BorshSerialize::try_to_vec(&result).expect("Failed to serialize the return value using Borsh."); + near_sdk::env::value_return(&result); + } + Err(err) => near_sdk::FunctionError::panic(&err) + } + } + ); + assert_eq!(expected.to_string(), actual.to_string()); + } } From f8494f4b7165e7cf67e7f444b49dc7d988b210e1 Mon Sep 17 00:00:00 2001 From: Daniyar Itegulov Date: Thu, 10 Mar 2022 08:56:24 +1100 Subject: [PATCH 6/6] add an entry to changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3911fe067..3dbf1b6c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,8 +3,8 @@ ## [unreleased] ### Features - Added `Debug` and `PartialEq` implementations for `PromiseError`. [PR 728](https://github.com/near/near-sdk-rs/pull/728). - - Added convenience function `env::block_timestamp_ms` to return ms since 1970. [PR 736](https://github.com/near/near-sdk-rs/pull/728) +- Added an optional way to handle contract errors with `Result`. [PR 745](https://github.com/near/near-sdk-rs/pull/745). ## `4.0.0-pre.7` [02-02-2022]