#19 Requiring @MappingTarget annotation for target parameters for the sake of readability/explicitness; Improving test method names

processor/src/main/java/org/mapstruct/ap/util/Executables.java

@@ -21,7 +21,6 @@ package org.mapstruct.ap.util;
 import java.beans.Introspector;
 import java.util.ArrayList;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
 import javax.lang.model.element.ExecutableElement;
@@ -129,22 +128,13 @@ public class Executables {
         List<? extends VariableElement> parameters = method.getParameters();
         List<Parameter> result = new ArrayList<Parameter>( parameters.size() );
 
-        boolean mappingTargetDefined = false;
+        for ( VariableElement parameter : parameters ) {
-        for ( Iterator<? extends VariableElement> it = parameters.iterator(); it.hasNext(); ) {
-            VariableElement parameter = it.next();
-
-            boolean isExplicitMappingTarget = null != MappingTargetPrism.getInstanceOn( parameter );
-            mappingTargetDefined |= isExplicitMappingTarget;
-
             result
                 .add(
                     new Parameter(
                         parameter.getSimpleName().toString(),
                         typeUtil.retrieveType( parameter.asType() ),
-                        // the parameter is a mapping target, if it was either defined explicitly or if if this is the
+                        MappingTargetPrism.getInstanceOn( parameter ) != null
-                        // last parameter in a multi-argument void method
-                        isExplicitMappingTarget
-                            || ( !mappingTargetDefined && isMultiArgVoidMethod( method ) && !it.hasNext() )
                     )
                 );
         }
@@ -152,10 +142,6 @@ public class Executables {
         return result;
     }
 
-    public boolean isMultiArgVoidMethod(ExecutableElement method) {
-        return method.getParameters().size() > 1 && Type.VOID == retrieveReturnType( method );
-    }
-
     public Type retrieveReturnType(ExecutableElement method) {
         return typeUtil.retrieveType( method.getReturnType() );
     }

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

@@ -95,28 +95,20 @@ public class DefaultCollectionImplementationTest extends MapperTestBase {
 
     @Test
     @IssueKey("19")
-    public void existingMapping1() {
+    public void shouldUseTargetParameterForMapping() {
         List<TargetFoo> target = new ArrayList<TargetFoo>();
-        SourceTargetMapper.INSTANCE.sourceFoosToTargetFoos1( createSourceFooList(), target );
+        SourceTargetMapper.INSTANCE.sourceFoosToTargetFoosUsingTargetParameter( target, createSourceFooList() );
 
         assertResultList( target );
     }
 
     @Test
     @IssueKey("19")
-    public void existingMapping2() {
+    public void shouldUseAndReturnTargetParameterForMapping() {
-        List<TargetFoo> target = new ArrayList<TargetFoo>();
-        SourceTargetMapper.INSTANCE.sourceFoosToTargetFoos2( target, createSourceFooList() );
-
-        assertResultList( target );
-    }
-
-    @Test
-    @IssueKey("19")
-    public void existingMapping3() {
         List<TargetFoo> target = new ArrayList<TargetFoo>();
         Iterable<TargetFoo> result =
-            SourceTargetMapper.INSTANCE.sourceFoosToTargetFoos3( createSourceFooList(), target );
+            SourceTargetMapper.INSTANCE
+                .sourceFoosToTargetFoosUsingTargetParameterAndReturn( createSourceFooList(), target );
 
         assertThat( target == result ).isTrue();
         assertResultList( target );

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

@@ -43,10 +43,9 @@ public interface SourceTargetMapper {
 
     Iterable<TargetFoo> sourceFoosToTargetFoos(Iterable<SourceFoo> foos);
 
-    void sourceFoosToTargetFoos1(Iterable<SourceFoo> sourceFoos, List<TargetFoo> targetFoos);
+    void sourceFoosToTargetFoosUsingTargetParameter(@MappingTarget List<TargetFoo> targetFoos,
+                                                    Iterable<SourceFoo> sourceFoos);
 
-    void sourceFoosToTargetFoos2(@MappingTarget List<TargetFoo> targetFoos, Iterable<SourceFoo> sourceFoos);
+    Iterable<TargetFoo> sourceFoosToTargetFoosUsingTargetParameterAndReturn(Iterable<SourceFoo> sourceFoos,
-
+                                                                            @MappingTarget List<TargetFoo> targetFoos);
-    Iterable<TargetFoo> sourceFoosToTargetFoos3(Iterable<SourceFoo> sourceFoos,
-                                                @MappingTarget List<TargetFoo> targetFoos);
 }

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

@@ -67,39 +67,27 @@ public class MapMappingTest extends MapperTestBase {
 
     @Test
     @IssueKey("19")
-    public void shouldCreateReverseMapMethodImplementation1() {
+    public void shouldCreateMapMethodImplementationWithTargetParameter() {
         Map<String, String> values = createStringStringMap();
 
         Map<Long, Date> target = new HashMap<Long, Date>();
         target.put( 66L, new GregorianCalendar( 2013, 7, 16 ).getTime() );
 
-        SourceTargetMapper.INSTANCE.stringStringMapToLongDateMap( values, target );
+        SourceTargetMapper.INSTANCE.stringStringMapToLongDateMapUsingTargetParameter( target, values );
 
         assertResult( target );
     }
 
     @Test
     @IssueKey("19")
-    public void shouldCreateReverseMapMethodImplementation2() {
+    public void shouldCreateMapMethodImplementationWithReturnedTargetParameter() {
         Map<String, String> values = createStringStringMap();
 
         Map<Long, Date> target = new HashMap<Long, Date>();
         target.put( 66L, new GregorianCalendar( 2013, 7, 16 ).getTime() );
 
-        SourceTargetMapper.INSTANCE.stringStringMapToLongDateMap2( target, values );
+        Map<Long, Date> returnedTarget = SourceTargetMapper.INSTANCE
-
+            .stringStringMapToLongDateMapUsingTargetParameterAndReturn( values, target );
-        assertResult( target );
-    }
-
-    @Test
-    @IssueKey("19")
-    public void shouldCreateReverseMapMethodImplementation3() {
-        Map<String, String> values = createStringStringMap();
-
-        Map<Long, Date> target = new HashMap<Long, Date>();
-        target.put( 66L, new GregorianCalendar( 2013, 7, 16 ).getTime() );
-
-        Map<Long, Date> returnedTarget = SourceTargetMapper.INSTANCE.stringStringMapToLongDateMap3( values, target );
 
         assertThat( target ).isSameAs( returnedTarget );
 

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

@@ -37,13 +37,12 @@ public interface SourceTargetMapper {
     Map<Long, Date> stringStringMapToLongDateMap(Map<String, String> source);
 
     @MapMapping(valueDateFormat = "dd.MM.yyyy")
-    void stringStringMapToLongDateMap(Map<String, String> source, Map<Long, Date> target);
+    void stringStringMapToLongDateMapUsingTargetParameter(@MappingTarget Map<Long, Date> target,
+                                                          Map<String, String> source);
 
     @MapMapping(valueDateFormat = "dd.MM.yyyy")
-    void stringStringMapToLongDateMap2(@MappingTarget Map<Long, Date> target, Map<String, String> source);
+    Map<Long, Date> stringStringMapToLongDateMapUsingTargetParameterAndReturn(Map<String, String> source,
-
+                                                                              @MappingTarget Map<Long, Date> target);
-    @MapMapping(valueDateFormat = "dd.MM.yyyy")
-    Map<Long, Date> stringStringMapToLongDateMap3(Map<String, String> source, @MappingTarget Map<Long, Date> target);
 
     Target sourceToTarget(Source source);
 

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

@@ -45,33 +45,22 @@ public class InheritanceTest extends MapperTestBase {
 
     @Test
     @IssueKey("19")
-    public void existingMapping1() {
+    public void shouldMapAttributeFromSuperTypeUsingTargetParameter() {
         SourceExt source = createSource();
 
         TargetExt target = new TargetExt();
-        SourceTargetMapper.INSTANCE.sourceToTarget1( source, target );
+        SourceTargetMapper.INSTANCE.sourceToTargetWithTargetParameter( target, source );
 
         assertResult( target );
     }
 
     @Test
     @IssueKey("19")
-    public void existingMapping2() {
+    public void shouldMapAttributeFromSuperTypeUsingReturnedTargetParameter() {
         SourceExt source = createSource();
 
         TargetExt target = new TargetExt();
-        SourceTargetMapper.INSTANCE.sourceToTarget2( target, source );
+        TargetBase result = SourceTargetMapper.INSTANCE.sourceToTargetWithTargetParameterAndReturn( source, target );
-
-        assertResult( target );
-    }
-
-    @Test
-    @IssueKey("19")
-    public void existingMapping3() {
-        SourceExt source = createSource();
-
-        TargetExt target = new TargetExt();
-        TargetBase result = SourceTargetMapper.INSTANCE.sourceToTarget3( source, target );
 
         assertThat( target ).isSameAs( result );
 

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

@@ -29,11 +29,9 @@ public interface SourceTargetMapper {
 
     TargetExt sourceToTarget(SourceExt source);
 
-    void sourceToTarget1(SourceExt source, TargetExt target);
+    void sourceToTargetWithTargetParameter(@MappingTarget TargetExt target, SourceExt source);
 
-    void sourceToTarget2(@MappingTarget TargetExt target, SourceExt source);
+    TargetBase sourceToTargetWithTargetParameterAndReturn(SourceExt source, @MappingTarget TargetExt target);
-
-    TargetBase sourceToTarget3(SourceExt source, @MappingTarget TargetExt target);
 
     SourceExt targetToSource(TargetExt target);
 }