Generate static factory method for generated builders in Java

[rmschots]

When mapping to the generated Java classes for the messages with MapStruct, it is complaining about ambiguous constructors.
It would be able to use the builder if there is a static factory method for it.
I know that this is not a issue with Wire, but rather with mapstruct..
..but it's not really a bad practice to have this design pattern in the generated code, so I'd request it to be added (maybe as an option?) to generate something like this for each message:

public static final Builder builder() {
  return new Builder();
}

This issue prevents me from using Wire in my project.

[oldergod]

Is there something we can improve on MapStruct? I don't really mind adding the Builder#builder method (or maybe rename it newBuilder() in any case, and it'd be nice to see in the PR that you can actually achieve what this PR is trying to fix.

[rmschots]

@oldergod
Mapstruct constructor usage is documented here: https://mapstruct.org/documentation/stable/reference/html/#mapping-with-constructors
MapStruct also supports mapping of immutable types via builders: https://mapstruct.org/documentation/stable/reference/html/#mapping-with-builders

So, currently mapstruct does following steps when trying to generate a mapping:

1. no builder found
2. There's no constructor with a custom '@default' annotation
3. There's no parameterless constructor
4. There's multiple eligible constructors (one with, and one without ByteString unknownFields)

So, first it tries to find a public static builder creation method, which would be the generated method I'm introducing in my PR.
As public Builder newBuilder() is already generated by Wire (which creates a new builder using the message data), I wouldn't be able to use the public static final Builder newBuilder() signature, as newBuilder() is already defined in a non-static way. Hence, I'm using builder() in my PR. Feel free to suggest alternative solutions.

I tested the solution to work correctly in another project already by manually editing the generated messages.
But it's unlikely a good idea to write a test for mapstruct integration this project, as it's not really of importance to Wire.

[oldergod]

Got it, thank you. I didn't know what MapStruct is. I don't think it's such a good idea for Wire to do that, specially since the Builder has a public constructor. Could you reach out to MapStruct and ask for them a hint? Maybe there's a way for you to point out which constructor to use or they could add a new place where they'd be searching for builders as an inner class called Builder with a unique constructor?

[rmschots]

@oldergod As indicated by the documentation, there is indeed a way to point out which constructor to use, by annotating the constructor with @default

If there are multiple eligible constructors then there will be a compilation error due to ambiguous constructors. In order to break the ambiguity an annotation named @default (from any package, see Non-shipped annotations) can used.

I felt like that would be a less graceful solution than just adding the static builder factory method, as it's a commonly used design pattern.
I understand your reluctancy to pass a change for just one external library though, so feel free to close this ticket if you believe there not much to gain from MapStruct compatibility.
From my point of view of course, it would be a big gain with no downsides.

[oldergod]

@rmschots I don't like having multiple APIs doing the same thing. We have Message.Builder() and I think that's sufficient for Wire. Thank you.