From f18a2a605d3487939271bd9113a96e9a798249e2 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 12 Oct 2019 15:31:51 -0700 Subject: [PATCH 1/5] Support Option<$source> --- impl/src/expand.rs | 73 +++++++++++++++++++++++++++++++++++++--------- impl/src/prop.rs | 16 +++++----- 2 files changed, 68 insertions(+), 21 deletions(-) diff --git a/impl/src/expand.rs b/impl/src/expand.rs index 25c50c8b..1f0c8d26 100644 --- a/impl/src/expand.rs +++ b/impl/src/expand.rs @@ -3,7 +3,7 @@ use crate::valid; use proc_macro2::TokenStream; use quote::{format_ident, quote, quote_spanned}; use syn::spanned::Spanned; -use syn::{DeriveInput, Member, Result}; +use syn::{DeriveInput, Member, PathArguments, Result, Type}; pub fn derive(node: &DeriveInput) -> Result { let input = Input::from_syn(node)?; @@ -18,8 +18,14 @@ fn impl_struct(input: Struct) -> TokenStream { let ty = &input.ident; let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); - let source_method = input.source_member().map(|source| { - let dyn_error = quote_spanned!(source.span()=> self.#source.as_dyn_error()); + let source_method = input.source_field().map(|source_field| { + let source = &source_field.member; + let asref = if type_is_option(source_field.ty) { + Some(quote_spanned!(source.span()=> .as_ref()?)) + } else { + None + }; + let dyn_error = quote_spanned!(source.span()=> self.#source #asref.as_dyn_error()); quote! { fn source(&self) -> std::option::Option<&(dyn std::error::Error + 'static)> { use thiserror::private::AsDynError; @@ -30,11 +36,20 @@ fn impl_struct(input: Struct) -> TokenStream { let backtrace_method = input.backtrace_field().map(|backtrace| { let backtrace = &backtrace.member; - let body = if let Some(source) = input.source_member() { - let dyn_error = quote_spanned!(source.span()=> self.#source.as_dyn_error()); + let body = if let Some(source_field) = input.source_field() { + let source = &source_field.member; + let source_backtrace = if type_is_option(source_field.ty) { + quote_spanned! {source.span()=> + self.#source.as_ref().and_then(|source| source.as_dyn_error().backtrace()) + } + } else { + quote_spanned! {source.span()=> + self.#source.as_dyn_error().backtrace() + } + }; quote!({ use thiserror::private::AsDynError; - #dyn_error.backtrace().unwrap_or(&self.#backtrace) + #source_backtrace.unwrap_or(&self.#backtrace) }) } else { quote! { @@ -77,9 +92,15 @@ fn impl_enum(input: Enum) -> TokenStream { let source_method = if input.has_source() { let arms = input.variants.iter().map(|variant| { let ident = &variant.ident; - match variant.source_member() { - Some(source) => { - let dyn_error = quote_spanned!(source.span()=> source.as_dyn_error()); + match variant.source_field() { + Some(source_field) => { + let source = &source_field.member; + let asref = if type_is_option(source_field.ty) { + Some(quote_spanned!(source.span()=> .as_ref()?)) + } else { + None + }; + let dyn_error = quote_spanned!(source.span()=> source #asref.as_dyn_error()); quote! { #ty::#ident {#source: source, ..} => std::option::Option::Some(#dyn_error), } @@ -104,10 +125,19 @@ fn impl_enum(input: Enum) -> TokenStream { let backtrace_method = if input.has_backtrace() { let arms = input.variants.iter().map(|variant| { let ident = &variant.ident; - match (variant.backtrace_field(), variant.source_member()) { - (Some(backtrace), Some(source)) if backtrace.attrs.backtrace.is_none() => { + match (variant.backtrace_field(), variant.source_field()) { + (Some(backtrace), Some(source_field)) if backtrace.attrs.backtrace.is_none() => { let backtrace = &backtrace.member; - let dyn_error = quote_spanned!(source.span()=> source.as_dyn_error()); + let source = &source_field.member; + let source_backtrace = if type_is_option(source_field.ty) { + quote_spanned! {source.span()=> + source.as_ref().and_then(|source| source.as_dyn_error().backtrace()) + } + } else { + quote_spanned! {source.span()=> + source.as_dyn_error().backtrace() + } + }; quote! { #ty::#ident { #backtrace: backtrace, @@ -115,7 +145,7 @@ fn impl_enum(input: Enum) -> TokenStream { .. } => std::option::Option::Some({ use thiserror::private::AsDynError; - #dyn_error.backtrace().unwrap_or(backtrace) + #source_backtrace.unwrap_or(backtrace) }), } } @@ -192,3 +222,20 @@ fn fields_pat(fields: &[Field]) -> TokenStream { None => quote!({}), } } + +fn type_is_option(ty: &Type) -> bool { + let path = match ty { + Type::Path(ty) => &ty.path, + _ => return false, + }; + + let last = path.segments.last().unwrap(); + if last.ident != "Option" { + return false; + } + + match &last.arguments { + PathArguments::AngleBracketed(bracketed) => bracketed.args.len() == 1, + _ => false, + } +} diff --git a/impl/src/prop.rs b/impl/src/prop.rs index 240de8c1..56b6108c 100644 --- a/impl/src/prop.rs +++ b/impl/src/prop.rs @@ -2,8 +2,8 @@ use crate::ast::{Enum, Field, Struct, Variant}; use syn::{Member, Type}; impl Struct<'_> { - pub(crate) fn source_member(&self) -> Option<&Member> { - source_member(&self.fields) + pub(crate) fn source_field(&self) -> Option<&Field> { + source_field(&self.fields) } pub(crate) fn backtrace_field(&self) -> Option<&Field> { @@ -15,7 +15,7 @@ impl Enum<'_> { pub(crate) fn has_source(&self) -> bool { self.variants .iter() - .any(|variant| variant.source_member().is_some()) + .any(|variant| variant.source_field().is_some()) } pub(crate) fn has_backtrace(&self) -> bool { @@ -34,8 +34,8 @@ impl Enum<'_> { } impl Variant<'_> { - pub(crate) fn source_member(&self) -> Option<&Member> { - source_member(&self.fields) + pub(crate) fn source_field(&self) -> Option<&Field> { + source_field(&self.fields) } pub(crate) fn backtrace_field(&self) -> Option<&Field> { @@ -43,15 +43,15 @@ impl Variant<'_> { } } -fn source_member<'a>(fields: &'a [Field]) -> Option<&'a Member> { +fn source_field<'a, 'b>(fields: &'a [Field<'b>]) -> Option<&'a Field<'b>> { for field in fields { if field.attrs.source.is_some() { - return Some(&field.member); + return Some(&field); } } for field in fields { match &field.member { - Member::Named(ident) if ident == "source" => return Some(&field.member), + Member::Named(ident) if ident == "source" => return Some(&field), _ => {} } } From 30b1561f65e92c71720769aa30362423cf89f1b0 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 12 Oct 2019 15:53:54 -0700 Subject: [PATCH 2/5] Add test for optional source --- .travis.yml | 3 ++- tests/test_option.rs | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 tests/test_option.rs diff --git a/.travis.yml b/.travis.yml index 274c9cc3..6c4e3ea8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,6 @@ language: rust rust: - - nightly - beta - stable - 1.34.0 @@ -10,6 +9,8 @@ script: cargo test matrix: include: + - rust: nightly + env: RUSTFLAGS='--cfg thiserror_nightly_testing' - rust: 1.31.0 script: cargo check - rust: nightly diff --git a/tests/test_option.rs b/tests/test_option.rs new file mode 100644 index 00000000..936f3e63 --- /dev/null +++ b/tests/test_option.rs @@ -0,0 +1,39 @@ +#![cfg(thiserror_nightly_testing)] +#![feature(backtrace)] + +use std::backtrace::Backtrace; +use thiserror::Error; + +#[derive(Error, Debug)] +#[error("...")] +pub struct OptSourceNoBacktraceStruct { + #[source] + source: Option, +} + +#[derive(Error, Debug)] +#[error("...")] +pub struct OptSourceAlwaysBacktraceStruct { + #[source] + source: Option, + backtrace: Backtrace, +} + +#[derive(Error, Debug)] +pub enum OptSourceNoBacktraceEnum { + #[error("...")] + Test { + #[source] + source: Option, + }, +} + +#[derive(Error, Debug)] +pub enum OptSourceAlwaysBacktraceEnum { + #[error("...")] + Test { + #[source] + source: Option, + backtrace: Backtrace, + }, +} From fac0a7aa0d750049b82a0ede61d33e766eb6ac80 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 12 Oct 2019 15:59:21 -0700 Subject: [PATCH 3/5] Support Option --- impl/src/expand.rs | 59 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 15 deletions(-) diff --git a/impl/src/expand.rs b/impl/src/expand.rs index 1f0c8d26..42c0e865 100644 --- a/impl/src/expand.rs +++ b/impl/src/expand.rs @@ -34,8 +34,8 @@ fn impl_struct(input: Struct) -> TokenStream { } }); - let backtrace_method = input.backtrace_field().map(|backtrace| { - let backtrace = &backtrace.member; + let backtrace_method = input.backtrace_field().map(|backtrace_field| { + let backtrace = &backtrace_field.member; let body = if let Some(source_field) = input.source_field() { let source = &source_field.member; let source_backtrace = if type_is_option(source_field.ty) { @@ -47,18 +47,31 @@ fn impl_struct(input: Struct) -> TokenStream { self.#source.as_dyn_error().backtrace() } }; - quote!({ + let combinator = if type_is_option(backtrace_field.ty) { + quote! { + #source_backtrace.or(self.#backtrace.as_ref()) + } + } else { + quote! { + std::option::Option::Some(#source_backtrace.unwrap_or(&self.#backtrace)) + } + }; + quote! { use thiserror::private::AsDynError; - #source_backtrace.unwrap_or(&self.#backtrace) - }) + #combinator + } + } else if type_is_option(backtrace_field.ty) { + quote! { + self.#backtrace.as_ref() + } } else { quote! { - &self.#backtrace + std::option::Option::Some(&self.#backtrace) } }; quote! { fn backtrace(&self) -> std::option::Option<&std::backtrace::Backtrace> { - std::option::Option::Some(#body) + #body } } }); @@ -126,8 +139,10 @@ fn impl_enum(input: Enum) -> TokenStream { let arms = input.variants.iter().map(|variant| { let ident = &variant.ident; match (variant.backtrace_field(), variant.source_field()) { - (Some(backtrace), Some(source_field)) if backtrace.attrs.backtrace.is_none() => { - let backtrace = &backtrace.member; + (Some(backtrace_field), Some(source_field)) + if backtrace_field.attrs.backtrace.is_none() => + { + let backtrace = &backtrace_field.member; let source = &source_field.member; let source_backtrace = if type_is_option(source_field.ty) { quote_spanned! {source.span()=> @@ -138,21 +153,35 @@ fn impl_enum(input: Enum) -> TokenStream { source.as_dyn_error().backtrace() } }; + let combinator = if type_is_option(backtrace_field.ty) { + quote! { + #source_backtrace.or(backtrace.as_ref()) + } + } else { + quote! { + std::option::Option::Some(#source_backtrace.unwrap_or(backtrace)) + } + }; quote! { #ty::#ident { #backtrace: backtrace, #source: source, .. - } => std::option::Option::Some({ + } => { use thiserror::private::AsDynError; - #source_backtrace.unwrap_or(backtrace) - }), + #combinator + } } } - (Some(backtrace), _) => { - let backtrace = &backtrace.member; + (Some(backtrace_field), _) => { + let backtrace = &backtrace_field.member; + let body = if type_is_option(backtrace_field.ty) { + quote!(backtrace.as_ref()) + } else { + quote!(std::option::Option::Some(backtrace)) + }; quote! { - #ty::#ident {#backtrace: backtrace, ..} => std::option::Option::Some(backtrace), + #ty::#ident {#backtrace: backtrace, ..} => #body, } } (None, _) => quote! { From d9ed0fba49d6ac3f804d41d0dc3fdff4f0d87671 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 12 Oct 2019 16:14:07 -0700 Subject: [PATCH 4/5] Add test for optional backtrace --- tests/test_option.rs | 108 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 85 insertions(+), 23 deletions(-) diff --git a/tests/test_option.rs b/tests/test_option.rs index 936f3e63..945e850f 100644 --- a/tests/test_option.rs +++ b/tests/test_option.rs @@ -4,36 +4,98 @@ use std::backtrace::Backtrace; use thiserror::Error; -#[derive(Error, Debug)] -#[error("...")] -pub struct OptSourceNoBacktraceStruct { - #[source] - source: Option, -} - -#[derive(Error, Debug)] -#[error("...")] -pub struct OptSourceAlwaysBacktraceStruct { - #[source] - source: Option, - backtrace: Backtrace, -} +pub mod structs { + use super::*; -#[derive(Error, Debug)] -pub enum OptSourceNoBacktraceEnum { + #[derive(Error, Debug)] #[error("...")] - Test { + pub struct OptSourceNoBacktrace { #[source] source: Option, - }, -} + } -#[derive(Error, Debug)] -pub enum OptSourceAlwaysBacktraceEnum { + #[derive(Error, Debug)] #[error("...")] - Test { + pub struct OptSourceAlwaysBacktrace { #[source] source: Option, backtrace: Backtrace, - }, + } + + #[derive(Error, Debug)] + #[error("...")] + pub struct NoSourceOptBacktrace { + #[backtrace] + backtrace: Option, + } + + #[derive(Error, Debug)] + #[error("...")] + pub struct AlwaysSourceOptBacktrace { + source: anyhow::Error, + #[backtrace] + backtrace: Option, + } + + #[derive(Error, Debug)] + #[error("...")] + pub struct OptSourceOptBacktrace { + #[source] + source: Option, + #[backtrace] + backtrace: Option, + } +} + +pub mod enums { + use super::*; + + #[derive(Error, Debug)] + pub enum OptSourceNoBacktrace { + #[error("...")] + Test { + #[source] + source: Option, + }, + } + + #[derive(Error, Debug)] + pub enum OptSourceAlwaysBacktrace { + #[error("...")] + Test { + #[source] + source: Option, + backtrace: Backtrace, + }, + } + + #[derive(Error, Debug)] + pub enum NoSourceOptBacktrace { + #[error("...")] + Test { + #[backtrace] + backtrace: Option, + }, + } + + #[derive(Error, Debug)] + pub enum AlwaysSourceOptBacktrace { + #[error("...")] + Test { + source: anyhow::Error, + #[backtrace] + backtrace: Option, + }, + } + + #[derive(Error, Debug)] + pub enum OptSourceOptBacktrace { + #[error("...")] + Test { + #[source] + source: Option, + #[backtrace] + backtrace: Option, + }, + } } From b4996def926cc36dda702f1f892f5a87a71f0caf Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 12 Oct 2019 16:18:43 -0700 Subject: [PATCH 5/5] Make test_option appear as ignored or passing --- tests/test_option.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/tests/test_option.rs b/tests/test_option.rs index 945e850f..4b41cc14 100644 --- a/tests/test_option.rs +++ b/tests/test_option.rs @@ -1,11 +1,9 @@ -#![cfg(thiserror_nightly_testing)] -#![feature(backtrace)] - -use std::backtrace::Backtrace; -use thiserror::Error; +#![cfg_attr(thiserror_nightly_testing, feature(backtrace))] +#[cfg(thiserror_nightly_testing)] pub mod structs { - use super::*; + use std::backtrace::Backtrace; + use thiserror::Error; #[derive(Error, Debug)] #[error("...")] @@ -47,8 +45,10 @@ pub mod structs { } } +#[cfg(thiserror_nightly_testing)] pub mod enums { - use super::*; + use std::backtrace::Backtrace; + use thiserror::Error; #[derive(Error, Debug)] pub enum OptSourceNoBacktrace { @@ -99,3 +99,7 @@ pub mod enums { }, } } + +#[test] +#[cfg_attr(not(thiserror_nightly_testing), ignore)] +fn test_option() {}