fix: span for conflicting error

Author: ModProg

CHANGELOG.md

@@ -4,6 +4,16 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.5.0] - 2023-03-02
+
+### Added
+- `IdentValue` to keep hold of both value and the source ident
+
+### Changed
+- **Breaking Change**: `ConvertParsed::aggregate()` now takes/returns
+  `IdentValue`
+- Improved span for conflicting values
+
 ## [0.4.0] - 2023-03-02
 ### Changed
 - **Breaking Change**: Moved some syn types behind feature `full`
@@ -12,5 +22,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **Breaking Change**: Compile time verify if ident is given through helper
   trait
 
-[unreleased]: https://github.com/ModProg/attribute-derive/compare/v0.4.0...HEAD
+[unreleased]: https://github.com/ModProg/attribute-derive/compare/v0.5.0...HEAD
+[0.5.0]: https://github.com/ModProg/attribute-derive/compare/v0.3.1...v0.5.0
 [0.4.0]: https://github.com/ModProg/attribute-derive/compare/v0.3.1...v0.4.0

Cargo.toml

@@ -23,7 +23,7 @@ syn =  "1"
 syn-full = ["syn/full"]
 
 [dependencies.attribute-derive-macro]
-version = "0.4.0"
+version = "0.5.0"
 path = "macro"
 
 [workspace]

example/tests/ui/custom.stderr

@@ -19,16 +19,16 @@ error: missing flag `mandatory_flag`
   = note: this error originates in the derive macro `Custom` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: conflict_a !!! conflict_b
-  --> tests/ui/custom.rs:23:18
+  --> tests/ui/custom.rs:23:5
    |
 23 | #[a(conflict_a = "hey")]
-   |                  ^^^^^
+   |     ^^^^^^^^^^
 
 error: conflict_b !!! conflict_a
-  --> tests/ui/custom.rs:24:18
+  --> tests/ui/custom.rs:24:5
    |
 24 | #[b(conflict_b = "hi")]
-   |                  ^^^^
+   |     ^^^^^^^^^^
 
 error: found `hello` but expected one of `optional_implicit`, `optional_explicit`, `optional_default`, `default`, `conflict_a`, `conflict_b`, `example`, `flag`, `mandatory_flag`
   --> tests/ui/custom.rs:28:9

example/tests/ui/normal.stderr

@@ -19,16 +19,16 @@ error: required `mandatory_flag` is not specified
   = note: this error originates in the derive macro `Normal` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: `conflict_a` conflicts with mutually exclusive `conflict_b`
-  --> tests/ui/normal.rs:23:18
+  --> tests/ui/normal.rs:23:5
    |
 23 | #[a(conflict_a = "hey")]
-   |                  ^^^^^
+   |     ^^^^^^^^^^
 
 error: `conflict_b` conflicts with mutually exclusive `conflict_a`
-  --> tests/ui/normal.rs:24:18
+  --> tests/ui/normal.rs:24:5
    |
 24 | #[b(conflict_b = "hi")]
-   |                  ^^^^
+   |     ^^^^^^^^^^
 
 error: supported fields are `optional_implicit`, `optional_explicit`, `optional_default`, `default`, `conflict_a`, `conflict_b`, `example`, `flag` and `mandatory_flag`
   --> tests/ui/normal.rs:28:9

macro/Cargo.toml

@@ -7,7 +7,7 @@ keywords = ["derive", "macro"]
 license = "MIT"
 readme = "README.md"
 repository = "https://github.com/ModProg/attribute-derive"
-version = "0.4.0"
+version = "0.5.0"
 edition = "2021"
 name = "attribute-derive-macro"
 

macro/src/lib.rs

@@ -410,7 +410,7 @@ pub fn attribute_derive(input: proc_macro::TokenStream) -> proc_macro::TokenStre
                 }
 
                 options_ty
