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

Lombok does not play nice with Error Prone #2605

Closed
sergeykad opened this issue Oct 12, 2020 · 5 comments
Closed

Lombok does not play nice with Error Prone #2605

sergeykad opened this issue Oct 12, 2020 · 5 comments

Comments

@sergeykad
Copy link

sergeykad commented Oct 12, 2020

Running Error Prone on classes with Lombok annotations (e.g. @Data, @Slf4j) crashes compilation.

Here is an example:
google/error-prone#1700

@Rawi01
Copy link
Collaborator

Rawi01 commented Dec 31, 2020

There are two problems here: Lombok does not update the end position table and Error Prone does not skip the generated code. The first one can be fixed but that unfortunately only replaces the exception with an warning/error (at least in some cases). To also remove that one Error Prone has to skip generated code. There is a command-line flag -XepDisableWarningsInGeneratedCode for that which can be used together with lombok.addJavaxGeneratedAnnotation = true but that will only work if you use Java <= 8 or if you add javax.annotation.Generated to your classpath.

A possible solution is to add javax.annotation.Generated for Java <= 8 and javax.annotation.processing.Generated for Java 9+.

@sergeykad
Copy link
Author

sergeykad commented Dec 31, 2020

@Rawi01 Adding Java @Generated annotation will not help, since it has @Retention(SOURCE).
In any case, the issue is that currently build crashes with exceptions instead of producing Error Prone messages. I understand from your comment that this is something that can/should be fixed in Lombok.

@rzwitserloot
Copy link
Collaborator

This has to be fixed by error-prone, perhaps try to file a feature request there / search for an existing one to subscribe to. There is nothing we can do, short of extensive hackery of error-prone's internals (well beyond what's reasonable / possible on our side). They need to add the ability to mark some annotation as indicating: "Skip this method, entirely", and then you can tell lombok to generate this annotation.

If such a facility already exists - can you reply to this issue with some more details about how to configure it (or, alternatively, perhaps there is e.g. @com.google.errorprone.Skip, and lombok can generate that annotation - then reply with the docs on this feature); we could add a page on the website that explains how to set this up, that sounds like a fine addition.

@Rawi01
Copy link
Collaborator

Rawi01 commented Jan 2, 2021

Lombok can (and should) add new fields/methods/types to the end position table (Javac.storeEnd).

@sergeykad Why should @Retention(SOURCE) be a problem for a compiler plugin like Error Prone? If it is useless the flag would not exist.

@rzwitserloot
Copy link
Collaborator

Lombok can (and should) add new fields/methods/types to the end position table (Javac.storeEnd).

Probably, but it won't fix the errorprone situation; at best, combining errorprone+lombok without errorprone having facilities to ignore generated methods results in errorprone raising a ton of irrelevant flags on lombok code (irrelevant, in that none of them are bugs or leaks or otherwise problematic).

About javax.annotation.Generated: It is a dumb and useless annotation. Partly because of that idiotic retentionlevel (source-level linting tooling can use it, but there's plenty of class-level linting tools around, they can't use this, not to mention other class-level tools that could use 'this code is generated' information), but mostly because during jigsaw this annotation got effectively backwards broken, so the few people and tools that generated it (like lombok) had to break backwards compatibility too and stop generating it.

I hate this annotation. It is beyond clear that the shepherds of it (oracle/openjdk) cannot be trusted with it. I strongly advocate that all libraries involved stop using it. The only reason lombok can be coerced into generating it at all is because we used to, and we wanted to give an easy upgrade path to those users of lombok who have integrated it into their projects and now cannot easily back out of its use.

Thus: We support it, whilst advocating in no uncertain terms that this annotation is bad and should never be used.

As a followup, that does mean life is so much harder for communicating the notion of 'generated'. I don't like that errorprone doesn't have a skip annotation, but in their defense, had they 'joined the bandwagon' and built in support for javax.annotation.Generated then errorprone+lombok still wouldn't actually work out of the box, and potentially they'd be in the same boat (where Generated causes issues when upgrading from 8 to a jigsaw-supporting JDK with broken Generated). Thus, it is understandable, perhaps, that errorprone has no such annotation nor support for it. Nevertheless, they're going to have to build it or lombok+errorprone is never going to be a nice experience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants