== check instead of opt.map_or(false, |val| val == n) or if let Some(n) = opt

[laralove143]

What it does

It converts a code like

opt.map_or(false, |val| val == 5)

to

opt == Some(5);

This could apply to Result and technically any enum wrapping a value, for example

matches!(err, Error::Io(5));

to

err == Error::Io(5);

Lint Name

Unnecessary pattern matching

Category

style

Advantage

• Readability
• Conciseness
• Possible optimization (Could someone that can read assembly verify this? The resulting assembly has fewer lines)

Drawbacks

The enum needs to implement PartialEq

Example

#[derive(PartialEq)]
enum Error {
    Io(u16),
    Os(u16),
}

fn unnecessary_pattern_match() {
    let err = Error::Os(1);
   matches!(err, Error::Io(5));
}

fn unnecessary_map_or() {
    let opt = Some(1);
    opt.map_or(false, |val| val == 5);
}

Could be written as:

#[derive(PartialEq)]
enum Error {
    Io(u16),
    Os(u16),
}

fn unnecessary_pattern_match() {
    let err = Error::Os(1);
    err == Error::Io(5);
}

fn unnecessary_map_or() {
    let opt = Some(1);
    opt == Some(5);
}

[llogiq]

Note that depending on the PartialEq implementation, this lint might change behavior! There is no guarantee that those will be equal. Consider:

enum AllTheSame {
    One(u32),
    Other(bool),
}

impl core::ops::PartialEq for AllTheSame {
    fn eq(&self, other: &Self) -> bool { true }
}

This is a valid type with a valid PartialEq impl, yet changing matches!(ats, AllTheSame::Other(true)) to ats == AllTheSame::Other(true) would effectively remove the check.

What we might try is check if the PartialEq impl was derived and only lint for those types.

[laralove143]

Good point but yes we can do that, or we can check if it's equal to the pattern match (not sure if you can run code while linting)

Another consideration though is if they implemented it custom, maybe they don't want you to use pattern matching, for example:

enum Num {
    Small(u8),
    Large(u64),
}

assert_eq!(Num::Small(5), Num::Large(5));

[llogiq]

We can run code while linting, but only constant code, which won't be too helpful in this case. Apart from checking if the impl is derived, we could also just have a list of types to check for that might be user-extensible via config.

[laralove143]

They could just allow the lint in that case, I think checking if the impl is derived is our best bet then

[llogiq]

There is still a difference between allowing the lint wholesale or giving the user the option to disable it by type.

[laralove143]

I meant allowing it for a module/method but the config makes sense too

[Jacherr]

I can take a crack at implementing this.

@rustbot assign

[rustbot]

Error: Parsing assign command in comment failed: ...' assign' | error: specify user to assign to at >| ''...

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

[Jacherr]

Whoops

@rustbot claim