Implement an unused result lint

[alexcrichton]

The general consensus is that we want to move away from conditions for I/O, and I propose a two-step plan for doing so:

1. Warn about unused Result types. When all of I/O returns Result, it will require you inspect the return value for an error only if you have a result you want to look at. By default, for things like write returning Result<(), Error>, these will all go silently ignored. This lint will prevent blind ignorance of these return values, letting you know that there's something you should do about them.
2. Implement a try! macro:

macro_rules! try( ($e:expr) => (match $e { Ok(e) => e, Err(e) => return Err(e) }) )

With these two tools combined, I feel that we get almost all the benefits of conditions. The first step (the lint) is a sanity check that you're not ignoring return values at callsites. The second step is to provide a convenience method of returning early out of a sequence of computations. After thinking about this for awhile, I don't think that we need the so-called "do-notation" in the compiler itself because I think it's just too specialized. Additionally, the try! macro is super lightweight, easy to understand, and works almost everywhere. As soon as you want to do something more fancy, my answer is "use match".

Basically, with these two tools in action, I would be comfortable removing conditions. What do others think about this strategy?

---

This PR specifically implements the unused_result lint. I actually added two lints, unused_result and unused_must_use, and the first commit has the rationale for why unused_result is turned off by default.

[bill-myers]

Nice; what about going much further for #[must_use] types and making them linear types?

That is, making this always an error, and triggering it whenever a #[must_use] type is dropped without destructuring, rather than just when that happens as a result of a statement with a #[must_use] type (other cases are variables going out of scope, being assigned other values to, _ patterns, maybe something else).

[alexcrichton]

That's not a bad idea, but for now I'd rather lean in the direction of keeping the lint super simple and easy to understand. The major use case this is trying to solve is silently ignoring return values from I/O operations by accident. If you specify a _ in a pattern, you had to at least explicitly ignore it (or at least that's the idea).

I think that the best trial run for this lint will be removing conditions from libstd. If it turns out that this lint isn't sufficient enough to catch most of the ignored errors (as today there are many because conditions fail by default), then we should certainly revisit this at that point.

Basically for now I want to try to keep this as simple as possible, but I don't mind revisiting at all if we start seeing common patterns that ignore errors.

[kud1ing]

Sounds good. Only concern is the association try! => try/catch.

[bill-myers]

The codebase already contains that macro with the name if_ok!, and or_return! was also proposed as a name.

None of these names seems particularly good; a descriptive name would either be "propagate_err!", "on_err_return!" or "return_if_err!", but those all seem too long.

[whitequark]

This is really nice. OCaml has -strict-sequence and my personal experience is that it catches more harmful errors than any other lint.
I'd also say it should be enabled (as a warning) by default.

[zargony]

This is great! I would maybe even make it an error if a #[must_use] result isn't used.

Now that it's possible to use return in a closure, how about introducing unwrap_or_else for Result? That should make it possible to do something like

let value = io_operation().unwrap_or_else(|e| return e);

That's even longer but also more readable I guess.

I suppose unwrap_or_return isn't possible as a method of Result (but it would be a nice method macro if macros could be used as methods)

[wycats]

@alexcrichton I like this. I already started playing around with the try! macro in my own projects and it works pretty well. The unused Result linter is the final piece of the puzzle.

[kud1ing]

I was wondering if a ignore! would be a useful companion. Sometimes we ignore a function result, because we either assume nothing can go wrong or we don't care if things go wrong. For example: if you just want to clean something up, and if removing a file fails e.g. due to permissions ... whatever, we tried. It could be handy being able to say "ignore the result for just this call".

[wycats]

You can easily implement ignore! via:

let _ = file.read()

It might be nice to have an alias for clarity, but I don't love adding a set of parens around everything vs. let _.

[kud1ing]

Right.