From 1a3a4c224a43b0352dd309bcfb67f42a4953aafb Mon Sep 17 00:00:00 2001 From: Justus Fluegel Date: Sun, 2 Feb 2025 21:00:52 +0100 Subject: [PATCH 1/9] Add required bounds to derived impl MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is implemented using a new TypeBoundsStore struct which tracks usage of types during parsing and stores required bounds for generics, trying to use some heuristics to remove unneeded bounds. Signed-off-by: Justus Fluegel Signed-off-by: Justus Flügel --- miette-derive/Cargo.toml | 2 +- miette-derive/src/diagnostic.rs | 46 +++- miette-derive/src/diagnostic_source.rs | 20 +- miette-derive/src/forward.rs | 15 +- miette-derive/src/label.rs | 28 +- miette-derive/src/lib.rs | 1 + miette-derive/src/related.rs | 18 +- miette-derive/src/source_code.rs | 48 +++- miette-derive/src/trait_bounds.rs | 341 +++++++++++++++++++++++++ 9 files changed, 484 insertions(+), 35 deletions(-) create mode 100644 miette-derive/src/trait_bounds.rs diff --git a/miette-derive/Cargo.toml b/miette-derive/Cargo.toml index a726479..e281272 100644 --- a/miette-derive/Cargo.toml +++ b/miette-derive/Cargo.toml @@ -13,4 +13,4 @@ proc-macro = true [dependencies] proc-macro2 = "1.0.83" quote = "1.0.35" -syn = "2.0.87" +syn = { version = "2.0.87", features = ["extra-traits"] } diff --git a/miette-derive/src/diagnostic.rs b/miette-derive/src/diagnostic.rs index 0173d2a..3206e83 100644 --- a/miette-derive/src/diagnostic.rs +++ b/miette-derive/src/diagnostic.rs @@ -11,6 +11,7 @@ use crate::label::Labels; use crate::related::Related; use crate::severity::Severity; use crate::source_code::SourceCode; +use crate::trait_bounds::TraitBoundStore; use crate::url::Url; pub enum Diagnostic { @@ -19,11 +20,13 @@ pub enum Diagnostic { ident: syn::Ident, fields: syn::Fields, args: DiagnosticDefArgs, + bound_store: TraitBoundStore, }, Enum { ident: syn::Ident, generics: syn::Generics, variants: Vec, + bound_store: TraitBoundStore, }, } @@ -71,12 +74,15 @@ pub struct DiagnosticConcreteArgs { } impl DiagnosticConcreteArgs { - fn for_fields(fields: &syn::Fields) -> Result { - let labels = Labels::from_fields(fields)?; - let source_code = SourceCode::from_fields(fields)?; - let related = Related::from_fields(fields)?; + fn for_fields( + fields: &syn::Fields, + bounds_store: &mut TraitBoundStore, + ) -> Result { + let labels = Labels::from_fields(fields, bounds_store)?; + let source_code = SourceCode::from_fields(fields, bounds_store)?; + let related = Related::from_fields(fields, bounds_store)?; let help = Help::from_fields(fields)?; - let diagnostic_source = DiagnosticSource::from_fields(fields)?; + let diagnostic_source = DiagnosticSource::from_fields(fields, bounds_store)?; Ok(DiagnosticConcreteArgs { code: None, help, @@ -156,6 +162,7 @@ impl DiagnosticDefArgs { _ident: &syn::Ident, fields: &syn::Fields, attrs: &[&syn::Attribute], + bounds_store: &mut TraitBoundStore, allow_transparent: bool, ) -> syn::Result { let mut errors = Vec::new(); @@ -166,7 +173,7 @@ impl DiagnosticDefArgs { attrs[0].parse_args_with(Punctuated::::parse_terminated) { if matches!(args.first(), Some(DiagnosticArg::Transparent)) { - let forward = Forward::for_transparent_field(fields)?; + let forward = Forward::for_transparent_field(fields, bounds_store)?; return Ok(Self::Transparent(forward)); } } @@ -182,7 +189,7 @@ impl DiagnosticDefArgs { matches!(d, DiagnosticArg::Transparent) } - let mut concrete = DiagnosticConcreteArgs::for_fields(fields)?; + let mut concrete = DiagnosticConcreteArgs::for_fields(fields, bounds_store)?; for attr in attrs { let args = attr.parse_args_with(Punctuated::::parse_terminated); @@ -226,10 +233,13 @@ impl Diagnostic { .collect::>(); Ok(match input.data { syn::Data::Struct(data_struct) => { + let mut bounds_store = TraitBoundStore::new(&input.generics); + let args = DiagnosticDefArgs::parse( &input.ident, &data_struct.fields, &input_attrs, + &mut bounds_store, true, )?; @@ -238,16 +248,23 @@ impl Diagnostic { ident: input.ident, generics: input.generics, args, + bound_store: bounds_store, } } syn::Data::Enum(syn::DataEnum { variants, .. }) => { let mut vars = Vec::new(); + let mut bound_store = TraitBoundStore::new(&input.generics); for var in variants { let mut variant_attrs = input_attrs.clone(); variant_attrs .extend(var.attrs.iter().filter(|x| x.path().is_ident("diagnostic"))); - let args = - DiagnosticDefArgs::parse(&var.ident, &var.fields, &variant_attrs, true)?; + let args = DiagnosticDefArgs::parse( + &var.ident, + &var.fields, + &variant_attrs, + &mut bound_store, + true, + )?; vars.push(DiagnosticDef { ident: var.ident, fields: var.fields, @@ -258,6 +275,7 @@ impl Diagnostic { ident: input.ident, generics: input.generics, variants: vars, + bound_store, } } syn::Data::Union(_) => { @@ -276,8 +294,11 @@ impl Diagnostic { fields, generics, args, + bound_store, } => { - let (impl_generics, ty_generics, where_clause) = &generics.split_for_impl(); + let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); + let where_clause = bound_store.merge_with(where_clause); + match args { DiagnosticDefArgs::Transparent(forward) => { let code_method = forward.gen_struct_method(WhichFn::Code); @@ -369,8 +390,11 @@ impl Diagnostic { ident, generics, variants, + bound_store, } => { - let (impl_generics, ty_generics, where_clause) = &generics.split_for_impl(); + let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); + let where_clause = bound_store.merge_with(where_clause); + let code_body = Code::gen_enum(variants); let help_body = Help::gen_enum(variants); let sev_body = Severity::gen_enum(variants); diff --git a/miette-derive/src/diagnostic_source.rs b/miette-derive/src/diagnostic_source.rs index 1104eb7..8e66e29 100644 --- a/miette-derive/src/diagnostic_source.rs +++ b/miette-derive/src/diagnostic_source.rs @@ -3,6 +3,7 @@ use quote::quote; use syn::spanned::Spanned; use crate::forward::WhichFn; +use crate::trait_bounds::TraitBoundStore; use crate::{ diagnostic::{DiagnosticConcreteArgs, DiagnosticDef}, utils::{display_pat_members, gen_all_variants_with}, @@ -11,17 +12,25 @@ use crate::{ pub struct DiagnosticSource(syn::Member); impl DiagnosticSource { - pub(crate) fn from_fields(fields: &syn::Fields) -> syn::Result> { + pub(crate) fn from_fields( + fields: &syn::Fields, + bounds_store: &mut TraitBoundStore, + ) -> syn::Result> { match fields { - syn::Fields::Named(named) => Self::from_fields_vec(named.named.iter().collect()), + syn::Fields::Named(named) => { + Self::from_fields_vec(named.named.iter().collect(), bounds_store) + } syn::Fields::Unnamed(unnamed) => { - Self::from_fields_vec(unnamed.unnamed.iter().collect()) + Self::from_fields_vec(unnamed.unnamed.iter().collect(), bounds_store) } syn::Fields::Unit => Ok(None), } } - fn from_fields_vec(fields: Vec<&syn::Field>) -> syn::Result> { + fn from_fields_vec( + fields: Vec<&syn::Field>, + bounds_store: &mut TraitBoundStore, + ) -> syn::Result> { for (i, field) in fields.iter().enumerate() { for attr in &field.attrs { if attr.path().is_ident("diagnostic_source") { @@ -33,6 +42,9 @@ impl DiagnosticSource { span: field.span(), }) }; + + bounds_store.register_source_usage(&field.ty); + return Ok(Some(DiagnosticSource(diagnostic_source))); } } diff --git a/miette-derive/src/forward.rs b/miette-derive/src/forward.rs index d170366..88c437c 100644 --- a/miette-derive/src/forward.rs +++ b/miette-derive/src/forward.rs @@ -6,6 +6,8 @@ use syn::{ spanned::Spanned, }; +use crate::trait_bounds::TraitBoundStore; + pub enum Forward { Unnamed(usize), Named(syn::Ident), @@ -90,7 +92,10 @@ impl WhichFn { } impl Forward { - pub fn for_transparent_field(fields: &syn::Fields) -> syn::Result { + pub fn for_transparent_field( + fields: &syn::Fields, + bounds_store: &mut TraitBoundStore, + ) -> syn::Result { let make_err = || { syn::Error::new( fields.span(), @@ -108,12 +113,18 @@ impl Forward { .ident .clone() .unwrap_or_else(|| format_ident!("unnamed")); + + bounds_store.register_transparent_usage(&field.ty); Ok(Self::Named(field_name)) } syn::Fields::Unnamed(unnamed) => { - if unnamed.unnamed.iter().len() != 1 { + let mut iter = unnamed.unnamed.iter(); + let field = iter.next().ok_or_else(make_err)?; + if iter.next().is_some() { return Err(make_err()); } + + bounds_store.register_transparent_usage(&field.ty); Ok(Self::Unnamed(0)) } _ => Err(syn::Error::new( diff --git a/miette-derive/src/label.rs b/miette-derive/src/label.rs index ab2ceac..d450fbf 100644 --- a/miette-derive/src/label.rs +++ b/miette-derive/src/label.rs @@ -11,6 +11,7 @@ use crate::{ diagnostic::{DiagnosticConcreteArgs, DiagnosticDef}, fmt::{self, Display}, forward::WhichFn, + trait_bounds::TraitBoundStore, utils::{display_pat_members, gen_all_variants_with}, }; @@ -101,22 +102,31 @@ impl Parse for LabelAttr { } else { (LabelType::Default, None) }; + Ok(LabelAttr { label, lbl_ty }) } } impl Labels { - pub fn from_fields(fields: &syn::Fields) -> syn::Result> { + pub fn from_fields( + fields: &syn::Fields, + bounds_store: &mut TraitBoundStore, + ) -> syn::Result> { match fields { - syn::Fields::Named(named) => Self::from_fields_vec(named.named.iter().collect()), + syn::Fields::Named(named) => { + Self::from_fields_vec(named.named.iter().collect(), bounds_store) + } syn::Fields::Unnamed(unnamed) => { - Self::from_fields_vec(unnamed.unnamed.iter().collect()) + Self::from_fields_vec(unnamed.unnamed.iter().collect(), bounds_store) } syn::Fields::Unit => Ok(None), } } - fn from_fields_vec(fields: Vec<&syn::Field>) -> syn::Result> { + fn from_fields_vec( + fields: Vec<&syn::Field>, + bounds_store: &mut TraitBoundStore, + ) -> syn::Result> { let mut labels = Vec::new(); for (i, field) in fields.iter().enumerate() { for attr in &field.attrs { @@ -144,6 +154,16 @@ impl Labels { )); } + match lbl_ty { + LabelType::Default | LabelType::Primary => { + bounds_store.register_source_span_usage(&field.ty); + } + + LabelType::Collection => { + bounds_store.register_source_span_collection_usage(&field.ty); + } + } + labels.push(Label { label, span, diff --git a/miette-derive/src/lib.rs b/miette-derive/src/lib.rs index 0f7e64e..81c6112 100644 --- a/miette-derive/src/lib.rs +++ b/miette-derive/src/lib.rs @@ -14,6 +14,7 @@ mod label; mod related; mod severity; mod source_code; +mod trait_bounds; mod url; mod utils; diff --git a/miette-derive/src/related.rs b/miette-derive/src/related.rs index 9b7f9e1..de189cf 100644 --- a/miette-derive/src/related.rs +++ b/miette-derive/src/related.rs @@ -5,23 +5,32 @@ use syn::spanned::Spanned; use crate::{ diagnostic::{DiagnosticConcreteArgs, DiagnosticDef}, forward::WhichFn, + trait_bounds::TraitBoundStore, utils::{display_pat_members, gen_all_variants_with}, }; pub struct Related(syn::Member); impl Related { - pub(crate) fn from_fields(fields: &syn::Fields) -> syn::Result> { + pub(crate) fn from_fields( + fields: &syn::Fields, + bounds_store: &mut TraitBoundStore, + ) -> syn::Result> { match fields { - syn::Fields::Named(named) => Self::from_fields_vec(named.named.iter().collect()), + syn::Fields::Named(named) => { + Self::from_fields_vec(named.named.iter().collect(), bounds_store) + } syn::Fields::Unnamed(unnamed) => { - Self::from_fields_vec(unnamed.unnamed.iter().collect()) + Self::from_fields_vec(unnamed.unnamed.iter().collect(), bounds_store) } syn::Fields::Unit => Ok(None), } } - fn from_fields_vec(fields: Vec<&syn::Field>) -> syn::Result> { + fn from_fields_vec( + fields: Vec<&syn::Field>, + bounds_store: &mut TraitBoundStore, + ) -> syn::Result> { for (i, field) in fields.iter().enumerate() { for attr in &field.attrs { if attr.path().is_ident("related") { @@ -33,6 +42,7 @@ impl Related { span: field.span(), }) }; + bounds_store.register_related_usage(&field.ty); return Ok(Some(Related(related))); } } diff --git a/miette-derive/src/source_code.rs b/miette-derive/src/source_code.rs index e1b85ab..fc0be64 100644 --- a/miette-derive/src/source_code.rs +++ b/miette-derive/src/source_code.rs @@ -1,10 +1,11 @@ use proc_macro2::TokenStream; use quote::{format_ident, quote}; -use syn::spanned::Spanned; +use syn::{spanned::Spanned, AngleBracketedGenericArguments, GenericArgument, PathArguments}; use crate::{ diagnostic::{DiagnosticConcreteArgs, DiagnosticDef}, forward::WhichFn, + trait_bounds::TraitBoundStore, utils::{display_pat_members, gen_all_variants_with}, }; @@ -14,17 +15,25 @@ pub struct SourceCode { } impl SourceCode { - pub fn from_fields(fields: &syn::Fields) -> syn::Result> { + pub fn from_fields( + fields: &syn::Fields, + bounds_store: &mut TraitBoundStore, + ) -> syn::Result> { match fields { - syn::Fields::Named(named) => Self::from_fields_vec(named.named.iter().collect()), + syn::Fields::Named(named) => { + Self::from_fields_vec(named.named.iter().collect(), bounds_store) + } syn::Fields::Unnamed(unnamed) => { - Self::from_fields_vec(unnamed.unnamed.iter().collect()) + Self::from_fields_vec(unnamed.unnamed.iter().collect(), bounds_store) } syn::Fields::Unit => Ok(None), } } - fn from_fields_vec(fields: Vec<&syn::Field>) -> syn::Result> { + fn from_fields_vec( + fields: Vec<&syn::Field>, + bounds_store: &mut TraitBoundStore, + ) -> syn::Result> { for (i, field) in fields.iter().enumerate() { for attr in &field.attrs { if attr.path().is_ident("source_code") { @@ -35,12 +44,33 @@ impl SourceCode { { segments .last() - .map(|seg| seg.ident == "Option") - .unwrap_or(false) + .filter(|seg| seg.ident == "Option") + .and_then(|seg| match &seg.arguments { + PathArguments::AngleBracketed(AngleBracketedGenericArguments { + args, + .. + }) => { + let mut iter = args.iter(); + + let ty = iter.next(); + iter.next().xor(ty) + } + _ => None, + }) + .and_then(|arg| match arg { + GenericArgument::Type(ty) => Some(ty), + _ => None, + }) } else { - false + None }; + if let Some(option_ty) = is_option { + bounds_store.register_source_code_usage(option_ty); + } else { + bounds_store.register_source_code_usage(&field.ty); + } + let source_code = if let Some(ident) = field.ident.clone() { syn::Member::Named(ident) } else { @@ -51,7 +81,7 @@ impl SourceCode { }; return Ok(Some(SourceCode { source_code, - is_option, + is_option: is_option.is_some(), })); } } diff --git a/miette-derive/src/trait_bounds.rs b/miette-derive/src/trait_bounds.rs new file mode 100644 index 0000000..fc00ed9 --- /dev/null +++ b/miette-derive/src/trait_bounds.rs @@ -0,0 +1,341 @@ +use std::{ + collections::{HashMap, VecDeque}, + iter::{once, FromIterator}, +}; + +use proc_macro2::Span; +use syn::{ + punctuated::Punctuated, token::Plus, AngleBracketedGenericArguments, AssocType, + GenericArgument, GenericParam, Generics, ParenthesizedGenericArguments, Path, PathArguments, + PathSegment, PredicateType, ReturnType, Token, Type, TypeArray, TypeGroup, TypeParamBound, + TypeParen, TypePath, TypePtr, TypeReference, TypeSlice, TypeTuple, WhereClause, WherePredicate, +}; + +#[derive(Default)] +pub struct RequiredTraitBound { + r#static: bool, + std_error: bool, + miette_diagnostic: bool, + source_code: bool, + into_source_span: bool, + std_into_iter: bool, +} + +impl RequiredTraitBound { + fn to_bounds(&self) -> Punctuated { + let mut bounds = Punctuated::new(); + if self.std_error && !self.miette_diagnostic { + bounds.push(TypeParamBound::Trait(syn::parse_quote!( + ::std::error::Error + ))); + } + + if self.miette_diagnostic { + bounds.push(TypeParamBound::Trait(syn::parse_quote!( + ::miette::Diagnostic + ))) + } + + if self.source_code { + bounds.push(TypeParamBound::Trait(syn::parse_quote!( + ::miette::SourceCode + ))) + } + + if self.into_source_span { + bounds.push(TypeParamBound::Trait(syn::parse_quote!( + ::std::convert::Into<::miette::SourceSpan> + ))) + } + + if self.std_into_iter { + bounds.push(TypeParamBound::Trait(syn::parse_quote!( + ::std::iter::IntoIterator + ))) + } + + if self.r#static { + bounds.push(TypeParamBound::Lifetime(syn::parse_quote!('static))) + } + + bounds + } + + fn register_transparent_usage(&mut self) { + self.r#static = true; + self.miette_diagnostic = true; + } + + fn register_source_code_usage(&mut self) { + self.source_code = true; + } + + fn register_source_span_usage(&mut self) { + self.into_source_span = true; + } + + fn register_collection_usage(&mut self) { + self.std_into_iter = true; + } + fn register_related_item_usage(&mut self) { + self.miette_diagnostic = true; + self.r#static = true; + } + + fn register_source_usage(&mut self) { + self.miette_diagnostic = true; + self.r#static = true; + } +} + +pub struct TraitBoundStore(HashMap); + +impl TraitBoundStore { + pub fn new(generics: &Generics) -> Self { + let hash_map = generics + .params + .iter() + .filter_map(|param| { + if let GenericParam::Type(ty) = param { + Some(ty) + } else { + None + } + }) + .map(|param| { + Type::Path(TypePath { + qself: None, + path: Path { + leading_colon: None, + segments: Punctuated::from_iter(once(PathSegment { + ident: param.ident.clone(), + arguments: PathArguments::None, + })), + }, + }) + }) + .map(|v| (v, RequiredTraitBound::default())) + .collect::>(); + + Self(hash_map) + } + + fn check_generic_usage<'ty>(&self, mut r#type: &'ty Type) -> Option<&'ty Type> { + // in theory we could skip all this logic and just allow trivial bounds but that would add redundant trait bounds + // to the derived impl - would be another choice to make. I choose to filter as much as possible so that we don't + // introduce unneccessary bounds. + + // this reduces the type down as much as possible to remove unneeded groups. + let original_type = loop { + match r#type { + Type::Paren(TypeParen { elem, .. }) => r#type = &**elem, + Type::Group(TypeGroup { elem, .. }) => r#type = &**elem, + x => break x, + } + }; + + let mut depends_on_generic = false; + + // max depth to check, after which we'll just add the (maybe redundant) bound anyways. + // this is a tradeoff between filtering speed and compiler speed so I'll keep it + // reasonably low for now, since I assume the compiler is better optimized for more complex + // checks. + let max_depth = 8; + + let mut to_check_queue: VecDeque<(&Type, usize)> = VecDeque::new(); + to_check_queue.push_back((original_type, 0)); + + while !depends_on_generic { + // this needs to be like this cuz if-let-chains aren't supported yet + let Some((elem, current_depth)) = to_check_queue.pop_front() else { + break; + }; + + // if we exceed the max depth we just assume it depends on the generic and let the compiler check it + if current_depth > max_depth { + depends_on_generic = true; + break; + } + + // the map contains types that we know depend on generics so we can just short circuit + // + // this is also the "bottom" check since we add the generics themselves to the map when + // constructing self + if self.0.contains_key(elem) { + depends_on_generic = true; + break; + } + + // basically go through the type and add all referenced types inside it to the check queue + match elem { + Type::Group(_) => unreachable!("This is unwrapped above"), + Type::Paren(_) => unreachable!("This is unwrapped above"), + // function pointer's can never implement the required trait bounds anyways so we just accept the errors + Type::BareFn(_) => return None, + // impl trait types aren't allowed from struct/enum definitions anyways so we can just ignore them + Type::ImplTrait(_) => return None, + // infered types aren't allowed either + Type::Infer(_) => return None, + // macros are opaque to us and i don't really know how to properly implement this. + // we could in theory I think introduce a type alias and use that instead but honestly + // type macros are such a niche usecase especially in combination with a generic, + // I would say we should just recommend to implement + // the trait manually, as such we just accept the error if any occurs (this still allows using macros when they + // return concrete types which don't depend on any generic or when the generic doesn't affect the + // required trait implementation) + Type::Macro(_) => return None, + // trait objects which depend on a generic inside them seem like very much a hassle to implement so i'll ignore + // them for now, if the need arises we could support that in a future pr maybe? + // + // this again doesn't restrict the usage of trait objects which implement the required traits regardless of the generics. + Type::TraitObject(_) => return None, + // Well never is never and never never. + Type::Never(_) => return None, + Type::Array(TypeArray { elem, .. }) + | Type::Ptr(TypePtr { elem, .. }) + | Type::Reference(TypeReference { elem, .. }) + | Type::Slice(TypeSlice { elem, .. }) => { + to_check_queue.push_back((&**elem, current_depth + 1)); + } + Type::Path(TypePath { qself, path }) => { + if let Some(qself) = qself { + to_check_queue.push_back((&qself.ty, current_depth + 1)); + } + + for segment in &path.segments { + match &segment.arguments { + PathArguments::None => {} + PathArguments::AngleBracketed(AngleBracketedGenericArguments { + args, + .. + }) => { + for argument in args { + match argument { + GenericArgument::Type(ty) + | GenericArgument::AssocType(AssocType { ty, .. }) => { + to_check_queue.push_back((ty, current_depth + 1)); + } + _ => {} + } + } + } + PathArguments::Parenthesized(ParenthesizedGenericArguments { + inputs, + output, + .. + }) => { + for inp in inputs { + to_check_queue.push_back((inp, current_depth + 1)); + } + + if let ReturnType::Type(_, ty) = output { + to_check_queue.push_back((ty, current_depth + 1)); + } + } + } + } + } + Type::Tuple(TypeTuple { elems, .. }) => { + for elem in elems { + to_check_queue.push_back((elem, current_depth + 1)); + } + } + // we can't really handle verbatim so we just assume it depends on the generics + Type::Verbatim(_) => depends_on_generic = true, + _ => depends_on_generic = true, + } + } + + depends_on_generic.then_some(original_type) + } + + pub fn merge_with(&self, where_clause: Option<&WhereClause>) -> Option { + if self.0.is_empty() { + return where_clause.cloned(); + } + + let mut where_clause = where_clause.cloned().unwrap_or_else(|| WhereClause { + where_token: Token![where](Span::mixed_site()), + predicates: Punctuated::new(), + }); + + where_clause + .predicates + .extend(self.0.iter().map(|(ty, bounds)| { + WherePredicate::Type(PredicateType { + lifetimes: None, + bounded_ty: ty.clone(), + colon_token: Token![:](Span::mixed_site()), + bounds: bounds.to_bounds(), + }) + })); + + Some(where_clause) + } + + pub fn register_transparent_usage(&mut self, r#type: &Type) { + let Some(r#type) = self.check_generic_usage(r#type) else { + return; + }; + + let type_opts = self.0.entry(r#type.clone()).or_default(); + type_opts.register_transparent_usage() + } + + pub fn register_source_code_usage(&mut self, r#type: &Type) { + let Some(r#type) = self.check_generic_usage(r#type) else { + return; + }; + + let type_opts = self.0.entry(r#type.clone()).or_default(); + type_opts.register_source_code_usage() + } + + pub fn register_source_span_usage(&mut self, r#type: &Type) { + let Some(r#type) = self.check_generic_usage(r#type) else { + return; + }; + + let type_opts = self.0.entry(r#type.clone()).or_default(); + type_opts.register_source_span_usage() + } + + pub fn register_source_span_collection_usage(&mut self, r#type: &Type) { + let Some(ty) = self.check_generic_usage(r#type) else { + return; + }; + + let type_opts = self.0.entry(ty.clone()).or_default(); + type_opts.register_collection_usage(); + + let type_opts_item = self + .0 + .entry(syn::parse_quote!(<#ty as ::std::iter::IntoIterator>::Item)) + .or_default(); + type_opts_item.register_source_span_usage(); + } + + pub fn register_related_usage(&mut self, r#type: &Type) { + let Some(ty) = self.check_generic_usage(r#type) else { + return; + }; + + let type_opts = self.0.entry(ty.clone()).or_default(); + type_opts.register_collection_usage(); + + let type_opts_item = self + .0 + .entry(syn::parse_quote!(<#ty as ::std::iter::IntoIterator>::Item)) + .or_default(); + type_opts_item.register_related_item_usage(); + } + + pub(crate) fn register_source_usage(&mut self, r#type: &Type) { + let Some(ty) = self.check_generic_usage(r#type) else { + return; + }; + + let type_opts = self.0.entry(ty.clone()).or_default(); + type_opts.register_source_usage(); + } +} From 89d30a3b37cbd6aef67ff1a397901c69ca5556b1 Mon Sep 17 00:00:00 2001 From: Justus Fluegel Date: Sun, 2 Feb 2025 23:14:52 +0100 Subject: [PATCH 2/9] Iron out some bugs 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 | 10 ++- miette-derive/src/source_code.rs | 28 +----- miette-derive/src/trait_bounds.rs | 144 +++++++++++++++++++++++------- tests/derive.rs | 2 + 4 files changed, 121 insertions(+), 63 deletions(-) diff --git a/miette-derive/src/label.rs b/miette-derive/src/label.rs index d450fbf..ad052db 100644 --- a/miette-derive/src/label.rs +++ b/miette-derive/src/label.rs @@ -156,11 +156,11 @@ impl Labels { match lbl_ty { LabelType::Default | LabelType::Primary => { - bounds_store.register_source_span_usage(&field.ty); + bounds_store.register_label_usage(&field.ty); } LabelType::Collection => { - bounds_store.register_source_span_collection_usage(&field.ty); + bounds_store.register_label_collection_usage(&field.ty); } } @@ -207,9 +207,10 @@ impl Labels { Some(quote! { miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(&self.#span) + .as_ref() .map(|#var| #ctor( #display, - #var.clone(), + (*#var).clone(), )) }) }); @@ -229,10 +230,11 @@ impl Labels { } else { quote! { std::option::Option::None } }; + Some(quote! { .chain({ let display = #display; - self.#span.iter().map(move |span| { + ::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()); if display.is_some() && labeled_span.label().is_none() { diff --git a/miette-derive/src/source_code.rs b/miette-derive/src/source_code.rs index fc0be64..493d3a5 100644 --- a/miette-derive/src/source_code.rs +++ b/miette-derive/src/source_code.rs @@ -37,33 +37,7 @@ impl SourceCode { for (i, field) in fields.iter().enumerate() { for attr in &field.attrs { if attr.path().is_ident("source_code") { - let is_option = if let syn::Type::Path(syn::TypePath { - path: syn::Path { segments, .. }, - .. - }) = &field.ty - { - segments - .last() - .filter(|seg| seg.ident == "Option") - .and_then(|seg| match &seg.arguments { - PathArguments::AngleBracketed(AngleBracketedGenericArguments { - args, - .. - }) => { - let mut iter = args.iter(); - - let ty = iter.next(); - iter.next().xor(ty) - } - _ => None, - }) - .and_then(|arg| match arg { - GenericArgument::Type(ty) => Some(ty), - _ => None, - }) - } else { - None - }; + let is_option = TraitBoundStore::extract_option(&field.ty); if let Some(option_ty) = is_option { bounds_store.register_source_code_usage(option_ty); diff --git a/miette-derive/src/trait_bounds.rs b/miette-derive/src/trait_bounds.rs index fc00ed9..6be6252 100644 --- a/miette-derive/src/trait_bounds.rs +++ b/miette-derive/src/trait_bounds.rs @@ -5,10 +5,11 @@ use std::{ use proc_macro2::Span; use syn::{ - punctuated::Punctuated, token::Plus, AngleBracketedGenericArguments, AssocType, + punctuated::Punctuated, token::Plus, AngleBracketedGenericArguments, AssocType, BoundLifetimes, GenericArgument, GenericParam, Generics, ParenthesizedGenericArguments, Path, PathArguments, - PathSegment, PredicateType, ReturnType, Token, Type, TypeArray, TypeGroup, TypeParamBound, - TypeParen, TypePath, TypePtr, TypeReference, TypeSlice, TypeTuple, WhereClause, WherePredicate, + PathSegment, PredicateType, ReturnType, Token, TraitBound, Type, TypeArray, TypeGroup, + TypeParamBound, TypeParen, TypePath, TypePtr, TypeReference, TypeSlice, TypeTuple, WhereClause, + WherePredicate, }; #[derive(Default)] @@ -19,6 +20,7 @@ pub struct RequiredTraitBound { source_code: bool, into_source_span: bool, std_into_iter: bool, + std_clone: bool, } impl RequiredTraitBound { @@ -54,6 +56,12 @@ impl RequiredTraitBound { ))) } + if self.std_clone { + bounds.push(TypeParamBound::Trait(syn::parse_quote!( + ::std::clone::Clone + ))) + } + if self.r#static { bounds.push(TypeParamBound::Lifetime(syn::parse_quote!('static))) } @@ -70,8 +78,9 @@ impl RequiredTraitBound { self.source_code = true; } - fn register_source_span_usage(&mut self) { + fn register_label_usage(&mut self) { self.into_source_span = true; + self.std_clone = true; } fn register_collection_usage(&mut self) { @@ -88,7 +97,7 @@ impl RequiredTraitBound { } } -pub struct TraitBoundStore(HashMap); +pub struct TraitBoundStore(HashMap<(Option, Type), RequiredTraitBound>); impl TraitBoundStore { pub fn new(generics: &Generics) -> Self { @@ -114,12 +123,41 @@ impl TraitBoundStore { }, }) }) - .map(|v| (v, RequiredTraitBound::default())) + .map(|v| ((None, v), RequiredTraitBound::default())) .collect::>(); Self(hash_map) } + pub fn extract_option(r#type: &Type) -> Option<&Type> { + if let syn::Type::Path(syn::TypePath { + path: syn::Path { segments, .. }, + .. + }) = r#type + { + segments + .last() + .filter(|seg| seg.ident == "Option") + .and_then(|seg| match &seg.arguments { + PathArguments::AngleBracketed(AngleBracketedGenericArguments { + args, .. + }) => { + let mut iter = args.iter(); + + let ty = iter.next(); + iter.next().xor(ty) + } + _ => None, + }) + .and_then(|arg| match arg { + GenericArgument::Type(ty) => Some(ty), + _ => None, + }) + } else { + None + } + } + fn check_generic_usage<'ty>(&self, mut r#type: &'ty Type) -> Option<&'ty Type> { // in theory we could skip all this logic and just allow trivial bounds but that would add redundant trait bounds // to the derived impl - would be another choice to make. I choose to filter as much as possible so that we don't @@ -161,7 +199,7 @@ impl TraitBoundStore { // // this is also the "bottom" check since we add the generics themselves to the map when // constructing self - if self.0.contains_key(elem) { + if self.0.contains_key(&(None, elem.clone())) { depends_on_generic = true; break; } @@ -250,10 +288,6 @@ impl TraitBoundStore { } pub fn merge_with(&self, where_clause: Option<&WhereClause>) -> Option { - if self.0.is_empty() { - return where_clause.cloned(); - } - let mut where_clause = where_clause.cloned().unwrap_or_else(|| WhereClause { where_token: Token![where](Span::mixed_site()), predicates: Punctuated::new(), @@ -261,15 +295,41 @@ impl TraitBoundStore { where_clause .predicates - .extend(self.0.iter().map(|(ty, bounds)| { - WherePredicate::Type(PredicateType { - lifetimes: None, - bounded_ty: ty.clone(), - colon_token: Token![:](Span::mixed_site()), - bounds: bounds.to_bounds(), + .extend(self.0.iter().filter_map(|(ty, bounds)| { + let bounds = bounds.to_bounds(); + (!bounds.is_empty()).then(|| { + WherePredicate::Type(PredicateType { + lifetimes: ty.0.clone(), + bounded_ty: ty.1.clone(), + colon_token: Token![:](Span::mixed_site()), + bounds, + }) }) })); + where_clause + .predicates + .push(WherePredicate::Type(PredicateType { + lifetimes: None, + bounded_ty: Type::Path(TypePath { + qself: None, + path: Path { + leading_colon: None, + segments: Punctuated::from_iter(once(PathSegment { + ident: syn::Ident::new("Self", Span::mixed_site()), + arguments: PathArguments::None, + })), + }, + }), + colon_token: syn::Token![:](Span::mixed_site()), + bounds: Punctuated::from_iter(once(TypeParamBound::Trait(TraitBound { + paren_token: None, + modifier: syn::TraitBoundModifier::None, + lifetimes: None, + path: syn::parse_quote!(::std::error::Error), + }))), + })); + Some(where_clause) } @@ -278,7 +338,7 @@ impl TraitBoundStore { return; }; - let type_opts = self.0.entry(r#type.clone()).or_default(); + let type_opts = self.0.entry((None, r#type.clone())).or_default(); type_opts.register_transparent_usage() } @@ -287,32 +347,45 @@ impl TraitBoundStore { return; }; - let type_opts = self.0.entry(r#type.clone()).or_default(); + let type_opts = self.0.entry((None, r#type.clone())).or_default(); type_opts.register_source_code_usage() } - pub fn register_source_span_usage(&mut self, r#type: &Type) { + 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 { return; }; - let type_opts = self.0.entry(r#type.clone()).or_default(); - type_opts.register_source_span_usage() + let type_opts = self.0.entry((None, r#type.clone())).or_default(); + type_opts.register_label_usage() } - pub fn register_source_span_collection_usage(&mut self, r#type: &Type) { + pub fn register_label_collection_usage(&mut self, r#type: &Type) { let Some(ty) = self.check_generic_usage(r#type) else { return; }; - let type_opts = self.0.entry(ty.clone()).or_default(); + let ty: syn::Type = syn::parse_quote!(&'__miette_internal_lt #ty); + + let type_opts = self + .0 + .entry(( + Some(syn::parse_quote!(for<'__miette_internal_lt>)), + ty.clone(), + )) + .or_default(); type_opts.register_collection_usage(); let type_opts_item = self .0 - .entry(syn::parse_quote!(<#ty as ::std::iter::IntoIterator>::Item)) + .entry(( + Some(syn::parse_quote!(for<'__miette_internal_lt>)), + syn::parse_quote!(<#ty as ::std::iter::IntoIterator>::Item), + )) .or_default(); - type_opts_item.register_source_span_usage(); + type_opts_item.register_label_usage(); } pub fn register_related_usage(&mut self, r#type: &Type) { @@ -320,22 +393,29 @@ impl TraitBoundStore { return; }; - let type_opts = self.0.entry(ty.clone()).or_default(); - type_opts.register_collection_usage(); - + // this is somewhat hacky and only supports concrete types for the #[related] type + // ittself but supports generics for the arguments, i.e. Vec where T is generic. + // + // I think that this is a current limitation of the design of the Diagnostic trait, + // since we'd need bounds on the method and we can't do that (to refer to the lifetime) + // + // Someone smarter than me might be able to figure out a better solution (?) let type_opts_item = self .0 - .entry(syn::parse_quote!(<#ty as ::std::iter::IntoIterator>::Item)) + .entry(( + None, + syn::parse_quote!(<#ty as ::std::iter::IntoIterator>::Item), + )) .or_default(); type_opts_item.register_related_item_usage(); } - pub(crate) fn register_source_usage(&mut self, r#type: &Type) { + pub fn register_source_usage(&mut self, r#type: &Type) { let Some(ty) = self.check_generic_usage(r#type) else { return; }; - let type_opts = self.0.entry(ty.clone()).or_default(); + let type_opts = self.0.entry((None, ty.clone())).or_default(); type_opts.register_source_usage(); } } diff --git a/tests/derive.rs b/tests/derive.rs index aa631dc..c141e44 100644 --- a/tests/derive.rs +++ b/tests/derive.rs @@ -217,6 +217,8 @@ fn fmt_help() { #[diagnostic(code(foo::x), help("{} x {len} x {:?}", 1, "2"))] Y { len: usize }, + // for some reason rust analyzer has a false positive with the self = self in the format! + // here but it compiles and tests just fine. (02/02/2025) #[diagnostic(code(foo::x), help("{} x {self:?} x {:?}", 1, "2"))] Z, } From 37f9e979de2cf6928b28e17bd0fd8da7bd8412ed Mon Sep 17 00:00:00 2001 From: Justus Fluegel Date: Mon, 3 Feb 2025 01:35:54 +0100 Subject: [PATCH 3/9] 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::>(); +} From e42637d428a85bd198a9c398319e4404d099bf90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Justus=20Fl=C3=BCgel?= Date: Mon, 10 Mar 2025 15:57:15 +0100 Subject: [PATCH 4/9] Add perfect-derive feature flag to both miette and miette-derive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Justus Flügel --- Cargo.toml | 4 ++++ miette-derive/Cargo.toml | 3 +++ 2 files changed, 7 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index f89f14b..24e52ef 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,6 +10,7 @@ license = "Apache-2.0" readme = "README.md" edition = "2018" rust-version = "1.82.0" +resolver = "2" exclude = ["images/", "tests/", "miette-derive/"] [dependencies] @@ -31,6 +32,8 @@ syntect = { version = "5.1.0", optional = true } [dev-dependencies] thiserror = "2.0.11" semver = "1.0.21" +# (kind of) hacky workaround to enable additional feature flags in tests, requires resolver = "2" +miette = { path = ".", features = ["perfect-derive"] } # Eyre devdeps futures = { version = "0.3", default-features = false } @@ -47,6 +50,7 @@ strip-ansi-escapes = "0.2.0" [features] default = ["derive"] derive = ["dep:miette-derive"] +perfect-derive = ["dep:miette-derive","miette-derive?/perfect-derive"] no-format-args-capture = [] fancy-base = [ "dep:owo-colors", diff --git a/miette-derive/Cargo.toml b/miette-derive/Cargo.toml index e281272..ae8c3b6 100644 --- a/miette-derive/Cargo.toml +++ b/miette-derive/Cargo.toml @@ -10,6 +10,9 @@ repository = "https://github.com/zkat/miette" [lib] proc-macro = true +[features] +perfect-derive = ["syn/extra-traits"] + [dependencies] proc-macro2 = "1.0.83" quote = "1.0.35" From a23114fffe34ad21515948349b12055ea892feda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Justus=20Fl=C3=BCgel?= Date: Mon, 10 Mar 2025 15:58:43 +0100 Subject: [PATCH 5/9] Make rustfmt no longer complain over lines that are too long which it can't format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Justus Flügel --- miette-derive/src/code.rs | 18 +++++++++++++++--- miette-derive/src/diagnostic.rs | 8 ++++++-- miette-derive/src/forward.rs | 8 ++++++-- miette-derive/src/help.rs | 12 +++++++++--- miette-derive/src/label.rs | 28 +++++++++++++++++++++++----- miette-derive/src/related.rs | 4 +++- miette-derive/src/severity.rs | 8 +++++--- miette-derive/src/trait_bounds.rs | 18 ++++++++++++++---- miette-derive/src/url.rs | 4 +++- src/handlers/graphical.rs | 16 ++++++++++------ 10 files changed, 94 insertions(+), 30 deletions(-) diff --git a/miette-derive/src/code.rs b/miette-derive/src/code.rs index 22dc795..4cf63cf 100644 --- a/miette-derive/src/code.rs +++ b/miette-derive/src/code.rs @@ -56,13 +56,25 @@ impl Code { let code = &code.as_ref()?.0; Some(match fields { syn::Fields::Named(_) => { - quote! { Self::#ident { .. } => std::option::Option::Some(std::boxed::Box::new(#code)), } + quote! { + Self::#ident { .. } => { + std::option::Option::Some(std::boxed::Box::new(#code)) + }, + } } syn::Fields::Unnamed(_) => { - quote! { Self::#ident(..) => std::option::Option::Some(std::boxed::Box::new(#code)), } + quote! { + Self::#ident(..) => { + std::option::Option::Some(std::boxed::Box::new(#code)) + }, + } } syn::Fields::Unit => { - quote! { Self::#ident => std::option::Option::Some(std::boxed::Box::new(#code)), } + quote! { + Self::#ident => { + std::option::Option::Some(std::boxed::Box::new(#code)) + }, + } } }) }, diff --git a/miette-derive/src/diagnostic.rs b/miette-derive/src/diagnostic.rs index 3206e83..629051e 100644 --- a/miette-derive/src/diagnostic.rs +++ b/miette-derive/src/diagnostic.rs @@ -312,7 +312,9 @@ impl Diagnostic { forward.gen_struct_method(WhichFn::DiagnosticSource); quote! { - impl #impl_generics miette::Diagnostic for #ident #ty_generics #where_clause { + impl #impl_generics miette::Diagnostic + for #ident #ty_generics + #where_clause { #code_method #help_method #url_method @@ -372,7 +374,9 @@ impl Diagnostic { .and_then(|x| x.gen_struct()) .or_else(|| forward(WhichFn::DiagnosticSource)); quote! { - impl #impl_generics miette::Diagnostic for #ident #ty_generics #where_clause { + impl #impl_generics miette::Diagnostic + for #ident #ty_generics + #where_clause { #code_body #help_body #sev_body diff --git a/miette-derive/src/forward.rs b/miette-derive/src/forward.rs index 88c437c..11cb3bb 100644 --- a/miette-derive/src/forward.rs +++ b/miette-derive/src/forward.rs @@ -72,10 +72,14 @@ impl WhichFn { fn severity(&self) -> std::option::Option }, Self::Related => quote! { - fn related(&self) -> std::option::Option + '_>> + fn related(&self) -> std::option::Option< + std::boxed::Box + '_> + > }, Self::Labels => quote! { - fn labels(&self) -> std::option::Option + '_>> + fn labels(&self) -> std::option::Option< + std::boxed::Box + '_> + > }, Self::SourceCode => quote! { fn source_code(&self) -> std::option::Option<&dyn miette::SourceCode> diff --git a/miette-derive/src/help.rs b/miette-derive/src/help.rs index 1c21054..398533c 100644 --- a/miette-derive/src/help.rs +++ b/miette-derive/src/help.rs @@ -94,7 +94,9 @@ impl Help { Help::Display(display) => { let (fmt, args) = display.expand_shorthand_cloned(&display_members); Some(quote! { - Self::#ident #display_pat => std::option::Option::Some(std::boxed::Box::new(format!(#fmt #args))), + Self::#ident #display_pat => { + std::option::Option::Some(std::boxed::Box::new(format!(#fmt #args))) + }, }) } Help::Field(member, ty) => { @@ -123,7 +125,9 @@ impl Help { Help::Display(display) => { let (fmt, args) = display.expand_shorthand_cloned(&display_members); Some(quote! { - fn help(&self) -> std::option::Option> { + fn help(&self) -> std::option::Option< + std::boxed::Box + > { #[allow(unused_variables, deprecated)] let Self #display_pat = self; std::option::Option::Some(std::boxed::Box::new(format!(#fmt #args))) @@ -133,7 +137,9 @@ impl Help { Help::Field(member, ty) => { let var = quote! { __miette_internal_var }; Some(quote! { - fn help(&self) -> std::option::Option> { + fn help(&self) -> std::option::Option< + std::boxed::Box + > { #[allow(unused_variables, deprecated)] let Self #display_pat = self; use miette::macro_helpers::ToOption; diff --git a/miette-derive/src/label.rs b/miette-derive/src/label.rs index 7fa1404..191b7b3 100644 --- a/miette-derive/src/label.rs +++ b/miette-derive/src/label.rs @@ -253,7 +253,9 @@ impl Labels { Some(quote! { #[allow(unused_variables)] - fn labels(&self) -> std::option::Option + '_>> { + fn labels(&self) -> std::option::Option< + std::boxed::Box + '_> + > { use miette::macro_helpers::ToOption; let Self #display_pat = self; @@ -263,7 +265,10 @@ impl Labels { .into_iter() #(#collections_chain)*; - std::option::Option::Some(Box::new(labels_iter.filter(Option::is_some).map(Option::unwrap))) + std::option::Option::Some(Box::new( + labels_iter + .filter_map(|x| x) + )) } }) } @@ -276,7 +281,12 @@ impl Labels { let (display_pat, display_members) = display_pat_members(fields); labels.as_ref().and_then(|labels| { let variant_labels = labels.0.iter().filter_map(|label| { - let Label { span, label, ty, lbl_ty } = label; + let Label { + span, + label, + ty, + lbl_ty, + } = label; if *lbl_ty == LabelType::Collection { return None; } @@ -313,7 +323,12 @@ impl Labels { }) }); let collections_chain = labels.0.iter().filter_map(|label| { - let Label { span, label, ty: _, lbl_ty } = label; + let Label { + span, + label, + ty: _, + lbl_ty, + } = label; if *lbl_ty != LabelType::Collection { return None; } @@ -357,7 +372,10 @@ impl Labels { ] .into_iter() #(#collections_chain)*; - std::option::Option::Some(std::boxed::Box::new(labels_iter.filter(Option::is_some).map(Option::unwrap))) + std::option::Option::Some(std::boxed::Box::new( + labels_iter + .filter_map(|x| x) + )) } }), } diff --git a/miette-derive/src/related.rs b/miette-derive/src/related.rs index de189cf..cd87899 100644 --- a/miette-derive/src/related.rs +++ b/miette-derive/src/related.rs @@ -78,7 +78,9 @@ impl Related { pub(crate) fn gen_struct(&self) -> Option { let rel = &self.0; Some(quote! { - fn related<'a>(&'a self) -> std::option::Option + 'a>> { + fn related<'a>(&'a self) -> std::option::Option< + std::boxed::Box + 'a> + > { use ::core::borrow::Borrow; std::option::Option::Some(std::boxed::Box::new( self.#rel.iter().map(|x| -> &(dyn miette::Diagnostic) { &*x.borrow() }) diff --git a/miette-derive/src/severity.rs b/miette-derive/src/severity.rs index 4f26e4e..9d66d41 100644 --- a/miette-derive/src/severity.rs +++ b/miette-derive/src/severity.rs @@ -71,9 +71,11 @@ impl Severity { syn::Fields::Unnamed(_) => quote! { (..) }, syn::Fields::Unit => quote! {}, }; - Some( - quote! { Self::#ident #fields => std::option::Option::Some(miette::Severity::#severity), }, - ) + Some(quote! { + Self::#ident #fields => { + std::option::Option::Some(miette::Severity::#severity) + }, + }) }, ) } diff --git a/miette-derive/src/trait_bounds.rs b/miette-derive/src/trait_bounds.rs index a1912ca..e69d3c1 100644 --- a/miette-derive/src/trait_bounds.rs +++ b/miette-derive/src/trait_bounds.rs @@ -412,8 +412,10 @@ impl TraitBoundStore { 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), + 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(); @@ -421,8 +423,16 @@ impl TraitBoundStore { 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), + 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(); diff --git a/miette-derive/src/url.rs b/miette-derive/src/url.rs index 734d1a4..70af09d 100644 --- a/miette-derive/src/url.rs +++ b/miette-derive/src/url.rs @@ -96,7 +96,9 @@ impl Url { } }; Some(quote! { - Self::#ident #pat => std::option::Option::Some(std::boxed::Box::new(format!(#fmt #args))), + Self::#ident #pat => { + std::option::Option::Some(std::boxed::Box::new(format!(#fmt #args))) + }, }) }, ) diff --git a/src/handlers/graphical.rs b/src/handlers/graphical.rs index 37b6bf8..90e457f 100644 --- a/src/handlers/graphical.rs +++ b/src/handlers/graphical.rs @@ -1401,13 +1401,17 @@ impl Line { /// text on this line fn span_applies(&self, span: &FancySpan) -> bool { let spanlen = if span.len() == 0 { 1 } else { span.len() }; - // Span starts in this line - (span.offset() >= self.offset && span.offset() < self.offset + self.length) - // Span passes through this line - || (span.offset() < self.offset && span.offset() + spanlen > self.offset + self.length) //todo - // Span ends on this line - || (span.offset() + spanlen > self.offset && span.offset() + spanlen <= self.offset + self.length) + let span_starts_this_line = + span.offset() >= self.offset && span.offset() < self.offset + self.length; + + let span_passes_through_this_line = + span.offset() < self.offset && span.offset() + spanlen > self.offset + self.length; + + let span_ends_on_this_line = span.offset() + spanlen > self.offset + && span.offset() + spanlen <= self.offset + self.length; + + span_starts_this_line || span_passes_through_this_line || span_ends_on_this_line } /// Returns whether `span` should be visible on this line in the gutter (so this excludes spans From ff32d81cb0e67e10ff3b780bdf79cb9c9bc2a7da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Justus=20Fl=C3=BCgel?= Date: Mon, 10 Mar 2025 21:36:11 +0100 Subject: [PATCH 6/9] Simplify the trait bound store and just register where clauses directly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Justus Flügel --- miette-derive/src/diagnostic.rs | 18 +- miette-derive/src/diagnostic_source.rs | 11 +- miette-derive/src/forward.rs | 12 +- miette-derive/src/label.rs | 34 +- miette-derive/src/related.rs | 18 +- miette-derive/src/source_code.rs | 20 +- miette-derive/src/trait_bounds.rs | 412 ++++++------------------- miette-derive/src/utils.rs | 35 ++- 8 files changed, 200 insertions(+), 360 deletions(-) diff --git a/miette-derive/src/diagnostic.rs b/miette-derive/src/diagnostic.rs index 629051e..df963f0 100644 --- a/miette-derive/src/diagnostic.rs +++ b/miette-derive/src/diagnostic.rs @@ -11,7 +11,7 @@ use crate::label::Labels; use crate::related::Related; use crate::severity::Severity; use crate::source_code::SourceCode; -use crate::trait_bounds::TraitBoundStore; +use crate::trait_bounds::TypeParamBoundStore; use crate::url::Url; pub enum Diagnostic { @@ -20,13 +20,13 @@ pub enum Diagnostic { ident: syn::Ident, fields: syn::Fields, args: DiagnosticDefArgs, - bound_store: TraitBoundStore, + bound_store: TypeParamBoundStore, }, Enum { ident: syn::Ident, generics: syn::Generics, variants: Vec, - bound_store: TraitBoundStore, + bound_store: TypeParamBoundStore, }, } @@ -76,7 +76,7 @@ pub struct DiagnosticConcreteArgs { impl DiagnosticConcreteArgs { fn for_fields( fields: &syn::Fields, - bounds_store: &mut TraitBoundStore, + bounds_store: &mut TypeParamBoundStore, ) -> Result { let labels = Labels::from_fields(fields, bounds_store)?; let source_code = SourceCode::from_fields(fields, bounds_store)?; @@ -162,7 +162,7 @@ impl DiagnosticDefArgs { _ident: &syn::Ident, fields: &syn::Fields, attrs: &[&syn::Attribute], - bounds_store: &mut TraitBoundStore, + bounds_store: &mut TypeParamBoundStore, allow_transparent: bool, ) -> syn::Result { let mut errors = Vec::new(); @@ -233,7 +233,7 @@ impl Diagnostic { .collect::>(); Ok(match input.data { syn::Data::Struct(data_struct) => { - let mut bounds_store = TraitBoundStore::new(&input.generics); + let mut bounds_store = TypeParamBoundStore::new(&input.generics); let args = DiagnosticDefArgs::parse( &input.ident, @@ -253,7 +253,7 @@ impl Diagnostic { } syn::Data::Enum(syn::DataEnum { variants, .. }) => { let mut vars = Vec::new(); - let mut bound_store = TraitBoundStore::new(&input.generics); + let mut bound_store = TypeParamBoundStore::new(&input.generics); for var in variants { let mut variant_attrs = input_attrs.clone(); variant_attrs @@ -297,7 +297,7 @@ impl Diagnostic { bound_store, } => { let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); - let where_clause = bound_store.merge_with(where_clause); + let where_clause = bound_store.add_to_where_clause(where_clause); match args { DiagnosticDefArgs::Transparent(forward) => { @@ -397,7 +397,7 @@ impl Diagnostic { bound_store, } => { let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); - let where_clause = bound_store.merge_with(where_clause); + let where_clause = bound_store.add_to_where_clause(where_clause); let code_body = Code::gen_enum(variants); let help_body = Help::gen_enum(variants); diff --git a/miette-derive/src/diagnostic_source.rs b/miette-derive/src/diagnostic_source.rs index 8e66e29..4fc9c28 100644 --- a/miette-derive/src/diagnostic_source.rs +++ b/miette-derive/src/diagnostic_source.rs @@ -3,7 +3,7 @@ use quote::quote; use syn::spanned::Spanned; use crate::forward::WhichFn; -use crate::trait_bounds::TraitBoundStore; +use crate::trait_bounds::TypeParamBoundStore; use crate::{ diagnostic::{DiagnosticConcreteArgs, DiagnosticDef}, utils::{display_pat_members, gen_all_variants_with}, @@ -14,7 +14,7 @@ pub struct DiagnosticSource(syn::Member); impl DiagnosticSource { pub(crate) fn from_fields( fields: &syn::Fields, - bounds_store: &mut TraitBoundStore, + bounds_store: &mut TypeParamBoundStore, ) -> syn::Result> { match fields { syn::Fields::Named(named) => { @@ -29,7 +29,7 @@ impl DiagnosticSource { fn from_fields_vec( fields: Vec<&syn::Field>, - bounds_store: &mut TraitBoundStore, + bounds_store: &mut TypeParamBoundStore, ) -> syn::Result> { for (i, field) in fields.iter().enumerate() { for attr in &field.attrs { @@ -43,7 +43,10 @@ impl DiagnosticSource { }) }; - bounds_store.register_source_usage(&field.ty); + let ty = &field.ty; + bounds_store.add_where_predicate( + syn::parse_quote!(#ty: ::miette::Diagnostic + 'static), + ); return Ok(Some(DiagnosticSource(diagnostic_source))); } diff --git a/miette-derive/src/forward.rs b/miette-derive/src/forward.rs index 11cb3bb..33d87c2 100644 --- a/miette-derive/src/forward.rs +++ b/miette-derive/src/forward.rs @@ -6,7 +6,7 @@ use syn::{ spanned::Spanned, }; -use crate::trait_bounds::TraitBoundStore; +use crate::trait_bounds::TypeParamBoundStore; pub enum Forward { Unnamed(usize), @@ -98,7 +98,7 @@ impl WhichFn { impl Forward { pub fn for_transparent_field( fields: &syn::Fields, - bounds_store: &mut TraitBoundStore, + bounds_store: &mut TypeParamBoundStore, ) -> syn::Result { let make_err = || { syn::Error::new( @@ -118,7 +118,9 @@ impl Forward { .clone() .unwrap_or_else(|| format_ident!("unnamed")); - bounds_store.register_transparent_usage(&field.ty); + let ty = &field.ty; + bounds_store + .add_where_predicate(syn::parse_quote! {#ty: ::miette::Diagnostic + 'static}); Ok(Self::Named(field_name)) } syn::Fields::Unnamed(unnamed) => { @@ -128,7 +130,9 @@ impl Forward { return Err(make_err()); } - bounds_store.register_transparent_usage(&field.ty); + let ty = &field.ty; + bounds_store + .add_where_predicate(syn::parse_quote! {#ty: ::miette::Diagnostic + 'static}); Ok(Self::Unnamed(0)) } _ => Err(syn::Error::new( diff --git a/miette-derive/src/label.rs b/miette-derive/src/label.rs index 191b7b3..3cc071b 100644 --- a/miette-derive/src/label.rs +++ b/miette-derive/src/label.rs @@ -4,15 +4,15 @@ use syn::{ parenthesized, parse::{Parse, ParseStream}, spanned::Spanned, - Token, + Lifetime, Token, }; use crate::{ diagnostic::{DiagnosticConcreteArgs, DiagnosticDef}, fmt::{self, Display}, forward::WhichFn, - trait_bounds::TraitBoundStore, - utils::{display_pat_members, gen_all_variants_with}, + trait_bounds::TypeParamBoundStore, + utils::{display_pat_members, extract_option, gen_all_variants_with}, }; pub struct Labels(Vec