Address review

Author: daxpedda

src/attr/field.rs

@@ -1,8 +1,8 @@
 //! Attribute parsing for fields.
 
-use syn::{punctuated::Punctuated, spanned::Spanned, Attribute, Meta, Result, Token};
+use syn::{spanned::Spanned, Attribute, Meta, Result};
 
-use crate::{DeriveWhere, Error, Skip, DERIVE_WHERE};
+use crate::{util::MetaListExt, DeriveWhere, Error, Skip, DERIVE_WHERE};
 #[cfg(feature = "zeroize")]
 use crate::{Trait, TraitImpl, ZeroizeFqs};
 
@@ -45,11 +45,7 @@ impl FieldAttr {
 		debug_assert!(meta.path().is_ident(DERIVE_WHERE));
 
 		if let Meta::List(list) = meta {
-			let nested = list.parse_args_with(Punctuated::<Meta, Token![,]>::parse_terminated)?;
-
-			if nested.is_empty() {
-				return Err(Error::empty(list.span()));
-			}
+			let nested = list.parse_non_empty_nested_metas()?;
 
 			for meta in &nested {
 				if meta.path().is_ident(Skip::SKIP) {

src/attr/item.rs

@@ -11,7 +11,10 @@ use syn::{
 	TraitBoundModifier, Type, TypeParamBound, TypePath, WhereClause, WherePredicate,
 };
 
-use crate::{util, Error, Incomparable, Item, Skip, SkipGroup, Trait, TraitImpl, DERIVE_WHERE};
+use crate::{
+	util::{self, MetaListExt},
+	Error, Incomparable, Item, Skip, SkipGroup, Trait, TraitImpl, DERIVE_WHERE,
+};
 
 /// Attributes on item.
 #[derive(Default)]
@@ -163,6 +166,8 @@ impl DeriveWhere {
 			let mut generics = Vec::new();
 
 			// Check for an empty list is already done in `ItemAttr::from_attrs`.
+			assert!(!input.is_empty());
+
 			while !input.is_empty() {
 				// Start with parsing a trait.
 				// Not checking for duplicates here, we do that after merging `derive_where`s
@@ -478,12 +483,7 @@ impl DeriveTrait {
 				match &meta {
 					Meta::Path(path) => Ok((path.span(), trait_.default_derive_trait())),
 					Meta::List(list) => {
-						let nested =
-							list.parse_args_with(Punctuated::<Meta, Token![,]>::parse_terminated)?;
-
-						if nested.is_empty() {
-							return Err(Error::option_empty(list.span()));
-						}
+						let nested = list.parse_non_empty_nested_metas()?;
 
 						// This will return an error if no options are supported.
 						Ok((list.span(), trait_.parse_derive_trait(meta.span(), nested)?))

src/attr/skip.rs

@@ -2,9 +2,9 @@
 
 use std::default::Default;
 
-use syn::{punctuated::Punctuated, spanned::Spanned, Meta, Path, Result, Token};
+use syn::{spanned::Spanned, Meta, Path, Result};
 
-use crate::{DeriveWhere, Error, Trait};
+use crate::{util::MetaListExt, DeriveWhere, Error, Trait};
 
 /// Stores what [`Trait`]s to skip this field or variant for.
 #[cfg_attr(test, derive(Debug))]
@@ -78,13 +78,7 @@ impl Skip {
 				}
 			}
 			Meta::List(list) => {
-				let nested =
-					list.parse_args_with(Punctuated::<Meta, Token![,]>::parse_terminated)?;
-
-				// Don't allow an empty list.
-				if nested.is_empty() {
-					return Err(Error::option_empty(list.span()));
-				}
+				let nested = list.parse_non_empty_nested_metas()?;
 
 				// Get traits already set to be skipped.
 				let traits = match self {

src/attr/variant.rs

@@ -1,10 +1,8 @@
 //! Attribute parsing for variants.
 
-use syn::{
-	punctuated::Punctuated, spanned::Spanned, Attribute, Fields, Meta, Result, Token, Variant,
-};
+use syn::{spanned::Spanned, Attribute, Fields, Meta, Result, Variant};
 
-use crate::{Default, DeriveWhere, Error, Incomparable, Skip, DERIVE_WHERE};
+use crate::{util::MetaListExt, Default, DeriveWhere, Error, Incomparable, Skip, DERIVE_WHERE};
 
 /// Attributes on variant.
 #[derive(Default)]
@@ -46,7 +44,7 @@ impl VariantAttr {
 		debug_assert!(meta.path().is_ident(DERIVE_WHERE));
 
 		if let Meta::List(list) = meta {
-			let nested = list.parse_args_with(Punctuated::<Meta, Token![,]>::parse_terminated)?;
+			let nested = list.parse_non_empty_nested_metas()?;
 
 			if nested.is_empty() {
 				return Err(Error::empty(list.span()));

src/attr/zeroize_fqs.rs

@@ -1,8 +1,8 @@
 //! Attribute parsing for the `Zeroize(fqs)` option.
 
-use syn::{punctuated::Punctuated, spanned::Spanned, Meta, Result, Token};
+use syn::{spanned::Spanned, Meta, Result};
 
-use crate::{DeriveWhere, Error, Trait, TraitImpl};
+use crate::{util::MetaListExt, DeriveWhere, Error, Trait, TraitImpl};
 
 /// Stores if this field should use FQS to call [`Zeroize::zeroize`](https://docs.rs/zeroize/latest/zeroize/trait.Zeroize.html#tymethod.zeroize).
 #[derive(Default)]
@@ -26,12 +26,7 @@ impl ZeroizeFqs {
 
 		match meta {
 			Meta::List(list) => {
-				let nested =
-					list.parse_args_with(Punctuated::<Meta, Token![,]>::parse_terminated)?;
-
-				if nested.is_empty() {
-					return Err(Error::option_empty(list.span()));
-				}
+				let nested = list.parse_non_empty_nested_metas()?;
 
 				for meta in &nested {
 					if let Meta::Path(path) = meta {

src/lib.rs

@@ -393,10 +393,10 @@ use std::{borrow::Cow, iter};
 use proc_macro2::TokenStream;
 use quote::{quote, quote_spanned, ToTokens};
 use syn::{
-	punctuated::Punctuated, spanned::Spanned, Attribute, DataEnum, DataStruct, DataUnion,
-	DeriveInput, Expr, ExprLit, ExprPath, Fields, FieldsNamed, FieldsUnnamed, Generics, Lit, Meta,
-	Path, Token, Variant,
+	spanned::Spanned, Attribute, DataEnum, DataStruct, DataUnion, DeriveInput, Expr, ExprLit,
+	ExprPath, Fields, FieldsNamed, FieldsUnnamed, Generics, Lit, Meta, Path, Result, Variant,
 };
+use util::MetaListExt;
 
 #[cfg(feature = "zeroize")]
 use self::attr::ZeroizeFqs;
@@ -476,16 +476,14 @@ pub fn derive_where(
 }
 
 /// Convenient way to deal with [`Result`] for [`derive_where()`].
-fn derive_where_internal(mut item: DeriveInput) -> Result<TokenStream, syn::Error> {
+fn derive_where_internal(mut item: DeriveInput) -> Result<TokenStream> {
 	let mut crate_ = None;
 
 	// Search for `crate` option.
 	for attr in &item.attrs {
 		if attr.path().is_ident(DERIVE_WHERE) {
 			if let Meta::List(list) = &attr.meta {
-				if let Ok(nested) =
-					list.parse_args_with(Punctuated::<Meta, Token![,]>::parse_terminated)
-				{
+				if let Ok(nested) = list.parse_non_empty_nested_metas() {
 					if nested.len() == 1 {
 						let meta = nested.into_iter().next().expect("unexpected empty list");
 

src/util.rs

@@ -1,7 +1,12 @@
 //! Utility functions.
 
 use proc_macro2::Span;
-use syn::{punctuated::Punctuated, Ident, Path, PathArguments, PathSegment, Token};
+use syn::{
+	punctuated::Punctuated, spanned::Spanned, Ident, Meta, MetaList, Path, PathArguments,
+	PathSegment, Result, Token,
+};
+
+use crate::error::Error;
 
 /// Convenience type to return two possible values.
 pub enum Either<L, R> {
@@ -49,3 +54,21 @@ pub fn path_from_root_and_strs(root: Path, segments: &[&str]) -> Path {
 			.collect(),
 	}
 }
+
+/// Extension for [`MetaList`].
+pub trait MetaListExt {
+	/// Shorthand for parsing a [`MetaList`] into a list of [`Meta`]s.
+	fn parse_non_empty_nested_metas(&self) -> Result<Punctuated<Meta, Token![,]>>;
+}
+
+impl MetaListExt for MetaList {
+	fn parse_non_empty_nested_metas(&self) -> Result<Punctuated<Meta, Token![,]>> {
+		let list = self.parse_args_with(Punctuated::<Meta, Token![,]>::parse_terminated)?;
+
+		if list.is_empty() {
+			return Err(Error::option_empty(self.span()));
+		}
+
+		Ok(list)
+	}
+}

tests/ui/not-zeroize/item_option_syntax.rs

@@ -4,3 +4,5 @@ use derive_where::derive_where;
 
 #[derive_where = "invalid"]
 struct WrongAttributeSyntax<T>(PhantomData<T>);
+
+fn main() {}

tests/ui/not-zeroize/item_option_syntax.stderr

@@ -9,9 +9,3 @@ error: unsupported trait syntax, expected one of Clone, Copy, Debug, Default, Eq
   |
 5 | #[derive_where = "invalid"]
   |                  ^^^^^^^^^
-
-error[E0601]: `main` function not found in crate `$CRATE`
- --> tests/ui/not-zeroize/item_option_syntax.rs:6:48
-  |
-6 | struct WrongAttributeSyntax<T>(PhantomData<T>);
-  |                                                ^ consider adding a `main` function to `$DIR/tests/ui/not-zeroize/item_option_syntax.rs`

tests/ui/zeroize/item_option_syntax.rs

@@ -4,3 +4,5 @@ use derive_where::derive_where;
 
 #[derive_where = "invalid"]
 struct WrongAttributeSyntax<T>(PhantomData<T>);
+
+fn main() {}