#1681 Do not call finaliser method on return when @MappingTarget is not a builder

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
+            );
         }
 
         /**

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);
+}

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" );
+    }
+}

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;
+    }
+}

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 );
+        }
+    }
+}