From 592743179109b781adc77ed8102b6fd94e2201b9 Mon Sep 17 00:00:00 2001 From: Andreas Gudian Date: Thu, 15 Sep 2016 22:50:00 +0200 Subject: [PATCH] #895 Fix generating forged iterable methods for mapping multi-dimensional Array types --- .../SetterWrapperForCollectionsAndMaps.java | 2 - .../internal/model/source/ForgedMethod.java | 4 +- .../mapstruct/ap/internal/util/Strings.java | 9 ++- .../internal/model/IterableMappingMethod.ftl | 3 +- .../ap/test/bugs/_895/Issue895Test.java | 50 ++++++++++++++++ .../ap/test/bugs/_895/MultiArrayMapper.java | 57 +++++++++++++++++++ 6 files changed, 119 insertions(+), 6 deletions(-) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_895/Issue895Test.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_895/MultiArrayMapper.java diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.java b/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.java index a7c590d53..926d4b5d9 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.java @@ -41,7 +41,6 @@ public class SetterWrapperForCollectionsAndMaps extends AssignmentWrapper { private final String targetGetterName; private final Assignment newCollectionOrMapAssignment; - private final Type targetType; private final String localVarName; public SetterWrapperForCollectionsAndMaps(Assignment decoratedAssignment, @@ -53,7 +52,6 @@ public class SetterWrapperForCollectionsAndMaps extends AssignmentWrapper { this.targetGetterName = targetGetterName; this.newCollectionOrMapAssignment = newCollectionOrMapAssignment; - this.targetType = targetType; this.localVarName = Strings.getSaveVariableName( targetType.getName(), existingVariableNames ); existingVariableNames.add( localVarName ); } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/ForgedMethod.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/ForgedMethod.java index 163cb4080..c3405287c 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/ForgedMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/ForgedMethod.java @@ -57,12 +57,12 @@ public class ForgedMethod implements Method { */ public ForgedMethod(String name, Type sourceType, Type targetType, MapperConfiguration mapperConfiguration, ExecutableElement positionHintElement) { - String sourceParamName = Strings.decapitalize( sourceType.getName().replace( "[]", "" ) ); + String sourceParamName = Strings.decapitalize( sourceType.getName() ); String sourceParamSafeName = Strings.getSaveVariableName( sourceParamName ); this.parameters = Arrays.asList( new Parameter( sourceParamSafeName, sourceType ) ); this.returnType = targetType; this.thrownTypes = new ArrayList(); - this.name = name; + this.name = Strings.sanitizeIdentifierName( name ); this.mapperConfiguration = mapperConfiguration; this.positionHintElement = positionHintElement; } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/Strings.java b/processor/src/main/java/org/mapstruct/ap/internal/util/Strings.java index 8552ee3fe..28bc2f9bd 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/util/Strings.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/util/Strings.java @@ -150,7 +150,7 @@ public class Strings { * any Java keyword; starting with a lower-case letter */ public static String getSaveVariableName(String name, Collection existingVariableNames) { - name = decapitalize( name ); + name = decapitalize( sanitizeIdentifierName( name ) ); Set conflictingNames = new HashSet( KEYWORDS ); conflictingNames.addAll( existingVariableNames ); @@ -162,4 +162,11 @@ public class Strings { return name; } + /** + * @param identifier identifier to sanitize + * @return the identifier without any characters that are not allowed as part of a Java identifier. + */ + public static String sanitizeIdentifierName(String identifier) { + return identifier.replace( "[]", "Array" ); + } } diff --git a/processor/src/main/resources/org/mapstruct/ap/internal/model/IterableMappingMethod.ftl b/processor/src/main/resources/org/mapstruct/ap/internal/model/IterableMappingMethod.ftl index 270ac68b5..6705ad7b7 100644 --- a/processor/src/main/resources/org/mapstruct/ap/internal/model/IterableMappingMethod.ftl +++ b/processor/src/main/resources/org/mapstruct/ap/internal/model/IterableMappingMethod.ftl @@ -54,7 +54,8 @@ <#if resultType.arrayType> <#if !existingInstanceMapping> - <@includeModel object=resultElementType/>[] ${resultName} = new <@includeModel object=resultElementType/>[<@iterableSize/>]; + <#assign elementTypeString><@includeModel object=resultElementType/> + ${elementTypeString}[] ${resultName} = new ${elementTypeString?keep_before('[]')}[<@iterableSize/>]${elementTypeString?replace('[^\\[\\]]+', '', 'r')}; <#else> <#if existingInstanceMapping> diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_895/Issue895Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_895/Issue895Test.java new file mode 100644 index 000000000..d8b24c44a --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_895/Issue895Test.java @@ -0,0 +1,50 @@ +/** + * Copyright 2012-2016 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.bugs._895; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mapstruct.ap.test.bugs._895.MultiArrayMapper.WithArrayOfByteArray; +import org.mapstruct.ap.test.bugs._895.MultiArrayMapper.WithListOfByteArray; +import org.mapstruct.ap.testutil.WithClasses; +import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner; +import org.mapstruct.factory.Mappers; + +/** + * Verifies that forged iterable mapping methods for multi-dimensional arrays are generated properly. + * + * @author Andreas Gudian + */ +@RunWith(AnnotationProcessorTestRunner.class) +@WithClasses(MultiArrayMapper.class) +public class Issue895Test { + @Test + public void properlyMapsMultiDimensionalArrays() { + WithArrayOfByteArray arrayOfByteArray = new WithArrayOfByteArray(); + arrayOfByteArray.setBytes( new byte[][] { new byte[] { 0, 1 }, new byte[] { 1, 2 } } ); + + WithListOfByteArray listOfByteArray = Mappers.getMapper( MultiArrayMapper.class ).convert( arrayOfByteArray ); + assertThat( listOfByteArray.getBytes() ).containsExactly( new byte[] { 0, 1 }, new byte[] { 1, 2 } ); + + arrayOfByteArray = Mappers.getMapper( MultiArrayMapper.class ).convert( listOfByteArray ); + assertThat( arrayOfByteArray.getBytes() ).containsExactly( new byte[] { 0, 1 }, new byte[] { 1, 2 } ); + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_895/MultiArrayMapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_895/MultiArrayMapper.java new file mode 100644 index 000000000..fcb29cd9f --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_895/MultiArrayMapper.java @@ -0,0 +1,57 @@ +/** + * Copyright 2012-2016 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.bugs._895; + +import java.util.List; + +import org.mapstruct.Mapper; + +/** + * @author Andreas Gudian + */ +@Mapper +public interface MultiArrayMapper { + WithListOfByteArray convert(WithArrayOfByteArray b); + + WithArrayOfByteArray convert(WithListOfByteArray b); + + class WithArrayOfByteArray { + private byte[][] bytes; + + public byte[][] getBytes() { + return bytes; + } + + public void setBytes(byte[][] bytes) { + this.bytes = bytes; + } + } + + class WithListOfByteArray { + private List bytes; + + public List getBytes() { + return bytes; + } + + public void setBytes(List bytes) { + this.bytes = bytes; + } + } +}