From 971abc48c7ed176a0977be8e41903da2b21d6e9c Mon Sep 17 00:00:00 2001 From: Sjaak Derksen Date: Sat, 4 Jul 2020 18:49:33 +0200 Subject: [PATCH] #2122 fix for 2 step mapping and generics (#2129) --- .../ap/internal/model/common/Type.java | 53 ++++++++ .../creation/MappingResolverImpl.java | 114 +++++++++--------- .../_2122/Issue2122Method2MethodMapper.java | 103 ++++++++++++++++ .../Issue2122Method2TypeConversionMapper.java | 54 +++++++++ .../ap/test/bugs/_2122/Issue2122Test.java | 80 ++++++++++++ .../Issue2122TypeConversion2MethodMapper.java | 54 +++++++++ 6 files changed, 404 insertions(+), 54 deletions(-) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2122/Issue2122Method2MethodMapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2122/Issue2122Method2TypeConversionMapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2122/Issue2122Test.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2122/Issue2122TypeConversion2MethodMapper.java 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 0974fe347..ea27153cc 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 @@ -24,9 +24,11 @@ import javax.lang.model.type.DeclaredType; import javax.lang.model.type.PrimitiveType; import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeMirror; +import javax.lang.model.type.TypeVariable; import javax.lang.model.type.WildcardType; import javax.lang.model.util.ElementFilter; import javax.lang.model.util.Elements; +import javax.lang.model.util.SimpleTypeVisitor8; import javax.lang.model.util.Types; import org.mapstruct.ap.internal.gem.CollectionMappingStrategyGem; @@ -1073,6 +1075,57 @@ public class Type extends ModelElement implements Comparable { return isLiteral; } + /** + * Steps through the declaredType in order to find a match for this typevar Type. It allignes with + * the provided parameterized type where this typeVar type is used. + * + * @param declaredType the type + * @param parameterizedType the parameterized type + * + * @return the matching declared type. + */ + public Type resolveTypeVarToType(Type declaredType, Type parameterizedType) { + if ( isTypeVar() ) { + TypeVarMatcher typeVarMatcher = new TypeVarMatcher( typeUtils, this ); + return typeVarMatcher.visit( parameterizedType.getTypeMirror(), declaredType ); + } + return this; + } + + private static class TypeVarMatcher extends SimpleTypeVisitor8 { + + private TypeVariable typeVarToMatch; + private Types types; + + TypeVarMatcher( Types types, Type typeVarToMatch ) { + super( null ); + this.typeVarToMatch = (TypeVariable) typeVarToMatch.getTypeMirror(); + this.types = types; + } + + @Override + public Type visitTypeVariable(TypeVariable t, Type parameterized) { + if ( types.isSameType( t, typeVarToMatch ) ) { + return parameterized; + } + return super.visitTypeVariable( t, parameterized ); + } + + @Override + public Type visitDeclared(DeclaredType t, Type parameterized) { + if ( types.isAssignable( types.erasure( t ), types.erasure( parameterized.getTypeMirror() ) ) ) { + // if same type, we can cast en assume number of type args are also the same + for ( int i = 0; i < t.getTypeArguments().size(); i++ ) { + Type result = visit( t.getTypeArguments().get( i ), parameterized.getTypeParameters().get( i ) ); + if ( result != null ) { + return result; + } + } + } + return super.visitDeclared( t, parameterized ); + } + } + /** * It strips all the {@code []} from the {@code className}. * diff --git a/processor/src/main/java/org/mapstruct/ap/internal/processor/creation/MappingResolverImpl.java b/processor/src/main/java/org/mapstruct/ap/internal/processor/creation/MappingResolverImpl.java index 2875cef67..a42ea3826 100755 --- a/processor/src/main/java/org/mapstruct/ap/internal/processor/creation/MappingResolverImpl.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/processor/creation/MappingResolverImpl.java @@ -23,6 +23,7 @@ import javax.lang.model.util.Types; import org.mapstruct.ap.internal.conversion.ConversionProvider; import org.mapstruct.ap.internal.conversion.Conversions; +import org.mapstruct.ap.internal.gem.ReportingPolicyGem; import org.mapstruct.ap.internal.model.Field; import org.mapstruct.ap.internal.model.HelperMethod; import org.mapstruct.ap.internal.model.MapperReference; @@ -43,7 +44,6 @@ import org.mapstruct.ap.internal.model.source.builtin.BuiltInMethod; import org.mapstruct.ap.internal.model.source.selector.MethodSelectors; import org.mapstruct.ap.internal.model.source.selector.SelectedMethod; import org.mapstruct.ap.internal.model.source.selector.SelectionCriteria; -import org.mapstruct.ap.internal.gem.ReportingPolicyGem; import org.mapstruct.ap.internal.util.Collections; import org.mapstruct.ap.internal.util.FormattingMessager; import org.mapstruct.ap.internal.util.Message; @@ -426,29 +426,31 @@ public class MappingResolverImpl implements MappingResolver { // sourceMethod or builtIn that fits the signature B to C. Only then there is a match. If we have a match // a nested method call can be called. so C = methodY( methodX (A) ) for ( Method methodYCandidate : methodYCandidates ) { - if ( Object.class.getName() - .equals( methodYCandidate.getSourceParameters().get( 0 ).getType().getName() ) ) { + Type ySourceType = methodYCandidate.getSourceParameters().get( 0 ).getType(); + if ( Object.class.getName().equals( ySourceType.getName() ) ) { // java.lang.Object as intermediate result continue; } - methodRefY = - resolveViaMethod( methodYCandidate.getSourceParameters().get( 0 ).getType(), targetType, true ); + ySourceType = ySourceType.resolveTypeVarToType( targetType, methodYCandidate.getResultType() ); - if ( methodRefY != null ) { - selectionCriteria.setPreferUpdateMapping( false ); - Assignment methodRefX = - resolveViaMethod( sourceType, methodYCandidate.getSourceParameters().get( 0 ).getType(), true ); - selectionCriteria.setPreferUpdateMapping( savedPreferUpdateMapping ); - if ( methodRefX != null ) { - methodRefY.setAssignment( methodRefX ); - methodRefX.setAssignment( sourceRHS ); - break; - } - else { - // both should match; - supportingMethodCandidates.clear(); - methodRefY = null; + if ( ySourceType != null ) { + methodRefY = resolveViaMethod( ySourceType, targetType, true ); + if ( methodRefY != null ) { + + selectionCriteria.setPreferUpdateMapping( false ); + Assignment methodRefX = resolveViaMethod( sourceType, ySourceType, true ); + selectionCriteria.setPreferUpdateMapping( savedPreferUpdateMapping ); + if ( methodRefX != null ) { + methodRefY.setAssignment( methodRefX ); + methodRefX.setAssignment( sourceRHS ); + break; + } + else { + // both should match; + supportingMethodCandidates.clear(); + methodRefY = null; + } } } } @@ -475,28 +477,30 @@ public class MappingResolverImpl implements MappingResolver { Assignment methodRefY = null; for ( Method methodYCandidate : methodYCandidates ) { - if ( Object.class.getName() - .equals( methodYCandidate.getSourceParameters().get( 0 ).getType().getName() ) ) { + Type ySourceType = methodYCandidate.getSourceParameters().get( 0 ).getType(); + if ( Object.class.getName().equals( ySourceType.getName() ) ) { // java.lang.Object as intermediate result continue; } - methodRefY = - resolveViaMethod( methodYCandidate.getSourceParameters().get( 0 ).getType(), targetType, true ); + ySourceType = ySourceType.resolveTypeVarToType( targetType, methodYCandidate.getResultType() ); - if ( methodRefY != null ) { - Type targetTypeX = methodYCandidate.getSourceParameters().get( 0 ).getType(); - ConversionAssignment conversionXRef = resolveViaConversion( sourceType, targetTypeX ); - if ( conversionXRef != null ) { - methodRefY.setAssignment( conversionXRef.getAssignment() ); - conversionXRef.getAssignment().setAssignment( sourceRHS ); - conversionXRef.reportMessageWhenNarrowing( messager, this ); - break; - } - else { - // both should match - supportingMethodCandidates.clear(); - methodRefY = null; + if ( ySourceType != null ) { + methodRefY = resolveViaMethod( ySourceType, targetType, true ); + if ( methodRefY != null ) { + Type targetTypeX = methodYCandidate.getSourceParameters().get( 0 ).getType(); + ConversionAssignment conversionXRef = resolveViaConversion( sourceType, targetTypeX ); + if ( conversionXRef != null ) { + methodRefY.setAssignment( conversionXRef.getAssignment() ); + conversionXRef.getAssignment().setAssignment( sourceRHS ); + conversionXRef.reportMessageWhenNarrowing( messager, this ); + break; + } + else { + // both should match + supportingMethodCandidates.clear(); + methodRefY = null; + } } } } @@ -524,29 +528,31 @@ public class MappingResolverImpl implements MappingResolver { // search the other way around for ( Method methodXCandidate : methodXCandidates ) { + Type xTargetType = methodXCandidate.getReturnType(); if ( methodXCandidate.isUpdateMethod() || - Object.class.getName().equals( methodXCandidate.getReturnType().getFullyQualifiedName() ) ) { + Object.class.getName().equals( xTargetType.getFullyQualifiedName() ) ) { // skip update methods || java.lang.Object as intermediate result continue; } - Assignment methodRefX = resolveViaMethod( - sourceType, - methodXCandidate.getReturnType(), - true - ); - if ( methodRefX != null ) { - conversionYRef = resolveViaConversion( methodXCandidate.getReturnType(), targetType ); - if ( conversionYRef != null ) { - conversionYRef.getAssignment().setAssignment( methodRefX ); - methodRefX.setAssignment( sourceRHS ); - conversionYRef.reportMessageWhenNarrowing( messager, this ); - break; - } - else { - // both should match; - supportingMethodCandidates.clear(); - conversionYRef = null; + xTargetType = + xTargetType.resolveTypeVarToType( sourceType, methodXCandidate.getParameters().get( 0 ).getType() ); + + if ( xTargetType != null ) { + Assignment methodRefX = resolveViaMethod( sourceType, xTargetType, true ); + if ( methodRefX != null ) { + conversionYRef = resolveViaConversion( methodXCandidate.getReturnType(), targetType ); + if ( conversionYRef != null ) { + conversionYRef.getAssignment().setAssignment( methodRefX ); + methodRefX.setAssignment( sourceRHS ); + conversionYRef.reportMessageWhenNarrowing( messager, this ); + break; + } + else { + // both should match; + supportingMethodCandidates.clear(); + conversionYRef = null; + } } } } diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2122/Issue2122Method2MethodMapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2122/Issue2122Method2MethodMapper.java new file mode 100644 index 000000000..033a314af --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2122/Issue2122Method2MethodMapper.java @@ -0,0 +1,103 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.mapstruct.ap.test.bugs._2122; + +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; +import org.mapstruct.factory.Mappers; + +/** + * @author Christian Bandowski + */ +@Mapper +public interface Issue2122Method2MethodMapper { + + Issue2122Method2MethodMapper INSTANCE = Mappers.getMapper( Issue2122Method2MethodMapper.class ); + + @Mapping(target = "embeddedTarget", source = "value") + @Mapping(target = "embeddedMapTarget", source = "value") + @Mapping(target = "embeddedListListTarget", source = "value") + Target toTarget(Source source); + + EmbeddedTarget toEmbeddedTarget(String value); + + default List singleEntry(T entry) { + return Collections.singletonList( entry ); + } + + default List> singleNestedListEntry(T entry) { + return Collections.singletonList( Collections.singletonList( entry ) ); + } + + default HashMap singleEntryMap(T entry) { + HashMap result = new HashMap<>( ); + result.put( "test", entry ); + return result; + } + + class Source { + String value; + + public String getValue() { + return value; + } + + public void setValue(String value) { + this.value = value; + } + } + + class Target { + List embeddedTarget; + + Map embeddedMapTarget; + + List> embeddedListListTarget; + + public List getEmbeddedTarget() { + return embeddedTarget; + } + + public void setEmbeddedTarget(List embeddedTarget) { + this.embeddedTarget = embeddedTarget; + } + + public Map getEmbeddedMapTarget() { + return embeddedMapTarget; + } + + public void setEmbeddedMapTarget( Map embeddedMapTarget) { + this.embeddedMapTarget = embeddedMapTarget; + } + + public List> getEmbeddedListListTarget() { + return embeddedListListTarget; + } + + public void setEmbeddedListListTarget( + List> embeddedListListTarget) { + this.embeddedListListTarget = embeddedListListTarget; + } + } + + class EmbeddedTarget { + String value; + + public String getValue() { + return value; + } + + public void setValue(String value) { + this.value = value; + } + } + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2122/Issue2122Method2TypeConversionMapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2122/Issue2122Method2TypeConversionMapper.java new file mode 100644 index 000000000..43d40aa77 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2122/Issue2122Method2TypeConversionMapper.java @@ -0,0 +1,54 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.mapstruct.ap.test.bugs._2122; + +import java.util.List; + +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; +import org.mapstruct.factory.Mappers; + +/** + * @author Sjaak Derksen + */ +@Mapper +public interface Issue2122Method2TypeConversionMapper { + + Issue2122Method2TypeConversionMapper INSTANCE = Mappers.getMapper( Issue2122Method2TypeConversionMapper.class ); + + @Mapping(target = "value", source = "strings") + Target toTarget(Source source); + + default T toFirstElement(List entry) { + return entry.get( 0 ); + } + + class Source { + List strings; + + public List getStrings() { + return strings; + } + + public void setStrings(List strings) { + this.strings = strings; + } + } + + class Target { + Integer value; + + public Integer getValue() { + return value; + } + + public void setValue(Integer value) { + this.value = value; + } + + } + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2122/Issue2122Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2122/Issue2122Test.java new file mode 100644 index 000000000..0d42a8f12 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2122/Issue2122Test.java @@ -0,0 +1,80 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.mapstruct.ap.test.bugs._2122; + +import java.util.Collections; + +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.assertj.core.api.Assertions.assertThat; + +/** + * @author Christian Bandowski + */ +@IssueKey("2122") +@RunWith(AnnotationProcessorTestRunner.class) + +public class Issue2122Test { + + @Test + @WithClasses( Issue2122Method2MethodMapper.class ) + public void shouldMapMethod2Method() { + Issue2122Method2MethodMapper.Source source = new Issue2122Method2MethodMapper.Source(); + source.setValue( "value" ); + + Issue2122Method2MethodMapper.Target target = Issue2122Method2MethodMapper.INSTANCE.toTarget( source ); + + assertThat( target ).isNotNull(); + assertThat( target.getEmbeddedTarget() ).isNotNull(); + assertThat( target.getEmbeddedTarget() ).hasSize( 1 ) + .element( 0 ) + .extracting( Issue2122Method2MethodMapper.EmbeddedTarget::getValue ).isEqualTo( "value" ); + assertThat( target.getEmbeddedMapTarget() ).isNotNull(); + assertThat( target.getEmbeddedMapTarget() ).hasSize( 1 ); + assertThat( target.getEmbeddedMapTarget().get( "test" ) ) + .extracting( Issue2122Method2MethodMapper.EmbeddedTarget::getValue ).isEqualTo( "value" ); + assertThat( target.getEmbeddedListListTarget() ).isNotNull(); + assertThat( target.getEmbeddedListListTarget() ).hasSize( 1 ); + assertThat( target.getEmbeddedListListTarget().get( 0 ) ).hasSize( 1 ) + .element( 0 ) + .extracting( Issue2122Method2MethodMapper.EmbeddedTarget::getValue ).isEqualTo( "value" ); + } + + @Test + @WithClasses( Issue2122TypeConversion2MethodMapper.class ) + public void shouldMapTypeConversion2Method() { + Issue2122TypeConversion2MethodMapper.Source source = new Issue2122TypeConversion2MethodMapper.Source(); + source.setValue( 5 ); + + Issue2122TypeConversion2MethodMapper.Target target = + Issue2122TypeConversion2MethodMapper.INSTANCE.toTarget( source ); + + assertThat( target ).isNotNull(); + assertThat( target.getStrings() ).isNotNull(); + assertThat( target.getStrings() ).hasSize( 1 ) + .element( 0 ) + .isEqualTo( "5" ); + } + + @Test + @WithClasses( Issue2122Method2TypeConversionMapper.class ) + public void shouldMapMethod2TypeConversion() { + Issue2122Method2TypeConversionMapper.Source source = new Issue2122Method2TypeConversionMapper.Source(); + source.setStrings( Collections.singletonList( "5" ) ); + + Issue2122Method2TypeConversionMapper.Target target = + Issue2122Method2TypeConversionMapper.INSTANCE.toTarget( source ); + + assertThat( target ).isNotNull(); + assertThat( target.getValue() ).isNotNull(); + assertThat( target.getValue() ).isEqualTo( 5 ); + } + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2122/Issue2122TypeConversion2MethodMapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2122/Issue2122TypeConversion2MethodMapper.java new file mode 100644 index 000000000..2555f53de --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2122/Issue2122TypeConversion2MethodMapper.java @@ -0,0 +1,54 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.mapstruct.ap.test.bugs._2122; + +import java.util.Collections; +import java.util.List; + +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; +import org.mapstruct.factory.Mappers; + +/** + * @author Sjaak Derksen + */ +@Mapper +public interface Issue2122TypeConversion2MethodMapper { + + Issue2122TypeConversion2MethodMapper INSTANCE = Mappers.getMapper( Issue2122TypeConversion2MethodMapper.class ); + + @Mapping(target = "strings", source = "value") + Target toTarget(Source source); + + default List singleEntry(T entry) { + return Collections.singletonList( entry ); + } + + class Source { + Integer value; + + public Integer getValue() { + return value; + } + + public void setValue(Integer value) { + this.value = value; + } + } + + class Target { + List strings; + + public List getStrings() { + return strings; + } + + public void setStrings(List strings) { + this.strings = strings; + } + } + +}