Adds nested builder functions

[aerb]

I've found myself repeating code working with nested messages. Something along the lines of:

(personBuilder.address?.newBuilder() ?: Address.Builder()).apply {
   city = "London"
}.build()

I think it'd be cool to inline that into wire generated code.

Example:

message Person {
  optional string name = 1;
  optional Address address = 2;
  message Address {
    optional string street = 1;
    optional string city = 2;
  }
}

Will generate a new function for the nested message field "address":

public fun address(builder: Address.Builder.() -> Unit): Builder {
  this.address = (address?.newBuilder() ?: Address.Builder()).apply(builder).build()
  return this
}

Which can be used like so:

val p = Person.Builder()

// Calls with uninitialized field constructs a new empty builder. 
p.address { city = "London" }

// Calls with initialized field copies existing values into new builder.
p.address { street = "Downing" }

// output Person{address=Address{street=Downing, city=London}}
println(p.build()) 

[JakeWharton]

This feels a bit underspecified and asymmetric, as if you're scratching one specific itch but not solving all itches. To be concrete, I would expect just the builder of a message to get a DSL long before this specific use case.

[aerb]

It is underspecified but figured I'd take a crack at what's been bothering me.

The itch is:
Updating nested fields (particularly if you want a copy behaviour) is awkward and repetitive.

Expanding on the example above: let's say Address has another field optional Contact contact. Contact has a field phone_number.

Person is being stored in a database.

If I want to update phone_number on a stored Person the resulting code will be something like:

person = (person?.newBuilder() ?: Person.Builder()).apply {
  address = (address?.newBuilder() ?: Address.Builder()).apply {
    phone_number = "555-555-5555"
  }
}

Getting something closer to below would be a win IMO:

person {
  address {
    phone_number = "555-555-5555"
  }
}

Hopefully that highlights the use case better.

[JakeWharton]

Yeah I get that for sure.

With that example API I'm a bit worried that there exists a top-level address function which creates and returns an Address, and a member address function on Person.Builder which creates or updates an Address.Builder. You could call address { } inside a car { } and it would compile, with the resulting Address being discarded. If we already had KEEP-412 it would help mitigate that, but it's a pretty big foot-gun.

I think I would prefer to separate top-level creation functions from the create-or-update builder members somehow by name.

For at least the top-level ones, I don't think prefixing with "build" is terrible (like buildList). This signifies that buildPerson is in fact creating a new Person via its Person.Builder.

The members can then stay named as the fields rather than the types. So let's say Person has an address field whose type is MailingAddress, your example becomes:

val p = buildPerson {
  address {
    // ...
  }
}

or

val p = buildPerson {
  address = buildMailingAddress {
    // ...
  }
}

[JakeWharton]

And then I'm tempted to round that out with like Xyz.builder(builder: Xyz.Builder.() -> Unit): Xyz. And maybe even make the receiver nullable...

[aerb]

And then I'm tempted to round that out with like Xyz.builder(builder: Xyz.Builder.() -> Unit): Xyz. And maybe even make the receiver nullable...

The nullable receiver approach could be a good middle ground to reduce some of the create-or-update boilerplate, while having a less hazardous API

person = person.builder {
  address = address.builder {
    phone_number = "555-555-5555"
  }
}

... don't know if builder should be renamed if returning Xyz or if builder should return Xyz.Builder