Immutables: Do not generate method if there is existing method with same name

[bartosz-bilicki]

This is feature request.

I have class SomeClass with field someField. I would like to provide my own implementation of ImmutableSomeClass.withSomeField method.

Currently this is not possible. final method ImmutableSomeClass.withSomeField is always generated, even if SomeClass already contains withSomeField method with matching signature. Method is generated even SomeClass.withSomeField is final (this causes compilation error for generated ImmutableSomeClass.withSomeMethod).

Could Immutables skip generating with* method, if matching method already exist in base class?
I do not mind using annotation like Value.Style on field level (to customize code generated for this field) but Value.Style is class-level annotation, not field-level anotation.

Currently nasty and subtle unexpected behavior occur if SomeClass has non-final method named withSomeField.

Having istance of SomeClass and going to implementation of withSomeField, IDE takes me to SomeClass.withSomeField. But this method is never used, because my object is in fact instance of ImmutableSomeClass and generated method from ImmutableSomeClass is used.

[Stephan202]

I like this feature request, but might it not make more sense to omit the auto-generated method only if the abstract class defines the method as final? If the abstract class' method is non-final, then by definition overriding is allowed, and the abstract class should be robust against that. If the method is not designed for extension, then it _should_ be declared final.

Well, that's my two cents, anyway.

[bartosz-bilicki]

I do not mind if Immutables would skip generating method only if existing method is final.
I also agree with 'design for extension' argument. But in case of Immutables I think that default behaviour is confusing and unwanted.

If I write my own implementation in base abstract class, then method is hidden by generated method in immutable class- then this is unwanted behaviour for me. I knew what I was doing when I wrote the method and I definitely want to use my own implementation. That's why I have written it.
If I were fine with generated implementation I would never bother writing my own.

Therefore I believe that for generated code, if generated class subclasses class writen by hand 'designed for extension rule' should be inverted.

Note that we have precedence https://immutables.github.io/immutable.html#customize-tostring-hashcode-and-equals
I could provide my own implementation for some methods. That will make Immutables to step away and do not generate code for them. Therefore, I believe it would be good to extend that rule to all methods (maybe with warning, or ability to configure that with Value.style annotations)?

Note that currently updating Immutables to new version may cause my code to malfunction.
Imagine following use case:

1. My abstract class has method _doSomething_.
2. New version of Immutables generates method _doSomething_
3. Out of the blue I am suffering from strange errors. My implementation is ignored, some other, generated, unknown to me (therefore, most likely wrong) implementation is used.
4. I have no clue whats going on :)

I am very open for discussion and alternative arguments. I agree that inverting such basic rule may be surprising for some other people, and may cause unwanted behaviour in some cases other than mine.

[elucash]

I think, in general, it would be cool if Immutables never override already implemented methods! In practice, we'll have to match signatures etc. Thank you for the feature request!

[Stephan202]

Just three more points from the devil's advocate:

1. Keep in mind that the implementation generated by Immutables may not be the abstract class' only concrete subtype, so deviating from the "design for extension" rule may yield a harder to understand class hierarchy.
2. If now the decision is made to prevent overriding unconditionally, then that's a backwards incompatible change.
3. If later we wish to reintroduce overriding, then for backwards compatibility reasons that will require a custom annotation. Contrast that with simplify respecting the final modifier: that approach works exactly as one would expect it to, and allows for both approaches.

[elucash]

Just from the top of my head, haven't decided actually, just thinking:

• if generated method overrides non-abstract method — issue a warning. It could be appropriate to let people know that a method would be overridden/shadowed.
• Having annotation like @Underride to disable the warning and do not generate overriding method. But on a second thought: having plain final modifier may be a way to go, no need to additional annotation, and if final is present, the generated code will be broken anyway, so we are not breaking anything by not generating overriding method in this case.

[Stephan202]

[..] if final is present, the generated code will be broken anyway, so we are not breaking anything by not generating overriding method in this case.

Exactly! It (a) fixes the bug, (b) relies on established Java concepts and (c) is backwards compatible.

[Stephan202]

And (d) just respecting final for the time being also doesn't close the door to introducing @Underride later. :)

[bartosz-bilicki]

I am fine with proposal to skip generating code only for _final_ method.
It seems to me it will not either suprise users, will not create some compatiblity issues with existing code and will not limit growth of Immutables library.

I think that documentation should be updated, saying something like:

Immutables library generates many methods for your classes. Exact method names and signatures depend on many factors, including

• fields in your class
• configuration on Value.Style annotation (prefixes, plularizing, etc)
• annotations on fields

New version of Immutables may introduce new methods. Therefore, if you write your own method in immutable base class it is advised to make that method final. Otherwise, Immutables library may generate method with same signature in gererated Immutable* class. You may be suprised that your hand-written method is never used (if you do have good coverage on unit tests).

[elucash]

@bartosz-bilicki thank you, this will be useful addition to the documentation