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

Lint if lets that could be better worded as equality #1716

Closed
clarfonthey opened this issue May 2, 2017 · 0 comments
Closed

Lint if lets that could be better worded as equality #1716

clarfonthey opened this issue May 2, 2017 · 0 comments
Labels
A-lint Area: New lints

Comments

@clarfonthey
Copy link
Contributor

This would also affect the single_match lint. Essentially, whenever someone writes a pattern like this:

if let Ordering::Greater = a.cmp(b) { ... }

It would be a bit clearer to write something like this:

if a.cmp(b) == Ordering::Greater { ... }

Specifically, if the left hand side of the if let contains a value that can be constructed directly, and the destructured value implements PartialEq, then is makes a lot more sense to refactor the if let into a direct comparison.

This would also affect the single_match and single_match_else lints to give proper suggestions, i.e. changing:

match a.cmp(b) {
    Ordering::Greater => { ... },
    _ => {},
}

to:

if a.cmp(b) == Ordering::Greater { ... }
@phansch phansch added the A-lint Area: New lints label Dec 11, 2018
bors added a commit that referenced this issue Oct 4, 2021
Add lint `equatable_if_let`

This is my attempt for #1716. There is a major false positive, which is people may implement `PartialEq` in a different way. It is unactionable at the moment so I put it into `nursery`.

There is a trait `StructuralPartialEq` for solving this problem which is promising but it has several problems currently:
* Integers and tuples doesn't implement it.
* Some types wrongly implement it, like `Option<T>` when `T` doesn't implement it.

I consider them bugs and against the propose of `StructuralPartialEq`. When they become fixed, this lint can become a useful lint with a single line change.

changelog: New lint: [`equatable_if_let`]
bors added a commit that referenced this issue Oct 4, 2021
Add lint `equatable_if_let`

This is my attempt for #1716. There is a major false positive, which is people may implement `PartialEq` in a different way. It is unactionable at the moment so I put it into `nursery`.

There is a trait `StructuralPartialEq` for solving this problem which is promising but it has several problems currently:
* Integers and tuples doesn't implement it.
* Some types wrongly implement it, like `Option<T>` when `T` doesn't implement it.

I consider them bugs and against the propose of `StructuralPartialEq`. When they become fixed, this lint can become a useful lint with a single line change.

changelog: New lint: [`equatable_if_let`]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants