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

Property names are not available with Lombok and constructor binding #18730

Closed
ghost opened this issue Oct 24, 2019 · 10 comments
Closed

Property names are not available with Lombok and constructor binding #18730

ghost opened this issue Oct 24, 2019 · 10 comments
Labels
for: external-project For an external project and not something we can fix

Comments

@ghost
Copy link

ghost commented Oct 24, 2019

The output of the spring-boot-configuration-processor looks like below. It can't determine the names of any arguments. Those classes are standard classes with all final fields, annotated with lombok's @Data.

The issue only happens if the constructor is created by lombok and the class is annotated with @ConstructorBinding. If I create the constructor by hand, then it all works just fine. If I make the class mutable and remove @ConstructorBinding, it also works fine, even with a lombok-generated constructor.

{
  "groups": [
    {
      "name": "config.processor",
      "type": "io.app.config.ProcessorConfig",
      "sourceType": "io.app.config.ProcessorConfig"
    },
    {
      "name": "config.sender",
      "type": "io.app.config.SenderConfig",
      "sourceType": "io.app.config.SenderConfig"
    }
  ],
  "properties": [
    {
      "name": "config.processor.arg0",
      "type": "java.util.Map<java.lang.String,io.app.config.ProcessorConfig$InnerClasss>",
      "sourceType": "io.app.config.ProcessorConfig"
    },
    {
      "name": "config.sender.arg0",
      "type": "java.lang.Boolean",
      "sourceType": "io.app.config.SenderConfig"
    },
    {
      "name": "config.sender.arg1",
      "type": "java.lang.String",
      "sourceType": "io.app.config.SenderConfig"
    },
    {
      "name": "config.sender.arg2",
      "type": "java.lang.String",
      "sourceType": "io.app.config.SenderConfig"
    },
    {
      "name": "config.sender.arg3",
      "type": "java.lang.String",
      "sourceType": "io.app.config.SenderConfig"
    },
    {
      "name": "config.sender.arg4",
      "type": "java.lang.String",
      "sourceType": "io.app.config.SenderConfig"
    }
  ],
  "hints": []
}
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 24, 2019
@snicoll snicoll added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 25, 2019
@snicoll snicoll added this to the 2.2.x milestone Oct 25, 2019
@snicoll snicoll changed the title spring-boot-configuration-processor can't determine property names of immutable ConfigurationProperties classes annotated with @ConstructorBinding with lombok-generated constructor Property names are not available with Lombok and constructor binding Oct 25, 2019
@snicoll
Copy link
Member

snicoll commented Oct 25, 2019

@andrebraitc thanks for the report, I was able to reproduce it. I've debugged things a bit and I can see the model Lombok gives us contain arg0 and the like unfortunately.

When a @ConstructorBinding is present, we switch to pure reflection mode. If we want to infer what Lombok is going to do (as we do for regular accessors) we'd have to infer what the constructor should be, meaning looking at final fields and those who have a non-nullable constraint on them as RequiredArgsConstructor states. I am tempted to go best effort and just look at final fields.

@wilkinsona
Copy link
Member

I wonder if it's worth raising a Lombok enhancement request to generate parameter names that are more meaningful than arg0 etc. It could, presumably, reuse the field names to which the constructor's parameters are going to be assigned.

@snicoll
Copy link
Member

snicoll commented Oct 25, 2019

Something else to mention is that if any property requires a default value, you'll have to generate the constructor anyway to provide it using @DefaultValue.

@snicoll
Copy link
Member

snicoll commented Oct 25, 2019

@wilkinsona I agree but that suggestion would only work if Lombok ran before us. Our lombok support doesn't have that assumption at the moment, it reads the lombok annotations and infer what lombok will do, ignoring whether the actual bytcode has been produced or not. This very example is a bit more involved unfortunately as we'd have to infer what the constructor would look like.

@snicoll snicoll added the for: team-attention An issue we'd like other members of the team to review label Oct 25, 2019
@wilkinsona
Copy link
Member

would only work if Lombok ran before us

That seems like a reasonable requirement for us to introduce in a new minor release. As I understand it, it would remove a lot of complexity from the annotation processor and also remove the risk of our understanding of what Lombok will do getting out of sync with what Lombok actually does.

@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Oct 25, 2019
@andrebrait
Copy link

I deleted my other GitHub account. Contact this account if you need.
As for the issue itself, I would think a lombok improvement would be in order, too.

However, I'm not sure you can do an annotation processor that assumes another has already run before it, can you? Perhaps something that would attach itself to a different build phase than the other annotation processor?

@snicoll
Copy link
Member

snicoll commented Oct 28, 2019

I've created projectlombok/lombok#2275

@wilkinsona
Copy link
Member

This has hopefully been addressed in Lombok by a combination of projectlombok/lombok#2497 and projectlombok/lombok#2556. The changes aren't in a release yet.

@snicoll
Copy link
Member

snicoll commented Sep 22, 2020

I've tested a snapshot of the lombok build and I confirm that the issue is fixed.

@snicoll snicoll closed this as completed Sep 22, 2020
@snicoll snicoll added for: external-project For an external project and not something we can fix and removed status: blocked An issue that's blocked on an external project change type: bug A general bug labels Sep 22, 2020
@snicoll snicoll removed this from the 2.2.x milestone Sep 22, 2020
@dan-lind
Copy link

I case anyone else stumbles upon this - I updated lombok to 1.18.16 and still faced issues, spring now didn't output any field data at all when I had my class annotated with

@Value
@ConstructorBinding
@RequiredArgsConstructor
@ConfigurationProperties

Turns out the order in which annotation processors are defined (in my case in build.gradle) is important.
If I define the spring annotation processor first like this, it doesn't work, I get no properties at all.

annotationProcessor("org.springframework.boot:spring-boot-configuration-processor") annotationProcessor("org.projectlombok:lombok:1.18.16")

but if I switch the order like this, everything works fine

annotationProcessor("org.projectlombok:lombok:1.18.16")
annotationProcessor("org.springframework.boot:spring-boot-configuration-processor") 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: external-project For an external project and not something we can fix
Projects
None yet
Development

No branches or pull requests

6 participants