From d50e41cdbb605edcb41d550e37da96733e6f1b1c Mon Sep 17 00:00:00 2001 From: Sjaak Derksen Date: Sat, 25 May 2019 18:14:00 +0200 Subject: [PATCH] #1776 adding a message when no qualifiers are found (#1786) --- .../model/ContainerMappingMethodBuilder.java | 25 ++++---- .../ap/internal/model/MapMappingMethod.java | 40 ++++++------- .../internal/model/MappingBuilderContext.java | 4 +- .../ap/internal/model/PropertyMapping.java | 57 ++++++++++--------- .../creation/MappingResolverImpl.java | 25 +++++++- .../mapstruct/ap/internal/util/Message.java | 1 + .../selection/qualifier/QualifierTest.java | 11 +++- 7 files changed, 97 insertions(+), 66 deletions(-) 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 c4dc73f61..f495375d6 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 @@ -89,25 +89,15 @@ public abstract class ContainerMappingMethodBuilder forge( sourceRHS, sourceElementType, targetElementType ) ); - if ( assignment == null && !criteria.hasQualfiers() ) { - assignment = forgeMapping( sourceRHS, sourceElementType, targetElementType ); - if ( assignment != null ) { - ctx.getMessager().note( 2, Message.ITERABLEMAPPING_CREATE_ELEMENT_NOTE, assignment ); - } - } - else { - ctx.getMessager().note( 2, Message.ITERABLEMAPPING_SELECT_ELEMENT_NOTE, assignment ); - } - if ( assignment == null ) { if ( method instanceof ForgedMethod ) { // leave messaging to calling property mapping @@ -124,6 +114,7 @@ public abstract class ContainerMappingMethodBuilder existingVariables, Assignment assignment, MethodReference factoryMethod, boolean mapNullToDefault, String loopVariableName, 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 81026c5ec..cc6155be9 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 @@ -97,19 +97,10 @@ public class MapMappingMethod extends NormalTypeMappingMethod { keyFormattingParameters, keyCriteria, keySourceRHS, - null + null, + () -> forge( keySourceRHS, keySourceType, keyTargetType, Message.MAPMAPPING_CREATE_KEY_NOTE ) ); - if ( keyAssignment == null && !keyCriteria.hasQualfiers( ) ) { - keyAssignment = forgeMapping( keySourceRHS, keySourceType, keyTargetType ); - if ( keyAssignment != null ) { - ctx.getMessager().note( 2, Message.MAPMAPPING_CREATE_KEY_NOTE, keyAssignment ); - } - } - else { - ctx.getMessager().note( 2, Message.MAPMAPPING_SELECT_KEY_NOTE, keyAssignment ); - } - if ( keyAssignment == null ) { if ( method instanceof ForgedMethod ) { // leave messaging to calling property mapping @@ -129,6 +120,9 @@ public class MapMappingMethod extends NormalTypeMappingMethod { ); } } + else { + ctx.getMessager().note( 2, Message.MAPMAPPING_SELECT_KEY_NOTE, keyAssignment ); + } // find mapping method or conversion for value Type valueSourceType = sourceTypeParams.get( 1 ).getTypeBound(); @@ -146,7 +140,8 @@ public class MapMappingMethod extends NormalTypeMappingMethod { valueFormattingParameters, valueCriteria, valueSourceRHS, - null + null, + () -> forge( valueSourceRHS, valueSourceType, valueTargetType, Message.MAPMAPPING_CREATE_VALUE_NOTE ) ); if ( method instanceof ForgedMethod ) { @@ -159,16 +154,6 @@ public class MapMappingMethod extends NormalTypeMappingMethod { } } - if ( valueAssignment == null && !valueCriteria.hasQualfiers( ) ) { - valueAssignment = forgeMapping( valueSourceRHS, valueSourceType, valueTargetType ); - if ( valueAssignment != null ) { - ctx.getMessager().note( 2, Message.MAPMAPPING_CREATE_VALUE_NOTE, valueAssignment ); - } - } - else { - ctx.getMessager().note( 2, Message.MAPMAPPING_SELECT_VALUE_NOTE, valueAssignment ); - } - if ( valueAssignment == null ) { if ( method instanceof ForgedMethod ) { // leave messaging to calling property mapping @@ -188,6 +173,9 @@ public class MapMappingMethod extends NormalTypeMappingMethod { ); } } + else { + ctx.getMessager().note( 2, Message.MAPMAPPING_SELECT_VALUE_NOTE, valueAssignment ); + } // mapNullToDefault boolean mapNullToDefault = false; @@ -222,6 +210,14 @@ public class MapMappingMethod extends NormalTypeMappingMethod { ); } + Assignment forge(SourceRHS sourceRHS, Type sourceType, Type targetType, Message message ) { + Assignment assignment = forgeMapping( sourceRHS, sourceType, targetType ); + if ( assignment != null ) { + ctx.getMessager().note( 2, message, assignment ); + } + return assignment; + } + @Override protected boolean shouldUsePropertyNamesInHistory() { return true; diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/MappingBuilderContext.java b/processor/src/main/java/org/mapstruct/ap/internal/model/MappingBuilderContext.java index bbbf69dbd..dae67e927 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/MappingBuilderContext.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/MappingBuilderContext.java @@ -11,6 +11,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.Supplier; import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.TypeElement; @@ -93,7 +94,8 @@ public class MappingBuilderContext { Assignment getTargetAssignment(Method mappingMethod, Type targetType, FormattingParameters formattingParameters, SelectionCriteria criteria, SourceRHS sourceRHS, - AnnotationMirror positionHint); + AnnotationMirror positionHint, + Supplier forger); Set getUsedSupportedMappings(); } 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 a439f6917..d298a710d 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 @@ -280,36 +280,17 @@ public class PropertyMapping extends ModelElement { formattingParameters, criteria, rightHandSide, - positionHint + positionHint, + () -> forge( ) ); } + else { + assignment = forge(); + } Type sourceType = rightHandSide.getSourceType(); - // No mapping found. Try to forge a mapping - if ( assignment == null && !criteria.hasQualfiers() ) { - if ( (sourceType.isCollectionType() || sourceType.isArrayType()) && targetType.isIterableType() ) { - assignment = forgeIterableMapping( sourceType, targetType, rightHandSide, method.getExecutable() ); - } - else if ( sourceType.isMapType() && targetType.isMapType() ) { - assignment = forgeMapMapping( sourceType, targetType, rightHandSide, method.getExecutable() ); - } - else if ( ( sourceType.isIterableType() && targetType.isStreamType() ) || - ( sourceType.isStreamType() && targetType.isStreamType() ) || - ( sourceType.isStreamType() && targetType.isIterableType() ) ) { - assignment = forgeStreamMapping( sourceType, targetType, rightHandSide, method.getExecutable() ); - } - else { - assignment = forgeMapping( rightHandSide ); - } - if ( assignment != null ) { - ctx.getMessager().note( 2, Message.PROPERTYMAPPING_CREATE_NOTE, assignment ); - } - } - else { - ctx.getMessager().note( 2, Message.PROPERTYMAPPING_SELECT_NOTE, assignment ); - } - if ( assignment != null ) { + ctx.getMessager().note( 2, Message.PROPERTYMAPPING_SELECT_NOTE, assignment ); if ( targetType.isCollectionOrMapType() ) { assignment = assignToCollection( targetType, targetWriteAccessorType, assignment ); } @@ -336,6 +317,29 @@ public class PropertyMapping extends ModelElement { ); } + private Assignment forge( ) { + Assignment assignment; + Type sourceType = rightHandSide.getSourceType(); + if ( (sourceType.isCollectionType() || sourceType.isArrayType()) && targetType.isIterableType() ) { + assignment = forgeIterableMapping( sourceType, targetType, rightHandSide, method.getExecutable() ); + } + else if ( sourceType.isMapType() && targetType.isMapType() ) { + assignment = forgeMapMapping( sourceType, targetType, rightHandSide, method.getExecutable() ); + } + else if ( ( sourceType.isIterableType() && targetType.isStreamType() ) + || ( sourceType.isStreamType() && targetType.isStreamType() ) + || ( sourceType.isStreamType() && targetType.isIterableType() ) ) { + assignment = forgeStreamMapping( sourceType, targetType, rightHandSide, method.getExecutable() ); + } + else { + assignment = forgeMapping( rightHandSide ); + } + if ( assignment != null ) { + ctx.getMessager().note( 2, Message.PROPERTYMAPPING_CREATE_NOTE, assignment ); + } + return assignment; + } + /** * Report that a mapping could not be created. */ @@ -844,7 +848,8 @@ public class PropertyMapping extends ModelElement { formattingParameters, criteria, new SourceRHS( constantExpression, sourceType, existingVariableNames, sourceErrorMessagePart ), - positionHint + positionHint, + () -> null ); } else { 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 d9e874893..cf3c588f0 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 @@ -9,6 +9,7 @@ import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.function.Supplier; import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.TypeElement; @@ -97,7 +98,8 @@ public class MappingResolverImpl implements MappingResolver { public Assignment getTargetAssignment(Method mappingMethod, Type targetType, FormattingParameters formattingParameters, SelectionCriteria criteria, SourceRHS sourceRHS, - AnnotationMirror positionHint) { + AnnotationMirror positionHint, + Supplier forger) { ResolvingAttempt attempt = new ResolvingAttempt( sourceModel, @@ -105,7 +107,8 @@ public class MappingResolverImpl implements MappingResolver { formattingParameters, sourceRHS, criteria, - positionHint + positionHint, + forger ); return attempt.getTargetAssignment( sourceRHS.getSourceTypeForMatching(), targetType ); @@ -136,6 +139,7 @@ public class MappingResolverImpl implements MappingResolver { private final boolean savedPreferUpdateMapping; private final FormattingParameters formattingParameters; private final AnnotationMirror positionHint; + private final Supplier forger; // resolving via 2 steps creates the possibility of wrong matches, first builtin method matches, // second doesn't. In that case, the first builtin method should not lead to a supported method @@ -145,7 +149,8 @@ public class MappingResolverImpl implements MappingResolver { private ResolvingAttempt(List sourceModel, Method mappingMethod, FormattingParameters formattingParameters, SourceRHS sourceRHS, SelectionCriteria criteria, - AnnotationMirror positionHint) { + AnnotationMirror positionHint, + Supplier forger) { this.mappingMethod = mappingMethod; this.methods = filterPossibleCandidateMethods( sourceModel ); @@ -156,6 +161,7 @@ public class MappingResolverImpl implements MappingResolver { this.selectionCriteria = criteria; this.savedPreferUpdateMapping = criteria.isPreferUpdateMapping(); this.positionHint = positionHint; + this.forger = forger; } private List filterPossibleCandidateMethods(List candidateMethods) { @@ -243,6 +249,19 @@ public class MappingResolverImpl implements MappingResolver { return conversion.getAssignment(); } + if ( hasQualfiers() ) { + messager.printMessage( + mappingMethod.getExecutable(), + positionHint, + Message.GENERAL_NO_QUALIFYING_METHOD, + Strings.join( selectionCriteria.getQualifiers(), ", " ), + Strings.join( selectionCriteria.getQualifiedByNames(), ", " ) + ); + } + else { + return forger.get(); + } + // if nothing works, alas, the result is null return null; } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/Message.java b/processor/src/main/java/org/mapstruct/ap/internal/util/Message.java index a7e6159bb..821e31db4 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/util/Message.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/util/Message.java @@ -113,6 +113,7 @@ public enum Message { GENERAL_JODA_NOT_ON_CLASSPATH( "Cannot validate Joda dateformat, no Joda on classpath. Consider adding Joda to the annotation processorpath.", Diagnostic.Kind.WARNING ), GENERAL_NOT_ALL_FORGED_CREATED( "Internal Error in creation of Forged Methods, it was expected all Forged Methods to finished with creation, but %s did not" ), GENERAL_NO_SUITABLE_CONSTRUCTOR( "%s does not have an accessible parameterless constructor." ), + GENERAL_NO_QUALIFYING_METHOD( "No qualifying method found for qualifiers: %s and / or qualifying names: %s" ), BUILDER_MORE_THAN_ONE_BUILDER_CREATION_METHOD( "More than one builder creation method for \"%s\". Found methods: \"%s\". Builder will not be used. Consider implementing a custom BuilderProvider SPI.", Diagnostic.Kind.WARNING ), BUILDER_NO_BUILD_METHOD_FOUND("No build method \"%s\" found in \"%s\" for \"%s\". Found methods: \"%s\".", Diagnostic.Kind.ERROR ), diff --git a/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/QualifierTest.java b/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/QualifierTest.java index 1d298f94d..6edbb93cf 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/QualifierTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/QualifierTest.java @@ -98,7 +98,16 @@ public class QualifierTest { @ExpectedCompilationOutcome( value = CompilationResult.FAILED, diagnostics = { - @Diagnostic(type = ErroneousMapper.class, + @Diagnostic( + type = ErroneousMapper.class, + kind = Kind.ERROR, + line = 28, + messageRegExp = + "No qualifying method found for qualifiers: " + + "org.mapstruct.ap.test.selection.qualifier.annotation.NonQualifierAnnotated and " + + "/ or qualifying names: .*"), + @Diagnostic( + type = ErroneousMapper.class, kind = Kind.ERROR, line = 28, messageRegExp =