Should closures of StreamExt::map and TryStream::map_{ok, err} return a Future?

[taiki-e]

Currently, closures of StreamExt::map and TryStream::{map_ok, map_err} return a value, not a Future. It is probably preferable to change this to return a Future like any other combinator.

The async closure version of map is then, but since the current Future does not return Result, I think it can be removed.

cc @cramertj @Nemo157

[seanmonstar]

As you mentioned, there is then that does this already.

[taiki-e]

@seanmonstar Yeah, but map_ok and map_err are not. And if we use async closure with them, I'm thinking about using the async closure in map for consistency and remove then.

[seanmonstar]

The equivalent is and_then and or_else.

[taiki-e]

No, their closures return TryFuture.

https://docs.rs/futures-preview/0.3.0-alpha.19/futures/stream/trait.TryStreamExt.html#method.and_then

[Nemo157]

That makes it seem like there's a missing then_ok/then_err pair or something.

If you think of Future::map as compared to {Result, Option, Iterator}::map then it makes sense that map takes a closure returning a value instead of a future (they all operate on the value within the monad). Future::then is the equivalent of Result::and_then/Option::and_then/Iterator::flat_map (monadic bind).

The difficulty with TryFuture's methods is that they're trying to create a composition of 2 monadic structures (one of which is monadic across 2 axes), combining Future::{map, then} with Result::{map, map_err, and_then, or_else} means there are 8 possible methods available, using the straightforward combination of their names:

trait TryFutureExt {
    // existing `map_ok`
    fn map_map<T>(self, impl FnOnce(Self::Ok) -> T)
        -> impl Future<Output = Result<T, Self::Err>>;

    // existing `map_err`
    fn map_map_err<T>(self, impl FnOnce(Self::Err) -> T)
        -> impl Future<Output = Result<Self::Ok, T>>;

    fn then_map<T>(self, impl FnOnce(Self::Ok) -> impl Future<Output = T>)
        -> impl Future<Output = Result<T, Self::Err>>;

    fn then_map_err<T>(self, impl FnOnce(Self::Err) -> impl Future<Output = T>)
        -> impl Future<Output = Result<Self::Ok, T>>;

    fn map_and_then<T>(self, impl FnOnce(Self::Ok) -> Result<T, Self::Err>)
        -> impl Future<Output = Result<T, Self::Err>>;

    fn map_or_else<T>(self, impl FnOnce(Self::Err) -> Result<Self::Ok, T>)
        -> impl Future<Output = Result<Self::Ok, T>>;

    // existing `and_then`
    fn then_and_then<T>(self, impl FnOnce(Self::Ok) -> impl Future<Output = Result<T, Self::Err>>)
        -> impl Future<Output = Result<T, Self::Err>>;

    // existing `or_else`
    fn then_or_else<T>(self, impl FnOnce(Self::Err) -> impl Future<Output = Result<Self::Ok, T>>)
        -> impl Future<Output = Result<Self::Ok, T>>;
}

(and I just realised that this is talking about Stream rather than Future, that complicates things even more as TryStream is a combination of 3 monads, Future + Iterator + Result, so with the same straightforward expansion from above has 16 possible transformation methods available depending on whether you want to be inside or outside each monad. The above block covers all the Iterator::map based methods, but there's also the Iterator::flat_map methods which don't exist yet).

[cramertj]

I'm removing this from the 0.3 release milestone. I think the current API makes a sort of sense, and we very well may want more versions of these, but settling on the exact naming of each and what combinations to offer is, I think, out of reach for this release.

[Ekleog]

Seeing that StreamExt::filter_map takes a closure that returns a future, the current behavior looks inconsistent to me.

FWIW, what just happened to me is I've been told by the compiler that filter_map takes a closure that returns a Future, then I added async blocks to both the filter_map and the map just above, and then the compiler told me there shouldn't be a Future, and then I had to go lookup the documentation to figure out what the proper APIs were.