Update to syn version 2

As there were quite substantial changes from syn version 1 to 2 some
minor changes to the codebase are necessary:

The `Attribute` type is more accurately typed now, that means instead
of having simply a `Path` entity and some tokens after it, there is now
a differentation between attributes with a `=`, in parentheses or
without any other tokens but the path. This makes parsing slightly
easier for us. Some test fixes were also required as these rely on the
old tokens we got from an attribute.

The `Spanned` trait was sealed in syn v2. We implemented it for some
of the internal meta types. As we still have the span information
readily available and the trait is otherwise required nowhere, simply
remove the impl.

Some syntax enums were marked as `non-exhaustive`. Handle these
additional options in a sensible way where possible and throw compile
errors in all other places.

Fixes jam1garner#285. Closes jam1garner#299.

Signed-off-by: Colin Snover <[email protected]>

Author: Urist-McGit

binrw_derive/Cargo.toml

@@ -19,10 +19,10 @@ either = "1.0.0"
 owo-colors = { version = "3.0.0", optional = true }
 proc-macro2 = { version = "1.0.53", features = ["span-locations"] }
 quote = "1.0.0"
-syn = { version = "1.0.66", features = ["extra-traits", "fold", "full", "visit"] }
+syn = { version = "2", features = ["extra-traits", "fold", "full", "visit"] }
 
 [dev-dependencies]
-runtime-macros-derive = "0.4.0"
+runtime-macros = "1.1.0"
 
 [features]
 default = []

binrw_derive/src/binrw/backtrace/syntax_highlighting.rs

@@ -7,7 +7,7 @@ use core::{
     ops::Range,
 };
 use owo_colors::{styles::BoldDisplay, XtermColors};
-use proc_macro2::{Span, TokenTree};
+use proc_macro2::Span;
 use quote::ToTokens;
 use std::collections::HashMap;
 use syn::{
@@ -135,13 +135,13 @@ fn highlight_attributes(attrs: &[syn::Attribute], visit: &mut Visitor) {
         // |____|______ path and pound_token
         //
         syntax_info.highlight_color(attr.pound_token.span(), Color::Keyword);
-        syntax_info.highlight_color(attr.path.span(), Color::Keyword);
+        syntax_info.highlight_color(attr.path().span(), Color::Keyword);
 
         // #[...]
         //  ^   ^
         //  |___|___ brackets
         //
-        let span = attr.bracket_token.span;
+        let span = attr.bracket_token.span.join();
         let start = start(span);
         let end = end(span);
 
@@ -158,9 +158,9 @@ fn highlight_attributes(attrs: &[syn::Attribute], visit: &mut Visitor) {
         //       ^   ^
         //       |___|___ parens
         //
-        if let Some(TokenTree::Group(group)) = attr.tokens.clone().into_iter().next() {
-            syntax_info.highlight_color(group.span_open(), Color::Keyword);
-            syntax_info.highlight_color(group.span_close(), Color::Keyword);
+        if let syn::Meta::List(l) = &attr.meta {
+            syntax_info.highlight_color(l.delimiter.span().open(), Color::Keyword);
+            syntax_info.highlight_color(l.delimiter.span().close(), Color::Keyword);
         }
     }
 }
@@ -291,7 +291,7 @@ impl<'ast> Visit<'ast> for Visitor {
                     Lit::Str(_) | Lit::ByteStr(_) => Color::String,
                     Lit::Byte(_) | Lit::Char(_) => Color::Char,
                     Lit::Int(_) | Lit::Float(_) | Lit::Bool(_) => Color::Number,
-                    Lit::Verbatim(_) => return,
+                    _ => return,
                 },
             ));
         }

binrw_derive/src/binrw/mod.rs

