diff --git a/processor/src/main/java/org/mapstruct/ap/model/MapMappingMethod.java b/processor/src/main/java/org/mapstruct/ap/model/MapMappingMethod.java index 20318f0fb..20ee546b8 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/MapMappingMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/model/MapMappingMethod.java @@ -27,7 +27,6 @@ import org.mapstruct.ap.model.assignment.Assignment; import org.mapstruct.ap.model.assignment.LocalVarWrapper; import org.mapstruct.ap.model.common.Parameter; import org.mapstruct.ap.model.common.Type; -import org.mapstruct.ap.model.common.TypeFactory; import org.mapstruct.ap.model.source.Method; import org.mapstruct.ap.prism.NullValueMappingPrism; import org.mapstruct.ap.util.MapperConfig; @@ -46,7 +45,6 @@ public class MapMappingMethod extends MappingMethod { private final MethodReference factoryMethod; private final boolean overridden; private final boolean mapNullToDefault; - private final TypeFactory typeFactory; public static class Builder { @@ -153,14 +151,13 @@ public class MapMappingMethod extends MappingMethod { keyAssignment, valueAssignment, factoryMethod, - mapNullToDefault, - ctx.getTypeFactory() + mapNullToDefault ); } } private MapMappingMethod(Method method, Assignment keyAssignment, Assignment valueAssignment, - MethodReference factoryMethod, boolean mapNullToDefault, TypeFactory typeFactory) { + MethodReference factoryMethod, boolean mapNullToDefault) { super( method ); this.keyAssignment = keyAssignment; @@ -168,7 +165,6 @@ public class MapMappingMethod extends MappingMethod { this.factoryMethod = factoryMethod; this.overridden = method.overridesMethod(); this.mapNullToDefault = mapNullToDefault; - this.typeFactory = typeFactory; } public Parameter getSourceParameter() { diff --git a/processor/src/main/java/org/mapstruct/ap/model/MappingBuilderContext.java b/processor/src/main/java/org/mapstruct/ap/model/MappingBuilderContext.java index 7164ea192..b5ebbbb53 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/MappingBuilderContext.java +++ b/processor/src/main/java/org/mapstruct/ap/model/MappingBuilderContext.java @@ -168,6 +168,25 @@ public class MappingBuilderContext { return mappingsToGenerate; } + public List getNamesOfMappingsToGenerate() { + List nameList = new ArrayList(); + for ( MappingMethod method : mappingsToGenerate ) { + nameList.add( method.getName() ); + } + return nameList; + } + + public MappingMethod getExistingMappingMethod(MappingMethod newMappingMethod) { + MappingMethod existingMappingMethod = null; + for ( MappingMethod mappingMethod : mappingsToGenerate ) { + if ( newMappingMethod.equals( mappingMethod ) ) { + existingMappingMethod = mappingMethod; + break; + } + } + return existingMappingMethod; + } + public Set getUsedVirtualMappings() { return mappingResolver.getUsedVirtualMappings(); } diff --git a/processor/src/main/java/org/mapstruct/ap/model/PropertyMapping.java b/processor/src/main/java/org/mapstruct/ap/model/PropertyMapping.java index a6ca797bf..644807ed6 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/PropertyMapping.java +++ b/processor/src/main/java/org/mapstruct/ap/model/PropertyMapping.java @@ -48,10 +48,9 @@ import static org.mapstruct.ap.model.assignment.Assignment.AssignmentType.TYPE_C import static org.mapstruct.ap.model.assignment.Assignment.AssignmentType.TYPE_CONVERTED_MAPPED; /** - * 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 - * and target property can differ. If they have different types, the mapping - * must either refer to a mapping method or a conversion. + * 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 and target property can differ. If they have different types, the + * mapping must either refer to a mapping method or a conversion. * * @author Gunnar Morling */ @@ -169,7 +168,7 @@ public class PropertyMapping extends ModelElement { Diagnostic.Kind.ERROR, String.format( "Can't map %s to \"%s %s\". " - + "Consider to declare/implement a mapping method: \"%s map(%s value)\".", + + "Consider to declare/implement a mapping method: \"%s map(%s value)\".", sourceElement, targetType, targetPropertyName, @@ -268,7 +267,6 @@ public class PropertyMapping extends ModelElement { return result; } - private Type getSourceType() { Parameter sourceParam = sourceReference.getParameter(); @@ -304,16 +302,17 @@ public class PropertyMapping extends ModelElement { PropertyEntry lastPropertyEntry = propertyEntries.get( propertyEntries.size() - 1 ); // forge a method from the parameter type to the last entry type. - String forgedMethodName = Strings.joinAndCamelize( sourceReference.getElementNames() ); - ForgedMethod methodToGenerate = new ForgedMethod( - forgedMethodName, + String forgedName = Strings.joinAndCamelize( sourceReference.getElementNames() ); + forgedName = Strings.getSaveVariableName( forgedName, ctx.getNamesOfMappingsToGenerate() ); + ForgedMethod methodRef = new ForgedMethod( + forgedName, sourceReference.getParameter().getType(), lastPropertyEntry.getType(), method.getExecutable() ); NestedPropertyMappingMethod.Builder builder = new NestedPropertyMappingMethod.Builder(); NestedPropertyMappingMethod nestedPropertyMapping = builder - .method( methodToGenerate ) + .method( methodRef ) .propertyEntries( sourceReference.getPropertyEntries() ) .build(); @@ -321,8 +320,11 @@ public class PropertyMapping extends ModelElement { if ( !ctx.getMappingsToGenerate().contains( nestedPropertyMapping ) ) { ctx.getMappingsToGenerate().add( nestedPropertyMapping ); } + else { + forgedName = ctx.getExistingMappingMethod( nestedPropertyMapping ).getName(); + } - return forgedMethodName + "( " + sourceParam.getName() + " )"; + return forgedName + "( " + sourceParam.getName() + " )"; } } @@ -374,43 +376,70 @@ public class PropertyMapping extends ModelElement { ExecutableElement element) { Assignment assignment = null; - if ( ( sourceType.isCollectionType() || sourceType.isArrayType() ) - && ( targetType.isCollectionType() || targetType.isArrayType() ) ) { - ForgedMethod methodToGenerate = new ForgedMethod( sourceType, targetType, element ); + String name = getName( sourceType, targetType ); + name = Strings.getSaveVariableName( name, ctx.getNamesOfMappingsToGenerate() ); + + if ( (sourceType.isCollectionType() || sourceType.isArrayType()) + && (targetType.isCollectionType() || targetType.isArrayType()) ) { + + ForgedMethod methodRef = new ForgedMethod( name, sourceType, targetType, element ); IterableMappingMethod.Builder builder = new IterableMappingMethod.Builder(); - IterableMappingMethod iterableMappingMethod = builder .mappingContext( ctx ) - .method( methodToGenerate ) + .method( methodRef ) .build(); if ( !ctx.getMappingsToGenerate().contains( iterableMappingMethod ) ) { ctx.getMappingsToGenerate().add( iterableMappingMethod ); } - assignment = AssignmentFactory.createMethodReference( methodToGenerate, null, targetType ); + else { + String existingName = ctx.getExistingMappingMethod( iterableMappingMethod ).getName(); + methodRef = new ForgedMethod( existingName, methodRef ); + } + + assignment = AssignmentFactory.createMethodReference( methodRef, null, targetType ); assignment.setAssignment( AssignmentFactory.createDirect( sourceReference ) ); } else if ( sourceType.isMapType() && targetType.isMapType() ) { - ForgedMethod methodToGenerate = new ForgedMethod( sourceType, targetType, element ); + ForgedMethod methodRef = new ForgedMethod( name, sourceType, targetType, element ); MapMappingMethod.Builder builder = new MapMappingMethod.Builder(); MapMappingMethod mapMappingMethod = builder .mappingContext( ctx ) - .method( methodToGenerate ) + .method( methodRef ) .build(); if ( !ctx.getMappingsToGenerate().contains( mapMappingMethod ) ) { ctx.getMappingsToGenerate().add( mapMappingMethod ); } - assignment = AssignmentFactory.createMethodReference( methodToGenerate, null, targetType ); + else { + String existingName = ctx.getExistingMappingMethod( mapMappingMethod ).getName(); + methodRef = new ForgedMethod( existingName, methodRef ); + } + assignment = AssignmentFactory.createMethodReference( methodRef, null, targetType ); assignment.setAssignment( AssignmentFactory.createDirect( sourceReference ) ); } return assignment; } + + private String getName(Type sourceType, Type targetType) { + String fromName = getName( sourceType ); + String toName = getName( targetType ); + return Strings.decapitalize( fromName + "To" + toName ); + } + + private String getName(Type type) { + StringBuilder builder = new StringBuilder(); + for ( Type typeParam : type.getTypeParameters() ) { + builder.append( typeParam.getName().replace( "[]", "Array" ) ); + } + builder.append( type.getName().replace( "[]", "Array" ) ); + return builder.toString(); + } } public static class ConstantMappingBuilder { @@ -554,10 +583,8 @@ public class PropertyMapping extends ModelElement { return new PropertyMapping( targetAccessor.getSimpleName().toString(), targetType, assignment ); } - } - // Constructor for creating mappings of constant expressions. private PropertyMapping(String targetAccessorName, Type targetType, Assignment propertyAssignment) { this( null, targetAccessorName, targetType, propertyAssignment ); @@ -596,10 +623,10 @@ public class PropertyMapping extends ModelElement { @Override public String toString() { - return "PropertyMapping {" + - "\n targetName='" + targetAccessorName + "\'," + - "\n targetType=" + targetType + "," + - "\n propertyAssignment=" + assignment + - "\n}"; + return "PropertyMapping {" + + "\n targetName='" + targetAccessorName + "\'," + + "\n targetType=" + targetType + "," + + "\n propertyAssignment=" + assignment + + "\n}"; } } diff --git a/processor/src/main/java/org/mapstruct/ap/model/source/ForgedMethod.java b/processor/src/main/java/org/mapstruct/ap/model/source/ForgedMethod.java index 355012f0d..7e864afc6 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/source/ForgedMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/model/source/ForgedMethod.java @@ -41,29 +41,7 @@ public class ForgedMethod implements Method { private final String name; private final ExecutableElement positionHintElement; - /** - * Creates a new forged method. - *

- * The name will be based on the source type name and target type name. - * - * @param sourceType the source type - * @param targetType the target type. - * @param positionHintElement element used to for reference to the position in the source file. - */ - public ForgedMethod(Type sourceType, Type targetType, ExecutableElement positionHintElement) { - String sourceParamName = Strings.decapitalize( sourceType.getName().replace( "[]", "" ) ); - String sourceParamSafeName = Strings.getSaveVariableName( sourceParamName ); - this.parameters = Arrays.asList( new Parameter( sourceParamSafeName, sourceType ) ); - this.returnType = targetType; - - String fromName = getName( parameters.iterator().next().getType() ); - String toName = getName( returnType ); - name = Strings.decapitalize( fromName + "To" + toName ); - - this.positionHintElement = positionHintElement; - } - - /** + /** * Creates a new forged method with the given name. * * @param name the (unique name) for this method @@ -80,14 +58,16 @@ public class ForgedMethod implements Method { this.positionHintElement = positionHintElement; } - private String getName(Type type) { - StringBuilder builder = new StringBuilder(); - for ( Type typeParam : type.getTypeParameters() ) { - - builder.append( typeParam.getName().replace( "[]", "Array" ) ); - } - builder.append( type.getName().replace( "[]", "Array" ) ); - return builder.toString(); + /** + * creates a new ForgedMethod with the same arguments but with a new name + * @param name the new name + * @param forgedMethod existing forge method + */ + public ForgedMethod(String name, ForgedMethod forgedMethod) { + this.parameters = forgedMethod.parameters; + this.returnType = forgedMethod.returnType; + this.positionHintElement = forgedMethod.positionHintElement; + this.name = name; } @Override diff --git a/processor/src/main/resources/org.mapstruct.ap.model.assignment.LocalVarWrapper.ftl b/processor/src/main/resources/org.mapstruct.ap.model.assignment.LocalVarWrapper.ftl index 5b862b949..7ad666a16 100644 --- a/processor/src/main/resources/org.mapstruct.ap.model.assignment.LocalVarWrapper.ftl +++ b/processor/src/main/resources/org.mapstruct.ap.model.assignment.LocalVarWrapper.ftl @@ -19,9 +19,9 @@ --> <#if (exceptionTypes?size == 0) > - <#if !ext.isTargetDefined?? >${ext.targetType.name} ${ext.targetAccessorName} = <@includeModel object=assignment targetType=ext.targetType raw=ext.raw/>; + <#if !ext.isTargetDefined?? ><@includeModel object=ext.targetType/> ${ext.targetAccessorName} = <@includeModel object=assignment targetType=ext.targetType raw=ext.raw/>; <#else> - <#if !ext.isTargetDefined?? >${ext.targetType.name} ${ext.targetAccessorName}; + <#if !ext.isTargetDefined?? ><@includeModel object=ext.targetType/> ${ext.targetAccessorName}; try { ${ext.targetAccessorName} = <@includeModel object=assignment targetType=ext.targetType raw=ext.raw/>; } diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/map/SameClassNameInDifferentPackageTest.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_394/SameClassNameInDifferentPackageTest.java similarity index 60% rename from processor/src/test/java/org/mapstruct/ap/test/collection/map/SameClassNameInDifferentPackageTest.java rename to processor/src/test/java/org/mapstruct/ap/test/bugs/_394/SameClassNameInDifferentPackageTest.java index 7c6fa4e08..ff2dace76 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/collection/map/SameClassNameInDifferentPackageTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_394/SameClassNameInDifferentPackageTest.java @@ -16,12 +16,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.mapstruct.ap.test.collection.map; +package org.mapstruct.ap.test.bugs._394; import org.junit.Test; import org.junit.runner.RunWith; -import org.mapstruct.ap.test.collection.map.source.AnotherCar; -import org.mapstruct.ap.test.collection.map.source.Cars; +import org.mapstruct.ap.test.bugs._394.source.AnotherCar; +import org.mapstruct.ap.test.bugs._394.source.Cars; import org.mapstruct.ap.testutil.IssueKey; import org.mapstruct.ap.testutil.WithClasses; import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner; @@ -31,31 +31,31 @@ import java.util.Map; import static org.fest.assertions.Assertions.assertThat; -@WithClasses({ SameNameForSourceAndTargetCarsMapper.class, Cars.class, org.mapstruct.ap.test.collection.map.targets.Cars - .class, AnotherCar.class, org.mapstruct.ap.test.collection.map.targets.AnotherCar.class }) +@WithClasses( { + SameNameForSourceAndTargetCarsMapper.class, + Cars.class, + org.mapstruct.ap.test.bugs._394.target.Cars.class, + AnotherCar.class, + org.mapstruct.ap.test.bugs._394.target.AnotherCar.class +} ) @IssueKey("394") @RunWith(AnnotationProcessorTestRunner.class) public class SameClassNameInDifferentPackageTest { + @Test public void shouldCreateMapMethodImplementation() { Map values = new HashMap(); //given - AnotherCar honda = new AnotherCar( - "Honda", - 2 - ); - AnotherCar toyota = new AnotherCar( - "Toyota", - 2 - ); - values.put("Honda", honda); - values.put("Toyota", toyota); + AnotherCar honda = new AnotherCar( "Honda", 2 ); + AnotherCar toyota = new AnotherCar( "Toyota", 2); + values.put( "Honda", honda ); + values.put( "Toyota", toyota ); Cars cars = new Cars(); - cars.setMakeToCar(values); - org.mapstruct.ap.test.collection.map.targets.Cars targetCars = SameNameForSourceAndTargetCarsMapper.INSTANCE - .sourceCarsToTargetCars(cars); - assertThat(targetCars).isNotNull(); - assertThat(targetCars.getMakeToCar()).hasSize(2); + cars.setMakeToCar( values ); + org.mapstruct.ap.test.bugs._394.target.Cars targetCars = SameNameForSourceAndTargetCarsMapper.INSTANCE + .sourceCarsToTargetCars( cars ); + assertThat( targetCars ).isNotNull(); + assertThat( targetCars.getMakeToCar() ).hasSize( 2 ); } } diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/map/SameNameForSourceAndTargetCarsMapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_394/SameNameForSourceAndTargetCarsMapper.java similarity index 53% rename from processor/src/test/java/org/mapstruct/ap/test/collection/map/SameNameForSourceAndTargetCarsMapper.java rename to processor/src/test/java/org/mapstruct/ap/test/bugs/_394/SameNameForSourceAndTargetCarsMapper.java index c9a59264e..a1a6bc54d 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/collection/map/SameNameForSourceAndTargetCarsMapper.java +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_394/SameNameForSourceAndTargetCarsMapper.java @@ -16,27 +16,39 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.mapstruct.ap.test.collection.map; +package org.mapstruct.ap.test.bugs._394; import org.mapstruct.Mapper; import org.mapstruct.Mapping; import org.mapstruct.Mappings; -import org.mapstruct.ap.test.collection.map.source.Cars; -import org.mapstruct.ap.test.collection.map.targets.AnotherCar; +import org.mapstruct.ap.test.bugs._394.source.Cars; +import org.mapstruct.ap.test.bugs._394.target.AnotherCar; import org.mapstruct.factory.Mappers; import java.util.List; +import org.mapstruct.InheritInverseConfiguration; @Mapper public interface SameNameForSourceAndTargetCarsMapper { - SameNameForSourceAndTargetCarsMapper INSTANCE = Mappers.getMapper(SameNameForSourceAndTargetCarsMapper.class); + SameNameForSourceAndTargetCarsMapper INSTANCE = Mappers.getMapper( SameNameForSourceAndTargetCarsMapper.class ); @Mappings({ - @Mapping(source = "numberOfSeats", target = "seatCount") + @Mapping(source = "numberOfSeats", target = "seatCount") }) - AnotherCar carToCarDto(org.mapstruct.ap.test.collection.map.source.AnotherCar car); + AnotherCar sourceCarToTargetCar(org.mapstruct.ap.test.bugs._394.source.AnotherCar car); - List carsToCarDtos(List cars); - org.mapstruct.ap.test.collection.map.targets.Cars sourceCarsToTargetCars(Cars source); -} \ No newline at end of file + List sourceCarListToTargetCarList(List cars); + + org.mapstruct.ap.test.bugs._394.target.Cars sourceCarsToTargetCars(Cars source); + + // Reverse mehtods + + @InheritInverseConfiguration + org.mapstruct.ap.test.bugs._394.source.AnotherCar targetCarToSourceCar(AnotherCar car); + + List targetCarListToSourceCarList(List cars); + + Cars targetCarsToSourceCars(org.mapstruct.ap.test.bugs._394.target.Cars source); + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/map/source/AnotherCar.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_394/source/AnotherCar.java similarity index 96% rename from processor/src/test/java/org/mapstruct/ap/test/collection/map/source/AnotherCar.java rename to processor/src/test/java/org/mapstruct/ap/test/bugs/_394/source/AnotherCar.java index 0a95699e6..c19361e00 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/collection/map/source/AnotherCar.java +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_394/source/AnotherCar.java @@ -16,7 +16,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.mapstruct.ap.test.collection.map.source; +package org.mapstruct.ap.test.bugs._394.source; public class AnotherCar { diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/map/source/Cars.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_394/source/Cars.java similarity index 95% rename from processor/src/test/java/org/mapstruct/ap/test/collection/map/source/Cars.java rename to processor/src/test/java/org/mapstruct/ap/test/bugs/_394/source/Cars.java index ab8aaef53..b74e251e5 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/collection/map/source/Cars.java +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_394/source/Cars.java @@ -16,7 +16,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.mapstruct.ap.test.collection.map.source; +package org.mapstruct.ap.test.bugs._394.source; import java.util.Map; diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/map/targets/AnotherCar.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_394/target/AnotherCar.java similarity index 96% rename from processor/src/test/java/org/mapstruct/ap/test/collection/map/targets/AnotherCar.java rename to processor/src/test/java/org/mapstruct/ap/test/bugs/_394/target/AnotherCar.java index e15df9fb7..12d844afa 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/collection/map/targets/AnotherCar.java +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_394/target/AnotherCar.java @@ -16,7 +16,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.mapstruct.ap.test.collection.map.targets; +package org.mapstruct.ap.test.bugs._394.target; public class AnotherCar { diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/map/targets/Cars.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_394/target/Cars.java similarity index 95% rename from processor/src/test/java/org/mapstruct/ap/test/collection/map/targets/Cars.java rename to processor/src/test/java/org/mapstruct/ap/test/bugs/_394/target/Cars.java index 493cbc365..0e06a8c1a 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/collection/map/targets/Cars.java +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_394/target/Cars.java @@ -16,7 +16,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.mapstruct.ap.test.collection.map.targets; +package org.mapstruct.ap.test.bugs._394.target; import java.util.Map;