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.38k stars 1.53k forks source link

New lint: suggest using `transpose` to deal with fallible iterable APIs #4171

Open Nemo157 opened 5 years ago

Nemo157 commented 5 years ago
struct Foo(usize);

impl Foo { 
    fn next(&mut self) -> Option<Result<usize, usize>> {
        self.0 += 1;
        match self.0 {
            i if i > 3 => None,
            3 => Some(Err(3)),
            i => Some(Ok(i)),
        }
    }
}

fn main() -> Result<(), usize> {
    let mut foo = Foo(0);
    while let Some(i) = foo.next() {
        let i = i?;
        println!("{}", i);
    }
    Ok(())
}

The loop in main could instead be written

    while let Some(i) = foo.next().transpose()? {
        println!("{}", i);
    }

I don't know how easy this would be to detect, and because of for loops it is not that widely applicable, but IMO it does lead to an appreciable increase in code readability.

One upcoming usecase where this will come up a lot is working with async Streams, those will commonly be streams of results and can't be used with for loops, so the standard way to work with them should be

while let Some(item) = stream.next().await.transpose()? {
}
cowlicks commented 2 months ago

I found transpose useful when I am creating an Option<T> within a function that returns some Result<..> because it lets you use ?. An example is illustrative:

struct Foo {
    x: Option<u64>,
}
// with transpose
fn parse_foo(x: &str) -> Result<Foo> {
    Ok(Foo { x: x.strip_prefix("z").map(|a| a.parse()).transpose()? })
}
/// without transpose
fn parse_foo2(x: &str) -> Result<Foo> {
    Ok(Foo {
        x: match x.strip_prefix("z") {
            Some(x) => Some(x.parse()?),
            None => None,
        },
    })
}

A match statement with None => None and where the Some branch uses ? would likely be a good candidate to be rewritten as .map(...).transpose()?.

This github search shows tons of examples (and many more that could just use regular old Option::map) https://github.com/search?q=language%3Arust+%22None+%3D%3E+None%22&type=code

cowlicks commented 2 months ago

This issue has an async/await tag but it does not relate to async/await. Just iterables.