[DRAFT] Builder paradigm overhaul

[mkrasnitski]

This is a draft PR as this work will not be ready all at once, and I'm open to comments and bikeshedding as we attempt to get this right. The main goal is to start addressing point number 3 listed in #1905: to rework the builder API to reduce monomorphization bloat (resulting from generic closure parameters), as well to allow for re-using preconstructed builders, and also to enable another later pass of changes to enforce (to a greater extent) API compliance at compile time using the builder API (e.g. always-required, conditionally-required, and mutually exclusive fields).

Some notable changes I've done:

1. Methods on model types that used to take builder construction closures now instead return an empty builder object.
2. Builders have an additional execute method that consumes them and actually perform the request. Essentially the functionality from the model types has been moved into the builder itself.
3. Because execute takes ownership of self, then in order to call Builder::new().method1().method2().execute(), the rest of the chain must take ownership as well. Therefore, builders will change to take and return Self instead of &mut Self.

So far I've tried reworking CreateSticker and CreateThread, with some success. I do have some concerns though:
1. A good deal of builders are called from multiple places. For example, GuildId/Guild/PartialGuild, or ChannelId/GuildChannel. Since the implementations on guild usually do a cache lookup, to preserve all functionality I had to make the builder store a reference to a generic GuildIdWrapper, which I will likely also have to do for ChannelId. Is this acceptable? I'm not yet satisfied with it as the implementation is exactly duplicated for Guild and PartialGuild, but that's something I may solve later, once I'm sure that this fact holds for all relevant builders.
2. Do we want to factor out execute and execute_with_cache into their own traits, e.g. Builder<T> and BuilderWithCache<T>? We could probably auto-import these so that the user wouldn't have to import them themselves.
3. The reason the execute method takes ownership is because json::hashmap_to_json_map() consumes the builder's hashmap. Potentially this is addressed by #1962, so I'll wait until that PR is complete before getting too deep into the weeds on this one.

Feedback very much wanted; I'm not great at writing docs so a review of that part is also greatly appreciated.

[mkrasnitski]

I've rebased on next now that #1962 has been merged, and wanted to mention that point 1 above is no longer relevant, as we can avoid passing around a reference to whatever GuildId/Guild/PartialGuild object called into the builder: simply give the builder a concrete GuildId, then for permissions cache lookups, do an additional lookup to get the Guild from its id before doing the permissions check on it. This lets us also perform cache lookups even if the method is called on a GuildId, so therefore I've elected to unify the execute and execute_with_cache functions, so cache lookups happen every time. To be honest, I think this is preferable.

[GnomedDev]

The builders should be made must_use aswell, instead of each method on them, imo

[mkrasnitski]

With the PR making good progress up to this point, I thought I'd write down some comments and concerns about the current state of the code.

1. The current obvious pain point that's bothering me is that when I reworked CreateMessage, it caused UserId::direct_message to take a hit in ergonomics. Since it first has to potentially create a direct message channel, before then giving the user a builder which they execute, it now not only requires a double .await, but also requires duplicating error handling code. One ugly solution is to separate CreateMessage's fields into CreateMessageData, then create a separate CreateDmMessage builder that wraps them. But, this would require moving the field method implementations into CreateMessageData, then adding wrappers to both CreateMessage and CreateDmMessage, resulting in a lot of code duplication.
2. Speaking of, a looming dilemma are the Edit builders. The implementations on model types (e.g. GuildChannel) assign the result of the http method to *self. Therefore, I'm probably going to have to give the builder a generic mutable reference to the caller struct to assign to. This does make a case for factoring the fields themselves into a BuilderFields to separate it out from metadata. Overall, having to carry execution metadata along with the builder while it's being constructed sort of sucks, but there doesn't seem to be a better alternative.
3. Maybe once every builder has been reworked here and the dust has settled, we could auto-generate these aforementioned wrapper impls using macros (my guess is it would require proc-macros), but that's a problem for later. For now, full steam ahead, and we'll see where this ends up.

[mkrasnitski]

One more issue to note about the Edit builders. As described above, they have to hold on to a mutable reference to the caller in order to mutate it after executing themselves. However, this means that they can no longer implement Clone, since &mut T cannot be Clone. Not sure yet what to do about this, as ideally it would be great if all builders could be cloned for easy re-use.

[mkrasnitski]

I've thought of an approach that solves these problems. Unfortunately it requires redoing a large part of what has already been pushed here, so I'm closing this and trying again in a new PR.