Lombok: Could we override parent's setters and return derived object annotated with Accessors?

[cloorc]

@ToString
@EqualsAndHashCode(callSuper = true)
@Accessors(chain = true)
@NoArgsConstructor
@AllArgsConstructor
@Setter
//Could we add an option assume override and it should looks like
//Setter(override = true), which will override its parent's setters as following
public class Consumer<T> extends Rpc<T> {
    // default
    // Consumer#setGroup(group) return Rpc<T> not Consumer<T>
    // but we need a Consumer<T> in fact.
    @Override
    public Consumer<T> setGroup(String group) {
        super.setGroup(group);
        return this;
    }
}

@Getter
@Setter
@ToString
@Accessors(chain = true)
@NoArgsConstructor
@AllArgsConstructor
public class Rpc<T> implements Serializable {
    private String group = "default";
    private String version = "1.0.0";

    // after delombok
    public Rpc<T> setGroup(String group) {
        this.group = group;
        return this;
    }
}

[kylerjensen]

I would like to see this implemented as well. It breaks chaining if attempting to set any fields from parent class and return an instance of the derived class. For instance:

@Setter
@Accessors(chain = true)
class Foo {
    private String foo;
}

@Setter
@Accessors(chain = true)
class Bar extends Foo {
    private String bar;
}

Given the above, one would like to do the following:

Bar myVar = new Bar().setFoo("foo").setBar("bar");

But this fails because setFoo(String) returns Foo even though it was called from an instance of Bar.

The same goes for the @Wither annotation.

[jaypatel512]

Same here. This makes it tricky specially when using non-IDE environment, and makes us dig into these classes every time we code. Feels counter-productive.

[bettykatya]

Any updates on this?

[alvint]

Since you're using generics anyway, why not accomplish this "top down" instead of "bottom up"? The disadvantage is the generic type gets uglier and you or Lombok has to create custom setters in the parent class, but the advantage is you don't have to override the setters for every step down the hierarchy and call super (I imagine that's a performance issue). You just have to tweak the parent class code and nothing else (the generic type of child classes also need to be the more specific):

@Getter
//@Setter
@ToString
//@Accessors(chain = true)
@NoArgsConstructor
@AllArgsConstructor
public class Rpc<T extends Rpc<T>> implements Serializable {
    private String group = "default";
    private String version = "1.0.0";

    protected final T that = (T)this;

    public T setGroup(String group) {
        this.group = group;
        return that;
    }
}

...

// any children's type also needs to be more specific
@ToString
@EqualsAndHashCode(callSuper = true)
@Accessors(chain = true)
@NoArgsConstructor
@AllArgsConstructor
@Setter
public class Consumer<T extends Consumer<T>> extends Rpc<T> {
    // more stuff

    // you can even use the `that` defined in the parent, and somehow it will be
    // the most specific type
}

This uses a perfectly legal generic recursive reference to require that the type of Rpc/Consumer must ultimately extend Rpc, which seems to always be the case here. There are some minor details that need to be handled if the code is generated with Lombok, for example potential name conflicts with that, but the Lombok guys are smart so I'm sure they can figure it out.

It's best not to think too hard about the signatures unless you enjoy getting dizzy. :)

[randakar]

Isn't that par for the course with generics? ;-)

On Wed, Jan 23, 2019 at 11:32 PM Alvin Thompson [e-mail]
wrote:

Since you're using generics anyway, why not accomplish this "top down"
instead of "bottom up"? The disadvantage is the generic type gets uglier
and you or Lombok has to create custom setters in the parent class, but the
advantage is you don't have to override the setters for every step down the
hierarchy and call super (I imagine that's a performance issue). You just
have to tweak the parent class code and nothing else (the generic type of
child classes also need to be the more specific):

@Getter//@Setter@ToString//@Accessors(chain = true)@NoArgsConstructor@AllArgsConstructorpublic class Rpc> implements Serializable {
private String group = "default";
private String version = "1.0.0";

protected final T that = (T)this;

public T setGroup(String group) {
    this.group = group;
    return that;
}

}

...

// any children's type also needs to be more specific@ToString@EqualsAndHashCode(callSuper = true)@Accessors(chain = true)@NoArgsConstructor@AllArgsConstructor@Setterpublic class Consumer> extends Rpc {
// more stuff
// you can even use the that defined in the parent, and somehow it will be the right type
}

This uses a perfectly legal generic recursive reference to require that
the type of Rpc/Consumer must ultimately extend Rpc, which seems to
always be the case here.

