[BUG] import static some.pkg.SomeType.something; broken in javac only

[rzwitserloot]

Describe the bug
A non-star import static statement which imports a method (or field, or even an inner type) that is either:

• Made static by lombok but isn't explicitly marked as such, for example by @UtilityClass, and/or
• is generated by lombok, for example the builder() method by @Builder

works fine in eclipse and the intellij plugin, but when compiling with javac, you get an error as if lombok hadn't run at all (an error with the gist of 'that is not static' for statically imported things that are static due to the influence of @UtilityClass, and an error with the gist of 'that does not exist' for produced static members like a builder() method).

To Reproduce

package pkg;
@lombok.experimental.UtilityClass
public class Example {
    public void method() {}
}

package pkg;
import pkg.Example.method;

public class Using {
    public void foo() {
        method();
    }
}

And then: javac *.java

Version info:

• All versions of lombok, including at least v1.18.6
• All javacs, from 1.6 to 11 at least.

Workarounds:

A static star import does work, as does a non-static import. So, for the above example, either:

• Import (non-static) just Example, and replace calls to method() with Example.method()
• or... import static pkg.Example.*;

[rzwitserloot]

NB: This is extremely hard to fix, probably impossible; javac bombs out without even initializing annotation processors. It is technically more or less allowed to do this, even though it is not consistent with how javac otherwise operates, which makes it pretty much pointless to attempt to file a bug with openjdk to fix this on their end.

Also, this is an error of the kind that stops annotation processors from running. That means if this error occurs, lombok doesn't run at all, and thus you tend to get a blast of errors. That's also not something we can fix.

[rzwitserloot]

JEP 216 seems relevant, but this bug existed before JEP216 was delivered, and still exists afterwards. All it really means is that this import stuff is complex. It already felt near impossible to get this fixed at the openjdk side, and with convoluted JEPs muddying the waters, even more so.

[lars-sh]

As this seems to be hardly fixable for javac, while it's working for Eclipse and IntelliJ:
Is it possible to add a warning/error [Edit: inside Eclipse and IntelliJ] to the static import statement in case it is based upon a @UtilityClass annotated class and the element being import is not marked as static?

That way people might not be confused at the time they see such problems.

[rspilker]

Unfortunately, we don't have resolution, and even if we had, we wouldn't necessarily know what code was generated by which lombok construct, possibly leading to false positives.

[yshavit]

Sorry to repeat myself from a closed duplicate issue, but I really do believe that if this can't be fixed (which, I believe you that it can't), then the annotation should be deprecated. If it's possible to require the static keyword, then that's a better option than full deprecation. But the current situation -- that it's pretty easy to get into a position where everything just breaks, with error messages that are very hard to break down -- is a bad one. A build tool needs to be more rock solid than this.

[janrieke]

There is no single lombok annotation that causes this. For instance, it also affects @Builder, and you cannot mark @Builder as deprecated, because it's a non-experimental feature that is crucial to the majority of lombok users.

[Maaartinus]

@janrieke That's sadly true, but I guess, @yshavit meant @UtilityClass and I agree with him. The bug-hunting when you statically import Whatever.builder may be a pain, but @Builder has still its value. At the same time, this bug makes @UtilityClass as is rather worthless. You save writing one word per field and expose yourself and your team to risking losing time on this issue.

Unfortunately, we don't have resolution, and even if we had, we wouldn't necessarily know what code was generated by which lombok construct, possibly leading to false positives.

@rspilker I'd consider producing a warning on any static import of any method named like any standard static lombok-generated method. This covers neither @UtilityClass nor XArgsConstructor(staticName=...), but it makes the user aware of the problem. Obviously, it must be possible to switch this warning off in the config.

[yshavit]

I think @Maaartinus hit the nail on the head. @UtilityClass basically does two things: a private constructor and marking members as static. The first one of those, lombok already has an annotation for; and the second one is (a) easy without lombok and (b) unsolvably broken with lombok.

In addition, statically importing a builder() method is rare -- if I've ever done it, I can't remember. Statically importing members of a utility class happens all the time (guava Preconditions, JUnit asserts, etc.), and is basically the use case that static import was designed for. If it doesn't work with lombok utility classes, then imho they're more trouble than they're worth.

[buster]

In addition, statically importing a builder() method is rare -- if I've ever done it, I can't remember.

Where this bug hits me, is when you define a custom builderMethodName.
So, i'd like to replace "Car.builder().brand("BMW").build();" with "car().brand("BMW").build()".
You'd want to import Car.car but because of this issue you'll have to use Car.car(), which makes the custom builder method name useless for my usecase.

[mahtaran]

