[fixes #3116] Add multi round support for mapstruct

[kanchev1]

Report to MapStruct that a type is completed when there aren't any Lombok annotations left on it. Lombok annotations are removed whenever a class is processed. This way, annotations which require multiple rounds to process are also correctly handled, and MapStruct processing will be delayed until Lombok completely finishes processing required types.

Fixes issues with e.g. @FieldNameConstants and @builder (see #3116)

[rzwitserloot]

Thanks, @kanchev1 ! I just restored the 'hey if lombok is entirely disabled, just tell mapstruct we are definitely not going to modify that AST anymore' and applied the house style. Good work.

[rgrunber]

Hi all, I think this commit introduced a regression in Lombok.

I have the following steps to reproduce :

1. git clone /~https://github.com/dxaraujo/vscode-java/ (this is a sample project that shows the issue)
2. Enter the project and set the lombok.version property to 1.18.35 in the pom.xml.
3. Now go into /~https://github.com/projectlombok/lombok and build against the commit before this one, 268b614 (ant dist).
4. Run mvn install:install-file -Dfile=dist/lombok-1.18.35.jar -DgroupId=org.projectlombok -DartifactId=lombok -Dversion=1.18.35 -Dpackaging=jar, which should install the buld artifact into the local Maven repository.
5. Go into the "vscode-java" sample project above and run mvn clean verify. It will use the Lombok we just built, and succeed.
6. Go back to the lombok project and checkout and build this commit, 04c9755 (ant dist).
7. Run mvn install:install-file -Dfile=dist/lombok-1.18.31.jar -DgroupId=org.projectlombok -DartifactId=lombok -Dversion=1.18.35 -Dpackaging=jar (Note the jar is lombok-1.18.31.jar but we'll treat it as 1.18.35 to simplify things)
8. Perform (5) again, and you'll see the build fails with :

[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] /home/rgrunber/sample-projects/issue-3836/src/main/java/com/vscode/java/mapper/UserMapper.java:[9,8] No implementation was created for UserMapper due to having a problem in the erroneous element java.util.ArrayList. Hint: this often means that some other annotation processor was supposed to process the erroneous element. You can also enable MapStruct verbose mode by setting -Amapstruct.verbose=true as a compilation argument.
[INFO] 1 error
[INFO] -------------------------------------------------------------
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE

Update: Filed #3777 for more visibility.

[testforstephen]

@rgrunber Could you open an issue for this in lombok repository?

[rzwitserloot]

This issue will now be reverted, and, we think, this approach of fixing it is a dead end. As far as we can tell:

1. The 'old' version has lombok itself track whether it has run or not, and uses that to answer mapstruct when it asks lombok: "Hey, is this type complete?". There are 2 problems with this; [A] lombok has its own rounds system and perhaps its an issue that we start answering 'yeah we are done' to the question right away, perhaps we should wait until our rounds are done (but this is only relevant if mapstruct asks us in the middle of lombok's processing, and I don't think it does, so this is probably irrelevant), and [B] if the annotation processing part of javac itself causes multiple rounds, which usually happens when APs generate new source files, then the newly generated files are immediately considered as 'yeah it is done' by lombok because lombok has already ran a full cycle. Even if that particular is completely new, and contains unprocessed lombok stuff.
2. The 'new' version ignores any bookkeeping that lombok does and instead simply checks if there is lombok stuff in the type that is being asked about. But this is impossible.

The impl simply asks the TypeMirror representing type X in mapstruct's question to lombok: "Hey, is type X complete; i.e. are you done adding methods and fields and such to it?" - do you have any annotations on you (on the type) that are lombok annotations?

This isn't complete! Lombok can trigger on annotations on local variables (such as @Cleanup), as well as var/val. Yes, right now those 'shows up in method bodies' lombok annotations don't affect the completeness of the type (they don't add new methods), but those aren't handcuffs we wanna put on. We don't want to close the door on ever doing that.

That's why its a dead end. Yes, the TypeMirror can be recursed to scan for lombok-related annotations anywhere inside it, but that's still not enough; TypeMirror does not allow you to drill all the way into method bodies, but it has to, to properly answer the question "is lombok completely done with this?"

Separately, lombok might want to leave some lombok artefacts behind. We already do that: @lombok.NonNull can remain; it doesn't currently conflict with kanchev's PR because NonNull is not a thing that can exist on the type itself, but that will immediately throw the PR's approach into chaos if you add a mechanism to drill into the content of the TypeMirror.

Hence, the strategy chosen seems fundamentally unworkable.

We think the following strategy would work:

Instead of 'globally' tracking "has lombok run yet?", track it per source file (in AST terms, per compilation unit). If lombok has completed an entire cycle on some source file, then that type is complete. It's not about whether lombok stuff remains in it (it might, stuff like @NonNull may remain), it's about whether lombok considers all lombok-related stuff in it as 'processed'. Assuming lombok is the only AP around that modifies types themselves, then a single cycle is sufficient to deem a compilation unit as 'as far as lombok is concerned, it is complete'.

I guess storing some sort of hashset containing every type being compiled in one go is possibly a slight memory hog, but it's probably fine. In the end shoving 20k strings each ~40 chars long into a hashset isn't all that much memory these days.