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

Hardcode error type in IntoVisitor #41

Merged
merged 9 commits into from
Nov 10, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
/target
/Cargo.lock
.DS_Store
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ members = [
"scale-decode-derive",
"testing/no_std",
]
resolver = "2"

[workspace.package]
version = "0.9.0"
Expand Down
7 changes: 7 additions & 0 deletions scale-decode/src/error/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,13 @@ impl From<DecodeError> for Error {
}
}

impl From<codec::Error> for Error {
fn from(err: codec::Error) -> Error {
let err: DecodeError = err.into();
Error::new(err.into())
}
}

/// The underlying nature of the error.
#[derive(Debug, derive_more::From, derive_more::Display)]
pub enum ErrorKind {
Expand Down
57 changes: 15 additions & 42 deletions scale-decode/src/impls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,6 @@ macro_rules! impl_decode_seq_via_collect {
impl <$generic> Visitor for BasicVisitor<$ty<$generic>>
where
$generic: IntoVisitor,
Error: From<<$generic::Visitor as Visitor>::Error>,
$( $($where)* )?
{
type Value<'scale, 'info> = $ty<$generic>;
Expand Down Expand Up @@ -306,11 +305,7 @@ macro_rules! array_method_impl {
Ok(arr)
}};
}
impl<const N: usize, T> Visitor for BasicVisitor<[T; N]>
where
T: IntoVisitor,
Error: From<<T::Visitor as Visitor>::Error>,
{
impl<const N: usize, T: IntoVisitor> Visitor for BasicVisitor<[T; N]> {
type Value<'scale, 'info> = [T; N];
type Error = Error;

Expand All @@ -331,22 +326,14 @@ where

visit_single_field_composite_tuple_impls!();
}
impl<const N: usize, T> IntoVisitor for [T; N]
where
T: IntoVisitor,
Error: From<<T::Visitor as Visitor>::Error>,
{
impl<const N: usize, T: IntoVisitor> IntoVisitor for [T; N] {
type Visitor = BasicVisitor<[T; N]>;
fn into_visitor() -> Self::Visitor {
BasicVisitor { _marker: core::marker::PhantomData }
}
}

impl<T> Visitor for BasicVisitor<BTreeMap<String, T>>
where
T: IntoVisitor,
Error: From<<T::Visitor as Visitor>::Error>,
{
impl<T: IntoVisitor> Visitor for BasicVisitor<BTreeMap<String, T>> {
type Error = Error;
type Value<'scale, 'info> = BTreeMap<String, T>;

Expand All @@ -365,19 +352,15 @@ where
// Decode the value now that we have a valid name.
let Some(val) = value.decode_item(T::into_visitor()) else { break };
// Save to the map.
let val = val.map_err(|e| Error::from(e).at_field(key.to_owned()))?;
let val = val.map_err(|e| e.at_field(key.to_owned()))?;
map.insert(key.to_owned(), val);
}
Ok(map)
}
}
impl_into_visitor!(BTreeMap<String, T>);

impl<T> Visitor for BasicVisitor<Option<T>>
where
T: IntoVisitor,
Error: From<<T::Visitor as Visitor>::Error>,
{
impl<T: IntoVisitor> Visitor for BasicVisitor<Option<T>> {
type Error = Error;
type Value<'scale, 'info> = Option<T>;

Expand All @@ -391,7 +374,7 @@ where
.fields()
.decode_item(T::into_visitor())
.transpose()
.map_err(|e| Error::from(e).at_variant("Some"))?
.map_err(|e| e.at_variant("Some"))?
.expect("checked for 1 field already so should be ok");
Ok(Some(val))
} else if value.name() == "None" && value.fields().remaining() == 0 {
Expand All @@ -407,13 +390,7 @@ where
}
impl_into_visitor!(Option<T>);

impl<T, E> Visitor for BasicVisitor<Result<T, E>>
where
T: IntoVisitor,
Error: From<<T::Visitor as Visitor>::Error>,
E: IntoVisitor,
Error: From<<E::Visitor as Visitor>::Error>,
{
impl<T: IntoVisitor, E: IntoVisitor> Visitor for BasicVisitor<Result<T, E>> {
type Error = Error;
type Value<'scale, 'info> = Result<T, E>;

Expand All @@ -427,15 +404,15 @@ where
.fields()
.decode_item(T::into_visitor())
.transpose()
.map_err(|e| Error::from(e).at_variant("Ok"))?
.map_err(|e| e.at_variant("Ok"))?
.expect("checked for 1 field already so should be ok");
Ok(Ok(val))
} else if value.name() == "Err" && value.fields().remaining() == 1 {
let val = value
.fields()
.decode_item(E::into_visitor())
.transpose()
.map_err(|e| Error::from(e).at_variant("Err"))?
.map_err(|e| e.at_variant("Err"))?
.expect("checked for 1 field already so should be ok");
Ok(Err(val))
} else {
Expand Down Expand Up @@ -541,7 +518,7 @@ macro_rules! tuple_method_impl {
let v = $value
.decode_item($t::into_visitor())
.transpose()
.map_err(|e| Error::from(e).at_idx(idx))?
.map_err(|e| e.at_idx(idx))?
.expect("length already checked via .remaining()");
idx += 1;
v
Expand Down Expand Up @@ -593,7 +570,6 @@ macro_rules! impl_decode_tuple {
impl < $($t),* > Visitor for BasicVisitor<($($t,)*)>
where $(
$t: IntoVisitor,
Error: From<<$t::Visitor as Visitor>::Error>,
)*
{
type Value<'scale, 'info> = ($($t,)*);
Expand Down Expand Up @@ -621,7 +597,7 @@ macro_rules! impl_decode_tuple {

// We can turn this tuple into a visitor which knows how to decode it:
impl < $($t),* > IntoVisitor for ($($t,)*)
where $( $t: IntoVisitor, Error: From<<$t::Visitor as Visitor>::Error>, )*
where $( $t: IntoVisitor, )*
{
type Visitor = BasicVisitor<($($t,)*)>;
fn into_visitor() -> Self::Visitor {
Expand All @@ -631,7 +607,7 @@ macro_rules! impl_decode_tuple {

// We can decode given a list of fields (just delegate to the visitor impl:
impl < $($t),* > DecodeAsFields for ($($t,)*)
where $( $t: IntoVisitor, Error: From<<$t::Visitor as Visitor>::Error>, )*
where $( $t: IntoVisitor, )*
{
fn decode_as_fields<'info>(input: &mut &[u8], fields: &mut dyn FieldIter<'info>, types: &'info scale_info::PortableRegistry) -> Result<Self, Error> {
let mut composite = crate::visitor::types::Composite::new(input, crate::EMPTY_SCALE_INFO_PATH, fields, types, false);
Expand Down Expand Up @@ -676,14 +652,11 @@ fn decode_items_using<'a, 'scale, 'info, D: DecodeItemIterator<'scale, 'info>, T
) -> impl Iterator<Item = Result<T, Error>> + 'a
where
T: IntoVisitor,
Error: From<<T::Visitor as Visitor>::Error>,
D: DecodeItemIterator<'scale, 'info>,
{
let mut idx = 0;
core::iter::from_fn(move || {
let item = decoder
.decode_item(T::into_visitor())
.map(|res| res.map_err(|e| Error::from(e).at_idx(idx)));
let item = decoder.decode_item(T::into_visitor()).map(|res| res.map_err(|e| e.at_idx(idx)));
idx += 1;
item
})
Expand All @@ -710,7 +683,7 @@ mod test {
fn assert_encode_decode_to_with<T, A, B>(a: &A, b: &B)
where
A: Encode,
B: DecodeAsType + PartialEq + core::fmt::Debug,
B: IntoVisitor + PartialEq + core::fmt::Debug,
T: scale_info::TypeInfo + 'static,
{
let (type_id, types) = make_type::<T>();
Expand All @@ -724,7 +697,7 @@ mod test {
fn assert_encode_decode_to<A, B>(a: &A, b: &B)
where
A: Encode + scale_info::TypeInfo + 'static,
B: DecodeAsType + PartialEq + core::fmt::Debug,
B: IntoVisitor + PartialEq + core::fmt::Debug,
{
assert_encode_decode_to_with::<A, A, B>(a, b);
}
Expand Down
19 changes: 10 additions & 9 deletions scale-decode/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ use alloc::vec::Vec;
/// This trait is implemented for any type `T` where `T` implements [`IntoVisitor`] and the errors returned
/// from this [`Visitor`] can be converted into [`Error`]. It's essentially a convenience wrapper around
/// [`visitor::decode_with_visitor`] that mirrors `scale-encode`'s `EncodeAsType`.
pub trait DecodeAsType: Sized {
pub trait DecodeAsType: Sized + IntoVisitor {
/// Given some input bytes, a `type_id`, and type registry, attempt to decode said bytes into
/// `Self`. Implementations should modify the `&mut` reference to the bytes such that any bytes
/// not used in the course of decoding are still pointed to after decoding is complete.
Expand All @@ -192,11 +192,7 @@ pub trait DecodeAsType: Sized {
) -> Result<Self, Error>;
}

impl<T> DecodeAsType for T
where
T: IntoVisitor,
Error: From<<T::Visitor as Visitor>::Error>,
{
impl<T: Sized + IntoVisitor> DecodeAsType for T {
fn decode_as_type_maybe_compact(
input: &mut &[u8],
type_id: u32,
Expand Down Expand Up @@ -267,11 +263,16 @@ pub trait FieldIter<'a>: Iterator<Item = Field<'a>> {}
impl<'a, T> FieldIter<'a> for T where T: Iterator<Item = Field<'a>> {}

/// This trait can be implemented on any type that has an associated [`Visitor`] responsible for decoding
/// SCALE encoded bytes to it. If you implement this on some type and the [`Visitor`] that you return has
/// an error type that converts into [`Error`], then you'll also get a [`DecodeAsType`] implementation for free.
/// SCALE encoded bytes to it whose error type is [`Error`]. Anything that implements this trait gets a
/// [`DecodeAsType`] implementation for free.
// Dev note: This used to allow for any Error type that could be converted into `scale_decode::Error`.
// The problem with this is that the `DecodeAsType` trait became tricky to use in some contexts, because it
// didn't automatically imply so much. Realistically, being stricter here shouldn't matter too much; derive
// impls all use `scale_decode::Error` anyway, and manual impls can just manually convert into the error
// rather than rely on auto conversion, if they care about also being able to impl `DecodeAsType`.
pub trait IntoVisitor {
/// The visitor type used to decode SCALE encoded bytes to `Self`.
type Visitor: for<'scale, 'info> visitor::Visitor<Value<'scale, 'info> = Self>;
type Visitor: for<'scale, 'info> visitor::Visitor<Value<'scale, 'info> = Self, Error = Error>;
/// A means of obtaining this visitor.
fn into_visitor() -> Self::Visitor;
}
Expand Down
Loading
Loading