I feel like this should be better documented if it really is unsolvable. Warnings in the javadoc of annotations which generate methods or some sort of “Frequent Problems” section on the website. Since no errors appear in an IDE, perhaps the IDE plugins could generate an error if you statically import such a method? No clue if that is possible though.
EDIT: it is documented in the small print of the @UtilityClass annotation, but not on the @Getter page. (I was having this same issue with @Getters.)

[nils4cosee]

There is an issue in the lombok-intellij plugin (mplushnikov/lombok-intellij-plugin#291) with the suggestion to add an enhancement.

[quiram]

I'll share my experience in case it helps anyone else. This issue puzzled me because I was able to statically import lombok-generated constructors in a project, but not in another one. It turned out to be that, in the project that was working, I was using static import only in test code, while in the project that failed I was using static import both in prod and test code. I guess, for the former, prod code was fully compiled first, so by the time test code was compiled the static constructor was built and ready to be statically imported. I hope this saves some head-scratching for someone else.

[rzwitserloot]

@buster we added the 'name the builder method differently' specifically to support that style, i.e. this:

import java.util.List;

List.of("a", "b", "c");

versus this (imagine that List's of method was named 'list' instead):

import static java.util.List.list;

list("a", "b", "c");

I do think the first form (of) is significantly more common than the second form, and the first form plays far better with IDEs (you can autocomplete the List class and go from there. At least in eclipse, to support the second form you'd have to explicitly add the list method to a global list of static methods to offer autocomplete/auto-add-the-import for. Nevertheless, we did want to support it....

and, yeah, then you'd run into this problem.

I can see how this needs issue needs some thought.

[quiram]

Whilst I won't dispute that the form List.of() may be generally more common than list() (I don't have data to base the dispute on), I'd say that, at least in certain circles, the second form is definitely more popular. Coming from a Scala shop, the first form feels quite alien to me; but I appreciate this is personal opinion.

I guess what I'm trying to say is that, if there was a way to support the second case easily, I'd very much support it.

[lars-sh]

I won't get into that discussion as there are examples for both ways, such as Stream.of(...) and Arrays.asList(...).

Though, in #2044 (comment) I suggested a workaround to simplify handling these kind of problems:

As this seems to be hardly fixable for javac, while it's working for Eclipse and IntelliJ:
Is it possible to add a warning/error [Edit: inside Eclipse and IntelliJ] to the static import statement in case it is based upon a @UtilityClass annotated class and the element being import is not marked as static?

That way people might not be confused at the time they see such problems.

Is that feasible?

[kutsal]

This works, however redundant with

@UtilityClass
class Foo {
    // Insert redundant static modifier here
    public static final String BAR = "baz";
    // other things
}

import static ...Foo.BAR;

class Etc {
// ...
    something(BAR, etc);
// ...
}

[rzwitserloot]

We can't (easily) add a warning either. Given this source file:

import static com.foo.YourClass.someMethodThatIsStaticDueToUtilityClass;

public class Foo {
  void method() {
    someMethodThatIsStaticDueToUtilityClass();
  }
}

We can't know that this is going to break in javac, we'd need to see the source of com.foo.YourClass which lombok doesn't have (it's resolution and in fact even worse than that, as now we need to resolve non-public details of the target). In theory we can - the crux of the problem is that this is only an issue if com.foo.YourClass is in the same compilation run, and the relevant compilation run is specifically javac, which may not match whatever your IDE is doing. Thus, at best we can do a heuristic scan and determine likely cases. That kind of best effort stuff is the domain of linting tools and demands a way to disable the check which is well beyond what is possible.

Therefore I see only 3 steps forward:

• Just leave it as is; if we do rewrite lombok as a javac plugin, which we are considering, we may be able to fix this problem straight up. Until then, we won't be able to.
• Marking @UtilityClass as not recommended: It would always generate a warning (or possibly we just deprecate it, which you can turn off with @SuppressWarnings("deprecation"), I believe), which you can disable with lombok.config.
• Update the documentation to highlight the problem.

I'm not sure which route forward is the best option from here. I'm tempted to combine #1 and #3: Anybody who makes a habit of using star imports and telling their IDE to replace static imports with star imports simply doesn't suffer from this problem, so it feels bad to force them into working with a 'not recommended' feature that blasts warnings until you explicitly tell lombok to cease with the warning bells. That's the gist of the documentation I intend to write: This feature is a really bad idea unless you adopt the style maxim of always star-importing when using static imports, at least, of utility classes.

[lars-sh]

Thanks for explaining this in that much detail @rzwitserloot. It finally makes sense to me. I agree with you, # 3 (updating the documentation) could be a good short-term solution for this.

[rzwitserloot]

It was already documented in the fine print. But, updated status to negative. I don't think this feature is getting out of experimental as long as this problem persists.