elastio / bon

Next-gen compile-time-checked builder generator, named function's arguments, and more!
https://bon-rs.com
Apache License 2.0
1.15k stars 17 forks source link

bon::private::Set is qualified in compiler errors while Unset is not #129

Closed taladar closed 1 month ago

taladar commented 1 month ago

I am not quite sure what causes that but I a got an error that said (in the full type name file written by the compiler)

OrderLogBuilder<(bon::private::Set<i32>, bon::private::Set<i32>, bon::private::Set<std::string::String>, Unset<bon::private::Required>, Unset<bon::private::Optional>, Unset<bon::private::Optional>, Unset<bon::private::Required>, Unset<bon::private::Required>, bon::private::Set<std::string::String>, bon::private::Set<std::string::String>)>

I assume Unset is missing a qualifier in some part of the generated code where Set has one.

This came up when I tried to check the type of a partially constructed builder so I can pass it into a function, a use case which I assume is not supported yet judging by the bon::private module path? It does seem useful now that those can be cloned.

Veetaha commented 1 month ago

Hi! Thank you for creating an issue.

Before we dive into the implementation details of bon and bon::private I'd like to ask you to elaborate on your specific use case. I'd like to understand the problem that you are trying to solve by using bon::private.

Additionally, I suspect that the feature that is currently WIP in PR https://github.com/elastio/bon/pull/125 may help with your use case as it allows you to pass additional parameters to the start_fn or finish_fn of the builder mimicking the use case of a function that accepts some positional parameters and returns a builder with partially filled fields from the positional arguments. Could you review the PR description and say if that would solve your problem?

Implementation details

Indeed bon::private is an internal implementation detail, so the type of the builder's state should be treated like more of an anonymous type. The bon::private module has been evolving quite a lot, and there may be significant changes to its interface between patch releases. I'll make sure to call them out in changelogs though if you really need to depend on bon::private, but in this case you should pin a specific version of bon e.g. bon = "=2.2.1" to make sure your code doesn't break accidentally in a new bon patch release due to the usage of bon::private symbols.

Veetaha commented 1 month ago

I think I slightly misunderstood the issue title. So your suspicion is that Unset is not qualified with ::bon::private::. That is indeed rather strange. I doubt there is a bug somewhere where we don't qualify it, otherwise nothing would work. I bet it's just rustc trying to de-qualify type names in error messages that are already unique. I remember some blog post in Rust blog about such a feature a long time ago, maybe it was already implemented, I just can't find it, I found this issue instead: https://github.com/rust-lang/rust/issues/50310

taladar commented 1 month ago

That could be the case that Set is qualified because there is some other Set type in one of the other dependencies (even though I don't think one is imported) while Unset is unique. In that case I guess there is no way to fix that.

My use-case right now for the type of the partially filled builder is basically that I am writing a trait to convert something in type A recursively into something in one or more values of type B. A contains a few fields that will not be available on every recursion level, just the top level. I could pass those into each call in the recursive conversion trait function, pass them on recursively and then construct a builder locally where I want to construct a value of type B but your blog post about the cloneable builders made me think I could instead just pass in a builder for B where those fields only available at the top level are already set.

Veetaha commented 1 month ago

In that case I guess there is no way to fix that.

I don't think it's a problem that Unset is not qualified in error messages. It's not meant to be referenced in code directly anyway.

My use-case right now

Yeah, I don't think using a partial builder would make it simpler because even if you do write the type signature of the partial builder, it'll be a big one, which needs to enumerate states of all members in the builder, at which point I think it gives you no benefit over just passing all "top-level" fields separately.

The use case with the partial builder works only in a limited scope (when you don't cross the fn-item boundary). For example, it's fine to use it in the function body to do conditional building, clone it to reuse values, but all of that should happen only within a local scope.

Veetaha commented 1 month ago

I think what you are missing (and probably Rust language as a whole) is an ability to generate a projection of your type (with only part of the fields available from the original struct). I have such macro in a private repo at work, which I plan to integrate into bon some day in the future under derive(bon::Projection), but that's a different story.

taladar commented 1 month ago

Yes, I think what would be most useful in my use-case would be that plus a way to convert that projected type into a partially filled builder for the full type.

Veetaha commented 1 month ago

I created https://github.com/elastio/bon/issues/130 for the projections feature. I'll close this issue as I think it's resolved.