Skip to content
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.

Sign up for GitHub

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

Jump to bottom

IsVariant should annotate the generated methods with #[must_use]? #349

Closed
TheLostLambda opened this issue Apr 18, 2024 · 1 comment · Fixed by #350
Closed

IsVariant should annotate the generated methods with #[must_use]? #349

TheLostLambda opened this issue Apr 18, 2024 · 1 comment · Fixed by #350
Assignees
@TheLostLambda
Labels
enhancement
Milestone
1.0.0

Comments

@TheLostLambda
Copy link
Contributor

I'm thinking about using IsVariant to replace a set of manual methods I'd previously implemented:
image

I was happy to see that the methods were already const, but they could still be made #[must_use]! The reason it's in my code is because I got a pedantic clippy lint about it, but since bools don't implement Drop and exist only to be eventually used (these static methods have no "side effects"), I think it's safe and reasonable to add this annotation! As I understand, this is what the standard library does for Result.

I think this would, technically, be a breaking change, so it might be good to get into 1.0, but I'm happy to quickly whip up a PR if you think this is a worthwhile addition!

Just let me know and I can get to work!
@JelteF
Copy link
Owner

JelteF commented Apr 18, 2024

Yeah, sounds great. A PR is definitely appreciated for this.

@JelteF JelteF added this to the 1.0.0 milestone Apr 18, 2024
@TheLostLambda TheLostLambda mentioned this issue Apr 18, 2024
Merged
3 tasks
tyranron pushed a commit that referenced this issue Apr 19, 2024
@TheLostLambda
Add #[must_use] annotation to derive(IsVariant) (#350, #349)
e0d1698 
## Synopsis & Solution

Adds a `#[must_use]` annotation to the static methods generated by
`IsVariant`. I also refactored the derive to use `matches!(...)` instead
of a manual `match` block with bools, but that's just a style tweak.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@TheLostLambda TheLostLambda

Labels
enhancement
Projects
None yet
Milestone
1.0.0
Development

Successfully merging a pull request may close this issue.

feat(is_variant): add #[must_use] annotation TheLostLambda/derive_more
3 participants
@JelteF @TheLostLambda @tyranron