Lombok: [FEATURE] Support JDK 14 Records

[GabrielBB]

Something like this library:

https://github.com/javahippie/jukebox

[rzwitserloot]

You bet :)

[rzwitserloot]

Support for records:

• Make sure the pretty printer can print records and can print their new constructor form (the one without parens and args).
• Make sure that any lombok anything inside a record works fine, i.e.: public record Foo(int x) { @SneakyThrows public void foo() {} } should work.
• Generate a sane error that you can't do it: @Data, @Value,@Getter, @Setter, @ToString, @EqualsAndHashCode, or any of the @XArgsConstructor annotations on a record. These don't make enough sense to allow, if it would be possible at all to generate.
• _DO_ allow @With and @WithBy on 'fields' (these are more like parameters to the class def, I assume they can be annotated).
• Do not allow @Synchronized on methods inside a record; we can't generate the lock field.
• Do allow @Log and co, but only if it makes a static field.
• Do allow @Builder. Don't allow @SuperBuilder. Ensure @Builder.Default and @Singular work on 'record parameters'.
• Do not allow @Accessors or @FieldDefaults.
• Do not allow @UtilityClass.
• Do allow @FieldNameConstants.
• Do allow @NonNull on the 'class parameters' – these are copyable as normal and should therefore automatically affect builder and with(by) as normal. Buuuut.. should we 'override' the getter just to document that it, too, is nonnull just for documentation purposes? We could do that I guess. We should probably make a constructor def if you don't already have one, just to inject some null tests; if you already have one, we should inject null tests the same as @NonNull on a parameter does.

[randakar]

Most of this makes sense, but UtilityClass should still work. Records can
have methods and static fields, and we can override the constructor as well.

So what am I missing?

On Thu, Feb 13, 2020, 20:20 Reinier Zwitserloot [e-mail]
wrote:

Support for records:

• Make sure the pretty printer can print records and can print their
  new constructor form (the one without parens and args).
• Make sure that any lombok anything inside a record works fine, i.e.: public
  record Foo(int x) { @SneakyThrows public void foo() {} } should work.
• Generate a sane error that you can't do it: @Data, @Value,@Getter,
  @Setter, @ToString, @EqualsAndHashCode, or any of the @XArgsConstructor
  annotations on a record. These don't make enough sense to allow, if it
  would be possible at all to generate.
• DO allow @With and @WithBy on 'fields' (these are more like
  parameters to the class def, I assume they can be annotated).
• Do not allow @Synchronized on methods inside a record; we can't
  generate the lock field.
• Do allow @Log and co, but only if it makes a static field.
• Do allow @Builder. Don't allow @SuperBuilder. Ensure @Builder.Default
  and @Singular work on 'record parameters'.
• Do not allow @Accessors or @FieldDefaults.
• Do not allow @UtilityClass.
• Do allow @FieldNameConstants.

—
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/2356?email_source=notifications&email_token=AABIERN3H3W4WDMM4RTM63TRCWMQVA5CNFSM4KRI3DV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELWI2XA#issuecomment-585928028,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AABIERPTJKHSC6YRXQQGI33RCWMQVANCNFSM4KRI3DVQ
.

[rzwitserloot]

@randakar nothing - just not really seeing the point. What possible benefit is there to making a record type a 'utilityclass'? I guess I'm open to the idea that we should be okay with it, but it feels more like anybody writing @UtilityClass public record whatever() {} is confused, than that they have a good reason.

[randakar]

Ah, ok.

Well, the thing is, it feels somewhat plausible that some compiler version
might behave differently with a utilityclass record than with a
utilityclass class, which means someone might prefer it for some reasons we
can't foresee right now.

A utilityclass is closer to a record than a regular class in the sense that
records already are final out of the box ;)

Unless allowing it is a burden, in the long run I think lombok should allow
it.
That said.. for now I think it's fine, it will make ticking the "record
support" box easier I guess.

On Thu, Feb 13, 2020, 23:14 Reinier Zwitserloot [e-mail]
wrote:

@randakar https://github.com/randakar nothing - just not really seeing
the point. What possible benefit is there to making a record type a
'utilityclass'? I guess I'm open to the idea that we should be okay with
it, but it feels more like anybody writing @UtilityClass public record
whatever() {} is confused, than that they have a good reason.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rzwitserloot/lombok/issues/2356?email_source=notifications&email_token=AABIERMI2VOOF5Y5XMQVVELRCXA4TA5CNFSM4KRI3DV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELWZ65A#issuecomment-585998196,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AABIERITTZU7TSHCXPQFTXTRCXA4TANCNFSM4KRI3DVQ
.

[Rawi01]

