Skip to content

Commit c7b81d2

Browse files
committed
Always print all traits in error messages
- Instead, if the crate feature is not enabled, we say that in the error message instead. - Implement `Serialize` skeleton. - Hook up `serde` in the CI. - Handle `#[serde(...)]` attributes without `De/Serialize`. - Add tests for all these new errors. - Add test to check for `#[derive(De/Serialize)`]` support without `#[derive_where(De/Serialize)]`.
1 parent 6e9e0d9 commit c7b81d2

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

54 files changed

+543
-299
lines changed

.github/workflows/lint.yml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,16 @@ jobs:
2828
features: --features safe
2929
- channel: nightly
3030
features: --features nightly
31+
- channel: stable
32+
features: --features serde
3133
- channel: stable
3234
features: --features zeroize
3335
- channel: stable
3436
features: --features zeroize-on-drop
3537
- channel: nightly
3638
features: --features safe,nightly
39+
- channel: stable
40+
features: --features safe,serde
3741
- channel: stable
3842
features: --features safe,zeroize
3943
- channel: stable
@@ -42,6 +46,12 @@ jobs:
4246
features: --features nightly,zeroize
4347
- channel: nightly
4448
features: --features nightly,zeroize-on-drop
49+
- channel: nightly
50+
features: --features nightly,serde
51+
- channel: stable
52+
features: --features serde,zeroize
53+
- channel: stable
54+
features: --features serde,zeroize-on-drop
4555
- channel: nightly
4656
features: --all-features
4757

.github/workflows/test.yml

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,23 @@ jobs:
2828
features:
2929
- ""
3030
- --features safe
31+
- --features serde
3132
- --features zeroize
3233
- --features zeroize-on-drop
34+
- --features safe,serde
3335
- --features safe,zeroize
3436
- --features safe,zeroize-on-drop
37+
- --features serde,zeroize
38+
- --features serde,zeroize-on-drop
3539
include:
3640
- rust: 1.57.0
3741
msrv: true
3842
- rust: nightly
3943
features: --features nightly
4044
- rust: nightly
4145
features: --features safe,nightly
46+
- rust: nightly
47+
features: --features nightly,serde
4248
- rust: nightly
4349
features: --features nightly,zeroize
4450
- rust: nightly
@@ -83,17 +89,23 @@ jobs:
8389
features:
8490
- ""
8591
- --features safe
92+
- --features serde
8693
- --features zeroize
8794
- --features zeroize-on-drop
95+
- --features safe,serde
8896
- --features safe,zeroize
8997
- --features safe,zeroize-on-drop
98+
- --features serde,zeroize
99+
- --features serde,zeroize-on-drop
90100
include:
91101
- rust: 1.57.0
92102
msrv: true
93103
- rust: nightly
94104
features: --features nightly
95105
- rust: nightly
96106
features: --features safe,nightly
107+
- rust: nightly
108+
features: --features nightly,serde
97109
- rust: nightly
98110
features: --features nightly,zeroize
99111
- rust: nightly
@@ -127,9 +139,9 @@ jobs:
127139
matrix:
128140
rust:
129141
- version: 1.57.0
130-
features: safe,zeroize-on-drop
142+
features: safe,serde,zeroize-on-drop
131143
- version: stable
132-
features: safe,zeroize-on-drop
144+
features: safe,serde,zeroize-on-drop
133145

134146
steps:
135147
- name: Checkout

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1111
- `no_drop` item-level option to `ZeroizeOnDrop` which does not implement
1212
`Drop` but instead only asserts that every field implements `ZeroizeOnDrop`.
1313

