Support for `fn`s in collections?

[Keshyu]

Hey! Really like this crate, especially the lack of unwraps (rest my soul (◡ ‿ ◡ .))

But while making a little toy game, I found that there's really no way to pass a collection of functions into the builder.

The following code:

use bon::Builder;
use std::collections::HashMap;

fn main() {
   let player_1 = Player::builder().skills(bon::map! {
      "slash": |field| { /* ... */ },
      "punch": |field| { /* ... */ },
   });
}

#[derive(Builder)]
struct Player {
   skills: HashMap<String, fn(Field)>,
}

struct Field(/* ... */);

Gives this error:

error[E0277]: the trait bound `for<'a> fn(Field<'a>): From<{closure@src/main.rs:7:16: 7:23}>` is not satisfied

---

I found that both fn pointers & dynamic dispatch break because of impl Into, so bon::map and the like don't work.
Maybe you can use as or other kind of cast but I imagine that'd be very awkward.

[Veetaha]

Hi, thank you for creating the issue!

Unfortunately, Rust's type inference is rather awkward in this case, and I think it's more of an issue with the type inference itself than with bon, because bon just generates a setter method that accepts a HashMap<String, fn(Field)> and that's it. bon::map! {} doesn't fit this use case, unfortunatelly because there should be no Into conversion for values and I don't think this should change in bon::map.

I don't think there is a good solution to this problem. The easiest way to work around this error is to construct the hashmap manually from tuples like this:

use bon::Builder;
use std::collections::HashMap;

fn main() {
    let skills: &[(_, fn(_))] = &[
        ("slash", |field| { /* ... */ }),
        ("punch", |field| { /* ... */ }),
    ];

    let skills = skills
        .iter()
        .copied()
        .map(|(name, skill)| (name.to_owned(), skill))
        .collect();

    let player_1 = Player::builder().skills(skills);
}

#[derive(Builder)]
struct Player {
    skills: HashMap<String, fn(Field)>,
}

struct Field(/* ... */);

I'd be glad to hear any suggestions of how this could be implemented better though. Right now the snippet above is the only thing that comes to my mind.

[Veetaha]

If you find youself creating such maps of functions quite often, then you can declare a helper function like this:

use bon::Builder;
use std::collections::HashMap;

fn fn_map<F, const N: usize>(items: [(&'static str, F); N]) -> HashMap<String, F> {
    items
        .into_iter()
        .map(|(name, func)| (name.to_owned(), func))
        .collect()
}

fn main() {
    let player_1 = Player::builder().skills(fn_map([
        ("slash", |field| { /* ... */ }),
        ("punch", |field| { /* ... */ }),
    ]));
}

#[derive(Builder)]
struct Player {
    skills: HashMap<String, fn(Field)>,
}

struct Field(/* ... */);

[Keshyu]

Yeah, this wouldn't be an issue if all closures implemented Into for their corresponding fn pointers, but I imagine that's a pretty niche problem.

As a workaround, I've currently just manually implemented the builder pattern for the HashMap like other crates do (it was already in an opaque type). It's ok apart from the 2 levels of indentation it adds (3 if you count the lambda body). ;-;

Also could do this if I'm feeling cursed:

.skills({
   let mut skills = HashMap::<_, fn(Field)>::new();
   skills.insert("slash".into(), |field| {});
   skills.insert("punch".into(), |field| {});
   skills
});

Anyway, thanks for the feedback :^)

[Veetaha]

I've currently just manually implemented the builder pattern for the HashMap like other crates do

I don't think I understand what "other crates do". Do you mean a method like player.insert_skill(name, func)?

Btw. it also looks fine to me if the builder could just create a Player with an empty HashMap internally, and a method on the Player itself could be used to add entries to the map (after build()).

[Keshyu]

I don't think I understand what "other crates do".

Like alternatives to bon I mean, where they generate method x on the builder that would add an item to the xs collection.

Btw. it also looks fine to me if the builder could just create a Player with an empty HashMap internally, and a method on the Player itself could be used to add entries to the map (after build()).

That would make it implicit that the skills collection needs to be (or even can be) filled. It also breaks modularity as in skills can only be filled as a part of the Player struct but not on its own. I think manually implementing a builder on the collection is good enough. It's a toy project after all.

[Veetaha]

method x on the builder that would add an item to the xs collection

I see, that's something to look forward to in the future. As the first step in this direction I'm planning to add an ability to add custom methods to the builder, and then custom fields, which would allow defining such a method that pushes into an internal collection manually.

Then this pattern could be granted syntax sugar with an attribute. I don't have a design for that attribute yet though.

[Keshyu]

Yeah, I was wondering about custom methods.
That would have to be macro, I think. 🤔
Otherwise, users would have to use all the "private" types in their method signatures.

The attribute design could be something like #[builder(collection)] or #[builder(accumulator)]. Or what do you mean by design? The names, right?

[Veetaha]

The design includes the name of the attribute and of the generated methods, their parametrization, behavior, API compatibility story, all the things.

For example the macro needs to know the type of the element in the collection to generate a method that a accepts a value of that type to push into it.

I think that'll require hardcoding the knowledge of the method and type signatures of well known collections such as vec, hashmap, btreemap, hashset, betreeset.

[Veetaha]

As for custom builder methods. The design and impl for that is in master pending documentation and release. An example of that is in #145.

The type signature of the builder will become stable

[Veetaha]

I'm closing this issue in favour of #190 and #189