Builder nested class not recognised by IntelliJ 2020.3.3 when called 'Builder'

[ruurd]

Short description

Update to intellij 2020.3.3 ultimate Open Java class that references XXX.Builder Observe red squiggles under Builder and IntelliJ indicated an error

Expected behavior

XXX.Builder nested class should be recognised

Version information

IntelliJ Ultimate 2020.3.3 OpenJDK Runtime Environment Zulu11.45+27-CA (build 11.0.10+9-LTS) macOS 11.2.3 (20D91) MacBookPro17,1 Apple M1 bundled 203.7717.56 lombok-1.18.16 through spring-boot-dependencies-2.4.2 through spring-boot-starter-parent-2.4.2

Steps to reproduce

1. Update to IntelliJ Ultimate 2020.3.3
2. Clear and Set lombok.builder.classname to Builder in lombok.config
3. Open Java class that references a Builder nested class
4. Observe squiggles.

Sample project

Please provide a sample project that exhibits the problem. You should also include .idea folder so we can inspect the settings.

ij-2020.3.3-lombok.zip

• [X] Sample project provided
• [X] I am able to reproduce this error on the sample project by following the steps described above

Additional information

Not specifying builder class name as Builder solves this problem however I do want to be able to call the nested class of XXX XXX.Builder instead of XXX.XXXBuilder because the second XXX is superfluous. I have encountered this problem in earlier incarnations of Lombok together with IntelliJ but 2020.3.2 did not flag this as an error.

Stacktrace

Not applicable

[XComp]

I observed the same issue with IntelliJ IDEA 2020.2.4 (Community Edition) Build #IC-202.8194.7, built on November 24, 2020 Runtime version: 11.0.9+11-b944.49 x86_64 VM: OpenJDK 64-Bit Server VM by JetBrains s.r.o. macOS 10.15.7

Lombok Plugin Version: 0.34-2020.2

The code compiles. I am able to run unit tests. It's just that for classes that are annotated with @Builder(builderClassName = "Builder") calling builder() on that class would mark it as an compilation error.

[XComp]

I observed the same issue with IntelliJ IDEA 2020.2.4 (Community Edition) Build #IC-202.8194.7, built on November 24, 2020 Runtime version: 11.0.9+11-b944.49 x86_64 VM: OpenJDK 64-Bit Server VM by JetBrains s.r.o. macOS 10.15.7

Lombok Plugin Version: 0.34-2020.2

The code compiles. I am able to run unit tests. It's just that for classes that are annotated with @Builder(builderClassName = "Builder") calling builder() on that class would mark it as an compilation error.

FYI: Manually installing Lombok Plugin version 0.33 fixed the problem for me.

[ruurd]

Using @Builder(builderClassName="BuildMe") tricked Lombok in behaving but I really dislike that. I particularly like the possibility of naming a nested builder class just Builder because OuterClazz.Builder is exactly the correct name for a builder class that builds OuterClazz instances...

And then again - as you noticed, lombok-plugin-0.33 does fix the problem so this might be a symptom of some deeper problem in the latest version.

[haoalisterw]

Same issue here and the behavior is just like what @XComp and @ruurd described.

@lombok.Value
public class BugClass {
    String value1;
    String value2;

    @lombok.Builder(builderClassName = "Builder")
    public BugClass(String value) {
        this.value1 = value;
        this.value2 = value;
    }
}

public class TestExample {
    public void testExample() {
        BugClass bug = BugClass.builder() // This gives "Cannot access xxx" warning, but everything works fine except the warning.
                .value("hey")
                .build();
    }
}

@lombok.Value
public class BugClass {
    String value1;
    String value2;

    @lombok.Builder(builderClassName = "OtherBuild")
    public BugClass(String value) {
        this.value1 = value;
        this.value2 = value;
    }
}

This will give no warning.

[fhomasp]

I observed the same issue with IntelliJ IDEA 2020.2.4 (Community Edition) Build #IC-202.8194.7, built on November 24, 2020 Runtime version: 11.0.9+11-b944.49 x86_64 VM: OpenJDK 64-Bit Server VM by JetBrains s.r.o. macOS 10.15.7 Lombok Plugin Version: 0.34-2020.2 The code compiles. I am able to run unit tests. It's just that for classes that are annotated with @Builder(builderClassName = "Builder") calling builder() on that class would mark it as an compilation error.

FYI: Manually installing Lombok Plugin version 0.33 fixed the problem for me.

Probably stating the obvious, but this won't work with IntelliJ 2020.3.x.

[vominhtrius]

Same issue with Intellij 2020.3.3 Community

[fhomasp]

Just updated to IntelliJ Idea Ultimate 2021.1. The issue is still there.

[ddiehl]

I just updated Ultimate 2021.1 and problem went away. Thank you.

