From 8fab3dd4e5002a2c9f2288efa9635f46386fd04f Mon Sep 17 00:00:00 2001 From: sjaakd Date: Wed, 1 Mar 2017 22:23:00 +0100 Subject: [PATCH] #1111 local variabele should not clash with loop variable --- .../ap/internal/model/BeanMappingMethod.java | 4 ++ .../model/ContainerMappingMethod.java | 16 ++++-- .../model/ContainerMappingMethodBuilder.java | 5 +- .../internal/model/IterableMappingMethod.java | 11 ++-- .../ap/internal/model/MapMappingMethod.java | 9 ++-- .../model/NormalTypeMappingMethod.java | 6 ++- .../internal/model/StreamMappingMethod.java | 11 ++-- .../ap/test/bugs/_1111/Issue1111Mapper.java | 44 +++++++++++++++ .../ap/test/bugs/_1111/Issue1111Test.java | 53 +++++++++++++++++++ 9 files changed, 140 insertions(+), 19 deletions(-) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1111/Issue1111Mapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1111/Issue1111Test.java diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java b/processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java index c9e438bd6..8911fef2d 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java @@ -22,6 +22,7 @@ import static org.mapstruct.ap.internal.util.Collections.first; import java.text.MessageFormat; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; @@ -211,6 +212,7 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { return new BeanMappingMethod( method, + existingVariableNames, propertyMappings, factoryMethod, mapNullToDefault, @@ -724,6 +726,7 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { } private BeanMappingMethod(Method method, + Collection existingVariableNames, List propertyMappings, MethodReference factoryMethod, boolean mapNullToDefault, @@ -732,6 +735,7 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { List afterMappingReferences) { super( method, + existingVariableNames, factoryMethod, mapNullToDefault, beforeMappingReferences, diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/ContainerMappingMethod.java b/processor/src/main/java/org/mapstruct/ap/internal/model/ContainerMappingMethod.java index 171a69a0e..8628ebe5b 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/ContainerMappingMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/ContainerMappingMethod.java @@ -18,6 +18,7 @@ */ package org.mapstruct.ap.internal.model; +import java.util.Collection; import java.util.List; import java.util.Set; @@ -39,16 +40,21 @@ public abstract class ContainerMappingMethod extends NormalTypeMappingMethod { private final Assignment elementAssignment; private final String loopVariableName; private final SelectionParameters selectionParameters; + private final String index1Name; + private final String index2Name; - ContainerMappingMethod(Method method, Assignment parameterAssignment, MethodReference factoryMethod, - boolean mapNullToDefault, String loopVariableName, + ContainerMappingMethod(Method method, Collection existingVariables, Assignment parameterAssignment, + MethodReference factoryMethod, boolean mapNullToDefault, String loopVariableName, List beforeMappingReferences, List afterMappingReferences, SelectionParameters selectionParameters) { - super( method, factoryMethod, mapNullToDefault, beforeMappingReferences, afterMappingReferences ); + super( method, existingVariables, factoryMethod, mapNullToDefault, beforeMappingReferences, + afterMappingReferences ); this.elementAssignment = parameterAssignment; this.loopVariableName = loopVariableName; this.selectionParameters = selectionParameters; + this.index1Name = Strings.getSaveVariableName( "i", existingVariables ); + this.index2Name = Strings.getSaveVariableName( "j", existingVariables ); } public Parameter getSourceParameter() { @@ -81,11 +87,11 @@ public abstract class ContainerMappingMethod extends NormalTypeMappingMethod { public abstract Type getResultElementType(); public String getIndex1Name() { - return Strings.getSaveVariableName( "i", loopVariableName, getSourceParameter().getName(), getResultName() ); + return index1Name; } public String getIndex2Name() { - return Strings.getSaveVariableName( "j", loopVariableName, getSourceParameter().getName(), getResultName() ); + return index2Name; } @Override diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/ContainerMappingMethodBuilder.java b/processor/src/main/java/org/mapstruct/ap/internal/model/ContainerMappingMethodBuilder.java index 009f507cd..be5d4ea99 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/ContainerMappingMethodBuilder.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/ContainerMappingMethodBuilder.java @@ -20,6 +20,7 @@ package org.mapstruct.ap.internal.model; import static org.mapstruct.ap.internal.util.Collections.first; +import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -143,6 +144,7 @@ public abstract class ContainerMappingMethodBuilder existingVariables, + Assignment assignment, MethodReference factoryMethod, boolean mapNullToDefault, String loopVariableName, List beforeMappingMethods, List afterMappingMethods, diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/IterableMappingMethod.java b/processor/src/main/java/org/mapstruct/ap/internal/model/IterableMappingMethod.java index 1f264afd3..2edafea3b 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/IterableMappingMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/IterableMappingMethod.java @@ -20,6 +20,7 @@ package org.mapstruct.ap.internal.model; import static org.mapstruct.ap.internal.util.Collections.first; +import java.util.Collection; import java.util.List; import org.mapstruct.ap.internal.model.assignment.Assignment; @@ -62,12 +63,13 @@ public class IterableMappingMethod extends ContainerMappingMethod { } @Override - protected IterableMappingMethod instantiateMappingMethod(Method method, Assignment assignment, - MethodReference factoryMethod, boolean mapNullToDefault, String loopVariableName, + protected IterableMappingMethod instantiateMappingMethod(Method method, Collection existingVariables, + Assignment assignment, MethodReference factoryMethod, boolean mapNullToDefault, String loopVariableName, List beforeMappingMethods, List afterMappingMethods, SelectionParameters selectionParameters) { return new IterableMappingMethod( method, + existingVariables, assignment, factoryMethod, mapNullToDefault, @@ -79,13 +81,14 @@ public class IterableMappingMethod extends ContainerMappingMethod { } } - private IterableMappingMethod(Method method, Assignment parameterAssignment, MethodReference factoryMethod, - boolean mapNullToDefault, String loopVariableName, + private IterableMappingMethod(Method method, Collection existingVariables, Assignment parameterAssignment, + MethodReference factoryMethod, boolean mapNullToDefault, String loopVariableName, List beforeMappingReferences, List afterMappingReferences, SelectionParameters selectionParameters) { super( method, + existingVariables, parameterAssignment, factoryMethod, mapNullToDefault, diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/MapMappingMethod.java b/processor/src/main/java/org/mapstruct/ap/internal/model/MapMappingMethod.java index 861cd4b1a..d7f59ecf7 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/MapMappingMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/MapMappingMethod.java @@ -18,6 +18,7 @@ */ package org.mapstruct.ap.internal.model; +import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -161,6 +162,7 @@ public class MapMappingMethod extends NormalTypeMappingMethod { return new MapMappingMethod( method, + existingVariables, keyAssignment, valueAssignment, factoryMethod, @@ -176,11 +178,12 @@ public class MapMappingMethod extends NormalTypeMappingMethod { } } - private MapMappingMethod(Method method, Assignment keyAssignment, Assignment valueAssignment, - MethodReference factoryMethod, boolean mapNullToDefault, + private MapMappingMethod(Method method, Collection existingVariableNames, Assignment keyAssignment, + Assignment valueAssignment, MethodReference factoryMethod, boolean mapNullToDefault, List beforeMappingReferences, List afterMappingReferences) { - super( method, factoryMethod, mapNullToDefault, beforeMappingReferences, afterMappingReferences ); + super( method, existingVariableNames, factoryMethod, mapNullToDefault, beforeMappingReferences, + afterMappingReferences ); this.keyAssignment = keyAssignment; this.valueAssignment = valueAssignment; diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/NormalTypeMappingMethod.java b/processor/src/main/java/org/mapstruct/ap/internal/model/NormalTypeMappingMethod.java index e1e72eb62..0743987b4 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/NormalTypeMappingMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/NormalTypeMappingMethod.java @@ -18,6 +18,7 @@ */ package org.mapstruct.ap.internal.model; +import java.util.Collection; import java.util.List; import java.util.Set; @@ -36,10 +37,11 @@ public abstract class NormalTypeMappingMethod extends MappingMethod { private final boolean overridden; private final boolean mapNullToDefault; - NormalTypeMappingMethod(Method method, MethodReference factoryMethod, boolean mapNullToDefault, + NormalTypeMappingMethod(Method method, Collection existingVariableNames, MethodReference factoryMethod, + boolean mapNullToDefault, List beforeMappingReferences, List afterMappingReferences) { - super( method, beforeMappingReferences, afterMappingReferences ); + super( method, existingVariableNames, beforeMappingReferences, afterMappingReferences ); this.factoryMethod = factoryMethod; this.overridden = method.overridesMethod(); this.mapNullToDefault = mapNullToDefault; diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/StreamMappingMethod.java b/processor/src/main/java/org/mapstruct/ap/internal/model/StreamMappingMethod.java index 318831bba..e4f41caef 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/StreamMappingMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/StreamMappingMethod.java @@ -18,6 +18,7 @@ */ package org.mapstruct.ap.internal.model; +import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -60,8 +61,8 @@ public class StreamMappingMethod extends ContainerMappingMethod { } @Override - protected StreamMappingMethod instantiateMappingMethod(Method method, Assignment assignment, - MethodReference factoryMethod, boolean mapNullToDefault, String loopVariableName, + protected StreamMappingMethod instantiateMappingMethod(Method method, Collection existingVariables, + Assignment assignment, MethodReference factoryMethod, boolean mapNullToDefault, String loopVariableName, List beforeMappingMethods, List afterMappingMethods, SelectionParameters selectionParameters) { @@ -78,6 +79,7 @@ public class StreamMappingMethod extends ContainerMappingMethod { return new StreamMappingMethod( method, + existingVariables, assignment, factoryMethod, mapNullToDefault, @@ -90,13 +92,14 @@ public class StreamMappingMethod extends ContainerMappingMethod { } } - private StreamMappingMethod(Method method, Assignment parameterAssignment, MethodReference factoryMethod, - boolean mapNullToDefault, String loopVariableName, + private StreamMappingMethod(Method method, Collection existingVariables, Assignment parameterAssignment, + MethodReference factoryMethod, boolean mapNullToDefault, String loopVariableName, List beforeMappingReferences, List afterMappingReferences, SelectionParameters selectionParameters, Set helperImports) { super( method, + existingVariables, parameterAssignment, factoryMethod, mapNullToDefault, diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1111/Issue1111Mapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1111/Issue1111Mapper.java new file mode 100644 index 000000000..8a81d91ed --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1111/Issue1111Mapper.java @@ -0,0 +1,44 @@ +/** + * Copyright 2012-2017 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._1111; + +import java.util.List; +import org.mapstruct.Mapper; +import org.mapstruct.factory.Mappers; + +/** + * + * @author Sjaak Derksen + */ +@Mapper +public interface Issue1111Mapper { + + Issue1111Mapper INSTANCE = Mappers.getMapper( Issue1111Mapper.class ); + + Target toTarget(Source in); + + List list(List in); + + List> listList(List> in); + + class Source { } + + class Target { } + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1111/Issue1111Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1111/Issue1111Test.java new file mode 100644 index 000000000..a908491e7 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1111/Issue1111Test.java @@ -0,0 +1,53 @@ +/** + * Copyright 2012-2017 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._1111; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Arrays; +import java.util.List; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mapstruct.ap.test.bugs._1111.Issue1111Mapper.Source; +import org.mapstruct.ap.test.bugs._1111.Issue1111Mapper.Target; +import org.mapstruct.ap.testutil.IssueKey; +import org.mapstruct.ap.testutil.WithClasses; +import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner; + +/** + * + * @author Sjaak Derksen + */ +@IssueKey( "1111") +@WithClasses({Issue1111Mapper.class}) +@RunWith(AnnotationProcessorTestRunner.class) +public class Issue1111Test { + + @Test + public void shouldCompile() { + + List> source = Arrays.asList( Arrays.asList( new Source() ) ); + + List> target = Issue1111Mapper.INSTANCE.listList( source ); + + assertThat( target ).hasSize( 1 ); + assertThat( target.get( 0 ) ).hasSize( 1 ); + assertThat( target.get( 0 ).get( 0 ) ).isInstanceOf( Target.class ); + } +}