-
Notifications
You must be signed in to change notification settings - Fork 78
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
feat: add #[borsh(crate = ...)]
item-level attribute
#210
Conversation
Sample crate with this attribute tested. |
4d02272
to
8b67dd6
Compare
let mut res = None; | ||
let attr = attrs.iter().find(|attr| attr.path() == BORSH); | ||
if let Some(attr) = attr { | ||
let _ = attr.parse_nested_meta(|meta| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parsing error was silenced here, allowing to declare following value for the attribute without any syn::Error
and returning None
#[derive(BorshSerialize, BorshDeserialize, PartialEq, Debug)]
#[borsh(init={strange; blocky})]
struct A {
lazy: Option<u64>,
}
#[derive(BorshSerialize, BorshDeserialize, PartialEq, Debug)]
#[borsh(init=42)]
struct A {
lazy: Option<u64>,
}
8b67dd6
to
4e9ce21
Compare
let actual = check_item_attributes(&item_struct); | ||
local_insta_assert_snapshot!(actual.unwrap_err().to_token_stream().to_string()); | ||
let actual = check_attributes(&item_struct); | ||
local_insta_assert_debug_snapshot!(actual.unwrap_err()); | ||
} | ||
#[test] | ||
fn test_init_function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this test doesn't change due to impl-ToTokens-for-Option
7f7da10
to
3a48cc1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, looks great to me!
if meta.path == USE_DISCRIMINANT || meta.path == INIT { | ||
let _value_expr: Expr = meta.value()?.parse()?; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you help me understand why we need to parse something completely unrelated here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we comment out these 115-117 highlighted lines, we get a failure in test with following message:
---- internals::attributes::item::tests::test_init_function_with_use_discriminant_with_crate stdout ----
thread 'internals::attributes::item::tests::test_init_function_with_use_discriminant_with_crate' panicked at 'called `Result::unwrap()` on an `Err` value: Error("expected `,`")', borsh-derive/src/internals/attributes/item/mod.rs:412:20
The recommended parsing helper
parses the args init = initialization_method, crate = "reexporter::borsh", use_discriminant=true
of #[borsh(init = initialization_method, crate = "reexporter::borsh", use_discriminant=true)]
with following
code somewhere down the way:
pub(crate) fn parse_nested_meta(
input: [ParseStream](https://docs.rs/syn/latest/src/syn/parse.rs.html#223),
mut logic: impl FnMut([ParseNestedMeta](https://docs.rs/syn/latest/src/syn/meta.rs.html#163-166)) -> Result<()>,
) -> Result<()> {
loop {
let path = input.call(parse_meta_path)?;
logic(ParseNestedMeta { path, input })?;
if input.is_empty() {
return Ok(());
}
input.parse::<Token![,]>()?;
if input.is_empty() {
return Ok(());
}
}
}
It expects to completely consume input ParseStream<'a>
, without skipping anything.
If we comment out let _value_expr: Expr = meta.value()?.parse()?;
it returns an error input.parse::<Token![,]>()?;
on a new loop iteration on a =
punctuation, presumably originating here.
c1a5ee0
to
e2b9829
Compare
resolves #95
Need for this discovered in scope of work in near-sdk-rs update