projectlombok / lombok

Very spicy additions to the Java programming language.
https://projectlombok.org/
Other
12.87k stars 2.38k forks source link

Builder not available to other Annotation Processors #1538

Closed filiphr closed 3 years ago

filiphr commented 6 years ago

I've been playing around with adding support for Builder in MapStruct and I have noticed that the public static method with no arguments created by Lombok and the Builder itself is not available during the annotation processing phase.

I don't know the code that much. However, I suppose that it needs something similar to what was done for getters and setter in https://github.com/rzwitserloot/lombok/commit/b867f81b8a251a8a32e42f53c2be34d520938bd7.

Would it be possible to have the builder creation method, the Builder and the builder method types to be available to other annotation processors?

rzwitserloot commented 6 years ago

The thing described in b867f81 is applied to all methods lombok generates.

Potentially this is an ordering issue; if mapstruct runs first, that's a problem.

Fortunately we solved this already, see: https://github.com/mapstruct/mapstruct/issues/510

can be closed at 2018-05-18

filiphr commented 6 years ago

@rzwitserloot I am one of the MapStruct team members. And I can guarantee you that the builder creation method, the builder class and it's methods cannot be seen during the annotation processing round. I debugged this and I can confirm that I can see the getters and setters, but not the builder. Are you creating the builder in the same time as the getters and setters?

If you want I can create a sample project where you can see what happens.

ramses-gomez commented 6 years ago

@rzwitserloot I can confirm what @filiphr had said I was trying to do a workaround and when the annotation is getting processed I can see the getters/setters but I can not see the builder method using the TypeElement for the type.

marcuswhit commented 6 years ago

Hi, any progress on this? In its current state I'm having to make my pojos mutable with setters so I don't have to go writing a heap of manual mapping code 👎

reza-mousavi commented 5 years ago

Hi,

I have tested mapscruct using lombok builders when mappers and models are defined in separate modules and it works perfectly.

Is there any plan to support lombok builders when both lombok classes and mapstruct mappers are defined in the same module?

Froodulous commented 5 years ago

This doesn't seem to be an issue in Lombok version 1.16.22 which is the version I get from spring boot 2.0.6. That combined with mapstruct 1.3.0.Beta2 seems to work fine with the model and the mapper in the same module. Is it possible I'm just getting lucky with the order that annotation processors are running, or is this due to changes in the newer versions of Lombok?

SimSonic commented 5 years ago

I confirm, just now tested Lombok 1.18.4 and MapStruct 1.3.0.Beta2. Builder and SuperBuilder are processed correctly. No need in separate module.

dlsrb6342 commented 5 years ago

@SimSonic I tested with same version with you. I use Builder annotation in Target model. but Mapstruct annotation processor cannot find accessor of target model. It cannot detect Builder class in Target model. Can i get example successful project ?

SimSonic commented 5 years ago

@dlsrb6342 here you are: https://github.com/SimSonic/lombok-mapstruct-builders-example I have to note: I use maven 3.6.0

binkley commented 5 years ago

@SimSonic Your example is very helpful, thank you!

So ... in Maven, everything is working just as you demonstrated. I'm trying the equivalent in Gradle 5.1, and not so good.

I've declared in dependencies both Lombok & mapstruct-processor as annotationProcessor, and get complaints I need a no-arg ctor (so assume the builder isn't seen by Mapstruct). I then add them also as compileOnly. This seems better, however, @Mapping annotations are not recognized. (In all cases, I also have mapstruct-jdk8 as plain implementation.)

I'm new to Mapstruct. Is there some magic formula for Gradle I need to use?

binkley commented 5 years ago

OK, I found putting Lombok first, before mapstruct-processor, at least fixed the issue with @Mapping not being handled. However, I'm still left with complaints of lacking a no-arg ctor.

Am I encountering https://github.com/mapstruct/mapstruct/issues/1581?

