From 73e8fd6152153ba8c2f76fc5e9a84b16d33f6a0b Mon Sep 17 00:00:00 2001 From: Filip Hrisafov Date: Tue, 27 Sep 2022 09:35:54 +0200 Subject: [PATCH] #2840, #2913, #2921: MethodMatcher should not match widening methods In the MethodMatcher we need to do a special check when the target type is primitive. The reason for that is that a Long is assignable to a primitive double. However, doing that means that information can be lost and thus we should not pick such methods. When the target type is primitive, then a method will be matched if and only if boxed equivalent of the target type is assignable to the boxed equivalent of the candidate return type --- .../internal/model/source/MethodMatcher.java | 11 +++ .../ap/test/bugs/_2840/Issue2840Mapper.java | 51 +++++++++++ .../ap/test/bugs/_2840/Issue2840Test.java | 31 +++++++ .../ap/test/bugs/_2913/Issue2913Mapper.java | 85 +++++++++++++++++++ .../ap/test/bugs/_2913/Issue2913Test.java | 35 ++++++++ .../ap/test/bugs/_2921/Issue2921Mapper.java | 49 +++++++++++ .../ap/test/bugs/_2921/Issue2921Test.java | 28 ++++++ 7 files changed, 290 insertions(+) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2840/Issue2840Mapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2840/Issue2840Test.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2913/Issue2913Mapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2913/Issue2913Test.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2921/Issue2921Mapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2921/Issue2921Test.java diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/MethodMatcher.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/MethodMatcher.java index baa3d7eac..42c26f09a 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/MethodMatcher.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/MethodMatcher.java @@ -82,6 +82,17 @@ public class MethodMatcher { // (the relation target / target type, target type being a class) if ( !analyser.candidateReturnType.isVoid() ) { + if ( targetType.isPrimitive() ) { + // If the target type is primitive + // then we are going to check if its boxed equivalent + // is assignable to the candidate return type + // This is done because primitives can be assigned from their own narrower counterparts + // directly without any casting. + // e.g. a Long is assignable to a primitive double + // However, in order not to lose information we are not going to allow this + return targetType.getBoxedEquivalent() + .isAssignableTo( analyser.candidateReturnType.getBoxedEquivalent() ); + } if ( !( analyser.candidateReturnType.isAssignableTo( targetType ) ) ) { return false; } diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2840/Issue2840Mapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2840/Issue2840Mapper.java new file mode 100644 index 000000000..d2baac8ca --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2840/Issue2840Mapper.java @@ -0,0 +1,51 @@ +/* + * 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._2840; + +import org.mapstruct.Mapper; +import org.mapstruct.factory.Mappers; + +/** + * @author Filip Hrisafov + */ +@Mapper +public interface Issue2840Mapper { + + Issue2840Mapper INSTANCE = + Mappers.getMapper( Issue2840Mapper.class ); + + Issue2840Mapper.Target map(Short shortValue, Integer intValue); + + default int toInt(Number number) { + return number.intValue() + 5; + } + + default short toShort(Number number) { + return (short) (number.shortValue() + 10); + } + + class Target { + + private int intValue; + private short shortValue; + + public int getIntValue() { + return intValue; + } + + public void setIntValue(int intValue) { + this.intValue = intValue; + } + + public short getShortValue() { + return shortValue; + } + + public void setShortValue(short shortValue) { + this.shortValue = shortValue; + } + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2840/Issue2840Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2840/Issue2840Test.java new file mode 100644 index 000000000..1c6b19306 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2840/Issue2840Test.java @@ -0,0 +1,31 @@ + +/* + * 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._2840; + +import org.mapstruct.ap.testutil.IssueKey; +import org.mapstruct.ap.testutil.ProcessorTest; +import org.mapstruct.ap.testutil.WithClasses; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Filip Hrisafov + */ +@IssueKey("2840") +@WithClasses({ + Issue2840Mapper.class, +}) +class Issue2840Test { + + @ProcessorTest + void shouldUseMethodWithMostSpecificReturnType() { + Issue2840Mapper.Target target = Issue2840Mapper.INSTANCE.map( (short) 10, 50 ); + + assertThat( target.getShortValue() ).isEqualTo( (short) 20 ); + assertThat( target.getIntValue() ).isEqualTo( 55 ); + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2913/Issue2913Mapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2913/Issue2913Mapper.java new file mode 100644 index 000000000..095f5457f --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2913/Issue2913Mapper.java @@ -0,0 +1,85 @@ +/* + * 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._2913; + +import java.math.BigDecimal; + +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; +import org.mapstruct.factory.Mappers; + +/** + * @author Filip Hrisafov + */ +@Mapper +public interface Issue2913Mapper { + + Issue2913Mapper INSTANCE = Mappers.getMapper( Issue2913Mapper.class ); + + @Mapping(target = "doublePrimitiveValue", source = "rounding") + @Mapping(target = "doubleValue", source = "rounding") + @Mapping(target = "longPrimitiveValue", source = "rounding") + @Mapping(target = "longValue", source = "rounding") + Target map(Source source); + + default Long mapAmount(BigDecimal amount) { + return amount != null ? amount.movePointRight( 2 ).longValue() : null; + } + + class Target { + + private double doublePrimitiveValue; + private Double doubleValue; + private long longPrimitiveValue; + private Long longValue; + + public double getDoublePrimitiveValue() { + return doublePrimitiveValue; + } + + public void setDoublePrimitiveValue(double doublePrimitiveValue) { + this.doublePrimitiveValue = doublePrimitiveValue; + } + + public Double getDoubleValue() { + return doubleValue; + } + + public void setDoubleValue(Double doubleValue) { + this.doubleValue = doubleValue; + } + + public long getLongPrimitiveValue() { + return longPrimitiveValue; + } + + public void setLongPrimitiveValue(long longPrimitiveValue) { + this.longPrimitiveValue = longPrimitiveValue; + } + + public Long getLongValue() { + return longValue; + } + + public void setLongValue(Long longValue) { + this.longValue = longValue; + } + } + + class Source { + + private final BigDecimal rounding; + + public Source(BigDecimal rounding) { + this.rounding = rounding; + } + + public BigDecimal getRounding() { + return rounding; + } + } + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2913/Issue2913Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2913/Issue2913Test.java new file mode 100644 index 000000000..7257c9ab7 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2913/Issue2913Test.java @@ -0,0 +1,35 @@ +/* + * 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._2913; + +import java.math.BigDecimal; + +import org.mapstruct.ap.testutil.IssueKey; +import org.mapstruct.ap.testutil.ProcessorTest; +import org.mapstruct.ap.testutil.WithClasses; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Filip Hrisafov + */ +@IssueKey("2913") +@WithClasses({ + Issue2913Mapper.class, +}) +class Issue2913Test { + + @ProcessorTest + void shouldNotWidenWithUserDefinedMethods() { + Issue2913Mapper.Source source = new Issue2913Mapper.Source( BigDecimal.valueOf( 10.543 ) ); + Issue2913Mapper.Target target = Issue2913Mapper.INSTANCE.map( source ); + + assertThat( target.getDoubleValue() ).isEqualTo( 10.543 ); + assertThat( target.getDoublePrimitiveValue() ).isEqualTo( 10.543 ); + assertThat( target.getLongValue() ).isEqualTo( 1054 ); + assertThat( target.getLongPrimitiveValue() ).isEqualTo( 1054 ); + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2921/Issue2921Mapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2921/Issue2921Mapper.java new file mode 100644 index 000000000..d686fcbc1 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2921/Issue2921Mapper.java @@ -0,0 +1,49 @@ +/* + * 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._2921; + +import org.mapstruct.Mapper; +import org.mapstruct.factory.Mappers; + +/** + * @author Filip Hrisafov + */ +@Mapper +public interface Issue2921Mapper { + + Issue2921Mapper INSTANCE = Mappers.getMapper( Issue2921Mapper.class ); + + Target map(Source source); + + default Short toShort(Integer value) { + throw new UnsupportedOperationException( "toShort method should not be used" ); + } + + class Source { + private final Integer value; + + public Source(Integer value) { + this.value = value; + } + + public Integer getValue() { + return value; + } + } + + class Target { + private final int value; + + public Target(int value) { + this.value = value; + } + + public int getValue() { + return value; + } + } + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2921/Issue2921Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2921/Issue2921Test.java new file mode 100644 index 000000000..5b8dd0386 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2921/Issue2921Test.java @@ -0,0 +1,28 @@ +/* + * 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._2921; + +import org.mapstruct.ap.testutil.IssueKey; +import org.mapstruct.ap.testutil.ProcessorTest; +import org.mapstruct.ap.testutil.WithClasses; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Filip Hrisafov + */ +@IssueKey("2921") +@WithClasses({ + Issue2921Mapper.class, +}) +class Issue2921Test { + + @ProcessorTest + void shouldNotUseIntegerToShortForMappingIntegerToInt() { + Issue2921Mapper.Target target = Issue2921Mapper.INSTANCE.map( new Issue2921Mapper.Source( 10 ) ); + assertThat( target.getValue() ).isEqualTo( 10 ); + } +}