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

[scottmcm]

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))
    }

[Jarcho]

Complicated conditions tend to be longer, so at least they would get formatted across lines. I don't think I would ever see the if version as simpler to understand, but maybe someone would.

Do you have a suggestion for the name. I'm at either ifs_as_logical_ops or if_then_bool_else_false. I'd rather the disjunction form also be the same lint which rules out the second name.

[barun511]

Hm, brand new lint seems challenging - but would love to give it a go.

[barun511]

@Jarcho are we aligned that this is something we want to do? happy to have a first crack. for the name, I like ifs_as_logical_ops better.

I'd probably write a first version to only do the conjunctive lint, and then flup with the disjunctive version
@rustbot claim

[Jarcho]

The lint itself is fine. There is a question as to what category to put it in, but that can wait till a PR is made.

[scottmcm]

Personally I would propose category style for this. There's nothing wrong with the code, there's just a slightly better way to write it. I guess you could call it manual_logical_and or something.

(It's not correctness or performance, at least.)