filiphr commented 5 years ago

@SimSonic have you tried to put lombok after MapStruct, like in https://github.com/mapstruct/mapstruct/issues/1581?

@rzwitserloot I tried with Lombok 1.18.6 and MapStruct 1.3.0.Final. With gradle and maven when Lombok is first we can only see the getters and setters, but not the builder method. If Lombok is second it is fine. Have a look at https://github.com/filiphr/mapstruct-examples/tree/lombok-ordering/mapstruct-lombok. That is one of our Lombok examples and when you try to compile with Maven or Gradle it won't work and it will give the error:

Property "testing" has no write accessor in com.mycompany.entities.TargetWithBuilder.

If you switch the order it will compile properly. You can put a break point in DefaultBuilderProvider.java#L162 and see the type element for the methods.

Krzysztof-Lempicki commented 5 years ago

Hi, To run mapstruct with lombok according to: working example from above most important change that I made was to downgrade java version in pom to 1.8. It not work with 1.10.

jarekkarpinski commented 4 years ago

Don't know if it is relevant but if I put lombok after mapstruct I lose warnings/errors about not mapped properties when property is in subclass:

@Data
@Builder
public class A extends B {}

@Data
@Builder
public abstract class B {
    String warnField;
}

In the end I have to remove @Mapping(target = "warnField", ignore = true) or else I will get an error: "Unknown property "warnField" in result type A. Did you mean "null"?"

I used @Value @Builder @AllArgsConstructor(access = AccessLevel.PRIVATE) on my X class that was mapped to class A:

@Mappings({
   @Mapping(target = "warnField", ignore = true)
})
A mapToA(X x)
archie-sh commented 4 years ago

Don't know if it is relevant but if I put lombok after mapstruct I lose warnings/errors about not mapped properties when property is in subclass:

@Data
@Builder
public class A extends B {}

@Data
@Builder
public abstract class B {
    String warnField;
}

In the end I have to remove @Mapping(target = "warnField", ignore = true) or else I will get an error: "Unknown property "warnField" in result type A. Did you mean "null"?"

I used @Value @Builder @AllArgsConstructor(access = AccessLevel.PRIVATE) on my X class that was mapped to class A:

@Mappings({
   @Mapping(target = "warnField", ignore = true)
})
A mapToA(X x)

Putting lombok after mapstruct also worked for me

As per the mapstruct lombok example https://github.com/filiphr/mapstruct-examples/tree/lombok-ordering/mapstruct-lombok/src/main/java/com/mycompany

I use @Getter @Builder

thunderhook commented 3 years ago

+1 for fixing this.

We run into this issue when we try to use annotation processing of lombok, then hibernate-jpamodelgen and then mapstruct-processor in this order. We use 1.8.

dnaumovich commented 3 years ago

+1 for fixing this

After updating Spring Boot from 2.3.4.RELEASE to 2.4.0 (with lombok upgraded automatically from 1.18.12 to 1.18.16) this issue occured and MapStruct couldn't see the generated builder classes.

filiphr commented 3 years ago

@dnaumovich the upgrade from 1.18.12 to 1.18.16 has nothing to do with this issue, but rather with the breaking change mentioned in the Lombok release notes. You need to add lombok-mapstruct-binding as a dependency as well.

briceamk commented 3 years ago

+1 for fixing this

After updating Spring Boot from 2.3.4.RELEASE to 2.4.0 (with lombok upgraded automatically from 1.18.12 to 1.18.16) this issue occured and MapStruct couldn't see the generated builder classes.

putting lombock before mapstruct work for me in annotationProcessorPaths section when defining plugin. I using springboot 2.4.1 and lombock 1.18.16 and mapstruct 1.4.1.FINAL

Rawi01 commented 3 years ago

@filiphr assumption is right and lombok has to create symbols/mirrors for the new methods and types. It works if you run lombok after mapstruct because mapstruct postpones mapper creation to a later round if it detects lombok. In a new annotation processing round javac recreates the type mirrors and the builder can be found by other annotation processors.

While working on this issue I noticed that the current soloution is somehow incomplete because it does not add type parameters to the method. It also seems to be a lot of work to always copy the type information by hand and it can get really tricky to do it right in more complex cases (@SuperBuilder...). I checked how Javac creates the existing information and found 3 classes that might be useful in this context: com.sun.tools.javac.comp.Enter, MemberEnter and TypeEnter. I tried to use them and it seems to work (with some NetBeans problem). @rzwitserloot Are there any know problems using these classes instead of doing everything ourself?

mjustin commented 3 years ago

I have not played around with it at all, so I do not know whether this truly is happening or if it is even related to this issue. That said, this answer on Stack Overflow asserts that the annotation ordering problem is due to Lombok manipulating files rather than creating it:

The problem is that the Lombok processor manipulates existing files instead of creating new ones, which it is not supposed to do.

Per the comments on the answer, this restriction is documented in the Filer class:

In general, processors must not knowingly attempt to overwrite existing files that were not generated by some processor

rzwitserloot commented 3 years ago

Hello folks who type '+1 for fixing this': Stop doing that. There's a thumbs up thing you can use.

rzwitserloot commented 3 years ago

@Rawi01 wrote: @rzwitserloot Are there any know problems using these classes instead of doing everything ourself?

Probably not. It's been so long, it's possible I tried that concluded it didn't work, but if memory serves, I just wrote the copy code because I thought 'eh, how hard could it be?' (and then generics says 'hi!' and I facepalm).

Netbeans is more generally problematic; I think it's acceptable if it's at the cost of netbeans, though I would then like to put something on the docket to at least meet netbeans halfway. Hopefully to just fix things so they work with netbeans, potentially that we resort to the old copying code if netbeans is detected.

One major issue there is that I don't currently have an easy setup to run lombok+netbeans together and e.g. add some breakpoints to see what's going on.

Rawi01 commented 3 years ago

One major issue there is that I don't currently have an easy setup to run lombok+netbeans together and e.g. add some breakpoints to see what's going on.

I added -J-agentlib:jdwp=transport=dt_socket,server=y,address=8899,suspend=y to the default options in <netbeans_folder>/etc/netbeans.conf and simply attached a remote debugger in eclipse. To apply any changes you have to recompile lombok and remove and add back the dependency.

Netbeans is more generally problematic; I think it's acceptable if it's at the cost of netbeans, though I would then like to put something on the docket to at least meet netbeans halfway. Hopefully to just fix things so they work with netbeans, potentially that we resort to the old copying code if netbeans is detected.

Disabling the whole copy code seems to work too, at least there are no errors and I also successfully run mapstruct after lombok in netbeans.

kaipengliu commented 3 years ago

@rzwitserloot I am one of the MapStruct team members. And I can guarantee you that the builder creation method, the builder class and it's methods cannot be seen during the annotation processing round. I debugged this and I can confirm that I can see the getters and setters, but not the builder. Are you creating the builder in the same time as the getters and setters?

If you want I can create a sample project where you can see what happens.

@filiphr I am very puzzled. In the compatibility scheme of lombok and mapstructs provided at the following URL https://mapstruct.org/documentation/stable/reference/html/#lombok I got a terrible result: in the automatically generated mapper class, no getter/setter methods are provided. I am using idea2020.03 lombok1.18.16 mapstruct1.4.2.Final What is even more puzzled is that when I rolled back the lombok version to 1.18.8, everything returned to normal

filiphr commented 3 years ago

I don't know what to tell you @kaipengliu. We have tests for this and it works correctly. This issue was closed on 18th of April 2021, the last release of Lombok is from 2nd of April 2021. Therefore, you can have problems with any of the current versions.

You need to wait for the Lombok team to do a new release or try the Lombok Edge version