From de39f410d5641480d9d7c40f7612f2377bbb3ec8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Sch=C3=A4fer?= Date: Fri, 2 May 2025 16:56:22 +0200 Subject: [PATCH] xso-proc: improve spans for error messages For codec-based error messages, this reduces the amount of errors per violation to one. For all others, it improves the placement of the error slightly, but we still get duplicates. I couldn't figure out the remaining discrepancies in the spans ... --- xso-proc/src/compound.rs | 4 +- xso-proc/src/field/attribute.rs | 20 ++++++--- xso-proc/src/field/child.rs | 6 ++- xso-proc/src/field/text.rs | 20 ++++++--- xso-proc/src/types.rs | 74 ++++++++++++++++++--------------- 5 files changed, 75 insertions(+), 49 deletions(-) diff --git a/xso-proc/src/compound.rs b/xso-proc/src/compound.rs index 802a3b45666714b655175d952d0e60f8cdd283a1..4200ffea76cda471411404ea4e32eff8c24e15c4 100644 --- a/xso-proc/src/compound.rs +++ b/xso-proc/src/compound.rs @@ -430,12 +430,14 @@ impl Compound { } => { let feed = feed_fn(builder.clone()); + let mut substate_data_ident = substate_data.clone(); + substate_data_ident.set_span(ty.span()); states.push(State::new_with_builder( state_name.clone(), builder_data_ident, &builder_data_ty, ).with_field( - substate_data, + &substate_data_ident, &builder, ).with_mut(substate_data).with_impl(quote! { match #feed(&mut #substate_data, ev, ctx)? { diff --git a/xso-proc/src/field/attribute.rs b/xso-proc/src/field/attribute.rs index ebe8d7409c3ac3797a556cf04d7c6976040619e3..2bd31764cf44ab9c84b65df2f2bc8495d8d7598e 100644 --- a/xso-proc/src/field/attribute.rs +++ b/xso-proc/src/field/attribute.rs @@ -9,8 +9,8 @@ //! In particular, it provides the `#[xml(attribute)]` implementation. use proc_macro2::Span; -use quote::{quote, ToTokens}; -use syn::*; +use quote::{quote, quote_spanned, ToTokens}; +use syn::{spanned::Spanned, *}; use std::borrow::Cow; @@ -115,8 +115,9 @@ impl Field for AttributeField { let finalize = match self.codec { Some(ref codec) => { - let decode = text_codec_decode_fn(ty.clone()); - quote! { + let span = codec.span(); + let decode = text_codec_decode_fn(ty.clone(), span); + quote_spanned! { span=> |value| #decode(&#codec, value) } } @@ -170,8 +171,15 @@ impl Field for AttributeField { let generator = match self.codec { Some(ref codec) => { - let encode = text_codec_encode_fn(ty.clone()); - quote! { #encode(&#codec, #bound_name)? } + let span = codec.span(); + let encode = text_codec_encode_fn(ty.clone(), span); + // NOTE: We need to fudge the span of `bound_name` here, + // because its span points outside the macro (the identifier + // of the field), which means that quote_spanned will not + // override it, which would make the error message ugly. + let mut bound_name = bound_name.clone(); + bound_name.set_span(span); + quote_spanned! { span=> #encode(&#codec, #bound_name)? } } None => { let as_optional_xml_text = as_optional_xml_text_fn(ty.clone()); diff --git a/xso-proc/src/field/child.rs b/xso-proc/src/field/child.rs index 14e71708ebcb06672180db06f61d55b71095d7eb..e210ae400f6fe57c37369700c5d3f4234d96368c 100644 --- a/xso-proc/src/field/child.rs +++ b/xso-proc/src/field/child.rs @@ -68,7 +68,8 @@ impl Field for ChildField { let from_events = from_events_fn(element_ty.clone()); - let matcher = quote! { #from_events(name, attrs, ctx) }; + let span = element_ty.span(); + let matcher = quote_spanned! { span=> #from_events(name, attrs, ctx) }; let builder = from_xml_builder_ty(element_ty.clone()); ( @@ -178,9 +179,10 @@ impl Field for ChildField { let as_xml_iter = as_xml_iter_fn(item_ty.clone()); let item_iter = item_iter_ty(item_ty.clone(), lifetime.clone()); + let span = item_ty.span(); ( TokenStream::default(), - quote! { #as_xml_iter(#bound_name)? }, + quote_spanned! { span=> #as_xml_iter(#bound_name)? }, item_iter, ) } diff --git a/xso-proc/src/field/text.rs b/xso-proc/src/field/text.rs index 88aa8e448f0d90626de5b45b967cb716bad5bcda..fc20bc9bb45ef3985aa2812ac2ba56d952665c57 100644 --- a/xso-proc/src/field/text.rs +++ b/xso-proc/src/field/text.rs @@ -9,8 +9,8 @@ //! In particular, it provides the `#[xml(text)]` implementation. use proc_macro2::Span; -use quote::quote; -use syn::*; +use quote::{quote, quote_spanned}; +use syn::{spanned::Spanned, *}; use crate::error_message::ParentRef; use crate::scope::{AsItemsScope, FromEventsScope}; @@ -38,8 +38,9 @@ impl Field for TextField { let field_access = scope.access_field(member); let finalize = match self.codec { Some(ref codec) => { - let decode = text_codec_decode_fn(ty.clone()); - quote! { + let span = codec.span(); + let decode = text_codec_decode_fn(ty.clone(), span); + quote_spanned! { span=> #decode(&#codec, #field_access)? } } @@ -71,8 +72,15 @@ impl Field for TextField { ) -> Result { let generator = match self.codec { Some(ref codec) => { - let encode = text_codec_encode_fn(ty.clone()); - quote! { #encode(&#codec, #bound_name)? } + let span = codec.span(); + let encode = text_codec_encode_fn(ty.clone(), span); + // NOTE: We need to fudge the span of `bound_name` here, + // because its span points outside the macro (the identifier + // of the field), which means that quote_spanned will not + // override it, which would make the error message ugly. + let mut bound_name = bound_name.clone(); + bound_name.set_span(span); + quote_spanned! { span=> #encode(&#codec, #bound_name)? } } None => { let as_xml_text = as_xml_text_fn(ty.clone()); diff --git a/xso-proc/src/types.rs b/xso-proc/src/types.rs index c4cf18cd9561c54e29cf3f2783a06706fd6f80b6..9ee858237bf0ca6400984e887ceaff30df816f15 100644 --- a/xso-proc/src/types.rs +++ b/xso-proc/src/types.rs @@ -316,43 +316,46 @@ pub(crate) fn as_xml_text_fn(ty: Type) -> Expr { /// Construct a [`syn::Path`] referring to `::xso::TextCodec::<#for_ty>`, /// returning the span of `for_ty` alongside it. -fn text_codec_of(for_ty: Type) -> (Span, Path) { - let span = for_ty.span(); - ( - span, - Path { - leading_colon: Some(syn::token::PathSep { - spans: [span, span], - }), - segments: [ - PathSegment { - ident: Ident::new("xso", span), - arguments: PathArguments::None, - }, - PathSegment { - ident: Ident::new("TextCodec", span), - arguments: PathArguments::AngleBracketed(AngleBracketedGenericArguments { - colon2_token: Some(syn::token::PathSep { - spans: [span, span], - }), - lt_token: syn::token::Lt { spans: [span] }, - args: [GenericArgument::Type(for_ty)].into_iter().collect(), - gt_token: syn::token::Gt { spans: [span] }, +/// +/// The span used is `codec_span`, in order to ensure that error messages +/// about a missing implementation point at the codec, not at the type. +fn text_codec_of(for_ty: Type, codec_span: Span) -> Path { + let span = codec_span; + Path { + leading_colon: Some(syn::token::PathSep { + spans: [span, span], + }), + segments: [ + PathSegment { + ident: Ident::new("xso", span), + arguments: PathArguments::None, + }, + PathSegment { + ident: Ident::new("TextCodec", span), + arguments: PathArguments::AngleBracketed(AngleBracketedGenericArguments { + colon2_token: Some(syn::token::PathSep { + spans: [span, span], }), - }, - ] - .into_iter() - .collect(), - }, - ) + lt_token: syn::token::Lt { spans: [span] }, + args: [GenericArgument::Type(for_ty)].into_iter().collect(), + gt_token: syn::token::Gt { spans: [span] }, + }), + }, + ] + .into_iter() + .collect(), + } } /// Construct a [`syn::Expr`] referring to /// `::xso::TextCodec::<#for_ty>::encode`. -pub(crate) fn text_codec_encode_fn(for_ty: Type) -> Expr { - let (span, mut path) = text_codec_of(for_ty); +/// +/// The span used is `codec_span`, in order to ensure that error messages +/// about a missing implementation point at the codec, not at the type. +pub(crate) fn text_codec_encode_fn(for_ty: Type, codec_span: Span) -> Expr { + let mut path = text_codec_of(for_ty, codec_span); path.segments.push(PathSegment { - ident: Ident::new("encode", span), + ident: Ident::new("encode", codec_span), arguments: PathArguments::None, }); Expr::Path(ExprPath { @@ -364,10 +367,13 @@ pub(crate) fn text_codec_encode_fn(for_ty: Type) -> Expr { /// Construct a [`syn::Expr`] referring to /// `::xso::TextCodec::<#for_ty>::decode`. -pub(crate) fn text_codec_decode_fn(for_ty: Type) -> Expr { - let (span, mut path) = text_codec_of(for_ty); +/// +/// The span used is `codec_span`, in order to ensure that error messages +/// about a missing implementation point at the codec, not at the type. +pub(crate) fn text_codec_decode_fn(for_ty: Type, codec_span: Span) -> Expr { + let mut path = text_codec_of(for_ty, codec_span); path.segments.push(PathSegment { - ident: Ident::new("decode", span), + ident: Ident::new("decode", codec_span), arguments: PathArguments::None, }); Expr::Path(ExprPath {