From cbea5b5f505ad9e0f1943f5f8b7f787a4d4da54e Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Wed, 15 Nov 2023 13:32:14 +0100 Subject: [PATCH 01/10] packable: only pass visitor to verify_with if it was provided --- .../invalid_field_type_verify_with_field.rs | 2 +- ...nvalid_field_type_verify_with_field.stderr | 2 +- .../invalid_field_type_verify_with_struct.rs | 2 +- ...valid_field_type_verify_with_struct.stderr | 2 +- .../tests/pass/verify_with_field.rs | 2 +- .../tests/pass/verify_with_field_visitor.rs | 37 ++++++++++++++++++ .../tests/pass/verify_with_struct.rs | 2 +- .../tests/pass/verify_with_struct_visitor.rs | 38 +++++++++++++++++++ packable/packable-derive/src/fragments.rs | 21 ++++++++-- packable/packable-derive/src/trait_impl.rs | 6 ++- .../src/unpack_visitor_info.rs | 7 +++- packable/packable/src/packable/bounded.rs | 4 +- 12 files changed, 110 insertions(+), 15 deletions(-) create mode 100644 packable/packable-derive-test/tests/pass/verify_with_field_visitor.rs create mode 100644 packable/packable-derive-test/tests/pass/verify_with_struct_visitor.rs diff --git a/packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_field.rs b/packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_field.rs index 9f62218..afb6397 100644 --- a/packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_field.rs +++ b/packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_field.rs @@ -21,7 +21,7 @@ impl From for PickyError { } } -fn verify_value(&value: &u64, _: &()) -> Result<(), PickyError> { +fn verify_value(&value: &u64) -> Result<(), PickyError> { if !VERIFY || value == 42 { Ok(()) } else { diff --git a/packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_field.stderr b/packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_field.stderr index f80bf8d..0e6b356 100644 --- a/packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_field.stderr +++ b/packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_field.stderr @@ -12,6 +12,6 @@ error[E0308]: mismatched types note: function defined here --> tests/fail/invalid_field_type_verify_with_field.rs:24:4 | -24 | fn verify_value(&value: &u64, _: &()) -> Result<(), PickyError> { +24 | fn verify_value(&value: &u64) -> Result<(), PickyError> { | ^^^^^^^^^^^^ ------------ = note: this error originates in the derive macro `Packable` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_struct.rs b/packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_struct.rs index 47737c2..11cef43 100644 --- a/packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_struct.rs +++ b/packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_struct.rs @@ -21,7 +21,7 @@ impl From for PickyError { } } -fn verify(&value: &u64, _: &()) -> Result<(), PickyError> { +fn verify(&value: &u64) -> Result<(), PickyError> { if !VERIFY || value == 42 { Ok(()) } else { diff --git a/packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_struct.stderr b/packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_struct.stderr index a88eb5d..b5e9756 100644 --- a/packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_struct.stderr +++ b/packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_struct.stderr @@ -12,6 +12,6 @@ error[E0308]: mismatched types note: function defined here --> tests/fail/invalid_field_type_verify_with_struct.rs:24:4 | -24 | fn verify(&value: &u64, _: &()) -> Result<(), PickyError> { +24 | fn verify(&value: &u64) -> Result<(), PickyError> { | ^^^^^^ ------------ = note: this error originates in the derive macro `Packable` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/packable/packable-derive-test/tests/pass/verify_with_field.rs b/packable/packable-derive-test/tests/pass/verify_with_field.rs index 31f27d9..0616e04 100644 --- a/packable/packable-derive-test/tests/pass/verify_with_field.rs +++ b/packable/packable-derive-test/tests/pass/verify_with_field.rs @@ -21,7 +21,7 @@ impl From for PickyError { } } -fn verify_value(&value: &u8, _: &()) -> Result<(), PickyError> { +fn verify_value(&value: &u8) -> Result<(), PickyError> { if !VERIFY || value == 42 { Ok(()) } else { diff --git a/packable/packable-derive-test/tests/pass/verify_with_field_visitor.rs b/packable/packable-derive-test/tests/pass/verify_with_field_visitor.rs new file mode 100644 index 0000000..1e6c67b --- /dev/null +++ b/packable/packable-derive-test/tests/pass/verify_with_field_visitor.rs @@ -0,0 +1,37 @@ +// Copyright 2023 IOTA Stiftung +// SPDX-License-Identifier: Apache-2.0 + +#![allow(unused_imports)] + +use core::convert::Infallible; + +use packable::{ + error::{UnknownTagError, UnpackError, UnpackErrorExt}, + packer::Packer, + unpacker::Unpacker, + Packable, +}; + +#[derive(Debug)] +pub struct PickyError(u8); + +impl From for PickyError { + fn from(err: Infallible) -> Self { + match err {} + } +} + +fn verify_value(&value: &u8, _: &()) -> Result<(), PickyError> { + if !VERIFY || value == 42 { + Ok(()) + } else { + Err(PickyError(value)) + } +} + +#[derive(Packable)] +#[packable(unpack_error = PickyError)] +#[packable(unpack_visitor = ())] +pub struct Picky(#[packable(verify_with = verify_value)] u8); + +fn main() {} diff --git a/packable/packable-derive-test/tests/pass/verify_with_struct.rs b/packable/packable-derive-test/tests/pass/verify_with_struct.rs index 3695e66..13ecead 100644 --- a/packable/packable-derive-test/tests/pass/verify_with_struct.rs +++ b/packable/packable-derive-test/tests/pass/verify_with_struct.rs @@ -21,7 +21,7 @@ impl From for PickyError { } } -fn verify(value: &Picky, _: &()) -> Result<(), PickyError> { +fn verify(value: &Picky) -> Result<(), PickyError> { if !VERIFY || value.0 == 42 { Ok(()) } else { diff --git a/packable/packable-derive-test/tests/pass/verify_with_struct_visitor.rs b/packable/packable-derive-test/tests/pass/verify_with_struct_visitor.rs new file mode 100644 index 0000000..2e8b20c --- /dev/null +++ b/packable/packable-derive-test/tests/pass/verify_with_struct_visitor.rs @@ -0,0 +1,38 @@ +// Copyright 2023 IOTA Stiftung +// SPDX-License-Identifier: Apache-2.0 + +#![allow(unused_imports)] + +use core::convert::Infallible; + +use packable::{ + error::{UnknownTagError, UnpackError, UnpackErrorExt}, + packer::Packer, + unpacker::Unpacker, + Packable, +}; + +#[derive(Debug)] +pub struct PickyError(u8); + +impl From for PickyError { + fn from(err: Infallible) -> Self { + match err {} + } +} + +fn verify(value: &Picky, _: &()) -> Result<(), PickyError> { + if !VERIFY || value.0 == 42 { + Ok(()) + } else { + Err(PickyError(value.0)) + } +} + +#[derive(Packable)] +#[packable(unpack_error = PickyError)] +#[packable(verify_with = verify)] +#[packable(unpack_visitor = ())] +pub struct Picky(u8); + +fn main() {} diff --git a/packable/packable-derive/src/fragments.rs b/packable/packable-derive/src/fragments.rs index 4188b88..6f96e7e 100644 --- a/packable/packable-derive/src/fragments.rs +++ b/packable/packable-derive/src/fragments.rs @@ -5,7 +5,7 @@ use proc_macro2::TokenStream; use quote::quote; use syn::{Ident, Path}; -use crate::record_info::RecordInfo; +use crate::{record_info::RecordInfo, unpack_visitor_info::UnpackVisitorInfo}; pub(crate) struct Fragments { // The pattern used to destructure the record. @@ -17,7 +17,12 @@ pub(crate) struct Fragments { } impl Fragments { - pub(crate) fn new(info: RecordInfo, verify_with: Option, crate_name: &Ident) -> Self { + pub(crate) fn new( + info: RecordInfo, + verify_with: Option, + unpack_visitor_info: &UnpackVisitorInfo, + crate_name: &Ident, + ) -> Self { let RecordInfo { path, fields_unpack_error_with, @@ -28,13 +33,21 @@ impl Fragments { } = info; let fields_verification = fields_verify_with.into_iter().zip(fields_ident.iter()).map(|(verify_with, field_ident)| match verify_with { - Some(verify_with) => quote!(#verify_with::(&#field_ident, visitor).map_err(#crate_name::error::UnpackError::from_packable)?;), + Some(verify_with) => if unpack_visitor_info.provided { + quote!(#verify_with::(&#field_ident, visitor).map_err(#crate_name::error::UnpackError::from_packable)?;) + } else { + quote!(#verify_with::(&#field_ident).map_err(#crate_name::error::UnpackError::from_packable)?;) + } None => quote!(), }); let verify_with = match verify_with { Some(verify_with) => { - quote!(#verify_with::(&unpacked, visitor).map_err(#crate_name::error::UnpackError::from_packable)?;) + if unpack_visitor_info.provided { + quote!(#verify_with::(&unpacked, visitor).map_err(#crate_name::error::UnpackError::from_packable)?;) + } else { + quote!(#verify_with::(&unpacked).map_err(#crate_name::error::UnpackError::from_packable)?;) + } } None => quote!(), }; diff --git a/packable/packable-derive/src/trait_impl.rs b/packable/packable-derive/src/trait_impl.rs index b0464de..48002cc 100644 --- a/packable/packable-derive/src/trait_impl.rs +++ b/packable/packable-derive/src/trait_impl.rs @@ -29,7 +29,8 @@ impl TraitImpl { let unpack_error = info.unpack_error.unpack_error.clone().into_token_stream(); let unpack_visitor = info.unpack_visitor.unpack_visitor.clone().into_token_stream(); - let Fragments { pattern, pack, unpack } = Fragments::new(info.inner, info.verify_with, &crate_name); + let Fragments { pattern, pack, unpack } = + Fragments::new(info.inner, info.verify_with, &info.unpack_visitor, &crate_name); Ok(Self { ident: input.ident, @@ -65,7 +66,8 @@ impl TraitImpl { for (index, VariantInfo { tag, inner }) in info.variants_info.into_iter().enumerate() { let variant_ident = inner.path.segments.last().unwrap().clone(); - let Fragments { pattern, pack, unpack } = Fragments::new(inner, None, &crate_name); + let Fragments { pattern, pack, unpack } = + Fragments::new(inner, None, &info.unpack_visitor, &crate_name); // @pvdrz: The span here is very important, otherwise the compiler won't detect // unreachable patterns in the generated code for some reason. I think this is related diff --git a/packable/packable-derive/src/unpack_visitor_info.rs b/packable/packable-derive/src/unpack_visitor_info.rs index e16bfae..981f2db 100644 --- a/packable/packable-derive/src/unpack_visitor_info.rs +++ b/packable/packable-derive/src/unpack_visitor_info.rs @@ -10,6 +10,7 @@ use crate::parse::{parse_kv, skip_stream}; pub(crate) struct UnpackVisitorInfo { pub(crate) unpack_visitor: syn::Type, + pub(crate) provided: bool, } struct Type(syn::Type); @@ -34,7 +35,10 @@ impl UnpackVisitorInfo { let opt_info = attr.parse_args_with( |stream: ParseStream| match parse_kv::("unpack_visitor", stream)? { - Some(Type(unpack_visitor)) => Ok(Some(Self { unpack_visitor })), + Some(Type(unpack_visitor)) => Ok(Some(Self { + unpack_visitor, + provided: true, + })), None => { skip_stream(stream)?; Ok(None) @@ -49,6 +53,7 @@ impl UnpackVisitorInfo { Ok(Self { unpack_visitor: default_unpack_visitor(), + provided: false, }) } } diff --git a/packable/packable/src/packable/bounded.rs b/packable/packable/src/packable/bounded.rs index 4c2700e..4752584 100644 --- a/packable/packable/src/packable/bounded.rs +++ b/packable/packable/src/packable/bounded.rs @@ -44,7 +44,7 @@ macro_rules! bounded { self.0 } - fn verify(&value: &$ty, _: &()) -> Result<(), $invalid_error> { + fn verify(&value: &$ty) -> Result<(), $invalid_error> { if VERIFY && !(MIN..=MAX).contains(&value) { Err($invalid_error(value)) } else { @@ -70,7 +70,7 @@ macro_rules! bounded { type Error = $invalid_error; fn try_from(value: $ty) -> Result { - Self::verify::(&value, &())?; + Self::verify::(&value)?; Ok(Self(value)) } } From f344761efa41a4f33f62985c498f41375bd72eee Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Wed, 15 Nov 2023 14:14:26 +0100 Subject: [PATCH 02/10] Some rename --- ...ype_verify_with_field.rs => invalid_verify_with_field_type.rs} | 0 ...fy_with_field.stderr => invalid_verify_with_field_type.stderr} | 0 ...e_verify_with_struct.rs => invalid_verify_with_struct_type.rs} | 0 ..._with_struct.stderr => invalid_verify_with_struct_type.stderr} | 0 4 files changed, 0 insertions(+), 0 deletions(-) rename packable/packable-derive-test/tests/fail/{invalid_field_type_verify_with_field.rs => invalid_verify_with_field_type.rs} (100%) rename packable/packable-derive-test/tests/fail/{invalid_field_type_verify_with_field.stderr => invalid_verify_with_field_type.stderr} (100%) rename packable/packable-derive-test/tests/fail/{invalid_field_type_verify_with_struct.rs => invalid_verify_with_struct_type.rs} (100%) rename packable/packable-derive-test/tests/fail/{invalid_field_type_verify_with_struct.stderr => invalid_verify_with_struct_type.stderr} (100%) diff --git a/packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_field.rs b/packable/packable-derive-test/tests/fail/invalid_verify_with_field_type.rs similarity index 100% rename from packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_field.rs rename to packable/packable-derive-test/tests/fail/invalid_verify_with_field_type.rs diff --git a/packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_field.stderr b/packable/packable-derive-test/tests/fail/invalid_verify_with_field_type.stderr similarity index 100% rename from packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_field.stderr rename to packable/packable-derive-test/tests/fail/invalid_verify_with_field_type.stderr diff --git a/packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_struct.rs b/packable/packable-derive-test/tests/fail/invalid_verify_with_struct_type.rs similarity index 100% rename from packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_struct.rs rename to packable/packable-derive-test/tests/fail/invalid_verify_with_struct_type.rs diff --git a/packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_struct.stderr b/packable/packable-derive-test/tests/fail/invalid_verify_with_struct_type.stderr similarity index 100% rename from packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_struct.stderr rename to packable/packable-derive-test/tests/fail/invalid_verify_with_struct_type.stderr From 3b22760d45028689a5087e44d605d9046ff70455 Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Thu, 16 Nov 2023 10:17:36 +0100 Subject: [PATCH 03/10] Prepare release --- packable/packable-derive-test/Cargo.toml | 2 +- packable/packable-derive/CHANGELOG.md | 6 ++++++ packable/packable-derive/Cargo.toml | 2 +- packable/packable/CHANGELOG.md | 6 ++++++ packable/packable/Cargo.toml | 4 ++-- 5 files changed, 16 insertions(+), 4 deletions(-) diff --git a/packable/packable-derive-test/Cargo.toml b/packable/packable-derive-test/Cargo.toml index 023a507..72ca59e 100644 --- a/packable/packable-derive-test/Cargo.toml +++ b/packable/packable-derive-test/Cargo.toml @@ -16,7 +16,7 @@ name = "tests" path = "tests/lib.rs" [dev-dependencies] -packable = { version = "=0.9.0", path = "../packable", default-features = false } +packable = { version = "=0.10.0", path = "../packable", default-features = false } rustversion = { version = "1.0.14", default-features = false } trybuild = { version = "1.0.85", default-features = false, features = ["diff"] } diff --git a/packable/packable-derive/CHANGELOG.md b/packable/packable-derive/CHANGELOG.md index 1de7422..016dee6 100644 --- a/packable/packable-derive/CHANGELOG.md +++ b/packable/packable-derive/CHANGELOG.md @@ -19,6 +19,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Security --> +## 0.9.0 - 2023-11-16 + +### Changed + +- Only pass visitor to `verify_with` if it was provided; + ## 0.8.0 - 2023-11-14 ### Added diff --git a/packable/packable-derive/Cargo.toml b/packable/packable-derive/Cargo.toml index 6b60ac5..5d807b1 100644 --- a/packable/packable-derive/Cargo.toml +++ b/packable/packable-derive/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "packable-derive" -version = "0.8.0" +version = "0.9.0" authors = ["IOTA Stiftung"] edition = "2021" description = "Derive macro for the `packable` crate." diff --git a/packable/packable/CHANGELOG.md b/packable/packable/CHANGELOG.md index 5e53248..0367b34 100644 --- a/packable/packable/CHANGELOG.md +++ b/packable/packable/CHANGELOG.md @@ -19,6 +19,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Security --> +## 0.10.0 - 2023-11-16 + +### Changed + +- Only pass visitor to `verify_with` if it was provided; + ## 0.9.0 - 2023-11-14 ### Added diff --git a/packable/packable/Cargo.toml b/packable/packable/Cargo.toml index 4e0835c..be99a21 100644 --- a/packable/packable/Cargo.toml +++ b/packable/packable/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "packable" -version = "0.9.0" +version = "0.10.0" authors = ["IOTA Stiftung"] edition = "2021" description = "A crate for packing and unpacking binary representations." @@ -19,7 +19,7 @@ usize = [] autocfg = { version = "1.1.0", default-features = false } [dependencies] -packable-derive = { version = "=0.8.0", path = "../packable-derive", default-features = false } +packable-derive = { version = "=0.9.0", path = "../packable-derive", default-features = false } primitive-types = { version = "0.12.2", default-features = false, optional = true } serde = { version = "1.0.192", default-features = false, features = [ From d09c2cb3538d35c8d0191a16ef123a59ad809d2b Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Thu, 16 Nov 2023 10:19:17 +0100 Subject: [PATCH 04/10] Fix tests --- .../tests/fail/invalid_verify_with_field_type.stderr | 4 ++-- .../tests/fail/invalid_verify_with_struct_type.stderr | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packable/packable-derive-test/tests/fail/invalid_verify_with_field_type.stderr b/packable/packable-derive-test/tests/fail/invalid_verify_with_field_type.stderr index 0e6b356..389de8a 100644 --- a/packable/packable-derive-test/tests/fail/invalid_verify_with_field_type.stderr +++ b/packable/packable-derive-test/tests/fail/invalid_verify_with_field_type.stderr @@ -1,5 +1,5 @@ error[E0308]: mismatched types - --> tests/fail/invalid_field_type_verify_with_field.rs:32:10 + --> tests/fail/invalid_verify_with_field_type.rs:32:10 | 32 | #[derive(Packable)] | ^^^^^^^^ @@ -10,7 +10,7 @@ error[E0308]: mismatched types = note: expected reference `&u64` found reference `&u8` note: function defined here - --> tests/fail/invalid_field_type_verify_with_field.rs:24:4 + --> tests/fail/invalid_verify_with_field_type.rs:24:4 | 24 | fn verify_value(&value: &u64) -> Result<(), PickyError> { | ^^^^^^^^^^^^ ------------ diff --git a/packable/packable-derive-test/tests/fail/invalid_verify_with_struct_type.stderr b/packable/packable-derive-test/tests/fail/invalid_verify_with_struct_type.stderr index b5e9756..5d95333 100644 --- a/packable/packable-derive-test/tests/fail/invalid_verify_with_struct_type.stderr +++ b/packable/packable-derive-test/tests/fail/invalid_verify_with_struct_type.stderr @@ -1,5 +1,5 @@ error[E0308]: mismatched types - --> tests/fail/invalid_field_type_verify_with_struct.rs:32:10 + --> tests/fail/invalid_verify_with_struct_type.rs:32:10 | 32 | #[derive(Packable)] | ^^^^^^^^ @@ -10,7 +10,7 @@ error[E0308]: mismatched types = note: expected reference `&u64` found reference `&Picky` note: function defined here - --> tests/fail/invalid_field_type_verify_with_struct.rs:24:4 + --> tests/fail/invalid_verify_with_struct_type.rs:24:4 | 24 | fn verify(&value: &u64) -> Result<(), PickyError> { | ^^^^^^ ------------ From cd165c7b46aecf5abb006c257f49d1e6c663ef3f Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Thu, 16 Nov 2023 14:34:43 +0100 Subject: [PATCH 05/10] provided -> explicit --- packable/packable-derive/src/enum_info.rs | 4 ++-- packable/packable-derive/src/fragments.rs | 4 ++-- packable/packable-derive/src/struct_info.rs | 18 ++++++++++++++++-- .../packable-derive/src/unpack_visitor_info.rs | 12 +++++++----- 4 files changed, 27 insertions(+), 11 deletions(-) diff --git a/packable/packable-derive/src/enum_info.rs b/packable/packable-derive/src/enum_info.rs index 2c33de3..d65b42a 100644 --- a/packable/packable-derive/src/enum_info.rs +++ b/packable/packable-derive/src/enum_info.rs @@ -40,8 +40,8 @@ impl EnumInfo { .next() .and_then(|variant| variant.fields.iter().next()) { - Some(Field { ty, .. }) => parse_quote!(<#ty as #crate_name::Packable>::UnpackVisitor), - None => parse_quote!(()), + Some(Field { ty, .. }) => Ok((parse_quote!(<#ty as #crate_name::Packable>::UnpackVisitor), true)), + None => Ok((parse_quote!(()), false)), } })?; diff --git a/packable/packable-derive/src/fragments.rs b/packable/packable-derive/src/fragments.rs index 6f96e7e..077a9f7 100644 --- a/packable/packable-derive/src/fragments.rs +++ b/packable/packable-derive/src/fragments.rs @@ -33,7 +33,7 @@ impl Fragments { } = info; let fields_verification = fields_verify_with.into_iter().zip(fields_ident.iter()).map(|(verify_with, field_ident)| match verify_with { - Some(verify_with) => if unpack_visitor_info.provided { + Some(verify_with) => if unpack_visitor_info.explicit { quote!(#verify_with::(&#field_ident, visitor).map_err(#crate_name::error::UnpackError::from_packable)?;) } else { quote!(#verify_with::(&#field_ident).map_err(#crate_name::error::UnpackError::from_packable)?;) @@ -43,7 +43,7 @@ impl Fragments { let verify_with = match verify_with { Some(verify_with) => { - if unpack_visitor_info.provided { + if unpack_visitor_info.explicit { quote!(#verify_with::(&unpacked, visitor).map_err(#crate_name::error::UnpackError::from_packable)?;) } else { quote!(#verify_with::(&unpacked).map_err(#crate_name::error::UnpackError::from_packable)?;) diff --git a/packable/packable-derive/src/struct_info.rs b/packable/packable-derive/src/struct_info.rs index 8102532..eb24fb6 100644 --- a/packable/packable-derive/src/struct_info.rs +++ b/packable/packable-derive/src/struct_info.rs @@ -41,8 +41,22 @@ impl StructInfo { } let unpack_visitor = UnpackVisitorInfo::new(filtered_attrs, || match fields.iter().next() { - Some(Field { ty, .. }) => parse_quote!(<#ty as #crate_name::Packable>::UnpackVisitor), - None => parse_quote!(()), + Some(Field { attrs, ty, .. }) => { + let mut explicit = false; + for attr in attrs.clone() { + if let Some(_) = attr.parse_args_with(|stream: ParseStream| { + let opt = parse_kv::("unpack_visitor", stream)?; + if opt.is_none() { + skip_stream(stream)?; + } + Ok(opt) + })? { + explicit = true; + } + } + Ok((parse_quote!(<#ty as #crate_name::Packable>::UnpackVisitor), explicit)) + } + None => Ok((parse_quote!(()), false)), })?; let inner = RecordInfo::new(path, fields, &unpack_error.with)?; diff --git a/packable/packable-derive/src/unpack_visitor_info.rs b/packable/packable-derive/src/unpack_visitor_info.rs index 981f2db..cae64fd 100644 --- a/packable/packable-derive/src/unpack_visitor_info.rs +++ b/packable/packable-derive/src/unpack_visitor_info.rs @@ -10,7 +10,7 @@ use crate::parse::{parse_kv, skip_stream}; pub(crate) struct UnpackVisitorInfo { pub(crate) unpack_visitor: syn::Type, - pub(crate) provided: bool, + pub(crate) explicit: bool, } struct Type(syn::Type); @@ -29,7 +29,7 @@ impl Parse for Type { impl UnpackVisitorInfo { pub(crate) fn new<'a>( filtered_attrs: impl Iterator, - default_unpack_visitor: impl FnOnce() -> syn::Type, + default_unpack_visitor: impl FnOnce() -> Result<(syn::Type, bool)>, ) -> Result { for attr in filtered_attrs { let opt_info = @@ -37,7 +37,7 @@ impl UnpackVisitorInfo { |stream: ParseStream| match parse_kv::("unpack_visitor", stream)? { Some(Type(unpack_visitor)) => Ok(Some(Self { unpack_visitor, - provided: true, + explicit: true, })), None => { skip_stream(stream)?; @@ -51,9 +51,11 @@ impl UnpackVisitorInfo { } } + let (unpack_visitor, explicit) = default_unpack_visitor()?; + Ok(Self { - unpack_visitor: default_unpack_visitor(), - provided: false, + unpack_visitor, + explicit, }) } } From efba57bae37d83728bd49fb251ef7fb51cdf3021 Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Fri, 17 Nov 2023 15:28:51 +0100 Subject: [PATCH 06/10] filter_attrs --- packable/packable-derive/src/struct_info.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/packable/packable-derive/src/struct_info.rs b/packable/packable-derive/src/struct_info.rs index eb24fb6..1e6dc8a 100644 --- a/packable/packable-derive/src/struct_info.rs +++ b/packable/packable-derive/src/struct_info.rs @@ -43,14 +43,17 @@ impl StructInfo { let unpack_visitor = UnpackVisitorInfo::new(filtered_attrs, || match fields.iter().next() { Some(Field { attrs, ty, .. }) => { let mut explicit = false; - for attr in attrs.clone() { - if let Some(_) = attr.parse_args_with(|stream: ParseStream| { - let opt = parse_kv::("unpack_visitor", stream)?; - if opt.is_none() { - skip_stream(stream)?; - } - Ok(opt) - })? { + for attr in filter_attrs(attrs) { + if attr + .parse_args_with(|stream: ParseStream| { + let opt = parse_kv::("unpack_visitor", stream)?; + if opt.is_none() { + skip_stream(stream)?; + } + Ok(opt) + })? + .is_some() + { explicit = true; } } From 5c95cb693c72482337dabb3058355ef75ca502e6 Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Fri, 17 Nov 2023 15:32:18 +0100 Subject: [PATCH 07/10] Bump dates --- packable/packable-derive/CHANGELOG.md | 2 +- packable/packable/CHANGELOG.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packable/packable-derive/CHANGELOG.md b/packable/packable-derive/CHANGELOG.md index 016dee6..bd5c577 100644 --- a/packable/packable-derive/CHANGELOG.md +++ b/packable/packable-derive/CHANGELOG.md @@ -19,7 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Security --> -## 0.9.0 - 2023-11-16 +## 0.9.0 - 2023-11-17 ### Changed diff --git a/packable/packable/CHANGELOG.md b/packable/packable/CHANGELOG.md index 0367b34..d827eb4 100644 --- a/packable/packable/CHANGELOG.md +++ b/packable/packable/CHANGELOG.md @@ -19,7 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Security --> -## 0.10.0 - 2023-11-16 +## 0.10.0 - 2023-11-17 ### Changed From 89de8287af425b5a0fb81d38f27138647ba66d4d Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Fri, 17 Nov 2023 15:51:09 +0100 Subject: [PATCH 08/10] Cleanup --- packable/packable-derive/src/enum_info.rs | 13 ++++-- packable/packable-derive/src/struct_info.rs | 43 +++++++++++-------- .../src/unpack_visitor_info.rs | 9 +--- 3 files changed, 36 insertions(+), 29 deletions(-) diff --git a/packable/packable-derive/src/enum_info.rs b/packable/packable-derive/src/enum_info.rs index d65b42a..dba4738 100644 --- a/packable/packable-derive/src/enum_info.rs +++ b/packable/packable-derive/src/enum_info.rs @@ -34,15 +34,20 @@ impl EnumInfo { )?; let unpack_visitor = UnpackVisitorInfo::new(filtered_attrs, || { - match data + let (unpack_visitor, explicit) = match data .variants .iter() .next() .and_then(|variant| variant.fields.iter().next()) { - Some(Field { ty, .. }) => Ok((parse_quote!(<#ty as #crate_name::Packable>::UnpackVisitor), true)), - None => Ok((parse_quote!(()), false)), - } + Some(Field { ty, .. }) => (parse_quote!(<#ty as #crate_name::Packable>::UnpackVisitor), true), + None => (parse_quote!(()), false), + }; + + Ok(UnpackVisitorInfo { + unpack_visitor, + explicit, + }) })?; let variants_info = data diff --git a/packable/packable-derive/src/struct_info.rs b/packable/packable-derive/src/struct_info.rs index 1e6dc8a..707efb7 100644 --- a/packable/packable-derive/src/struct_info.rs +++ b/packable/packable-derive/src/struct_info.rs @@ -40,26 +40,33 @@ impl StructInfo { } } - let unpack_visitor = UnpackVisitorInfo::new(filtered_attrs, || match fields.iter().next() { - Some(Field { attrs, ty, .. }) => { - let mut explicit = false; - for attr in filter_attrs(attrs) { - if attr - .parse_args_with(|stream: ParseStream| { - let opt = parse_kv::("unpack_visitor", stream)?; - if opt.is_none() { - skip_stream(stream)?; - } - Ok(opt) - })? - .is_some() - { - explicit = true; + let unpack_visitor = UnpackVisitorInfo::new(filtered_attrs, || { + let (unpack_visitor, explicit) = match fields.iter().next() { + Some(Field { attrs, ty, .. }) => { + let mut explicit = false; + for attr in filter_attrs(attrs) { + if attr + .parse_args_with(|stream: ParseStream| { + let opt = parse_kv::("unpack_visitor", stream)?; + if opt.is_none() { + skip_stream(stream)?; + } + Ok(opt) + })? + .is_some() + { + explicit = true; + } } + (parse_quote!(<#ty as #crate_name::Packable>::UnpackVisitor), explicit) } - Ok((parse_quote!(<#ty as #crate_name::Packable>::UnpackVisitor), explicit)) - } - None => Ok((parse_quote!(()), false)), + None => (parse_quote!(()), false), + }; + + Ok(UnpackVisitorInfo { + unpack_visitor, + explicit, + }) })?; let inner = RecordInfo::new(path, fields, &unpack_error.with)?; diff --git a/packable/packable-derive/src/unpack_visitor_info.rs b/packable/packable-derive/src/unpack_visitor_info.rs index cae64fd..15c29ea 100644 --- a/packable/packable-derive/src/unpack_visitor_info.rs +++ b/packable/packable-derive/src/unpack_visitor_info.rs @@ -29,7 +29,7 @@ impl Parse for Type { impl UnpackVisitorInfo { pub(crate) fn new<'a>( filtered_attrs: impl Iterator, - default_unpack_visitor: impl FnOnce() -> Result<(syn::Type, bool)>, + default_unpack_visitor: impl FnOnce() -> Result, ) -> Result { for attr in filtered_attrs { let opt_info = @@ -51,11 +51,6 @@ impl UnpackVisitorInfo { } } - let (unpack_visitor, explicit) = default_unpack_visitor()?; - - Ok(Self { - unpack_visitor, - explicit, - }) + default_unpack_visitor() } } From c255961d1447aaf72d31aa4f5290978f9db41604 Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Fri, 17 Nov 2023 15:54:16 +0100 Subject: [PATCH 09/10] Add breaks --- packable/packable-derive/src/struct_info.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packable/packable-derive/src/struct_info.rs b/packable/packable-derive/src/struct_info.rs index 707efb7..a8c1cc3 100644 --- a/packable/packable-derive/src/struct_info.rs +++ b/packable/packable-derive/src/struct_info.rs @@ -37,6 +37,7 @@ impl StructInfo { Ok(opt) })? { verify_with_opt = Some(verify_with); + break; } } @@ -56,6 +57,7 @@ impl StructInfo { .is_some() { explicit = true; + break; } } (parse_quote!(<#ty as #crate_name::Packable>::UnpackVisitor), explicit) From 2249da658d180e63e16855a2dcc0aa55c7c26a92 Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Fri, 17 Nov 2023 16:05:21 +0100 Subject: [PATCH 10/10] Clippy --- packable/packable-derive/src/struct_info.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packable/packable-derive/src/struct_info.rs b/packable/packable-derive/src/struct_info.rs index a8c1cc3..0775b09 100644 --- a/packable/packable-derive/src/struct_info.rs +++ b/packable/packable-derive/src/struct_info.rs @@ -45,8 +45,9 @@ impl StructInfo { let (unpack_visitor, explicit) = match fields.iter().next() { Some(Field { attrs, ty, .. }) => { let mut explicit = false; + for attr in filter_attrs(attrs) { - if attr + explicit = attr .parse_args_with(|stream: ParseStream| { let opt = parse_kv::("unpack_visitor", stream)?; if opt.is_none() { @@ -54,12 +55,12 @@ impl StructInfo { } Ok(opt) })? - .is_some() - { - explicit = true; + .is_some(); + if explicit { break; } } + (parse_quote!(<#ty as #crate_name::Packable>::UnpackVisitor), explicit) } None => (parse_quote!(()), false),