rust-lang / rust-lang/rust-clippy

Suggest to replace impl Iterator return types with wrapper types

Open
#5,989 2 comments 0 reactions 1 assignee View on GitHub

@0xkelvin is already working on this.

Since Feb 23, 2024.

A-lint good first issue L-restriction T-middle
Dominant language
Rust
Stars
13.5k
Forks
2.2k
Avg merge
2d 10h
Merged PRs (30d)
32

Description

What it does

It suggests replacing impl Iterator with a wrapper type, if the function is public (other crates can call it).

The impl Trait syntax erases types completely, which makes it impossible to use them as struct fields. This can make the API of a crate less convenient, because you have to for example collect the returned Iterator into a Vec (function_returns_impl_iterator().collect::<Vec<_>>()).

The syntax should be mainly used for situations, where you cannot represent the returned value as a concrete type.

For example

use std::future::Future;

async fn hello() -> impl Future<Output = &'static str> { "hello" }

this function can only return impl Future, because there is no type for the returned value.

What is the advantage of the recommended code over the original code

The returned value can be used by other structs as fields or it could be used with functions that have bounds like this:

fn takes_iterator<I: Iterator<Item=usize>>(iterator: I);
Example

original code:

#![warn(clippy::pedantic, clippy::nursery)]

use std::iter::Iterator;

fn greetings() -> impl Iterator<Item = (usize, &'static str)> {
    vec!["hello", "hallo", "salut", "hola", "ciao"]
        .into_iter()
        .enumerate()
}

fn main() {
    for (number, greeting) in greetings() {
        println!("{}\t=> {:?}", number, greeting);
    }
}

better code

#![warn(clippy::pedantic, clippy::nursery)]

use std::iter::Iterator;

pub struct IntoIter(pub(crate) std::iter::Enumerate<std::vec::IntoIter<&'static str>>);

impl Iterator for IntoIter {
    type Item = (usize, &'static str);

    fn next(&mut self) -> Option<Self::Item> {
        self.0.next()
    }
}

fn greetings() -> IntoIter {
    IntoIter(
        vec!["hello", "hallo", "salut", "hola", "ciao"]
            .into_iter()
            .enumerate(),
    )
}

fn main() {
    for (number, greeting) in greetings() {
        println!("{}\t=> {:?}", number, greeting);
    }
}
Drawbacks

Writing wrapper types can result in quite a lot of boilerplate code that might not be worth the time, because almost every
downstream crate would just iterate over the returned value. Therefore, I suggest making this lint pedantic.

This lint can not be applied to every usage of impl Trait, so I would suggest limiting it to Iterator (can be extended in the future).

Categories (optional)
  • clippy::style
  • clippy::pedantic

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.