#214 addressing superfluous NPE checks and adding NPE rudimentary unit tests

processor/src/main/java/org/mapstruct/ap/conversion/PrimitiveToPrimitiveConversion.java

@@ -44,6 +44,6 @@ public class PrimitiveToPrimitiveConversion extends SimpleConversion {
 
     @Override
     public String getFromExpression(ConversionContext conversionContext) {
-        return "(" + sourceType + ")<SOURCE>";
+        return "(" + sourceType + ") <SOURCE>";
     }
 }

processor/src/main/java/org/mapstruct/ap/conversion/PrimitiveToWrapperConversion.java

@@ -50,7 +50,7 @@ public class PrimitiveToWrapperConversion extends SimpleConversion {
             return "<SOURCE>";
         }
         else {
-            return "(" + targetType.getName() + ")<SOURCE>";
+            return "(" + targetType.getName() + ") <SOURCE>";
         }
     }
 

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

@@ -29,6 +29,21 @@ import org.mapstruct.ap.model.common.Type;
  */
 public interface Assignment {
 
+    public static enum AssignmentType {
+        /** assignment is direct */
+        DIRECT,
+        /** assignment is type converted */
+        TYPE_CONVERTED,
+        /** assignment is mapped (builtin/custom) */
+        MAPPED,
+        /** 2 mapping methods (builtin/custom) are applied to get the target */
+        MAPPED_TWICE,
+        /** assignment is first mapped (builtin/custom), then the result is type converted */
+        MAPPED_TYPE_CONVERTED,
+        /** assignment is first type converted, and then mapped (builtin/custom) */
+        TYPE_CONVERTED_MAPPED
+    }
+
     /**
      * returns all types required as import by the assignment statement.
      *
@@ -62,10 +77,10 @@ public interface Assignment {
      String getSourceReference();
 
      /**
-      * Returns whether the implemented assignment is a plain source assignment (Simple assignment)
+      * Returns whether the type of assignment
-      * (so not a MethodReference or TypeConversion).
       *
-      * @return true when this is a (wrapped) Simple Assignment
+      * @return {@link  AssignmentType}
       */
-     boolean isSimple();
+     AssignmentType getType();
+
 }

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

@@ -24,6 +24,8 @@ import java.util.Set;
 import org.mapstruct.ap.model.common.ModelElement;
 import org.mapstruct.ap.model.common.Type;
 
+import static org.mapstruct.ap.model.Assignment.AssignmentType.DIRECT;
+
 /**
  * Represents the mapping between a source and target property, e.g. from
  * {@code String Source#foo} to {@code int Target#bar}. Name and type of source
@@ -35,65 +37,31 @@ import org.mapstruct.ap.model.common.Type;
 public class PropertyMapping extends ModelElement {
 
     private final String sourceBeanName;
-    private final String sourceName;
-    private final String sourceAccessorName;
-    private final Type sourceType;
 
-    private final String targetName;
     private final String targetAccessorName;
     private final Type targetType;
-    private final boolean isTargetAccessorSetter;
-    private final String targetReadAccessorName;
 
-    private final Assignment propertyAssignment;
+    private final Assignment assignment;
 
-    /**
+    // Constructor for creating mappings of constant expressions.
-     * Constructor for creating mappings of constant expressions.
+    public PropertyMapping(String targetAccessorName, Type targetType, Assignment propertyAssignment) {
-     */
+        this( null, targetAccessorName, targetType, propertyAssignment );
-    public PropertyMapping(Type sourceType, String targetName, String targetAccessorName, Type targetType,
-                           Assignment propertyAssignment) {
-        this( null, null, null, sourceType, targetName, targetAccessorName, targetType, propertyAssignment );
     }
 
