skinny85 / jilt

Java annotation processor library for auto-generating Builder (including Staged Builder) pattern classes
Other
240 stars 12 forks source link

Support for Java Records #9

Closed maciejwalkowiak closed 10 months ago

maciejwalkowiak commented 10 months ago

Putting @Builder on the Java record like:

@Builder
public record Person(String firstName, 
                     String lastName, 
                     LocalDate dateOfBirth) {
}

results in:

error: @Builder can only be placed on classes, constructors or static methods

The workaround is to create compact constructor:

public record Person(String firstName,
                     String lastName,
                     LocalDate dateOfBirth) {
    @Builder
    public Person {
    }
}

Ideally, this would not be needed and it would be enough to put @Builder on the record declaration.

maciejwalkowiak commented 10 months ago

For that the minimum Java version would have to be bumped to 16+ (likely 17 makes more sense) which means quite a bit of updates to Gradle files.

skinny85 commented 10 months ago

Hi @maciejwalkowiak, thanks a lot for opening the issue!

To clarify: does the workaround you posted (placing @Builder on the constructor of the record) work - as in, does Jilt generate a Builder that can be used to create a valid instance of the record?

Thanks, Adam

maciejwalkowiak commented 10 months ago

Yes it does, the workaround works!

Adding support for records without the workaround is trivial (it's a one liner). Upgrading Gradle config to make it work with Java 17 is more work.

skinny85 commented 10 months ago

Is the problem in this line?

https://github.com/skinny85/jilt/blob/5646306d00095552d39b323698bf644acfa8c565/src/main/java/org/jilt/Builder.java#L228

Because there is a new ElementType enum value for records in Java 16+?

skinny85 commented 10 months ago

(Sorry, I didn’t have time to look into this on a laptop yet 😛)

skinny85 commented 10 months ago

OK, I've finally had some time to dig into this issue 🙂.

As it turns out, the declaration of the @Builder annotation, and its usage of ElementType is not the problem, since TYPE still works for records - the JavaDocs of ElementType say:

public enum ElementType {
    /** Class, interface (including annotation interface), enum, or record
     * declaration */
    TYPE,
    // ...

But, the problem is in this code: https://github.com/skinny85/jilt/blob/5646306d00095552d39b323698bf644acfa8c565/src/main/java/org/jilt/internal/BuilderGeneratorFactory.java#L35-L35

Since the ElementKind enum does have a new value for records, ElementKind.RECORD.

But that's great news - it means there's a chance it's possible to add record support without making the library require Java 16+, which I'd like to avoid if possible (the problem is not the Gradle configuration - I've already migrated it to 8.5 on the develop branch. I just want the library to be as widely usable for folks as possible).

I'll update the issue once I know more.

skinny85 commented 10 months ago

OK, I managed to figure it out. I'll do a 1.3 release fixing this issue sometime this week.

skinny85 commented 10 months ago

This has been fixed in the latest release, cc.jilt:jilt:1.3.

I'm resolving this one, please comment if you encounter any problems related to this, and I'll reopen the issue.

maciejwalkowiak commented 10 months ago

Fantastic, thanks @skinny85!