False positive: nonminimal_bool for types which implement PartialOrd but not Ord

[0ndorio]

The nonminimal_bool lint suggests to minimize the following expression !(a <= b) into a > b. While this is a proper improvement for types implementing Ord it doesn't fit for all types of PartialOrd and would actually change the result of the expression as it means 'greater or incomparable' and not just 'greater'.

Maybe clippy should instead suggest to rewrite the expression using the partial_cmp() method and a match statement.

Example:
https://play.rust-lang.org/?gist=25676640c1f106e733d008dc224a9d49&version=stable

[oli-obk]

Can you create a small example that we can use for our test suite? I tried, but failed: https://play.rust-lang.org/?gist=2261653bc50d1ca6d8e0e0b7c1caf3e1&version=stable

all assertions succeed, I would have expected one of them to fail.

[0ndorio]

There seems to something weird with the derived PartialOrd trait in my case. I opened another issue in the rust repository (rust-lang/rust#49650).

If you exchange StructWithoutOrd with a type witch implements the partial ordering properly (e.g. f64) your asserts start to fail:

https://play.rust-lang.org/?gist=25c68ced5fa5fb0c2470f51490998d70&version=stable

[oli-obk]

We should be able to bail out on those optimizations when the type does not impl Ord. Still.. I think we should rather lint such patterns generally, because everyone reading !(x <= y) will be totally flabbergasted. This pattern should have its own lint just telling the user to seriously consider using a specialized method or a match. Might be more verbose, but better verbose than winning an obfuscation championship.

If we just allow this pattern without a new lint it'll show up in the next underhanded rust competition

[0ndorio]

I am totally with you and would encourage a related lint warning about this. This issue is only here to warn about the false positive.

[0ndorio]

I would like to fix that issue and introduce the missing lint.

[oli-obk]

Great! It's all yours