I started to add support for records and I noticed that @Builder.Default support seems to be impossible because record parameters does not allow initializers. I also noticed that the result printed in eclipse tests is a little bit ugly:

import lombok.With;
@With record WithOnRecord(String a, String b) {
/* Implicit */  private final String a;
/* Implicit */  private final String b;
  public WithOnRecord(String a, String b) {
    super();
    .a = a;
    .b = b;
  }
  public @java.lang.SuppressWarnings("all") WithOnRecord withA(final String a) {
    return ((this.a == a) ? this : new WithOnRecord(a, this.b));
  }
  public @java.lang.SuppressWarnings("all") WithOnRecord withB(final String b) {
    return ((this.b == b) ? this : new WithOnRecord(this.a, b));
  }
}

We can either keep it like that, patch the print methods or write a custom print function for tests. What do you prefer @rzwitserloot ?

[randakar]

Is it possible to have the builder default stored in the inner builder
class instead?

On Tue, Sep 8, 2020 at 12:39 AM Rawi01 [e-mail] wrote:

I started to add support for records and I noticed that @Builder.Default
support seems to be impossible because record parameters does not allow
initializers. I also noticed that the result printed in eclipse tests is a
little bit ugly:

import lombok.With;@With record WithOnRecord(String a, String b) {/* Implicit / private final String a;/ Implicit */ private final String b;
public WithOnRecord(String a, String b) {
super();
.a = a;
.b = b;
}
public @java.lang.SuppressWarnings("all") WithOnRecord withA(final String a) {
return ((this.a == a) ? this : new WithOnRecord(a, this.b));
}
public @java.lang.SuppressWarnings("all") WithOnRecord withB(final String b) {
return ((this.b == b) ? this : new WithOnRecord(this.a, b));
}
}

We can either keep it like that, patch the print methods or write a custom
print function for tests. What do you prefer @rzwitserloot
https://github.com/rzwitserloot ?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rzwitserloot/lombok/issues/2356#issuecomment-688530275,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AABIERMIA3J6JQNCMLHALFLSEVOJPANCNFSM4KRI3DVQ
.

--
"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

[Rawi01]

The defaults are already part of the builder class, the problem is that you can not write something like this:

@Builder 
record WithOnRecord(@Builder.Default String a = "12") {
}

[randakar]

Ahh. Right.

On Tue, Sep 8, 2020, 16:15 Rawi01 [e-mail] wrote:

The defaults are already part of the builder class, the problem is that
you can not write something like this:

@Builder
record WithOnRecord(@Builder.Default String a = "12") {
}

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rzwitserloot/lombok/issues/2356#issuecomment-688905517,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AABIERIO5YQX2SGMYPHY7ZDSEY365ANCNFSM4KRI3DVQ
.

[randakar]

Reading https://openjdk.java.net/jeps/384 a bit more closely you are allowed to add normal constructors to records on top of the parameters at the top. And fields.

So in theory you can write:

@Builder
record WithOnRecord(String a) {
  @Builder.Default String a = "12";
}

[Rawi01]

A record cannot explicitly declare instance fields, and cannot contain instance initializers.

That's what I refer to, am I missing something?

[randakar]

No, you're not, I just overlooked it in the spec.

Without that I don't see how to do that either.

Maybe we can put the default value inside the annotations themselves but
that would only work for simple types, not for anything more complex than
Strings..

.. I wonder if it's possible to poke the java language designers over this.

On Thu, Sep 10, 2020, 19:31 Rawi01 [e-mail] wrote:

A record cannot explicitly declare instance fields, and cannot contain
instance initializers.

That's what I refer to, am I missing something?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rzwitserloot/lombok/issues/2356#issuecomment-690540100,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AABIEROVUCSAUVDQMVFKRBDSFEENZANCNFSM4KRI3DVQ
.

[janrieke]

Does the example of @randakar pass the syntactic analysis and thus will that code reach Lombok? I mean this looks like syntactically valid Java:

@Builder record WithOnRecord(String a) { @Builder.Default String a = "12"; }

If it does, Lombok could simply remove the declaration (or even better only the default assignment) and put it to its builder.

[Rawi01]

Eclipse accepts almost everything but javac do not like that and stops before lombok can do anything...

Edit: Something like this should be possible.

@Builder
record BuilderSimpleOnRecord<T>(List<T> l, String a) {
    static BuilderSimpleOnRecord $lombokBuilderDefault = new BuilderSimpleOnRecord(java.util.Collections.emptyList(), "default");
}

We cannot add @Lombok.Default to the static field (javac bug, https://bugs.openjdk.java.net/browse/JDK-8243057) so we need to use something else e.g. a fixed name like in this example.

[randakar]

Posted a JDK bug for this one: https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8254009

[Rawi01]

Added in 1.18.20