Don't use PartialEq to check if an Option is None

[lukaslueg]

What it does

I've seen people use foo == None or foo != None to check if Option<T>::is_none() / is_some(). This works fine as long as T is PartialEq, yet is unnecessary and may lead to surprising type errors later on in the programmer's life when the T is not PartialEq. The pattern most likely stems from other programming languages, where checking for null is expressed as foo is None or something similar. The programmer may not be aware that the comparison piggybacks on T: PartialEq, which is only practically useful in the Some-case.

Equality comparisons with Option::None should not be done in that way, and we can recommend Option::is_none() / is_some() or structural comparison, which are strictly more powerful and work for any Option<T> regardless if T: PartialEq

Lint Name

partialeq_to_none

Category

style, pedantic

Advantage

• Intention is expressed more clearly
• The triggering code is using the PartialEq trait on T in Option<T>, which the programmer is probably not aware of
• We can replace the triggering code with a strictly more powerful alternative.

Drawbacks

No response

Example

pub fn foo(x: Option<u32>) -> &'static str {
    if x != None {
        "yay"
    } else {
        "nay"
    }
}

Could be written as:

pub fn foo(x: Option<u32>) -> &'static str {
    if x.is_some() {
        "yay"
    } else {
        "nay"
    }
}

[xFrednet]

@rustbot label +good-first-issue

[jmj0502]

@xFrednet Hey! Hope you're doing great. I'm a Rust newbie and would like to make a contribution. Can I take this issue (I may need some guidance along the way)?

[xFrednet]

@jmj0502 hey I'm doing great, hopefully you as well and welcome to Clippy! Sure you can claim it with @rustbot claim.

If you need any help feel free to ping me! To get started you might want to look at our documentation. CONTRIBUTING.md should contain links to it 🙃

[jmj0502]

@xFrednet I'm glad you're doing cool! I'm doing great as well! c:
Thanks for all the support! Do have any idea of what files should I check to start working on this specific issue (aside from the CONTRIBUTING.md of course)?

[jmj0502]

@rustbot claim

[xFrednet]

Good question, after some searching I found clippy::redundant_pattern_matching which does a check for if let None = <expr>. While I wouldn't add it to that file, it might give you some pointers.

Now, I'm debating if this should actually be an additional to that lint, but the name wouldn't fit and the reason for the suggestion is also not the same. Having this as an extra lint is probably better. Maybe this is also applicable to Result, but we can look at that after the initial implementation :)

I couldn't find a lint with such a pattern, but some that do similar checks. So, I would suggest crating a new file, maybe with cargo dev new_lint --pass late. Then you check each expression in check_expr for ExprKind::Binary with BinOpKind::Ne and None on either side. This might need some creativity, are you stil up for it? 🙃

[jmj0502]

@xFrednet Hey! Hope you're doing great! Sorry for the late reply (I was sick 🤕). Everything sounds interesting, but I see that @lukaslueg is already working on the issue. Thanks for all the guidance 👍! Looking forward to contribute in the future!

[xFrednet]

Hey @jmj0502, there is no reason to apologies. hope you have fully recovered again. If you want to work on another issue feel free to ping me if you need some assistance 🙂