#973 Setter and Update wrapper refactoring and cleanup

processor/src/main/java/org/mapstruct/ap/internal/model/MethodReference.java

@@ -152,6 +152,11 @@ public class MethodReference extends MappingMethod implements Assignment {
         assignment.setSourceLocalVarName( sourceLocalVarName );
     }
 
+    @Override
+    public String getSourceParameterName() {
+        return assignment.getSourceParameterName();
+    }
+
     /**
      * @return the type of the single source parameter that is not the {@code @TargetType} parameter
      */
@@ -206,7 +211,7 @@ public class MethodReference extends MappingMethod implements Assignment {
     }
 
     @Override
-    public boolean isUpdateMethod() {
+    public boolean isCallingUpdateMethod() {
         return isUpdateMethod;
     }
 }

processor/src/main/java/org/mapstruct/ap/internal/model/PropertyMapping.java

@@ -19,9 +19,6 @@
 package org.mapstruct.ap.internal.model;
 
 import static org.mapstruct.ap.internal.model.assignment.Assignment.AssignmentType.DIRECT;
-import static org.mapstruct.ap.internal.model.assignment.Assignment.AssignmentType.MAPPED_TYPE_CONVERTED;
-import static org.mapstruct.ap.internal.model.assignment.Assignment.AssignmentType.TYPE_CONVERTED;
-import static org.mapstruct.ap.internal.model.assignment.Assignment.AssignmentType.TYPE_CONVERTED_MAPPED;
 import static org.mapstruct.ap.internal.prism.NullValueCheckStrategyPrism.ALWAYS;
 import static org.mapstruct.ap.internal.util.Collections.first;
 import static org.mapstruct.ap.internal.util.Collections.last;
@@ -43,7 +40,6 @@ import org.mapstruct.ap.internal.model.assignment.GetterWrapperForCollectionsAnd
 import org.mapstruct.ap.internal.model.assignment.NullCheckWrapper;
 import org.mapstruct.ap.internal.model.assignment.SetterWrapper;
 import org.mapstruct.ap.internal.model.assignment.SetterWrapperForCollectionsAndMaps;
-import org.mapstruct.ap.internal.model.assignment.UpdateNullCheckWrapper;
 import org.mapstruct.ap.internal.model.assignment.UpdateWrapper;
 import org.mapstruct.ap.internal.model.common.ModelElement;
 import org.mapstruct.ap.internal.model.common.Parameter;
@@ -55,6 +51,7 @@ import org.mapstruct.ap.internal.model.source.PropertyEntry;
 import org.mapstruct.ap.internal.model.source.SelectionParameters;
 import org.mapstruct.ap.internal.model.source.Method;
 import org.mapstruct.ap.internal.model.source.SourceReference;
+import org.mapstruct.ap.internal.prism.NullValueCheckStrategyPrism;
 import org.mapstruct.ap.internal.util.Executables;
 import org.mapstruct.ap.internal.util.MapperConfiguration;
 import org.mapstruct.ap.internal.util.Message;