[v1nc3n4]

I just updated Ultimate 2021.1 and problem went away. Thank you.

On my side, I can also confirm issue seems to be fixed after upgrading to Ultimate 2021.1.

[topr]

The difference in above reports about 2021.1 seems to be due the fact that one issue was solved but a new one emerges. I may yet to confirm that for myself but by a report of a teammate 2021.1 ignores lombok.config property lombok.builder.className.

So if it happens that inline annotation @Builder(builderClassName="Builder") is used on a class directly the issue existing in 2020.3 no longer manifests itself in 2021.1.

However if it is set at lombok.builder.className=Builder via lombok.config then 2021.1 ignores it. Any name is ignored, not only when it's set to "Builder".

@fhomasp Could you confirm if builder class name is set via lombok.config in your case?

[v1nc3n4]

Hi @topr In my case, I'm using the lombok.config file hereafter at the root directory of my project:

config.stopBubbling=true
lombok.log.fieldname=LOGGER
lombok.builder.className=Builder

The Builder class is correctly generated, as well as the static builder() method after upgrading to 2021.1. Hope this helps. BR

[fhomasp]

Upon closer examination I notice that most of the issues are ok now. However, I do still get into trouble using

@Builder(builderClassName = "Builder")

on a constructor.

[topr]

@v1nc3n4 thanks, to clarify. It's always been generated all just fine. Only IntelliJ got it in red not being able to recognise. I mean, build works both with Gradle/Maven and even with IntelliJ itself but builder class and method are in red / no auto-complete, etc.

@fhomasp there you go, I can confirm for me on 2021.1 also it is the case constructor annotated with @Builder isn't working - IDE does not recognise neither builder class or builder() method.

It goes as follows:

• name specified as "Builder" at lombok.config and constructor annotated with @Builder :x:
• name specified as "Foo" at lombok.config and constructor annotated with @Builder :heavy_check_mark:
• name specified as "Foo" as @Builder annotation param and constructor annotated with it :heavy_check_mark:
• name specified as "Builder" as @Builder annotation param and constructor annotated with it :x:
• name specified as "Builder" at lombok.config and class annotated with @Builder :heavy_check_mark:
• name specified as "Foo" at lombok.config and class annotated with @Builder :heavy_check_mark:
• name specified as "Foo" as @Builder annotation param and class annotated with it :heavy_check_mark:
• name specified as "Builder" as @Builder annotation param and class annotated with it :heavy_check_mark:

In short it does not work properly only when constructor is annotated and builder class name is "Builder", regardless of whether the name is set at the annotation or at lombok.config file.

Checked with IntelliJ 2021.1 and Lombok 1.18.20.

Downgrading to 2020.2 for the time being :roll_eyes:

[ruurd]

OK, question - is there a reason to annotate a constructor with the @Builder annotation? That seems odd to me. One would think that a builder is for a class instance so why not annotate the class instead?

IOW - what is the use case for annotating a constructor with Builder?

Just askin...

[stefnoten]

@ruurd @Builder on a constructor builds the class using that constructor instead of a default constructor. So it makes sure you can't build invalid objects. The constructor may validate business rules, may set up multiple fields based on a single parameter in a valid way, etc. Skipping this completely breaks encapsulation and all the guarantees given by it (unless your validations are limited to @NonNull on fields which lombok still validates).

For the domain layer, our team's rule is to use @Builder only ever on constructors. For mapping layers (e.g. DTOs) we use it at the class level, since those classes typically don't have many constraints. However, even in that case, maybe the better strategy is to build domain objects (constructor-level @Builder) and use those as parameters to construct DTOs so that you e.g. don't build instances in test code that can never occur in practice.

[ruurd]

@stefnoten Ah I see. Thank you for that.

This has been something nagging in the back of my mind for some time. I would prefer that the builder would do the sanity checks before even attempting to build the instance in the first place - preventing the chance that halfway a constructor decides the instance cannot be constructed because then what? Exception? Zombie instance?

My preference is that a builder builds a sane instance or does not attempt construction in the first place. I.e. I would rather not create half-baked instances or misuse a constructor as a validation method possibly yielding half baked instances. But that might be my C++ background.

I encountered annotating a method with @Builder. Maybe that is an approach to what I want...

[2is10]

@ruurd In Java, validation in a constructor (throwing an exception like IllegalArgumentException or NullPointerException) is standard practice. For example:

• new java.util.Properties(int)
• new java.net.InetSocketAddress(String, int)
• new java.io.File(URI)

It’s also fine to declare a factory method that validates and throws, such as:

• java.lang.Integer.valueOf(String)
• java.util.regex.Pattern.compile(String, int)

There’s no risk of callers ending up with a partially constructed or zombie instance.

