From 37f9e979de2cf6928b28e17bd0fd8da7bd8412ed Mon Sep 17 00:00:00 2001 From: Justus Fluegel Date: Mon, 3 Feb 2025 01:35:54 +0100 Subject: [PATCH] Add tests, fix remaining bug in label implementation, make label implementation strictly more flexible using to_owned over clone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Justus Fluegel Signed-off-by: Justus Flügel --- miette-derive/src/label.rs | 35 +++++--- miette-derive/src/trait_bounds.rs | 56 +++++++++++-- tests/test_derive_attr.rs | 130 ++++++++++++++++++++++++++++++ 3 files changed, 202 insertions(+), 19 deletions(-) diff --git a/miette-derive/src/label.rs b/miette-derive/src/label.rs index ad052db..7fa1404 100644 --- a/miette-derive/src/label.rs +++ b/miette-derive/src/label.rs @@ -208,10 +208,14 @@ impl Labels { Some(quote! { miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(&self.#span) .as_ref() - .map(|#var| #ctor( - #display, - (*#var).clone(), - )) + .map(|#var| { + use ::std::borrow::ToOwned; + + #ctor( + #display, + (*#var).to_owned(), + ) + }) }) }); let collections_chain = self.0.iter().filter_map(|label| { @@ -236,7 +240,8 @@ impl Labels { let display = #display; ::std::iter::IntoIterator::into_iter(&self.#span).map(move |span| { use miette::macro_helpers::{ToLabelSpanWrapper,ToLabeledSpan}; - let mut labeled_span = ToLabelSpanWrapper::to_labeled_span(span.clone()); + use ::std::borrow::ToOwned; + let mut labeled_span = ToLabelSpanWrapper::to_labeled_span(span.to_owned()); if display.is_some() && labeled_span.label().is_none() { labeled_span.set_label(display.clone()) } @@ -296,10 +301,15 @@ impl Labels { Some(quote! { miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(#field) - .map(|#var| #ctor( - #display, - #var.clone(), - )) + .as_ref() + .map(|#var| { + use ::std::borrow::ToOwned; + + #ctor( + #display, + (*#var).to_owned(), + ) + }) }) }); let collections_chain = labels.0.iter().filter_map(|label| { @@ -322,9 +332,12 @@ impl Labels { Some(quote! { .chain({ let display = #display; - #field.iter().map(move |span| { + ::std::iter::IntoIterator::into_iter(#field).map(move |span| { use miette::macro_helpers::{ToLabelSpanWrapper,ToLabeledSpan}; - let mut labeled_span = ToLabelSpanWrapper::to_labeled_span(span.clone()); + use ::std::borrow::ToOwned; + let mut labeled_span = ToLabelSpanWrapper::to_labeled_span( + span.to_owned() + ); if display.is_some() && labeled_span.label().is_none() { labeled_span.set_label(display.clone()); } diff --git a/miette-derive/src/trait_bounds.rs b/miette-derive/src/trait_bounds.rs index 6be6252..a1912ca 100644 --- a/miette-derive/src/trait_bounds.rs +++ b/miette-derive/src/trait_bounds.rs @@ -20,7 +20,8 @@ pub struct RequiredTraitBound { source_code: bool, into_source_span: bool, std_into_iter: bool, - std_clone: bool, + std_deref: bool, + std_to_owned: bool, } impl RequiredTraitBound { @@ -56,9 +57,13 @@ impl RequiredTraitBound { ))) } - if self.std_clone { + if self.std_deref { + bounds.push(TypeParamBound::Trait(syn::parse_quote!(::std::ops::Deref))) + } + + if self.std_to_owned { bounds.push(TypeParamBound::Trait(syn::parse_quote!( - ::std::clone::Clone + ::std::borrow::ToOwned ))) } @@ -80,7 +85,6 @@ impl RequiredTraitBound { fn register_label_usage(&mut self) { self.into_source_span = true; - self.std_clone = true; } fn register_collection_usage(&mut self) { @@ -95,6 +99,14 @@ impl RequiredTraitBound { self.miette_diagnostic = true; self.r#static = true; } + + fn register_deref_usage(&mut self) { + self.std_deref = true; + } + + fn register_to_owned_usage(&mut self) { + self.std_to_owned = true; + } } pub struct TraitBoundStore(HashMap<(Option, Type), RequiredTraitBound>); @@ -354,12 +366,22 @@ impl TraitBoundStore { pub fn register_label_usage(&mut self, r#type: &Type) { let r#type = Self::extract_option(r#type).unwrap_or(r#type); - let Some(r#type) = self.check_generic_usage(r#type) else { + let Some(ty) = self.check_generic_usage(r#type) else { return; }; - let type_opts = self.0.entry((None, r#type.clone())).or_default(); - type_opts.register_label_usage() + let type_opts = self.0.entry((None, ty.clone())).or_default(); + + type_opts.register_to_owned_usage(); + + let type_opts_to_owned = self + .0 + .entry(( + None, + syn::parse_quote!(<#ty as ::std::borrow::ToOwned>::Owned), + )) + .or_default(); + type_opts_to_owned.register_label_usage(); } pub fn register_label_collection_usage(&mut self, r#type: &Type) { @@ -385,7 +407,25 @@ impl TraitBoundStore { syn::parse_quote!(<#ty as ::std::iter::IntoIterator>::Item), )) .or_default(); - type_opts_item.register_label_usage(); + type_opts_item.register_deref_usage(); + + let type_opts_deref_item = self + .0 + .entry(( + Some(syn::parse_quote!(for<'__miette_internal_lt>)), + syn::parse_quote!(<<#ty as ::std::iter::IntoIterator>::Item as ::std::ops::Deref>::Target), + )) + .or_default(); + type_opts_deref_item.register_to_owned_usage(); + + let type_opts_deref_to_owned_item = self + .0 + .entry(( + Some(syn::parse_quote!(for<'__miette_internal_lt>)), + syn::parse_quote!(<<<#ty as ::std::iter::IntoIterator>::Item as ::std::ops::Deref>::Target as ::std::borrow::ToOwned>::Owned), + )) + .or_default(); + type_opts_deref_to_owned_item.register_label_usage(); } pub fn register_related_usage(&mut self, r#type: &Type) { diff --git a/tests/test_derive_attr.rs b/tests/test_derive_attr.rs index f1b0f3d..19487a0 100644 --- a/tests/test_derive_attr.rs +++ b/tests/test_derive_attr.rs @@ -145,3 +145,133 @@ fn attr_not_required() { let expectation = LabeledSpan::new(Some("this bit here".into()), 9usize, 4usize); assert_eq!(err_span, expectation); } + +fn assert_impl_diagnostic() {} + +#[test] +fn transparent_generic() { + #[derive(Debug, Diagnostic, Error)] + enum Combined { + #[error(transparent)] + #[diagnostic(transparent)] + Other(T), + #[error("foo")] + Custom, + } + + assert_impl_diagnostic::>(); +} + +#[test] +fn generic_label() { + #[derive(Debug, Diagnostic, Error)] + #[error("foo")] + struct Combined { + #[label] + label: T, + } + + assert_impl_diagnostic::>(); + assert_impl_diagnostic::>(); +} + +#[test] +fn generic_source_code() { + #[derive(Debug, Diagnostic, Error)] + #[error("foo")] + struct Combined { + #[source_code] + label: T, + } + + assert_impl_diagnostic::>(); +} + +#[test] +fn generic_optional_source_code() { + #[derive(Debug, Diagnostic, Error)] + #[error("foo")] + struct Combined { + #[source_code] + label: Option, + } + + assert_impl_diagnostic::>(); +} + +#[test] +fn generic_label_primary() { + #[derive(Debug, Diagnostic, Error)] + #[error("foo")] + struct Combined { + #[label(primary)] + label: T, + } + + assert_impl_diagnostic::>(); + assert_impl_diagnostic::>(); +} + +#[test] +fn generic_label_collection() { + #[derive(Debug, Diagnostic, Error)] + #[error("foo")] + struct Combined { + #[label(collection)] + label: Vec, + } + + assert_impl_diagnostic::>(); + assert_impl_diagnostic::>(); +} + +#[test] +fn generic_label_generic_collection() { + #[derive(Debug, Diagnostic, Error)] + #[error("foo")] + struct Combined { + #[label(collection)] + label: T, + } + + assert_impl_diagnostic::>>(); + assert_impl_diagnostic::>>(); +} + +#[test] +fn generic_related() { + #[derive(Debug, Diagnostic, Error)] + #[error("foo")] + struct Combined { + #[related] + label: Vec, + } + + assert_impl_diagnostic::>(); +} + +#[test] +fn generic_diagnostic_source() { + #[derive(Debug, Diagnostic, Error)] + enum Combined { + #[error(transparent)] + Other(#[diagnostic_source] T), + #[error("foo")] + Custom, + } + + assert_impl_diagnostic::>(); +} + +#[test] +fn generic_not_influencing_default() { + #[derive(Debug, Diagnostic, Error)] + enum Combined { + #[error("bar")] + Other(T), + #[error("foo")] + Custom, + } + + assert_impl_diagnostic::>(); +}