From 266c5fa41ce6a07c4fb11b745b461c4ca19a8301 Mon Sep 17 00:00:00 2001 From: Filip Hrisafov Date: Sun, 2 Oct 2022 19:20:13 +0200 Subject: [PATCH] #1216 Pick candidate method with most specific return type When there are multiple candidate methods returning different types. We should be able to use the method with the most specific return type (if such a method exists) --- .../source/selector/MethodSelectors.java | 4 +- .../MostSpecificResultTypeSelector.java | 45 +++++++++++++ .../source/selector/SelectionCriteria.java | 8 +++ ...MostSpecificResultTypeSelectingMapper.java | 31 +++++++++ .../selection/resulttype/FruitFamily.java | 22 ++++++ .../resulttype/InheritanceSelectionTest.java | 67 +++++++++++++++++++ ...MostSpecificResultTypeSelectingMapper.java | 30 +++++++++ ...ecificResultTypeSelectingUpdateMapper.java | 65 ++++++++++++++++++ 8 files changed, 271 insertions(+), 1 deletion(-) create mode 100644 processor/src/main/java/org/mapstruct/ap/internal/model/source/selector/MostSpecificResultTypeSelector.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/selection/resulttype/ErroneousAmbiguousMostSpecificResultTypeSelectingMapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/selection/resulttype/FruitFamily.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/selection/resulttype/MostSpecificResultTypeSelectingMapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/selection/resulttype/MostSpecificResultTypeSelectingUpdateMapper.java diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/selector/MethodSelectors.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/selector/MethodSelectors.java index 519e1c3d6..74fa1a33a 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/selector/MethodSelectors.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/selector/MethodSelectors.java @@ -37,7 +37,9 @@ public class MethodSelectors { new InheritanceSelector(), new CreateOrUpdateSelector(), new SourceRhsSelector(), - new FactoryParameterSelector() ); + new FactoryParameterSelector(), + new MostSpecificResultTypeSelector() + ); } /** diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/selector/MostSpecificResultTypeSelector.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/selector/MostSpecificResultTypeSelector.java new file mode 100644 index 000000000..6e86a30bc --- /dev/null +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/selector/MostSpecificResultTypeSelector.java @@ -0,0 +1,45 @@ +/* + * 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.internal.model.source.selector; + +import java.util.ArrayList; +import java.util.List; + +import org.mapstruct.ap.internal.model.common.Type; +import org.mapstruct.ap.internal.model.source.Method; + +/** + * @author Filip Hrisafov + */ +public class MostSpecificResultTypeSelector implements MethodSelector { + + @Override + public List> getMatchingMethods(Method mappingMethod, + List> candidates, + List sourceTypes, Type mappingTargetType, + Type returnType, SelectionCriteria criteria) { + if ( candidates.size() < 2 || !criteria.isForMapping() || criteria.getQualifyingResultType() != null) { + return candidates; + } + + List> result = new ArrayList<>(); + + for ( SelectedMethod candidate : candidates ) { + if ( candidate.getMethod() + .getResultType() + .getBoxedEquivalent() + .equals( mappingTargetType.getBoxedEquivalent() ) ) { + // If the result type is the same as the target type + // then this candidate has the most specific match and should be used + result.add( candidate ); + } + } + + + // If not most specific types were found then return the current candidates + return result.isEmpty() ? candidates : result; + } +} diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/selector/SelectionCriteria.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/selector/SelectionCriteria.java index da8bae365..7e12bbd18 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/selector/SelectionCriteria.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/selector/SelectionCriteria.java @@ -68,6 +68,14 @@ public class SelectionCriteria { this.type = type; } + /** + * + * @return {@code true} if only mapping methods should be selected + */ + public boolean isForMapping() { + return type == null || type == Type.PREFER_UPDATE_MAPPING; + } + /** * @return true if factory methods should be selected, false otherwise. */ diff --git a/processor/src/test/java/org/mapstruct/ap/test/selection/resulttype/ErroneousAmbiguousMostSpecificResultTypeSelectingMapper.java b/processor/src/test/java/org/mapstruct/ap/test/selection/resulttype/ErroneousAmbiguousMostSpecificResultTypeSelectingMapper.java new file mode 100644 index 000000000..28a925c32 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/selection/resulttype/ErroneousAmbiguousMostSpecificResultTypeSelectingMapper.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.selection.resulttype; + +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; + +/** + * @author Filip Hrisafov + */ +@Mapper +public interface ErroneousAmbiguousMostSpecificResultTypeSelectingMapper { + + @Mapping( target = "apple", source = "fruit") + AppleFamily map(FruitFamily fruitFamily); + + default GoldenDelicious toGolden(IsFruit fruit) { + return fruit != null ? new GoldenDelicious( fruit.getType() ) : null; + } + + default Apple toApple1(IsFruit fruit) { + return fruit != null ? new Apple( fruit.getType() ) : null; + } + + default Apple toApple2(IsFruit fruit) { + return fruit != null ? new Apple( fruit.getType() ) : null; + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/selection/resulttype/FruitFamily.java b/processor/src/test/java/org/mapstruct/ap/test/selection/resulttype/FruitFamily.java new file mode 100644 index 000000000..347e5a0be --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/selection/resulttype/FruitFamily.java @@ -0,0 +1,22 @@ +/* + * 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.selection.resulttype; + +/** + * @author Filip Hrisafov + */ +public class FruitFamily { + + private IsFruit fruit; + + public IsFruit getFruit() { + return fruit; + } + + public void setFruit(IsFruit fruit) { + this.fruit = fruit; + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/selection/resulttype/InheritanceSelectionTest.java b/processor/src/test/java/org/mapstruct/ap/test/selection/resulttype/InheritanceSelectionTest.java index a6a4f1ad0..05245bafe 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/selection/resulttype/InheritanceSelectionTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/selection/resulttype/InheritanceSelectionTest.java @@ -240,4 +240,71 @@ public class InheritanceSelectionTest { .hasMessage( "Not allowed to change citrus type" ); assertThat( citrus.getType() ).isEqualTo( "lemon" ); } + + @ProcessorTest + @IssueKey("1216") + @WithClasses({ + Citrus.class, + GoldenDelicious.class, + FruitFamily.class, + AppleFamily.class, + MostSpecificResultTypeSelectingMapper.class, + Citrus.class + }) + public void testShouldUseMethodWithMostSpecificReturnType() { + FruitFamily fruitFamily = new FruitFamily(); + fruitFamily.setFruit( new Citrus( "citrus" ) ); + AppleFamily appleFamily = MostSpecificResultTypeSelectingMapper.INSTANCE.map( fruitFamily ); + + assertThat( appleFamily.getApple() ).isExactlyInstanceOf( Apple.class ); + assertThat( appleFamily.getApple().getType() ).isEqualTo( "citrus" ); + } + + @ProcessorTest + @IssueKey("1216") + @WithClasses({ + Citrus.class, + FruitFamily.class, + GoldenDelicious.class, + MostSpecificResultTypeSelectingUpdateMapper.class, + Citrus.class + }) + public void testShouldUseMethodWithMostSpecificReturnTypeForUpdateMappings() { + FruitFamily fruitFamily = new FruitFamily(); + fruitFamily.setFruit( new Citrus( "citrus" ) ); + MostSpecificResultTypeSelectingUpdateMapper.Target target = + new MostSpecificResultTypeSelectingUpdateMapper.Target( + new Apple( "from_test" ), + new GoldenDelicious( "from_test" ) + ); + MostSpecificResultTypeSelectingUpdateMapper.INSTANCE.update( target, fruitFamily ); + + assertThat( target.getApple() ).isExactlyInstanceOf( Apple.class ); + assertThat( target.getApple().getType() ).isEqualTo( "apple updated citrus" ); + assertThat( target.getGoldenApple() ).isExactlyInstanceOf( GoldenDelicious.class ); + assertThat( target.getGoldenApple().getType() ).isEqualTo( "golden updated citrus" ); + } + + @ProcessorTest + @IssueKey("1216") + @WithClasses({ + GoldenDelicious.class, + FruitFamily.class, + AppleFamily.class, + ErroneousAmbiguousMostSpecificResultTypeSelectingMapper.class + }) + @ExpectedCompilationOutcome( + value = CompilationResult.FAILED, + diagnostics = { + @Diagnostic(type = ErroneousAmbiguousMostSpecificResultTypeSelectingMapper.class, + kind = Kind.ERROR, + line = 17, + message = "Ambiguous mapping methods found for mapping property \"IsFruit fruit\" to Apple: " + + "Apple toApple1(IsFruit fruit), Apple toApple2(IsFruit fruit). " + + "See https://mapstruct.org/faq/#ambiguous for more info." + ) + } + ) + public void testAmbiguousMostSpecificResultTypeErroneous() { + } } diff --git a/processor/src/test/java/org/mapstruct/ap/test/selection/resulttype/MostSpecificResultTypeSelectingMapper.java b/processor/src/test/java/org/mapstruct/ap/test/selection/resulttype/MostSpecificResultTypeSelectingMapper.java new file mode 100644 index 000000000..16d9750ec --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/selection/resulttype/MostSpecificResultTypeSelectingMapper.java @@ -0,0 +1,30 @@ +/* + * 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.selection.resulttype; + +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; +import org.mapstruct.factory.Mappers; + +/** + * @author Filip Hrisafov + */ +@Mapper +public interface MostSpecificResultTypeSelectingMapper { + + MostSpecificResultTypeSelectingMapper INSTANCE = Mappers.getMapper( MostSpecificResultTypeSelectingMapper.class ); + + @Mapping( target = "apple", source = "fruit") + AppleFamily map(FruitFamily fruitFamily); + + default GoldenDelicious toGolden(IsFruit fruit) { + return fruit != null ? new GoldenDelicious( fruit.getType() ) : null; + } + + default Apple toApple(IsFruit fruit) { + return fruit != null ? new Apple( fruit.getType() ) : null; + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/selection/resulttype/MostSpecificResultTypeSelectingUpdateMapper.java b/processor/src/test/java/org/mapstruct/ap/test/selection/resulttype/MostSpecificResultTypeSelectingUpdateMapper.java new file mode 100644 index 000000000..5f270116d --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/selection/resulttype/MostSpecificResultTypeSelectingUpdateMapper.java @@ -0,0 +1,65 @@ +/* + * 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.selection.resulttype; + +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; +import org.mapstruct.MappingTarget; +import org.mapstruct.ObjectFactory; +import org.mapstruct.factory.Mappers; + +/** + * @author Filip Hrisafov + */ +@Mapper +public interface MostSpecificResultTypeSelectingUpdateMapper { + + MostSpecificResultTypeSelectingUpdateMapper INSTANCE = Mappers.getMapper( + MostSpecificResultTypeSelectingUpdateMapper.class ); + + @Mapping(target = "apple", source = "fruit") + @Mapping(target = "goldenApple", source = "fruit") + void update(@MappingTarget Target target, FruitFamily fruitFamily); + + default void updateGolden(@MappingTarget GoldenDelicious target, IsFruit fruit) { + target.setType( "golden updated " + fruit.getType() ); + } + + default void updateApple(@MappingTarget Apple target, IsFruit fruit) { + target.setType( "apple updated " + fruit.getType() ); + } + + @ObjectFactory + default GoldenDelicious createGolden() { + return new GoldenDelicious( "from_object_factory" ); + } + + class Target { + protected Apple apple; + protected GoldenDelicious goldenApple; + + public Target(Apple apple, GoldenDelicious goldenApple) { + this.apple = apple; + this.goldenApple = goldenApple; + } + + public Apple getApple() { + return apple; + } + + public void setApple(Apple apple) { + this.apple = apple; + } + + public GoldenDelicious getGoldenApple() { + return goldenApple; + } + + public void setGoldenApple(GoldenDelicious goldenApple) { + this.goldenApple = goldenApple; + } + } +}