It would be difficult to have an auto-generated Builder class do validation for a few reasons. I’ll avoid going into details here since it isn’t really relevant to this issue.

Here is the complete documentation for Lombok’s Builder, in case it’s helpful to whomever is working on a fix for this issue. Thanks in advance!

[ruurd]

@2is10 harumpf, all examples of very old code IMO, the latest version is 1.4 and a number of others are from 1.0 - so almost 20 years old. All the examples you're giving seem to be precondition checks. But we digress even if this is a very nice discussion to have. So

If you use @Builder annotation on a method or a constructor and you set the name of the builder class to Builder then the following pops up during compilation with JDK-11 and JDK15:

incompatible types: nl.bureaupels.bugs.Model.Builder cannot be converted to java.lang.annotation.Annotation

which is logical even if unfortunate.

ij-2020.3.3-lombok.zip

[2is10]

@ruurd That new Properties(int) constructor was added in Java 10 (released in Mar 2018, 3 years ago); it’s not in Java 9. I was just looking for well-known examples to help you understand the pattern. I could also point to examples of new throwing constructors and factory methods in Java 15 and Java 16. In constructors, sanity checks, precondition checks, and argument validation all amount to the same thing.

The compile error you’ve mentioned is due to the generated Builder class shadowing lombok.Builder. When @Builder is used on a constructor or method to generate a builder class named Builder, one must qualify it with its package name: @lombok.Builder(builderClassName = "Builder").

[topr]

@ruurd: short answer about why would a constructor be the target of the @Builder annotation is that ultimately a class in Java is constructed - no surprise - by a constructor :trollface:

Constructor is what a builder (regardless of whether Lombok's generated or not) is invoking in the end. It is only a convenience convention of Lombok that when there is no constructor to annotate, then a private constructor is generated anyway. As per the @Builder javadocs:

If a member is annotated, it must be either a constructor or a method. If a class is annotated, then a private constructor is generated with all fields as arguments

Ref.

Why would anyone have the flexibility to not rely on the implicit generated constructor? There are cases where a class has more than one constructor with different arguments - each may or may not have a corresponding builder or a sole builder needs to know which one to invoke. Constructor may have some extra logic or arguments conversion / validation. Static factory methods may be annotated with @Builder for similar reasons too.

All of that is not really in scope of the issue here. It has been working and now it does not. It is an accidental breaking change - a bug. Hence fixing it seems like a relevant aim.

Disallowing to annotate constructors/factory methods with @Builder perhaps may be discussed separately as a breaking change and major shift in the library itself. A corresponding issue may be raised at the Lombok's GitHub if you think it would be a good thing to address. Lombok isn't broken though, the IDE plugin is. Some Lombok features are no longer supported as they used to be :bug:

[topr]

Is there a chance for a fix on this one, please? I am still locked on the 2020.2.4 but now the Lombok plugin has got upgraded to the 0.34-2020.2 version on it and its broken too :sob:

BTW. how can I downgrade a plugin?

[mihxil]

I'd like to know too. If this will not be fixed, it might be easier to start changing code than keep clicking away the update suggestions of IntelliJ....

[alexejk]

You can download older versions either from https://plugins.jetbrains.com/plugin/6317-lombok/versions or https://github.com/mplushnikov/lombok-intellij-plugin/releases and manually install it from ZIP.

A PR fixing the issue is welcome :)

[topr]

@alexejk thanks for the tip. ~Do we know which is the last version without the bug?~ Ok, this has been answered above: the version 0.33 is the last working one.

@mihxil

it might be easier to start changing code than keep clicking away the update suggestions of IntelliJ...

Might be true for some projects, depending on a codebase. I've got this issue manifesting on a repo which has about 200 @Builder annotated classes and it's not the only repo in the organisation affected. Plus there is some older not lombokized builder classes following the same pattern so that would either break project convention or these would need to be renamed too.

Also, if I simply change the lombok.builder.className=Builder to something else, this is going to break compilation in so many places and require so many manual editing to "fix", I don't even want to see managers' faces while we plan to burn project time for doing so.

There is yet a remaining question: change it to what actually. SomeClassName.Builder is a widely recognised industry standard. Should a developer invent some artificial name as a workaround to an IDE plugin bug :roll_eyes: What would that be... smth like lombok.builder.className=Buildah or perhaps ...=Builderrr? :trollface:

Sorry, that bit of bitterness in the last paragraph is not aimed at you by any means. Just to show a general point.

[mihxil]

@topr Sorry, I was a bit sarcastic too. Though in our case it would be probably feasible to just drop the configuration and conform to defaults, which would probably have prevented the hassle in the first place. There would be no need to invent anything, just the readiness to accept SomeClassName.SomeClassNameBuilder.

[topr]

