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.

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

Improve unnecessary_map_or lint with negated expressions #13995

Open
chrisduerr opened this issue Jan 13, 2025 · 1 comment · May be fixed by #13998
Open

Improve unnecessary_map_or lint with negated expressions #13995

chrisduerr opened this issue Jan 13, 2025 · 1 comment · May be fixed by #13998
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages C-question Category: Questions

Comments

@chrisduerr
Copy link
Contributor

Description

The unnecessary_map_or lint just (almost) caused a bug in one of my projects, due to the way its suggestions work:

alacritty/crossfont#72 (comment)

If you take a close look at the suggestion, it's clear that the leading ! is not part of the suggestion:

Image

However my argument is that this mistake is far too easy to make, effectively making this suggestion have an unreasonably high chance to break something. It's possible this might apply to other clippy lints too, I'm not aware of all of them.

Humans are pretty bad at this whole reading thing, so my suggestion would be to explicitly include leading ! into the expression if possible. I think this would reduce the chance of bugs like this happening. Replacing an entire line is always going to be easier for people than replacing just a part of it.

I'm curious if this is something you'd be interested in. This would obviously only work in a best-effort heuristic manner, but I do think it could actually help people and prevent bugs. I have no doubt that sooner or later this would cause bugs otherwise (and maybe it already has).

Version


Additional Labels

@rustbot label +C-enhancement
@rustbot label +C-question

@rustbot rustbot added C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages C-question Category: Questions labels Jan 13, 2025
@y21
Copy link
Member

y21 commented Jan 13, 2025

Another option could be to emit a verbose multipart suggestion that only changes the map_or to is_some_and and removes the false, argument rather than printing the whole expression to begin with. IMO that also helps readability a fair amount in highlighting exactly what needs to change:

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages C-question Category: Questions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants