From 45968f9fd7352569865dba5444a28725683ab035 Mon Sep 17 00:00:00 2001 From: Gunnar Morling Date: Sun, 21 Jul 2013 12:23:43 +0200 Subject: [PATCH] #19 Requiring @MappingTarget annotation for target parameters for the sake of readability/explicitness; Improving test method names --- .../org/mapstruct/ap/util/Executables.java | 18 ++------------- .../DefaultCollectionImplementationTest.java | 18 +++++---------- .../SourceTargetMapper.java | 9 ++++---- .../test/collection/map/MapMappingTest.java | 22 +++++-------------- .../collection/map/SourceTargetMapper.java | 9 ++++---- .../ap/test/inheritance/InheritanceTest.java | 19 ++++------------ .../test/inheritance/SourceTargetMapper.java | 6 ++--- 7 files changed, 26 insertions(+), 75 deletions(-) diff --git a/processor/src/main/java/org/mapstruct/ap/util/Executables.java b/processor/src/main/java/org/mapstruct/ap/util/Executables.java index 10c9e4a80..c6681fc55 100644 --- a/processor/src/main/java/org/mapstruct/ap/util/Executables.java +++ b/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 parameters = method.getParameters(); List result = new ArrayList( parameters.size() ); - boolean mappingTargetDefined = false; - for ( Iterator it = parameters.iterator(); it.hasNext(); ) { - VariableElement parameter = it.next(); - - boolean isExplicitMappingTarget = null != MappingTargetPrism.getInstanceOn( parameter ); - mappingTargetDefined |= isExplicitMappingTarget; - + for ( VariableElement parameter : parameters ) { 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 - // last parameter in a multi-argument void method - isExplicitMappingTarget - || ( !mappingTargetDefined && isMultiArgVoidMethod( method ) && !it.hasNext() ) + MappingTargetPrism.getInstanceOn( parameter ) != null ) ); } @@ -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() ); } diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/DefaultCollectionImplementationTest.java b/processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/DefaultCollectionImplementationTest.java index b025484fc..49ceec2b3 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/DefaultCollectionImplementationTest.java +++ b/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 target = new ArrayList(); - SourceTargetMapper.INSTANCE.sourceFoosToTargetFoos1( createSourceFooList(), target ); + SourceTargetMapper.INSTANCE.sourceFoosToTargetFoosUsingTargetParameter( target, createSourceFooList() ); assertResultList( target ); } @Test @IssueKey("19") - public void existingMapping2() { - List target = new ArrayList(); - SourceTargetMapper.INSTANCE.sourceFoosToTargetFoos2( target, createSourceFooList() ); - - assertResultList( target ); - } - - @Test - @IssueKey("19") - public void existingMapping3() { + public void shouldUseAndReturnTargetParameterForMapping() { List target = new ArrayList(); Iterable result = - SourceTargetMapper.INSTANCE.sourceFoosToTargetFoos3( createSourceFooList(), target ); + SourceTargetMapper.INSTANCE + .sourceFoosToTargetFoosUsingTargetParameterAndReturn( createSourceFooList(), target ); assertThat( target == result ).isTrue(); assertResultList( target ); diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/SourceTargetMapper.java b/processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/SourceTargetMapper.java index 20cc46392..c34a051e6 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/SourceTargetMapper.java +++ b/processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/SourceTargetMapper.java @@ -43,10 +43,9 @@ public interface SourceTargetMapper { Iterable sourceFoosToTargetFoos(Iterable foos); - void sourceFoosToTargetFoos1(Iterable sourceFoos, List targetFoos); + void sourceFoosToTargetFoosUsingTargetParameter(@MappingTarget List targetFoos, + Iterable sourceFoos); - void sourceFoosToTargetFoos2(@MappingTarget List targetFoos, Iterable sourceFoos); - - Iterable sourceFoosToTargetFoos3(Iterable sourceFoos, - @MappingTarget List targetFoos); + Iterable sourceFoosToTargetFoosUsingTargetParameterAndReturn(Iterable sourceFoos, + @MappingTarget List targetFoos); } diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/map/MapMappingTest.java b/processor/src/test/java/org/mapstruct/ap/test/collection/map/MapMappingTest.java index c6f3c5dc4..a1139afb4 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/collection/map/MapMappingTest.java +++ b/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 values = createStringStringMap(); Map target = new HashMap(); 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 values = createStringStringMap(); Map target = new HashMap(); target.put( 66L, new GregorianCalendar( 2013, 7, 16 ).getTime() ); - SourceTargetMapper.INSTANCE.stringStringMapToLongDateMap2( target, values ); - - assertResult( target ); - } - - @Test - @IssueKey("19") - public void shouldCreateReverseMapMethodImplementation3() { - Map values = createStringStringMap(); - - Map target = new HashMap(); - target.put( 66L, new GregorianCalendar( 2013, 7, 16 ).getTime() ); - - Map returnedTarget = SourceTargetMapper.INSTANCE.stringStringMapToLongDateMap3( values, target ); + Map returnedTarget = SourceTargetMapper.INSTANCE + .stringStringMapToLongDateMapUsingTargetParameterAndReturn( values, target ); assertThat( target ).isSameAs( returnedTarget ); diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/map/SourceTargetMapper.java b/processor/src/test/java/org/mapstruct/ap/test/collection/map/SourceTargetMapper.java index 4f1100306..6b90d0b0d 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/collection/map/SourceTargetMapper.java +++ b/processor/src/test/java/org/mapstruct/ap/test/collection/map/SourceTargetMapper.java @@ -37,13 +37,12 @@ public interface SourceTargetMapper { Map stringStringMapToLongDateMap(Map source); @MapMapping(valueDateFormat = "dd.MM.yyyy") - void stringStringMapToLongDateMap(Map source, Map target); + void stringStringMapToLongDateMapUsingTargetParameter(@MappingTarget Map target, + Map source); @MapMapping(valueDateFormat = "dd.MM.yyyy") - void stringStringMapToLongDateMap2(@MappingTarget Map target, Map source); - - @MapMapping(valueDateFormat = "dd.MM.yyyy") - Map stringStringMapToLongDateMap3(Map source, @MappingTarget Map target); + Map stringStringMapToLongDateMapUsingTargetParameterAndReturn(Map source, + @MappingTarget Map target); Target sourceToTarget(Source source); diff --git a/processor/src/test/java/org/mapstruct/ap/test/inheritance/InheritanceTest.java b/processor/src/test/java/org/mapstruct/ap/test/inheritance/InheritanceTest.java index e898c7f5b..41bf326a7 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/inheritance/InheritanceTest.java +++ b/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 ); - - assertResult( target ); - } - - @Test - @IssueKey("19") - public void existingMapping3() { - SourceExt source = createSource(); - - TargetExt target = new TargetExt(); - TargetBase result = SourceTargetMapper.INSTANCE.sourceToTarget3( source, target ); + TargetBase result = SourceTargetMapper.INSTANCE.sourceToTargetWithTargetParameterAndReturn( source, target ); assertThat( target ).isSameAs( result ); diff --git a/processor/src/test/java/org/mapstruct/ap/test/inheritance/SourceTargetMapper.java b/processor/src/test/java/org/mapstruct/ap/test/inheritance/SourceTargetMapper.java index bfed071be..a10d30270 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/inheritance/SourceTargetMapper.java +++ b/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 sourceToTarget3(SourceExt source, @MappingTarget TargetExt target); + TargetBase sourceToTargetWithTargetParameterAndReturn(SourceExt source, @MappingTarget TargetExt target); SourceExt targetToSource(TargetExt target); }