-    public PropertyMapping(String sourceBeanName, String sourceName, String sourceAccessorName, Type sourceType,
+    public PropertyMapping(String sourceBeanName, String targetAccessorName, Type targetType, Assignment assignment) {
-                           String targetName, String targetAccessorName, Type targetType,
-                           Assignment propertyAssignment) {
 
         this.sourceBeanName = sourceBeanName;
-        this.sourceName = sourceName;
-        this.sourceAccessorName = sourceAccessorName;
-        this.sourceType = sourceType;
 
-        this.targetName = targetName;
         this.targetAccessorName = targetAccessorName;
         this.targetType = targetType;
-        this.isTargetAccessorSetter = targetAccessorName.startsWith( "set" );
-        this.targetReadAccessorName =
-            this.isTargetAccessorSetter ? "get" + targetAccessorName.substring( 3 ) : targetAccessorName;
 
-        this.propertyAssignment = propertyAssignment;
+        this.assignment = assignment;
     }
 
     public String getSourceBeanName() {
         return sourceBeanName;
     }
 
-    public String getSourceName() {
-        return sourceName;
-    }
-
-    public String getSourceAccessorName() {
-        return sourceAccessorName;
-    }
-
-    public Type getSourceType() {
-        return sourceType;
-    }
-
-    public String getTargetName() {
-        return targetName;
-    }
-
     public String getTargetAccessorName() {
         return targetAccessorName;
     }
@@ -102,41 +70,20 @@ public class PropertyMapping extends ModelElement {
         return targetType;
     }
 
-    public Assignment getPropertyAssignment() {
+    public Assignment getAssignment() {
-        return propertyAssignment;
+        return assignment;
-    }
-
-    /**
-     * Whether the target accessor is a setter method or not. The only case where it is not a setter but a getter is a
-     * collection-typed property without a getter, to which elements are set by adding the source elements to the
-     * collection retrieved via the getter.
-     *
-     * @return {@code true} if the target accessor is a setter, {@code false} otherwise
-     */
-    public boolean isTargetAccessorSetter() {
-        return isTargetAccessorSetter;
-    }
-
-    /**
-     * @return the read-accessor for the target property (i.e. the getter method)
-     */
-    public String getTargetReadAccessorName() {
-        return targetReadAccessorName;
     }
 
     @Override
     public Set<Type> getImportTypes() {
         Set<Type> importTypes = new HashSet<Type>();
-        if ( propertyAssignment != null ) {
+        if ( assignment.getType() == DIRECT  ) {
-            if ( isTargetAccessorSetter()
+            if ( targetType.isCollectionOrMapType() ) {
-                && propertyAssignment.isSimple()
-                && ( targetType.isCollectionType() || targetType.isMapType() ) ) {
                 importTypes.addAll( targetType.getImportTypes() );
             }
-
+        }
-            if ( !propertyAssignment.isSimple() ) {
+        else {
-                importTypes.addAll( propertyAssignment.getImportTypes() );
+            importTypes.addAll( assignment.getImportTypes() );
-            }
         }
         return importTypes;
     }
@@ -144,11 +91,9 @@ public class PropertyMapping extends ModelElement {
     @Override
     public String toString() {
         return "PropertyMapping {" +
-            "\n    sourceName='" + sourceAccessorName + "\'," +
-            "\n    sourceType=" + sourceType + "," +
             "\n    targetName='" + targetAccessorName + "\'," +
             "\n    targetType=" + targetType + "," +
-            "\n    propertyAssignment=" + propertyAssignment +
+            "\n    propertyAssignment=" + assignment +
             "\n}";
     }
 }

processor/src/main/java/org/mapstruct/ap/model/assignment/AssignmentFactory.java

@@ -56,8 +56,8 @@ public class AssignmentFactory {
         return new MethodReference( method, contextParam );
     }
 
-    public static Simple createSimple(String sourceRef) {
+    public static Direct createSimple(String sourceRef) {
-        return new Simple( sourceRef );
+        return new Direct( sourceRef );
     }
 
 }

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

@@ -62,7 +62,7 @@ public abstract class AssignmentWrapper extends ModelElement implements Assignme
     }
 
     @Override
-    public boolean isSimple() {
+    public AssignmentType getType() {
-        return decoratedAssignment.isSimple();
+        return decoratedAssignment.getType();
     }
 }

processor/src/main/java/org/mapstruct/ap/model/assignment/Direct.java

@@ -26,15 +26,15 @@ import org.mapstruct.ap.model.common.ModelElement;
 import org.mapstruct.ap.model.common.Type;
 
 /**
- * Simple Assignment. Just a source reference
+ * Direct Assignment. Just a source reference
  *
  * @author Sjaak Derksen
  */
-public class Simple extends ModelElement implements Assignment {
+public class Direct extends ModelElement implements Assignment {
 
     private final String sourceReference;
 
-    public Simple( String sourceReference ) {
+    public Direct( String sourceReference ) {
         this.sourceReference = sourceReference;
     }
 
@@ -59,7 +59,7 @@ public class Simple extends ModelElement implements Assignment {
     }
 
     @Override
-    public boolean isSimple() {
+    public AssignmentType getType() {
-        return true;
+        return AssignmentType.DIRECT;
     }
 }

processor/src/main/java/org/mapstruct/ap/model/assignment/GetterCollectionOrMapWrapper.java

@@ -0,0 +1,49 @@
+/**
+ *  Copyright 2012-2014 Gunnar Morling (http://www.gunnarmorling.de/)
+ *  and/or other contributors as indicated by the @authors tag. See the
+ *  copyright.txt file in the distribution for a full listing of all
+ *  contributors.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.mapstruct.ap.model.assignment;
+
+import org.mapstruct.ap.model.Assignment;
+
+/**
+ * This wrapper handles the situation were an assignment must be done via a target getter method because there
+ * is no setter available.
+ *
+ * The wrapper checks if there is an collection or map initialized on the target bean (not null). If so it uses the
+ * addAll (for collections) or putAll (for maps). The collection / map is cleared in case of a pre-existing target
+ * {@link org.mapstruct.MappingTarget }before adding the source entries. The goal is that the same collection / map
+ * is used as target.
+ *
+ * Nothing can be added if the getter on the target returns null.
+ *
+ * @author Sjaak Derksen
+ */
+public class GetterCollectionOrMapWrapper extends AssignmentWrapper {
+
+    private final String targetGetterName;
+
+    public GetterCollectionOrMapWrapper( Assignment decoratedAssignment, String targetGetterName ) {
+        super( decoratedAssignment );
+        this.targetGetterName = targetGetterName;
+    }
+
+    public String getTargetGetterName() {
+        return targetGetterName;
+    }
+
+}

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

@@ -148,7 +148,17 @@ public class MethodReference extends MappingMethod implements Assignment, Factor
     }
 
     @Override
-    public boolean isSimple() {
+    public AssignmentType getType() {
-        return false;
+
+        switch ( assignment.getType() ) {
+            case DIRECT:
+                return AssignmentType.MAPPED;
+            case MAPPED:
+                return AssignmentType.MAPPED_TWICE;
+            case TYPE_CONVERTED:
+                return AssignmentType.TYPE_CONVERTED_MAPPED;
+            default:
+                return null;
+        }
     }
 }

processor/src/main/java/org/mapstruct/ap/model/assignment/SetterCollectionOrMapWrapper.java

@@ -0,0 +1,48 @@
+/**
+ *  Copyright 2012-2014 Gunnar Morling (http://www.gunnarmorling.de/)
+ *  and/or other contributors as indicated by the @authors tag. See the
+ *  copyright.txt file in the distribution for a full listing of all
+ *  contributors.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.mapstruct.ap.model.assignment;
+
+import org.mapstruct.ap.model.Assignment;
+
+/**
+ * This wrapper handles the situation were an assignment is done via the setter.
+ *
+ * In case of a pre-existing target the wrapper checks if there is an collection or map initialized on the target bean
+ * (not null). If so it uses the addAll (for collections) or putAll (for maps). The collection / map is cleared in case
+ * of a pre-existing target {@link org.mapstruct.MappingTarget }before adding the source entries.
+ *
+ * If there is no pre-existing target, or the target Collection / Map is not initialized (null) the setter is used to
+ * create a new Collection / Map with the copy constructor.
+ *
+ * @author Sjaak Derksen
+ */
+public class SetterCollectionOrMapWrapper extends AssignmentWrapper {
+
+    private final String targetGetterName;
+
+    public SetterCollectionOrMapWrapper( Assignment decoratedAssignment, String targetSetterName ) {
+        super( decoratedAssignment );
+        this.targetGetterName = "get" +  targetSetterName.substring( 3 );
+    }
+
+    public String getTargetGetterName() {
+        return targetGetterName;
+    }
+
+}

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

@@ -92,7 +92,15 @@ public class TypeConversion extends ModelElement implements Assignment {
     }
 
     @Override
-    public boolean isSimple() {
+    public AssignmentType getType() {
-        return false;
+
+        switch ( assignment.getType() ) {
+            case DIRECT:
+                return AssignmentType.TYPE_CONVERTED;
+            case MAPPED:
+                return AssignmentType.MAPPED_TYPE_CONVERTED;
+            default:
+                return null;
+        }
     }
 }

processor/src/main/java/org/mapstruct/ap/model/common/Type.java

@@ -183,6 +183,10 @@ public class Type extends ModelElement implements Comparable<Type> {
         return isMapType;
     }
 
+    public boolean isCollectionOrMapType() {
+        return isCollectionType || isMapType;
+    }
+
     public String getFullyQualifiedName() {
         return qualifiedName;
     }

processor/src/main/java/org/mapstruct/ap/processor/MapperCreationProcessor.java

@@ -26,7 +26,6 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-
 import javax.annotation.processing.Messager;
 import javax.lang.model.element.ExecutableElement;
 import javax.lang.model.element.TypeElement;
@@ -36,8 +35,9 @@ import javax.lang.model.util.ElementFilter;
 import javax.lang.model.util.Elements;
 import javax.lang.model.util.Types;
 import javax.tools.Diagnostic.Kind;
-
 import org.mapstruct.ap.model.Assignment;
+import static org.mapstruct.ap.model.Assignment.AssignmentType.DIRECT;
+import static org.mapstruct.ap.model.Assignment.AssignmentType.TYPE_CONVERTED;
 import org.mapstruct.ap.model.BeanMappingMethod;
 import org.mapstruct.ap.model.Decorator;
 import org.mapstruct.ap.model.DefaultMapperReference;
@@ -51,9 +51,11 @@ import org.mapstruct.ap.model.MapperReference;
 import org.mapstruct.ap.model.MappingMethod;
 import org.mapstruct.ap.model.PropertyMapping;
 import org.mapstruct.ap.model.assignment.AssignmentFactory;
+import org.mapstruct.ap.model.assignment.GetterCollectionOrMapWrapper;
 import org.mapstruct.ap.model.assignment.LocalVarWrapper;
 import org.mapstruct.ap.model.assignment.NewCollectionOrMapWrapper;
 import org.mapstruct.ap.model.assignment.NullCheckWrapper;
+import org.mapstruct.ap.model.assignment.SetterCollectionOrMapWrapper;
 import org.mapstruct.ap.model.assignment.SetterWrapper;
 import org.mapstruct.ap.model.common.Parameter;
 import org.mapstruct.ap.model.common.Type;
@@ -79,6 +81,8 @@ import org.mapstruct.ap.util.Strings;
  */
 public class MapperCreationProcessor implements ModelElementProcessor<List<SourceMethod>, Mapper> {
 
+    private enum TargetAccessorType { GETTER, SETTER };
+
     private Elements elementUtils;
     private Types typeUtils;
     private Messager messager;
@@ -655,14 +659,17 @@ public class MapperCreationProcessor implements ModelElementProcessor<List<Sourc
                                                ExecutableElement sourceAccessor,
                                                ExecutableElement targetAcessor,
                                                String dateFormat) {
+
+        TargetAccessorType targetAccessorType = TargetAccessorType.SETTER;
         Type sourceType = typeFactory.getReturnType( sourceAccessor );
-        Type targetType = null;
+        Type targetType;
         String sourceReference = parameter.getName() + "." + sourceAccessor.getSimpleName().toString() + "()";
         if ( Executables.isSetterMethod( targetAcessor ) ) {
             targetType = typeFactory.getSingleParameter( targetAcessor ).getType();
         }
-        else if ( Executables.isGetterMethod( targetAcessor ) ) {
+        else  { // must be getter
             targetType = typeFactory.getReturnType( targetAcessor );
+            targetAccessorType = TargetAccessorType.GETTER;
         }
 
         String targetPropertyName = Executables.getPropertyName( targetAcessor );
@@ -683,19 +690,50 @@ public class MapperCreationProcessor implements ModelElementProcessor<List<Sourc
 
         if ( assignment != null ) {
 
-            // create a new Map or Collection implementation if no method or type conversion
+            if ( targetType.isCollectionOrMapType() ) {
-            if ( targetType != null && ( targetType.isCollectionType() || targetType.isMapType() ) ) {
+
-                if ( assignment.isSimple() && Executables.isSetterMethod( targetAcessor ) ) {
+                // wrap the assignment in a new Map or Collection implementation if this is not done in a mapping
+                // method. Note, typeconversons do not apply to collections or maps
+                if ( assignment.getType() == DIRECT ) {
+
                     assignment = new NewCollectionOrMapWrapper( assignment );
                 }
+
+                // wrap the assignment in the setter method
+                assignment = new SetterWrapper( assignment, method.getThrownTypes() );
+
+                // wrap the setter in the collection / map initializers
+                switch ( targetAccessorType ) {
+
+                    case GETTER:
+                        // target accessor is getter, so decorate assignment as getter
+                        assignment =  new GetterCollectionOrMapWrapper( assignment,
+                                targetAcessor.getSimpleName().toString() );
+                        break;
+
+                    default: // setter
+
+                        assignment =  new SetterCollectionOrMapWrapper( assignment,
+                                targetAcessor.getSimpleName().toString() );
+                        break;
+                }
+
+                // For collections and maps include a null check, when the assignment type is DIRECT.
+                // for mapping methods (builtin / custom), the mapping method is responsible for the
+                // null check. Typeconversions do not apply to collections and maps.
+                if ( assignment.getType() == DIRECT ) {
+                    assignment = new NullCheckWrapper( assignment );
+                }
+
             }
+            else {
 
-            // target accessor is setter, so decorate assignment as setter
+                assignment = new SetterWrapper( assignment, method.getThrownTypes() );
-            assignment = new SetterWrapper( assignment, method.getThrownTypes() );
+                if ( !sourceType.isPrimitive() && ( assignment.getType() == TYPE_CONVERTED ) ) {
-
+                    // for primitive types null check is not possible at all, but a conversion needs
-            // decorate assignment with null check of source can be null (is not primitive)
+                    // a null check.
-            if ( !sourceType.isPrimitive() ) {
+                    assignment = new NullCheckWrapper( assignment );
-                assignment = new NullCheckWrapper( assignment );
+                }
             }
         }
         else {
@@ -713,10 +751,6 @@ public class MapperCreationProcessor implements ModelElementProcessor<List<Sourc
         }
         return new PropertyMapping(
             parameter.getName(),
-            Executables.getPropertyName( sourceAccessor ),
-            sourceAccessor.getSimpleName().toString(),
-            sourceType,
-            Executables.getPropertyName( targetAcessor ),
             targetAcessor.getSimpleName().toString(),
             targetType,
             assignment
@@ -764,7 +798,7 @@ public class MapperCreationProcessor implements ModelElementProcessor<List<Sourc
 
             // create a new Map or Collection implementation if no method or type conversion
             if ( targetType != null && ( targetType.isCollectionType() || targetType.isMapType() ) ) {
-                if ( assignment.isSimple() ) {
+                if ( assignment.getType() == DIRECT ) {
                     assignment = new NewCollectionOrMapWrapper( assignment );
                 }
             }
@@ -786,13 +820,7 @@ public class MapperCreationProcessor implements ModelElementProcessor<List<Sourc
             );
         }
 
-        return new PropertyMapping(
+        return new PropertyMapping( targetAcessor.getSimpleName().toString(), targetType, assignment );
-            sourceType,
-            targetPropertyName,
-            targetAcessor.getSimpleName().toString(),
-            targetType,
-            assignment
-        );
     }
 
     private IterableMappingMethod getIterableMappingMethod(List<MapperReference> mapperReferences,

processor/src/main/java/org/mapstruct/ap/processor/creation/MappingResolver.java

@@ -35,7 +35,7 @@ import org.mapstruct.ap.model.Assignment;
 import org.mapstruct.ap.model.MapperReference;
 import org.mapstruct.ap.model.VirtualMappingMethod;
 import org.mapstruct.ap.model.assignment.AssignmentFactory;
-import org.mapstruct.ap.model.assignment.Simple;
+import org.mapstruct.ap.model.assignment.Direct;
 import org.mapstruct.ap.model.common.ConversionContext;
 import org.mapstruct.ap.model.common.DefaultConversionContext;
 import org.mapstruct.ap.model.common.Type;
@@ -111,7 +111,7 @@ public class MappingResolver {
      * <ol>
      * <li>MethodReference</li>
      * <li>TypeConversion</li>
-     * <li>Simple Assignment (empty TargetAssignment)</li>
+     * <li>Direct Assignment (empty TargetAssignment)</li>
      * <li>null, no assignment found</li>
      * </ol>
      */
@@ -342,7 +342,7 @@ public class MappingResolver {
                         );
                         if ( conversionXRef != null ) {
                             methodRefY.setAssignment( conversionXRef );
-                            conversionXRef.setAssignment( new Simple( sourceReference ) );
+                            conversionXRef.setAssignment( new Direct( sourceReference ) );
                             break;
                         }
                         else {
@@ -382,7 +382,7 @@ public class MappingResolver {
                         conversionYRef = resolveViaConversion( methodXCandidate.getReturnType(), targetType );
                         if ( conversionYRef != null ) {
                             conversionYRef.setAssignment( methodRefX );
-                            methodRefX.setAssignment( new Simple( sourceReference ) );
+                            methodRefX.setAssignment( new Direct( sourceReference ) );
                             break;
                         }
                         else {

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

@@ -32,7 +32,7 @@
    </#if>
 
     for ( <@includeModel object=sourceParameter.type.typeParameters[0]/> ${loopVariableName} : ${sourceParameter.name} ) {
-     <@includeModel object=elementAssignment target="${resultName}.add" targetType="${resultType.typeParameters[0].name}"/>
+     <@includeModel object=elementAssignment targetBeanName=resultName targetAccessorName="add" targetType="${resultType.typeParameters[0].name}"/>
     }
     <#if returnType.name != "void">
 

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

@@ -34,12 +34,12 @@
     for ( java.util.Map.Entry<<#list sourceParameter.type.typeParameters as typeParameter><@includeModel object=typeParameter /><#if typeParameter_has_next>, </#if></#list>> ${entryVariableName} : ${sourceParameter.name}.entrySet() ) {
     <#-- key -->
         <@includeModel object=keyAssignment
-                   target=keyVariableName
+                   targetAccessorName=keyVariableName
                    targetType=typeName(resultType.typeParameters[0])
                    isLocalVar=true/>
     <#-- value -->
         <@includeModel object=valueAssignment
-                   target=valueVariableName
+                   targetAccessorName=valueVariableName
                    targetType=typeName(resultType.typeParameters[1])
                    isLocalVar=true/>
         ${resultName}.put( ${keyVariableName}, ${valueVariableName} );

processor/src/main/resources/org.mapstruct.ap.model.PropertyMapping.ftl

@@ -18,29 +18,9 @@
      limitations under the License.
 
 -->
- <#if !( targetType.collectionType || targetType.mapType ) >
+<@includeModel object=assignment
-    <#-- non collections or maps -->
+               targetBeanName=ext.targetBeanName
-    <@assignment aTargetType=targetType/>
+               raw=ext.raw
-<#else>
+               existingInstanceMapping=ext.existingInstanceMapping
-    <#-- collections or maps -->
+               targetAccessorName=targetAccessorName
-    <#if ( ext.existingInstanceMapping || !targetAccessorSetter ) >
+               targetType=targetType/>
-        if ( ${ext.targetBeanName}.${targetReadAccessorName}() != null ) {
-            <#if ext.existingInstanceMapping>
-                ${ext.targetBeanName}.${targetReadAccessorName}().clear();
-             </#if><#t>
-                <#if targetType.collectionType>
-                    <@assignment aTarget="${ext.targetBeanName}.${targetReadAccessorName}().addAll"/>
-                <#else>
-                    <@assignment aTarget="${ext.targetBeanName}.${targetReadAccessorName}().putAll"/>
-                </#if>
-        }
-        <#if targetAccessorSetter>
-           else <@assignment/>
-        </#if>
-    <#elseif targetAccessorSetter>
-           <@assignment/>
-     </#if>
- </#if>
- <#macro assignment aTarget="${ext.targetBeanName}.${targetAccessorName}" aTargetType=targetType>
-         <@includeModel object=propertyAssignment target=aTarget targetType=aTargetType raw=true/>
-</#macro>

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

@@ -0,0 +1,40 @@
+<#--
+
+     Copyright 2012-2014 Gunnar Morling (http://www.gunnarmorling.de/)
+     and/or other contributors as indicated by the @authors tag. See the
+     copyright.txt file in the distribution for a full listing of all
+     contributors.
+
+     Licensed under the Apache License, Version 2.0 (the "License");
+     you may not use this file except in compliance with the License.
+     You may obtain a copy of the License at
+
+         http://www.apache.org/licenses/LICENSE-2.0
+
+     Unless required by applicable law or agreed to in writing, software
+     distributed under the License is distributed on an "AS IS" BASIS,
+     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+     See the License for the specific language governing permissions and
+     limitations under the License.
+
+-->
+if ( ${ext.targetBeanName}.${targetGetterName}() != null ) {
+    <#if ext.existingInstanceMapping>
+        ${ext.targetBeanName}.${targetGetterName}().clear();
+    </#if>
+    <#if ext.targetType.collectionType>
+        <@includeModel object=assignment
+                   targetBeanName=ext.targetBeanName
+                   raw=ext.raw
+                   existingInstanceMapping=ext.existingInstanceMapping
+                   targetAccessorName="${targetGetterName}().addAll"
+                   targetType=ext.targetType/>
+    <#else>
+        <@includeModel object=assignment
+                   targetBeanName=ext.targetBeanName
+                   raw=ext.raw
+                   existingInstanceMapping=ext.existingInstanceMapping
+                   targetAccessorName="${targetGetterName}().putAll"
+                   targetType=ext.targetType/>
+    </#if>
+}

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

@@ -19,11 +19,11 @@
 
 -->
 <#if (exceptionTypes?size == 0) >
-    ${ext.targetType} ${ext.target} = <@includeModel object=assignment target=ext.target targetType=ext.targetType raw=ext.raw/>;
+    ${ext.targetType} ${ext.targetAccessorName} = <@includeModel object=assignment targetType=ext.targetType raw=ext.raw/>;
 <#else>
-    ${ext.targetType} ${ext.target};
+    ${ext.targetType} ${ext.targetAccessorName};
     try {
-        ${ext.target} = <@includeModel object=assignment target=ext.target targetType=ext.targetType raw=ext.raw/>;
+        ${ext.targetAccessorName} = <@includeModel object=assignment targetType=ext.targetType raw=ext.raw/>;
     }
     <#list exceptionTypes as exceptionType>
     catch ( <@includeModel object=exceptionType/> e ) {

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

@@ -24,5 +24,5 @@ new <#if ext.targetType.implementationType??>
 <#else>
     <@includeModel object=ext.targetType/>
 </#if>
-( <@includeModel object=assignment target=ext.target targetType=ext.targetType raw=ext.raw/> )
+( <@includeModel object=assignment targetBeanName=ext.targetBeanName targetAccessorName=ext.targetAccessorName targetType=ext.targetType raw=ext.raw/> )
 </@compress>

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

@@ -19,5 +19,10 @@
 
 -->
 if ( ${sourceReference} != null ) {
-    <@includeModel object=assignment target=ext.target targetType=ext.targetType raw=ext.raw/>
+    <@includeModel object=assignment
+                targetBeanName=ext.targetBeanName
+                raw=ext.raw
+                existingInstanceMapping=ext.existingInstanceMapping
+                targetAccessorName=ext.targetAccessorName
+                targetType=ext.targetType/>
 }

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

@@ -0,0 +1,55 @@
+<#--
+
+     Copyright 2012-2014 Gunnar Morling (http://www.gunnarmorling.de/)
+     and/or other contributors as indicated by the @authors tag. See the
+     copyright.txt file in the distribution for a full listing of all
+     contributors.
+
+     Licensed under the Apache License, Version 2.0 (the "License");
+     you may not use this file except in compliance with the License.
+     You may obtain a copy of the License at
+
+         http://www.apache.org/licenses/LICENSE-2.0
+
+     Unless required by applicable law or agreed to in writing, software
+     distributed under the License is distributed on an "AS IS" BASIS,
+     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+     See the License for the specific language governing permissions and
+     limitations under the License.
+
+-->
+<#if ( ext.existingInstanceMapping ) >
+    if ( ${ext.targetBeanName}.${targetGetterName}() != null ) {
+        ${ext.targetBeanName}.${targetGetterName}().clear();
+        <#if ext.targetType.collectionType>
+            <@includeModel object=assignment
+                   targetBeanName=ext.targetBeanName
+                   raw=ext.raw
+                   existingInstanceMapping=ext.existingInstanceMapping
+                   targetAccessorName="${targetGetterName}().addAll"
+                   targetType=ext.targetType/>
+        <#else>
+            <@includeModel object=assignment
+                   targetBeanName=ext.targetBeanName
+                   raw=ext.raw
+                   existingInstanceMapping=ext.existingInstanceMapping
+                   targetAccessorName="${targetGetterName}().putAll"
+                   targetType=ext.targetType/>
+        </#if>
+    }
+    else {
+        <@includeModel object=assignment
+                targetBeanName=ext.targetBeanName
+                raw=ext.raw
+                existingInstanceMapping=ext.existingInstanceMapping
+                targetAccessorName=ext.targetAccessorName
+                targetType=ext.targetType/>
+    }
+<#else>
+    <@includeModel object=assignment
+            targetBeanName=ext.targetBeanName
+            raw=ext.raw
+            existingInstanceMapping=ext.existingInstanceMapping
+            targetAccessorName=ext.targetAccessorName
+            targetType=ext.targetType/>
+</#if>

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

@@ -19,14 +19,17 @@
 
 -->
 <#if (exceptionTypes?size == 0) >
-    ${ext.target}( <@includeModel object=assignment target=ext.target targetType=ext.targetType raw=ext.raw/> );
+    ${ext.targetBeanName}.${ext.targetAccessorName}( <@_assignment/> );
 <#else>
     try {
-        ${ext.target}( <@includeModel object=assignment target=ext.target targetType=ext.targetType raw=ext.raw/> );
+        ${ext.targetBeanName}.${ext.targetAccessorName}( <@_assignment/> );
     }
     <#list exceptionTypes as exceptionType>
     catch ( <@includeModel object=exceptionType/> e ) {
         throw new RuntimeException( e );
     }
     </#list>
-</#if>
+</#if>
+<#macro _assignment>
+    <@includeModel object=assignment raw=ext.raw targetType=ext.targetType/>
+</#macro>

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

@@ -18,4 +18,4 @@
      limitations under the License.
 
 -->
-${openExpression}<@includeModel object=assignment target=ext.target targetType=ext.targetType raw=ext.raw/>${closeExpression}
+${openExpression}<@includeModel object=assignment targetType=ext.targetType raw=ext.raw/>${closeExpression}

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

@@ -20,6 +20,8 @@ package org.mapstruct.ap.test.collection.defaultimplementation;
 
 import java.util.Arrays;
 import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
 
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -53,5 +55,22 @@ public class NoSetterCollectionMappingTest {
 
         assertThat( target.getListValues() ).containsExactly( "foo", "bar" );
         assertThat( target.getMapValues() ).includes( entry( "fooKey", "fooVal" ), entry( "barKey", "barVal" ) );
+
+        // now test existing instances
+
+        NoSetterSource source2 = new NoSetterSource();
+        source2.setListValues( Arrays.asList( "baz" ) );
+        List<String> originalCollectionInstance = target.getListValues();
+        Map<String, String> originalMapInstance = target.getMapValues();
+
+        NoSetterTarget target2 = NoSetterMapper.INSTANCE.toTargetWithExistingTarget( source2, target );
+
+        assertThat( target2.getListValues() ).isSameAs( originalCollectionInstance );
+        assertThat( target2.getListValues() ).containsExactly( "baz" );
+        assertThat( target2.getMapValues() ).isSameAs( originalMapInstance );
+        // source2 mapvalues is empty, so the map is not cleared
+        assertThat( target2.getMapValues() ).includes( entry( "fooKey", "fooVal" ), entry( "barKey", "barVal" ) );
+
+
     }
 }

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

@@ -19,6 +19,7 @@
 package org.mapstruct.ap.test.collection.defaultimplementation;
 
 import org.mapstruct.Mapper;
+import org.mapstruct.MappingTarget;
 import org.mapstruct.factory.Mappers;
 
 /**
@@ -31,4 +32,6 @@ public interface NoSetterMapper {
     NoSetterMapper INSTANCE = Mappers.getMapper( NoSetterMapper.class );
 
     NoSetterTarget toTarget(NoSetterSource source);
+
+    NoSetterTarget toTargetWithExistingTarget(NoSetterSource source, @MappingTarget NoSetterTarget preExistTarget);
 }

processor/src/test/java/org/mapstruct/ap/test/npe/NullObject.java

@@ -0,0 +1,27 @@
+/**
+ *  Copyright 2012-2014 Gunnar Morling (http://www.gunnarmorling.de/)
+ *  and/or other contributors as indicated by the @authors tag. See the
+ *  copyright.txt file in the distribution for a full listing of all
+ *  contributors.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.mapstruct.ap.test.npe;
+
+/**
+ *
+ * @author Sjaak Derksen
+ */
+public class NullObject {
+
+}

processor/src/test/java/org/mapstruct/ap/test/npe/NullObjectMapper.java

@@ -0,0 +1,31 @@
+/**
+ *  Copyright 2012-2014 Gunnar Morling (http://www.gunnarmorling.de/)
+ *  and/or other contributors as indicated by the @authors tag. See the
+ *  copyright.txt file in the distribution for a full listing of all
+ *  contributors.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.mapstruct.ap.test.npe;
+
+/**
+ *
+ * @author Sjaak Derksen
+ */
+public class NullObjectMapper {
+
+
+    public String toNullString(NullObject in) {
+        return in.toString();
+    }
+}

processor/src/test/java/org/mapstruct/ap/test/npe/NullPtrCheckTest.java

@@ -0,0 +1,73 @@
+/**
+ *  Copyright 2012-2014 Gunnar Morling (http://www.gunnarmorling.de/)
+ *  and/or other contributors as indicated by the @authors tag. See the
+ *  copyright.txt file in the distribution for a full listing of all
+ *  contributors.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.mapstruct.ap.test.npe;
+
+import static org.fest.assertions.Assertions.assertThat;
+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;
+
+/**
+ * Test for correct handling of null checks.
+ *
+ * @author Sjaak Derksen
+ */
+@IssueKey( "134" )
+@WithClasses( {
+    SourceTargetMapper.class,
+    NullObjectMapper.class,
+    NullObject.class,
+    Source.class,
+    Target.class
+} )
+@RunWith( AnnotationProcessorTestRunner.class )
+public class NullPtrCheckTest {
+
+    @Test( expected = NullPointerException.class )
+    public void shouldThrowNullptrWhenCustomMapperIsInvoked() {
+
+        Source source = new Source();
+        source.setNumber( "5" );
+        SourceTargetMapper.INSTANCE.sourceToTarget( source );
+    }
+
+    @Test
+    public void shouldSurroundTypeConversionWithNPECheck() {
+
+        Source source = new Source();
+        source.setSomeObject( new NullObject() );
+        Target target =  SourceTargetMapper.INSTANCE.sourceToTarget( source );
+
+        assertThat( target.getNumber() ).isNull();
+
+    }
+
+   @Test
+    public void shouldSurroundArrayListConstructionWithNPECheck() {
+
+        Source source = new Source();
+        source.setSomeObject( new NullObject() );
+        Target target =  SourceTargetMapper.INSTANCE.sourceToTarget( source );
+
+        assertThat( target.getSomeList() ).isNull();
+    }
+
+}

processor/src/test/java/org/mapstruct/ap/test/npe/Source.java

@@ -0,0 +1,59 @@
+/**
+ *  Copyright 2012-2014 Gunnar Morling (http://www.gunnarmorling.de/)
+ *  and/or other contributors as indicated by the @authors tag. See the
+ *  copyright.txt file in the distribution for a full listing of all
+ *  contributors.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.mapstruct.ap.test.npe;
+
+import java.util.List;
+
+/**
+ *
+ * @author Sjaak Derksen
+ */
+public class Source {
+
+    private NullObject someObject;
+    private String number;
+    private List<String> someList;
+
+    public NullObject getSomeObject() {
+        return someObject;
+    }
+
+    public void setSomeObject( NullObject someObject ) {
+        this.someObject = someObject;
+    }
+
+    public String getNumber() {
+        return number;
+    }
+
+    public void setNumber( String number ) {
+        this.number = number;
+    }
+
+    public List<String> getSomeList() {
+        return someList;
+    }
+
+    public void setSomeList( List<String> someList ) {
+        this.someList = someList;
+    }
+
+
+
+}

processor/src/test/java/org/mapstruct/ap/test/npe/SourceTargetMapper.java

@@ -0,0 +1,34 @@
+/**
+ *  Copyright 2012-2014 Gunnar Morling (http://www.gunnarmorling.de/)
+ *  and/or other contributors as indicated by the @authors tag. See the
+ *  copyright.txt file in the distribution for a full listing of all
+ *  contributors.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.mapstruct.ap.test.npe;
+
+import org.mapstruct.Mapper;
+import org.mapstruct.factory.Mappers;
+
+/**
+ *
+ * @author Sjaak Derksen
+ */
+@Mapper (uses = NullObjectMapper.class)
+public interface SourceTargetMapper {
+
+    SourceTargetMapper INSTANCE = Mappers.getMapper( SourceTargetMapper.class );
+
+    Target sourceToTarget(Source source);
+}

processor/src/test/java/org/mapstruct/ap/test/npe/Target.java

@@ -0,0 +1,59 @@
+/**
+ *  Copyright 2012-2014 Gunnar Morling (http://www.gunnarmorling.de/)
+ *  and/or other contributors as indicated by the @authors tag. See the
+ *  copyright.txt file in the distribution for a full listing of all
+ *  contributors.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.mapstruct.ap.test.npe;
+
+import java.util.List;
+
+/**
+ *
+ * @author Sjaak Derksen
+ */
+public class Target {
+
+    private String someObject;
+    private Integer number;
+    private List<String> someList;
+
+    public String getSomeObject() {
+        return someObject;
+    }
+
+    public void setSomeObject( String someObject ) {
+        this.someObject = someObject;
+    }
+
+    public Integer getNumber() {
+        return number;
+    }
+
+    public void setNumber( Integer number ) {
+        this.number = number;
+    }
+
+    public List<String> getSomeList() {
+        return someList;
+    }
+
+    public void setSomeList( List<String> someList ) {
+        this.someList = someList;
+    }
+
+
+
+}