Skip to content

Commit 92da5d8

Browse files
authored
Fix FromStr generics check (#476)
Follows #468 ## Synopsis In #468, the `FromStr` derive was reworked. However, the check for inferring trait bounds for the inner type was simplified to `!generics.params.is_empty()`, which is not a very correct assumption, because applies the redundant bounds for cases like these: ```rust #[derive(FromStr)] struct Int<const N: usize>(i32); ``` ```rust impl<const N: usize> derive_more::core::str::FromStr for Int<N> where i32: derive_more::core::str::FromStr { /* ... */} ``` ## Solution Use `utils::GenericSearch` machinery as a common mechanism for this purpose. ## Additionally Refactored `utils::GenericSearch` to use `Vec` instead `HashSet`, because in the majority of cases there won't be any duplicates in generic parameters and their number won't be big enough for a `HashSet` to make a positive performance benefit. At the same moment, `HashSet` requires `Hash` implementation of `syn::Ident` which is gated by the `syn/extra-traits` feature, and thus we can lift it requirement for `utils::GenericSearch` usage.
1 parent b982f90 commit 92da5d8

File tree

4 files changed

+135
-16
lines changed

4 files changed

+135
-16
lines changed

impl/Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,10 @@ debug = ["syn/extra-traits", "dep:unicode-xid"]
5757
deref = []
5858
deref_mut = []
5959
display = ["syn/extra-traits", "dep:unicode-xid", "dep:convert_case"]
60-
eq = ["syn/extra-traits", "syn/visit"]
60+
eq = ["syn/visit"]
6161
error = ["syn/extra-traits"]
6262
from = ["syn/extra-traits"]
63-
from_str = ["dep:convert_case"]
63+
from_str = ["syn/visit", "dep:convert_case"]
6464
index = []
6565
index_mut = []
6666
into = ["syn/extra-traits"]

impl/src/from_str.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use syn::{parse_quote, spanned::Spanned as _};
1010

1111
use crate::utils::{
1212
attr::{self, ParseMultiple as _},
13-
Either, Spanning,
13+
Either, GenericsSearch, Spanning,
1414
};
1515

1616
/// Expands a [`FromStr`] derive macro.
@@ -88,8 +88,9 @@ impl ToTokens for ForwardExpansion<'_> {
8888
let inner_ty = &self.inner.ty;
8989
let ty = self.self_ty.0;
9090

91+
let generics_search = GenericsSearch::from(self.self_ty.1);
9192
let mut generics = self.self_ty.1.clone();
92-
if !generics.params.is_empty() {
93+
if generics_search.any_in(inner_ty) {
9394
generics.make_where_clause().predicates.push(parse_quote! {
9495
#inner_ty: derive_more::core::str::FromStr
9596
});

impl/src/utils.rs

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use syn::{
2424
pub(crate) use self::either::Either;
2525
#[cfg(any(feature = "from", feature = "into"))]
2626
pub(crate) use self::fields_ext::FieldsExt;
27-
#[cfg(any(feature = "as_ref", feature = "eq"))]
27+
#[cfg(any(feature = "as_ref", feature = "eq", feature = "from_str"))]
2828
pub(crate) use self::generics_search::GenericsSearch;
2929
#[cfg(any(
3030
feature = "as_ref",
@@ -2381,23 +2381,21 @@ mod fields_ext {
23812381
impl<T: Len + ?Sized> FieldsExt for T {}
23822382
}
23832383

2384-
#[cfg(any(feature = "as_ref", feature = "eq"))]
2384+
#[cfg(any(feature = "as_ref", feature = "eq", feature = "from_str"))]
23852385
mod generics_search {
23862386
use syn::visit::Visit;
23872387

2388-
use super::HashSet;
2389-
23902388
/// Search of whether some generics (type parameters, lifetime parameters or const parameters)
23912389
/// are present in some [`syn::Type`].
23922390
pub(crate) struct GenericsSearch<'s> {
23932391
/// Type parameters to look for.
2394-
pub(crate) types: HashSet<&'s syn::Ident>,
2392+
pub(crate) types: Vec<&'s syn::Ident>,
23952393

23962394
/// Lifetime parameters to look for.
2397-
pub(crate) lifetimes: HashSet<&'s syn::Ident>,
2395+
pub(crate) lifetimes: Vec<&'s syn::Ident>,
23982396

23992397
/// Const parameters to look for.
2400-
pub(crate) consts: HashSet<&'s syn::Ident>,
2398+
pub(crate) consts: Vec<&'s syn::Ident>,
24012399
}
24022400

24032401
impl<'s> From<&'s syn::Generics> for GenericsSearch<'s> {
@@ -2436,15 +2434,15 @@ mod generics_search {
24362434
self.found |= ep
24372435
.path
24382436
.get_ident()
2439-
.is_some_and(|ident| self.search.consts.contains(ident));
2437+
.is_some_and(|ident| self.search.consts.contains(&ident));
24402438

24412439
if !self.found {
24422440
syn::visit::visit_expr_path(self, ep);
24432441
}
24442442
}
24452443

24462444
fn visit_lifetime(&mut self, lf: &'ast syn::Lifetime) {
2447-
self.found |= self.search.lifetimes.contains(&lf.ident);
2445+
self.found |= self.search.lifetimes.contains(&&lf.ident);
24482446

24492447
if !self.found {
24502448
syn::visit::visit_lifetime(self, lf);
@@ -2453,14 +2451,15 @@ mod generics_search {
24532451

24542452
fn visit_type_path(&mut self, tp: &'ast syn::TypePath) {
24552453
self.found |= tp.path.get_ident().is_some_and(|ident| {
2456-
self.search.types.contains(ident) || self.search.consts.contains(ident)
2454+
self.search.types.contains(&ident)
2455+
|| self.search.consts.contains(&ident)
24572456
});
24582457

24592458
if !self.found {
24602459
// `TypeParam::AssocType` case.
24612460
self.found |= tp.path.segments.first().is_some_and(|segment| {
24622461
matches!(segment.arguments, syn::PathArguments::None)
2463-
&& self.search.types.contains(&segment.ident)
2462+
&& self.search.types.contains(&&segment.ident)
24642463
});
24652464
}
24662465

tests/from_str.rs

Lines changed: 120 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,17 @@ extern crate alloc;
66

77
#[cfg(not(feature = "std"))]
88
use alloc::string::ToString;
9+
use core::{convert::Infallible, marker::PhantomData};
910

10-
use derive_more::FromStr;
11+
use derive_more::with_trait::FromStr;
12+
13+
trait Some {
14+
type Assoc;
15+
}
16+
17+
impl<T> Some for T {
18+
type Assoc = i32;
19+
}
1120

1221
mod structs {
1322
use super::*;
@@ -135,6 +144,116 @@ mod structs {
135144
"invalid digit found in string",
136145
);
137146
}
147+
148+
#[test]
149+
fn assoc() {
150+
#[derive(Debug, Eq, FromStr, PartialEq)]
151+
struct Point1D<I: Some> {
152+
x: I::Assoc,
153+
}
154+
155+
assert_eq!("3".parse::<Point1D<()>>().unwrap(), Point1D { x: 3 });
156+
assert_eq!("0".parse::<Point1D<()>>().unwrap(), Point1D { x: 0 });
157+
assert_eq!(
158+
"2147483647".parse::<Point1D<()>>().unwrap(),
159+
Point1D { x: i32::MAX },
160+
);
161+
assert_eq!(
162+
"-2147483648".parse::<Point1D<()>>().unwrap(),
163+
Point1D { x: i32::MIN },
164+
);
165+
166+
assert_eq!(
167+
"2147483648".parse::<Point1D<()>>().unwrap_err().to_string(),
168+
"number too large to fit in target type",
169+
);
170+
assert_eq!(
171+
"-2147483649"
172+
.parse::<Point1D<()>>()
173+
.unwrap_err()
174+
.to_string(),
175+
"number too small to fit in target type",
176+
);
177+
assert_eq!(
178+
"wow".parse::<Point1D<()>>().unwrap_err().to_string(),
179+
"invalid digit found in string",
180+
);
181+
}
182+
183+
#[test]
184+
fn lifetime() {
185+
#[derive(Debug, Eq, PartialEq)]
186+
struct Empty<'a>(PhantomData<&'a i32>);
187+
188+
impl FromStr for Empty<'_> {
189+
type Err = Infallible;
190+
191+
fn from_str(_: &str) -> Result<Self, Self::Err> {
192+
Ok(Self(PhantomData))
193+
}
194+
}
195+
196+
#[derive(Debug, Eq, FromStr, PartialEq)]
197+
struct Wrapper<'a>(Empty<'a>);
198+
199+
assert_eq!(
200+
"3".parse::<Wrapper>().unwrap(),
201+
Wrapper(Empty(PhantomData)),
202+
);
203+
}
204+
205+
#[test]
206+
fn const_param() {
207+
#[derive(Debug, Eq, FromStr, PartialEq)]
208+
struct Int<const N: usize>(i32);
209+
210+
#[derive(Debug, Eq, FromStr, PartialEq)]
211+
struct Point1D<const N: usize> {
212+
x: Int<N>,
213+
}
214+
215+
assert_eq!("3".parse::<Int<1>>().unwrap(), Int(3));
216+
assert_eq!("0".parse::<Int<1>>().unwrap(), Int(0));
217+
assert_eq!("2147483647".parse::<Int<1>>().unwrap(), Int(i32::MAX));
218+
assert_eq!("-2147483648".parse::<Int<1>>().unwrap(), Int(i32::MIN));
219+
220+
assert_eq!(
221+
"2147483648".parse::<Int<1>>().unwrap_err().to_string(),
222+
"number too large to fit in target type",
223+
);
224+
assert_eq!(
225+
"-2147483649".parse::<Int<1>>().unwrap_err().to_string(),
226+
"number too small to fit in target type",
227+
);
228+
assert_eq!(
229+
"wow".parse::<Int<1>>().unwrap_err().to_string(),
230+
"invalid digit found in string",
231+
);
232+
233+
assert_eq!("3".parse::<Point1D<3>>().unwrap(), Point1D { x: Int(3) });
234+
assert_eq!("0".parse::<Point1D<3>>().unwrap(), Point1D { x: Int(0) });
235+
assert_eq!(
236+
"2147483647".parse::<Point1D<3>>().unwrap(),
237+
Point1D { x: Int(i32::MAX) },
238+
);
239+
assert_eq!(
240+
"-2147483648".parse::<Point1D<3>>().unwrap(),
241+
Point1D { x: Int(i32::MIN) },
242+
);
243+
244+
assert_eq!(
245+
"2147483648".parse::<Point1D<3>>().unwrap_err().to_string(),
246+
"number too large to fit in target type",
247+
);
248+
assert_eq!(
249+
"-2147483649".parse::<Point1D<3>>().unwrap_err().to_string(),
250+
"number too small to fit in target type",
251+
);
252+
assert_eq!(
253+
"wow".parse::<Point1D<3>>().unwrap_err().to_string(),
254+
"invalid digit found in string",
255+
);
256+
}
138257
}
139258
}
140259

0 commit comments

Comments
 (0)