Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented syn::Error::combine for ItemImplInfo and ItemTraitInfo #1065

Closed
89 changes: 56 additions & 33 deletions near-sdk-macros/src/core_impl/info_extractor/arg_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::core_impl::info_extractor::{SerializerAttr, SerializerType};
use crate::core_impl::utils;
use proc_macro2::{Span, TokenStream};
use quote::ToTokens;
use syn::{spanned::Spanned, Attribute, Error, Ident, Pat, PatType, Token, Type};
use syn::{Attribute, Error, Ident, Pat, PatType, Token, Type};

pub enum BindgenArgType {
/// Argument that we read from `env::input()`.
Expand Down Expand Up @@ -45,29 +45,28 @@ impl ArgInfo {
/// Extract near-sdk specific argument info.
pub fn new(original: &mut PatType, source_type: &TokenStream) -> syn::Result<Self> {
let mut non_bindgen_attrs = vec![];
let pat_reference;
let pat_mutability;
let ident = match original.pat.as_ref() {
let pat_info = match original.pat.as_ref() {
Pat::Ident(pat_ident) => {
pat_reference = pat_ident.by_ref;
pat_mutability = pat_ident.mutability;
pat_ident.ident.clone()
}
_ => {
return Err(Error::new(
original.span(),
"Only identity patterns are supported in function arguments.",
));
Ok((pat_ident.by_ref, pat_ident.mutability, pat_ident.ident.clone()))
}
_ => Err(Error::new_spanned(
&original.pat,
"Only identity patterns are supported in function arguments.",
)),
};
let sanitize_self = utils::sanitize_self(&original.ty, source_type)?;
*original.ty.as_mut() = sanitize_self.ty;
let (reference, mutability, ty) =
utils::extract_ref_mut(original.ty.as_ref(), original.span())?;

let result_sanitize_and_ty = (|| {
let sanitize_self = utils::sanitize_self(&original.ty, source_type)?;
*original.ty.as_mut() = sanitize_self.ty.clone();
let ty_info = utils::extract_ref_mut(original.ty.as_ref())?;
Ok((sanitize_self, ty_info))
})();

// In the absence of callback attributes this is a regular argument.
let mut bindgen_ty = BindgenArgType::Regular;
// In the absence of serialization attributes this is a JSON serialization.
let mut serializer_ty = SerializerType::JSON;
let mut more_errors: Vec<Error> = Vec::new();
for attr in &original.attrs {
let attr_str = attr.path.to_token_stream().to_string();
match attr_str.as_str() {
Expand All @@ -80,10 +79,15 @@ impl ArgInfo {
"callback_vec" => {
bindgen_ty = BindgenArgType::CallbackArgVec;
}
"serializer" => {
let serializer: SerializerAttr = syn::parse2(attr.tokens.clone())?;
serializer_ty = serializer.serializer_type;
}
"serializer" => match syn::parse2::<SerializerAttr>(attr.tokens.clone()) {
Ok(serializer) => {
serializer_ty = serializer.serializer_type;
}
Err(e) => {
let spanned_error = syn::Error::new_spanned(attr, e.to_string());
more_errors.push(spanned_error);
}
},
_ => {
non_bindgen_attrs.push((*attr).clone());
}
Expand All @@ -99,18 +103,37 @@ impl ArgInfo {
&& attr_str != "callback_unwrap"
});

Ok(Self {
non_bindgen_attrs,
ident,
pat_reference,
pat_mutability,
reference,
mutability,
ty,
bindgen_ty,
serializer_ty,
self_occurrences: sanitize_self.self_occurrences,
original: original.clone(),
match (&pat_info, &result_sanitize_and_ty, more_errors.is_empty()) {
(
Ok((pat_reference, pat_mutability, ident)),
Ok((sanitize_self, (reference, mutability, ty))),
true,
) => Ok(Self {
non_bindgen_attrs,
ident: ident.clone(),
pat_reference: *pat_reference,
pat_mutability: *pat_mutability,
reference: *reference,
mutability: *mutability,
ty: ty.clone(),
bindgen_ty,
serializer_ty,
self_occurrences: sanitize_self.self_occurrences.clone(),
original: original.clone(),
}),
_ => {
more_errors.extend(pat_info.err());
Copy link
Contributor

@uint uint Aug 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clever and succinct!

more_errors.extend(result_sanitize_and_ty.err());
Err(Self::combine_errors(more_errors).unwrap())
}
}
}

// helper function
fn combine_errors(errors: impl IntoIterator<Item = Error>) -> Option<Error> {
errors.into_iter().reduce(|mut acc, e| {
acc.combine(syn::Error::new(e.span(), e.to_string()));
acc
})
}
}
17 changes: 15 additions & 2 deletions near-sdk-macros/src/core_impl/info_extractor/item_impl_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,26 @@ impl ItemImplInfo {
let ty = (*original.self_ty.as_ref()).clone();

let mut methods = vec![];
let mut errors = vec![];
for subitem in &mut original.items {
if let ImplItem::Method(m) = subitem {
if let Some(method_info) = ImplItemMethodInfo::new(m, is_trait_impl, ty.clone())? {
methods.push(method_info);
match ImplItemMethodInfo::new(m, is_trait_impl, ty.clone()) {
Ok(Some(method_info)) => methods.push(method_info),
Ok(None) => {} // do nothing
Err(e) => errors.push(e),
}
}
}

if !errors.is_empty() {
// Combine all errors into one
let combined_error = errors.into_iter().reduce(|mut l, r| {
l.combine(r);
l
});
return Err(combined_error.unwrap());
}

Ok(Self { ty, methods })
}
}
29 changes: 20 additions & 9 deletions near-sdk-macros/src/core_impl/info_extractor/item_trait_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,20 @@ impl ItemTraitInfo {
});

let mut methods = vec![];
let mut errors = vec![];
for item in &mut original.items {
match item {
TraitItem::Type(_) => {
return Err(Error::new(
item.span(),
"Traits for external contracts do not support associated trait types yet.",
))
}
TraitItem::Type(_) => errors.push(Error::new(
item.span(),
"Traits for external contracts do not support associated trait types yet.",
)),
TraitItem::Method(method) => {
methods
.push(TraitItemMethodInfo::new(method, &original.ident.to_token_stream())?);
match TraitItemMethodInfo::new(method, &original.ident.to_token_stream()) {
Ok(method_info) => methods.push(method_info),
Err(e) => errors.push(e),
};
if method.default.is_some() {
return Err(Error::new(
errors.push(Error::new(
method.span(),
"Traits that are used to describe external contract should not include
default implementations because this is not a valid use case of traits
Expand All @@ -45,6 +46,16 @@ impl ItemTraitInfo {
_ => {}
}
}

if !errors.is_empty() {
// Combine all errors into one
let combined_error = errors.into_iter().reduce(|mut l, r| {
l.combine(r);
l
});
return Err(combined_error.unwrap());
}

Ok(Self { original: original.clone(), mod_name, methods })
}
}
7 changes: 2 additions & 5 deletions near-sdk-macros/src/core_impl/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,16 +79,13 @@ pub(crate) fn extract_vec_type(ty: &Type) -> Option<&Type> {
}

/// Extracts reference and mutability tokens from a `Type` object. Also, strips top-level lifetime binding if present.
pub(crate) fn extract_ref_mut(
ty: &Type,
span: Span,
) -> syn::Result<(Option<And>, Option<Mut>, Type)> {
pub(crate) fn extract_ref_mut(ty: &Type) -> syn::Result<(Option<And>, Option<Mut>, Type)> {
match ty {
x @ (Type::Array(_) | Type::Path(_) | Type::Tuple(_) | Type::Group(_)) => {
Ok((None, None, (*x).clone()))
}
Type::Reference(r) => Ok((Some(r.and_token), r.mutability, (*r.elem.as_ref()).clone())),
_ => Err(syn::Error::new(span, "Unsupported contract API type.")),
_ => Err(syn::Error::new_spanned(ty, "Unsupported contract API type.")),
}
}

Expand Down
4 changes: 2 additions & 2 deletions near-sdk/compilation_tests/bad_argument.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: Unsupported contract API type.
--> compilation_tests/bad_argument.rs:30:56
--> compilation_tests/bad_argument.rs:30:59
|
30 | pub fn insert(&mut self, key: TypeA, value: TypeB, t: impl MyTrait) -> Option<TypeB> {
| ^
| ^^^^^^^^^^^^
8 changes: 6 additions & 2 deletions near-sdk/compilation_tests/invalid_arg_pat.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
//! Method with non-deserializable argument type.

//This tests also checks whether argument errors gets combined or not.
//faulty_method checks a combination of serialiser and type not not supported
//faulty_method1 checks a combination of serialiser and only Identity pattern allowed.
//It is not possible to check Identity pattern and Type not supported together.
use borsh::{BorshDeserialize, BorshSerialize};
use near_sdk::{near_bindgen, PanicOnDefault};

Expand All @@ -9,7 +12,8 @@ struct Storage {}

#[near_bindgen]
impl Storage {
pub fn insert(&mut self, (a, b): (u8, u32)) {}
pub fn faulty_method(&mut self, #[serializer(SomeNonExistentSerializer)] _a: *mut u32) {}
pub fn faulty_method1(&mut self, #[serializer(SomeNonExistentSerializer)] (a, b): (u8, u32)) {}
}

fn main() {}
24 changes: 21 additions & 3 deletions near-sdk/compilation_tests/invalid_arg_pat.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,23 @@
error: Unsupported serializer type.
--> compilation_tests/invalid_arg_pat.rs:15:37
|
15 | pub fn faulty_method(&mut self, #[serializer(SomeNonExistentSerializer)] _a: *mut u32) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: Unsupported contract API type.
--> compilation_tests/invalid_arg_pat.rs:15:82
|
15 | pub fn faulty_method(&mut self, #[serializer(SomeNonExistentSerializer)] _a: *mut u32) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, this would highlight SomeNonExistentSerializer. The error would be much more helpful then.

| ^

error: Unsupported serializer type.
--> compilation_tests/invalid_arg_pat.rs:16:38
|
16 | pub fn faulty_method1(&mut self, #[serializer(SomeNonExistentSerializer)] (a, b): (u8, u32)) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, this is a possible regression. Previously, the tuple was highlighted here. Now it's the the # of the attribute.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, this is a possible regression. Previously, (a, b) was highlighted here. Now it's the the # of the attribute.

| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: Only identity patterns are supported in function arguments.
--> compilation_tests/invalid_arg_pat.rs:12:30
--> compilation_tests/invalid_arg_pat.rs:16:79
|
12 | pub fn insert(&mut self, (a, b): (u8, u32)) {}
| ^^^^^^
16 | pub fn faulty_method1(&mut self, #[serializer(SomeNonExistentSerializer)] (a, b): (u8, u32)) {}
| ^^^^^^
Loading