Discourage use of Option<Error>

[behnam]

I'm seeing Option<Error> function return types, which are actually something very hard to handle using Rust's core/std libs. Basically, there's no one method to do expect-to-be-None.

Also, IIUC, means that ? cannot be used on it.

And, there's no single method to convert it to Result<(), Error> to handle.

So, maybe we can have a guideline for how to use Option vs Result and discourage use of Option<Error> ?

[BurntSushi]

Option<Error> is useful for describing a state of partial failure.

[behnam]

I think partial failures need to use a custom type that also set #[must_use], otherwise it depends on documentation just a bit too much.

[BurntSushi]

It would be nice to annotate the method itself if that were possible as #[must_use], since I feel like declaring a custom Option is also somewhat unfortunate.

[BurntSushi]

Unbeknownst to me, there was actually an RFC proposing the addition of #[must_use] to functions/methods, and it was just recently merged. So it seems like we could just wait for that to drop and then apply #[must_use] to functions that return partial failures.

@behnam What do you think of renaming this issue? I think it's less about "discourage Option<Error>" and more about "create guidelines for reporting partial success/failure."

[behnam]

Oh, I didn't realize that the function/method must-use has got a new RFC! That's great!

Sure, @BurntSushi!

With this RFC, the focus of this matter shifts towards "remember to must-use if returning any failure status with a type that's not must-use", for me.

I don't have much clue on what a good API for "partial success/failure" would be.

Here's another example that kept me busy today: std::fs::ReadDir has an iterator with type Item = Result<DirEntry>, meaning that I can map these values, but cannot use ? inside the map. So, just to walk inside a dir, I have to write a for loop if want to handle possible errors, which are not actually generally expected.

[KodrAus]

I can't actually think of an example of an API that returns Option<Error>, and would personally be surprised to see it in an API as opposed to Result<(), Error>. The Option API already bakes in assumptions that Some is a "success" variant and None is an "error" variant with methods like Option::ok_or and its implementation of Try so feel like Option<Error> should be discouraged in favour of Result<(), Error>.

[BurntSushi]

@KodrAus It's not an either-or thing. Of course, if Result<T, Error> works then folks should use that. But as I said above, Option<Error> is useful for reporting partial failure. Here's an example: https://docs.rs/ignore/0.4.17/ignore/gitignore/struct.GitignoreBuilder.html#method.add

But this is somewhat of a niche use case. Maybe the right thing to do here is to say that folks "should use Result<T, Error> in the vast majority of cases, and use something only different in special circumstances, e.g., to deal with partial success."

Stepping back a bit, it seems a little weird to even suggest this at all. I haven't really seen folks use Option<Error> when Result<T, Error> would be appropriate? @behnam Maybe you could say where you saw this happening? (Although I acknowledge you opened this issue 4 years ago... So perhaps things have changed.)

[behnam]

Iirc, @BurntSushi, I was actually integrating the gitignore library into cargo when noticed this pattern, and filed this issue because (again, iirc) it was surprising to see on the public-facing part of the API. (I believe the project page has mentioned following these API guidelines, or something like that.)

Now, after a few years, I think it's true that it's not such a common pattern to worry about. However, both Option and Error types being built to the language, I can imagine that "can I just use Option<Error>?" being a frequent question for people learning the language and thinking about private/public API design patterns.

So, maybe it would be nice to add a conclusion of this discussion to the guidelines?