google/auto

Renaming nested builders

perceptron8 opened this issue · 2 comments

Hi! I'm looking for a way to declare / use nested property builders with names other than default, but can't make it work. I've read https://github.com/google/auto/blob/main/value/userguide/builders-howto.md#accumulate, yet found nothing related.

@AutoValue
public abstract class Animal {
	public abstract String name();
	public abstract int numberOfLegs();
	public abstract ImmutableSet<String> countries();
	
	public static Builder builder() {
		return new AutoValue_Animal.Builder();
	}
	
	@AutoValue.Builder
	public abstract static class Builder {
		public abstract Builder setName(String value);
		public abstract Builder setNumberOfLegs(int value);
		public abstract ImmutableSet.Builder<String> countriesAccumulator(); // <- this doesn't work
		public abstract Animal build();
	}
}

Possibility to use anything other than fooBuilder() would be great. This probably means that logic would have to be based entirely on method's return type. This seems to be the way it works in the case of public static Builder builder() which can be renamed freely (https://github.com/google/auto/blob/main/value/userguide/builders-howto.md#-use-different-names-besides-builderbuilderbuild).

Ideally, I would like to be able to declare public abstract ImmutableSet.Builder<String> countries() with no suffix at all. Currently it leads to name collision with countries getter (as [AutoValueBuilderReturnType] nicely explains), but I hope that it is still possible to extend the implementation in a backward compatible way. Distinction between getters and builders seems to be rather unambiguous.

I don't think using the method's return type alone would work in general. What if you had more than one property of type ImmutableSet<String>? We could imagine accepting any suffix on the end of the property name, including the empty suffix, so all these would work:

abstract ImmutableSet.Builder<String> countriesBuilder(); // the thing that works today
abstract ImmutableSet.Builder<String> countriesAccumulator();
abstract ImmutableSet.Builder<String> countries();

That's potentially ambiguous, though, if one property has a name that is a prefix of another.

On the other hand, there's already a fairly simple workaround:

  public abstract static class Builder {
    public abstract Builder setName(String value);
    public abstract Builder setNumberOfLegs(int value);
    public ImmutableSet.Builder<String> countries() {
      return countriesBuilder();
    }
    abstract ImmutableSet.Builder<String> countriesBuilder();  // not public
    public abstract Animal build();
  }

I don't think using the method's return type alone would work in general.

You are right! And thank you for a workaround. However I see one more problem related to name conflicts that didn't came to my mind before. This already doesn't compile:

@AutoValue
public abstract class Animal {
	public abstract ImmutableSet<String> countries();
	public abstract ImmutableSet<String> countriesBuilder();
	
	public static Builder builder() {
		return new AutoValue_Animal.Builder();
	}
	
	@AutoValue.Builder
	public abstract static class Builder {
		public abstract ImmutableSet.Builder<String> countriesBuilder(); // error: [AutoValueBuilderReturnType] Method matches a property of sandbox.Animal...
		public abstract ImmutableSet.Builder<String> countriesBuilderBuilder();
		public abstract Animal build();
	}
}

Very unlikely to happen, sure. But possible.

As for workaround (thanks again!), it obviously forces builder to be an abstract class. With @AutoBuilder and no need for custom logic, it could be an interface. I can't imagine mocking a builder with java.lang.reflect.Proxy, but who knows, what crazy use cases live in the wild...