rust-lang/rust-clippy

if_same_then_else Should not warn for `else if` blocks.

Open

#3770 opened on Feb 16, 2019

View on GitHub
 (29 comments) (5 reactions) (0 assignees)Rust (10,406 stars) (1,391 forks)batch import
I-false-positiveS-needs-discussiongood first issue

Description

if condition1() {
  1
} else if condition2() {
  1
} else if condition3() {
  2
} else {
  3
}

There are many cases where code like this shouldn't be warned on. While you could do condition1() || condition2() often times it is more clear of the intent if they are separate. Especially if the conditions are non-trivial compared to the value being returned.

I think the right time for this warning to warn is if the last if has the same condition as the else. For example the following two cases would warn.

if condition1() {
  1
} else {
  1
}

if condition1() {
  1
} else if condition2() {
  2
} else {
  2
}

Contributor guide

Tech stack
rust
Domain
toolingdeveloper experience
Issue type
feature
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
3
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
3-5 days
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
stale
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
clear
Prerequisites
Rust programmingClippy lint internals
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
40
Research direction
Modify the `if same then else` lint in `clippy lints/src/if same then else.rs` to skip warning on `else if` blocks where the last `if` does not have the same condition as the final `else`. The current logic should be adjusted so that only when the last `if` matches the `else` branch does it warn. Update the corresponding test file `tests/ui/if same then else.rs` to include new test cases. Comments in the issue discuss edge cases like non trivial conditions and nested else ifs.