-                    .push(quote!(#ident: Option<<#ty as ::attribute_derive::ConvertParsed>::Type>));
+                    .push(quote!(#ident: Option<::attribute_derive::IdentValue<<#ty as ::attribute_derive::ConvertParsed>::Type>>));
 
                 let error = format(
                     struct_error.duplicate_field(),
@@ -468,22 +468,24 @@ pub fn attribute_derive(input: proc_macro::TokenStream) -> proc_macro::TokenStre
                 // } else {
                 parsing.push(quote! {
                         # use ::attribute_derive::__private::{syn, proc_macro2};
+                        # use ::attribute_derive::IdentValue;
                         #ident_str => {
-                            $parser.#ident = Some(
-                                if let Some(Some(__value)) = $is_flag.then(|| <#ty as ::attribute_derive::ConvertParsed>::as_flag()) {
-                                    __value
-                                } else {
-                                    $input.step(|__cursor| match __cursor.punct() {
-                                        Some((__punct, __rest))
-                                            if __punct.as_char() == '=' && __punct.spacing() == proc_macro2::Spacing::Alone =>
-                                        {
-                                            Ok(((), __rest))
-                                        }
-                                        _ => Err(__cursor.error("expected assignment `=`")),
-                                    })?;
-                                    syn::parse::Parse::parse($input)#error?
-                                }
-                            );
+                            $parser.#ident = Some(IdentValue{
+                                value: if let Some(Some(__value)) = $is_flag.then(|| <#ty as ::attribute_derive::ConvertParsed>::as_flag()) {
+                                        __value
+                                    } else {
+                                        $input.step(|__cursor| match __cursor.punct() {
+                                            Some((__punct, __rest))
+                                                if __punct.as_char() == '=' && __punct.spacing() == proc_macro2::Spacing::Alone =>
+                                            {
+                                                Ok(((), __rest))
+                                            }
+                                            _ => Err(__cursor.error("expected assignment `=`")),
+                                        })?;
+                                        syn::parse::Parse::parse($input)#error?
+                                    },
+                                ident: $variable
+                            });
                         }
                     });
                 // }
@@ -531,14 +533,14 @@ pub fn attribute_derive(input: proc_macro::TokenStream) -> proc_macro::TokenStre
                 assignments.push(match optional{
                     Some(true) => {
                         quote! {
-                            #ident: $parser.#ident.map(|t| ::attribute_derive::ConvertParsed::convert(t)).unwrap_or_else(|| Ok(#default))?
+                            #ident: $parser.#ident.map(|t| ::attribute_derive::ConvertParsed::convert(t.value)).unwrap_or_else(|| Ok(#default))?
                         }
                     }
                     Some(false) => {
                         quote! {
                             # use ::attribute_derive::__private::{syn, proc_macro2};
                             # use ::attribute_derive::{ConvertParsed};
-                            #ident: match $parser.#ident.map(|t| ConvertParsed::convert(t)) {
+                            #ident: match $parser.#ident.map(|t| ConvertParsed::convert(t.value)) {
                                     Some(__option) => __option?,
                                     None if <#ty as ConvertParsed>::as_flag().is_some() => Err(syn::Error::new(proc_macro2::Span::call_site(), #flag_error))?,
                                     _ => Err(syn::Error::new(proc_macro2::Span::call_site(), #field_error))?,
@@ -549,7 +551,7 @@ pub fn attribute_derive(input: proc_macro::TokenStream) -> proc_macro::TokenStre
                         quote! {
                             # use ::attribute_derive::__private::{syn, proc_macro2};
                             # use ::attribute_derive::{ConvertParsed};
-                            #ident: match $parser.#ident.map(|t| ConvertParsed::convert(t)) {
+                            #ident: match $parser.#ident.map(|t| ConvertParsed::convert(t.value)) {
                                     Some(__option) => __option?,
                                     None if <#ty as ConvertParsed>::default_by_default() => <#ty as ConvertParsed>::default(),
                                     _ => Err(syn::Error::new(proc_macro2::Span::call_site(), #field_error))?,
@@ -583,11 +585,11 @@ pub fn attribute_derive(input: proc_macro::TokenStream) -> proc_macro::TokenStre
             # use ::attribute_derive::__private::proc_macro2;
             # use ::attribute_derive::__private::syn::{Error, Spanned};
             if let (Some($a), Some($b)) = (&$parser.#a, &$parser.#b) {
-                if let Some($joined_span) = $a.span().join($b.span()) {
+                if let Some($joined_span) = $a.ident.span().join($b.ident.span()) {
                     return Err(Error::new($joined_span, #error_a_to_b));
                 } else {
-                    let mut $error = Error::new_spanned(&$a, #error_a_to_b);
-                    $error.combine(Error::new_spanned(&$b, #error_b_to_a));
+                    let mut $error = Error::new_spanned(&$a.ident, #error_a_to_b);
+                    $error.combine(Error::new_spanned(&$b.ident, #error_b_to_a));
                     return Err($error);
                 }
             }

src/lib.rs

@@ -170,7 +170,7 @@ use syn::{
 
 #[doc(hidden)]
 pub mod __private {
-    pub use {proc_macro2, syn};
+    pub use {proc_macro2, quote, syn};
 }
 
 /// Helper trait providing the path for an attribute.
@@ -393,16 +393,16 @@ where
     /// Currently this is only implemented for [`Arrays`](Array)
     #[allow(unused)]
     fn aggregate(
-        this: Option<Self::Type>,
-        other: Option<Self::Type>,
+        this: Option<IdentValue<Self::Type>>,
+        other: Option<IdentValue<Self::Type>>,
         error_msg: &str,
-    ) -> Result<Option<Self::Type>> {
+    ) -> Result<Option<IdentValue<Self::Type>>> {
         match (this, other) {
             (None, value) => Ok(value),
             (value, None) => Ok(value),
             (Some(this), Some(other)) => {
-                let mut error = this.error(error_msg);
-                syn::Error::combine(&mut error, other.error(error_msg));
+                let mut error = this.ident.error(error_msg);
+                syn::Error::combine(&mut error, other.ident.error(error_msg));
                 Err(error)
             }
         }
@@ -425,6 +425,14 @@ where
     }
 }
 
+/// Helper struct to hold a value and the ident of its property
+pub struct IdentValue<T> {
+    /// The value
+    pub value: T,
+    /// The ident
+    pub ident: Ident,
+}
+
 /// Macro to easily implement [`ConvertParsed`] for syn types
 macro_rules! convert_parsed {
     ($(#[$meta:meta])* $type:path) => {
@@ -507,17 +515,22 @@ where
     }
 
     fn aggregate(
-        this: Option<Self::Type>,
-        other: Option<Self::Type>,
+        this: Option<IdentValue<Self::Type>>,
+        other: Option<IdentValue<Self::Type>>,
         _: &str,
-    ) -> Result<Option<Self::Type>> {
+    ) -> Result<Option<IdentValue<Self::Type>>> {
         Ok(match (this, other) {
             (None, None) => None,
             (None, value) => value,
             (value, None) => value,
             (Some(mut this), Some(other)) => {
-                this.data.extend_from_slice(&other.data);
-                this.span = this.span.join(other.span).unwrap_or(this.span);
+                this.value.data.extend_from_slice(&other.value.data);
+                this.value.span = this
+                    .value
+                    .span
+                    .join(other.value.span)
+                    .unwrap_or(this.value.span);
+                // TODO figure out what to do with the idents
                 Some(this)
             }
         })