unwrap() of Option<_> or Result<_>

[mkpankov]

I believe we should warn about usage of unwrap() of optional types. unwrap() is essentially a pattern match with _ => panic!(), but there are often ways to continue execution of program w/o failing.

Today, it's common to find code like

extern crate term;

fn main() {
    let mut t = term::stdout().unwrap();

    t.fg(term::color::GREEN).unwrap();
    (write!(t, "hello, ")).unwrap();

    t.fg(term::color::RED).unwrap();
    (writeln!(t, "world!")).unwrap();

    t.reset().unwrap();
}

As one might learn pretty quickly, unwrap()ing a None returned by term::stdout() will lead to failure of task. This is, of course, specified in docs. But it's certainly not obvious that this is not a "safe" operation — it may lead to "crash". Not "hard" crash, because it will be handled by Rust runtime. It produces error about unwrapping None. But it's a crash nevertheless.

Warnings from compiler that .unwrap() is essentially a pattern match w/o "catch-all" case are missing here. We do get warnings when not all variants of enum are handled, so why skip it here?

unwrap() of Option<_> is a hack. It's convenient, I know — but it's the sort of convenience we disdain JavaScript and PHP for. I also understand that one is extremely likely to use unwrap() in closures because "heck, I only need to filter/map/reduce stuff, I don't want my closure to spend 5 lines!". Programmer uses it there and then forgets about it. Voila — safety is needlessly violated.

I think a programmer should be forced to actively discard any failure condition. It makes for explicit decision making about not handling an error.

In Rust, in it's present state, unwrap() is the easiest and simplest way to Get Things Done. This shortcut will be taken by many and many people who will then later complain about Rust not being secure despite claiming itself "preventing almost all crashes".

[kmcallister]

Option::unwrap() is usually an anti-pattern, but Result::unwrap() is often perfectly reasonable, because it carries along an error message. It's the simplest way to say "panic the task if IO has failed", which is often what you want.

[llogiq]

Also you can use if myopt.is_some() { ... myopt.unwrap() ... } else { ... }, which can be a bit more composable than a match. On the other hand, in those cases my_opt.map(|...| ...).unwrap_or_else(|| ...) is probably a better solution.

So, after having used Option more and getting the hang of the methods, I think warning on Option::unwrap() may be a good idea, if only to suggest using expect("some useful information here") instead.

[llogiq]

Also see issues #6, #10, #19.