Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Placeholder type THIS to be replaced in derived classes. #2432

Open
TreffnonX opened this issue Apr 18, 2020 · 10 comments
Open

[FEATURE] Placeholder type THIS to be replaced in derived classes. #2432

TreffnonX opened this issue Apr 18, 2020 · 10 comments

Comments

@TreffnonX
Copy link

TreffnonX commented Apr 18, 2020

Java does not have a generic placeholder to represent the actual current class, as opposed to the declaring class of any field, method, and thelike. This would be tremendously helpful e.g. with methods that are supposed to return the object which's method is called, similar to a builder-pattern:

MySubclass myObject= ...;
myObject.refresh().doSomethingElse();

While the above will work, if doSomething is defined on or above the same type as refresh, this is - more often than not - not the case, especially if MySubclass is derived from a type declaring many standard functions for it's derivates. Assuming that refresh returns the called upon instance, it could be the same type as myObject, assuming at the time of invokation, I knew that type.

To achieve this, and handle similar issues, a placeholder annotation type could be introduced, similar to the var and val types, which is then replaced in the declaring class (and all compilation units marked with a certain annotation). Consider the following example:

@...
public class MySupertype {
    // will become signature:
    // public MySupertype refresh();
    public THIS refresh() {
        //...
        return this;
    }
}
@Redeclare
public class MySubtype  extends MySupertype {
    // will redeclare all fields and methods with 'THIS':
    // public MySubtype refresh() {
    //     return (MySubtype) super.refresh();
    // }
    // this implicitly declares the refresh method again, with the more specific return type of this class.
}

In this example, both MySupertype and MySubtype declare a refresh method, but with different return types. The only issue I see is that the compilation units are separate. However, since MySubtype is a subtype of MySupertype it should theoretically be possible to leave some trace in the supertype, which could then be used in the subtype to save the day.

@randakar
Copy link

randakar commented Apr 18, 2020 via email

@janrieke
Copy link
Contributor

janrieke commented Apr 18, 2020

