From c58f80cc5f403a1d177411c6c633fa4faaaa9d83 Mon Sep 17 00:00:00 2001 From: Filip Hrisafov Date: Sun, 12 Apr 2020 13:27:31 +0200 Subject: [PATCH] #2069 Refactor target reference to report errors during bean mapping instead of creation of the target reference With this we can more easily go in the direction of using constructor to map into target beans. --- .../resources/build-config/checkstyle.xml | 4 +- .../ap/internal/model/BeanMappingMethod.java | 227 ++++++++--- .../internal/model/MappingBuilderContext.java | 4 + .../NestedTargetPropertyMappingHolder.java | 107 +++-- .../ap/internal/model/PropertyMapping.java | 34 +- .../model/beanmapping/MappingReference.java | 6 +- .../model/beanmapping/MappingReferences.java | 14 +- .../model/beanmapping/PropertyEntry.java | 48 +-- .../model/beanmapping/TargetReference.java | 381 +++++------------- .../internal/model/source/MappingOptions.java | 12 + .../DefaultModelElementProcessorContext.java | 1 + .../processor/MapperCreationProcessor.java | 6 +- .../ap/internal/util/FormattingMessager.java | 2 + .../mapstruct/ap/internal/util/Message.java | 2 + .../common/DefaultConversionContextTest.java | 4 + .../ap/test/bugs/_1153/Issue1153Test.java | 10 +- .../SuggestMostSimilarNameTest.java | 16 +- .../selection/generics/ConversionTest.java | 5 +- .../ErroneousSourceTargetMapper6.java | 8 + 19 files changed, 436 insertions(+), 455 deletions(-) diff --git a/build-config/src/main/resources/build-config/checkstyle.xml b/build-config/src/main/resources/build-config/checkstyle.xml index 5ae53dc46..945015d5c 100644 --- a/build-config/src/main/resources/build-config/checkstyle.xml +++ b/build-config/src/main/resources/build-config/checkstyle.xml @@ -122,7 +122,9 @@ - + + + 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 1fcf8e4ce..5cb2192aa 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 @@ -22,12 +22,13 @@ import java.util.stream.Collectors; import javax.lang.model.type.DeclaredType; import javax.tools.Diagnostic; +import org.mapstruct.ap.internal.gem.CollectionMappingStrategyGem; +import org.mapstruct.ap.internal.gem.ReportingPolicyGem; import org.mapstruct.ap.internal.model.PropertyMapping.ConstantMappingBuilder; import org.mapstruct.ap.internal.model.PropertyMapping.JavaExpressionMappingBuilder; import org.mapstruct.ap.internal.model.PropertyMapping.PropertyMappingBuilder; import org.mapstruct.ap.internal.model.beanmapping.MappingReference; import org.mapstruct.ap.internal.model.beanmapping.MappingReferences; -import org.mapstruct.ap.internal.model.beanmapping.PropertyEntry; import org.mapstruct.ap.internal.model.beanmapping.SourceReference; import org.mapstruct.ap.internal.model.beanmapping.TargetReference; import org.mapstruct.ap.internal.model.common.BuilderType; @@ -40,8 +41,6 @@ import org.mapstruct.ap.internal.model.source.MappingOptions; import org.mapstruct.ap.internal.model.source.Method; import org.mapstruct.ap.internal.model.source.SelectionParameters; import org.mapstruct.ap.internal.model.source.SourceMethod; -import org.mapstruct.ap.internal.gem.CollectionMappingStrategyGem; -import org.mapstruct.ap.internal.gem.ReportingPolicyGem; import org.mapstruct.ap.internal.util.Message; import org.mapstruct.ap.internal.util.Strings; import org.mapstruct.ap.internal.util.accessor.Accessor; @@ -96,7 +95,6 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { public Builder sourceMethod(SourceMethod sourceMethod) { this.method = sourceMethod; - this.mappingReferences = forSourceMethod( sourceMethod, ctx.getMessager(), ctx.getTypeFactory() ); return this; } @@ -131,6 +129,7 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { MethodReference factoryMethod = null; // determine which return type to construct + boolean cannotConstructReturnType = false; if ( !method.getReturnType().isVoid() ) { Type returnTypeImpl = getReturnTypeToConstructFromSelectionParameters( selectionParameters ); if ( returnTypeImpl != null ) { @@ -138,6 +137,9 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { if ( factoryMethod != null || canResultTypeFromBeanMappingBeConstructed( returnTypeImpl ) ) { returnTypeToConstruct = returnTypeImpl; } + else { + cannotConstructReturnType = true; + } } else if ( isBuilderRequired() ) { returnTypeImpl = returnTypeBuilder.getBuilder(); @@ -145,6 +147,9 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { if ( factoryMethod != null || canReturnTypeBeConstructed( returnTypeImpl ) ) { returnTypeToConstruct = returnTypeImpl; } + else { + cannotConstructReturnType = true; + } } else if ( !method.isUpdateMethod() ) { returnTypeImpl = method.getReturnType(); @@ -152,9 +157,17 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { if ( factoryMethod != null || canReturnTypeBeConstructed( returnTypeImpl ) ) { returnTypeToConstruct = returnTypeImpl; } + else { + cannotConstructReturnType = true; + } } } + if ( cannotConstructReturnType ) { + // If the return type cannot be constructed then no need to try to create mappings + return null; + } + /* the type that needs to be used in the mapping process as target */ Type resultTypeToMap = returnTypeToConstruct == null ? method.getResultType() : returnTypeToConstruct; @@ -187,8 +200,10 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { } } + initializeMappingReferencesIfNeeded( resultTypeToMap ); + // map properties with mapping - boolean mappingErrorOccurred = handleDefinedMappings(); + boolean mappingErrorOccurred = handleDefinedMappings( resultTypeToMap ); if ( mappingErrorOccurred ) { return null; } @@ -261,6 +276,20 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { ); } + private void initializeMappingReferencesIfNeeded(Type resultTypeToMap) { + if ( mappingReferences == null && method instanceof SourceMethod ) { + Set readAndWriteTargetProperties = new HashSet<>( unprocessedTargetProperties.keySet() ); + readAndWriteTargetProperties.addAll( resultTypeToMap.getPropertyReadAccessors().keySet() ); + mappingReferences = forSourceMethod( + (SourceMethod) method, + resultTypeToMap, + readAndWriteTargetProperties, + ctx.getMessager(), + ctx.getTypeFactory() + ); + } + } + /** * @return builder is required when there is a returnTypeBuilder and the mapping method is not update method. * However, builder is also required when there is a returnTypeBuilder, the mapping target is the builder and @@ -320,9 +349,11 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { PropertyMapping propertyMapping = new PropertyMappingBuilder() .mappingContext( ctx ) .sourceMethod( method ) - .targetWriteAccessor( unprocessedTargetProperties.get( propertyName ) ) - .targetReadAccessor( targetPropertyReadAccessor ) - .targetPropertyName( propertyName ) + .target( + propertyName, + targetPropertyReadAccessor, + unprocessedTargetProperties.get( propertyName ) + ) .sourceReference( reference ) .existingVariableNames( existingVariableNames ) .dependsOn( mappingRefs.collectNestedDependsOn() ) @@ -464,22 +495,24 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { *

* It is furthermore checked whether the given mappings are correct. When an error occurs, the method continues * in search of more problems. + * + * @param resultTypeToMap the type in which the defined target properties are defined */ - private boolean handleDefinedMappings() { + private boolean handleDefinedMappings(Type resultTypeToMap) { boolean errorOccurred = false; Set handledTargets = new HashSet<>(); // first we have to handle nested target mappings if ( mappingReferences.hasNestedTargetReferences() ) { - errorOccurred = handleDefinedNestedTargetMapping( handledTargets ); + errorOccurred = handleDefinedNestedTargetMapping( handledTargets, resultTypeToMap ); } for ( MappingReference mapping : mappingReferences.getMappingReferences() ) { if ( mapping.isValid() ) { String target = mapping.getTargetReference().getShallowestPropertyName(); if ( !handledTargets.contains( target ) ) { - if ( handleDefinedMapping( mapping, handledTargets ) ) { + if ( handleDefinedMapping( mapping, resultTypeToMap, handledTargets ) ) { errorOccurred = true; } } @@ -504,11 +537,13 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { return errorOccurred; } - private boolean handleDefinedNestedTargetMapping(Set handledTargets) { + private boolean handleDefinedNestedTargetMapping(Set handledTargets, Type resultTypeToMap) { NestedTargetPropertyMappingHolder holder = new NestedTargetPropertyMappingHolder.Builder() .mappingContext( ctx ) .method( method ) + .targetPropertiesWriteAccessors( unprocessedTargetProperties ) + .targetPropertyType( resultTypeToMap ) .mappingReferences( mappingReferences ) .existingVariableNames( existingVariableNames ) .build(); @@ -517,26 +552,24 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { propertyMappings.addAll( holder.getPropertyMappings() ); handledTargets.addAll( holder.getHandledTargets() ); // Store all the unprocessed defined targets. - for ( Entry> entry : holder.getUnprocessedDefinedTarget() + for ( Entry> entry : holder.getUnprocessedDefinedTarget() .entrySet() ) { if ( entry.getValue().isEmpty() ) { continue; } - unprocessedDefinedTargets.put( entry.getKey().getName(), entry.getValue() ); + unprocessedDefinedTargets.put( entry.getKey(), entry.getValue() ); } return holder.hasErrorOccurred(); } - private boolean handleDefinedMapping(MappingReference mappingRef, Set handledTargets) { - + private boolean handleDefinedMapping(MappingReference mappingRef, Type resultTypeToMap, + Set handledTargets) { boolean errorOccured = false; PropertyMapping propertyMapping = null; TargetReference targetRef = mappingRef.getTargetReference(); MappingOptions mapping = mappingRef.getMapping(); - PropertyEntry targetProperty = first( targetRef.getPropertyEntries() ); - String targetPropertyName = targetProperty.getName(); // unknown properties given via dependsOn()? for ( String dependency : mapping.getDependsOn() ) { @@ -552,16 +585,95 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { } } + String targetPropertyName = first( targetRef.getPropertyEntries() ); + // check if source / expression / constant are not somehow handled already if ( unprocessedDefinedTargets.containsKey( targetPropertyName ) ) { return false; } + Accessor targetWriteAccessor = unprocessedTargetProperties.get( targetPropertyName ); + Accessor targetReadAccessor = resultTypeToMap.getPropertyReadAccessors().get( targetPropertyName ); + + if ( targetWriteAccessor == null ) { + if ( targetReadAccessor == null ) { + Set readAccessors = resultTypeToMap.getPropertyReadAccessors().keySet(); + String mostSimilarProperty = Strings.getMostSimilarWord( targetPropertyName, readAccessors ); + + Message msg; + Object[] args; + + if ( targetRef.getPathProperties().isEmpty() ) { + msg = Message.BEANMAPPING_UNKNOWN_PROPERTY_IN_RESULTTYPE; + args = new Object[] { + targetPropertyName, + resultTypeToMap, + mostSimilarProperty + }; + } + else { + List pathProperties = new ArrayList<>( targetRef.getPathProperties() ); + pathProperties.add( mostSimilarProperty ); + msg = Message.BEANMAPPING_UNKNOWN_PROPERTY_IN_TYPE; + args = new Object[] { + targetPropertyName, + resultTypeToMap, + mapping.getTargetName(), + Strings.join( pathProperties, "." ) + }; + } + + ctx.getMessager() + .printMessage( + mapping.getElement(), + mapping.getMirror(), + mapping.getTargetAnnotationValue(), + msg, + args + ); + return true; + } + else if ( mapping.getInheritContext() != null && mapping.getInheritContext().isReversed() ) { + // read only reversed mappings are implicitly ignored + return false; + } + else if ( !mapping.isIgnored() ) { + // report an error for read only mappings + Message msg; + Object[] args; + + if ( Objects.equals( targetPropertyName, mapping.getTargetName() ) ) { + msg = Message.BEANMAPPING_PROPERTY_HAS_NO_WRITE_ACCESSOR_IN_RESULTTYPE; + args = new Object[] { + mapping.getTargetName(), + resultTypeToMap + }; + } + else { + msg = Message.BEANMAPPING_PROPERTY_HAS_NO_WRITE_ACCESSOR_IN_TYPE; + args = new Object[] { + targetPropertyName, + resultTypeToMap, + mapping.getTargetName() + }; + } + ctx.getMessager() + .printMessage( + mapping.getElement(), + mapping.getMirror(), + mapping.getTargetAnnotationValue(), + msg, + args + ); + return true; + } + } + // check the mapping options // its an ignored property mapping if ( mapping.isIgnored() ) { propertyMapping = null; - handledTargets.add( targetProperty.getName() ); + handledTargets.add( targetPropertyName ); } // its a constant @@ -573,8 +685,7 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { .mappingContext( ctx ) .sourceMethod( method ) .constantExpression( mapping.getConstant() ) - .targetProperty( targetProperty ) - .targetPropertyName( targetPropertyName ) + .target( targetPropertyName, targetReadAccessor, targetWriteAccessor ) .formattingParameters( mapping.getFormattingParameters() ) .selectionParameters( mapping.getSelectionParameters() ) .options( mapping ) @@ -595,8 +706,7 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { .sourceMethod( method ) .javaExpression( mapping.getJavaExpression() ) .existingVariableNames( existingVariableNames ) - .targetProperty( targetProperty ) - .targetPropertyName( targetPropertyName ) + .target( targetPropertyName, targetReadAccessor, targetWriteAccessor ) .dependsOn( mapping.getDependsOn() ) .mirror( mapping.getMirror() ) .build(); @@ -618,8 +728,7 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { propertyMapping = new PropertyMappingBuilder() .mappingContext( ctx ) .sourceMethod( method ) - .targetProperty( targetProperty ) - .targetPropertyName( targetPropertyName ) + .target( targetPropertyName, targetReadAccessor, targetWriteAccessor ) .sourcePropertyName( mapping.getSourceName() ) .sourceReference( sourceRef ) .selectionParameters( mapping.getSelectionParameters() ) @@ -723,15 +832,13 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { method.getResultType().getPropertyReadAccessors().get( targetPropertyName ); MappingReferences mappingRefs = extractMappingReferences( targetPropertyName, false ); PropertyMapping propertyMapping = new PropertyMappingBuilder().mappingContext( ctx ) - .sourceMethod( method ) - .targetWriteAccessor( targetPropertyWriteAccessor ) - .targetReadAccessor( targetPropertyReadAccessor ) - .targetPropertyName( targetPropertyName ) - .sourceReference( sourceRef ) - .existingVariableNames( existingVariableNames ) - .forgeMethodWithMappingReferences( mappingRefs ) - .options( method.getOptions().getBeanMapping() ) - .build(); + .sourceMethod( method ) + .target( targetPropertyName, targetPropertyReadAccessor, targetPropertyWriteAccessor ) + .sourceReference( sourceRef ) + .existingVariableNames( existingVariableNames ) + .forgeMethodWithMappingReferences( mappingRefs ) + .options( method.getOptions().getBeanMapping() ) + .build(); unprocessedSourceParameters.remove( sourceRef.getParameter() ); @@ -770,9 +877,7 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { PropertyMapping propertyMapping = new PropertyMappingBuilder() .mappingContext( ctx ) .sourceMethod( method ) - .targetWriteAccessor( targetProperty.getValue() ) - .targetReadAccessor( targetPropertyReadAccessor ) - .targetPropertyName( targetProperty.getKey() ) + .target( targetProperty.getKey(), targetPropertyReadAccessor, targetProperty.getValue() ) .sourceReference( sourceRef ) .existingVariableNames( existingVariableNames ) .forgeMethodWithMappingReferences( mappingRefs ) @@ -866,17 +971,25 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { } else if ( !unprocessedTargetProperties.isEmpty() && unmappedTargetPolicy.requiresReport() ) { - Message msg = unmappedTargetPolicy.getDiagnosticKind() == Diagnostic.Kind.ERROR ? - Message.BEANMAPPING_UNMAPPED_TARGETS_ERROR : Message.BEANMAPPING_UNMAPPED_TARGETS_WARNING; - Object[] args = new Object[] { - MessageFormat.format( - "{0,choice,1#property|1 processedSourceParameters; private final Set handledTargets; private final List propertyMappings; - private final Map> unprocessedDefinedTarget; + private final Map> unprocessedDefinedTarget; private final boolean errorOccurred; public NestedTargetPropertyMappingHolder( List processedSourceParameters, Set handledTargets, List propertyMappings, - Map> unprocessedDefinedTarget, boolean errorOccurred) { + Map> unprocessedDefinedTarget, boolean errorOccurred) { this.processedSourceParameters = processedSourceParameters; this.handledTargets = handledTargets; this.propertyMappings = propertyMappings; @@ -75,7 +79,7 @@ public class NestedTargetPropertyMappingHolder { /** * @return a map of all the unprocessed defined targets that can be applied to name forged base methods */ - public Map> getUnprocessedDefinedTarget() { + public Map> getUnprocessedDefinedTarget() { return unprocessedDefinedTarget; } @@ -94,6 +98,9 @@ public class NestedTargetPropertyMappingHolder { private Set existingVariableNames; private List propertyMappings; private Set handledTargets; + private Map targetPropertiesWriteAccessors; + private Type targetType; + private boolean errorOccurred; public Builder mappingReferences(MappingReferences mappingReferences) { this.mappingReferences = mappingReferences; @@ -115,6 +122,16 @@ public class NestedTargetPropertyMappingHolder { return this; } + public Builder targetPropertiesWriteAccessors(Map targetPropertiesWriteAccessors) { + this.targetPropertiesWriteAccessors = targetPropertiesWriteAccessors; + return this; + } + + public Builder targetPropertyType(Type targetType) { + this.targetType = targetType; + return this; + } + public NestedTargetPropertyMappingHolder build() { List processedSourceParameters = new ArrayList<>(); handledTargets = new HashSet<>(); @@ -123,11 +140,11 @@ public class NestedTargetPropertyMappingHolder { // first we group by the first property in the target properties and for each of those // properties we get the new mappings as if the first property did not exist. GroupedTargetReferences groupedByTP = groupByTargetReferences( ); - Map> unprocessedDefinedTarget = new LinkedHashMap<>(); + Map> unprocessedDefinedTarget = new LinkedHashMap<>(); - for ( Map.Entry> entryByTP : + for ( Map.Entry> entryByTP : groupedByTP.poppedTargetReferences.entrySet() ) { - PropertyEntry targetProperty = entryByTP.getKey(); + String targetProperty = entryByTP.getKey(); //Now we are grouping the already popped mappings by the source parameter(s) of the method GroupedBySourceParameters groupedBySourceParam = groupBySourceParameter( entryByTP.getValue(), @@ -188,7 +205,7 @@ public class NestedTargetPropertyMappingHolder { .type( sourceEntry.getType() ) .readAccessor( sourceEntry.getReadAccessor() ) .presenceChecker( sourceEntry.getPresenceChecker() ) - .name( targetProperty.getName() ) + .name( targetProperty ) .build(); // If we have multiple source parameters that are mapped to the target reference, or @@ -205,7 +222,7 @@ public class NestedTargetPropertyMappingHolder { propertyMappings.add( propertyMapping ); } - handledTargets.add( entryByTP.getKey().getName() ); + handledTargets.add( entryByTP.getKey() ); } // For the nonNested mappings (assymetric) Mappings we also forge mappings @@ -216,7 +233,7 @@ public class NestedTargetPropertyMappingHolder { new MappingReferences( groupedSourceReferences.nonNested, true ); SourceReference reference = new SourceReference.BuilderFromProperty() .sourceParameter( sourceParameter ) - .name( targetProperty.getName() ) + .name( targetProperty ) .build(); boolean forceUpdateMethodForNonNested = @@ -236,7 +253,7 @@ public class NestedTargetPropertyMappingHolder { propertyMappings.add( propertyMapping ); } - handledTargets.add( entryByTP.getKey().getName() ); + handledTargets.add( entryByTP.getKey() ); } handleSourceParameterMappings( @@ -255,7 +272,7 @@ public class NestedTargetPropertyMappingHolder { handledTargets, propertyMappings, unprocessedDefinedTarget, - groupedByTP.errorOccurred + errorOccurred ); } @@ -268,7 +285,7 @@ public class NestedTargetPropertyMappingHolder { * @param forceUpdateMethod whether we need to force an update method */ private void handleSourceParameterMappings(Set sourceParameterMappings, - PropertyEntry targetProperty, Parameter sourceParameter, + String targetProperty, Parameter sourceParameter, boolean forceUpdateMethod) { if ( !sourceParameterMappings.isEmpty() ) { // The source parameter mappings have no mappings, the source name is actually the parameter itself @@ -276,7 +293,7 @@ public class NestedTargetPropertyMappingHolder { new MappingReferences( Collections.emptySet(), false, true ); SourceReference reference = new SourceReference.BuilderFromProperty() .sourceParameter( sourceParameter ) - .name( targetProperty.getName() ) + .name( targetProperty ) .build(); PropertyMapping propertyMapping = createPropertyMappingForNestedTarget( @@ -290,7 +307,7 @@ public class NestedTargetPropertyMappingHolder { propertyMappings.add( propertyMapping ); } - handledTargets.add( targetProperty.getName() ); + handledTargets.add( targetProperty ); } } @@ -340,16 +357,11 @@ public class NestedTargetPropertyMappingHolder { */ private GroupedTargetReferences groupByTargetReferences( ) { // group all mappings based on the top level name before popping - Map> mappingsKeyedByProperty = new LinkedHashMap<>(); - Map> singleTargetReferences = new LinkedHashMap<>(); - boolean errorOccurred = false; + Map> mappingsKeyedByProperty = new LinkedHashMap<>(); + Map> singleTargetReferences = new LinkedHashMap<>(); for ( MappingReference mapping : mappingReferences.getMappingReferences() ) { TargetReference targetReference = mapping.getTargetReference(); - if ( !targetReference.isValid() ) { - errorOccurred = true; - continue; - } - PropertyEntry property = first( targetReference.getPropertyEntries() ); + String property = first( targetReference.getPropertyEntries() ); MappingReference newMapping = mapping.popTargetReference(); if ( newMapping != null ) { // group properties on current name. @@ -362,7 +374,7 @@ public class NestedTargetPropertyMappingHolder { } } - return new GroupedTargetReferences( mappingsKeyedByProperty, singleTargetReferences, errorOccurred ); + return new GroupedTargetReferences( mappingsKeyedByProperty, singleTargetReferences ); } /** @@ -625,14 +637,45 @@ public class NestedTargetPropertyMappingHolder { } private PropertyMapping createPropertyMappingForNestedTarget(MappingReferences mappingReferences, - PropertyEntry targetProperty, + String targetPropertyName, SourceReference sourceReference, boolean forceUpdateMethod) { + + Accessor targetWriteAccessor = targetPropertiesWriteAccessors.get( targetPropertyName ); + Accessor targetReadAccessor = targetType.getPropertyReadAccessors().get( targetPropertyName ); + if ( targetWriteAccessor == null ) { + Set readAccessors = targetType.getPropertyReadAccessors().keySet(); + String mostSimilarProperty = Strings.getMostSimilarWord( targetPropertyName, readAccessors ); + + for ( MappingReference mappingReference : mappingReferences.getMappingReferences() ) { + MappingOptions mapping = mappingReference.getMapping(); + List pathProperties = new ArrayList<>( mappingReference.getTargetReference() + .getPathProperties() ); + if ( !pathProperties.isEmpty() ) { + pathProperties.set( pathProperties.size() - 1, mostSimilarProperty ); + } + + mappingContext.getMessager() + .printMessage( + mapping.getElement(), + mapping.getMirror(), + mapping.getTargetAnnotationValue(), + Message.BEANMAPPING_UNKNOWN_PROPERTY_IN_TYPE, + targetPropertyName, + targetType, + mapping.getTargetName(), + Strings.join( pathProperties, "." ) + ); + } + + errorOccurred = true; + return null; + } + return new PropertyMapping.PropertyMappingBuilder() .mappingContext( mappingContext ) .sourceMethod( method ) - .targetProperty( targetProperty ) - .targetPropertyName( targetProperty.getName() ) + .target( targetPropertyName, targetReadAccessor, targetWriteAccessor ) .sourceReference( sourceReference ) .existingVariableNames( existingVariableNames ) .dependsOn( mappingReferences.collectNestedDependsOn() ) @@ -685,21 +728,19 @@ public class NestedTargetPropertyMappingHolder { * references (target references that were not nested). */ private static class GroupedTargetReferences { - private final Map> poppedTargetReferences; - private final Map> singleTargetReferences; - private final boolean errorOccurred; + private final Map> poppedTargetReferences; + private final Map> singleTargetReferences; - private GroupedTargetReferences(Map> poppedTargetReferences, - Map> singleTargetReferences, boolean errorOccurred) { + private GroupedTargetReferences(Map> poppedTargetReferences, + Map> singleTargetReferences) { this.poppedTargetReferences = poppedTargetReferences; this.singleTargetReferences = singleTargetReferences; - this.errorOccurred = errorOccurred; } @Override public String toString() { return "GroupedTargetReferences{" + "poppedTargetReferences=" + poppedTargetReferences - + ", singleTargetReferences=" + singleTargetReferences + ", errorOccurred=" + errorOccurred + '}'; + + ", singleTargetReferences=" + singleTargetReferences + "}"; } } 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 06de2aa39..24facce72 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 @@ -94,21 +94,9 @@ public class PropertyMapping extends ModelElement { return super.method( sourceMethod ); } - public T targetProperty(PropertyEntry targetProp) { - this.targetReadAccessor = targetProp.getReadAccessor(); - this.targetWriteAccessor = targetProp.getWriteAccessor(); - this.targetType = targetProp.getType(); - this.targetBuilderType = targetProp.getBuilderType(); - this.targetWriteAccessorType = targetWriteAccessor.getAccessorType(); - return (T) this; - } - - public T targetReadAccessor(Accessor targetReadAccessor) { + public T target(String targetPropertyName, Accessor targetReadAccessor, Accessor targetWriteAccessor) { + this.targetPropertyName = targetPropertyName; this.targetReadAccessor = targetReadAccessor; - return (T) this; - } - - public T targetWriteAccessor(Accessor targetWriteAccessor) { this.targetWriteAccessor = targetWriteAccessor; this.targetType = ctx.getTypeFactory().getType( targetWriteAccessor.getAccessedType() ); BuilderGem builder = method.getOptions().getBeanMapping().getBuilder(); @@ -122,11 +110,6 @@ public class PropertyMapping extends ModelElement { return (T) this; } - public T targetPropertyName(String targetPropertyName) { - this.targetPropertyName = targetPropertyName; - return (T) this; - } - public T sourcePropertyName(String sourcePropertyName) { this.sourcePropertyName = sourcePropertyName; return (T) this; @@ -326,6 +309,11 @@ public class PropertyMapping extends ModelElement { * Report that a mapping could not be created. */ private void reportCannotCreateMapping() { + if ( forgeMethodWithMappingReferences != null && ctx.isErroneous() ) { + // If we arrived here, there is an error it means that we couldn't forge a mapping method + // so skip the cannot create mapping + return; + } if ( method instanceof ForgedMethod && ( (ForgedMethod) method ).getHistory() != null ) { // The history that is part of the ForgedMethod misses the information from the current right hand // side. Therefore we need to extract the most relevant history and use that in the error reporting. @@ -363,9 +351,7 @@ public class PropertyMapping extends ModelElement { .existingVariableNames( existingVariableNames ) .mappingContext( ctx ) .sourceMethod( method ) - .targetPropertyName( targetPropertyName ) - .targetReadAccessor( targetReadAccessor ) - .targetWriteAccessor( targetWriteAccessor ) + .target( targetPropertyName, targetReadAccessor, targetWriteAccessor ) .build(); return build.getAssignment(); } @@ -378,9 +364,7 @@ public class PropertyMapping extends ModelElement { .existingVariableNames( existingVariableNames ) .mappingContext( ctx ) .sourceMethod( method ) - .targetPropertyName( targetPropertyName ) - .targetReadAccessor( targetReadAccessor ) - .targetWriteAccessor( targetWriteAccessor ) + .target( targetPropertyName, targetReadAccessor, targetWriteAccessor ) .build(); return build.getAssignment(); } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/beanmapping/MappingReference.java b/processor/src/main/java/org/mapstruct/ap/internal/model/beanmapping/MappingReference.java index 2675e2900..e2501ea95 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/beanmapping/MappingReference.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/beanmapping/MappingReference.java @@ -80,11 +80,7 @@ public class MappingReference { } public boolean isValid( ) { - boolean result = false; - if ( targetReference.isValid() ) { - result = sourceReference != null ? sourceReference.isValid() : true; - } - return result; + return sourceReference == null || sourceReference.isValid(); } @Override diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/beanmapping/MappingReferences.java b/processor/src/main/java/org/mapstruct/ap/internal/model/beanmapping/MappingReferences.java index 63af65f2e..06f0ed06c 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/beanmapping/MappingReferences.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/beanmapping/MappingReferences.java @@ -11,6 +11,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Set; +import org.mapstruct.ap.internal.model.common.Type; import org.mapstruct.ap.internal.model.common.TypeFactory; import org.mapstruct.ap.internal.model.source.MappingOptions; import org.mapstruct.ap.internal.model.source.SourceMethod; @@ -29,8 +30,11 @@ public class MappingReferences { return EMPTY; } - public static MappingReferences forSourceMethod(SourceMethod sourceMethod, FormattingMessager messager, - TypeFactory typeFactory) { + public static MappingReferences forSourceMethod(SourceMethod sourceMethod, + Type targetType, + Set targetProperties, + FormattingMessager messager, + TypeFactory typeFactory) { Set references = new LinkedHashSet<>(); List targetThisReferences = new ArrayList<>( ); @@ -49,6 +53,8 @@ public class MappingReferences { .method( sourceMethod ) .messager( messager ) .typeFactory( typeFactory ) + .targetProperties( targetProperties ) + .targetType( targetType ) .build(); // add when inverse is also valid @@ -121,7 +127,7 @@ public class MappingReferences { for ( MappingReference mappingRef : mappingReferences ) { TargetReference targetReference = mappingRef.getTargetReference(); - if ( targetReference.isValid() && targetReference.isNested()) { + if ( targetReference.isNested()) { return true; } @@ -141,7 +147,7 @@ public class MappingReferences { private static boolean isValidWhenInversed(MappingReference mappingRef) { MappingOptions mapping = mappingRef.getMapping(); if ( mapping.getInheritContext() != null && mapping.getInheritContext().isReversed() ) { - return mappingRef.getTargetReference().isValid() && ( mappingRef.getSourceReference() == null || + return ( mappingRef.getSourceReference() == null || mappingRef.getSourceReference().isValid() ); } return true; diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/beanmapping/PropertyEntry.java b/processor/src/main/java/org/mapstruct/ap/internal/model/beanmapping/PropertyEntry.java index f2d087cd9..8ac5e4667 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/beanmapping/PropertyEntry.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/beanmapping/PropertyEntry.java @@ -7,55 +7,33 @@ package org.mapstruct.ap.internal.model.beanmapping; import java.util.Arrays; -import org.mapstruct.ap.internal.model.common.BuilderType; import org.mapstruct.ap.internal.model.common.Type; import org.mapstruct.ap.internal.util.Strings; import org.mapstruct.ap.internal.util.accessor.Accessor; /** - * A PropertyEntry contains information on the name, readAccessor (for source), readAccessor and writeAccessor - * (for targets) and return type of a property. + * A PropertyEntry contains information on the name, readAccessor and presenceCheck (for source) + * and return type of a property. */ public class PropertyEntry { private final String[] fullName; private final Accessor readAccessor; - private final Accessor writeAccessor; private final Accessor presenceChecker; private final Type type; - private final BuilderType builderType; /** * Constructor used to create {@link TargetReference} property entries from a mapping * * @param fullName * @param readAccessor - * @param writeAccessor * @param type */ - private PropertyEntry(String[] fullName, Accessor readAccessor, Accessor writeAccessor, - Accessor presenceChecker, Type type, BuilderType builderType) { + private PropertyEntry(String[] fullName, Accessor readAccessor, Accessor presenceChecker, Type type) { this.fullName = fullName; this.readAccessor = readAccessor; - this.writeAccessor = writeAccessor; this.presenceChecker = presenceChecker; this.type = type; - this.builderType = builderType; - } - - /** - * Constructor used to create {@link TargetReference} property entries - * - * @param fullName name of the property (dot separated) - * @param readAccessor its read accessor - * @param writeAccessor its write accessor - * @param type type of the property - * @param builderType the builder for the type - * @return the property entry for given parameters. - */ - public static PropertyEntry forTargetReference(String[] fullName, Accessor readAccessor, - Accessor writeAccessor, Type type, BuilderType builderType ) { - return new PropertyEntry( fullName, readAccessor, writeAccessor, null, type, builderType ); } /** @@ -69,7 +47,7 @@ public class PropertyEntry { */ public static PropertyEntry forSourceReference(String[] name, Accessor readAccessor, Accessor presenceChecker, Type type) { - return new PropertyEntry( name, readAccessor, null, presenceChecker, type, null ); + return new PropertyEntry( name, readAccessor, presenceChecker, type ); } public String getName() { @@ -80,10 +58,6 @@ public class PropertyEntry { return readAccessor; } - public Accessor getWriteAccessor() { - return writeAccessor; - } - public Accessor getPresenceChecker() { return presenceChecker; } @@ -92,24 +66,10 @@ public class PropertyEntry { return type; } - public BuilderType getBuilderType() { - return builderType; - } - public String getFullName() { return Strings.join( Arrays.asList( fullName ), "." ); } - public PropertyEntry pop() { - if ( fullName.length > 1 ) { - String[] newFullName = Arrays.copyOfRange( fullName, 1, fullName.length ); - return new PropertyEntry(newFullName, readAccessor, writeAccessor, presenceChecker, type, builderType ); - } - else { - return null; - } - } - @Override public int hashCode() { int hash = 7; diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/beanmapping/TargetReference.java b/processor/src/main/java/org/mapstruct/ap/internal/model/beanmapping/TargetReference.java index 8a2a39823..fee97f840 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/beanmapping/TargetReference.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/beanmapping/TargetReference.java @@ -7,26 +7,21 @@ package org.mapstruct.ap.internal.model.beanmapping; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Objects; import java.util.Set; import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.AnnotationValue; -import javax.lang.model.type.DeclaredType; -import org.mapstruct.ap.internal.model.common.BuilderType; import org.mapstruct.ap.internal.model.common.Parameter; import org.mapstruct.ap.internal.model.common.Type; import org.mapstruct.ap.internal.model.common.TypeFactory; import org.mapstruct.ap.internal.model.source.MappingOptions; import org.mapstruct.ap.internal.model.source.Method; -import org.mapstruct.ap.internal.gem.BuilderGem; -import org.mapstruct.ap.internal.gem.CollectionMappingStrategyGem; import org.mapstruct.ap.internal.util.FormattingMessager; import org.mapstruct.ap.internal.util.Message; import org.mapstruct.ap.internal.util.Strings; -import org.mapstruct.ap.internal.util.accessor.Accessor; -import org.mapstruct.ap.internal.util.accessor.AccessorType; import static org.mapstruct.ap.internal.util.Collections.first; @@ -48,11 +43,77 @@ import static org.mapstruct.ap.internal.util.Collections.first; *

  • {@code propertyEntries[1]} will describe {@code propB}
    • * * - * After building, {@link #isValid()} will return true when when no problems are detected during building. - * * @author Sjaak Derksen */ -public class TargetReference extends AbstractReference { +public class TargetReference { + + private final List pathProperties; + private final Parameter parameter; + private final List propertyEntries; + + public TargetReference(Parameter parameter, List propertyEntries) { + this( parameter, propertyEntries, Collections.emptyList() ); + } + + public TargetReference(Parameter parameter, List propertyEntries, List pathProperties) { + this.pathProperties = pathProperties; + this.parameter = parameter; + this.propertyEntries = propertyEntries; + } + + public List getPathProperties() { + return pathProperties; + } + + public List getPropertyEntries() { + return propertyEntries; + } + + public List getElementNames() { + List elementNames = new ArrayList<>(); + if ( parameter != null ) { + // only relevant for source properties + elementNames.add( parameter.getName() ); + } + elementNames.addAll( propertyEntries ); + return elementNames; + } + + /** + * returns the property name on the shallowest nesting level + */ + public String getShallowestPropertyName() { + if ( propertyEntries.isEmpty() ) { + return null; + } + return first( propertyEntries ); + } + + public boolean isNested() { + return propertyEntries.size() > 1; + } + + @Override + public String toString() { + + String result = ""; + if ( propertyEntries.isEmpty() ) { + if ( parameter != null ) { + result = String.format( "parameter \"%s %s\"", parameter.getType(), parameter.getName() ); + } + } + else if ( propertyEntries.size() == 1 ) { + String propertyEntry = propertyEntries.get( 0 ); + result = String.format( "property \"%s\"", propertyEntry ); + } + else { + result = String.format( + "property \"%s\"", + Strings.join( getElementNames(), "." ) + ); + } + return result; + } /** * Builds a {@link TargetReference} from an {@code @Mappping}. @@ -62,21 +123,15 @@ public class TargetReference extends AbstractReference { private Method method; private FormattingMessager messager; private TypeFactory typeFactory; + private Set targetProperties; + private Type targetType; // mapping parameters - private boolean isReversed = false; - private boolean isIgnored = false; private String targetName = null; + private MappingOptions mapping; private AnnotationMirror annotationMirror = null; private AnnotationValue targetAnnotationValue = null; private AnnotationValue sourceAnnotationValue = null; - private Method templateMethod = null; - /** - * During {@link #getTargetEntries(Type, String[])} an error can occur. However, we are invoking - * that multiple times because the first entry can also be the name of the parameter. Therefore we keep - * the error message until the end when we can report it. - */ - private MappingErrorMessage errorMessage; public Builder messager(FormattingMessager messager) { this.messager = messager; @@ -84,11 +139,7 @@ public class TargetReference extends AbstractReference { } public Builder mapping(MappingOptions mapping) { - if ( mapping.getInheritContext() != null ) { - this.isReversed = mapping.getInheritContext().isReversed(); - this.templateMethod = mapping.getInheritContext().getTemplateMethod(); - } - this.isIgnored = mapping.isIgnored(); + this.mapping = mapping; this.targetName = mapping.getTargetName(); this.annotationMirror = mapping.getMirror(); this.targetAnnotationValue = mapping.getTargetAnnotationValue(); @@ -106,11 +157,22 @@ public class TargetReference extends AbstractReference { return this; } + public Builder targetProperties(Set targetProperties) { + this.targetProperties = targetProperties; + return this; + } + + public Builder targetType(Type targetType) { + this.targetType = targetType; + return this; + } + public TargetReference build() { Objects.requireNonNull( method ); Objects.requireNonNull( typeFactory ); Objects.requireNonNull( messager ); + Objects.requireNonNull( targetType ); if ( targetName == null ) { return null; @@ -130,185 +192,27 @@ public class TargetReference extends AbstractReference { String[] segments = targetNameTrimmed.split( "\\." ); Parameter parameter = method.getMappingTargetParameter(); - boolean foundEntryMatch; - Type resultType = typeBasedOnMethod( method.getResultType() ); - // there can be 4 situations // 1. Return type // 2. An inverse target reference where the source parameter name is used in the original mapping // 3. @MappingTarget, with // 4. or without parameter name. String[] targetPropertyNames = segments; - List entries = getTargetEntries( resultType, targetPropertyNames ); - foundEntryMatch = (entries.size() == targetPropertyNames.length); - if ( !foundEntryMatch && segments.length > 1 - && matchesSourceOrTargetParameter( segments[0], parameter, isReversed ) ) { - targetPropertyNames = Arrays.copyOfRange( segments, 1, segments.length ); - entries = getTargetEntries( resultType, targetPropertyNames ); - foundEntryMatch = (entries.size() == targetPropertyNames.length); - } - - if ( !foundEntryMatch && errorMessage != null && !isReversed ) { - // This is called only for reporting errors - errorMessage.report( ); - } - - // foundEntryMatch = isValid, errors are handled here, and the BeanMapping uses that to ignore - // the creation of mapping for invalid TargetReferences - return new TargetReference( parameter, entries, foundEntryMatch ); - } - - private List getTargetEntries(Type type, String[] entryNames) { - - // initialize - CollectionMappingStrategyGem cms = method.getOptions().getMapper().getCollectionMappingStrategy(); - List targetEntries = new ArrayList<>(); - Type nextType = type; - - // iterate, establish for each entry the target write accessors. Other than setter is only allowed for - // last entry - for ( int i = 0; i < entryNames.length; i++ ) { - - Type mappingType = typeBasedOnMethod( nextType ); - Accessor targetReadAccessor = mappingType.getPropertyReadAccessors().get( entryNames[i] ); - Accessor targetWriteAccessor = mappingType.getPropertyWriteAccessors( cms ).get( entryNames[i] ); - boolean isLast = i == entryNames.length - 1; - boolean isNotLast = i < entryNames.length - 1; - if ( isWriteAccessorNotValidWhenNotLast( targetWriteAccessor, isNotLast ) - || isWriteAccessorNotValidWhenLast( targetWriteAccessor, targetReadAccessor, isIgnored, isLast ) ) { - // there should always be a write accessor (except for the last when the mapping is ignored and - // there is a read accessor) and there should be read accessor mandatory for all but the last - setErrorMessage( targetWriteAccessor, targetReadAccessor, entryNames, i, nextType ); - break; - } - - if ( isLast || ( targetWriteAccessor.getAccessorType() == AccessorType.SETTER || - targetWriteAccessor.getAccessorType() == AccessorType.FIELD ) ) { - // only intermediate nested properties when they are a true setter or field accessor - // the last may be other readAccessor (setter / getter / adder). - - nextType = findNextType( nextType, targetWriteAccessor, targetReadAccessor ); - - // check if an entry alread exists, otherwise create - String[] fullName = Arrays.copyOfRange( entryNames, 0, i + 1 ); - BuilderType builderType; - PropertyEntry propertyEntry = null; - if ( method.isUpdateMethod() ) { - propertyEntry = PropertyEntry.forTargetReference( fullName, - targetReadAccessor, - targetWriteAccessor, - nextType, - null - ); + if ( segments.length > 1 ) { + String firstTargetProperty = targetPropertyNames[0]; + // If the first target property is not within the defined target properties, then check if it matches + // the source or target parameter + if ( !targetProperties.contains( firstTargetProperty ) ) { + if ( matchesSourceOrTargetParameter( firstTargetProperty, parameter ) ) { + targetPropertyNames = Arrays.copyOfRange( segments, 1, segments.length ); } - else { - BuilderGem builder = method.getOptions().getBeanMapping().getBuilder(); - builderType = typeFactory.builderTypeFor( nextType, builder ); - propertyEntry = PropertyEntry.forTargetReference( fullName, - targetReadAccessor, - targetWriteAccessor, - nextType, - builderType - ); - - } - targetEntries.add( propertyEntry ); } - } - return targetEntries; - } - /** - * Finds the next type based on the initial type. - * - * @param initial for which a next type should be found - * @param targetWriteAccessor the write accessor - * @param targetReadAccessor the read accessor - * @return the next type that should be used for finding a property entry - */ - private Type findNextType(Type initial, Accessor targetWriteAccessor, Accessor targetReadAccessor) { - Type nextType; - Accessor toUse = targetWriteAccessor != null ? targetWriteAccessor : targetReadAccessor; - if ( toUse.getAccessorType() == AccessorType.GETTER || toUse.getAccessorType() == AccessorType.FIELD ) { - nextType = typeFactory.getReturnType( - (DeclaredType) typeBasedOnMethod( initial ).getTypeMirror(), - toUse - ); - } - else { - nextType = typeFactory.getSingleParameter( - (DeclaredType) typeBasedOnMethod( initial ).getTypeMirror(), - toUse - ).getType(); - } - return nextType; - } + List entries = new ArrayList<>( Arrays.asList( targetPropertyNames ) ); - private void setErrorMessage(Accessor targetWriteAccessor, Accessor targetReadAccessor, String[] entryNames, - int index, Type nextType) { - if ( targetWriteAccessor == null && targetReadAccessor == null ) { - errorMessage = new NoPropertyErrorMessage( this, entryNames, index, nextType ); - } - else if ( targetWriteAccessor == null ) { - errorMessage = new NoWriteAccessorErrorMessage(this ); - } - else { - //TODO there is no read accessor. What should we do here? - errorMessage = new NoPropertyErrorMessage( this, entryNames, index, nextType ); - } - } - - /** - * When we are in an update method, i.e. source parameter with {@code @MappingTarget} then the type should - * be itself, otherwise, we always get the effective type. The reason is that when doing updates we always - * search for setters and getters within the updating type. - */ - private Type typeBasedOnMethod(Type type) { - if ( method.isUpdateMethod() ) { - return type; - } - else { - BuilderGem builder = method.getOptions().getBeanMapping().getBuilder(); - return typeFactory.effectiveResultTypeFor( type, builder ); - } - } - - /** - * A write accessor is not valid if it is {@code null} and it is not last. i.e. for nested target mappings - * there must be a write accessor for all entries except the last one. - * - * @param accessor that needs to be checked - * @param isNotLast whether or not this is the last write accessor in the entry chain - * - * @return {@code true} if the accessor is not valid, {@code false} otherwise - */ - private static boolean isWriteAccessorNotValidWhenNotLast(Accessor accessor, boolean isNotLast) { - return accessor == null && isNotLast; - } - - /** - * For a last accessor to be valid, a read accessor should exist and the mapping should be ignored. All other - * cases represent an invalid write accessor. This method will evaluate to {@code true} if the following is - * {@code true}: - *
      - *
    • {@code writeAccessor} is {@code null}
      • - *
    • It is for the last entry
      • - *
    • A read accessor does not exist, or the mapping is not ignored
      • - *
    - * - * @param writeAccessor that needs to be checked - * @param readAccessor that is used - * @param isIgnored true when ignored - * @param isLast whether or not this is the last write accessor in the entry chain - * - * @return {@code true} if the write accessor is not valid, {@code false} otherwise. See description for more - * information - */ - private static boolean isWriteAccessorNotValidWhenLast(Accessor writeAccessor, Accessor readAccessor, - boolean isIgnored, boolean isLast) { - return writeAccessor == null && isLast && ( readAccessor == null || !isIgnored ); + return new TargetReference( parameter, entries ); } /** @@ -317,16 +221,14 @@ public class TargetReference extends AbstractReference { * * @param segment that needs to be checked * @param targetParameter the target parameter if it exists - - * @param isInverse whether a inverse {@link TargetReference} is being built * * @return {@code true} if the segment matches the name of the {@code targetParameter} or the name of the * {@code inverseSourceParameter} when this is a inverse {@link TargetReference} is being built, {@code * false} otherwise */ - private boolean matchesSourceOrTargetParameter(String segment, Parameter targetParameter, boolean isInverse) { + private boolean matchesSourceOrTargetParameter(String segment, Parameter targetParameter) { boolean matchesTargetParameter = targetParameter != null && targetParameter.getName().equals( segment ); - return matchesTargetParameter || matchesSourceOnInverseSourceParameter( segment, isInverse ); + return matchesTargetParameter || matchesSourceOnInverseSourceParameter( segment ); } /** @@ -344,13 +246,16 @@ public class TargetReference extends AbstractReference { * a inverse is created. * * @param segment that needs to be checked* - * @param isInverse whether a inverse {@link TargetReference} is being built * * @return on match when inverse and segment matches the one and only source parameter */ - private boolean matchesSourceOnInverseSourceParameter( String segment, boolean isInverse ) { + private boolean matchesSourceOnInverseSourceParameter(String segment) { + boolean result = false; - if ( isInverse ) { + MappingOptions.InheritContext inheritContext = mapping.getInheritContext(); + if ( inheritContext != null && inheritContext.isReversed() ) { + + Method templateMethod = inheritContext.getTemplateMethod(); // there is only source parameter by definition when applying @InheritInverseConfiguration Parameter inverseSourceParameter = first( templateMethod.getSourceParameters() ); result = inverseSourceParameter.getName().equals( segment ); @@ -359,87 +264,19 @@ public class TargetReference extends AbstractReference { } } - private TargetReference(Parameter sourceParameter, List targetPropertyEntries, boolean isValid) { - super( sourceParameter, targetPropertyEntries, isValid ); - } - public TargetReference pop() { if ( getPropertyEntries().size() > 1 ) { - List newPropertyEntries = new ArrayList<>( getPropertyEntries().size() - 1 ); - for ( PropertyEntry propertyEntry : getPropertyEntries() ) { - PropertyEntry newPropertyEntry = propertyEntry.pop(); - if ( newPropertyEntry != null ) { - newPropertyEntries.add( newPropertyEntry ); - } - } - return new TargetReference( null, newPropertyEntries, isValid() ); + List newPathProperties = new ArrayList<>( this.pathProperties ); + newPathProperties.add( getPropertyEntries().get( 0 ) ); + return new TargetReference( + null, + getPropertyEntries().subList( 1, getPropertyEntries().size() ), + newPathProperties + ); } else { return null; } } - private abstract static class MappingErrorMessage { - private final Builder builder; - - private MappingErrorMessage(Builder builder) { - this.builder = builder; - } - - abstract void report(); - - protected void printErrorMessage(Message message, Object... args) { - Object[] errorArgs = new Object[args.length + 2]; - errorArgs[0] = builder.targetName; - errorArgs[1] = builder.method.getResultType(); - System.arraycopy( args, 0, errorArgs, 2, args.length ); - AnnotationMirror annotationMirror = builder.annotationMirror; - builder.messager.printMessage( builder.method.getExecutable(), - annotationMirror, - builder.sourceAnnotationValue, - message, - errorArgs - ); - } - } - - private static class NoWriteAccessorErrorMessage extends MappingErrorMessage { - - private NoWriteAccessorErrorMessage(Builder builder) { - super( builder ); - } - - @Override - public void report() { - printErrorMessage( Message.BEANMAPPING_PROPERTY_HAS_NO_WRITE_ACCESSOR_IN_RESULTTYPE ); - } - } - - private static class NoPropertyErrorMessage extends MappingErrorMessage { - - private final String[] entryNames; - private final int index; - private final Type nextType; - - private NoPropertyErrorMessage(Builder builder, String[] entryNames, int index, - Type nextType) { - super( builder ); - this.entryNames = entryNames; - this.index = index; - this.nextType = nextType; - } - - @Override - public void report() { - - Set readAccessors = nextType.getPropertyReadAccessors().keySet(); - String mostSimilarProperty = Strings.getMostSimilarWord( entryNames[index], readAccessors ); - - List elements = new ArrayList<>( Arrays.asList( entryNames ).subList( 0, index ) ); - elements.add( mostSimilarProperty ); - - printErrorMessage( Message.BEANMAPPING_UNKNOWN_PROPERTY_IN_RESULTTYPE, Strings.join( elements, "." ) ); - } - } - } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/MappingOptions.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/MappingOptions.java index ed88e5107..b826295f7 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/MappingOptions.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/MappingOptions.java @@ -16,6 +16,7 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.AnnotationValue; +import javax.lang.model.element.Element; import javax.lang.model.element.ExecutableElement; import javax.lang.model.util.Elements; import javax.lang.model.util.Types; @@ -50,6 +51,7 @@ public class MappingOptions extends DelegatingOptions { private final boolean isIgnored; private final Set dependsOn; + private final Element element; private final AnnotationValue sourceAnnotationValue; private final AnnotationValue targetAnnotationValue; private final MappingGem mapping; @@ -135,6 +137,7 @@ public class MappingOptions extends DelegatingOptions { MappingOptions options = new MappingOptions( mapping.target().getValue(), + method, mapping.target().getAnnotationValue(), source, mapping.source().getAnnotationValue(), @@ -169,6 +172,7 @@ public class MappingOptions extends DelegatingOptions { null, null, null, + null, true, null, null, @@ -249,6 +253,7 @@ public class MappingOptions extends DelegatingOptions { @SuppressWarnings("checkstyle:parameternumber") private MappingOptions(String targetName, + Element element, AnnotationValue targetAnnotationValue, String sourceName, AnnotationValue sourceAnnotationValue, @@ -266,6 +271,7 @@ public class MappingOptions extends DelegatingOptions { ) { super( next ); this.targetName = targetName; + this.element = element; this.targetAnnotationValue = targetAnnotationValue; this.sourceName = sourceName; this.sourceAnnotationValue = sourceAnnotationValue; @@ -377,6 +383,10 @@ public class MappingOptions extends DelegatingOptions { return Optional.ofNullable( mapping ).map( MappingGem::mirror ).orElse( null ); } + public Element getElement() { + return element; + } + public AnnotationValue getDependsOnAnnotationValue() { return Optional.ofNullable( mapping ) .map( MappingGem::dependsOn ) @@ -434,6 +444,7 @@ public class MappingOptions extends DelegatingOptions { MappingOptions mappingOptions = new MappingOptions( sourceName != null ? sourceName : targetName, + templateMethod.getExecutable(), targetAnnotationValue, sourceName != null ? targetName : null, sourceAnnotationValue, @@ -462,6 +473,7 @@ public class MappingOptions extends DelegatingOptions { BeanMappingOptions beanMappingOptions ) { MappingOptions mappingOptions = new MappingOptions( targetName, + templateMethod.getExecutable(), targetAnnotationValue, sourceName, sourceAnnotationValue, diff --git a/processor/src/main/java/org/mapstruct/ap/internal/processor/DefaultModelElementProcessorContext.java b/processor/src/main/java/org/mapstruct/ap/internal/processor/DefaultModelElementProcessorContext.java index bc7374d39..a9f9eff4a 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/processor/DefaultModelElementProcessorContext.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/processor/DefaultModelElementProcessorContext.java @@ -167,6 +167,7 @@ public class DefaultModelElementProcessorContext implements ProcessorContext { } } + @Override public boolean isErroneous() { return isErroneous; } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/processor/MapperCreationProcessor.java b/processor/src/main/java/org/mapstruct/ap/internal/processor/MapperCreationProcessor.java index 197d9fa63..7746dd677 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/processor/MapperCreationProcessor.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/processor/MapperCreationProcessor.java @@ -368,10 +368,10 @@ public class MapperCreationProcessor implements ModelElementProcessor does not have an accessible parameterless constructor."), - @Diagnostic(type = ErroneousSourceTargetMapper6.class, kind = javax.tools.Diagnostic.Kind.ERROR, - line = 16, message = + line = 17, message = "No target bean properties found: can't map property \"org.mapstruct.ap.test.NoProperties " + "foo.wrapped\" to" + " \"org.mapstruct.ap.test.selection.generics.TypeA " diff --git a/processor/src/test/java/org/mapstruct/ap/test/selection/generics/ErroneousSourceTargetMapper6.java b/processor/src/test/java/org/mapstruct/ap/test/selection/generics/ErroneousSourceTargetMapper6.java index 731b743b8..210cc3692 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/selection/generics/ErroneousSourceTargetMapper6.java +++ b/processor/src/test/java/org/mapstruct/ap/test/selection/generics/ErroneousSourceTargetMapper6.java @@ -6,6 +6,7 @@ package org.mapstruct.ap.test.selection.generics; import org.mapstruct.Mapper; +import org.mapstruct.ObjectFactory; import org.mapstruct.factory.Mappers; @Mapper( uses = GenericTypeMapper.class ) @@ -14,4 +15,11 @@ public interface ErroneousSourceTargetMapper6 { ErroneousSourceTargetMapper6 INSTANCE = Mappers.getMapper( ErroneousSourceTargetMapper6.class ); ErroneousTarget6 sourceToTarget(ErroneousSource6 source); + + // We are testing that we can't create the nested + // not whether or not we can instantiate the WildCardSuperWrapper + @ObjectFactory + default WildCardSuperWrapper createWildCardSuperWrapper() { + return new WildCardSuperWrapper<>( null ); + } }