Skip to content

Rework FromStr derive #468

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
May 8, 2025
Merged

Rework FromStr derive #468

merged 14 commits into from
May 8, 2025

Conversation

tyranron
Copy link
Collaborator

@tyranron tyranron commented May 7, 2025

Required for #467, #469

This PR fully reworks FromStr derive implementation to avoid using old utils machinery.

Also, adds compile_fail tests for FromStr.

Checklist

  • Documentation is updated (not required)
  • Tests are added/updated
  • CHANGELOG entry is added (not required)

@tyranron tyranron added this to the 2.1.0 milestone May 7, 2025
@tyranron tyranron self-assigned this May 7, 2025
@tyranron tyranron enabled auto-merge (squash) May 7, 2025 14:36
@tyranron tyranron requested a review from JelteF May 7, 2025 14:36
@tyranron tyranron mentioned this pull request May 7, 2025
3 tasks
@tyranron tyranron disabled auto-merge May 7, 2025 17:31
@tyranron tyranron enabled auto-merge (squash) May 7, 2025 17:31
@tyranron
Copy link
Collaborator Author

tyranron commented May 8, 2025

ping @JelteF

@tyranron tyranron merged commit 1c2fe2a into master May 8, 2025
17 checks passed
@tyranron tyranron deleted the rework-fromstr branch May 8, 2025 21:22
tyranron added a commit that referenced this pull request May 13, 2025
Required for #467  
Requires #468

## Synopsis

We support flat structs for `Display`:
```rust
#[derive(Display)]
struct Foo;
```
But `FromStr` doesn't support it.

## Solution

Allow using `#[derive(FromStr)]` on structs with no fields.

Similarly to enums it will use fuzzy matching. Exact matching would be
possible to enable in #469 by using `#[from_str(rename_all = "...")]`
attribute.
tyranron added a commit that referenced this pull request May 24, 2025
Resolves #216   
Requires #468, #469

## Synopsis

We have `#[display(rename_all = "...")]` attribute for flat enums. The
counterpart for `FromStr` derive is missing.

## Solution

This PR adds support `#[from_str(rename_all = "...")]` attribute on
enums. Once this attribute is specified, the matching is always exact
(for now, by default, matching could be fuzzy if there is no similarly
named variants).
@tyranron tyranron mentioned this pull request May 28, 2025
3 tasks
tyranron added a commit that referenced this pull request Jun 2, 2025
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants