From da2676b5e2581bbc2f043300034c196c8a5c2fc6 Mon Sep 17 00:00:00 2001 From: Tomasz Kurcz Date: Thu, 20 Jul 2023 18:41:08 +0200 Subject: [PATCH 1/4] Have `#[handle_result]` support any `Result` type --- .../src/core_impl/abi/abi_generator.rs | 12 +++++++---- .../info_extractor/impl_item_method_info.rs | 12 ----------- .../src/core_impl/info_extractor/mod.rs | 2 +- .../src/core_impl/info_extractor/visitor.rs | 9 +-------- near-sdk/compilation_tests/all.rs | 1 + .../compilation_tests/handle_result_alias.rs | 20 +++++++++++++++++++ near-sdk/src/private/mod.rs | 3 +++ near-sdk/src/private/result_type_ext.rs | 17 ++++++++++++++++ 8 files changed, 51 insertions(+), 25 deletions(-) create mode 100644 near-sdk/compilation_tests/handle_result_alias.rs create mode 100644 near-sdk/src/private/result_type_ext.rs diff --git a/near-sdk-macros/src/core_impl/abi/abi_generator.rs b/near-sdk-macros/src/core_impl/abi/abi_generator.rs index 32eca8698..09542bd71 100644 --- a/near-sdk-macros/src/core_impl/abi/abi_generator.rs +++ b/near-sdk-macros/src/core_impl/abi/abi_generator.rs @@ -1,6 +1,6 @@ use proc_macro2::{Span, TokenStream as TokenStream2}; use quote::{format_ident, quote}; -use syn::{Attribute, Lit::Str, Meta::NameValue, MetaNameValue, Type}; +use syn::{parse_quote, Attribute, Lit::Str, Meta::NameValue, MetaNameValue, Type}; use crate::core_impl::{ utils, BindgenArgType, ImplItemMethodInfo, ItemImplInfo, MethodKind, ReturnKind, SerializerType, @@ -202,7 +202,11 @@ impl ImplItemMethodInfo { match &self.attr_signature_info.returns.kind { Default => quote! { None }, General(ty) => self.abi_result_tokens_with_return_value(ty), - HandlesResult { ok_type } => self.abi_result_tokens_with_return_value(ok_type), + HandlesResult { ty } => { + // extract the `Ok` type from the result + let ty = parse_quote! { <#ty as near_sdk::__private::ResultTypeExt>::Okay }; + self.abi_result_tokens_with_return_value(&ty) + } } } @@ -329,7 +333,7 @@ mod tests { callbacks: vec![], callbacks_vec: None, result: Some(near_sdk::__private::AbiType::Json { - type_schema: gen.subschema_for::(), + type_schema: gen.subschema_for::< as near_sdk::__private::ResultTypeExt>::Okay>(), }) } }; @@ -366,7 +370,7 @@ mod tests { callbacks: vec![], callbacks_vec: None, result: Some(near_sdk::__private::AbiType::Borsh { - type_schema: ::schema_container(), + type_schema: < as near_sdk::__private::ResultTypeExt>::Okay as near_sdk::borsh::BorshSchema>::schema_container(), }) } }; diff --git a/near-sdk-macros/src/core_impl/info_extractor/impl_item_method_info.rs b/near-sdk-macros/src/core_impl/info_extractor/impl_item_method_info.rs index aed41846b..ad90d5f1e 100644 --- a/near-sdk-macros/src/core_impl/info_extractor/impl_item_method_info.rs +++ b/near-sdk-macros/src/core_impl/info_extractor/impl_item_method_info.rs @@ -62,18 +62,6 @@ mod tests { assert!(matches!(actual, ReturnType::Type(_, ty) if ty.as_ref() == &expected)); } - #[test] - fn handle_result_incorrect_return_type() { - let impl_type: Type = syn::parse_str("Hello").unwrap(); - let mut method: ImplItemMethod = parse_quote! { - #[handle_result] - pub fn method(&self) -> &'static str { } - }; - let actual = ImplItemMethodInfo::new(&mut method, false, impl_type).map(|_| ()).unwrap_err(); - let expected = "Function marked with #[handle_result] should return Result (where E implements FunctionError)."; - assert_eq!(expected, actual.to_string()); - } - #[test] fn handle_result_without_marker() { let impl_type: Type = syn::parse_str("Hello").unwrap(); diff --git a/near-sdk-macros/src/core_impl/info_extractor/mod.rs b/near-sdk-macros/src/core_impl/info_extractor/mod.rs index 622761597..ec64fb98f 100644 --- a/near-sdk-macros/src/core_impl/info_extractor/mod.rs +++ b/near-sdk-macros/src/core_impl/info_extractor/mod.rs @@ -84,5 +84,5 @@ pub struct Returns { pub enum ReturnKind { Default, General(Type), - HandlesResult { ok_type: Type }, + HandlesResult { ty: Type }, } diff --git a/near-sdk-macros/src/core_impl/info_extractor/visitor.rs b/near-sdk-macros/src/core_impl/info_extractor/visitor.rs index 1f94c30bc..b698a8897 100644 --- a/near-sdk-macros/src/core_impl/info_extractor/visitor.rs +++ b/near-sdk-macros/src/core_impl/info_extractor/visitor.rs @@ -209,14 +209,7 @@ fn is_view(sig: &Signature) -> bool { fn parse_return_kind(typ: &Type, handles_result: bool) -> syn::Result { if handles_result { - if let Some(ok_type) = utils::extract_ok_type(typ) { - Ok(ReturnKind::HandlesResult { ok_type: ok_type.clone() }) - } else { - Err(Error::new( - typ.span(), - "Function marked with #[handle_result] should return Result (where E implements FunctionError).", - )) - } + Ok(ReturnKind::HandlesResult { ty: typ.clone() }) } else if utils::type_is_result(typ) { Err(Error::new( typ.span(), diff --git a/near-sdk/compilation_tests/all.rs b/near-sdk/compilation_tests/all.rs index 09f9d73c7..99415e049 100644 --- a/near-sdk/compilation_tests/all.rs +++ b/near-sdk/compilation_tests/all.rs @@ -25,6 +25,7 @@ fn compilation_tests() { t.compile_fail("compilation_tests/generic_function.rs"); t.compile_fail("compilation_tests/generic_const_function.rs"); t.pass("compilation_tests/self_support.rs"); + t.pass("compilation_tests/handle_result_alias.rs"); // The following couple tests should be activated before releasing 5.0 // See: https://github.com/near/near-sdk-rs/issues/1005 // diff --git a/near-sdk/compilation_tests/handle_result_alias.rs b/near-sdk/compilation_tests/handle_result_alias.rs new file mode 100644 index 000000000..193e63c01 --- /dev/null +++ b/near-sdk/compilation_tests/handle_result_alias.rs @@ -0,0 +1,20 @@ +use borsh::{BorshDeserialize, BorshSerialize}; +use near_sdk::near_bindgen; + +type MyResult = Result; + +#[near_bindgen] +#[derive(Default, BorshDeserialize, BorshSerialize)] +struct Contract { + value: u32, +} + +#[near_bindgen] +impl Contract { + #[handle_result] + pub fn fun(&self) -> MyResult { + Err("error") + } +} + +fn main() {} diff --git a/near-sdk/src/private/mod.rs b/near-sdk/src/private/mod.rs index 15edef0f1..a174702d3 100644 --- a/near-sdk/src/private/mod.rs +++ b/near-sdk/src/private/mod.rs @@ -12,6 +12,9 @@ pub use schemars; mod metadata; pub use metadata::{Metadata, MethodMetadata}; +mod result_type_ext; +pub use result_type_ext::ResultTypeExt; + use crate::IntoStorageKey; use borsh::BorshSerialize; diff --git a/near-sdk/src/private/result_type_ext.rs b/near-sdk/src/private/result_type_ext.rs new file mode 100644 index 000000000..2739c8e26 --- /dev/null +++ b/near-sdk/src/private/result_type_ext.rs @@ -0,0 +1,17 @@ +pub trait ResultTypeExt: seal::ResultTypeExtSeal { + type Okay; + type Error; +} + +impl ResultTypeExt for Result { + type Okay = T; + type Error = E; +} + +// This is the "sealed trait" pattern: +// https://rust-lang.github.io/api-guidelines/future-proofing.html +mod seal { + pub trait ResultTypeExtSeal {} + + impl ResultTypeExtSeal for Result {} +} From 42a4676b48e60871ca193a5852a10aea56da75cc Mon Sep 17 00:00:00 2001 From: Tomasz Kurcz Date: Fri, 18 Aug 2023 19:21:27 +0200 Subject: [PATCH 2/4] macros: add `#[handle_result(aliased)]` --- .../core_impl/info_extractor/attr_sig_info.rs | 7 ++- .../info_extractor/handle_result_attr.rs | 43 +++++++++++++++ .../info_extractor/impl_item_method_info.rs | 12 ++++ .../src/core_impl/info_extractor/mod.rs | 3 + .../src/core_impl/info_extractor/visitor.rs | 55 ++++++++++++++----- .../compilation_tests/handle_result_alias.rs | 2 +- 6 files changed, 105 insertions(+), 17 deletions(-) create mode 100644 near-sdk-macros/src/core_impl/info_extractor/handle_result_attr.rs 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 12c259d48..2fe16198f 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 @@ -1,5 +1,7 @@ use super::visitor::Visitor; -use super::{ArgInfo, BindgenArgType, InitAttr, MethodKind, SerializerAttr, SerializerType}; +use super::{ + ArgInfo, BindgenArgType, HandleResultAttr, InitAttr, MethodKind, SerializerAttr, SerializerType, +}; use crate::core_impl::{utils, Returns}; use proc_macro2::{Span, TokenStream as TokenStream2}; use quote::ToTokens; @@ -102,7 +104,8 @@ impl AttrSigInfo { visitor.visit_result_serializer_attr(attr, &serializer)?; } "handle_result" => { - visitor.visit_handle_result_attr(); + let handle_result = HandleResultAttr::from_attr(attr)?; + visitor.visit_handle_result_attr(&handle_result); } _ => { non_bindgen_attrs.push((*attr).clone()); diff --git a/near-sdk-macros/src/core_impl/info_extractor/handle_result_attr.rs b/near-sdk-macros/src/core_impl/info_extractor/handle_result_attr.rs new file mode 100644 index 000000000..18adef3fa --- /dev/null +++ b/near-sdk-macros/src/core_impl/info_extractor/handle_result_attr.rs @@ -0,0 +1,43 @@ +use syn::spanned::Spanned as _; +use syn::{Attribute, Meta, NestedMeta}; + +pub struct HandleResultAttr { + pub check: bool, +} + +impl HandleResultAttr { + pub fn from_attr(attr: &Attribute) -> syn::Result { + let meta = attr.parse_meta()?; + + let mut check = true; + + match meta { + Meta::Path(_) => {} + Meta::List(l) => { + for meta in l.nested { + let span = meta.span(); + let err = || Err(syn::Error::new(span, "invalid attribute")); + + match meta { + NestedMeta::Meta(Meta::Path(p)) => { + if let Some(ident) = p.get_ident() { + match ident.to_string().as_str() { + "aliased" => check = false, + _ => return err(), + } + } else { + return err(); + } + } + _ => return err(), + } + } + } + Meta::NameValue(_) => { + return Err(syn::Error::new(attr.span(), "unexpected name-value pair")) + } + }; + + Ok(Self { check }) + } +} diff --git a/near-sdk-macros/src/core_impl/info_extractor/impl_item_method_info.rs b/near-sdk-macros/src/core_impl/info_extractor/impl_item_method_info.rs index ad90d5f1e..bb4f9ac9a 100644 --- a/near-sdk-macros/src/core_impl/info_extractor/impl_item_method_info.rs +++ b/near-sdk-macros/src/core_impl/info_extractor/impl_item_method_info.rs @@ -62,6 +62,18 @@ mod tests { assert!(matches!(actual, ReturnType::Type(_, ty) if ty.as_ref() == &expected)); } + #[test] + fn handle_result_incorrect_return_type() { + let impl_type: Type = syn::parse_str("Hello").unwrap(); + let mut method: ImplItemMethod = parse_quote! { + #[handle_result] + pub fn method(&self) -> &'static str { } + }; + let actual = ImplItemMethodInfo::new(&mut method, false, impl_type).map(|_| ()).unwrap_err(); + let expected = "Function marked with #[handle_result] should return Result (where E implements FunctionError). If you're trying to use a type alias for `Result`, try `#[handle_result(aliased)]`."; + assert_eq!(expected, actual.to_string()); + } + #[test] fn handle_result_without_marker() { let impl_type: Type = syn::parse_str("Hello").unwrap(); diff --git a/near-sdk-macros/src/core_impl/info_extractor/mod.rs b/near-sdk-macros/src/core_impl/info_extractor/mod.rs index ec64fb98f..11900599b 100644 --- a/near-sdk-macros/src/core_impl/info_extractor/mod.rs +++ b/near-sdk-macros/src/core_impl/info_extractor/mod.rs @@ -3,6 +3,9 @@ use syn::{Receiver, ReturnType, Type}; mod serializer_attr; pub use serializer_attr::SerializerAttr; +mod handle_result_attr; +pub use handle_result_attr::HandleResultAttr; + mod arg_info; pub use arg_info::{ArgInfo, BindgenArgType}; diff --git a/near-sdk-macros/src/core_impl/info_extractor/visitor.rs b/near-sdk-macros/src/core_impl/info_extractor/visitor.rs index b698a8897..f9d6777ed 100644 --- a/near-sdk-macros/src/core_impl/info_extractor/visitor.rs +++ b/near-sdk-macros/src/core_impl/info_extractor/visitor.rs @@ -1,4 +1,4 @@ -use super::{InitAttr, MethodKind, ReturnKind, SerializerAttr}; +use super::{HandleResultAttr, InitAttr, MethodKind, ReturnKind, SerializerAttr}; use crate::core_impl::{utils, CallMethod, InitMethod, Returns, SerializerType, ViewMethod}; use quote::ToTokens; use syn::{spanned::Spanned, Attribute, Error, FnArg, Receiver, ReturnType, Signature, Type}; @@ -11,7 +11,7 @@ pub struct Visitor { } struct ParsedData { - handles_result: bool, + handles_result: ResultHandling, is_payable: bool, is_private: bool, ignores_state: bool, @@ -19,6 +19,22 @@ struct ParsedData { receiver: Option, } +#[derive(Copy, Clone, PartialEq, Eq)] +enum ResultHandling { + // No result handling. + None, + // Attempt to handle the `Result` without performing a heuristic type check. + NoCheck, + // Attempt to handle the `Result` with a heuristic type check. + Check, +} + +impl Default for ResultHandling { + fn default() -> Self { + Self::None + } +} + impl Default for ParsedData { fn default() -> Self { Self { @@ -125,8 +141,9 @@ impl Visitor { } } - pub fn visit_handle_result_attr(&mut self) { - self.parsed_data.handles_result = true + pub fn visit_handle_result_attr(&mut self, params: &HandleResultAttr) { + self.parsed_data.handles_result = + if params.check { ResultHandling::Check } else { ResultHandling::NoCheck } } pub fn visit_receiver(&mut self, receiver: &Receiver) -> syn::Result<()> { @@ -207,19 +224,29 @@ fn is_view(sig: &Signature) -> bool { } } -fn parse_return_kind(typ: &Type, handles_result: bool) -> syn::Result { - if handles_result { - Ok(ReturnKind::HandlesResult { ty: typ.clone() }) - } else if utils::type_is_result(typ) { - Err(Error::new( - typ.span(), - "Serializing Result has been deprecated. Consider marking your method \ +fn parse_return_kind(typ: &Type, handles_result: ResultHandling) -> syn::Result { + match handles_result { + ResultHandling::NoCheck => Ok(ReturnKind::HandlesResult { ty: typ.clone() }), + ResultHandling::Check => { + if !utils::type_is_result(typ) { + Err(Error::new(typ.span(), "Function marked with #[handle_result] should return Result (where E implements FunctionError). If you're trying to use a type alias for `Result`, try `#[handle_result(aliased)]`.")) + } else { + Ok(ReturnKind::HandlesResult { ty: typ.clone() }) + } + } + ResultHandling::None => { + if utils::type_is_result(typ) { + Err(Error::new( + typ.span(), + "Serializing Result has been deprecated. Consider marking your method \ with #[handle_result] if the second generic represents a panicable error or \ replacing Result with another two type sum enum otherwise. If you really want \ to keep the legacy behavior, mark the method with #[handle_result] and make \ it return Result, near_sdk::Abort>.", - )) - } else { - Ok(ReturnKind::General(typ.clone())) + )) + } else { + Ok(ReturnKind::General(typ.clone())) + } + } } } diff --git a/near-sdk/compilation_tests/handle_result_alias.rs b/near-sdk/compilation_tests/handle_result_alias.rs index 193e63c01..eda2ae205 100644 --- a/near-sdk/compilation_tests/handle_result_alias.rs +++ b/near-sdk/compilation_tests/handle_result_alias.rs @@ -11,7 +11,7 @@ struct Contract { #[near_bindgen] impl Contract { - #[handle_result] + #[handle_result(aliased)] pub fn fun(&self) -> MyResult { Err("error") } From fcc5a0c22812f782a7b9aad6ac72ba8dacb3d1ff Mon Sep 17 00:00:00 2001 From: Tomasz Kurcz Date: Fri, 18 Aug 2023 19:39:06 +0200 Subject: [PATCH 3/4] cosmetic --- near-sdk-macros/src/core_impl/info_extractor/mod.rs | 2 +- near-sdk-macros/src/core_impl/info_extractor/visitor.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/near-sdk-macros/src/core_impl/info_extractor/mod.rs b/near-sdk-macros/src/core_impl/info_extractor/mod.rs index 11900599b..93d00bf6e 100644 --- a/near-sdk-macros/src/core_impl/info_extractor/mod.rs +++ b/near-sdk-macros/src/core_impl/info_extractor/mod.rs @@ -87,5 +87,5 @@ pub struct Returns { pub enum ReturnKind { Default, General(Type), - HandlesResult { ty: Type }, + HandlesResult(Type), } diff --git a/near-sdk-macros/src/core_impl/info_extractor/visitor.rs b/near-sdk-macros/src/core_impl/info_extractor/visitor.rs index f9d6777ed..a327a6fa5 100644 --- a/near-sdk-macros/src/core_impl/info_extractor/visitor.rs +++ b/near-sdk-macros/src/core_impl/info_extractor/visitor.rs @@ -226,12 +226,12 @@ fn is_view(sig: &Signature) -> bool { fn parse_return_kind(typ: &Type, handles_result: ResultHandling) -> syn::Result { match handles_result { - ResultHandling::NoCheck => Ok(ReturnKind::HandlesResult { ty: typ.clone() }), + ResultHandling::NoCheck => Ok(ReturnKind::HandlesResult(typ.clone())), ResultHandling::Check => { if !utils::type_is_result(typ) { Err(Error::new(typ.span(), "Function marked with #[handle_result] should return Result (where E implements FunctionError). If you're trying to use a type alias for `Result`, try `#[handle_result(aliased)]`.")) } else { - Ok(ReturnKind::HandlesResult { ty: typ.clone() }) + Ok(ReturnKind::HandlesResult(typ.clone())) } } ResultHandling::None => { From 6b2aef4e612388fb72201643cfab02a7b532bef2 Mon Sep 17 00:00:00 2001 From: Tomasz Kurcz Date: Fri, 18 Aug 2023 20:31:46 +0200 Subject: [PATCH 4/4] lint --- near-sdk-macros/src/core_impl/abi/abi_generator.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/near-sdk-macros/src/core_impl/abi/abi_generator.rs b/near-sdk-macros/src/core_impl/abi/abi_generator.rs index 3362a0e1e..a3a143669 100644 --- a/near-sdk-macros/src/core_impl/abi/abi_generator.rs +++ b/near-sdk-macros/src/core_impl/abi/abi_generator.rs @@ -204,7 +204,7 @@ impl ImplItemMethodInfo { match &self.attr_signature_info.returns.kind { Default => quote! { ::std::option::Option::None }, General(ty) => self.abi_result_tokens_with_return_value(ty), - HandlesResult { ty } => { + HandlesResult(ty) => { // extract the `Ok` type from the result let ty = parse_quote! { <#ty as near_sdk::__private::ResultTypeExt>::Okay }; self.abi_result_tokens_with_return_value(&ty)