mapstruct / mapstruct-idea

An IntelliJ IDEA plugin for working with MapStruct
Other
142 stars 38 forks source link

Code completion doesn't work with lombok @SuperBuilder annotation #159

Closed PyotrSpetsnaz closed 8 months ago

PyotrSpetsnaz commented 1 year ago

Plugin Information

Problem Description

The code completion feature of the MapStruct Support plugin is not working properly when using the lombok @SuperBuilder (lombok.experimental.SuperBuilder) annotation.

Steps to Reproduce

  1. Create a new Java class.
  2. Add the lombok @SuperBuilder annotation to the class.
  3. Try to use code completion for any field in @Mapper annotation.

Expected Behavior

When using the @SuperBuilder annotation, the code completion feature should suggest the available fields.

Actual Behavior

The code completion feature does not suggest any fields or methods.

Screenshots and Code Snippets

// Class with annotation
@Getter
@Setter
@SuperBuilder
@NoArgsConstructor
public class Builder {
    private Integer bar;
}
// Class without annotation
@Getter
@Setter
public class WithoutBuilder {
    private Integer foo;
    private Builder innerBuilder; // To show that autocompletion works for inner builders
}
// Mapper with broken code completion
@Mapper
public interface BuilderMapper {
    @Mapping(target = "", source = "")
    Builder map(WithoutBuilder builder);
}

Doesn't work for class with @SuperBuilder:

image

But works for class without @SuperBuilder:

image

Even works for class with @SuperBuilder which inside a class without @SuperBuilder:

image

Additional Information

Generated class for @Mapping(target = "bar", source = "foo") to show that it is valid mapper:

@Generated(
    value = "org.mapstruct.ap.MappingProcessor",
    date = "2023-11-13T17:06:33+0700",
    comments = "version: 1.5.5.Final, compiler: javac, environment: Java 17.0.8 (BellSoft)"
)
public class BuilderMapperImpl implements BuilderMapper {

    @Override
    public Builder map(WithoutBuilder builder) {
        if ( builder == null ) {
            return null;
        }

        Builder builder1 = new Builder();

        builder1.setBar( builder.getFoo() );

        return builder1;
    }
}
thunderhook commented 1 year ago

Yes you're right. A minified reproducal without lombok:


import org.mapstruct.Mapper;
import org.mapstruct.Mapping;

class SuperBuilderTarget {

    private Integer bar;

    protected SuperBuilderTarget(SuperBuilderTargetBuilder<?, ?> b) {
        this.bar = b.bar;
    }

    public static SuperBuilderTargetBuilder<?, ?> builder() {
        return new SuperBuilderTargetBuilderImpl();
    }

    public static abstract class SuperBuilderTargetBuilder<C extends SuperBuilderTarget, B extends SuperBuilderTargetBuilder<C, B>> {
        private Integer bar;

        public B bar(Integer bar) {
            this.bar = bar;
            return self();
        }

        protected abstract B self();

        public abstract C build();

        public String toString() {
            return "SuperBuilderTarget.SuperBuilderTargetBuilder(bar=" + this.bar + ")";
        }
    }

    private static final class SuperBuilderTargetBuilderImpl
        extends SuperBuilderTargetBuilder<SuperBuilderTarget, SuperBuilderTargetBuilderImpl> {
        private SuperBuilderTargetBuilderImpl() {
        }

        protected SuperBuilderTargetBuilderImpl self() {
            return this;
        }

        public SuperBuilderTarget build() {
            return new SuperBuilderTarget( this );
        }
    }
}

class Source {
    public Integer foo;
}

@Mapper
interface BuilderMapper {
    @Mapping(target = "", source = "foo")  
    SuperBuilderTarget map(Source builder);
}

First quick analysis shows that this evaluates to false when the bar(...) method is picked, therefore this is ignored.

filiphr commented 1 year ago

We don't support Lombok @SuperBuilder in MapStruct. The example is a bit confusing because the target type is also called Builder.

If you look at the generated code you'll see that it is calling the setter on your target type

builder1.setBar( builder.getFoo() );

I would say the bug is that the plugin even tries to use the builder for the detection

thunderhook commented 1 year ago

We don't support Lombok @SuperBuilder in MapStruct.

We use it a lot in our company code. I'm pretty sure that this is getting picked up, because we don't have any constructors that mapstruct could use. But I'm going to double-check.

The inheritance builder pattern is a pain, and an ugly mess using generic hell, so we were really happy when we found the @SuperBuilder annotation that hides all that away.

The example is a bit confusing because the target type is also called Builder.

You're right, sorry about that.

If you look at the generated code you'll see that it is calling the setter on your target type

builder1.setBar( builder.getFoo() );

I would say the bug is that the plugin even tries to use the builder for the detection

Interesting, for me the (confusing 😬 ) reproducal above generates the mapper like this:

@Generated(
    value = "org.mapstruct.ap.MappingProcessor",
    date = "2023-11-26T23:25:35+0100",
    comments = "version: 1.5.0.Final, compiler: javac, environment: Java 17 (Oracle Corporation)"
)
class BuilderMapperImpl implements BuilderMapper {

    @Override
    public SuperBuilderTarget map(Source builder) {
        if ( builder == null ) {
            return null;
        }

        SuperBuilderTarget.SuperBuilderTargetBuilder<?, ?> superBuilderTarget = SuperBuilderTarget.builder();

        superBuilderTarget.bar( builder.foo );

        return superBuilderTarget.build();
    }
}

Copying the test classes from the unit test of the pull in a separate project also generates this for me:

@Generated(
    value = "org.mapstruct.ap.MappingProcessor",
    date = "2023-11-26T23:25:35+0100",
    comments = "version: 1.5.0.Final, compiler: javac, environment: Java 17 (Oracle Corporation)"
)
class AllMappingMapperImpl implements AllMappingMapper {

    @Override
    public UnmappedFluentTargetPropertiesData.Target mapWithAllMapping(UnmappedFluentTargetPropertiesData.Source source) {
        if ( source == null ) {
            return null;
        }

        UnmappedFluentTargetPropertiesData.Target.TargetBuilder<?, ?> target = UnmappedFluentTargetPropertiesData.Target.builder();

        target.testName( source.getName() );
        target.moreTarget( source.getMoreSource() );
        target.matching( source.getMatching() );

        return target.build();
    }
}

Could you please try again? 🙏 🙇