diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java b/processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java index abd488e03..4e05ad5d6 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java @@ -27,7 +27,6 @@ import javax.tools.Diagnostic; import org.mapstruct.ap.internal.model.PropertyMapping.ConstantMappingBuilder; import org.mapstruct.ap.internal.model.PropertyMapping.JavaExpressionMappingBuilder; import org.mapstruct.ap.internal.model.PropertyMapping.PropertyMappingBuilder; -import org.mapstruct.ap.internal.model.common.BuilderType; import org.mapstruct.ap.internal.model.common.Parameter; import org.mapstruct.ap.internal.model.common.Type; import org.mapstruct.ap.internal.model.dependency.GraphAnalyzer; @@ -245,8 +244,11 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { ( (ForgedMethod) method ).addThrownTypes( factoryMethod.getThrownTypes() ); } - MethodReference finalizeMethod = getFinalizerMethod( - resultType == null ? method.getReturnType() : resultType ); + MethodReference finalizeMethod = null; + + if ( shouldCallFinalizerMethod( resultType == null ? method.getResultType() : resultType ) ) { + finalizeMethod = getFinalizerMethod(); + } return new BeanMappingMethod( method, @@ -261,18 +263,27 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { ); } - private MethodReference getFinalizerMethod(Type resultType) { - if ( method.getReturnType().isVoid() || - resultType.getEffectiveType().isAssignableTo( resultType ) ) { - return null; + private boolean shouldCallFinalizerMethod(Type resultType) { + Type returnType = method.getReturnType(); + if ( returnType.isVoid() ) { + return false; } - BuilderType builderType = resultType.getBuilderType(); - if ( builderType == null ) { - // If the mapping type is assignable to the result type this should never happen - return null; + Type mappingType = method.isUpdateMethod() ? resultType : resultType.getEffectiveType(); + if ( mappingType.isAssignableTo( returnType ) ) { + // If the mapping type can be assigned to the return type then we + // don't need a finalizer method + return false; } - return BuilderFinisherMethodResolver.getBuilderFinisherMethod( method, builderType, ctx ); + return returnType.getBuilderType() != null; + } + + private MethodReference getFinalizerMethod() { + return BuilderFinisherMethodResolver.getBuilderFinisherMethod( + method, + method.getReturnType().getBuilderType(), + ctx + ); } /** diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1681/Issue1681Mapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1681/Issue1681Mapper.java new file mode 100644 index 000000000..a8fcc2ec7 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1681/Issue1681Mapper.java @@ -0,0 +1,25 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.mapstruct.ap.test.bugs._1681; + +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; +import org.mapstruct.MappingTarget; +import org.mapstruct.factory.Mappers; + +/** + * @author Filip Hrisafov + */ +@Mapper +public interface Issue1681Mapper { + + Issue1681Mapper INSTANCE = Mappers.getMapper( Issue1681Mapper.class ); + + Target update(@MappingTarget Target target, Source source); + + @Mapping(target = "builderValue", source = "value") + Target update(@MappingTarget Target.Builder targetBuilder, Source source); +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1681/Issue1681Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1681/Issue1681Test.java new file mode 100644 index 000000000..812400be0 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1681/Issue1681Test.java @@ -0,0 +1,53 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.mapstruct.ap.test.bugs._1681; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mapstruct.ap.testutil.IssueKey; +import org.mapstruct.ap.testutil.WithClasses; +import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Filip Hrisafov + */ +@RunWith(AnnotationProcessorTestRunner.class) +@IssueKey("1681") +@WithClasses({ + Issue1681Mapper.class, + Source.class, + Target.class +}) +public class Issue1681Test { + + @Test + public void shouldCompile() { + Target target = new Target( "before" ); + Source source = new Source(); + source.setValue( "after" ); + + Target updatedTarget = Issue1681Mapper.INSTANCE.update( target, source ); + + assertThat( updatedTarget ).isSameAs( target ); + assertThat( updatedTarget.getValue() ).isEqualTo( "after" ); + } + + @Test + public void shouldCompileWithBuilder() { + Target.Builder targetBuilder = Target.builder(); + targetBuilder.builderValue( "before" ); + Source source = new Source(); + source.setValue( "after" ); + + Target updatedTarget = Issue1681Mapper.INSTANCE.update( targetBuilder, source ); + + assertThat( updatedTarget ).isNotNull(); + assertThat( updatedTarget.getValue() ).isEqualTo( "after" ); + assertThat( targetBuilder.getBuilderValue() ).isEqualTo( "after" ); + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1681/Source.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1681/Source.java new file mode 100644 index 000000000..384dbcca1 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1681/Source.java @@ -0,0 +1,22 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.mapstruct.ap.test.bugs._1681; + +/** + * @author Filip Hrisafov + */ +public class Source { + + private String value; + + public String getValue() { + return value; + } + + public void setValue(String value) { + this.value = value; + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1681/Target.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1681/Target.java new file mode 100644 index 000000000..5a392e196 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1681/Target.java @@ -0,0 +1,48 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.mapstruct.ap.test.bugs._1681; + +/** + * @author Filip Hrisafov + */ +public class Target { + + private String value; + + public Target(String value) { + this.value = value; + } + + public String getValue() { + return value; + } + + public void setValue(String value) { + this.value = value; + } + + public static Builder builder() { + return new Builder(); + } + + public static class Builder { + + private String builderValue; + + public String getBuilderValue() { + return builderValue; + } + + public Builder builderValue(String builderValue) { + this.builderValue = builderValue; + return this; + } + + public Target build() { + return new Target( builderValue ); + } + } +}