Anti-pattern `is_err()`, `return`, then `.unwrap()`

[code423n4]

Handle

nascent

Vulnerability details

[I-02] Anti-pattern is_err(), return, then .unwrap()

Severity: Medium

Throughout the code this pattern is used:

let res: Result<Bar, ()> = foo();
if res.is_err() {
    log!("error");
    return;
}
let res: Bar = res.unwrap();
let res2: Result<Baz, ()> = do_stuff(&res);
if res2.is_err() {
    log!("error");
    return;
}
let res2 = res2.unwrap();
let res3: Result<Bar, ()> = do_stuff2(res2);

The same pattern is used with Option<> types (is_some()). Problems can arise when many .unwrap()s become the norm, allowing for unintentional opportunities for crashes.

Recommendation

One alternative that avoids .unwrap()ing intermediate Results and returning could be to use combinators

let res3 = foo()
    .as_ref()
    .map_err(|_| log!("error"))
    .and_then(do_stuff)
    .and_then(do_stuff2);

[jkilpatr]

I am really strongly opposed to cult like rules like 'no unwrap'. I can agree that moving this sort of log, unwrap behavior into a handler function that is then re-used could prevent mistakes where unwrap is improperly used. But banning tools is not a good way to engineer anything.

Anyways I agree this is a valid style nit.

[loudoguno]

reopening as per judges assessment as "primary issue" on findings sheet