From 264a8f65afd4c06106874fac9b3c36ed6c5db94b Mon Sep 17 00:00:00 2001 From: Andreas Gudian Date: Thu, 25 Jun 2015 19:55:47 +0200 Subject: [PATCH] #574 determine property type from getter/setter methods only in combination with ExecutableType (i.e. using asMemberOf to determine the type with any generics being resolved) --- .../ap/internal/model/BeanMappingMethod.java | 5 +- .../ap/internal/model/PropertyMapping.java | 24 ++++++--- .../ap/internal/model/common/Type.java | 4 +- .../ap/internal/model/common/TypeFactory.java | 48 +++++------------- .../processor/MethodRetrievalProcessor.java | 2 +- .../ap/test/generics/AbstractIdHoldingTo.java | 34 +++++++++++++ .../ap/test/generics/AbstractTo.java | 26 ++++++++++ .../ap/test/generics/GenericsTest.java | 50 +++++++++++++++++++ .../mapstruct/ap/test/generics/Source.java | 35 +++++++++++++ .../ap/test/generics/SourceTargetMapper.java | 36 +++++++++++++ .../mapstruct/ap/test/generics/TargetTo.java | 29 +++++++++++ 11 files changed, 248 insertions(+), 45 deletions(-) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/generics/AbstractIdHoldingTo.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/generics/AbstractTo.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/generics/GenericsTest.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/generics/Source.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/generics/SourceTargetMapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/generics/TargetTo.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 832e78539..e18faa32c 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 @@ -33,6 +33,7 @@ import java.util.Map.Entry; import java.util.Set; import javax.lang.model.element.ExecutableElement; +import javax.lang.model.element.TypeElement; import javax.lang.model.type.TypeMirror; import javax.tools.Diagnostic; @@ -404,9 +405,11 @@ public class BeanMappingMethod extends MappingMethod { if ( sourceAccessor != null ) { Mapping mapping = method.getSingleMappingByTargetPropertyName( targetProperty.getKey() ); + TypeElement sourceType = sourceParameter.getType().getTypeElement(); + SourceReference sourceRef = new SourceReference.BuilderFromProperty() .sourceParameter( sourceParameter ) - .type( ctx.getTypeFactory().getReturnType( sourceAccessor ) ) + .type( ctx.getTypeFactory().getReturnType( sourceType, sourceAccessor ) ) .accessor( sourceAccessor ) .name( targetProperty.getKey() ) .build(); diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/PropertyMapping.java b/processor/src/main/java/org/mapstruct/ap/internal/model/PropertyMapping.java index c43570432..3bc2a5bb7 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/PropertyMapping.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/PropertyMapping.java @@ -454,10 +454,14 @@ public class PropertyMapping extends ModelElement { switch ( targetAccessorType ) { case ADDER: case SETTER: - return ctx.getTypeFactory().getSingleParameter( targetWriteAccessor ).getType(); + return ctx.getTypeFactory().getSingleParameter( + method.getResultType().getTypeElement(), + targetWriteAccessor ).getType(); case GETTER: default: - return ctx.getTypeFactory().getReturnType( targetWriteAccessor ); + return ctx.getTypeFactory().getReturnType( + method.getResultType().getTypeElement(), + targetWriteAccessor ); } } @@ -570,10 +574,14 @@ public class PropertyMapping extends ModelElement { // target Type targetType; if ( Executables.isSetterMethod( targetWriteAccessor ) ) { - targetType = ctx.getTypeFactory().getSingleParameter( targetWriteAccessor ).getType(); + targetType = ctx.getTypeFactory().getSingleParameter( + method.getResultType().getTypeElement(), + targetWriteAccessor ).getType(); } else { - targetType = ctx.getTypeFactory().getReturnType( targetWriteAccessor ); + targetType = ctx.getTypeFactory().getReturnType( + method.getResultType().getTypeElement(), + targetWriteAccessor ); } Assignment assignment = ctx.getMappingResolver().getTargetAssignment( @@ -659,10 +667,14 @@ public class PropertyMapping extends ModelElement { if ( Executables.isSetterMethod( targetWriteAccessor ) ) { // setter, so wrap in setter assignment = new SetterWrapper( assignment, method.getThrownTypes() ); - targetType = ctx.getTypeFactory().getSingleParameter( targetWriteAccessor ).getType(); + targetType = ctx.getTypeFactory().getSingleParameter( + method.getResultType().getTypeElement(), + targetWriteAccessor ).getType(); } else { - targetType = ctx.getTypeFactory().getReturnType( targetWriteAccessor ); + targetType = ctx.getTypeFactory().getReturnType( + method.getResultType().getTypeElement(), + targetWriteAccessor ); // target accessor is getter, so wrap the setter in getter map/ collection handling assignment = new GetterWrapperForCollectionsAndMaps( assignment, diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java b/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java index bad94d5e2..611a315c4 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java @@ -398,7 +398,7 @@ public class Type extends ModelElement implements Comparable { // first check if there's a setter method. ExecutableElement adderMethod = null; if ( Executables.isSetterMethod( candidate ) ) { - Type targetType = typeFactory.getSingleParameter( candidate ).getType(); + Type targetType = typeFactory.getSingleParameter( typeElement, candidate ).getType(); // ok, the current accessor is a setter. So now the strategy determines what to use if ( cmStrategy == CollectionMappingStrategyPrism.ADDER_PREFERRED ) { adderMethod = getAdderForType( targetType, targetPropertyName ); @@ -407,7 +407,7 @@ public class Type extends ModelElement implements Comparable { else if ( Executables.isGetterMethod( candidate ) ) { // the current accessor is a getter (no setter available). But still, an add method is according // to the above strategy (SETTER_PREFERRED || ADDER_PREFERRED) preferred over the getter. - Type targetType = typeFactory.getReturnType( candidate ); + Type targetType = typeFactory.getReturnType( typeFactory.getMethodType( typeElement, candidate ) ); adderMethod = getAdderForType( targetType, targetPropertyName ); } if ( adderMethod != null ) { diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/common/TypeFactory.java b/processor/src/main/java/org/mapstruct/ap/internal/model/common/TypeFactory.java index 2adb58e6d..5c5628586 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/common/TypeFactory.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/common/TypeFactory.java @@ -55,6 +55,7 @@ import javax.lang.model.util.Types; import org.mapstruct.ap.internal.prism.MappingTargetPrism; import org.mapstruct.ap.internal.prism.TargetTypePrism; import org.mapstruct.ap.internal.util.AnnotationProcessingException; +import org.mapstruct.ap.internal.util.Collections; import org.mapstruct.ap.internal.util.SpecificCompilerWorkarounds; import static org.mapstruct.ap.internal.util.SpecificCompilerWorkarounds.erasure; @@ -254,18 +255,18 @@ public class TypeFactory { * Get the ExecutableType for given method as part of usedMapper. Possibly parameterized types in method declaration * will be evaluated to concrete types then. * - * @param usedMapper the type declaring the method + * @param includingType the type on which's scope the method type shall be evaluated * @param method the method * @return the ExecutableType representing the method as part of usedMapper */ - public ExecutableType getMethodType(TypeElement usedMapper, ExecutableElement method) { - DeclaredType asType = (DeclaredType) replaceTypeElementIfNecessary( elementUtils, usedMapper ).asType(); + public ExecutableType getMethodType(TypeElement includingType, ExecutableElement method) { + DeclaredType asType = (DeclaredType) replaceTypeElementIfNecessary( elementUtils, includingType ).asType(); TypeMirror asMemberOf = typeUtils.asMemberOf( asType, method ); ExecutableType methodType = asMemberOf.accept( new ExecutableTypeRetrievalVisitor(), null ); return methodType; } - public Parameter getSingleParameter(ExecutableElement method) { + public Parameter getSingleParameter(TypeElement includingType, ExecutableElement method) { List parameters = method.getParameters(); if ( parameters.size() != 1 ) { @@ -273,31 +274,11 @@ public class TypeFactory { return null; } - VariableElement parameter = parameters.get( 0 ); - - return new Parameter( - parameter.getSimpleName().toString(), - getType( parameter.asType() ) - ); + return Collections.first( getParameters( includingType, method ) ); } - public List getParameters(ExecutableElement method) { - List parameters = method.getParameters(); - List result = new ArrayList( parameters.size() ); - - for ( VariableElement parameter : parameters ) { - result - .add( - new Parameter( - parameter.getSimpleName().toString(), - getType( parameter.asType() ), - MappingTargetPrism.getInstanceOn( parameter ) != null, - TargetTypePrism.getInstanceOn( parameter ) != null - ) - ); - } - - return result; + public List getParameters(TypeElement includingType, ExecutableElement method) { + return getParameters( getMethodType( includingType, method ), method ); } public List getParameters(ExecutableType methodType, ExecutableElement method) { @@ -322,20 +303,16 @@ public class TypeFactory { return result; } - public Type getReturnType(ExecutableElement method) { - return getType( method.getReturnType() ); + public Type getReturnType(TypeElement includingType, ExecutableElement method) { + return getReturnType( getMethodType( includingType, method ) ); } public Type getReturnType(ExecutableType method) { return getType( method.getReturnType() ); } - public List getThrownTypes(ExecutableElement method) { - List thrownTypes = new ArrayList(); - for (TypeMirror exceptionType : method.getThrownTypes() ) { - thrownTypes.add( getType( exceptionType ) ); - } - return thrownTypes; + public List getThrownTypes(TypeElement includingType, ExecutableElement method) { + return getThrownTypes( getMethodType( includingType, method ) ); } public List getThrownTypes(ExecutableType method) { @@ -488,4 +465,5 @@ public class TypeFactory { return collectionOrMap; } + } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/processor/MethodRetrievalProcessor.java b/processor/src/main/java/org/mapstruct/ap/internal/processor/MethodRetrievalProcessor.java index d4153cba5..b17acb78f 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/processor/MethodRetrievalProcessor.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/processor/MethodRetrievalProcessor.java @@ -181,7 +181,7 @@ public class MethodRetrievalProcessor implements ModelElementProcessor parameters = typeFactory.getParameters( methodType, method ); - Type returnType = typeFactory.getReturnType( method ); + Type returnType = typeFactory.getReturnType( methodType ); boolean methodRequiresImplementation = method.getModifiers().contains( Modifier.ABSTRACT ); boolean containsTargetTypeParameter = SourceMethod.containsTargetTypeParameter( parameters ); diff --git a/processor/src/test/java/org/mapstruct/ap/test/generics/AbstractIdHoldingTo.java b/processor/src/test/java/org/mapstruct/ap/test/generics/AbstractIdHoldingTo.java new file mode 100644 index 000000000..48a9ce680 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/generics/AbstractIdHoldingTo.java @@ -0,0 +1,34 @@ +/** + * Copyright 2012-2015 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.generics; + +/** + * @author Andreas Gudian + */ +public abstract class AbstractIdHoldingTo { + private ID id; + + public ID getId() { + return this.id; + } + + public void setId(ID id) { + this.id = id; + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/generics/AbstractTo.java b/processor/src/test/java/org/mapstruct/ap/test/generics/AbstractTo.java new file mode 100644 index 000000000..c9d9e000d --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/generics/AbstractTo.java @@ -0,0 +1,26 @@ +/** + * Copyright 2012-2015 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.generics; + + +/** + * @author Andreas Gudian + */ +public abstract class AbstractTo extends AbstractIdHoldingTo { +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/generics/GenericsTest.java b/processor/src/test/java/org/mapstruct/ap/test/generics/GenericsTest.java new file mode 100644 index 000000000..ec6dcafb7 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/generics/GenericsTest.java @@ -0,0 +1,50 @@ +/** + * Copyright 2012-2015 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.generics; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mapstruct.ap.testutil.IssueKey; +import org.mapstruct.ap.testutil.WithClasses; +import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner; + +import static org.fest.assertions.Assertions.assertThat; + +/** + * @author Andreas Gudian + * + */ +@WithClasses({ + AbstractTo.class, + AbstractIdHoldingTo.class, + Source.class, + SourceTargetMapper.class, + TargetTo.class +}) +@RunWith(AnnotationProcessorTestRunner.class) +public class GenericsTest { + + @Test + @IssueKey("574") + public void mapsIdCorrectly() { + TargetTo target = new TargetTo(); + target.setId( 41L ); + assertThat( SourceTargetMapper.INSTANCE.toSource( target ).getId() ).isEqualTo( 41L ); + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/generics/Source.java b/processor/src/test/java/org/mapstruct/ap/test/generics/Source.java new file mode 100644 index 000000000..477d68be3 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/generics/Source.java @@ -0,0 +1,35 @@ +/** + * Copyright 2012-2015 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.generics; + +/** + * @author Andreas Gudian + * + */ +public class Source { + private Long id; + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/generics/SourceTargetMapper.java b/processor/src/test/java/org/mapstruct/ap/test/generics/SourceTargetMapper.java new file mode 100644 index 000000000..c95cb948e --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/generics/SourceTargetMapper.java @@ -0,0 +1,36 @@ +/** + * Copyright 2012-2015 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.generics; + +import org.mapstruct.Mapper; +import org.mapstruct.factory.Mappers; + +/** + * @author Andreas Gudian + * + */ +@Mapper +public abstract class SourceTargetMapper { + + public static final SourceTargetMapper INSTANCE = Mappers.getMapper( SourceTargetMapper.class ); + + public abstract Source toSource(TargetTo target); + + public abstract TargetTo toTarget(Source source); +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/generics/TargetTo.java b/processor/src/test/java/org/mapstruct/ap/test/generics/TargetTo.java new file mode 100644 index 000000000..691322918 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/generics/TargetTo.java @@ -0,0 +1,29 @@ +/** + * Copyright 2012-2015 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.generics; + + +/** + * @author Andreas Gudian + * + */ +public class TargetTo extends AbstractTo { + + +}