14+
### Changed
15+
- Error messages now point to crate features instead of reporting traits as
16+
unsupported.
17+
1418
### Fixed
1519
- Stop depending on unstable APIs for `Eq` for `ZeroizeOnDrop`.
1620

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ syn = { version = "2", default-features = false, features = [
4343
pretty_assertions = "1"
4444
rustversion = "1"
4545
serde_ = { package = "serde", version = "1", default-features = false, features = ["derive"] }
46+
serde_test = "1"
4647
trybuild = { version = "1.0.18", default-features = false }
4748
zeroize_ = { package = "zeroize", version = "1.5", default-features = false }
4849

src/attr/item.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,18 @@ impl ItemAttr {
9696
return Err(Error::none(span));
9797
}
9898

99+
// Check for `#[serde(...)]` attributes without `De/Serialize`.
100+
#[cfg(feature = "serde")]
101+
if !self_.derive_wheres.iter().any(|derive_where| {
102+
derive_where.contains(Trait::Deserialize) | derive_where.contains(Trait::Serialize)
103+
}) {
104+
for attr in attrs {
105+
if attr.path().is_ident("serde") {
106+
return Err(Error::serde_without_serde(attr.span()));
107+
}
108+
}
109+
}
110+
99111
// Merge `DeriveWhere`s with the same bounds.
100112
self_
101113
.derive_wheres

src/attr/skip.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ impl SkipGroup {
271271
| Trait::PartialEq
272272
| Trait::PartialOrd => true,
273273
#[cfg(feature = "serde")]
274-
Trait::Deserialize => false,
274+
Trait::Deserialize | Trait::Serialize => false,
275275
#[cfg(feature = "zeroize")]
276276
Trait::Zeroize | Trait::ZeroizeOnDrop => true,
277277
}

src/error.rs

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ impl Error {
7474
}
7575

7676
/// Unsupported option in attribute.
77-
#[cfg(feature = "zeroize")]
77+
#[cfg(any(feature = "serde", feature = "zeroize"))]
7878
pub fn option_trait(span: Span, attribute: &str) -> syn::Error {
7979
syn::Error::new(span, format!("`{}` doesn't support this option", attribute))
8080
}
@@ -191,6 +191,24 @@ impl Error {
191191
)
192192
}
193193

194+
/// Requires crate feature `serde`.
195+
#[cfg(not(feature = "serde"))]
196+
pub fn serde_feature(span: Span) -> syn::Error {
197+
syn::Error::new(span, "requires crate feature `serde`")
198+
}
199+
200+
/// Requires crate feature `zeroize`.
201+
#[cfg(not(feature = "zeroize"))]
202+
pub fn zeroize_feature(span: Span) -> syn::Error {
203+
syn::Error::new(span, "requires crate feature `zeroize`")
204+
}
205+
206+
/// Requires crate feature `zeroize-on-drop`.
207+
#[cfg(all(feature = "zeroize", not(feature = "zeroize-on-drop")))]
208+
pub fn zeroize_on_drop_feature(span: Span) -> syn::Error {
209+
syn::Error::new(span, "requires crate feature `zeroize-on-drop`")
210+
}
211+
194212
/// Invalid delimiter in `derive_where` attribute for
195213
/// [`Trait`](crate::Trait)s.
196214
pub fn derive_where_delimiter(span: Span) -> syn::Error {
@@ -282,37 +300,35 @@ impl Error {
282300
syn::Error::new(skip_clone, "Cannot skip `Clone` while deriving `Copy`")
283301
}
284302

303+
/// Unsupported `serde(...)` without deriving `De/Serialize`.
304+
#[cfg(feature = "serde")]
305+
pub fn serde_without_serde(serde: Span) -> syn::Error {
306+
syn::Error::new(serde, "Found unused `#[serde(...)]`")
307+
}
308+
285309
/// List of available [`Trait`](crate::Trait)s.
286310
fn trait_list() -> String {
287311
[
288312
"Clone",
289313
"Copy",
290314
"Debug",
291315
"Default",
316+
"Deserialize",
292317
"Eq",
293318
"Hash",
294319
"Ord",
295320
"PartialEq",
296321
"PartialOrd",
297-
#[cfg(feature = "zeroize")]
322+
"Serialize",
298323
"Zeroize",
299-
#[cfg(feature = "zeroize")]
300324
"ZeroizeOnDrop",
301325
]
302326
.join(", ")
303327
}
304328

305329
/// List of available [`SkipGroup`](crate::SkipGroup)s.
306330
fn skip_group_list() -> String {
307-
[
308-
"Clone",
309-
"Debug",
310-
"EqHashOrd",
311-
"Hash",
312-
#[cfg(feature = "zeroize")]
313-
"Zeroize",
314-
]
315-
.join(", ")
331+
["Clone", "Debug", "EqHashOrd", "Hash", "Zeroize"].join(", ")
316332
}
317333

318334
/// Unsupported `Zeroize` option if [`Zeroize`](https://docs.rs/zeroize/latest/zeroize/trait.Zeroize.html) isn't implemented.

src/trait_.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ pub mod hash;
1212
pub mod ord;
1313
pub mod partial_eq;
1414
pub mod partial_ord;
15+
#[cfg(feature = "serde")]
16+
mod serde;
17+
#[cfg(feature = "serde")]
18+
pub mod serialize;
1519
#[cfg(feature = "zeroize")]
1620
pub mod zeroize;
1721
#[cfg(feature = "zeroize")]
@@ -56,6 +60,9 @@ pub enum Trait {
5660
PartialEq,
5761
/// [`PartialOrd`].
5862
PartialOrd,
63+
/// [`Serialize`](https://docs.rs/serde/latest/serde/derive.Serialize.html).
64+
#[cfg(feature = "serde")]
65+
Serialize,
5966
/// [`Zeroize`](https://docs.rs/zeroize/latest/zeroize/trait.Zeroize.html).
6067
#[cfg(feature = "zeroize")]
6168
Zeroize,
@@ -78,6 +85,8 @@ macro_rules! trait_dispatch {
7885
Trait::Ord => ord::Ord::$method($($par),*),
7986
Trait::PartialEq => partial_eq::PartialEq::$method($($par),*),
8087
Trait::PartialOrd => partial_ord::PartialOrd::$method($($par),*),
88+
#[cfg(feature = "serde")]
89+
Trait::Serialize => serialize::Serialize::$method($($par),*),
8190
#[cfg(feature = "zeroize")]
8291
Trait::Zeroize => zeroize::Zeroize::$method($($par),*),
8392
#[cfg(feature = "zeroize")]
@@ -99,15 +108,25 @@ impl Trait {
99108
"Default" => Ok(Default),
100109
#[cfg(feature = "serde")]
101110
"Deserialize" => Ok(Deserialize),
111+
#[cfg(not(feature = "serde"))]
112+
"Deserialize" => Err(Error::serde_feature(path.span())),
102113
"Eq" => Ok(Eq),
103114
"Hash" => Ok(Hash),
104115
"Ord" => Ok(Ord),
105116
"PartialEq" => Ok(PartialEq),
106117
"PartialOrd" => Ok(PartialOrd),
118+
#[cfg(feature = "serde")]
119+
"Serialize" => Ok(Serialize),
120+
#[cfg(not(feature = "serde"))]
121+
"Serialize" => Err(Error::serde_feature(path.span())),
107122
#[cfg(feature = "zeroize")]
108123
"Zeroize" => Ok(Zeroize),
124+
#[cfg(not(feature = "zeroize"))]
125+
"Zeroize" => Err(Error::zeroize_feature(path.span())),
109126
#[cfg(feature = "zeroize")]
110127
"ZeroizeOnDrop" => Ok(ZeroizeOnDrop),
128+
#[cfg(not(feature = "zeroize"))]
129+
"ZeroizeOnDrop" => Err(Error::zeroize_feature(path.span())),
111130
"crate" => Err(Error::crate_(path.span())),
112131
_ => Err(Error::trait_(path.span())),
113132
}
@@ -170,6 +189,9 @@ pub enum DeriveTrait {
170189
PartialEq,
171190
/// [`PartialOrd`].
172191
PartialOrd,
192+
/// [`Serialize`](https://docs.rs/serde/latest/serde/derive.Serialize.html).
193+
#[cfg(feature = "serde")]
194+
Serialize(serialize::Serialize),
173195
/// [`Zeroize`](https://docs.rs/zeroize/latest/zeroize/trait.Zeroize.html).
174196
#[cfg(feature = "zeroize")]
175197
Zeroize(zeroize::Zeroize),
@@ -196,6 +218,8 @@ impl Deref for DeriveTrait {
196218
Ord => &ord::Ord,
197219
PartialEq => &partial_eq::PartialEq,
198220
PartialOrd => &partial_ord::PartialOrd,
221+
#[cfg(feature = "serde")]
222+
Serialize(trait_) => trait_,
199223
#[cfg(feature = "zeroize")]
200224
Zeroize(trait_) => trait_,
201225
#[cfg(feature = "zeroize")]

src/trait_/deserialize.rs

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,14 @@
22
33
use std::{borrow::Cow, ops::Deref};
44

5-
use proc_macro2::TokenStream;
5+
use proc_macro2::{Span, TokenStream};
66
use quote::quote;
7-
use syn::{DeriveInput, Ident, ImplGenerics, Path, TypeGenerics, WhereClause};
7+
use syn::{
8+
punctuated::Punctuated, DeriveInput, Ident, ImplGenerics, Meta, Path, Result, TypeGenerics,
9+
WhereClause,
10+
};
811

12+
use super::serde;
913
use crate::{util, DeriveTrait, Trait, TraitImpl, DERIVE_WHERE};
1014

1115
/// [`TraitImpl`] for [`Deserialize`](https://docs.rs/serde/latest/serde/derive.Deserialize.html).
@@ -30,6 +34,15 @@ impl TraitImpl for Deserialize {
3034
DeriveTrait::Deserialize(Self { crate_: None })
3135
}
3236

37+
fn parse_derive_trait(span: Span, list: Punctuated<Meta, syn::Token![,]>) -> Result<DeriveTrait>
38+
where
39+
Self: Sized,
40+
{
41+
Ok(DeriveTrait::Deserialize(Self {
42+
crate_: serde::parse_derive_trait(Trait::Deserialize, span, list)?,
43+
}))
44+
}
45+
3346
fn path(&self) -> Path {
3447
util::path_from_root_and_strs(self.crate_(), &["Deserialize"])
3548
}

src/trait_/serde.rs

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
//! Common functionality between
2+
//! [`Deserialize`](https://docs.rs/serde/latest/serde/derive.Deserialize.html) and
3+
//! [`Serialize`](https://docs.rs/serde/latest/serde/derive.Serialize.html).
4+
5+
use proc_macro2::Span;
6+
use syn::{
7+
punctuated::Punctuated, spanned::Spanned, Expr, ExprLit, ExprPath, Lit, Meta, Path, Token,
8+
};
9+
10+
use crate::{util, Error, Result, Trait};
11+
12+
/// Parses
13+
pub fn parse_derive_trait(
14+
trait_: Trait,
15+
_span: Span,
16+
list: Punctuated<Meta, Token![,]>,
17+
) -> Result<Option<Path>> {
18+
// This is already checked in `DeriveTrait::from_stream`.
19+
debug_assert!(!list.is_empty());
20+
21+
let mut crate_ = None;
22+
23+
for meta in list {
24+
match &meta {
25+
Meta::Path(path) => {
26+
return Err(Error::option_trait(path.span(), trait_.as_str()));
27+
}
28+
Meta::NameValue(name_value) => {
29+
if name_value.path.is_ident("crate") {
30+
// Check for duplicate `crate` option.
31+
if crate_.is_none() {
32+
let path = match &name_value.value {
33+
Expr::Lit(ExprLit {
34+
lit: Lit::Str(lit_str),
35+
..
36+
}) => match lit_str.parse::<Path>() {
37+
Ok(path) => path,
38+
Err(error) => return Err(Error::path(lit_str.span(), error)),
39+
},
40+
Expr::Path(ExprPath { path, .. }) => path.clone(),
41+
_ => return Err(Error::option_syntax(name_value.value.span())),
42+
};
43+
44+
if path == util::path_from_strs(&["serde"]) {
45+
return Err(Error::path_unnecessary(path.span(), "::serde"));
46+
}
47+
48+
crate_ = Some(path);
49+
} else {
50+
return Err(Error::option_duplicate(name_value.span(), "crate"));
51+
}
52+
} else {
53+
return Err(Error::option_trait(name_value.path.span(), trait_.as_str()));
54+
}
55+
}
56+
_ => {
57+
return Err(Error::option_syntax(meta.span()));
58+
}
59+
}
60+
}
61+
62+
Ok(crate_)
63+
}

0 commit comments

Comments
 (0)