It's best not to think too hard about the signatures unless you enjoy
getting dizzy. :)

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/rzwitserloot/lombok/issues/1147#issuecomment-456992641,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAKCRTojxIpLun0dhf6w5n5-cSUgiEvdks5vGOMPgaJpZM4JB5GB
.

--
"Don't only practice your art, but force your way into it's secrets, for it
and knowledge can raise men to the divine."
-- Ludwig von Beethoven

[alvint]

@randakar The dizziness? Yep. :)

[randakar]

Yeah, that's what I meant :)

On Thu, Jan 31, 2019 at 9:54 PM Alvin Thompson [e-mail]
wrote:

@randakar https://github.com/randakar The dizziness? Yep. :)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rzwitserloot/lombok/issues/1147#issuecomment-459502594,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAKCRTWVh1jbrIue_rjLpAUdYvuMJqzpks5vI1fvgaJpZM4JB5GB
.

--
"Don't only practice your art, but force your way into it's secrets, for it
and knowledge can raise men to the divine."
-- Ludwig von Beethoven

[benmccann]

This is also the most requested feature in the lombok-pg repo: https://github.com/peichhorn/lombok-pg/issues/78

[crimsonvspurple]

+1 for this. Thank you!

[rzwitserloot]

This is incredibly complicated because it requires resolution to know which setters exist. It also breaks on separate compilation and nobody can fix that (if you edit and compile the parent separately from the child). There are ways to tackle this that remain compatible with such separate compilation (and, incidentally, can be supported by lombok): The @SuperBuilder feature does it. But, beware! Here be dragons - that requires some serious fancy generics footwork to hack it together.

We're not going to add this feature; it would be a massive maintenance burden, and it's 'solving' a problem in a crappy way (because it is weak to split compilation, which is rather relevant; plenty of libraries have classes that are intended to be subclassed!)

I get that the value is significant, but, trust me, the cost is higher. Can't be done.

[alvint]

@rzwitserloot Doesn't the "top down" approach I mentioned in my previous comment solve these issues?

[rzwitserloot]

@alvint yes it would, but the maintenance of SuperBuilder is already pushing the limit. Java just does not have self-types, and hacking around it is a high cost. Sometimes, for nicer API, this is worth doing.

But it just clashes, a lot, to try to blaze new ground and introduce new ways of doing things in... code with old timey setters.

The amount of times i ever even write a setter in my code is.. fractions compared to getters, for example. The maintenance burden is not worth it; I don't think the generics hack (even if lombok makes the effort go away) is worth it; that generics is still part of your signature and it just feels incredibly sheepish to document your class with @param S self-type; just a hack so we can give you, API user, the convenience of being able to chain setter calls.

Issue remain

[Maaartinus]

@rzwitserloot Compared to SuperBuilder, this feature - done @alvint's way - is extremely trivial. All it takes is given a field or a method annotated like

@This
protected final T that = (T) this;

then replace all occurrences of return this by return that (the field value or method result) in generated code and declare the return type of the method correspondingly.

The generic hack itself (i.e., declaring class Rpc<T extends Rpc<T>> and declaring the that field) should be the responsibility of the user. This makes Lombok's job simple (no generics hacking, no class declaration rewriting) while imposing a tiny overhead on the user.

It's ugly like hell, but it's the Java way, unfortunately. I also avoid mutability, but when dealing with persistency and other frameworks, it's usually not possible. Moreover, such classes tend to have tons of fields, implying tons of setters.

[rzwitserloot]

Creative; I guess to make this feature as useful as can be without delving into the 'too much magic' territory, we move away from setter entirely, and we introduce a new feature, @SelfReturning, which an annotation you put on a class. This will add some generics unless it finds <T extends Self<T>> on the type already. Then it finds all methods where ALL return statements are of the form 'return this;' and no other return statements are found, updating their return type from Self to S. So, example input:

    @ReturnsSelf public class Example {
        @Setter String x;
        public Example chainable() {
            System.out.println("Hello!");
            return this;
        }
    }

becomes:

    public class Example<S extends Example<S>> {
        @SuppressWarnings S $lombok$self = (S) this;
        String x;

        @Generated public S setX(String x) { this.x = x; return $lombok$self; }

        public S chainable() {
            System.out.println("Hello!");
            return $lombok$self;
        }
    }

I... think I'd accept a PR, but not sure how high on the priority list this is gonna go. Will discuss with @rspilker tonight.

[Maaartinus]

@rzwitserloot I'm not sure about the field. @alvint proposed a protected field, I use a method (and hope for inlining) as I want to keep the instances as small as possible. I'd say, Lombok needs neither as replacing
return this;
by
@SuppressWarnings S $lombok$self = (S) this; return $lombok$self;
is verbose, but nobody cares.

