Skip to content

Suggestion: comparing the order of Options or Results #15851

@teor2345

Description

@teor2345

What it does

Lint when an ordered comparison is used on Option<T> or Result<T, E>.

Advantage

  • if the comparison was originally coded on T, then the Option or Result was added, it will still compile, but might have unexpected outcomes
  • the ordering of Some and Ok is different, which is confusing (and I have to look it up every time)
  • the developer most likely wants to handle the None and Err cases specially
  • the suggested code explicitly handles None and Err, instead of hiding it in the comparison

Drawbacks

It could fire on a lot of code, but that code is still suspicious code. So this could be a "correctness" or "suspicious" lint, even if it starts as "allow by default".

Example

For example, with Option, None is less than Some:

let a = Some(1);
let b = None;
if a > b { ... }

Is equivalent to:

let a = Some(1);
let b = None;
if b.is_none() || let Some(a) = a && let Some(b) = b && a > b { ... }

For Result the logic is reversed (Ok is less than Err), which is confusing:

let a = Err(());
let b = Ok(1);
if a > b { ... }

Could be written as:

let a = Err(());
let b = Ok(1);
if b.is_ok() || let Ok(a) = a && let Ok(b) = b && a > b { ... }

Note that is it b.is_none() and b.is_ok(), which is counter-intuitive.

Comparison with existing lints

There are no clippy or rustc lints that currently trigger on this code.

Additional Context

The developer might have actually wanted:

if let Some(a) = a && let Some(b) = b && a > b { ... }

Or:

if a? > b? { ... }

But those are not what a > b means.

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-lintArea: New lints

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions