packable: only pass visitor to verify_with if it was provided

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"] }

packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_field.rs → packable/packable-derive-test/tests/fail/invalid_verify_with_field_type.rs

@@ -21,7 +21,7 @@ impl From<Infallible> for PickyError {
     }
 }
 
-fn verify_value<const VERIFY: bool>(&value: &u64, _: &()) -> Result<(), PickyError> {
+fn verify_value<const VERIFY: bool>(&value: &u64) -> Result<(), PickyError> {
     if !VERIFY || value == 42 {
         Ok(())
     } else {

packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_field.stderr → 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,8 +10,8 @@ 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<const VERIFY: bool>(&value: &u64, _: &()) -> Result<(), PickyError> {
+24 | fn verify_value<const VERIFY: bool>(&value: &u64) -> Result<(), PickyError> {
    |    ^^^^^^^^^^^^                     ------------
    = note: this error originates in the derive macro `Packable` (in Nightly builds, run with -Z macro-backtrace for more info)

packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_struct.rs → packable/packable-derive-test/tests/fail/invalid_verify_with_struct_type.rs

@@ -21,7 +21,7 @@ impl From<Infallible> for PickyError {
     }
 }
 
-fn verify<const VERIFY: bool>(&value: &u64, _: &()) -> Result<(), PickyError> {
+fn verify<const VERIFY: bool>(&value: &u64) -> Result<(), PickyError> {
     if !VERIFY || value == 42 {
         Ok(())
     } else {

packable/packable-derive-test/tests/fail/invalid_field_type_verify_with_struct.stderr → 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,8 +10,8 @@ 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<const VERIFY: bool>(&value: &u64, _: &()) -> Result<(), PickyError> {
+24 | fn verify<const VERIFY: bool>(&value: &u64) -> Result<(), PickyError> {
    |    ^^^^^^                     ------------
    = note: this error originates in the derive macro `Packable` (in Nightly builds, run with -Z macro-backtrace for more info)

packable/packable-derive-test/tests/pass/verify_with_field.rs

@@ -21,7 +21,7 @@ impl From<Infallible> for PickyError {
     }
 }
 
-fn verify_value<const VERIFY: bool>(&value: &u8, _: &()) -> Result<(), PickyError> {
+fn verify_value<const VERIFY: bool>(&value: &u8) -> Result<(), PickyError> {
     if !VERIFY || value == 42 {
         Ok(())
     } else {

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<Infallible> for PickyError {
+    fn from(err: Infallible) -> Self {
+        match err {}
+    }
+}
+
+fn verify_value<const VERIFY: bool>(&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() {}

packable/packable-derive-test/tests/pass/verify_with_struct.rs

@@ -21,7 +21,7 @@ impl From<Infallible> for PickyError {
     }
 }
 
-fn verify<const VERIFY: bool>(value: &Picky, _: &()) -> Result<(), PickyError> {
+fn verify<const VERIFY: bool>(value: &Picky) -> Result<(), PickyError> {
     if !VERIFY || value.0 == 42 {
         Ok(())
     } else {

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<Infallible> for PickyError {
+    fn from(err: Infallible) -> Self {
+        match err {}
+    }
+}
+
+fn verify<const VERIFY: bool>(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() {}

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-17
+
+### Changed
+
+- Only pass visitor to `verify_with` if it was provided;
+
 ## 0.8.0 - 2023-11-14
 
 ### Added

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."

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, .. }) => parse_quote!(<#ty as #crate_name::Packable>::UnpackVisitor),
-                None => parse_quote!(()),
-            }
+                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

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<Path>, crate_name: &Ident) -> Self {
+    pub(crate) fn new(
+        info: RecordInfo,
+        verify_with: Option<Path>,
+        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::<VERIFY>(&#field_ident, visitor).map_err(#crate_name::error::UnpackError::from_packable)?;),
+            Some(verify_with) => if unpack_visitor_info.explicit {
+                quote!(#verify_with::<VERIFY>(&#field_ident, visitor).map_err(#crate_name::error::UnpackError::from_packable)?;)
+            } else {
+                quote!(#verify_with::<VERIFY>(&#field_ident).map_err(#crate_name::error::UnpackError::from_packable)?;)
+            }
             None => quote!(),
         });
 
         let verify_with = match verify_with {
             Some(verify_with) => {
-                quote!(#verify_with::<VERIFY>(&unpacked, visitor).map_err(#crate_name::error::UnpackError::from_packable)?;)
+                if unpack_visitor_info.explicit {
+                    quote!(#verify_with::<VERIFY>(&unpacked, visitor).map_err(#crate_name::error::UnpackError::from_packable)?;)
+                } else {
+                    quote!(#verify_with::<VERIFY>(&unpacked).map_err(#crate_name::error::UnpackError::from_packable)?;)
+                }
             }
             None => quote!(),
         };

packable/packable-derive/src/struct_info.rs

@@ -37,12 +37,39 @@ impl StructInfo {
                 Ok(opt)
             })? {
                 verify_with_opt = Some(verify_with);
+                break;
             }
         }
 
-        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!(()),
+        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) {
+                        explicit = attr
+                            .parse_args_with(|stream: ParseStream| {
+                                let opt = parse_kv::<Path>("unpack_visitor", stream)?;
+                                if opt.is_none() {
+                                    skip_stream(stream)?;
+                                }
+                                Ok(opt)
+                            })?
+                            .is_some();
+                        if explicit {
+                            break;
+                        }
+                    }
+
+                    (parse_quote!(<#ty as #crate_name::Packable>::UnpackVisitor), explicit)
+                }
+                None => (parse_quote!(()), false),
+            };
+
+            Ok(UnpackVisitorInfo {
+                unpack_visitor,
+                explicit,
+            })
         })?;
 
         let inner = RecordInfo::new(path, fields, &unpack_error.with)?;

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

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) explicit: bool,
 }
 
 struct Type(syn::Type);
@@ -28,13 +29,16 @@ impl Parse for Type {
 impl UnpackVisitorInfo {
     pub(crate) fn new<'a>(
         filtered_attrs: impl Iterator<Item = &'a Attribute>,
-        default_unpack_visitor: impl FnOnce() -> syn::Type,
+        default_unpack_visitor: impl FnOnce() -> Result<UnpackVisitorInfo>,
     ) -> Result<Self> {
         for attr in filtered_attrs {
             let opt_info =
                 attr.parse_args_with(
                     |stream: ParseStream| match parse_kv::<Type>("unpack_visitor", stream)? {
-                        Some(Type(unpack_visitor)) => Ok(Some(Self { unpack_visitor })),
+                        Some(Type(unpack_visitor)) => Ok(Some(Self {
+                            unpack_visitor,
+                            explicit: true,
+                        })),
                         None => {
                             skip_stream(stream)?;
                             Ok(None)
@@ -47,8 +51,6 @@ impl UnpackVisitorInfo {
             }
         }
 
-        Ok(Self {
-            unpack_visitor: default_unpack_visitor(),
-        })
+        default_unpack_visitor()
     }
 }

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-17
+
+### Changed
+
+- Only pass visitor to `verify_with` if it was provided;
+
 ## 0.9.0 - 2023-11-14
 
 ### Added

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 = [

packable/packable/src/packable/bounded.rs

@@ -44,7 +44,7 @@ macro_rules! bounded {
                 self.0
             }
 
-            fn verify<const VERIFY: bool>(&value: &$ty, _: &()) -> Result<(), $invalid_error<MIN, MAX>> {
+            fn verify<const VERIFY: bool>(&value: &$ty) -> Result<(), $invalid_error<MIN, MAX>> {
                 if VERIFY && !(MIN..=MAX).contains(&value) {
                     Err($invalid_error(value))
                 } else {
@@ -70,7 +70,7 @@ macro_rules! bounded {
             type Error = $invalid_error<MIN, MAX>;
 
             fn try_from(value: $ty) -> Result<Self, Self::Error> {
-                Self::verify::<true>(&value, &())?;
+                Self::verify::<true>(&value)?;
                 Ok(Self(value))
             }
         }