should comparison_chain only trigger for "if x > y {} else if x <y {} else {}" ?

[matthiaskrgr]

This code

fn bla(a: i32, b: i32) {
    if a > b {
    } else if a < b {
    }
}

Triggers clippy::comparison_chain:

warning: `if` chain can be rewritten with `match`
 --> src/lib.rs:2:5
  |
2 | /     if a > b {
3 | |     } else if a < b {
4 | |     }
  | |_____^
  |
  = note: `#[warn(clippy::comparison_chain)]` on by default
  = help: Consider rewriting the `if` chain to use `cmp` and `match`.
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#comparison_chain

however using a match forces us to cover one additional case which the if-construct does not have: Ordering::Equal is not covered originally and we would have to use something like
_ => {} to get our code to compile.

Perhaps the lint should be refactored to only warn if there is also a final else/if there are 3 cases that are handled by the if-condition construct?

clippy 0.0.212 (e8d5a9e 2019-10-22)

[flip1995]

Or suggest

if let Ordering::Greater = _ {

} else if let Ordering::Less = _ {

}

[rohitjoshi]

I don't understand this suggestion. Condition is checking if random_size > 0.

warning: `if` chain can be rewritten with `match`
   --> xxx/src/cluster.rs:193:9
    |
193 | /         if random_size > 0 {
194 | |             let my_uuid = Uuid::new_v4();
195 | |             let uuid_str =  my_uuid.to_simple().encode_lower(&mut Uuid::encode_buffer()).to_string();
196 | |             let (prefix, _)  = uuid_str.split_at(random_size as usize);
...   |

200 | |         }
    | |_________^
    |
    = note: `#[warn(clippy::comparison_chain)]` on by default
    = help: Consider rewriting the `if` chain to use `cmp` and `match`.
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#comparison_chain

[flip1995]

The lint suggest that you rewrite it with

match random_size.cmp(&0) {
    Ordering::Greater => { .. },
    _ => {},
}

which admittedly makes the code worse, IMO. We shouldn't lint, if literals are involved.

[camsteffen]

@rohitjoshi Are you saying that it can lint when there is just an if with no else? That definitely should not be the case.

For the original suggestion, I agree, but only for primitive types. For large, complex types, it is good to avoid the second comparison, maybe for strings too.

[rohitjoshi]

Yes, with if statement comparing with const value 0, It shows suggestion to replace with match/compare statement.

[camsteffen]

Can you reproduce that in a Rust playground? I wasn't able to.

[rohitjoshi]

Sorry, it does have else if < 0 condition.