Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

if_not_else lints firing on else-ifs #7892

Closed
dswij opened this issue Oct 28, 2021 · 4 comments · Fixed by #7895
Closed

if_not_else lints firing on else-ifs #7892

dswij opened this issue Oct 28, 2021 · 4 comments · Fixed by #7895
Assignees
Labels
C-bug Category: Clippy is not doing the correct thing good-first-issue These issues are a good way to get started with Clippy I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@dswij
Copy link
Member

dswij commented Oct 28, 2021

Lint name: if_not_else

I tried this code:

fn foo() {}
fn bar() {}
fn nothing() {}

#[warn(clippy::if_not_else)]
fn main() {
    let foo_disabled = true;
    let bar_disabled = true;
    
    if !foo_disabled {
        foo()
    } else if !bar_disabled {
        bar()
    } else {
        nothing()
    }
} 

I expected to see this happen: nothing

Instead, this happened: clippy suggests swapping the order of else-if and else block, which makes it less clear imo. There are scenarios where using not in else-ifs makes it more readable like the one above - you would expect it to be written in order of priority/preference. (try foo(), then try bar(), else do nothing)

warning: unnecessary boolean `not` operation
  --> src/main.rs:13:10
   |
13 |       else if !bar_disabled {
   |  __________^
14 | |         bar()
15 | |     } else {
16 | |         nothing()
17 | |     }
   | |_____^
   |
note: the lint level is defined here
  --> src/main.rs:5:8
   |
5  | #[warn(clippy::if_not_else)]
   |        ^^^^^^^^^^^^^^^^^^^
   = help: remove the `!` and swap the blocks of the `if`/`else`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#if_not_else

warning: `playground` (bin "playground") generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 4.06s

Meta

Rust version (rustc -Vv):

rustc 1.46.0-nightly (f455e46ea 2020-06-20)
binary: rustc
commit-hash: f455e46eae1a227d735091091144601b467e1565
commit-date: 2020-06-20
host: x86_64-unknown-linux-gnu
release: 1.46.0-nightly
LLVM version: 10.0

See #7877 (comment)

@dswij dswij added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Oct 28, 2021
@xFrednet
Copy link
Member

Clippy has an until function to check if a given if expression is an else if.

@rustbot label +good-first-issue

@rustbot rustbot added the good-first-issue These issues are a good way to get started with Clippy label Oct 28, 2021
@ahmedkrmn
Copy link
Contributor

Hi @xFrednet, can I work on this one?

@xFrednet
Copy link
Member

Hey @ahmedkrmn, sure, go ahead 👍. You can claim this issue by commenting @rustbot claim to also indicate to others that you're working on it. You are welcome to ask questions if you get stuck 🙃

@ahmedkrmn
Copy link
Contributor

@xFrednet Thanks!
@rustbot claim

bors added a commit that referenced this issue Oct 29, 2021
Disable "if_not_else" lints from firing on else-ifs

Fixes #7892

1. Convert `['if_not_else']` to `LateLintPass` and use `clippy_utils::is_else_clause` for checking.
2. Update tests.
@bors bors closed this as completed in 4c70c18 Oct 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing good-first-issue These issues are a good way to get started with Clippy I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants