rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.25k stars 1.52k forks source link

`clippy::never_loop` suggests #12931

Open ErichDonGubler opened 3 months ago

ErichDonGubler commented 3 months ago

Summary

In some cases, clippy::never_loop suggests a replacement that would still trigger clippy::iter_skip_next.

Minimizing clippy feedback loops seems important to avoid user frustration. Therefore, it seems valuable to modify never_loop suggestions based on a check against iter_skip_next (and maybe other lints?), to ensure the modified snippet won't get rejected again.

Reproducer

I tried this code in the Rust playground and ran Clippy:

fn main() {
    for thing in [1, 2, 3].iter().skip(1) {
        panic!("oh noes, too many things");
    }
}

I expected to see this happen:

    Checking playground v0.0.1 (/playground)
warning: unused variable: `thing`
<snip>

error: this loop never actually loops
 --> src/main.rs:2:5
  |
2 | /     for thing in [1, 2, 3].iter().skip(1) {
3 | |         panic!("oh noes, too many things");
4 | |     }
  | |_____^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#never_loop
  = note: `#[deny(clippy::never_loop)]` on by default
help: if you need the first element of the iterator, try writing
  |
2 |     if let Some(thing) = [1, 2, 3].iter().nth(1) {
  |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

warning: `playground` (bin "playground") generated 1 warning
error: could not compile `playground` (bin "playground") due to 1 previous error; 1 warning emitted

Instead, this happened:

    Checking playground v0.0.1 (/playground)
warning: unused variable: `thing`
<snip>

error: this loop never actually loops
 --> src/main.rs:2:5
  |
2 | /     for thing in [1, 2, 3].iter().skip(1) {
3 | |         panic!("oh noes, too many things");
4 | |     }
  | |_____^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#never_loop
  = note: `#[deny(clippy::never_loop)]` on by default
help: if you need the first element of the iterator, try writing
  |
2 |     if let Some(thing) = [1, 2, 3].iter().skip(1).next() {
  |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

warning: `playground` (bin "playground") generated 1 warning
error: could not compile `playground` (bin "playground") due to 1 previous error; 1 warning emitted

Version

rustc 1.78.0 (9b00956e5 2024-04-29)
binary: rustc
commit-hash: 9b00956e56009bab2aa15d7bff10916599e3d6d6
commit-date: 2024-04-29
host: aarch64-apple-darwin
release: 1.78.0
LLVM version: 18.1.2

Additional Labels

@rustbot label +I-suggestion-causes-error +L-suggestion

Alexendoo commented 3 months ago

Marked it as not https://github.com/rust-lang/rust-clippy/labels/I-suggestion-causes-error even though yeah that's an error because the label generally refers to non-lint errors

Suggestions triggering other lints is sometimes worked around for common occurrences but often we leave it as is, the extra checks can make the existing lints substantially more complicated