Except... you do it the @SuperBuilder way: Create a static inner class ThisTypeHelper (with fancy type params), which you could use as return type. That class provides a get() method, which has the correct return type (with the help of the type params).
I've not tested this, so I'm not sure that this really works. Similar to @SuperBuilder, you'd need it on every class in the hierarchy, and you'd have to call the get() method instead of just using the returned value. The @ActualThisType (or whatever it's called) annotation could replace relevant occurrences of THIS and this with the correct statement (e.g. return this in a method with return type THIS becomes return ThisTypeHelper.for(this);)
Don't know if that's worth it...

@randakar
Copy link

randakar commented Apr 18, 2020 via email

@janrieke
Copy link
Contributor

janrieke commented Apr 19, 2020

I thought again and tried around with this. I'm pretty sure this will only work if we put the type param directly on the annotated class. Generating a parameterized inner helper class will not work. So in the end the code will look like this:

public static class Superclass<THIS extends Superclass<THIS>> {
	@SuppressWarnings("unchecked")
	public THIS refresh() {
		return (THIS) this;
	}
}

public static class Subclass<THIS extends Subclass<THIS>> extends Superclass<THIS> {
}

public static class Subsubclass<THIS extends Subsubclass<THIS>> extends Subclass<THIS> {
	public THIS refresh() {
		// Do something else here.
		return super.thisReturn();
	}
}

public static void main(String[] args) {
	Subclass<?> sub1 = new Subclass<>();
	Subclass<?> sub2 = sub1.refresh();

	Subsubclass<?> subsub1 = new Subsubclass<>();
	Subsubclass<?> subsub2 = subsub1.refresh();
}

It works, but the price is too high in my opinion, because you need those seemingly useless type params everywhere you use that class. Furthermore, you have to suppress a warning, which is also not so nice (although you can extract that to an extra method so you won't have to suppress warnings in the affected this-returning methods).

@TreffnonX
Copy link
Author

Thank you for trying though, it seemed simpler when I thought about it. But I understand that - more often than not - the restraints lombok must function under prevent such things. Hopefully, one day there will be a solution to this, but for now it seems we have to stick with the verbose redeclaraton.

@janrieke
Copy link
Contributor

Let's wait what the Lombok maintainers say. Maybe they have another idea.

@janrieke
Copy link
Contributor

janrieke commented Apr 26, 2020

There is a possible solution that works without those type params:

  • Rename the annotated class (e.g. AbstractMySupertype), make it abstract, and add the <THIS extends A<THIS>> type param and use it as return type.
    • Subclasses inherit from this abstract type (and will also be processed according to these steps).
  • Create a public concrete implementation MySupertype without a type parameter.
  • Create an interface with the orginal inheritance, so that instanceof on the concrete classes still works as expected.

However, there is a technical problem: lombok does not generate new compilation units, it just modifies existing ones. As the JLS does not allow more than one public class in a compilation unit, all other have to be package-private. So if you want to inherit from such a class across package borders, that won't work. That's a show-stopper IMO.

@Maaartinus
Copy link
Contributor

@janrieke

So if you want to inherit from such a class across package borders, that won't work. That's a show-stopper IMO.

On the opposite, it's a feature. You need threes type per class, which is a lot and making them all top-level makes it even worse. When you want to extend them in a different package, you need to make the abstract scaffold public, too. So I'd go for classes nested in an interface which would be IMHO much nicer as the interface is what's needed most of the time (actually, all the time; see below). It could look like

public interface Node {
    public static abstract class Scaffold<THIS extends Node.Scaffold<THIS>> 
        implements Node {....}
    public static class Impl extends Node.Scaffold<Impl> {....}

    public static Node create(....) {....}

    .... declarations
}

public interface SpecialNode extends Node {
    public static abstract class Scaffold<THIS extends Scaffold<THIS>>
        extends Node.Scaffold<THIS>
        implements SpecialNode {....}
    public static class Impl extends SpecialNode.Scaffold<Impl> {....}

    public static SpecialNode create(....) {....}

    ....
}

....

The nesting and the use of strange name helps to prevent mistakes like o instanceof Impl. Normally, all you need is the top-level interface, only when extending, you need the nested classes. But this extending gets done by Lombok, the user could write

@THIS interface VerySpecialNode extends SpecialNode {
    /// define methods like in a class, use `lombok.THIS` as the return type
    THIS clone() {....}
}

or something like this. No generics, no references to either Scaffold or Impl anywhere.

I may be missing something....

@janrieke
Copy link
Contributor

janrieke commented Apr 27, 2020

So users of the annotated class should only work with the interface? Then you need the type params on the interface, otherwise you won't have the actual return type. And then the problem with the unnecessary <?> remains. Or did I get you wrong?

@Maaartinus
Copy link
Contributor

@janrieke
So users of the annotated class should only work with the interface? Then you need the type params on the interface, otherwise you won't have the actual return type. And then the problem with the unnecessary <?> remains. Or did I get you wrong?

I wanted to hide all generics in the generated code, but I may be completely wrong....

It should go as follows: The user defines the classes using e.g.,

@THIS
@RequiredArgsConstructor
@Getter
interface SpecialNode extends Node {   
    public THIS clone() {
        return new SpecialNode(parent);
    }

    private final THIS parent;
    @Setter private THIS otherNode;
}

@THIS
@Getter
interface VerySpecialNode extends SpecialNode {
    VerySpecialNode(THIS parent, String someString) {
        super(parent);
        this.someString = someString;
    }

    public THIS clone() {
        return new VerySpecialNode(parent, someString);
    }

    public THIS clone() {
        return new VerySpecialNode(parent);
    }

    private final String someString;
}

and Lombok creates the nested Scaffold class and move the code therein. Additionally it adds the Impl class and the static create method to the interface. The user would write things like

VerySpecialNode vsn = VerySpecialNode.create(null);
vsn.setOtherNode(VerySpecialNode.create(null, "someString");
assertTrue(vsn instanceof SpecialNode);
assertTrue(vsn.clone() instanceof VerySpecialNode);
assertEquals("someString", vsn.getOtherNode().getSomeString());

with no generics at all.

What obviously can't work ideally, is using .getClass(). This may be a problem in @EqAHC, too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants