#899 Refactoring adderwrapper, simplifying SourceRHS.

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

@@ -18,6 +18,7 @@
  */
 package org.mapstruct.ap.internal.model;
 
+import java.util.HashSet;
 import static org.mapstruct.ap.internal.util.Collections.first;
 
 import java.util.List;
@@ -105,12 +106,11 @@ public class IterableMappingMethod extends MappingMethod {
             Assignment assignment = ctx.getMappingResolver().getTargetAssignment(
                 method,
                 "collection element",
-                sourceElementType,
                 targetElementType,
                 null, // there is no targetPropertyName
                 formattingParameters,
                 selectionParameters,
-                new SourceRHS( loopVariableName, sourceElementType ),
+                new SourceRHS( loopVariableName, sourceElementType, new HashSet<String>() ),
                 false
             );
 

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

@@ -18,6 +18,7 @@
  */
 package org.mapstruct.ap.internal.model;
 
+import java.util.HashSet;
 import static org.mapstruct.ap.internal.util.Collections.first;
 
 import java.util.List;
@@ -108,12 +109,11 @@ public class MapMappingMethod extends MappingMethod {
             Assignment keyAssignment = ctx.getMappingResolver().getTargetAssignment(
                 method,
                 "map key",
-                keySourceType,
                 keyTargetType,
                 null, // there is no targetPropertyName
                 keyFormattingParameters,
                 keySelectionParameters,
-                new SourceRHS( "entry.getKey()", keySourceType ),
+                new SourceRHS( "entry.getKey()", keySourceType, new HashSet<String>() ),
                 false
             );
 
@@ -135,12 +135,11 @@ public class MapMappingMethod extends MappingMethod {
             Assignment valueAssignment = ctx.getMappingResolver().getTargetAssignment(
                 method,
                 "map value",
-                valueSourceType,
                 valueTargetType,
                 null, // there is no targetPropertyName
                 valueFormattingParameters,
                 valueSelectionParameters,
-                new SourceRHS( "entry.getValue()", valueSourceType ),
+                new SourceRHS( "entry.getValue()", valueSourceType, new HashSet<String>() ),
                 false
             );
 

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

@@ -76,7 +76,6 @@ public class MappingBuilderContext {
          *
          * @param mappingMethod target mapping method
          * @param mappedElement used for error messages
-         * @param sourceType parameter to match
          * @param targetType return type to match
          * @param targetPropertyName name of the target property
          * @param formattingParameters used for formatting dates and numbers
@@ -93,7 +92,7 @@ public class MappingBuilderContext {
          * </ol>
          */
         @SuppressWarnings("checkstyle:parameternumber")
-        Assignment getTargetAssignment(Method mappingMethod, String mappedElement, Type sourceType, Type targetType,
+        Assignment getTargetAssignment(Method mappingMethod, String mappedElement, Type targetType,
                                        String targetPropertyName, FormattingParameters formattingParameters,
                                        SelectionParameters selectionParameters, SourceRHS sourceRHS,
                                        boolean preferUpdateMethods);

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

@@ -133,6 +133,11 @@ public class MethodReference extends MappingMethod implements Assignment {
         return assignment.getSourceType();
     }
 
+    @Override
+    public String createLocalVarName( String desiredName ) {
+        return assignment.createLocalVarName( desiredName );
+    }
+
     @Override
     public String getSourceLocalVarName() {
         return assignment.getSourceLocalVarName();

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

@@ -212,15 +212,11 @@ public class PropertyMapping extends ModelElement {
             // handle source
             String sourceElement = getSourceElement();
             Type sourceType = getSourceType();
-            SourceRHS sourceRHS;
+            SourceRHS sourceRHS = getSourceRHS();
             if ( targetWriteAccessorType == TargetWriteAccessorType.ADDER && sourceType.isCollectionType() ) {
                 // handle adder, if source is collection then use iterator element type as source type.
-                // sourceRef becomes a local variable in the itereation.
                 sourceType = sourceType.getTypeParameters().get( 0 );
-                sourceRHS = new SourceRHS( Executables.getElementNameForAdder( targetWriteAccessor ), sourceType );
+                sourceRHS = new SourceRHS( sourceRHS.getSourceReference(), sourceType, existingVariableNames );
-            }
-            else {
-                sourceRHS = getSourceRHS();
             }
 
             // all the tricky cases will be excluded for the time being.
@@ -235,7 +231,6 @@ public class PropertyMapping extends ModelElement {
             Assignment assignment = ctx.getMappingResolver().getTargetAssignment(
                 method,
                 sourceElement,
-                sourceType,
                 targetType,
                 targetPropertyName,
                 formattingParameters,
@@ -320,7 +315,7 @@ public class PropertyMapping extends ModelElement {
                 result = assignToPlainViaSetter( sourceType, targetType, rightHandSide );
             }
             else {
-                result = assignToPlainViaAdder( sourceType, rightHandSide );
+                result = assignToPlainViaAdder( rightHandSide );
             }
             return result;
 
@@ -391,18 +386,12 @@ public class PropertyMapping extends ModelElement {
             }
         }
 
-        private Assignment assignToPlainViaAdder(Type sourceType, Assignment rightHandSide) {
+        private Assignment assignToPlainViaAdder( Assignment rightHandSide) {
 
             Assignment result = rightHandSide;
 
             if ( getSourceType().isCollectionType() ) {
-                result = new AdderWrapper(
+                result = new AdderWrapper( result, method.getThrownTypes() );
-                    result,
-                    method.getThrownTypes(),
-                    getSourceRHS().getSourceReference(),
-                    sourceType
-                );
-                result = new NullCheckWrapper( result, getSourcePresenceCheckerRef() );
             }
             else {
                 // Possibly adding null to a target collection. So should be surrounded by an null check.
@@ -495,13 +484,13 @@ public class PropertyMapping extends ModelElement {
 
             // parameter reference
             if ( propertyEntries.isEmpty() ) {
-                return new SourceRHS( sourceParam.getName(), getSourceType() );
+                return new SourceRHS( sourceParam.getName(), getSourceType(), existingVariableNames );
             }
             // simple property
             else if ( propertyEntries.size() == 1 ) {
                 PropertyEntry propertyEntry = propertyEntries.get( 0 );
-                return new SourceRHS( sourceParam.getName()
+                String sourceRef = sourceParam.getName() + "." + propertyEntry.getReadAccessor().getSimpleName() + "()";
-                    + "." + propertyEntry.getReadAccessor().getSimpleName() + "()", propertyEntry.getType() );
+                return new SourceRHS( sourceRef, propertyEntry.getType(), existingVariableNames );
             }
             // nested property given as dot path
             else {
@@ -532,7 +521,8 @@ public class PropertyMapping extends ModelElement {
                 else {
                     forgedName = ctx.getExistingMappingMethod( nestedPropertyMapping ).getName();
                 }
-                return new SourceRHS( forgedName + "( " + sourceParam.getName() + " )", getSourceType() );
+                String sourceRef = forgedName + "( " + sourceParam.getName() + " )";
+                return new SourceRHS( sourceRef, getSourceType(), existingVariableNames );
 
             }
         }
@@ -686,12 +676,11 @@ public class PropertyMapping extends ModelElement {
                 assignment = ctx.getMappingResolver().getTargetAssignment(
                     method,
                     mappedElement,
-                    sourceType,
                     targetType,
                     targetPropertyName,
                     formattingParameters,
                     selectionParameters,
-                    new SourceRHS( constantExpression, sourceType ),
+                    new SourceRHS( constantExpression, sourceType, existingVariableNames ),
                     method.getMappingTargetParameter() != null
                 );
             }
@@ -756,7 +745,7 @@ public class PropertyMapping extends ModelElement {
             // String String quotation marks.
             String enumExpression = constantExpression.substring( 1, constantExpression.length() - 1 );
             if ( targetType.getEnumConstants().contains( enumExpression ) ) {
-                assignment = new SourceRHS( enumExpression, targetType );
+                assignment = new SourceRHS( enumExpression, targetType, existingVariableNames );
                 assignment = new EnumConstantWrapper( assignment, targetType );
             }
             else {
@@ -782,7 +771,7 @@ public class PropertyMapping extends ModelElement {
         }
 
         public PropertyMapping build() {
-            Assignment assignment = new SourceRHS( javaExpression, null );
+            Assignment assignment = new SourceRHS( javaExpression, null, existingVariableNames );
 
             if ( Executables.isSetterMethod( targetWriteAccessor ) ) {
                 // setter, so wrap in setter

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

@@ -25,6 +25,7 @@ import java.util.Set;
 import org.mapstruct.ap.internal.model.assignment.Assignment;
 import org.mapstruct.ap.internal.model.common.ModelElement;
 import org.mapstruct.ap.internal.model.common.Type;
+import org.mapstruct.ap.internal.util.Strings;
 
 /**
  * SourceRHS Assignment. Right Hand Side (RHS), source part of the assignment.
@@ -36,10 +37,12 @@ public class SourceRHS extends ModelElement implements Assignment {
     private final String sourceReference;
     private final Type sourceType;
     private String sourceLocalVarName;
+    private final Set<String> existingVariableNames;
 
-    public SourceRHS(String sourceReference, Type sourceType ) {
+    public SourceRHS(String sourceReference, Type sourceType, Set<String> existingVariableNames ) {
         this.sourceReference = sourceReference;
         this.sourceType = sourceType;
+        this.existingVariableNames = existingVariableNames;
     }
 
     @Override
@@ -52,6 +55,11 @@ public class SourceRHS extends ModelElement implements Assignment {
         return sourceType;
     }
 
+    @Override
+    public String createLocalVarName(String desiredName) {
+        return Strings.getSaveVariableName( desiredName, existingVariableNames );
+    }
+
     @Override
     public String getSourceLocalVarName() {
         return sourceLocalVarName;

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

@@ -91,6 +91,11 @@ public class TypeConversion extends ModelElement implements Assignment {
         return assignment.getSourceType();
     }
 
+    @Override
+    public String createLocalVarName( String desiredName ) {
+        return assignment.createLocalVarName( desiredName );
+    }
+
     @Override
     public String getSourceLocalVarName() {
         return assignment.getSourceLocalVarName();

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

@@ -33,18 +33,14 @@ import org.mapstruct.ap.internal.model.common.Type;
 public class AdderWrapper extends AssignmentWrapper {
 
     private final List<Type> thrownTypesToExclude;
-    private final String sourceReference;
+    private final String sourceIteratorName;
-    private final Type sourceType;
 
-    public AdderWrapper(
+    public AdderWrapper( Assignment decoratedAssignment, List<Type> thrownTypesToExclude ) {
-        Assignment decoratedAssignment,
-        List<Type> thrownTypesToExclude,
-        String sourceReference,
-        Type sourceType) {
         super( decoratedAssignment );
         this.thrownTypesToExclude = thrownTypesToExclude;
-        this.sourceReference = sourceReference;
+        this.sourceIteratorName =
-        this.sourceType = sourceType;
+            decoratedAssignment.createLocalVarName( decoratedAssignment.getSourceType().getName() );
+        decoratedAssignment.setSourceLocalVarName( sourceIteratorName );
     }
 
     @Override
@@ -61,25 +57,15 @@ public class AdderWrapper extends AssignmentWrapper {
         return result;
     }
 
-    public Type getSourceType() {
-        return sourceType;
-    }
-
-    @Override
-    public String getSourceReference() {
-        return sourceReference;
-    }
-
     @Override
     public Set<Type> getImportTypes() {
         Set<Type> imported = new HashSet<Type>();
         imported.addAll( super.getImportTypes() );
-        imported.add( sourceType );
+        imported.add( getSourceType() );
         return imported;
     }
 
-    public String getIteratorReference() {
+    public String getSourceIteratorName() {
-        return getAssignment().getSourceReference();
+        return sourceIteratorName;
     }
-
 }

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

@@ -78,12 +78,20 @@ public interface Assignment {
     String getSourceReference();
 
     /**
-     * the source type.
+     * the source type used in the matching process
      *
      * @return source type (can be null)
      */
     Type getSourceType();
 
+    /**
+     * safe (local) element variable name when dealing with collections.
+     *
+     * @param desiredName the desired name
+     * @return the desired name, made unique
+     */
+    String createLocalVarName( String desiredName );
+
     /**
      * a local variable name for supporting a null check and avoiding executing a nested method forged method twice
      *

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

@@ -86,4 +86,9 @@ public abstract class AssignmentWrapper extends ModelElement implements Assignme
         return decoratedAssignment.isUpdateMethod();
     }
 
+    @Override
+    public String createLocalVarName( String desiredName ) {
+        return decoratedAssignment.createLocalVarName( desiredName );
+    }
+
 }

processor/src/main/java/org/mapstruct/ap/internal/processor/creation/MappingResolverImpl.java

@@ -106,7 +106,7 @@ public class MappingResolverImpl implements MappingResolver {
 
     @Override
     @SuppressWarnings("checkstyle:parameternumber")
-    public Assignment getTargetAssignment(Method mappingMethod, String mappedElement, Type sourceType,
+    public Assignment getTargetAssignment(Method mappingMethod, String mappedElement,
         Type targetType, String targetPropertyName, FormattingParameters formattingParameters,
         SelectionParameters selectionParameters, SourceRHS sourceRHS, boolean preferUpdateMapping) {
 
@@ -130,7 +130,7 @@ public class MappingResolverImpl implements MappingResolver {
             criteria
         );
 
-        return attempt.getTargetAssignment( sourceType, targetType );
+        return attempt.getTargetAssignment( sourceRHS.getSourceType(), targetType );
     }
 
     @Override

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

@@ -18,29 +18,11 @@
      limitations under the License.
 
 -->
-<#if (thrownTypes?size == 0) >
+<#import "../macro/CommonMacros.ftl" as lib>
-    for ( <@includeModel object=sourceType/> ${iteratorReference} : ${sourceReference} ) {
+<@lib.handleExceptions>
-        ${ext.targetBeanName}.${ext.targetWriteAccessorName}( <@includeModel object=assignment
+  if ( ${sourceReference} != null ) {
-                targetBeanName=ext.targetBeanName
+      for ( <@includeModel object=sourceType/> ${sourceIteratorName} : ${sourceReference} ) {
-                existingInstanceMapping=ext.existingInstanceMapping
+          ${ext.targetBeanName}.${ext.targetWriteAccessorName}( <@lib.handleAssignment/> );
-                targetReadAccessorName=ext.targetReadAccessorName
-                targetWriteAccessorName=ext.targetWriteAccessorName
-                targetType=ext.targetType/> );
-    }
-<#else>
-    try {
-        for ( <@includeModel object=sourceType/> ${iteratorReference} : ${sourceReference} ) {
-            ${ext.targetBeanName}.${ext.targetWriteAccessorName}( <@includeModel object=assignment
-                    targetBeanName=ext.targetBeanName
-                    existingInstanceMapping=ext.existingInstanceMapping
-                    targetReadAccessorName=ext.targetReadAccessorName
-                    targetWriteAccessorName=ext.targetWriteAccessorName
-                    targetType=ext.targetType/> );
       }
   }
-    <#list thrownTypes as exceptionType>
+</@lib.handleExceptions>
-    catch ( <@includeModel object=exceptionType/> e ) {
-        throw new RuntimeException( e );
-    }
-    </#list>
-</#if>