`iter_skip_zero` denies code where `Skip<T>` is required in the type

[holly-hacker]

Summary

I'm writing some branching code that returns a value on each branch and their types must match. In some branches I call .skip(n) with a non-zero n, so I must call .skip(0) in the other branches so the type matches up. Clippy currently denies this code by default even though it is correct.

This has also been brought up in the PR that introduced this lint, but no issue has been made yet: #11046 (comment)

Lint Name

iter_skip_zero

Reproducer

I tried this code:

fn main() {
    let my_input = "A";
    let my_data = "123456";

    let my_slice = match my_input {
        "A" => my_data.chars().skip(1).take(3),
        "B" => my_data.chars().skip(2).take(1),
        "C" => my_data.chars().skip(2).take(4),
        _ => my_data.chars().skip(0).take(6),
    };

    println!("{}", my_slice.collect::<String>());
}

I saw this happen:

error: usage of `.skip(0)`
 --> src\main.rs:9:35
  |
9 |         _ => my_data.chars().skip(0).take(6),
  |                                   ^ help: if you meant to skip the first element, use: `1`
  |
  = note: this call to `skip` does nothing and is useless; remove it
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_zero
  = note: `#[deny(clippy::iter_skip_zero)]` on by default

error: could not compile `clippy-lint-repro` (bin "clippy-lint-repro") due to previous error

I expected to see this happen:
Clippy should recognize that skip(n) changes an iterator's type and may be required.

Version

rustc 1.73.0 (cc66ad468 2023-10-03)
binary: rustc
commit-hash: cc66ad468955717ab92600c770da8c1601a4ff33
commit-date: 2023-10-03
host: x86_64-pc-windows-msvc
release: 1.73.0
LLVM version: 17.0.2

Additional Labels

No response

[RoelKluin]

A similar pattern that also triggers an false positive error occurs when using map_or_else like so:

self.tuple_index().map_or_else(
    || in_idx[0].iter().skip(0).take(in_idx[0].len()),
    |i| in_idx[0].iter().skip(i).take(1)
).map(|&item_index| {
    let ci = task_manager.get_item(item_index);
    self.apply_to_in(ci, task, task_manager, None, keep)
})
.collect::<Result<Vec<ChannelItem>, ChannelError>>()

Similarly, if you try to remove the skip(0) then the differing iterators cause compilation errors.

Edit: a way to prevent the issue is assigning skip and take values first:

let (skip, take) = match self.tuple_index() {
    None => (0, in_idx[0].len()),
    Some(i) => (i, 1)
};
in_idx[0].iter().skip(skip).take(take).map(|&item_index| {
    let ci = task_manager.get_item(item_index);
    self.apply_to_in(ci, task, task_manager, None, keep)
})
.collect::<Result<Vec<ChannelItem>, ChannelError>>()

and I think that code is cleaner, could apply to holly-hackers code as well.

Maybe the clippy lint could suggest assigning a skip and take variable if required to overcome branch differences.