Skip to content

Commit 3b23fc8

Browse files
committed
Overhaul use-case checking
1 parent 7880bb8 commit 3b23fc8

File tree

8 files changed

+83
-138
lines changed

8 files changed

+83
-138
lines changed

src/attr/item.rs

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ pub struct DeriveWhere {
104104
/// Save [`Span`] for error messages.
105105
pub span: Span,
106106
/// [traits](DeriveTrait) to implement.
107-
pub traits: Vec<DeriveTrait>,
107+
pub traits: Vec<DeriveTraitWrapper>,
108108
/// [generics](Generic) for where clause.
109109
pub generics: Vec<Generic>,
110110
}
@@ -125,7 +125,7 @@ impl DeriveWhere {
125125
// Start with parsing a trait.
126126
// Not checking for duplicates here, because these should produce a Rust error
127127
// anyway.
128-
traits.push(DeriveTrait::from_stream(span, data, input)?);
128+
traits.push(DeriveTraitWrapper::from_stream(span, data, input)?);
129129

130130
if !input.is_empty() {
131131
let mut fork = input.fork();
@@ -178,7 +178,8 @@ impl DeriveWhere {
178178
pub fn trait_(&self, trait_: &Trait) -> Option<&DeriveTrait> {
179179
self.traits
180180
.iter()
181-
.find(|derive_trait| &***derive_trait == trait_)
181+
.map(|wrapper| &wrapper.trait_)
182+
.find(|derive_trait| derive_trait == trait_)
182183
}
183184

184185
/// Returns `true` if any [`CustomBound`](Generic::CustomBound) is present.
@@ -275,6 +276,14 @@ impl Parse for Generic {
275276
}
276277
}
277278

279+
/// Wrapper around [`DeriveTrait`] to add [`Span`].
280+
pub struct DeriveTraitWrapper {
281+
/// [`Span`] for error messages.
282+
pub span: Span,
283+
/// Trait in this wrapper.
284+
trait_: DeriveTrait,
285+
}
286+
278287
/// Trait to implement.
279288
pub enum DeriveTrait {
280289
/// [`Clone`].
@@ -305,6 +314,14 @@ pub enum DeriveTrait {
305314
},
306315
}
307316

317+
impl Deref for DeriveTraitWrapper {
318+
type Target = DeriveTrait;
319+
320+
fn deref(&self) -> &Self::Target {
321+
&self.trait_
322+
}
323+
}
324+
308325
impl Deref for DeriveTrait {
309326
type Target = Trait;
310327

@@ -327,7 +344,21 @@ impl Deref for DeriveTrait {
327344
}
328345
}
329346

330-
impl DeriveTrait {
347+
impl PartialEq<Trait> for &DeriveTraitWrapper {
348+
fn eq(&self, other: &Trait) -> bool {
349+
let trait_: &DeriveTrait = self;
350+
trait_ == *other
351+
}
352+
}
353+
354+
impl PartialEq<Trait> for &DeriveTrait {
355+
fn eq(&self, other: &Trait) -> bool {
356+
let trait_: &Trait = self;
357+
trait_ == other
358+
}
359+
}
360+
361+
impl DeriveTraitWrapper {
331362
/// Create [`DeriveTrait`] from [`ParseStream`].
332363
fn from_stream(span: Span, data: &Data, input: ParseStream) -> Result<Self> {
333364
match Meta::parse(input) {
@@ -342,23 +373,31 @@ impl DeriveTrait {
342373
}
343374

344375
match meta {
345-
Meta::Path(_) => Ok(trait_.default_derive_trait()),
376+
Meta::Path(path) => Ok(Self {
377+
span: path.span(),
378+
trait_: trait_.default_derive_trait(),
379+
}),
346380
Meta::List(list) => {
347381
if list.nested.is_empty() {
348382
return Err(Error::option_empty(list.span()));
349383
}
350384

351-
// This will return an error if no options are supported.
352-
trait_.parse_derive_trait(list)
385+
Ok(Self {
386+
span: list.span(),
387+
// This will return an error if no options are supported.
388+
trait_: trait_.parse_derive_trait(list)?,
389+
})
353390
}
354391
Meta::NameValue(name_value) => Err(Error::option_syntax(name_value.span())),
355392
}
356393
}
357394
Err(error) => Err(Error::trait_syntax(error.span())),
358395
}
359396
}
397+
}
360398

361-
/// Returns fully qualified path for this trait.
399+
impl DeriveTrait {
400+
/// Returns fully qualified [`Path`] for this trait.
362401
pub fn path(&self) -> Path {
363402
use DeriveTrait::*;
364403

src/attr/skip.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -167,12 +167,4 @@ impl Skip {
167167
}
168168
}
169169
}
170-
171-
/// Returns `true` if any field is skipped.
172-
pub fn any_skip(&self) -> bool {
173-
match self {
174-
Skip::None => false,
175-
Skip::All | Skip::Traits(_) => true,
176-
}
177-
}
178170
}

src/data.rs

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -261,15 +261,6 @@ impl<'a> Data<'a> {
261261
self.iter_fields(trait_).count() == 0
262262
}
263263

264-
/// Returns `true` if any field is skipped.
265-
pub fn any_skip(&self) -> bool {
266-
self.skip_inner.any_skip()
267-
|| match self.fields() {
268-
Either::Left(fields) => fields.any_skip(),
269-
Either::Right(_) => false,
270-
}
271-
}
272-
273264
/// Returns `true` if a field is skipped with that [`Trait`].
274265
pub fn any_skip_trait(&self, trait_: &Trait) -> bool {
275266
self.skip_inner.skip(trait_)

src/data/field.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -124,11 +124,6 @@ impl<'a> Field<'a> {
124124
}
125125
}
126126

127-
/// Returns `true` if this field is skipped with any [`Trait`].
128-
pub fn any_skip(&self) -> bool {
129-
self.attr.skip.any_skip()
130-
}
131-
132127
/// Returns `true` if this field is skipped with the given [`Trait`].
133128
pub fn skip(&self, trait_: &Trait) -> bool {
134129
self.attr.skip.skip(trait_)

src/data/fields.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -146,11 +146,6 @@ impl<'a> Fields<'a> {
146146
pattern
147147
}
148148

149-
/// Returns `true` if any field is skipped.
150-
pub fn any_skip(&self) -> bool {
151-
self.fields.iter().any(|field| field.any_skip())
152-
}
153-
154149
/// Returns `true` if any field is skipped with that [`Trait`].
155150
pub fn any_skip_trait(&self, trait_: &Trait) -> bool {
156151
self.fields.iter().any(|field| field.skip(trait_))

src/error.rs

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,21 +20,11 @@ impl Error {
2020
}
2121

2222
/// Item has no use-case because it's covered by standard `#[derive(..)]`.
23-
pub fn item(span: Span) -> syn::Error {
23+
pub fn use_case(span: Span) -> syn::Error {
2424
syn::Error::new(
2525
span,
26-
"derive-where doesn't support items without generics, `skip` attributes or `enum`s \
27-
implementing `Default`, as this can already be handled by standard `#[derive(..)]`",
28-
)
29-
}
30-
31-
/// Using the same generic type parameters as the item is unsupported,
32-
/// because it's covered by standard `#[derive(..)]`.
33-
pub fn generics(span: Span) -> syn::Error {
34-
syn::Error::new(
35-
span,
36-
"derive-where doesn't support items with the same generic type parameters as the \
37-
item, as this can already be handled by standard `#[derive(..)]`",
26+
"this can be handled by standard `#[derive(..)]`, use a `skip` attribute, implement \
27+
`Default` on an enum, of different generic type parameters",
3828
)
3929
}
4030

src/input.rs

Lines changed: 23 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -89,43 +89,6 @@ impl<'a> Input<'a> {
8989
}
9090
};
9191

92-
// Don't allow no use-case compared to std `derive`.
93-
let mut found_use_case = false;
94-
// Any generics used.
95-
found_use_case |= generics.params.iter().any(|generic_param|
96-
// MSRV: `matches!` was added in 1.42.0.
97-
match generic_param {
98-
GenericParam::Type(_) => true,
99-
GenericParam::Lifetime(_) | GenericParam::Const(_) => false,
100-
});
101-
// Any field is skipped.
102-
found_use_case |= item.any_skip();
103-
// `Default` is used on an enum.
104-
found_use_case |= item.any_default(&derive_wheres);
105-
#[cfg(feature = "zeroize")]
106-
{
107-
// `Zeroize(crate = "..")` is used.
108-
found_use_case |= derive_wheres.iter().any(|derive_where| {
109-
#[allow(clippy::match_like_matches_macro)]
110-
{
111-
if let Some(DeriveTrait::Zeroize {
112-
crate_: Some(_), ..
113-
}) = derive_where.trait_(&Trait::Zeroize)
114-
{
115-
true
116-
} else {
117-
false
118-
}
119-
}
120-
});
121-
// `Zeroize(fqs)` is used on any field.
122-
found_use_case |= item.any_fqs();
123-
}
124-
125-
if !found_use_case {
126-
return Err(Error::item(span));
127-
}
128-
12992
// Don't allow generic constraints be the same as generics on item unless there
13093
// is a use-case for it.
13194
// Count number of generic type parameters.
@@ -168,40 +131,32 @@ impl<'a> Input<'a> {
168131
// The `for` loop should short-circuit to the `'outer` loop if not all generic
169132
// type parameters were found.
170133

171-
// Make sure we aren't using any other features.
172-
let mut found_use_case = false;
173-
// `Default` is used on an enum.
174-
found_use_case |= match item {
175-
Item::Enum { .. } => derive_where.trait_(&Trait::Default).is_some(),
176-
Item::Item(_) => false,
177-
};
178-
// Any field is skipped with a corresponding `Trait`.
179-
found_use_case |= derive_where
180-
.traits
181-
.iter()
182-
.any(|trait_| item.any_skip_trait(trait_));
183-
#[cfg(feature = "zeroize")]
184-
{
185-
// `Zeroize(crate = "..")` is used.
186-
found_use_case |= {
187-
#[allow(clippy::match_like_matches_macro)]
134+
// Don't allow no use-case compared to std `derive`.
135+
for trait_ in &derive_where.traits {
136+
// `Default` is used on an enum.
137+
if trait_ == Trait::Default && item.is_enum() {
138+
continue;
139+
}
140+
// Any field is skipped with a corresponding `Trait`.
141+
if item.any_skip_trait(trait_) {
142+
continue;
143+
}
144+
#[cfg(feature = "zeroize")]
145+
{
146+
// `Zeroize(crate = "..")` is used.
147+
if let DeriveTrait::Zeroize {
148+
crate_: Some(_), ..
149+
} = **trait_
188150
{
189-
if let Some(DeriveTrait::Zeroize {
190-
crate_: Some(_), ..
191-
}) = derive_where.trait_(&Trait::Zeroize)
192-
{
193-
true
194-
} else {
195-
false
196-
}
151+
continue;
197152
}
198-
};
199-
// `Zeroize(fqs)` is used on any field.
200-
found_use_case |= derive_where.trait_(&Trait::Zeroize).is_some() && item.any_fqs();
201-
}
153+
// `Zeroize(fqs)` is used on any field.
154+
if trait_ == Trait::Zeroize && item.any_fqs() {
155+
continue;
156+
}
157+
}
202158

203-
if !found_use_case {
204-
return Err(Error::generics(derive_where.span));
159+
return Err(Error::use_case(trait_.span));
205160
}
206161
}
207162

src/item.rs

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
33
use syn::Ident;
44

5-
use crate::{Data, DeriveWhere, Trait};
5+
use crate::{Data, Trait};
66

77
/// Fields or variants of an item.
88
#[allow(clippy::large_enum_variant)]
@@ -19,39 +19,27 @@ pub enum Item<'a> {
1919
}
2020

2121
impl Item<'_> {
22-
/// Return [`struct@Ident`] of this [`Item`].
22+
/// Returns [`struct@Ident`] of this [`Item`].
2323
pub fn ident(&self) -> &Ident {
2424
match self {
2525
Item::Item(data) => data.ident,
2626
Item::Enum { ident, .. } => ident,
2727
}
2828
}
2929

30-
/// Returns `true` if any field is skipped with that [`Trait`].
31-
pub fn any_skip_trait(&self, trait_: &Trait) -> bool {
32-
match self {
33-
Item::Item(data) => data.any_skip_trait(trait_),
34-
Item::Enum { variants, .. } => variants.iter().any(|data| data.any_skip_trait(trait_)),
35-
}
36-
}
37-
38-
/// Returns `true` if any field is skipped.
39-
pub fn any_skip(&self) -> bool {
30+
/// Returns `true` if this [`Item`] if an enum.
31+
pub fn is_enum(&self) -> bool {
4032
match self {
41-
Item::Item(data) => data.any_skip(),
42-
Item::Enum { variants, .. } => variants.iter().any(|data| data.any_skip()),
33+
Item::Enum { .. } => true,
34+
Item::Item(_) => false,
4335
}
4436
}
4537

46-
/// Returns `true` if any field uses `default`.
47-
// MSRV: `matches!` was added in 1.42.0.
48-
#[allow(clippy::match_like_matches_macro)]
49-
pub fn any_default(&self, derive_wheres: &[DeriveWhere]) -> bool {
38+
/// Returns `true` if any field is skipped with that [`Trait`].
39+
pub fn any_skip_trait(&self, trait_: &Trait) -> bool {
5040
match self {
51-
Item::Enum { .. } => derive_wheres
52-
.iter()
53-
.any(|derive_where| derive_where.trait_(&Trait::Default).is_some()),
54-
_ => false,
41+
Item::Item(data) => data.any_skip_trait(trait_),
42+
Item::Enum { variants, .. } => variants.iter().any(|data| data.any_skip_trait(trait_)),
5543
}
5644
}
5745

0 commit comments

Comments
 (0)