#1752 Always return mapping target when using update methods with return

processor/src/main/resources/org/mapstruct/ap/internal/model/BeanMappingMethod.ftl

@@ -20,7 +20,7 @@
     </#list>
     <#if !mapNullToDefault>
     if ( <#list sourceParametersExcludingPrimitives as sourceParam>${sourceParam.name} == null<#if sourceParam_has_next> && </#if></#list> ) {
-        return<#if returnType.name != "void"> null</#if>;
+        return<#if returnType.name != "void"> <#if existingInstanceMapping>${resultName}<#if finalizerMethod??>.<@includeModel object=finalizerMethod /></#if><#else>null</#if></#if>;
     }
     </#if>
 

processor/src/main/resources/org/mapstruct/ap/internal/model/IterableMappingMethod.ftl

@@ -16,8 +16,7 @@
     </#list>
     if ( ${sourceParameter.name} == null ) {
         <#if !mapNullToDefault>
-            <#-- returned target type starts to miss-align here with target handed via param, TODO is this right? -->
+            return<#if returnType.name != "void"> <#if existingInstanceMapping>${resultName}<#else>null</#if></#if>;
-            return<#if returnType.name != "void"> null</#if>;
         <#else>
             <#if resultType.arrayType>
                 <#if existingInstanceMapping>

processor/src/main/resources/org/mapstruct/ap/internal/model/MapMappingMethod.ftl

@@ -16,7 +16,7 @@
     </#list>
     if ( ${sourceParameter.name} == null ) {
         <#if !mapNullToDefault>
-            return<#if returnType.name != "void"> null</#if>;
+            return<#if returnType.name != "void"> <#if existingInstanceMapping>${resultName}<#else>null</#if></#if>;
         <#else>
             <#if existingInstanceMapping>
                  ${resultName}.clear();

processor/src/main/resources/org/mapstruct/ap/internal/model/StreamMappingMethod.ftl

@@ -17,8 +17,7 @@
     </#list>
     if ( ${sourceParameter.name} == null ) {
         <#if !mapNullToDefault>
-            <#-- returned target type starts to miss-align here with target handed via param, TODO is this right? -->
+            return<#if returnType.name != "void"> <#if existingInstanceMapping>${resultName}<#else>null</#if></#if>;
-            return<#if returnType.name != "void"> null</#if>;
         <#else>
             <#if resultType.arrayType>
                 <#if existingInstanceMapping>

processor/src/test/java/org/mapstruct/ap/test/array/ArrayMappingTest.java

@@ -123,15 +123,15 @@ public class ArrayMappingTest {
     }
 
     @ProcessorTest
-    public void shouldMapTargetToNullWhenNullSource() {
+    public void shouldReturnMapTargetWhenNullSource() {
-        // TODO: What about existing target?
 
         ScientistDto[] existingTarget =
                 new ScientistDto[]{ new ScientistDto( "Jim" ) };
 
         ScientistDto[] target = ScienceMapper.INSTANCE.scientistsToDtos( null, existingTarget );
 
-        assertThat( target ).isNull();
+        assertThat( target ).isNotNull();
+        assertThat( target ).isEqualTo( existingTarget );
         assertThat( existingTarget ).extracting( "name" ).containsOnly( "Jim" );
     }
 
@@ -143,6 +143,7 @@ public class ArrayMappingTest {
         boolean[] target = ScienceMapper.INSTANCE.nvmMapping( null, existingTarget );
 
         assertThat( target ).containsOnly( false );
+        assertThat( target ).isEqualTo( existingTarget );
         assertThat( existingTarget ).containsOnly( false );
 
         assertThat( ScienceMapper.INSTANCE.nvmMapping( null ) ).isEmpty();
@@ -154,6 +155,7 @@ public class ArrayMappingTest {
         short[] target = ScienceMapper.INSTANCE.nvmMapping( null, existingTarget );
 
         assertThat( target ).containsOnly( new short[] { 0 } );
+        assertThat( target ).isEqualTo( existingTarget );
         assertThat( existingTarget ).containsOnly( new short[] { 0 } );
     }
 
@@ -163,6 +165,7 @@ public class ArrayMappingTest {
         char[] target = ScienceMapper.INSTANCE.nvmMapping( null, existingTarget );
 
         assertThat( target ).containsOnly( new char[] { 0 } );
+        assertThat( target ).isEqualTo( existingTarget );
         assertThat( existingTarget ).containsOnly( new char[] { 0 } );
     }
 
@@ -172,6 +175,7 @@ public class ArrayMappingTest {
         int[] target = ScienceMapper.INSTANCE.nvmMapping( null, existingTarget );
 
         assertThat( target ).containsOnly( 0 );
+        assertThat( target ).isEqualTo( existingTarget );
         assertThat( existingTarget ).containsOnly( 0 );
     }
 
@@ -181,6 +185,7 @@ public class ArrayMappingTest {
         long[] target = ScienceMapper.INSTANCE.nvmMapping( null, existingTarget );
 
         assertThat( target ).containsOnly( 0L );
+        assertThat( target ).isEqualTo( existingTarget );
         assertThat( existingTarget ).containsOnly( 0L );
     }
 
@@ -190,6 +195,7 @@ public class ArrayMappingTest {
         float[] target = ScienceMapper.INSTANCE.nvmMapping( null, existingTarget );
 
         assertThat( target ).containsOnly( 0.0f );
+        assertThat( target ).isEqualTo( existingTarget );
         assertThat( existingTarget ).containsOnly( 0.0f );
     }
 
@@ -199,6 +205,7 @@ public class ArrayMappingTest {
         double[] target = ScienceMapper.INSTANCE.nvmMapping( null, existingTarget );
 
         assertThat( target ).containsOnly( 0.0d );
+        assertThat( target ).isEqualTo( existingTarget );
         assertThat( existingTarget ).containsOnly( 0.0d );
     }
 

processor/src/test/java/org/mapstruct/ap/test/bugs/_374/Issue374Test.java

@@ -41,7 +41,8 @@ public class Issue374Test {
 
         Target target2 = new Target();
         Target result2 = Issue374Mapper.INSTANCE.map2( null, target2 );
-        assertThat( result2 ).isNull();
+        assertThat( result2 ).isNotNull();
+        assertThat( result2 ).isEqualTo( target2 );
         assertThat( target2.getTest() ).isNull();
         assertThat( target2.getConstant() ).isNull();
     }

processor/src/test/java/org/mapstruct/ap/test/builder/mappingTarget/simple/BuilderInfoTargetTest.java

@@ -6,6 +6,7 @@
 package org.mapstruct.ap.test.builder.mappingTarget.simple;
 
 import org.junit.jupiter.api.extension.RegisterExtension;
+import org.mapstruct.ap.testutil.IssueKey;
 import org.mapstruct.ap.testutil.ProcessorTest;
 import org.mapstruct.ap.testutil.WithClasses;
 import org.mapstruct.ap.testutil.runner.GeneratedSource;
@@ -36,6 +37,18 @@ public class BuilderInfoTargetTest {
         assertThat( targetObject.getName() ).isEqualTo( "Bob" );
     }
 
+    @ProcessorTest
+    @IssueKey("1752")
+    public void testSimpleImmutableBuilderFromNullSource() {
+        SimpleImmutableTarget targetObject = SimpleBuilderMapper.INSTANCE.toImmutable(
+            null,
+            SimpleImmutableTarget.builder().age( 3 ).name( "Bob" )
+        );
+        assertThat( targetObject ).isNotNull();
+        assertThat( targetObject.getAge() ).isEqualTo( 3 );
+        assertThat( targetObject.getName() ).isEqualTo( "Bob" );
+    }
+
     @ProcessorTest
     public void testMutableTargetWithBuilder() {
         SimpleMutableSource source = new SimpleMutableSource();

processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/DefaultCollectionImplementationTest.java

@@ -159,7 +159,21 @@ public class DefaultCollectionImplementationTest {
             SourceTargetMapper.INSTANCE
                 .sourceFoosToTargetFoosUsingTargetParameterAndReturn( createSourceFooList(), target );
 
-        assertThat( target == result ).isTrue();
+        assertThat( result ).isSameAs( target );
+        assertResultList( target );
+    }
+
+    @ProcessorTest
+    @IssueKey("1752")
+    public void shouldUseAndReturnTargetParameterForNullMapping() {
+        List<TargetFoo> target = new ArrayList<>();
+        target.add( new TargetFoo( "Bob" ) );
+        target.add( new TargetFoo( "Alice" ) );
+        Iterable<TargetFoo> result =
+            SourceTargetMapper.INSTANCE
+                .sourceFoosToTargetFoosUsingTargetParameterAndReturn( null, target );
+
+        assertThat( result ).isSameAs( target );
         assertResultList( target );
     }
 

processor/src/test/java/org/mapstruct/ap/test/collection/map/MapMappingTest.java

@@ -82,6 +82,20 @@ public class MapMappingTest {
         assertResult( target );
     }
 
+    @ProcessorTest
+    @IssueKey("1752")
+    public void shouldCreateMapMethodImplementationWithReturnedTargetParameterAndNullSource() {
+        Map<Long, Date> target = new HashMap<>();
+        target.put( 42L, new GregorianCalendar( 1980, Calendar.JANUARY, 1 ).getTime() );
+        target.put( 121L, new GregorianCalendar( 2013, Calendar.JULY, 20 ).getTime() );
+
+        Map<Long, Date> returnedTarget = SourceTargetMapper.INSTANCE
+            .stringStringMapToLongDateMapUsingTargetParameterAndReturn( null, target );
+
+        assertThat( target ).isSameAs( returnedTarget );
+        assertResult( target );
+    }
+
     private void assertResult(Map<Long, Date> target) {
         assertThat( target ).isNotNull();
         assertThat( target ).hasSize( 2 );

processor/src/test/java/org/mapstruct/ap/test/inheritance/InheritanceTest.java

@@ -53,6 +53,20 @@ public class InheritanceTest {
         assertResult( target );
     }
 
+    @ProcessorTest
+    @IssueKey("1752")
+    public void shouldMapAttributeFromSuperTypeUsingReturnedTargetParameterAndNullSource() {
+
+        TargetExt target = new TargetExt();
+        target.setFoo( 42L );
+        target.publicFoo = 52L;
+        target.setBar( 23 );
+        TargetBase result = SourceTargetMapper.INSTANCE.sourceToTargetWithTargetParameterAndReturn( null, target );
+
+        assertThat( target ).isSameAs( result );
+        assertResult( target );
+    }
+
     private void assertResult(TargetExt target) {
         assertThat( target ).isNotNull();
         assertThat( target.getFoo() ).isEqualTo( Long.valueOf( 42 ) );

processor/src/test/java/org/mapstruct/ap/test/java8stream/defaultimplementation/DefaultStreamImplementationTest.java

@@ -141,6 +141,19 @@ public class DefaultStreamImplementationTest {
         assertThat( target ).containsOnly( new TargetFoo( "Bob" ) );
     }
 
+    @ProcessorTest
+    @IssueKey("1752")
+    public void shouldUseAndReturnTargetParameterArrayForNullSource() {
+        TargetFoo[] target = new TargetFoo[1];
+        target[0] = new TargetFoo( "Bob" );
+        TargetFoo[] result =
+            SourceTargetMapper.INSTANCE.streamToArrayUsingTargetParameterAndReturn( null, target );
+
+        assertThat( result ).isSameAs( target );
+        assertThat( target ).isNotNull();
+        assertThat( target ).containsOnly( new TargetFoo( "Bob" ) );
+    }
+
     @ProcessorTest
     public void shouldUseAndReturnTargetParameterForMapping() {
         List<TargetFoo> target = new ArrayList<>();
@@ -152,6 +165,20 @@ public class DefaultStreamImplementationTest {
         assertResultList( target );
     }
 
+    @ProcessorTest
+    @IssueKey("1752")
+    public void shouldUseAndReturnTargetParameterForNullMapping() {
+        List<TargetFoo> target = new ArrayList<>();
+        target.add( new TargetFoo( "Bob" ) );
+        target.add( new TargetFoo( "Alice" ) );
+        Iterable<TargetFoo> result =
+            SourceTargetMapper.INSTANCE
+                .sourceFoosToTargetFoosUsingTargetParameterAndReturn( null, target );
+
+        assertThat( result ).isSameAs( target );
+        assertResultList( target );
+    }
+
     @ProcessorTest
     public void shouldUseDefaultImplementationForListWithoutSetter() {
         Source source = new Source();

processor/src/test/resources/fixtures/org/mapstruct/ap/test/array/ScienceMapperImpl.java

@@ -92,7 +92,7 @@ public class ScienceMapperImpl implements ScienceMapper {
     @Override
     public ScientistDto[] scientistsToDtos(Scientist[] scientists, ScientistDto[] target) {
         if ( scientists == null ) {
-            return null;
+            return target;
         }
 
         int i = 0;

processor/src/test/resources/fixtures/org/mapstruct/ap/test/bugs/_913/DomainDtoWithNcvsAlwaysMapperImpl.java

@@ -128,7 +128,7 @@ public class DomainDtoWithNcvsAlwaysMapperImpl implements DomainDtoWithNcvsAlway
     @Override
     public Domain updateWithReturn(DtoWithPresenceCheck source, Domain target) {
         if ( source == null ) {
-            return null;
+            return target;
         }
 
         if ( target.getStrings() != null ) {

processor/src/test/resources/fixtures/org/mapstruct/ap/test/bugs/_913/DomainDtoWithNvmsNullMapperImpl.java

@@ -143,7 +143,7 @@ public class DomainDtoWithNvmsNullMapperImpl implements DomainDtoWithNvmsNullMap
     @Override
     public Domain updateWithReturn(Dto source, Domain target) {
         if ( source == null ) {
-            return null;
+            return target;
         }
 
         if ( target.getStrings() != null ) {

processor/src/test/resources/fixtures/org/mapstruct/ap/test/bugs/_913/DomainDtoWithPresenceCheckMapperImpl.java

@@ -128,7 +128,7 @@ public class DomainDtoWithPresenceCheckMapperImpl implements DomainDtoWithPresen
     @Override
     public Domain updateWithReturn(DtoWithPresenceCheck source, Domain target) {
         if ( source == null ) {
-            return null;
+            return target;
         }
 
         if ( target.getStrings() != null ) {

processor/src/test/resources/fixtures/org/mapstruct/ap/test/collection/defaultimplementation/SourceTargetMapperImpl.java

@@ -133,7 +133,7 @@ public class SourceTargetMapperImpl implements SourceTargetMapper {
     @Override
     public Iterable<TargetFoo> sourceFoosToTargetFoosUsingTargetParameterAndReturn(Iterable<SourceFoo> sourceFoos, List<TargetFoo> targetFoos) {
         if ( sourceFoos == null ) {
-            return null;
+            return targetFoos;
         }
 
         targetFoos.clear();