diff --git a/Cargo.lock b/Cargo.lock index f528850..31837a6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -113,7 +113,7 @@ dependencies = [ "proc-macro2", "quote", "serde", - "syn 3.0.2", + "syn 3.0.5", ] [[package]] @@ -129,9 +129,9 @@ dependencies = [ [[package]] name = "syn" -version = "3.0.2" +version = "3.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a207d6d6a2b7fc470b80443726053f18a2481b7e1eee970597051596567987a3" +checksum = "12df2e0110f65b775f769bb17ef989067a1d931b2eb822bd4346631eeada89f9" dependencies = [ "proc-macro2", "quote", @@ -155,7 +155,7 @@ dependencies = [ "quote", "serde", "serde_tokenstream", - "syn 3.0.2", + "syn 3.0.5", "trybuild", ] diff --git a/Cargo.toml b/Cargo.toml index 2d40871..6bc4a0d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,4 +22,4 @@ rust-version = "1.85" proc-macro2 = "1.0" quote = "1.0" serde = { version = "1.0", features = ["derive"] } -syn = { version = "3.0.2", features = ["full"] } +syn = { version = "3.0.5", features = ["full"] } diff --git a/README.md b/README.md index f9da48b..d3433fb 100644 --- a/README.md +++ b/README.md @@ -182,6 +182,34 @@ wrapper around `syn::Parse` that implements `Deserialize`. The latter is useful for passing in, for example, a `syn::Path`, or other specific entities from the `syn` crate. +## String values with spans + +A `String` field accepts either a string literal or a bare identifier, but the +deserialized `String` doesn't record where it came from. If your macro +interprets a string further (as an identifier, a file name, and so on), +deserialize it as a `ParseWrapper` instead. It accepts the same +inputs and records the span of the token, so that errors about the value can +point at the value itself: + +```rust +#[derive(Deserialize)] +struct Config { + module: ParseWrapper, +} + +let config = from_tokenstream::(&attr)?; +let module = config.module.parse::().map_err(|err| { + syn::Error::new( + config.module.span(), + format!("`{}` is not a valid module name: {err}", config.module.value()), + ) +})?; +``` + +Given, for example, `module = "not a module"`, the error is reported at `"not a +module"`. Values parsed out of a `SpannedString` (via `parse` or `parse_with`) +also carry the span. + ## OrderedMap You may want to use the map syntax with keys that cannot be used by types such diff --git a/src/ibidem.rs b/src/ibidem.rs index 94f9c8d..e96bb9c 100644 --- a/src/ibidem.rs +++ b/src/ibidem.rs @@ -2,8 +2,9 @@ use std::cell::RefCell; -use proc_macro2::{TokenStream, TokenTree}; +use proc_macro2::{Span, TokenStream, TokenTree}; use serde::{Deserialize, de::Error, de::Visitor}; +use syn::ext::IdentExt; use crate::serde_tokenstream::spanned_error; @@ -141,6 +142,163 @@ impl std::ops::Deref for ParseWrapper

{ } } +/// A string value that remembers its span. +/// +/// Use this as [`ParseWrapper`]``. It accepts the same inputs +/// as [`String`] (a string literal or a bare identifier), but also records the +/// span of that token. Macros that interpret a string further (as an +/// identifier, a file name, and so on) can use the span to report errors at +/// the value itself rather than at the attribute as a whole. +/// +/// The [`parse`](Self::parse) and [`parse_with`](Self::parse_with) methods +/// parse the value as Rust syntax, similar to [`syn::LitStr::parse`]. The +/// resulting types and errors have the correct span information associated with +/// them. +/// +/// Equality and hashing compare only the value, not the span. +/// +/// # Example +/// +/// ``` +/// use quote::quote; +/// use serde::Deserialize; +/// use serde_tokenstream::{ParseWrapper, SpannedString, from_tokenstream}; +/// +/// #[derive(Deserialize)] +/// struct Config { +/// module: ParseWrapper, +/// } +/// +/// // In a proc macro, this would be the macro's input. +/// let attr = quote! { module = "not a module" }; +/// let config = from_tokenstream::(&attr)?; +/// +/// // Interpret the value further, reporting errors at the value rather than +/// // at the attribute as a whole. +/// let module = config.module.parse::().map_err(|err| { +/// syn::Error::new( +/// config.module.span(), +/// format!( +/// "`{}` is not a valid module name: {err}", +/// config.module.value() +/// ), +/// ) +/// }); +/// assert!(module.is_err()); +/// # Ok::<(), syn::Error>(()) +/// ``` +/// +/// # Limitations +/// +/// See the [`ParseWrapper`] documentation for limitations. +/// +/// [`from_tokenstream`]: crate::from_tokenstream +/// [`from_tokenstream_spanned`]: crate::from_tokenstream_spanned +#[derive(Debug, Clone)] +pub struct SpannedString { + value: String, + span: Span, +} + +impl SpannedString { + /// Creates a `SpannedString` from a value and a span. + /// + /// Deserializing a `ParseWrapper` is the usual way to + /// obtain a `SpannedString` -- this is for cases like default values and + /// tests. + pub fn new(value: impl Into, span: Span) -> Self { + Self { value: value.into(), span } + } + + /// Returns the value. + /// + /// In case of a string literal: + /// + /// - The quotes are stripped from the value. + /// - Escapes are processed, so that (e.g.) `"\n"` becomes a newline. + /// + /// Identifiers are stored verbatim, so raw identifiers keep the `r#` prefix. + pub fn value(&self) -> &str { + &self.value + } + + /// Returns the span of the token the value was written as. + pub fn span(&self) -> Span { + self.span + } + + /// Returns the value, discarding the span. + pub fn into_string(self) -> String { + self.value + } + + /// Parses the value as a `T`. + /// + /// In both success and error cases, the span points to this value. + pub fn parse(&self) -> syn::Result { + self.parse_with(T::parse) + } + + /// Invokes `parser` on the value. + /// + /// In both success and error cases, the span points to this value. + pub fn parse_with( + &self, + parser: F, + ) -> syn::Result { + syn::LitStr::new(&self.value, self.span).parse_with(parser) + } +} + +impl AsRef for SpannedString { + fn as_ref(&self) -> &str { + &self.value + } +} + +impl std::fmt::Display for SpannedString { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(&self.value) + } +} + +impl PartialEq for SpannedString { + fn eq(&self, other: &Self) -> bool { + self.value == other.value + } +} + +impl Eq for SpannedString {} + +impl std::hash::Hash for SpannedString { + fn hash(&self, state: &mut H) { + self.value.hash(state); + } +} + +impl syn::parse::Parse for SpannedString { + fn parse(input: syn::parse::ParseStream<'_>) -> syn::Result { + let value = if input.peek(syn::LitStr) { + let lit: syn::LitStr = input.parse()?; + Self { value: lit.value(), span: lit.span() } + } else if input.peek(syn::Ident::peek_any) { + // Keywords are accepted, as they are for `String`. + let ident = syn::Ident::parse_any(input)?; + Self { value: ident.to_string(), span: ident.span() } + } else { + return Err(match input.cursor().token_tree() { + Some((tt, _)) => spanned_error( + &tt, + format!("expected a string, but found `{tt}`"), + ), + None => input.error("expected a string"), + }); + }; + + Ok(value) + } +} + /// We would like to be able to pass `TokenStream`s through unperturbed, but /// that isn't directly possible with serde's model, because /// serde--wisely--does not permit this kind of unholy communion between diff --git a/src/lib.rs b/src/lib.rs index 050acf7..ab267b5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -53,12 +53,19 @@ //! //! For attributes that are nested inside a top-level macro, use the //! [`from_tokenstream_spanned`] function. See its help for an example. +//! +//! ## Values with spans +//! +//! To report errors at a particular value rather than at the attribute as a +//! whole, deserialize it as a [`ParseWrapper`]: over a `syn` type for Rust +//! syntax, or over [`SpannedString`] for a plain string or identifier. mod ibidem; mod ordered_map; mod serde_tokenstream; pub use crate::ibidem::ParseWrapper; +pub use crate::ibidem::SpannedString; pub use crate::ibidem::TokenStreamWrapper; pub use crate::ordered_map::OrderedMap; pub use crate::serde_tokenstream::Error; diff --git a/src/serde_tokenstream.rs b/src/serde_tokenstream.rs index 1f550b9..8c69fef 100644 --- a/src/serde_tokenstream.rs +++ b/src/serde_tokenstream.rs @@ -1344,7 +1344,7 @@ impl<'de> Deserializer<'de> for &mut TokenDe { #[cfg(test)] mod tests { - use crate::{ParseWrapper, ibidem::TokenStreamWrapper}; + use crate::{ParseWrapper, SpannedString, ibidem::TokenStreamWrapper}; use super::*; use quote::{ToTokens, quote}; @@ -2101,6 +2101,96 @@ mod tests { assert_eq!(d.thing, Thing::D { d: "d".to_string() }); } + #[test] + fn test_spanned_string() { + #[derive(Deserialize)] + struct Stuff { + lit: ParseWrapper, + ident: ParseWrapper, + keyword: ParseWrapper, + raw: ParseWrapper, + missing: Option>, + present: Option>, + many: Vec>, + } + + let Stuff { lit, ident, keyword, raw, missing, present, many } = + from_tokenstream::("e! { + lit = "howdy", + ident = word, + keyword = mod, + raw = r#type, + present = "here", + many = ["a", b], + }) + .unwrap(); + + // SpannedString accepts either a string literal or a bare identifier. + assert_eq!(lit.value(), "howdy"); + assert_eq!(ident.value(), "word"); + assert_eq!(keyword.value(), "mod"); + assert_eq!(raw.value(), "r#type"); + assert!(missing.is_none()); + assert_eq!(present.unwrap().value(), "here"); + assert_eq!( + many.iter().map(|s| s.value()).collect::>(), + ["a", "b"] + ); + + // The value can be parsed further. + assert_eq!(lit.parse::().unwrap(), "howdy"); + assert_eq!(ident.parse::().unwrap(), "word"); + assert_eq!(raw.parse::().unwrap(), "r#type"); + assert_eq!( + keyword.parse::().unwrap_err().to_string(), + "expected identifier, found keyword `mod`" + ); + assert_eq!( + lit.parse_with(syn::Path::parse_mod_style) + .unwrap() + .to_token_stream() + .to_string(), + "howdy" + ); + assert_eq!( + SpannedString::new("\"", proc_macro2::Span::call_site()) + .parse::() + .unwrap_err() + .to_string(), + "cannot parse string into token stream" + ); + + // Equality compares the value. + assert_eq!( + *lit, + SpannedString::new("howdy", proc_macro2::Span::call_site()) + ); + assert_ne!(lit, ident); + assert_eq!(lit.to_string(), "howdy"); + } + + #[test] + fn test_spanned_string_error() { + // Errors for the wrong kind of token match those for String, and a + // missing value is caught by deserialize_bytes. + for (tokens, expected) in [ + (quote! { s = 123 }, "expected a string, but found `123`"), + (quote! { s = [a] }, "expected a string, but found `[a]`"), + (quote! { s = a b }, "expected `,` or nothing, but found `b`"), + (quote! { s = }, "expected a value following `=`"), + ] { + #[derive(Deserialize)] + struct Test { + #[allow(dead_code)] + s: ParseWrapper, + } + match from_tokenstream::(&tokens) { + Err(err) => assert_eq!(err.to_string(), expected), + Ok(_) => panic!("unexpected success for `{tokens}`"), + } + } + } + // Make sure ParseWrapper is Hash #[test] fn test_parse_wrapper_hash() { diff --git a/testlib/src/lib.rs b/testlib/src/lib.rs index 970c01c..6c85604 100644 --- a/testlib/src/lib.rs +++ b/testlib/src/lib.rs @@ -9,6 +9,7 @@ use serde::Deserialize; use serde_tokenstream::from_tokenstream; use serde_tokenstream::from_tokenstream_spanned; use serde_tokenstream::ParseWrapper; +use serde_tokenstream::SpannedString; use syn::parse_macro_input; #[derive(Deserialize)] @@ -23,6 +24,15 @@ struct Annotation { tup: (u32, f32), bool_expr: Option>, painted: Option>, + /// A value that must be a valid identifier. + /// + /// Used to test `SpannedString` span attribution. + ident: Option>, + /// A compound `Parse` type containing `SpannedString` values. + /// + /// Used to test that `SpannedString` composes, and that errors land on the + /// correct value. + pair: Option>, } #[derive(Deserialize)] @@ -82,6 +92,21 @@ impl syn::parse::Parse for Painted { } } +#[allow(dead_code)] +struct KeyValue { + key: SpannedString, + value: SpannedString, +} + +impl syn::parse::Parse for KeyValue { + fn parse(input: syn::parse::ParseStream) -> syn::Result { + let key = input.parse()?; + let _: syn::Token![:] = input.parse()?; + let value = input.parse()?; + Ok(KeyValue { key, value }) + } +} + /// Used to test error attribution for `#[serde(flatten)]` fields, which serde /// deserializes from buffered entries after the map has been consumed. #[derive(Deserialize)] @@ -107,6 +132,58 @@ pub fn annotation( Ok(attrs) => { let item = proc_macro2::TokenStream::from(item); + let mut ident_use = None; + if let Some(ident) = &attrs.ident { + match ident.parse::() { + Ok(parsed) => { + // The parsed identifier should carry the span of the + // attribute value it came from. Emit a use of it so + // that rustc reports the (deliberately undefined) name + // at that span. Then the UI test can verify that span + // attribution is correct. + ident_use = Some(quote! { + const _: () = { + let _ = #parsed; + }; + }); + } + Err(err) => { + // The syn::Error disallowed_methods lint is meant to + // apply to the main codebase, not to this test library. + #[expect(clippy::disallowed_methods)] + let mut wrapped = syn::Error::new( + ident.span(), + format!( + "`{}` is not a valid identifier: {err}", + ident.value() + ), + ); + // Include the raw error so that its span, which comes + // from `parse`, is covered by the UI tests. + wrapped.combine(err); + return wrapped.to_compile_error().into(); + } + } + } + + if let Some(pair) = &attrs.pair { + if let Err(err) = pair.value.parse::() { + // The syn::Error disallowed_methods lint is meant to apply + // to the main codebase, not to this test library. + #[expect(clippy::disallowed_methods)] + let mut wrapped = syn::Error::new( + pair.value.span(), + format!( + "`{}` is not a valid identifier for key `{}`: {err}", + pair.value.value(), + pair.key.value() + ), + ); + wrapped.combine(err); + return wrapped.to_compile_error().into(); + } + } + let bool_assertion = attrs.bool_expr.map(|expr| { // Ensure that the bool_expr really is a boolean expression. let expr = expr.into_inner(); @@ -120,6 +197,8 @@ pub fn annotation( quote! { #bool_assertion + #ident_use + #item } .into() diff --git a/ui-tests/tests/ui/bad_spanned_string.rs b/ui-tests/tests/ui/bad_spanned_string.rs new file mode 100644 index 0000000..7e29844 --- /dev/null +++ b/ui-tests/tests/ui/bad_spanned_string.rs @@ -0,0 +1,17 @@ +// Copyright 2026 Oxide Computer Company + +// Ensure that an error about a SpannedString value is reported at the value +// rather than at the attribute as a whole. + +use testlib::annotation; + +#[annotation { + string = "test", + options = OptionA, + unit = (), + tup = (1, 2.0), + ident = "not an identifier", +}] +fn test() {} + +fn main() {} diff --git a/ui-tests/tests/ui/bad_spanned_string.stderr b/ui-tests/tests/ui/bad_spanned_string.stderr new file mode 100644 index 0000000..d6262e3 --- /dev/null +++ b/ui-tests/tests/ui/bad_spanned_string.stderr @@ -0,0 +1,11 @@ +error: `not an identifier` is not a valid identifier: unexpected token + --> tests/ui/bad_spanned_string.rs:13:13 + | +13 | ident = "not an identifier", + | ^^^^^^^^^^^^^^^^^^^ + +error: unexpected token + --> tests/ui/bad_spanned_string.rs:13:13 + | +13 | ident = "not an identifier", + | ^^^^^^^^^^^^^^^^^^^ diff --git a/ui-tests/tests/ui/bad_spanned_string_from_macro_rules.rs b/ui-tests/tests/ui/bad_spanned_string_from_macro_rules.rs new file mode 100644 index 0000000..1d85676 --- /dev/null +++ b/ui-tests/tests/ui/bad_spanned_string_from_macro_rules.rs @@ -0,0 +1,23 @@ +// Copyright 2026 Oxide Computer Company + +// A SpannedString provided a non-string substitution must report a diagnostic +// at the invocation site, not at the declaration site. + +use testlib::annotation; + +macro_rules! wrap { + ($s:expr) => { + #[annotation { + string = "test", + options = OptionA, + unit = (), + tup = (1, 2.0), + ident = $s, + }] + fn test() {} + }; +} + +wrap!(123); + +fn main() {} diff --git a/ui-tests/tests/ui/bad_spanned_string_from_macro_rules.stderr b/ui-tests/tests/ui/bad_spanned_string_from_macro_rules.stderr new file mode 100644 index 0000000..bf54e08 --- /dev/null +++ b/ui-tests/tests/ui/bad_spanned_string_from_macro_rules.stderr @@ -0,0 +1,5 @@ +error: expected a string, but found `123` + --> tests/ui/bad_spanned_string_from_macro_rules.rs:21:7 + | +21 | wrap!(123); + | ^^^ diff --git a/ui-tests/tests/ui/bad_spanned_string_ident.rs b/ui-tests/tests/ui/bad_spanned_string_ident.rs new file mode 100644 index 0000000..48f5e8b --- /dev/null +++ b/ui-tests/tests/ui/bad_spanned_string_ident.rs @@ -0,0 +1,17 @@ +// Copyright 2026 Oxide Computer Company + +// Ensure that a SpannedString value written as a bare identifier (here, a +// keyword) is reported at the identifier. + +use testlib::annotation; + +#[annotation { + string = "test", + options = OptionA, + unit = (), + tup = (1, 2.0), + ident = mod, +}] +fn test() {} + +fn main() {} diff --git a/ui-tests/tests/ui/bad_spanned_string_ident.stderr b/ui-tests/tests/ui/bad_spanned_string_ident.stderr new file mode 100644 index 0000000..ce06eff --- /dev/null +++ b/ui-tests/tests/ui/bad_spanned_string_ident.stderr @@ -0,0 +1,11 @@ +error: `mod` is not a valid identifier: expected identifier, found keyword `mod` + --> tests/ui/bad_spanned_string_ident.rs:13:13 + | +13 | ident = mod, + | ^^^ + +error: expected identifier, found keyword `mod` + --> tests/ui/bad_spanned_string_ident.rs:13:13 + | +13 | ident = mod, + | ^^^ diff --git a/ui-tests/tests/ui/bad_spanned_string_lex.rs b/ui-tests/tests/ui/bad_spanned_string_lex.rs new file mode 100644 index 0000000..3594903 --- /dev/null +++ b/ui-tests/tests/ui/bad_spanned_string_lex.rs @@ -0,0 +1,17 @@ +// Copyright 2026 Oxide Computer Company + +// Ensure that a SpannedString value that doesn't even lex as Rust tokens is +// reported at the value. + +use testlib::annotation; + +#[annotation { + string = "test", + options = OptionA, + unit = (), + tup = (1, 2.0), + ident = "\"", +}] +fn test() {} + +fn main() {} diff --git a/ui-tests/tests/ui/bad_spanned_string_lex.stderr b/ui-tests/tests/ui/bad_spanned_string_lex.stderr new file mode 100644 index 0000000..337f56c --- /dev/null +++ b/ui-tests/tests/ui/bad_spanned_string_lex.stderr @@ -0,0 +1,11 @@ +error: `"` is not a valid identifier: cannot parse string into token stream + --> tests/ui/bad_spanned_string_lex.rs:13:13 + | +13 | ident = "\"", + | ^^^^ + +error: cannot parse string into token stream + --> tests/ui/bad_spanned_string_lex.rs:13:13 + | +13 | ident = "\"", + | ^^^^ diff --git a/ui-tests/tests/ui/bad_spanned_string_pair.rs b/ui-tests/tests/ui/bad_spanned_string_pair.rs new file mode 100644 index 0000000..de2c8a9 --- /dev/null +++ b/ui-tests/tests/ui/bad_spanned_string_pair.rs @@ -0,0 +1,18 @@ +// Copyright 2026 Oxide Computer Company + +// Ensure that SpannedString composes inside a hand-written Parse type: the +// value is parsed in the middle of the token stream, and an error about it +// is reported at the value rather than at the separator or the attribute. + +use testlib::annotation; + +#[annotation { + string = "test", + options = OptionA, + unit = (), + tup = (1, 2.0), + pair = foo: "not an identifier", +}] +fn test() {} + +fn main() {} diff --git a/ui-tests/tests/ui/bad_spanned_string_pair.stderr b/ui-tests/tests/ui/bad_spanned_string_pair.stderr new file mode 100644 index 0000000..24ca317 --- /dev/null +++ b/ui-tests/tests/ui/bad_spanned_string_pair.stderr @@ -0,0 +1,11 @@ +error: `not an identifier` is not a valid identifier for key `foo`: unexpected token + --> tests/ui/bad_spanned_string_pair.rs:14:17 + | +14 | pair = foo: "not an identifier", + | ^^^^^^^^^^^^^^^^^^^ + +error: unexpected token + --> tests/ui/bad_spanned_string_pair.rs:14:17 + | +14 | pair = foo: "not an identifier", + | ^^^^^^^^^^^^^^^^^^^ diff --git a/ui-tests/tests/ui/bad_spanned_string_respan.rs b/ui-tests/tests/ui/bad_spanned_string_respan.rs new file mode 100644 index 0000000..07ef85f --- /dev/null +++ b/ui-tests/tests/ui/bad_spanned_string_respan.rs @@ -0,0 +1,18 @@ +// Copyright 2026 Oxide Computer Company + +// Ensure that a value parsed out of a SpannedString carries the span of the +// value: the macro emits a use of the parsed identifier, and rustc reports +// the unresolved name at the string literal in the attribute. + +use testlib::annotation; + +#[annotation { + string = "test", + options = OptionA, + unit = (), + tup = (1, 2.0), + ident = "undefined_thing", +}] +fn test() {} + +fn main() {} diff --git a/ui-tests/tests/ui/bad_spanned_string_respan.stderr b/ui-tests/tests/ui/bad_spanned_string_respan.stderr new file mode 100644 index 0000000..ed0e4fa --- /dev/null +++ b/ui-tests/tests/ui/bad_spanned_string_respan.stderr @@ -0,0 +1,5 @@ +error[E0425]: cannot find value `undefined_thing` in this scope + --> tests/ui/bad_spanned_string_respan.rs:14:13 + | +14 | ident = "undefined_thing", + | ^^^^^^^^^^^^^^^^^ not found in this scope diff --git a/ui-tests/tests/ui/bad_spanned_string_respan_from_macro_rules.rs b/ui-tests/tests/ui/bad_spanned_string_respan_from_macro_rules.rs new file mode 100644 index 0000000..8e7c5e5 --- /dev/null +++ b/ui-tests/tests/ui/bad_spanned_string_respan_from_macro_rules.rs @@ -0,0 +1,23 @@ +// Copyright 2026 Oxide Computer Company + +// Like bad_spanned_string_respan, with the value provided by a macro_rules +// substitution. + +use testlib::annotation; + +macro_rules! wrap { + ($s:expr) => { + #[annotation { + string = "test", + options = OptionA, + unit = (), + tup = (1, 2.0), + ident = $s, + }] + fn test() {} + }; +} + +wrap!("undefined_thing"); + +fn main() {} diff --git a/ui-tests/tests/ui/bad_spanned_string_respan_from_macro_rules.stderr b/ui-tests/tests/ui/bad_spanned_string_respan_from_macro_rules.stderr new file mode 100644 index 0000000..588bb44 --- /dev/null +++ b/ui-tests/tests/ui/bad_spanned_string_respan_from_macro_rules.stderr @@ -0,0 +1,5 @@ +error[E0425]: cannot find value `undefined_thing` in this scope + --> tests/ui/bad_spanned_string_respan_from_macro_rules.rs:21:7 + | +21 | wrap!("undefined_thing"); + | ^^^^^^^^^^^^^^^^^ not found in this scope