@mihxil you are right, however the default has an unnecessary name redundancy which is preferable to avoid as it carries no extra information. Nested classes need no prefix their names with parent class name as they are already contained in it. In short SomeClassName.SomeClassNameBuilder is worse SNR.

I am pretty sure you knew that already. Just stating the obvious.

[2is10]

The redundancy of Something.SomethingBuilder can be avoided if you import the inner builder class directly, as in:

import some.package.Something;
import some.package.Something.SomethingBuilder;
...

SomethingBuilder builder = Something.builder();

Also, in my experience, most uses of a builder don’t require referencing the builder's class directly—especially if var (introduced in Java 10) is used for local variables.

Something something = Something.builder().foo(0).bar(true).build();

var builder = Something.builder(); // requires Java 10

When this issue was filed, my company’s codebase had 2,300+ Lombok builders, about 500 of which used the name "Builder" instead of Lombok’s default naming scheme. I pretty quickly reduced that 500 to about 200 by recognizing that 1) the builder class names usually weren’t used anywhere, and 2) var can often eliminate the need to reference the builder’s class name. The remaining 200 will be more work to eliminate since we’ll need to change Something.Builder to SomethingBuilder and add imports. Also, uses of the builder type within the containing class (Something in this example) become more verbose (Builder → SomethingBuilder), and there’s no uncontroversial way to get around that.

I also found a benefit to avoiding widespread use of the inner class name Builder in large codebases. Usage searches and automated refactors (such as method renames, inlines, or signature changes) in IntelliJ that involve any class named Builder can require much more time and/or memory if the codebase has many inner classes named Builder. One "Find usages" operation on a Builder method was reliably taking 12-15 seconds across several trials on my system. When I increased IntelliJ’s max heap from 2GB to 4GB, that decreased to 1.5 seconds. Accepting Lombok’s default naming scheme (SomethingBuilder) made the usage search nearly instantaneous even with the smaller max heap.

All that said, (my company and) I would really appreciate a fix.

[topr]

A PR fixing the issue is welcome :)

@alexejk given that:

Breaking News: Starting with IntelliJ version 2020.3 lombok plugin will be integrated and included in IntelliJ by default!.

...is this repo still the relevant place to discuss the issue and apply a fix or has the upstream been relocated? I see no tags/releases here for the IDEA 2021.x

@2is10 thank for the tips.

It's redundant already at the point of declaration, not only usages and that cannot be fixed. Consider:

1. import static some.package.SomethingBasedOnSomethingElseAboutSomethingFactory.Builder;
2. import static some.package.SomethingBasedOnSomethingElseAboutSomethingFactory.SomethingBasedOnSomethingElseAboutSomethingFactoryBuilder;

The example class has a really long name on purpose to emphasize SNR degradation. Most of classes might not have names as long but rarely are a single word either. Anyway, is any of these two lines above carrying more information than the other? :roll_eyes:

Yet another example of an unavoidable redundancy:

1. @Builder
   class FooBarPlentyOfSomethings {
   
       // some fields/constructor declarations go here...
   
       static Collector<SomeElement, FooBarPlentyOfSomethingsBuilder, FooBarPlentyOfSomethings> toFooBarPlentyOfSomethings() {
           return Collector.of(
                   FooBarPlentyOfSomethingsBuilder::new,
                   FooBarPlentyOfSomethingsBuilder::element,
                   FooBarPlentyOfSomethingsBuilder::combine,
                   FooBarPlentyOfSomethingsBuilder::build
           );
       }
   }
   

2. @Builder
   class FooBarPlentyOfSomethings {
   
       // some fields/constructor declarations go here...
   
       static Collector<SomeElement, Builder, FooBarPlentyOfSomethings> toFooBarPlentyOfSomethings() {
           return Collector.of(Builder::new, Builder::element, Builder::combine, Builder::build);
       }
   }
   

Anyway, it's not about listing examples. Surely there are ways to decrease the pain, thanks for pointing some good ones out. The main point, though, is: altering correctly working codebase due to a bug in an editing tool isn't exactly a solution. Not to mention it is a waste of time. I know, you know that :grin:

Although, I don't really face the slow search usages on builders, I must say I was tempted by the vision of speeding up the IDE responsiveness. Appreciate this insight, that's a productive one. I am trying to adapt the project I work on to the lombok's default and boy... it's cumbersome. Already modified 64 files and still got over 100 compilation errors (it trims at 100 so not sure how many more is there left). Need to park that, the actual work is waiting... :disappointed:

[mihxil]

according to https://youtrack.jetbrains.com/issue/IDEA-262693 this issue will be more fully fixed in 2021.2

[anuragagarwal561994]

In my case in he annotation processing using Obtain from classpath works, however it should be taken care of by default.