rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.28k stars 1.52k forks source link

Lint x != a || x != b #3715

Open baumanj opened 5 years ago

baumanj commented 5 years ago

I recently came across a bug in production code of the form:

if x != a || x != b {
    // create entry with value x
}

This was a bug, but it's not terribly obvious in that form. My guess is that it was copy-pasted from a different part of the code that did a something like:

if x == a || x == b {
    // use x
}

which was then inverted but without changing the || to &&. If we use De Morgan's law, it becomes clearer why a test like x != a || x != b is almost certainly wrong:

x != a || x != b

is equivalent to

!(x == a && x == b)

which makes it easier to see that the inner expression is false unless a == b. But this way of writing it is very unclear. If that's the actual intent, this reformulation is more legible (assuming transitivity of std::cmpEq:

a == b && x == a

Since I think x != a || x != b is more likely the result of a a mistaken conversion of x == a || x == b, it's worth linting.

oli-obk commented 5 years ago

I feel your pain, but I don't see how we can sensibly do anything here.

a == b && a == x is just as unclear as x == a && x == b or z == y && z == w, it's just a name substitution in the end. The only reason it seems clearer is that x is in a different "set" than a and b.

baumanj commented 5 years ago

Sure, but I'm not suggesting linting a == b && a == x, I'm suggesting linting x != a || x != b. It's the unusual combination of != and || that defines the lint. I think the instances where that's really what people intended to write is vanishingly small, no?

oli-obk commented 5 years ago

Probably.. I still worry about false positives, even though I have to admit I can't come up with a counterexample.

We can definitely start with a nursery or pedantic lint.

baumanj commented 5 years ago

That would be awesome! I love pedantry.