Builder.build to return interface instead of abstract class

[andrey-vasilyev]

Say I have an interface which I don't want to (or can't because it is not under my control) pollute with @Value.Immutable annotations.

public interface A {
   int getX();

   interface Builder {
       Builder x(int x);
       A build(); 
   }
}

So I create an abstract class and implement the interface:

abstract class AA implements A {
       public abstract static class Builder implements A.Builder {}
}

Now when I call aBuilder.build() I get abstract AA while I would like to get interface A. In this example AA will be package-private and I'll get warnings like return type is package-private. Is there a way to generate build() method with return type A

[elucash]

One thing you can do is overload/override somewhere to limit builder's visible type to A.Builder. There's also Value.Include annotation to create immutable implementations from other packages/types into current package. In conjunction with Style.overshadowImplementation=true, it can give similar to what you want.

public interface Ainf {
  int a();

  interface Builder {
    Builder a(int a);
    Ainf build();
  }
}

@Value.Style(overshadowImplementation = true)
@Value.Include(Ainf.class)
public class Bing {} // this 'Bing' class doesn't matter, can put Include into package-info.java too over the package decl

[andrey-vasilyev]

Ok, you got me ;) Say I need an abstract class, because I want to add validations, default values or jackson annotations. Is there away to do similar thing?

[elucash]

I don't think it's possible to do in automatic manner without creating wasteful features+configuration for those. Can as well create package-private or protected builder and expose only your public interface.

public interface A {
   int getX();
   interface Builder {
       Builder x(int x);
       A build(); 
   }
}
// Styles have visibility and builderVisibility toggles
abstract class AA implements A {
  abstract static class Builder implements A.Builder {}
  public static A.Builder builder() { return  ImmutableAA.builder(); }
}

[andrey-vasilyev]

But ImmutableAA.builder().build will return ImmutableAA - the very thing I am trying to avoid. I hide implementation (ImmutableAA) and the builder is the only exposed class. So it seems reasonable to want all return values from the builder to be interfaces (not abstract or actual implementations) - just like in your first answer.

I actually don't really understand what is the value of abstract or concrete implementations if they are package-private (or worse). Nobody is able to use them anyway. Interface seem to be the only thing that make sense really. May be I am too narrow sighted on this one, correct me if I am wrong.

[elucash]

In that example with A and AA, if you do return ImmutableAA.builder(); from static A.Builder builder(), then caller will only see A.Builder, not ImmutableAA.Builder, so the build() method will return A, not AA or ImmutableAA if the returned type is concerned, at runtime it would be an instance of ImmutableAA.

Well, I guess, I assumed (given examples with toBuilder etc) you familiar with how AutoValue works etc. Here's example of implementation hiding https://immutables.github.io/immutable.html#hide-implementation-class . the concept is that publicly visible API is what you manually curate and users of your code don't know or don't touch generated code directly. Aside of being just an alternative to using Immutable* implementations, it has great benefit of having no (or less) of cascade of compilation errors across the codebase if there are problem with code generation: if using generated implementations, then you change smth, error in annotation processing occurs, no java files generated, javac gives dozens of symbol not found errors, real error is buried somewhere there... pita and is unsolvable problem with annotation processing currently (requires some redesigns inside the compiler I guess). (ok, it's not an issue if generated code is already compiled as part of another module, so small modules/separate datatypes modules mitigate the problem to some degree)

If you ask me, there are 4 main styles of using Immutables:

1. The default-legacy style using generated ImmutableValue impl, subject to List/ImmutableList API decision problems here and there.
2. Hiding implementation/similar to AutoValue, all uses are of abstract value type and factories, generated implementations are only used inside factories.
3. An Inverse generation is popular at many OSS projects: define AbstractX, generate X, generated X is used everywhere, abstract is technicality and usually package private.
4. The "sandwich" way interface X { ...attrs class Builder extends ImmutableX.Builder {} } ..... new X.Builder().attr(...).build() . It combines 2. with no need to repetitive builder interface definition, using bytecode thinks it's calling virtual methods on X.Builder, not ImmutableX.Builder, so no bytecode dependency on generated code, some small level of "encapsulation" if you will while still using a bunch of generated stuff. By the way there are also interface X extends WithX, where WithX generated on demand too, so basically by using abstract X, there will still will be all those with* method on it.

Given all that variety of ways using the toolkit, you choose what you primarily up for, using simple name X or ImmutableX, using abstract value type or generated implementation class, hiding some parts of the codegen, all hide all the generated code. There are tradeoffs of course, some richness of generated implementations vs codebase resilience to cascade errors or having a dependency on generated code or manually written/curated API.

[andrey-vasilyev]

Wow, thanks for the big answer! Couple comments to get things straight.

1. I've never used AutoValue and at the first glance Immutables seem superior to me
2. Client code can't call public static A.Builder builder() because the enclosing generated class ImmutableAA is package-private. So the trick doesn't work. Making ImmutableAA public - is the thing I am trying to avoid, as I want only builder exposed and public interfaces. Minimum possible exposure I believe.

Please don't get me wrong, I am not trying to be annoying but it seems like we are missing the point here. Again the thing I don't understand about Immutables code gen - is why does it return package-private types? What is the value in that for the user of generated objects? I see none really. So it seems logical for the code gen to look for the closest super type with public visibility and return it. If on the other hand a developer of these generated value objects really needs abstract classes (to do something with them) it is ok to just cast - he knows what is under the hood and everything is under his control anyway.

[elucash]

is why does it return package-private types?

You still can use those inside the package, Immutables do not fully know how you will use generated types. There are cases where you want to use full access to generated methods inside the package, while returning only restricted interface to the outside world. I think you expect very specific "smartness" in handling public vs non-public signatures and it just isn't there, mainly because this is an edge case. For example, we have that overshadowImplementation style flag which does very similar to what you imagine but in the different context. You can even generate public builder with private class implementation of the value class (new ABuilder().attr(..).build() -> A). But those are all relatively rare cases, and I would say I even regret we implemented some of those, exactly because those are rarely used (and additional complexity to the implementation).

2. no, public static A.Builder builder() is smth you put in some public class in the package where you generate, AA (if it's public) or any other. It's the body of factory method is calling package private ImmutableAA.builder()

public class VeryPublicClassWithFactoriesAndBuilders {
   public static A.Builder aaBuilder() { return ImmutableAA.builder(); }
}

A a = VeryPublicClassWithFactoriesAndBuilders.aaBuilder().attr(..).build();

[andrey-vasilyev]

You still can use those inside the package

But client code can't access private-package, isn't it why package-privacy was invented in the first place? While a developer of objects controls everything anyway, so he will still be able to do what he wants (with casts or other magic).

You can even generate public builder with private class implementation of the value class

This is exactly what I do. And I don't think it should be considered rare. Actually you can do neat tricks with a builder - like partially constructing a hidden DTO object each time you call builder methods and then on build() just return reference to that DTO - no memory wasted (builder itself does not keep references to object fields) and DTO has only public interface methods! The client code won't notice a thing.

no, public static A.Builder builder() is smth you put in some public class in the package where you generate

This is something I was trying to avoid. Each package would have to have this hand written and maintained - which are the very things immutables is suppose to save me from.

Overall I understand there is no way to do what I want at the moment. I was just wondering what is the rational behind returning package-private classes and if you are willing to consider to change this and return closest public visible super types