---

When you start with something like class Example<K, E extends Enum<E>>, then the question arises where to add the new type param (probably last?).

[rzwitserloot]

Good idea, @Maaartinus. We shall replace return this; with { @SW S $lombok$self = (S) this; return $lombok$self; } - including the braces, making us robust vs. being an unbraced 'action' part of an if statement, as well as ensuring that the created variable rolls out of scope as fast as possible.

I think if generics is already on the class, we have 2 choices to make:

1. Just error; say it is too difficult then. This entire feature just will not work at all.

2. Scan for a self-returning construct and just use that variable instead of generating S. As in, we count how many generics args the class has, then we scan each if they are NAME extends SelfType<_, _, NAME, _> where NAME appears in the right position. If this occurs, we don't mess with the signature at all, and upgrade all methods whose return type is yourself and where every return statement is return this; to returning type NAME and returning a properly cast/suppresswarningsed variant of this to make it work.

I don't think it's feasible to try to shove another self-returning construct in there 'to keep it simple' ; that seems too hostile to programmers. If one is already there we must reuse it. Then, another choice to make if we do NOT find a selftype in the generics already:

1. Require the programmer to manually add it, at which one we'll use it.
2. At the end
3. At the beginning

I'm tempted to go with door number 1 here.

But most of all I don't want to do this. I was playing around and I don't know how to construct these things. It's very confusing to me; I assume it's very confusing to everybody. And all this pain for.. what? A chainable API? It feels incredulous to me that you'd take on all this pain in your API (I'm not talking about the boilerplate, lombok makes that go away if this feature is implemented, JUST the API aspect) – just for chainable methods? That's... just nuts, no?

What am I doing wrong here:

class Parent<S extends Parent<S>> {
        public S chain() {
                System.out.println("Hello");
                { @SuppressWarnings("unchecked") S $lombok$self = (S) this; return $lombok$self; }
        }
}

class Child<S extends Child<S>> extends Parent<S> {
        public S chain2() {
                System.out.println("Hello");
                { @SuppressWarnings("unchecked") S $lombok$self = (S) this; return $lombok$self; }
        }
}

class Test {
        public static void main(String[] args) {
                 // line below deos not work; Child is not a valid arg for S. What am I messing up?
                Child<Child> x = new Child<Child>().chain().chain2().chain();
        }
}

I'm pretty sure based on what SuperBuilder does and what this treatise on generics to fake self types explains, you can't.

[Maaartinus]

@rzwitserloot Braces, sure.

I think if generics is already on the class, we have 2 choices to make

1. Just error

IMHO that's too bad, given that it's probably not going complicated.

1. Scan for a self-returning construct and just use that variable instead of generating S.

Yes, that should be simple and work well.

where NAME appears in the right position

This took me a while. FTR: You mean in Example<K extends Example<V, K>, V extends Example<V, K>> both type parameters have wrong position.

I don't think it's feasible to try to shove another self-returning construct in there 'to keep it simple' ; that seems too hostile to programmers. If one is already there we must reuse it.

Agreed!

Then, another choice to make if we do NOT find a selftype in the generics already:

1. Require the programmer to manually add it, at which one we'll use it.

That's IMHO good enough as it's trivial for the programmer to fix the problem.

I assume it's very confusing to everybody. And all this pain for.. what? A chainable API?

You can see it as a good generic-teaching exercise for every programmer. :rofl:

And there's more than just chaining. I have non-generic User extends Entity with a fluent chained id setter in the superclass. Jusr today, instead of

if (input.users.isEmpty()) input.users = ImmutableList.of(u.id(1));

I had to write

if (input.users.isEmpty()) {
    User u = new User();
    u.id(1);
    input.users = ImmutableList.of(u);
}

---

Your example code isn't very realistic as the bottom classes are usually not parameterized, so your problem doesn't happen. Anyway, it has an easy and nice solution:

    static <S extends Child<S>> Child<S> newChild() {
        return new Child<>();
    }
    Child<?> x = newChild().chain().chain2().chain();

Once you leave the S-context, you can't parameterize Child anymore (except with the question mark). Luckily, you usually don't need it. Actually, the simple way

    Child<?> x = new Child<>().chain().chain2().chain();

works too. The perfect solution consists of fully expanding the definition, i.e.,

    Child<Child<Child<...............
    >>>...................
    x = new Child<Child<Child<....................
    >>>...................
    ().chain().chain2().chain();

No idea, why Java can't get a real self-type instead of this mess.

[rzwitserloot]

