Immutables: immutable: refine subtyping for immutable objects

[elucash]

_Immuables_ supports inheritance of immutable objects of some sort. While we want to preserve this ability to some degree, I also think that we should constrain it to be safer and also resolve some current and potential issues.

I think it is "well known" that for immutable objects to form sub-typing and composition trees, following statements should be true (sort of, I don't want to refine it to the death)

1. Immutable object should not inherit another immutable object.
2. Immutable object could subtype stateless abstract type.
3. Stateless abstract supertypes may in turn subtype some other types.

Therefore we could have sub-typing tree where all node are abstract and stateless types, and all leaves are non-extendable immutable types.

As immutable value objects are defined by abstract value types, even now inheritance model is not too much flawed. (Let's forget for a while about possible parasitic mutable state which might be introduced into intermediate abstract classes). Most value comes from supertypes that defines sets of abstract attributes, which in turn are implemented by leaf immutable types. Multiple interfaces inheritance gives even more power to define sets of abstract attributes.
Also we want to prevent ambiguity and potential problems, by giving enough power, but cutting down edge cases and unwanted interactions.

I would propose to do the following:

• 1) Prohibit for abstract value type (@Value.Immutable) to extend or implement other annotated immutable value type directly or indirectly, insisting on using common abstract types which are not abstract value types. This will also significantly simplify design of visitors for ADT-like classes: logic should not depend on implementation types, abstract value type will be sufficient to figure out each case.
• 2) Prohibit inheriting attributes with @Value.Parameter annotation. Insisting to redeclare any constructor parameter attributes in abstract value type. This will make constructor stable, without hideous breakages by supertypes. In the same time allow any other annotated attribute to be inherited (including non-abstract in terms of java, but annotated attributes with special initializers like Default or Derived), there no such problem with derived, lazy attributes or using builders.

Any thoughts welcome

[elucash]

Started to poke around to implement this checks. And while I have no doubts about 2),
but 1) started to feel awkward.

• We already have mutable state tracking in superclasses (#78)
• Additional work to scan all supertypes (performance?)
• No guarantees, we still could create immutable implementation using @Value.Import.

One of the options would be to issue error or warning when generating Visitors and CaseChains for ADT-like types (#47)

Still thinking and experimenting.

[elucash]

Released in 2.0

[alexanderkjeldaas]

I can't find any documentation or rationale for 1) - immutable types should not inherit another immutable type.

This creates an enormous amount of boiler plate - separating every superclass into one class that includes all fields, and an empty subclass that only includes the @Immutable annotation. They even need to be in different files.

I think this needs its rationale to be documented somewhere.

[Sarev0k]

I would also like the ability to have my generated immutable code inherit from each other.

This would allow me to define interfaces like so:

@Value.Modifiable
interface Animal {
    int getFeet();
}

@Value.Modifiable
interface Cat extends Animal {
}

@Value.Modifiable
interface Dog extends Animal {
}

And get the following generated classes:

class ModifiableAnimal implements Animal {
    public void setFeet(int numFeet) {
    }
}

class ModifiableCat extends ModifiableAnimal implements Cat {
}

class ModifiableDog extends ModifiableAnimal implements Dog {
}

With this, if I'm given just the Animal interface, I will know it is always safe to cast it as a ModifiableAnimal if I want to change the number of feet the animal has.

void updateFeet(final Animal animal, final int numFeet) {
    ((ModifiableAnimal) animal).setFeet(numFeet);
}

The current implementation requires that I cast to the to the specific modifiable implementation, which requires significantly more code, and is much less elegant.

void updateFeet(final Animal animal, final int numFeet) {
    if (animal instanceof Cat) {
        ((ModifiableCat)animal).setFeet(numFeet);
    } else if (animal instanceof Dog) {
        ((ModifiableDog)animal).setFeet(numFeet);
    }
}

Could you please reconsider?

[elucash]

Thank you for the suggestion. I think any additional flexibility can be used to some benefit, here I'm trying to understand if it's actually worth it.
In my view, you can already do it

interface Animal {
    Animal setFeet(int numFeet);
    int getFeet();
}

@Value.Modifiable
interface Cat extends Animal {
}

@Value.Modifiable
interface Dog extends Animal {
}

// then you can call setFeet on any Animal
Animal d = ModifiableDog.create();
d.setFeet(2);

The only requirement is that signature of Animal setFeet(int numFeet); would be compatible with any implementation of setter derived from int getFeet(); which generated in corresponding modifiable classes.

[Sarev0k]

I don't want to pollute the interface of Animal with setters though.

When passing an Animal externally, I just want to only provide access to the getters. When interacting with an animal internally, I want to be able to perform modifications via the setters.

For external users of Animal I would also prevent access to the Modifiable implementations by scoping them package private.

[elucash]

public interface Animal {
    int getFeet();
}
interface ModifiableAnimal extends Animal { // package-private
    Animal setFeet(int numFeet);
}
@Value.Modifiable
public interface Cat extends ModifiableAnimal {}
@Value.Modifiable
public interface Dog extends ModifiableAnimal {}

[eamelink]

I'm running into the same thing that @alexanderkjeldaas ran into! An unsatisfied curiosity of why the inheritance of immutable values is not allowed :P

The first message alludes to it being _well known_. Would that be the potential to create an asymmetric equals? If B extends A, then it could happen that b.equals(a) is false while a.equals(b) is true. Obviously, that would make the world come crushing down.

But I don't think that's the case for Immutables? Maybe it's dependent on configuration, but by default the immutable classes that are generated don't have this issue, because the immutable class generated for A isn't a supertype of the one for B.

Or, in code:

@Value.Immutable
public interface Foo {
    String a();
}

@Value.Immutable
public interface Bar extends Foo {
    String b();
}

@Test
public void equalsMustBeSymmetric() {
    Foo foo = ImmutableFoo.builder().a("a").build();
    Bar bar = ImmutableBar.builder().a("a").b("b").build();

    Assert.assertFalse(foo.equals(bar)); // passes
    Assert.assertFalse(bar.equals(foo)); // passes
}

What's the incoherence that the error message mentions?

The only other thing that's mentioned here is Visitors, are they the only reason?

[elucash]

Visitor case and equality symmetry go in the same basket of problems which impact depends if one uses abstract value type synonymously with immutable generated type. Indeed, if you use abstract value type and generated immutable type separately, you already have that non-controversial "abstract intermediate nodes, immutable leafs" inheritance structure under which everything works fine. But if you use [possibly] hidden immutable implementations using abstract value types as first-class values, then there're potential of problems for which I don't want to open pandora box of encouraging inheritance.

On the practical side, this is longer an error, it's a warning and you can suppress it with @SuppressWarnings("immutables:subtype"). What else can be done, would adding parallel warning suppression using styles which easier to apply project-wide?

Here's an example of the recommended pattern for inheritance. fully abstract intermediate nodes, flat hierarchy (which is basically quite alike sum types/sealed case classes etc):

interface A {
   interface S {} // shared trait
   @Immutable interface B extends A {}
   @Immutable interface C extends A, S {}
   @Immutable interface D extends A, S {}
}