@@ -335,7 +332,7 @@ public class PropertyMapping extends ModelElement {
 
             if ( targetAccessorType == TargetWriteAccessorType.SETTER ||
                 targetAccessorType == TargetWriteAccessorType.FIELD ) {
-                result = assignToPlainViaSetter( sourceType, targetType, rightHandSide );
+                result = assignToPlainViaSetter( targetType, rightHandSide );
             }
             else {
                 result = assignToPlainViaAdder( rightHandSide );
@@ -344,11 +341,9 @@ public class PropertyMapping extends ModelElement {
 
         }
 
-        private Assignment assignToPlainViaSetter(Type sourceType, Type targetType, Assignment rhs) {
+        private Assignment assignToPlainViaSetter(Type targetType, Assignment rhs) {
 
-            Assignment result;
+            if ( rhs.isCallingUpdateMethod() ) {
-
-            if ( rhs.isUpdateMethod() ) {
                 if ( targetReadAccessor == null ) {
                     ctx.getMessager().printMessage(
                         method.getExecutable(),
@@ -356,59 +351,13 @@ public class PropertyMapping extends ModelElement {
                         targetPropertyName
                     );
                 }
-                Assignment factoryMethod = ctx.getMappingResolver().getFactoryMethod( method, targetType, null );
+                Assignment factory = ctx.getMappingResolver().getFactoryMethod( method, targetType, null );
-                result = new UpdateWrapper( rhs, method.getThrownTypes(), factoryMethod,
+                return new UpdateWrapper( rhs, method.getThrownTypes(), factory, isFieldAssignment(),  targetType,
-                    targetType, isFieldAssignment()
+                    true );
-                );
             }
             else {
-                result = new SetterWrapper( rhs, method.getThrownTypes(), isFieldAssignment() );
+                NullValueCheckStrategyPrism nvcs = method.getMapperConfiguration().getNullValueCheckStrategy();
-            }
+                return new SetterWrapper( rhs, method.getThrownTypes(), nvcs, isFieldAssignment(), targetType );
-
-            // if the sourceReference is the bean mapping method parameter itself, no null check is required
-            // since this is handled by the BeanMapping
-            if ( sourceReference.getPropertyEntries().isEmpty() ) {
-                return result;
-            }
-
-            // add a null / presence checked when required
-            if ( sourceType.isPrimitive() ) {
-                if ( rhs.getSourcePresenceCheckerReference() != null ) {
-                    result = new NullCheckWrapper( result );
-                }
-            }
-            else {
-
-                if ( result.isUpdateMethod() ) {
-                    result = new UpdateNullCheckWrapper( result, isFieldAssignment() );
-                }
-                else if ( rhs.getSourcePresenceCheckerReference() != null ) {
-                    result = new NullCheckWrapper( result );
-                }
-                else if ( ALWAYS.equals( method.getMapperConfiguration().getNullValueCheckStrategy() ) ) {
-                    result = new NullCheckWrapper( result );
-                    useLocalVarWhenNested( result );
-                }
-                else if ( result.getType() == TYPE_CONVERTED
-                    || result.getType() == TYPE_CONVERTED_MAPPED
-                    || result.getType() == MAPPED_TYPE_CONVERTED
-                    || (result.getType() == DIRECT && targetType.isPrimitive() ) ) {
-                    // for primitive types null check is not possible at all, but a conversion needs
-                    // a null check.
-                    result = new NullCheckWrapper( result );
-                    useLocalVarWhenNested( result );
-                }
-            }
-
-            return result;
-        }
-
-        private void useLocalVarWhenNested(Assignment rightHandSide) {
-            if ( sourceReference.getPropertyEntries().size() > 1 ) {
-                String name = first( sourceReference.getPropertyEntries() ).getName();
-                String safeName = Strings.getSaveVariableName( name, existingVariableNames );
-                existingVariableNames.add( safeName );
-                rightHandSide.setSourceLocalVarName( safeName );
             }
         }
 
@@ -421,8 +370,7 @@ public class PropertyMapping extends ModelElement {
             }
             else {
                 // Possibly adding null to a target collection. So should be surrounded by an null check.
-                result = new SetterWrapper( result, method.getThrownTypes(), isFieldAssignment() );
+                result = new SetterWrapper( result, method.getThrownTypes(), ALWAYS, isFieldAssignment(), targetType );
-                result = new NullCheckWrapper( result );
             }
             return result;
         }
@@ -435,7 +383,7 @@ public class PropertyMapping extends ModelElement {
             if ( targetAccessorType == TargetWriteAccessorType.SETTER ||
                 targetAccessorType == TargetWriteAccessorType.FIELD ) {
 
-                if ( result.isUpdateMethod() ) {
+                if ( result.isCallingUpdateMethod() ) {
                     // call to an update method
                     if ( targetReadAccessor == null ) {
                         ctx.getMessager().printMessage(
@@ -449,8 +397,9 @@ public class PropertyMapping extends ModelElement {
                         result,
                         method.getThrownTypes(),
                         factoryMethod,
+                        isFieldAssignment(),
                         targetType,
-                        isFieldAssignment()
+                        true
                     );
                 }
                 else {
@@ -458,9 +407,8 @@ public class PropertyMapping extends ModelElement {
                     result = new SetterWrapperForCollectionsAndMaps(
                         result,
                         method.getThrownTypes(),
-                        existingVariableNames,
                         targetType,
-                        ALWAYS == method.getMapperConfiguration().getNullValueCheckStrategy(),
+                        method.getMapperConfiguration().getNullValueCheckStrategy(),
                         ctx.getTypeFactory(),
                         targetWriteAccessorType == TargetWriteAccessorType.FIELD
                     );
@@ -470,7 +418,6 @@ public class PropertyMapping extends ModelElement {
                 // target accessor is getter, so wrap the setter in getter map/ collection handling
                 result = new GetterWrapperForCollectionsAndMaps( result,
                                                                  method.getThrownTypes(),
-                                                                 existingVariableNames,
                                                                  targetType,
                                                                  isFieldAssignment()
                                                                );
@@ -560,6 +507,11 @@ public class PropertyMapping extends ModelElement {
                                                      existingVariableNames,
                                                      sourceReference.toString()
                 );
+
+                // create a local variable to which forged method can be assigned.
+                String desiredName = first( sourceReference.getPropertyEntries() ).getName();
+                sourceRhs.setSourceLocalVarName( sourceRhs.createLocalVarName( desiredName ) );
+
                 return sourceRhs;
 
             }
@@ -767,7 +719,7 @@ public class PropertyMapping extends ModelElement {
                     Executables.isFieldAccessor( targetWriteAccessor ) ) {
 
                     // target accessor is setter, so decorate assignment as setter
-                    if ( assignment.isUpdateMethod() ) {
+                    if ( assignment.isCallingUpdateMethod() ) {
                         if ( targetReadAccessor == null ) {
                             ctx.getMessager().printMessage(
                                 method.getExecutable(),
@@ -778,8 +730,7 @@ public class PropertyMapping extends ModelElement {
                         Assignment factoryMethod =
                             ctx.getMappingResolver().getFactoryMethod( method, targetType, null );
                         assignment = new UpdateWrapper( assignment, method.getThrownTypes(), factoryMethod,
-                            targetType, isFieldAssignment()
+                            isFieldAssignment(), targetType, false );
-                        );
                     }
                     else {
                         assignment = new SetterWrapper( assignment, method.getThrownTypes(), isFieldAssignment() );
@@ -790,7 +741,6 @@ public class PropertyMapping extends ModelElement {
                     // target accessor is getter, so getter map/ collection handling
                     assignment = new GetterWrapperForCollectionsAndMaps( assignment,
                                                                          method.getThrownTypes(),
-                                                                         existingVariableNames,
                                                                          targetType,
                                                                          isFieldAssignment()
                                                                        );
@@ -863,7 +813,6 @@ public class PropertyMapping extends ModelElement {
                 // target accessor is getter, so wrap the setter in getter map/ collection handling
                 assignment = new GetterWrapperForCollectionsAndMaps( assignment,
                                                                      method.getThrownTypes(),
-                                                                     existingVariableNames,
                                                                      targetType,
                                                                      isFieldAssignment()
                                                                    );

processor/src/main/java/org/mapstruct/ap/internal/model/SourceRHS.java

@@ -47,7 +47,7 @@ public class SourceRHS extends ModelElement implements Assignment {
 
     public SourceRHS(String sourceReference, Type sourceType, Set<String> existingVariableNames,
         String sourceErrorMessagePart ) {
-        this( null, sourceReference, null, sourceType, existingVariableNames, sourceErrorMessagePart );
+        this( sourceReference, sourceReference, null, sourceType, existingVariableNames, sourceErrorMessagePart );
     }
 
     public SourceRHS(String sourceParameterName, String sourceReference, String sourcePresenceCheckerReference,
@@ -77,7 +77,9 @@ public class SourceRHS extends ModelElement implements Assignment {
 
     @Override
     public String createLocalVarName(String desiredName) {
-        return Strings.getSaveVariableName( desiredName, existingVariableNames );
+        String result = Strings.getSaveVariableName( desiredName, existingVariableNames );
+        existingVariableNames.add( result );
+        return result;
     }
 
     @Override
@@ -111,7 +113,7 @@ public class SourceRHS extends ModelElement implements Assignment {
     }
 
     @Override
-    public boolean isUpdateMethod() {
+    public boolean isCallingUpdateMethod() {
         return false;
     }
 
@@ -137,12 +139,13 @@ public class SourceRHS extends ModelElement implements Assignment {
     /**
      * For collection type, use element as source type to find a suitable mapping method.
      *
-     * @param useElementAsSourceTypeForMatching
+     * @param useElementAsSourceTypeForMatching uses the element of a collection as source type for the matching process
      */
     public void setUseElementAsSourceTypeForMatching(boolean useElementAsSourceTypeForMatching) {
         this.useElementAsSourceTypeForMatching = useElementAsSourceTypeForMatching;
     }
 
+    @Override
     public String getSourceParameterName() {
         return sourceParameterName;
     }

processor/src/main/java/org/mapstruct/ap/internal/model/TypeConversion.java

@@ -111,6 +111,11 @@ public class TypeConversion extends ModelElement implements Assignment {
         assignment.setSourceLocalVarName( sourceLocalVarName );
     }
 
+    @Override
+    public String getSourceParameterName() {
+        return assignment.getSourceParameterName();
+    }
+
     @Override
     public void setAssignment( Assignment assignment ) {
         this.assignment = assignment;
@@ -130,7 +135,7 @@ public class TypeConversion extends ModelElement implements Assignment {
     }
 
     @Override
-    public boolean isUpdateMethod() {
+    public boolean isCallingUpdateMethod() {
        return false;
     }
 }

processor/src/main/java/org/mapstruct/ap/internal/model/assignment/AdderWrapper.java

@@ -33,14 +33,12 @@ import org.mapstruct.ap.internal.model.common.Type;
 public class AdderWrapper extends AssignmentWrapper {
 
     private final List<Type> thrownTypesToExclude;
-    private final String sourceIteratorName;
 
-    public AdderWrapper( Assignment decoratedAssignment, List<Type> thrownTypesToExclude, boolean fieldAssignment ) {
+    public AdderWrapper( Assignment rhs, List<Type> thrownTypesToExclude, boolean fieldAssignment ) {
-        super( decoratedAssignment, fieldAssignment );
+        super( rhs, fieldAssignment );
         this.thrownTypesToExclude = thrownTypesToExclude;
-        String desiredName = decoratedAssignment.getSourceType().getTypeParameters().get( 0 ).getName();
+        String desiredName = rhs.getSourceType().getTypeParameters().get( 0 ).getName();
-        this.sourceIteratorName = decoratedAssignment.createLocalVarName( desiredName );
+        rhs.setSourceLocalVarName( rhs.createLocalVarName( desiredName ) );
-        decoratedAssignment.setSourceLocalVarName( sourceIteratorName );
     }
 
     @Override
@@ -65,7 +63,4 @@ public class AdderWrapper extends AssignmentWrapper {
         return imported;
     }
 
-    public String getSourceIteratorName() {
-        return sourceIteratorName;
-    }
 }

processor/src/main/java/org/mapstruct/ap/internal/model/assignment/Assignment.java

@@ -32,17 +32,34 @@ public interface Assignment {
 
     enum AssignmentType {
         /** assignment is direct */
-        DIRECT,
+        DIRECT( true, false ),
         /** assignment is type converted */
-        TYPE_CONVERTED,
+        TYPE_CONVERTED( false, true ),
         /** assignment is mapped (builtin/custom) */
-        MAPPED,
+        MAPPED( false, false ),
         /** 2 mapping methods (builtin/custom) are applied to get the target */
-        MAPPED_TWICE,
+        MAPPED_TWICE( false, false ),
         /** assignment is first mapped (builtin/custom), then the result is type converted */
-        MAPPED_TYPE_CONVERTED,
+        MAPPED_TYPE_CONVERTED( false, true ),
         /** assignment is first type converted, and then mapped (builtin/custom) */
-        TYPE_CONVERTED_MAPPED
+        TYPE_CONVERTED_MAPPED( false, true );
+
+        private final boolean direct;
+        private final boolean converted;
+
+        AssignmentType( boolean isDirect, boolean isConverted ) {
+            this.direct = isDirect;
+            this.converted = isConverted;
+        }
+
+        public boolean isDirect() {
+            return direct;
+        }
+
+        public boolean isConverted() {
+            return converted;
+        }
+
     }
 
     /**
@@ -106,6 +123,14 @@ public interface Assignment {
      */
     String getSourceLocalVarName();
 
+    /**
+     * Returns the source parameter name, to which this assignment applies. Note: the source parameter itself might
+     * be mapped by this assignment, or one of its properties
+     *
+     * @return  the source parameter name
+     */
+    String getSourceParameterName();
+
     /**
      * Use sourceLocalVarName iso sourceReference
      * @param sourceLocalVarName source local variable name
@@ -119,5 +144,5 @@ public interface Assignment {
      */
     AssignmentType getType();
 
-    boolean isUpdateMethod();
+    boolean isCallingUpdateMethod();
 }

processor/src/main/java/org/mapstruct/ap/internal/model/assignment/AssignmentWrapper.java

@@ -83,14 +83,19 @@ public abstract class AssignmentWrapper extends ModelElement implements Assignme
         decoratedAssignment.setSourceLocalVarName( sourceLocalVarName );
     }
 
+    @Override
+    public String getSourceParameterName() {
+        return decoratedAssignment.getSourceParameterName();
+    }
+
     @Override
     public AssignmentType getType() {
         return decoratedAssignment.getType();
     }
 
      @Override
-    public boolean isUpdateMethod() {
+    public boolean isCallingUpdateMethod() {
-        return decoratedAssignment.isUpdateMethod();
+        return decoratedAssignment.isCallingUpdateMethod();
     }
 
     @Override

processor/src/main/java/org/mapstruct/ap/internal/model/assignment/GetterWrapperForCollectionsAndMaps.java

@@ -19,7 +19,6 @@
 package org.mapstruct.ap.internal.model.assignment;
 
 import java.util.List;
-import java.util.Set;
 
 import org.mapstruct.ap.internal.model.common.Type;
 
@@ -39,22 +38,19 @@ import org.mapstruct.ap.internal.model.common.Type;
 public class GetterWrapperForCollectionsAndMaps extends WrapperForCollectionsAndMaps {
 
     /**
-     * @param decoratedAssignment
+     * @param decoratedAssignment source RHS
-     * @param thrownTypesToExclude
+     * @param thrownTypesToExclude set of types to exclude from re-throwing
-     * @param existingVariableNames
+     * @param targetType the target type
-     * @param targetType
+     * @param fieldAssignment true when this the assignment is to a field rather than via accessors
-     * @param fieldAssignment
      */
     public GetterWrapperForCollectionsAndMaps(Assignment decoratedAssignment,
                                               List<Type> thrownTypesToExclude,
-                                              Set<String> existingVariableNames,
                                               Type targetType,
                                               boolean fieldAssignment) {
 
         super(
             decoratedAssignment,
             thrownTypesToExclude,
-            existingVariableNames,
             targetType,
             fieldAssignment
         );

processor/src/main/java/org/mapstruct/ap/internal/model/assignment/SetterWrapper.java

@@ -22,6 +22,9 @@ import java.util.ArrayList;
 import java.util.List;
 
 import org.mapstruct.ap.internal.model.common.Type;
+import org.mapstruct.ap.internal.prism.NullValueCheckStrategyPrism;
+
+import static org.mapstruct.ap.internal.prism.NullValueCheckStrategyPrism.ALWAYS;
 
 /**
  * Wraps the assignment in a target setter.
@@ -31,10 +34,23 @@ import org.mapstruct.ap.internal.model.common.Type;
 public class SetterWrapper extends AssignmentWrapper {
 
     private final List<Type> thrownTypesToExclude;
+    private final boolean includeSourceNullCheck;
 
-    public SetterWrapper(Assignment decoratedAssignment, List<Type> thrownTypesToExclude, boolean fieldAssignment) {
+    public SetterWrapper(Assignment rhs,
-        super( decoratedAssignment, fieldAssignment );
+                         List<Type> thrownTypesToExclude,
+                         NullValueCheckStrategyPrism nvms,
+                         boolean fieldAssignment,
+                         Type targetType) {
+
+        super( rhs, fieldAssignment );
         this.thrownTypesToExclude = thrownTypesToExclude;
+        this.includeSourceNullCheck = includeSourceNullCheck( rhs, nvms, targetType );
+    }
+
+    public SetterWrapper(Assignment rhs, List<Type> thrownTypesToExclude, boolean fieldAssignment  ) {
+        super( rhs, fieldAssignment );
+        this.thrownTypesToExclude = thrownTypesToExclude;
+        this.includeSourceNullCheck = false;
     }
 
     @Override
@@ -51,4 +67,30 @@ public class SetterWrapper extends AssignmentWrapper {
         return result;
     }
 
+    public boolean isIncludeSourceNullCheck() {
+        return includeSourceNullCheck;
+    }
+
+   /**
+    * Wraps the assignment in a target setter. include a null check when
+    *
+    * - Not if source is the parameter iso property, because the null check is than handled by the bean mapping
+    * - Not when source is primitive, you can't null check a primitive
+    * - The source property is fed to a conversion somehow before its assigned to the target
+    * - The user decided to ALLWAYS include a null check
+    * - When there's a source local variable name defined (e.g. nested source properties)
+    * - TODO: The last one I forgot..?
+    *
+    * @param rhs the source righthand side
+    * @param nvms null value check strategy
+    * @param targetType the target type
+    *
+    * @return include a null check
+    */
+    private boolean includeSourceNullCheck(Assignment rhs, NullValueCheckStrategyPrism nvms, Type targetType) {
+        return !rhs.getSourceReference().equals( rhs.getSourceParameterName() )
+            && !rhs.getSourceType().isPrimitive()
+            && (ALWAYS == nvms || rhs.getType().isConverted() || rhs.getSourceLocalVarName() != null
+            || (rhs.getType().isDirect() && targetType.isPrimitive()));
+    }
 }

processor/src/main/java/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.java

@@ -26,6 +26,9 @@ import java.util.Set;
 
 import org.mapstruct.ap.internal.model.common.Type;
 import org.mapstruct.ap.internal.model.common.TypeFactory;
+import org.mapstruct.ap.internal.prism.NullValueCheckStrategyPrism;
+
+import static org.mapstruct.ap.internal.prism.NullValueCheckStrategyPrism.ALWAYS;
 
 /**
  * This wrapper handles the situation were an assignment is done via the setter.
@@ -41,26 +44,24 @@ import org.mapstruct.ap.internal.model.common.TypeFactory;
  */
 public class SetterWrapperForCollectionsAndMaps extends WrapperForCollectionsAndMaps {
 
-    private final boolean allwaysIncludeNullCheck;
+    private final boolean includeSourceNullCheck;
     private final Type targetType;
     private final TypeFactory typeFactory;
 
     public SetterWrapperForCollectionsAndMaps(Assignment decoratedAssignment,
                                               List<Type> thrownTypesToExclude,
-                                              Set<String> existingVariableNames,
                                               Type targetType,
-                                              boolean allwaysIncludeNullCheck,
+                                              NullValueCheckStrategyPrism nvms,
                                               TypeFactory typeFactory,
                                               boolean fieldAssignment) {
 
         super(
             decoratedAssignment,
             thrownTypesToExclude,
-            existingVariableNames,
             targetType,
             fieldAssignment
         );
-        this.allwaysIncludeNullCheck = allwaysIncludeNullCheck;
+        this.includeSourceNullCheck = ALWAYS == nvms;
         this.targetType = targetType;
         this.typeFactory = typeFactory;
     }
@@ -83,8 +84,8 @@ public class SetterWrapperForCollectionsAndMaps extends WrapperForCollectionsAnd
         return imported;
     }
 
-    public boolean isAllwaysIncludeNullCheck() {
+    public boolean isIncludeSourceNullCheck() {
-        return allwaysIncludeNullCheck;
+        return includeSourceNullCheck;
     }
 
     public boolean isDirectAssignment() {

processor/src/main/java/org/mapstruct/ap/internal/model/assignment/UpdateWrapper.java

@@ -35,13 +35,19 @@ public class UpdateWrapper extends AssignmentWrapper {
     private final List<Type> thrownTypesToExclude;
     private final Assignment factoryMethod;
     private final Type targetImplementationType;
+    private final boolean includeSourceNullCheck;
 
-    public UpdateWrapper(Assignment decoratedAssignment, List<Type> thrownTypesToExclude, Assignment factoryMethod,
+    public UpdateWrapper( Assignment decoratedAssignment,
-        Type targetImplementationType, boolean fieldAssignment ) {
+                          List<Type> thrownTypesToExclude,
+                          Assignment factoryMethod,
+                          boolean fieldAssignment,
+                          Type targetType,
+                          boolean includeSourceNullCheck ) {
         super( decoratedAssignment, fieldAssignment );
         this.thrownTypesToExclude = thrownTypesToExclude;
         this.factoryMethod = factoryMethod;
-        this.targetImplementationType = determineImplType( factoryMethod, targetImplementationType );
+        this.targetImplementationType = determineImplType( factoryMethod, targetType );
+        this.includeSourceNullCheck = includeSourceNullCheck;
     }
 
     private static Type determineImplType(Assignment factoryMethod, Type targetType) {
@@ -87,4 +93,7 @@ public class UpdateWrapper extends AssignmentWrapper {
         return factoryMethod;
     }
 
+    public boolean isIncludeSourceNullCheck() {
+        return includeSourceNullCheck;
+    }
 }

processor/src/main/java/org/mapstruct/ap/internal/model/assignment/WrapperForCollectionsAndMaps.java

@@ -24,7 +24,6 @@ import java.util.List;
 import java.util.Set;
 
 import org.mapstruct.ap.internal.model.common.Type;
-import org.mapstruct.ap.internal.util.Strings;
 
 /**
  * This is the base class for the {@link GetterWrapperForCollectionsAndMaps} and
@@ -35,26 +34,24 @@ import org.mapstruct.ap.internal.util.Strings;
 public class WrapperForCollectionsAndMaps extends AssignmentWrapper {
 
     private final List<Type> thrownTypesToExclude;
-    private final String localVarName;
+    private final String nullCheckLocalVarName;
-    private final Type localVarType;
+    private final Type nullCheckLocalVarType;
 
-    public WrapperForCollectionsAndMaps(Assignment decoratedAssignment,
+    public WrapperForCollectionsAndMaps(Assignment rhs,
                                         List<Type> thrownTypesToExclude,
-                                        Set<String> existingVariableNames,
                                         Type targetType,
                                         boolean fieldAssignment) {
 
-        super( decoratedAssignment, fieldAssignment );
+        super( rhs, fieldAssignment );
 
         this.thrownTypesToExclude = thrownTypesToExclude;
-        if ( getType() == AssignmentType.DIRECT && getSourceType() != null ) {
+        if ( rhs.getType() == AssignmentType.DIRECT && rhs.getSourceType() != null ) {
-            this.localVarType = getSourceType();
+            this.nullCheckLocalVarType = rhs.getSourceType();
         }
         else {
-            this.localVarType = targetType;
+            this.nullCheckLocalVarType = targetType;
         }
-        this.localVarName = Strings.getSaveVariableName( localVarType.getName(), existingVariableNames );
+        this.nullCheckLocalVarName =  rhs.createLocalVarName( nullCheckLocalVarType.getName() );
-        existingVariableNames.add( this.localVarName );
     }
 
     @Override
@@ -75,16 +72,16 @@ public class WrapperForCollectionsAndMaps extends AssignmentWrapper {
     public Set<Type> getImportTypes() {
         Set<Type> imported = new HashSet<Type>();
         imported.addAll( super.getImportTypes() );
-        imported.add( localVarType );
+        imported.add( nullCheckLocalVarType );
-        imported.addAll( localVarType.getTypeParameters() );
+        imported.addAll( nullCheckLocalVarType.getTypeParameters() );
         return imported;
     }
 
-    public String getLocalVarName() {
+    public String getNullCheckLocalVarName() {
-        return localVarName;
+        return nullCheckLocalVarName;
     }
 
-    public Type getLocalVarType() {
+    public Type getNullCheckLocalVarType() {
-        return localVarType;
+        return nullCheckLocalVarType;
     }
 }

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

@@ -21,7 +21,7 @@
 <#import "../macro/CommonMacros.ftl" as lib>
 <@lib.handleExceptions>
   if ( ${sourceReference} != null ) {
-      for ( <@includeModel object=sourceType.typeParameters[0]/> ${sourceIteratorName} : ${sourceReference} ) {
+      for ( <@includeModel object=sourceType.typeParameters[0]/> ${sourceLocalVarName} : ${sourceReference} ) {
           ${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWrite><@lib.handleAssignment/></@lib.handleWrite>;
       }
   }

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

@@ -19,13 +19,14 @@
 
 -->
 <#import "../macro/CommonMacros.ftl" as lib>
+<@lib.sourceLocalVarAssignment/>
 if ( ${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWriteAccesing /> != null ) {
     <@lib.handleExceptions>
       <#if ext.existingInstanceMapping>
         ${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWriteAccesing />.clear();
       </#if>
       <@lib.handleNullCheck>
-        ${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWriteAccesing />.<#if ext.targetType.collectionType>addAll<#else>putAll</#if>( ${localVarName} );
+        ${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWriteAccesing />.<#if ext.targetType.collectionType>addAll<#else>putAll</#if>( ${nullCheckLocalVarName} );
       </@lib.handleNullCheck>
     </@lib.handleExceptions>
 }

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

@@ -19,44 +19,33 @@
 
 -->
 <#import "../macro/CommonMacros.ftl" as lib>
-<#if (thrownTypes?size == 0) >
+<@lib.handleExceptions>
-    <@assignment_w_defaultValue/>
+    <@lib.sourceLocalVarAssignment/>
-<#else>
+    <#if sourcePresenceCheckerReference??>
-    try {
+        if ( ${sourcePresenceCheckerReference} ) {
-        <@assignment_w_defaultValue/>
+            <@assignment/>;
-    }
-    <#list thrownTypes as exceptionType>
-    catch ( <@includeModel object=exceptionType/> e ) {
-        throw new RuntimeException( e );
-    }
-    </#list>
-</#if>
-<#macro _assignment>
-    <@includeModel object=assignment
-               targetBeanName=ext.targetBeanName
-               existingInstanceMapping=ext.existingInstanceMapping
-               targetReadAccessorName=ext.targetReadAccessorName
-               targetWriteAccessorName=ext.targetWriteAccessorName
-               targetType=ext.targetType
-               defaultValueAssignment=ext.defaultValueAssignment/>
-</#macro>
-<#macro _defaultValueAssignment>
-    <@includeModel object=ext.defaultValueAssignment.assignment
-               targetBeanName=ext.targetBeanName
-               existingInstanceMapping=ext.existingInstanceMapping
-               targetWriteAccessorName=ext.targetWriteAccessorName
-               targetType=ext.targetType/>
-</#macro>
-<#macro assignment_w_defaultValue>
-    <#if ext.defaultValueAssignment?? >
-        <#-- if the assignee property is a primitive, defaulValueAssignment will not be set -->
-        if ( ${sourceReference} != null ) {
-            ${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWrite><@_assignment/></@lib.handleWrite>;
         }
-        else {
+        <@elseDefaultAssignment/>
-            ${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWrite><@_defaultValueAssignment/></@lib.handleWrite>;
+    <#elseif includeSourceNullCheck || ext.defaultValueAssignment??>
+        if ( <#if sourceLocalVarName??>${sourceLocalVarName}<#else>${sourceReference}</#if> != null ) {
+            <@assignment/>;
         }
+        <@elseDefaultAssignment/>
     <#else>
-        ${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWrite><@_assignment/></@lib.handleWrite>;
+        <@assignment/>;
+    </#if>
+</@lib.handleExceptions>
+<#--
+    standard assignment
+-->
+<#macro assignment>${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWrite><@lib.handleAssignment/></@lib.handleWrite></#macro>
+<#--
+    add default assignment when required
+-->
+<#macro elseDefaultAssignment>
+    <#if ext.defaultValueAssignment?? >
+      else {
+        <@lib.handeDefaultAssigment/>
+      }
     </#if>
 </#macro>

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

@@ -19,14 +19,15 @@
 
 -->
 <#import "../macro/CommonMacros.ftl" as lib>
+<@lib.sourceLocalVarAssignment/>
 <@lib.handleExceptions>
   <#if ext.existingInstanceMapping>
     if ( ${ext.targetBeanName}.${ext.targetReadAccessorName} != null ) {
         <@lib.handleNullCheck>
             ${ext.targetBeanName}.${ext.targetReadAccessorName}.clear();
-            ${ext.targetBeanName}.${ext.targetReadAccessorName}.<#if ext.targetType.collectionType>addAll<#else>putAll</#if>( ${localVarName} );
+            ${ext.targetBeanName}.${ext.targetReadAccessorName}.<#if ext.targetType.collectionType>addAll<#else>putAll</#if>( ${nullCheckLocalVarName} );
         </@lib.handleNullCheck>
-        <#if !ext.defaultValueAssignment?? && !sourcePresenceCheckerReference?? && !allwaysIncludeNullCheck>else {<#-- the opposite (defaultValueAssignment) case is handeld inside lib.handleNullCheck -->
+        <#if !ext.defaultValueAssignment?? && !sourcePresenceCheckerReference?? && !includeSourceNullCheck>else {<#-- the opposite (defaultValueAssignment) case is handeld inside lib.handleNullCheck -->
           ${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWrite>null</@lib.handleWrite>;
         }
         </#if>
@@ -43,7 +44,7 @@
 -->
 <#macro callTargetWriteAccessor>
     <@lib.handleNullCheck>
-        ${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWrite><#if directAssignment><@wrapLocalVarInCollectionInitializer/><#else>${localVarName}</#if></@lib.handleWrite>;
+        ${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWrite><#if directAssignment><@wrapLocalVarInCollectionInitializer/><#else>${nullCheckLocalVarName}</#if></@lib.handleWrite>;
   </@lib.handleNullCheck>
 </#macro>
 <#--
@@ -51,8 +52,8 @@
 -->
 <#macro wrapLocalVarInCollectionInitializer><@compress single_line=true>
     <#if enumSet>
-      EnumSet.copyOf( ${localVarName} )
+      EnumSet.copyOf( ${nullCheckLocalVarName} )
     <#else>
-      new <#if ext.targetType.implementationType??><@includeModel object=ext.targetType.implementationType/><#else><@includeModel object=ext.targetType/></#if>( ${localVarName} )
+      new <#if ext.targetType.implementationType??><@includeModel object=ext.targetType.implementationType/><#else><@includeModel object=ext.targetType/></#if>( ${nullCheckLocalVarName} )
     </#if>
 </@compress></#macro>

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

@@ -19,28 +19,25 @@
 
 -->
 <#import '../macro/CommonMacros.ftl' as lib >
-<#if (thrownTypes?size == 0) >
+<@lib.handleExceptions>
-    <@_assignment/>;
+  <#if includeSourceNullCheck>
-<#else>
+    if ( <#if sourcePresenceCheckerReference?? >${sourcePresenceCheckerReference}<#else>${sourceReference} != null</#if> ) {
-    try {
+      <@assignToExistingTarget/>
-        <@_assignment/>;
+      <@lib.handleAssignment/>;
     }
-    <#list thrownTypes as exceptionType>
+    else {
-    catch ( <@includeModel object=exceptionType/> e ) {
+      ${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWrite>null</@lib.handleWrite>;
-        throw new RuntimeException( e );
     }
-    </#list>
+  <#else>
-</#if>
+    <@assignToExistingTarget/>
-<#macro _assignment>
+    <@lib.handleAssignment/>;
+  </#if>
+</@lib.handleExceptions>
+<#--
+    target innner check and assignment
+-->
+<#macro assignToExistingTarget>
     if ( ${ext.targetBeanName}.${ext.targetReadAccessorName} == null ) {
-        ${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWrite><#if factoryMethod??><@includeModel object=factoryMethod targetType=ext.targetType/><#else><@_newObject/></#if></@lib.handleWrite>;
+        ${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWrite><@lib.initTargetObject/></@lib.handleWrite>;
     }
-    <@includeModel object=assignment
-               targetBeanName=ext.targetBeanName
-               existingInstanceMapping=ext.existingInstanceMapping
-               targetReadAccessorName=ext.targetReadAccessorName
-               targetWriteAccessorName=ext.targetWriteAccessorName
-               targetType=ext.targetType/>
 </#macro>
-<#macro _newObject>new <#if ext.targetType.implementationType??><@includeModel object=ext.targetType.implementationType/><#else><@includeModel object=ext.targetType/></#if>()</#macro>
-

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

@@ -30,18 +30,18 @@
 <#macro handleNullCheck>
   <#if sourcePresenceCheckerReference??>
     if ( ${sourcePresenceCheckerReference} ) {
-      <@includeModel object=localVarType/> ${localVarName} = <@lib.handleAssignment/>;
+      <@includeModel object=nullCheckLocalVarType/> ${nullCheckLocalVarName} = <@lib.handleAssignment/>;
       <#nested>
     }
   <#else>
-    <@includeModel object=localVarType/> ${localVarName} = <@lib.handleAssignment/>;
+    <@includeModel object=nullCheckLocalVarType/> ${nullCheckLocalVarName} = <@lib.handleAssignment/>;
-    if ( ${localVarName} != null ) {
+    if ( ${nullCheckLocalVarName} != null ) {
       <#nested>
     }
   </#if>
   <#if ext.defaultValueAssignment?? >
   else {
-    <@lib.handeDefaultAssigment/>
+    <@handeDefaultAssigment/>
   }
   </#if>
 </#macro>
@@ -105,3 +105,37 @@ Performs a default assignment with a default value.
 <#macro handleWriteAccesing>
     <#t><#if fieldAssignment><#else>()</#if>
 </#macro>
+<#--
+  macro: initTargetObject
+
+  purpose: To factorize or construct a new target object
+-->
+<#macro initTargetObject><@compress single_line=true>
+    <#if factoryMethod??>
+        <@includeModel object=factoryMethod targetType=ext.targetType/>
+    <#else>
+         new <@constructTargetObject/>()
+    </#if>
+</@compress></#macro>
+<#--
+  macro: constructTargetObject
+
+  purpose: Either call the constructor of the target object directly or of the implementing type.
+-->
+<#macro constructTargetObject><@compress single_line=true>
+    <#if ext.targetType.implementationType??>
+        <@includeModel object=ext.targetType.implementationType/>
+    <#else>
+        <@includeModel object=ext.targetType/>
+    </#if>
+</@compress></#macro>
+<#--
+  macro: sourceLocalVarAssignment
+
+  purpose: assigment for source local variables
+-->
+<#macro sourceLocalVarAssignment>
+    <#if sourceLocalVarName??>
+      <@includeModel object=sourceType/> ${sourceLocalVarName} = ${sourceReference};
+    </#if>
+</#macro>