No idea, why Java can't get a real self-type instead of this mess.

That's the crux of the issue, isn't it? Whatever lombok can do here, it would be vastly inferior to the true solution, which is a real selftype. In general I'm against putting inferior hackery in lombok while we twiddle our thumbs waiting for java to get it. (As an example, I do not think @Cleanup is inferior to ARM; it's about as good; better in some ways, worse in others, which is why I was in favour of adding it way back then. Cleanup predates ARM by about a year).

Hence why I am still against the idea; but we're still exploring, maybe we find a cool way that's worth doing.

Your example code isn't very realistic as the bottom classes are usually not parameterized, so your problem doesn't happen.

But having to write in the docs of @SelfReturning: "Do not use this except on abstract classes" feels bad. And this feature is already falling well short on the bang for the buck scale, so it needs MORE general applicability, not less of it.

static <S extends Child<S>> Child<S> newChild() { return new Child<>(); }

Okay, so, then we need to basically enforce @SomeArgsConstructor(staticName = something) to be used in combination with @ReturnsSelf on a non-abstract class, right?

We can do that, just.. again that makes it a worse feature and it already wasn't really cutting it.

I have very little hope we're going to get to something that is worth adding to lombok.

[Maaartinus]

No idea, why Java can't get a real self-type instead of this mess.

That's the crux of the issue, isn't it? Whatever lombok can do here, it would be vastly inferior to the true solution, which is a real selftype.

Yes, but there's no JEP, so we're speaking about tens of year, if ever. What's worse: We already have a "solution", so we can't hope to get a real one in this millennium (and similarly, because of nullable annotation, we can't hope on getting null-aware type system).

I do not think @Cleanup is inferior to ARM

Syntactically, it's much better as ARM needs a new useless block. Semantically - I can't recall the details.

But having to write in the docs of @SelfReturning: "Do not use this except on abstract classes" feels bad.

But it works on non-abstract classes, too; it's just that instantiating them is bit tricky. But when you need such an instance, chances are that you have a context providing the type variable, too. Whenever you want to return a Child, you probably should declare the method with generics just like my newChild above.

Okay, so, then we need to basically enforce @SomeArgsConstructor(staticName = something) to be used in combination with @ReturnsSelf on a non-abstract class, right?

Maybe "enforce" is too much as it's not strictly necessary.... what's worse: There may be a superclass requiring a super constructor call. :cry:

I have very little hope we're going to get to something that is worth adding to lombok.

It's far from perfect... however, in one project, I made a few classes @SelfReturning manually, which is even less perfect. And I'm very happy about that as it makes the usage much simpler. Things like ImmutableList.of(new User().id(1)) are far superior to the three lines otherwise needed (note that there's no use of setter chaining).

---

There's a won'tfix linking to a paper (one link is broken, but the one with ".gz" works) and slides.

[rzwitserloot]

Well, this issue got closer to making the cut than I was expecting, but, @rspilker and I discussed it and it's a pass. Not enough value (yes, self-returning APIs can be nice, but they don't save that much boilerplate), not enough commonality (probably a chicken-and-egg thing, but still, that's a point this feature did not score: In the field, self-returning stuff is relatively rare outside of builders), too much of a maintenance burden (already thinking about the stuff we'd have to do in those static constructor is giving me a small headache), too many caveats we'd have to name in the documentation, lacking in role as 'advisory' feature (generally, we prefer it if using a lombok feature means your code is definitely better vs. not doing that, but the generics shenanigans means that there are plenty of situations where you shouldn't just blindly toss selfreturning at a class).

Thanks for brainstorming with us, though!

NB: The dream is that you have an annotation or other marker that you apply to a method. This enforces that either [A] the method's return type is void, or [B] that it is its own type or any parent type thereof, and that all return statements inside are either return this; or return X() where X is a method that is also so annotated and being invoked with itself as received (so return super.someMethodAnnotatedWithTheSelfReturningThing(); is also okay. Then, the compiler will just.. fix it for you. This is technically possible as 'call site lombok tool', which is a rich vein of lombok features we aren't yet ready to delve into, and java itself can add this by way of a JEP that will mesh perfectly with both currently common styles (void returns AND self returns, both of them can get this annotation without breaking backwards compatibility in any way. Once added, you can never remove it though). Given that this dream exists, that's yet another nail in the coffin of this feature; I really dislike making known-inferior features.

[Schoaf]

Couldn't the problem also be solved by allowing the @Getter annotation to override parents setters or just create setters for parent fileds?
Then the @accessor should be able to pick them up and work as intended (just return this). It should always be possible to return the child class instead of the parent class in an overridden method.