Make it possible to add `#[builder]` to existing code without changing the name of the function under the macro

[drmason13]

First of all, thank you for this fantastic crate, I love the idea of functions and structs sharing a builder derive, and the documentation is brilliant, the big effort that went into it all is clear to see.

I read through
https://elastio.github.io/bon/docs/guide/compatibility#adding-builder-to-existing-code

and followed the link to

https://elastio.github.io/bon/docs/reference/builder#expose-positional-fn

but I couldn't see how to make the preserved fn backwards compatible after adding the builder.

use bon::builder;

#[builder(expose_positional_fn = example_positional)] 
fn example(x: u32, y: u32) {}

// Positional function is now available under the given name
example_positional(1, 2);                                     

// Builder syntax is also available (unchanged)
example()
    .x(1)
    .y(2)
    .call();

Old code would do example(1, 2) and expect the function result, not a builder type. It is a small breaking change, as code could be changed to use example_positional(1, 2) instead easily with find/replace.

Would it be possible to change the name of the builder and keep the original function unchanged? E.g. for this to work:

use bon::builder;

#[builder(builder_fn = example_builder, expose_positional_fn)] 
fn example(x: u32, y: u32) {}

// Positional function is now available under the *original* name
example(1, 2);                                     

// Builder syntax is also available with the *explicit* name
example_builder()
    .x(1)
    .y(2)
    .call();

I'm not sure I want this as a feature request, but I think it deserves calling out specifically that expose_positional_fn is useful for "easing the breakage" but not able to provide backwards compatibility - if indeed that is the case.

I would add that statement to the compatibility section.

[Veetaha]

Would it be possible to change the name of the builder and keep the original function unchanged? E.g. for this to work:

The main idea is that the function's name under the #[builder] macro becomes the name of the function that returns the builder. In this case the code that you'd like to have should be this:

use bon::builder;

#[builder(expose_positional_fn = example)] 
fn example_builder(x: u32, y: u32) {}

example(1, 2);                                     

example_builder()
    .x(1)
    .y(2)
    .call();

The positional function becomes hidden by default and the current name of the function is the name of the function that returns a builder. I can add this as an example to that section to avoid confusion, thanks!

[Veetaha]

On the other hand.. It's understandable that you'd expect expose_positional_fn to flip this behavior where the positional function name is preserved and you need to specify a function for the builder.

I can also imagine it as a feature where providing a builder_fn (or in bon's terminology it's start_fn) flips the naming behavior of function's name such that it stays as positional function's name.

[Veetaha]

I updated the docs with the example above. I'll keep this issue open as a feature request to implement the syntax in your proposal where the positional function's name stays the same and the start_fn (that returns the builder) is explicitly specified in the attribute.

[drmason13]

Oh, I see, thank you for the example, that makes it clear to me how it is intended to be used.

It seems obvious in retrospect, but thank you for clarifying! I guess the new syntax will be more intuitive, but we can make do without it until (if) there's a new major version.

[Veetaha]

I don't think a major version will be required for this feature. The default behavior should still be such that the function's name is used for the builder function. However when setting an explicit start_fn the flipped behavior is used. This extends the existing API and doesn't break it

[drmason13]

use bon::builder;

// The previous name of the positional function needs to be specified
// as the value for `expose_positional_fn`.
#[builder(start_fn = example_builder, expose_positional_fn = example)] 
fn example(x: u32, y: u32) {}      

// The positional function is available
example(1, 2);

// The builder syntax is available with the given name
example_builder()
    .x(1)
    .y(2)
    .call();

Does the above compile today?

That's the most awkward example I can think of that might be broken by the proposed change.

I think that should still compile, but the = example becomes unnecessary with the change:

// The previous name of the positional function needs *is inferred*
// as the value for `expose_positional_fn`.
#[builder(start_fn = example_builder, expose_positional_fn)]
fn example(x: u32, y: u32) {}

[Veetaha]

#[builder(start_fn = example_builder, expose_positional_fn = example)] 

Such syntax should be a compile error, because now there are 3 names involved, when only 2 are needed.

Does the above compile today?

It doesn't because start_fn isn't available on functions today. It's exposed only for #[builder] on structs. The reference for this attribute specifies that it "applies to structs" only.

[Veetaha]

Hey @drmason13 I've just found a probably unrelated bug, but I want to make sure the fix won't affect your code.

The bug is that when you place #[builder] on a method called new(), then that method is exposed under the name new() by default. Also, the type name of the builder is slightly different than expected {Type}NewBuilder instead of {Type}Builder, which breaks the promise of compatibility with #[builder] placed on a struct.

Instead the positional method new() is supposed to be renamed to __orig_new() and #[doc(hidden)] should be added to it by default.

I prepared a fix for that #19, that will be released as a new patch

[drmason13]

I haven't used this crate properly yet so there's no risk of breakage for me, but I appreciate the concern :)