Improper error about else if collapse.

[wdanilo]

Hi, I've got the following code:

        if t1 > t2 {
            if      value >= t1 { ValueCheck::Correct }
            else if value >= t2 { ValueCheck::Warning }
            else                { ValueCheck::Error   }
        } else {
            if      value <= t1 { ValueCheck::Correct }
            else if value <= t2 { ValueCheck::Warning }
            else                { ValueCheck::Error   }
        }

Clippy tells me:

warning: this `else { if .. }` block can be collapsed
   --> monitor.rs:328:16
    |
328 |           } else {
    |  ________________^
329 | |             if      value <= t1 { ValueCheck::Correct }
330 | |             else if value <= t2 { ValueCheck::Warning }
331 | |             else                { ValueCheck::Error   }
332 | |         }
    | |_________^

I believe this should not happen.

[flip1995]

That is technically correct. You could write this as

if t1 > t2 {
    if      value >= t1 { ValueCheck::Correct }
    else if value >= t2 { ValueCheck::Warning }
    else                { ValueCheck::Error   }
} else if value <= t1 { 
    ValueCheck::Correct 
} else if value <= t2 { 
    ValueCheck::Warning 
} else { 
    ValueCheck::Error   
}

which has the same semantic.

Anyway, I agree, that in this case not collapsing the else { if is more readable. Maybe we could bail out when the ExprKind of all if arms is an if {..} else {} expression and nothing else.

[Manish-Giri]

Hello, can I work on this?

[flip1995]

Sure! If you have any questions, just ask here or open a WIP PR anytime. A good place to start is this documentation how clippy lints are implemented and tested.

[boxdot]

@Manish-Giri: Are you still working on this? Otherwise, I would like to try to fix it.

[Manish-Giri]

@boxdot Hello, sorry I thought this got re-assigned. I had already done some work on this earlier, so I'll get on it rightaway and finish it as soon as I can.

[rwardd]

@rustbot claim

[rwardd]

Hi, since this has been inactive for a while, I thought I'd go ahead and implement a fix