atomicDEX-API icon indicating copy to clipboard operation
atomicDEX-API copied to clipboard

enhancement(derives): extend enum macro to accept any ident and refactor enums

Open borngraced opened this issue 10 months ago • 7 comments

#1595 Previously, the EnumFromStringy macro was exclusively utilized to produce the From<T> trait from one enum to another enum only when the inner identifier was "String". However, to enhance this utility, I've expanded the macro's functionality to allow any identifier.

This will eliminates the need for code repetition such as the following while decreasing the codebase to some extent :) :

impl From<rlp::DecoderError> for ValidatePaymentError {
    fn from(err: rlp::DecoderError) -> Self { Self::TxDeserializationError(err.to_string()) }
}

impl From<web3::Error> for ValidatePaymentError {
    fn from(err: web3::Error) -> Self { Self::Transport(err.to_string()) }
}

impl From<NumConversError> for ValidatePaymentError {
    fn from(err: NumConversError) -> Self { Self::InternalError(err.to_string()) }
}

impl From<SPVError> for ValidatePaymentError {
    fn from(err: SPVError) -> Self { Self::SPVError(err) }
}

impl From<serialization::Error> for ValidatePaymentError {
    fn from(err: serialization::Error) -> Self { Self::TxDeserializationError(err.to_string()) }
}

impl From<UnexpectedDerivationMethod> for ValidatePaymentError {
    fn from(err: UnexpectedDerivationMethod) -> Self { Self::InternalError(err.to_string()) }
}


And just do this instead 


pub enum ValidatePaymentError {
    /// Should be used to indicate internal MM2 state problems (e.g., DB errors, etc.).
    #[from_stringify("NumConversError", "UnexpectedDerivationMethod", "keys::Error")]
    InternalError(String),
    /// Problem with deserializing the transaction, or one of the transaction parts is invalid.
    #[from_stringify("rlp::DecoderError", "serialization::Error")]
    TxDeserializationError(String),
    /// SPV client error.
    #[from_stringify("SPVError")]
    SPVError(SPVError),
    …
}


So it doesn’t matter if a field inner ident is string or not

borngraced avatar Mar 28 '24 12:03 borngraced

btw there is a similar feature https://docs.rs/thiserror/latest/thiserror/. An interesting feature in it is formatting error message with interpolating nested error

dimxy avatar Mar 31 '24 05:03 dimxy

btw there is a similar feature https://docs.rs/thiserror/latest/thiserror/. An interesting feature in it is formatting error message with interpolating nested error

seems cool really, but it still depends on anyhow error crate. we might want to keep this and keep extending I guess

borngraced avatar Mar 31 '24 19:03 borngraced

@borngraced please resolve conflicts in this PR.

shamardy avatar May 06 '24 14:05 shamardy

I tried EnumFromStringify with the new option, like:

#[from_stringify("SPVError")]
    SPVError(SPVError),

I think this may be preferable, because when printing the outer error in the debug format this option prints the nested error in debug as well. How about from_nested or just from?

Question: why the macro is called 'from_stringify' although there is no 'stringify' in the new option? Also: maybe good to mention when #[from_stringify("SPVError")] SPVError(String), is used (with conversion to String) then nested SPVError must implement Display trait.

dimxy avatar Jul 04 '24 14:07 dimxy

BTW the function derive_enum_from_macro in the unsupported case of 'struct' or 'union' always returns ENUM_FROM_INNER_IDENT tag in the error, although derive_enum_from_macro fn could be also called for EnumFromStringify and EnumFromTrait

dimxy avatar Jul 04 '24 16:07 dimxy

Also I think you need to update the doc for EnumFromStringify:

  • This note // E.G, this converts from Bar, Man to FooBar::Bar(String) does not fully match to the example code (#[from_stringify("u64", "std::io::Error")]).
  • Please, add a sample for the new feature with non-String nested types.

There are identical doc comments /// Implements `From<Inner>` trait for the given enumeration both for EnumFromInner and EnumFromTrait macros. Should the second one be changed for EnumFromTrait?

dimxy avatar Jul 05 '24 18:07 dimxy

Also I think you need to update the doc for EnumFromStringify:

  • This note // E.G, this converts from Bar, Man to FooBar::Bar(String) does not fully match to the example code (#[from_stringify("u64", "std::io::Error")]).
  • Please, add a sample for the new feature with non-String nested types.

There are identical doc comments /// Implements `From<Inner>` trait for the given enumeration both for EnumFromInner and EnumFromTrait macros. Should the second one be changed for EnumFromTrait?

updated

borngraced avatar Jul 08 '24 15:07 borngraced