Support try_into annotation?

[fiadliel]

I recently wrote a with which uses TryInto for a fallible conversion. This was doable, but seemed like something that was a bit boilerplate-y. In addition, I had to fix the error type to something known on the output side.

I think this could be improved by an annotation; less boilerplate, and I think the output error type then can be set by the error of the impl in use (by adding a generic parameter to the setter).

Example of what I currently do:

#[builder(with = |v: impl TryInto<MyValue, Error = impl Into<MyError>>| -> Result<_, MyError> { v.try_into().map_err(|e| e.into()) })]

How it could look:

#[builder(try_into)]

How it would look in terms of codegen:

pub fn my_field<Error>(
        self,
        v: impl TryInto<MyValue, Error = Error>,
    ) -> Result<MyConfigBuilder<my_config_builder::SetMyField<S>>, Error>
    where
        S::MyField: my_config_builder::IsUnset,
    {
        self.maybe_my_field(Some(v))
    }

   pub fn maybe_my_field<Error>(
        mut self,
        value: Option<impl TryInto<MyValue, Error = Error>>,
    ) -> Result<MyConfigBuilder<my_config_builder::SetMyField<S>>, Error>
    where
        S::MyField: my_config_builder::IsUnset,
    {
        ... // call try_into here
    }

Would this be worth doing?

[Veetaha]

Hi! TryInto support does look reasonable.

Although, I wonder if we could instead make it less magical with #[builder(with = TryInto::try_into)] syntax instead. I.e. making TryInto a well-known function that you could pass to with.

However, there are several problems.

Uninlined error type (rustdoc issue)

Suppose we use #[builder(with = TryInto::try_into)] or even the shorter #[builder(try_into)] syntax. Then how does the macro know the resulting error type? It could use <T as TryInto<U>>::Error type projection syntax when generating the setter:

fn member<T: TryInto<U>>(self, value: T) -> Result<Builder<SetMember<S>>, <T as TryInto<U>>::Error>;

But.. When you generate documentation for a such a method, rustdoc won't inline this <T as TryInto<U>>::Error. And this.. looks rather noisy. The same problem will be with Rust Analyzer signature hints.

Do we really need to have impl Into<ErrorType> here?

I suppose by default #[builder(try_into)] and #[builder(with = TryInto::try_into)] would generate a signature like this:

fn member<T: TryInto<U>>(value: T) -> Result<U, T::Error>

which seems intuitive. But then how would you ask it to generate a signature like this?:

fn member<T: TryInto<U, Error: Into<ErrorType>>>(value: T) -> Result<U, ErrorType>

The macro needs to be specified with the ErrorType in this case. It could be specified as a parameter to the macro, but.. I'd like to step back a bit.

Is this really a clean API to desire at all?

I'd say the correct question is "is this really desired at all"? I've never seen such setter signature with the APIs I've been working with. The API's that accept TryInto are rather exotic. This is one of the reasons why bon hasn't shipped with builtin special TryInto support yet.

For example if you add #[builder(try_into)] your syntax would change only very slightly:

From this where the setter accepts T:

Foo::builder()
    .setter(foo.parse().try_into()?)
    .build()

to this, where the setter accepts impl TryInto<T>:

Foo::builder()
    .setter(foo.parse())?
    .build()

But in this case, why wouldn't you just make the setter accept impl Into<T> with #[builder(into)] and write code like this instead:

Foo::builder()
    .setter(foo.parse()?)
    .build()

(notice the ? is right after parse() in this case, not after setter(...)).

---

To my mind try_into is overrated, and could instead be avoided and expressed with other means. Maybe if you could provide an example of your use case, I could see if that's not the case. Feel free to respond here or text me on bon's Discord (https://bon.rs.com/discord)

[fiadliel]

My use case is

I'm using https://docs.rs/matchit/latest/matchit/ to generate a set of paths for route matching. The basic API here is somewhat imperative; create a new router, then call insert with each entry, each insert call can potentially fail. There are other values that can also be inserted, which do not use a router.

So I can write a TryInto that encapsulates some simpler APIs for creating these routes, e.g.

impl<IS: Into<String>, IP: Into<ProxyResponse>> TryInto<DomainRoutes> for Vec<(IS, IP)> {
    type Error = RouteError;

    fn try_into(self) -> Result<DomainRoutes, Self::Error> {
        let mut router = matchit::Router::new();

        for (path, response) in self {
            router.insert(path, response.into()).map_err(RouteError)?
        }

        Ok(DomainRoutes::Routes(router))
    }
}

This one above lets me pass a vec![] of routes. It then makes life a lot easier for users of the API, as the calls are a lot more flexible.

The resulting value I'm interested in storing in the built structure is this DomainRoutes value.

---

The error type and impl Into<ErrorType>

I muddied the waters in my example by mentioning the Into<ErrorType> at all. When trying to use the with syntax with TryInto, the associated type Error in TryInto cannot be constrained to be the same as the output type (because we have no generic parameter to perform this function). What we can do is one of two things: hardcode both the associated type and the output type of the fallible builder; OR make the error type slightly more flexible by letting it be something that can be transformed into the output type. In my current code, I followed the second course, but it confuses the basic story here.

So, no we do not need a impl Into<ErrorType> in a general sense, this was just trying to make using with more flexible.

A possible encoding

By adding a generic parameter to the setters, we can fix both the associated type of TryInto and the output value to be the same. By following this encoding, we get rustdoc output that looks like .

What annotation to use?

I don't really care, whatever works for you.

Is this worth it?

I tried your workaround, and it works fine - fine enough that I think I'll drop my use of with in this case. Should have thought about it in the beginning.

Is it worth it to add this in general? I think as shown above, the rustdoc doesn't have to be that bad - I think it's quite clear with the generic parameter. The argument is not that dissimilar to whether into should be supported, as without into, the workaround would be exactly the same.

The burden is mostly yours as maintainer, so I guess that's a decision for you 😄

[MareStare]

I tried your workaround, and it works fine - fine enough that I think I'll drop my use of with in this case. Should have thought about it in the beginning.

That's great then. I guess I'd rather not introduce special try_into support just yet. I'd like to wait and see if someone comes with a compelling use case for it, where usual Into and early ? in the argument to the setter doesn't suffice. I believe encoding fallibility into the arguments can likely be avoided in general, and otherwise promoting a #[builder(try_into)] may nudge people into writing APIs, that could use infallible signature otherwise, and thus probably be better and simpler.

Also, try_into with its generic error parameter just makes things a bit more complicated for what I think very little benefit, given that you could define custom setter methods with the TryInto signture if really desired.