@@ -87,14 +87,19 @@ pub(super) fn derive_from_attribute(
     let mut mixed_rw = false;
     let opposite_attr = if write { "binread" } else { "binwrite" };
     for attr in &mut derive_input.attrs {
-        if let Some(seg) = attr.path.segments.last() {
+        if let Some(seg) = attr.path().segments.last() {
             let ident = &seg.ident;
             if ident == "binrw" || ident == "binread" || ident == "binwrite" {
-                attr.tokens = quote! { (ignore) };
-
                 if ident == "binrw" || ident == opposite_attr {
                     mixed_rw = true;
                 }
+
+                attr.meta = syn::MetaList {
+                    path: attr.path().clone(),
+                    delimiter: syn::MacroDelimiter::Paren(syn::token::Paren::default()),
+                    tokens: quote! { ignore },
+                }
+                .into();
             }
         }
     }
@@ -151,11 +156,11 @@ pub(super) fn derive_from_input(
 }
 
 fn is_binread_attr(attr: &syn::Attribute) -> bool {
-    attr.path.is_ident("br") || attr.path.is_ident("brw")
+    attr.path().is_ident("br") || attr.path().is_ident("brw")
 }
 
 fn is_binwrite_attr(attr: &syn::Attribute) -> bool {
-    attr.path.is_ident("bw") || attr.path.is_ident("brw")
+    attr.path().is_ident("bw") || attr.path().is_ident("brw")
 }
 
 fn parse(
@@ -175,7 +180,7 @@ fn parse(
 #[cfg_attr(coverage_nightly, coverage(off))]
 #[test]
 fn derive_binread_code_coverage_for_tool() {
-    use runtime_macros_derive::emulate_derive_expansion_fallible;
+    use runtime_macros::emulate_derive_macro_expansion;
     use std::{env, fs};
 
     let derive_tests_folder = env::current_dir()
@@ -190,16 +195,19 @@ fn derive_binread_code_coverage_for_tool() {
         let entry = entry.unwrap();
         if entry.file_type().unwrap().is_file() {
             let file = fs::File::open(entry.path()).unwrap();
-            if emulate_derive_expansion_fallible(file, "BinRead", |input| {
-                parse(
-                    &input,
-                    Options {
-                        derive: true,
-                        write: false,
-                    },
-                )
-                .1
-            })
+            if emulate_derive_macro_expansion(
+                file,
+                &[("BinRead", |input| {
+                    parse(
+                        &syn::parse2::<syn::DeriveInput>(input).unwrap(),
+                        Options {
+                            derive: true,
+                            write: false,
+                        },
+                    )
+                    .1
+                })],
+            )
             .is_err()
             {
                 run_success = false;
@@ -214,7 +222,7 @@ fn derive_binread_code_coverage_for_tool() {
 #[cfg_attr(coverage_nightly, coverage(off))]
 #[test]
 fn derive_binwrite_code_coverage_for_tool() {
-    use runtime_macros_derive::emulate_derive_expansion_fallible;
+    use runtime_macros::emulate_derive_macro_expansion;
     use std::{env, fs};
 
     let derive_tests_folder = env::current_dir()
@@ -230,16 +238,19 @@ fn derive_binwrite_code_coverage_for_tool() {
         let entry = entry.unwrap();
         if entry.file_type().unwrap().is_file() {
             let file = fs::File::open(entry.path()).unwrap();
-            if emulate_derive_expansion_fallible(file, "BinWrite", |input| {
-                parse(
-                    &input,
-                    Options {
-                        derive: true,
-                        write: true,
-                    },
-                )
-                .1
-            })
+            if emulate_derive_macro_expansion(
+                file,
+                &[("BinWrite", |input| {
+                    parse(
+                        &syn::parse2::<syn::DeriveInput>(input).unwrap(),
+                        Options {
+                            derive: true,
+                            write: true,
+                        },
+                    )
+                    .1
+                })],
+            )
             .is_err()
             {
                 run_success = false;

binrw_derive/src/binrw/parser/mod.rs

@@ -40,12 +40,16 @@ trait FromAttrs<Attr: syn::parse::Parse> {
                     is_binread_attr(attr)
                 }
             })
-            .flat_map(
-                |attr| match syn::parse2::<MetaAttrList<Attr>>(attr.tokens.clone()) {
+            .flat_map(|attr| {
+                let tokens = match attr.meta.require_list() {
+                    Ok(m) => m.tokens.clone(),
+                    Err(err) => return either::Right(core::iter::once(Err(err))),
+                };
+                match syn::parse2::<MetaAttrList<Attr>>(tokens) {
                     Ok(list) => either::Left(list.into_iter().map(Ok)),
                     Err(err) => either::Right(core::iter::once(Err(err))),
-                },
-            );
+                }
+            });
 
         let mut all_errors = None::<syn::Error>;
         for attr in attrs {

binrw_derive/src/binrw/parser/types/magic.rs

@@ -93,6 +93,7 @@ impl TryFrom<attrs::Magic> for SpannedValue<Inner> {
                     "expected byte string, byte, float, or int",
                 ))
             }
+            _ => return Err(syn::Error::new(value.span(), "unexpected literal")),
         };
 
         Ok(Self::new(

binrw_derive/src/binrw/parser/types/spanned_value.rs

@@ -16,6 +16,10 @@ impl<T> SpannedValue<T> {
     pub(crate) fn into_value(self) -> T {
         self.value
     }
+
+    pub(crate) fn span(&self) -> Span {
+        self.span
+    }
 }
 
 impl<T> AsRef<T> for SpannedValue<T> {
@@ -32,14 +36,6 @@ impl<T> core::ops::Deref for SpannedValue<T> {
     }
 }
 
-// It is not possible to implement this *and* ToTokens because syn has a generic
-// implementation of Spanned for all ToTokens
-impl<T> syn::spanned::Spanned for SpannedValue<T> {
-    fn span(&self) -> Span {
-        self.span
-    }
-}
-
 impl<T: Into<To> + KeywordToken, To> From<T> for SpannedValue<To> {
     fn from(value: T) -> Self {
         let span = value.keyword_span();

binrw_derive/src/meta_types.rs

@@ -143,7 +143,7 @@ impl<Keyword: Parse, ItemType: Parse> Parse for MetaList<Keyword, ItemType> {
         parenthesized!(content in input);
         Ok(MetaList {
             ident,
-            fields: content.parse_terminated::<_, Token![,]>(ItemType::parse)?,
+            fields: content.parse_terminated(ItemType::parse, Token![,])?,
         })
     }
 }
@@ -184,15 +184,15 @@ where
             Ok(Self {
                 ident,
                 list: Enclosure::Paren {
-                    fields: content.parse_terminated::<_, Token![,]>(ParenItemType::parse)?,
+                    fields: content.parse_terminated(ParenItemType::parse, Token![,])?,
                 },
             })
         } else if lookahead.peek(token::Brace) {
             braced!(content in input);
             Ok(Self {
                 ident,
                 list: Enclosure::Brace {
-                    fields: content.parse_terminated::<_, Token![,]>(BraceItemType::parse)?,
+                    fields: content.parse_terminated(BraceItemType::parse, Token![,])?,
                 },
             })
         } else {
@@ -268,9 +268,7 @@ impl<P> MetaAttrList<P> {
 
 impl<P: Parse> Parse for MetaAttrList<P> {
     fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
-        let content;
-        parenthesized!(content in input);
-        Ok(MetaAttrList(Fields::parse_terminated(&content)?))
+        Ok(MetaAttrList(Fields::parse_terminated(input)?))
     }
 }
 
@@ -458,19 +456,13 @@ mod tests {
     try_parse_fail!(ident_type_missing_colon, "expected `:`", IdentTypeMaybeDefault, { foo u8 });
     try_parse_fail!(ident_type_missing_ident, "expected identifier", IdentTypeMaybeDefault, { :u8 });
 
-    try_parse!(meta_attr_list, MetaAttrListTest, { (1u8, 2u8, 3u8) });
-    try_parse!(meta_attr_list_empty, MetaAttrListTest, { () });
+    try_parse!(meta_attr_list, MetaAttrListTest, { 1u8, 2u8, 3u8 });
+    try_parse!(meta_attr_list_empty, MetaAttrListTest, {});
     try_parse_fail!(
         meta_attr_list_wrong_type,
         "expected literal",
         MetaAttrListTest,
-        { (i32) }
-    );
-    try_parse_fail!(
-        meta_attr_list_confused_as_list,
-        "expected parentheses",
-        MetaAttrListTest,
-        { wrong(i32) }
+        { i32 }
     );
 
     #[test]
@@ -482,7 +474,7 @@ mod tests {
             Lit::new(proc_macro2::Literal::u8_suffixed(3)),
         ];
 
-        let value = syn::parse2::<MetaAttrListTest>(quote::quote! { (1u8, 2u8, 3u8) }).unwrap();
+        let value = syn::parse2::<MetaAttrListTest>(quote::quote! { 1u8, 2u8, 3u8 }).unwrap();
         assert_eq!(expected, value.into_iter().collect::<Vec<_>>()[..]);
     }
 }

binrw_derive/src/named_args/mod.rs

@@ -39,7 +39,11 @@ pub(crate) fn derive_from_imports(
         result_name,
         fields: &args.map(Into::into).collect::<Vec<_>>(),
         generics: lifetime
-            .map(|lifetime| [syn::GenericParam::Lifetime(syn::LifetimeDef::new(lifetime))])
+            .map(|lifetime| {
+                [syn::GenericParam::Lifetime(syn::LifetimeParam::new(
+                    lifetime,
+                ))]
+            })
             .as_ref()
             .map_or(&[], |generics| generics.as_slice()),
         vis,
@@ -60,7 +64,7 @@ fn from_input(input: DeriveInput) -> syn::Result<TokenStream> {
             .into_iter()
             .map(|field| {
                 let attrs = field.attrs.iter().filter_map(|attr| {
-                    attr.path
+                    attr.path()
                         .get_ident()
                         .filter(|ident| *ident == "named_args")
                         .map(|_| attr.parse_args::<NamedArgAttr>())
@@ -149,9 +153,15 @@ mod kw {
 #[cfg_attr(coverage_nightly, coverage(off))]
 #[test]
 fn derive_named_args_code_coverage_for_tool() {
-    use runtime_macros_derive::emulate_derive_expansion_fallible;
+    use runtime_macros::emulate_derive_macro_expansion;
     let file = std::fs::File::open("../binrw/tests/named_args.rs").unwrap();
-    emulate_derive_expansion_fallible(file, "NamedArgs", |input| derive_from_input(input)).unwrap();
+    emulate_derive_macro_expansion(
+        file,
+        &[("NamedArgs", |input| {
+            derive_from_input(syn::parse2::<syn::DeriveInput>(input).unwrap())
+        })],
+    )
+    .unwrap();
 }
 
 #[cfg(test)]
@@ -207,7 +217,7 @@ mod tests {
         }
     });
 
-    try_error!(missing_default_value: "expected expression" {
+    try_error!(missing_default_value: "unexpected end of input, expected an expression" {
         struct Foo<A> {
             #[named_args(default = )]
             a: A,