feat: remove proc-macro-error3 in favor of syn::Error - #90
Conversation
Signed-off-by: Antoine Lavandier <antoine.lavandier@inria.fr>
Signed-off-by: Antoine Lavandier <antoine.lavandier@inria.fr>
| let ident = attr | ||
| .path() | ||
| .get_ident() | ||
| .map(ToString::to_string) | ||
| .unwrap_or_default(); |
There was a problem hiding this comment.
Here I decided to use the fact that match statement below has a _ => None to use .map(ToString::to_string).unwrap_or_default().
| impl TimeoutAttribute { | ||
| fn from_attr(attr: &Attribute) -> Self { | ||
| fn from_attr(attr: &Attribute) -> syn::Result<Self> { | ||
| match attr.parse_args::<TimeoutAttribute>() { | ||
| Ok(timeout_attr) => timeout_attr, | ||
| Ok(timeout_attr) => Ok(timeout_attr), | ||
| Err(e) => { | ||
| abort!( | ||
| attr, | ||
| "failed to parse `timeout` attribute. Must be of the form #[timeout(10)] where 10 is the timeout in seconds. Error: {}", | ||
| e | ||
| ); | ||
| Err(syn::Error::new( | ||
| attr.span(), | ||
| format!( | ||
| "failed to parse `timeout` attribute. Must be of the form #[timeout(10)] where 10 is the timeout in seconds. Error: {}", | ||
| e | ||
| ))) |
There was a problem hiding this comment.
This function is a good case study of what the majority of the changes look like. Functions that could previously panic! using abort!(x, y) have heir signature changed to syn::Result<> and the previously panicking path returns syn::Err::new(x.span(), y) instead.
| impl TryFrom<ItemFn> for FunctionWithAttributes { | ||
| type Error = syn::Error; | ||
|
|
||
| fn try_from(mut func: ItemFn) -> syn::Result<Self> { |
There was a problem hiding this comment.
Here since I need to output syn::Result<> the From implementations that could panic have been changed to TryFrom<Error = syn::Error> impls.
|
|
||
| impl MacroArgs { | ||
| pub(crate) fn parse(args: TokenStream) -> Result<Self, syn::Error> { | ||
| pub(crate) fn parse(args: TokenStream) -> syn::Result<Self> { |
There was a problem hiding this comment.
This change was done for consistency since syn::Result<X> is now common across the codebase.
| && self.macro_args.executor.is_some() | ||
| { | ||
| abort_call_site!( | ||
| return Err(syn::Error::new(Span::call_site(), |
There was a problem hiding this comment.
abort_call_site!(Y) are replaced with syn::Error::new(Span::call_site(), Y)
1 participant
This PRs removes instances of
proc-macro-error3sabort!andabort_call_site!and replaces them with the use ofsyn::Resutandsyn::Error.Why am I doing this
proc-macro-error3was put there as a drop-in replacement forproc-macro-error2which is unmaintained and contains code will be rejected from futures versions of the compiler.While nothing at the moment leads me to believe that
proc-macro-error3will ever contain such code, it is still way more likely forproc-macro-error3to become unmaintained over time than it is forsyn. There also aren't really reasons to use it whensynhas the necessary functionality and needs to be used inproc-macrocrates anyway.Dropping
proc-macro-error3will makeembedded-test:proc-macro-error2proc-macro-error3has any ill intentions, people needed to switch quickly from one unmaintained is the perfect storm for someone willing to fork said crate and play the long game. Again I'm not accusing anybody of anything but as a downstream user ofembedded-testthrough Ariel OS I worry about supply-chain attacks and removingproc-macro-error3remove one such vector.The solution proposed in this PR is the one chosen by other libs in the embedded ecosystem like
pio-rsrp-rs/pio-rs#85 anddefmtknurling-rs/defmt#1070Testing
Successfully ran :
embedded-testandembedded-test-macroscd tests-build && cargo test -- --test-threads 1