rust-lang/rust-clippy

Suggest `A && B` for `if A { B } else { false }`

Open

#14865 opened on May 21, 2025

View on GitHub
 (6 comments) (4 reactions) (1 assignee)Rust (10,406 stars) (1,391 forks)batch import
A-lintgood first issue

Description

What it does

Inspired by this code in the standard library: https://doc.rust-lang.org/src/std/collections/hash/set.rs.html#864-866

    pub fn is_subset(&self, other: &HashSet<T, S>) -> bool {
        if self.len() <= other.len() { self.iter().all(|v| other.contains(v)) } else { false }
    }

which could instead be

    pub fn is_subset(&self, other: &HashSet<T, S>) -> bool {
        self.len() <= other.len() && self.iter().all(|v| other.contains(v))
    }

When the "then" block is a single simple expression (rather than a complex thing with a bunch of lets, say), seeing else { false } is a hint that the code can be simplified without changing behaviour or performance.

Playground repro that this is not linted today: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2024&gist=90c86a06c5fce6c0f816f5f4ec170cc5


Similarly, suggesting A || B instead of if A { true } else { B } would also be good.

Advantage

  • Shorter and arguably simpler code
  • Makes it clearer that this is an "and"
    • By having things that are just &&s be &&s it helps emphasize the places that do need the if-else for their less-common operation.

Drawbacks

If either condition is particularly complicated, splitting it up into the two pieces might help a reader grok the intent.

Example

    pub fn is_subset(&self, other: &HashSet<T, S>) -> bool {
        if self.len() <= other.len() { self.iter().all(|v| other.contains(v)) } else { false }
    }

Could be written as:

    pub fn is_subset(&self, other: &HashSet<T, S>) -> bool {
        self.len() <= other.len() && self.iter().all(|v| other.contains(v))
    }

Contributor guide