From 70a0906221253266688d62a726ab332c1357085e Mon Sep 17 00:00:00 2001 From: sjaakd Date: Sun, 28 Apr 2019 20:55:43 +0200 Subject: [PATCH 1/9] #1742 refactor accessors (make consistent) to have accessed type always available --- .../ap/internal/model/BeanMappingMethod.java | 2 +- .../model/CollectionAssignmentBuilder.java | 18 +-- .../ap/internal/model/PropertyMapping.java | 82 ++++-------- .../ap/internal/model/common/Type.java | 117 +++++++++--------- .../ap/internal/model/common/TypeFactory.java | 11 +- .../internal/model/source/PropertyEntry.java | 24 ++-- .../model/source/SourceReference.java | 6 +- .../model/source/TargetReference.java | 8 +- .../ap/internal/util/AccessorNamingUtils.java | 44 +++---- .../ap/internal/util/Executables.java | 103 ++++----------- .../mapstruct/ap/internal/util/Fields.java | 107 ++++++++++++++++ .../mapstruct/ap/internal/util/Filters.java | 74 ++++++++--- .../ap/internal/util/ValueProvider.java | 3 +- .../util/accessor/AbstractAccessor.java | 1 + .../ap/internal/util/accessor/Accessor.java | 13 +- .../internal/util/accessor/AccessorType.java | 15 +++ .../accessor/ExecutableElementAccessor.java | 17 ++- .../accessor/VariableElementAccessor.java | 9 +- 18 files changed, 370 insertions(+), 284 deletions(-) create mode 100644 processor/src/main/java/org/mapstruct/ap/internal/util/Fields.java create mode 100644 processor/src/main/java/org/mapstruct/ap/internal/util/accessor/AccessorType.java 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 dc4c005bb7..a0e4975e7b 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 @@ -590,7 +590,7 @@ private void applyPropertyNameBasedMapping() { Accessor sourceReadAccessor = sourceParameter.getType().getPropertyReadAccessors().get( targetPropertyName ); - ExecutableElementAccessor sourcePresenceChecker = + Accessor sourcePresenceChecker = sourceParameter.getType().getPropertyPresenceCheckers().get( targetPropertyName ); if ( sourceReadAccessor != null ) { diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/CollectionAssignmentBuilder.java b/processor/src/main/java/org/mapstruct/ap/internal/model/CollectionAssignmentBuilder.java index ad9defb9d2..6422a354d4 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/CollectionAssignmentBuilder.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/CollectionAssignmentBuilder.java @@ -20,6 +20,7 @@ import org.mapstruct.ap.internal.prism.NullValuePropertyMappingStrategyPrism; import org.mapstruct.ap.internal.util.Message; import org.mapstruct.ap.internal.util.accessor.Accessor; +import org.mapstruct.ap.internal.util.accessor.AccessorType; import static org.mapstruct.ap.internal.prism.NullValuePropertyMappingStrategyPrism.SET_TO_DEFAULT; import static org.mapstruct.ap.internal.prism.NullValuePropertyMappingStrategyPrism.SET_TO_NULL; @@ -57,7 +58,7 @@ public class CollectionAssignmentBuilder { private Accessor targetReadAccessor; private Type targetType; private String targetPropertyName; - private PropertyMapping.TargetWriteAccessorType targetAccessorType; + private AccessorType targetAccessorType; private Assignment assignment; private SourceRHS sourceRHS; private NullValueCheckStrategyPrism nvcs; @@ -88,7 +89,7 @@ public CollectionAssignmentBuilder targetPropertyName(String targetPropertyName) return this; } - public CollectionAssignmentBuilder targetAccessorType(PropertyMapping.TargetWriteAccessorType targetAccessorType) { + public CollectionAssignmentBuilder targetAccessorType(AccessorType targetAccessorType) { this.targetAccessorType = targetAccessorType; return this; } @@ -129,8 +130,7 @@ public Assignment build() { CollectionMappingStrategyPrism cms = method.getMapperConfiguration().getCollectionMappingStrategy(); boolean targetImmutable = cms == CollectionMappingStrategyPrism.TARGET_IMMUTABLE || targetReadAccessor == null; - if ( targetAccessorType == PropertyMapping.TargetWriteAccessorType.SETTER || - targetAccessorType == PropertyMapping.TargetWriteAccessorType.FIELD ) { + if ( targetAccessorType == AccessorType.SETTER || targetAccessorType == AccessorType.FIELD ) { if ( result.isCallingUpdateMethod() && !targetImmutable ) { @@ -149,7 +149,7 @@ public Assignment build() { result, method.getThrownTypes(), factoryMethod, - PropertyMapping.TargetWriteAccessorType.isFieldAssignment( targetAccessorType ), + targetAccessorType == AccessorType.FIELD, targetType, true, nvpms == SET_TO_NULL && !targetType.isPrimitive(), @@ -165,7 +165,7 @@ else if ( method.isUpdateMethod() && !targetImmutable ) { nvcs, nvpms, ctx.getTypeFactory(), - PropertyMapping.TargetWriteAccessorType.isFieldAssignment( targetAccessorType ) + targetAccessorType == AccessorType.FIELD ); } else if ( result.getType() == Assignment.AssignmentType.DIRECT || @@ -176,7 +176,7 @@ else if ( result.getType() == Assignment.AssignmentType.DIRECT || method.getThrownTypes(), targetType, ctx.getTypeFactory(), - PropertyMapping.TargetWriteAccessorType.isFieldAssignment( targetAccessorType ) + targetAccessorType == AccessorType.FIELD ); } else { @@ -185,7 +185,7 @@ else if ( result.getType() == Assignment.AssignmentType.DIRECT || result, method.getThrownTypes(), targetType, - PropertyMapping.TargetWriteAccessorType.isFieldAssignment( targetAccessorType ) + targetAccessorType == AccessorType.FIELD ); } } @@ -203,7 +203,7 @@ else if ( result.getType() == Assignment.AssignmentType.DIRECT || result, method.getThrownTypes(), targetType, - PropertyMapping.TargetWriteAccessorType.isFieldAssignment( targetAccessorType ) + targetAccessorType == AccessorType.FIELD ); } 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 2f892e6b5e..57cb84bb80 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 @@ -22,6 +22,7 @@ import org.mapstruct.ap.internal.model.assignment.StreamAdderWrapper; import org.mapstruct.ap.internal.model.assignment.UpdateWrapper; import org.mapstruct.ap.internal.model.common.Assignment; +import org.mapstruct.ap.internal.model.common.BuilderType; import org.mapstruct.ap.internal.model.common.FormattingParameters; import org.mapstruct.ap.internal.model.common.ModelElement; import org.mapstruct.ap.internal.model.common.Parameter; @@ -48,6 +49,7 @@ import org.mapstruct.ap.internal.util.Strings; import org.mapstruct.ap.internal.util.ValueProvider; import org.mapstruct.ap.internal.util.accessor.Accessor; +import org.mapstruct.ap.internal.util.accessor.AccessorType; import static org.mapstruct.ap.internal.model.common.Assignment.AssignmentType.DIRECT; import static org.mapstruct.ap.internal.prism.NullValuePropertyMappingStrategyPrism.SET_TO_DEFAULT; @@ -73,38 +75,14 @@ public class PropertyMapping extends ModelElement { private final List dependsOn; private final Assignment defaultValueAssignment; - public enum TargetWriteAccessorType { - FIELD, - GETTER, - SETTER, - ADDER; - - public static TargetWriteAccessorType of(AccessorNamingUtils accessorNaming, Accessor accessor) { - if ( accessorNaming.isSetterMethod( accessor ) ) { - return TargetWriteAccessorType.SETTER; - } - else if ( accessorNaming.isAdderMethod( accessor ) ) { - return TargetWriteAccessorType.ADDER; - } - else if ( accessorNaming.isGetterMethod( accessor ) ) { - return TargetWriteAccessorType.GETTER; - } - else { - return TargetWriteAccessorType.FIELD; - } - } - - public static boolean isFieldAssignment(TargetWriteAccessorType accessorType) { - return accessorType == FIELD; - } - } @SuppressWarnings("unchecked") private static class MappingBuilderBase> extends AbstractBaseBuilder { protected Accessor targetWriteAccessor; - protected TargetWriteAccessorType targetWriteAccessorType; + protected AccessorType targetWriteAccessorType; protected Type targetType; + protected BuilderType targetBuilderType; protected Accessor targetReadAccessor; protected String targetPropertyName; protected String sourcePropertyName; @@ -125,7 +103,8 @@ public T targetProperty(PropertyEntry targetProp) { this.targetReadAccessor = targetProp.getReadAccessor(); this.targetWriteAccessor = targetProp.getWriteAccessor(); this.targetType = targetProp.getType(); - this.targetWriteAccessorType = TargetWriteAccessorType.of( ctx.getAccessorNaming(), targetWriteAccessor ); + this.targetBuilderType = targetProp.getBuilderType(); + this.targetWriteAccessorType = targetWriteAccessor.getAccessorType(); return (T) this; } @@ -136,7 +115,7 @@ public T targetReadAccessor(Accessor targetReadAccessor) { public T targetWriteAccessor(Accessor targetWriteAccessor) { this.targetWriteAccessor = targetWriteAccessor; - this.targetWriteAccessorType = TargetWriteAccessorType.of( ctx.getAccessorNaming(), targetWriteAccessor ); + this.targetWriteAccessorType = targetWriteAccessor.getAccessorType(); this.targetType = determineTargetType(); return (T) this; @@ -148,25 +127,7 @@ T mirror(AnnotationMirror mirror) { } private Type determineTargetType() { - // This is a bean mapping method, so we know the result is a declared type - Type mappingType = method.getResultType(); - if ( !method.isUpdateMethod() ) { - mappingType = mappingType.getEffectiveType(); - } - DeclaredType resultType = (DeclaredType) mappingType.getTypeMirror(); - - switch ( targetWriteAccessorType ) { - case ADDER: - case SETTER: - return ctx.getTypeFactory() - .getSingleParameter( resultType, targetWriteAccessor ) - .getType(); - case GETTER: - case FIELD: - default: - return ctx.getTypeFactory() - .getReturnType( resultType, targetWriteAccessor ); - } + return ctx.getTypeFactory().getType( targetWriteAccessor.getAccessedType() ); } public T targetPropertyName(String targetPropertyName) { @@ -190,7 +151,7 @@ public T existingVariableNames(Set existingVariableNames) { } protected boolean isFieldAssignment() { - return targetWriteAccessorType == TargetWriteAccessorType.FIELD; + return targetWriteAccessorType == AccessorType.FIELD; } } @@ -300,11 +261,11 @@ public PropertyMapping build() { ctx.getMessager().note( 2, Message.PROPERTYMAPPING_MAPPING_NOTE, rightHandSide, targetWriteAccessor ); rightHandSide.setUseElementAsSourceTypeForMatching( - targetWriteAccessorType == TargetWriteAccessorType.ADDER ); + targetWriteAccessorType == AccessorType.ADDER ); // all the tricky cases will be excluded for the time being. boolean preferUpdateMethods; - if ( targetWriteAccessorType == TargetWriteAccessorType.ADDER ) { + if ( targetWriteAccessorType == AccessorType.ADDER ) { preferUpdateMethods = false; } else { @@ -446,13 +407,12 @@ private Assignment getDefaultValueAssignment( Assignment rhs ) { return null; } - private Assignment assignToPlain(Type targetType, TargetWriteAccessorType targetAccessorType, + private Assignment assignToPlain(Type targetType, AccessorType targetAccessorType, Assignment rightHandSide) { Assignment result; - if ( targetAccessorType == TargetWriteAccessorType.SETTER || - targetAccessorType == TargetWriteAccessorType.FIELD ) { + if ( targetAccessorType == AccessorType.SETTER || targetAccessorType == AccessorType.FIELD ) { result = assignToPlainViaSetter( targetType, rightHandSide ); } else { @@ -530,7 +490,7 @@ else if ( result.getSourceType().isStreamType() ) { return result; } - private Assignment assignToCollection(Type targetType, TargetWriteAccessorType targetAccessorType, + private Assignment assignToCollection(Type targetType, AccessorType targetAccessorType, Assignment rhs) { return new CollectionAssignmentBuilder() .mappingBuilderContext( ctx ) @@ -738,7 +698,7 @@ private Assignment forgeMapMapping(Type sourceType, Type targetType, SourceRHS s private Assignment forgeMapping(SourceRHS sourceRHS) { Type sourceType; - if ( targetWriteAccessorType == TargetWriteAccessorType.ADDER ) { + if ( targetWriteAccessorType == AccessorType.ADDER ) { sourceType = sourceRHS.getSourceTypeForMatching(); } else { @@ -764,7 +724,7 @@ private Assignment forgeMapping(SourceRHS sourceRHS) { // because we are forging a Mapping for a method with multiple source parameters. // If the target type is enum, then we can't create an update method if ( !targetType.isEnumType() && ( method.isUpdateMethod() || forceUpdateMethod ) - && targetWriteAccessorType != TargetWriteAccessorType.ADDER) { + && targetWriteAccessorType != AccessorType.ADDER) { parameters.add( Parameter.forForgedMappingTarget( targetType ) ); returnType = ctx.getTypeFactory().createVoidType(); } @@ -783,7 +743,7 @@ private Assignment forgeMapping(SourceRHS sourceRHS) { forgeMethodWithMappingOptions, forgedNamedBased ); - return createForgedAssignment( sourceRHS, forgedMethod ); + return createForgedAssignment( sourceRHS, targetBuilderType, forgedMethod ); } private ForgedMethodHistory getForgedMethodHistory(SourceRHS sourceRHS) { @@ -899,8 +859,8 @@ public PropertyMapping build() { if ( assignment != null ) { - if ( ctx.getAccessorNaming().isSetterMethod( targetWriteAccessor ) || - Executables.isFieldAccessor( targetWriteAccessor ) ) { + if ( targetWriteAccessor.getAccessorType() == AccessorType.SETTER || + targetWriteAccessor.getAccessorType() == AccessorType.FIELD ) { // target accessor is setter, so decorate assignment as setter if ( assignment.isCallingUpdateMethod() ) { @@ -1015,8 +975,8 @@ public JavaExpressionMappingBuilder javaExpression(String javaExpression) { public PropertyMapping build() { Assignment assignment = new SourceRHS( javaExpression, null, existingVariableNames, "" ); - if ( ctx.getAccessorNaming().isSetterMethod( targetWriteAccessor ) || - Executables.isFieldAccessor( targetWriteAccessor ) ) { + if ( targetWriteAccessor.getAccessorType() == AccessorType.SETTER || + targetWriteAccessor.getAccessorType() == AccessorType.FIELD ) { // setter, so wrap in setter assignment = new SetterWrapper( assignment, method.getThrownTypes(), isFieldAssignment() ); } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java b/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java index 5992a687bd..1b8f233106 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java @@ -13,6 +13,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; import java.util.stream.Stream; import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.Element; @@ -34,12 +35,12 @@ import org.mapstruct.ap.internal.prism.CollectionMappingStrategyPrism; import org.mapstruct.ap.internal.util.AccessorNamingUtils; import org.mapstruct.ap.internal.util.Executables; +import org.mapstruct.ap.internal.util.Fields; import org.mapstruct.ap.internal.util.Filters; import org.mapstruct.ap.internal.util.JavaStreamConstants; import org.mapstruct.ap.internal.util.Nouns; import org.mapstruct.ap.internal.util.accessor.Accessor; -import org.mapstruct.ap.internal.util.accessor.ExecutableElementAccessor; -import org.mapstruct.ap.spi.BuilderInfo; +import org.mapstruct.ap.internal.util.accessor.AccessorType; import static org.mapstruct.ap.internal.util.Collections.first; import org.mapstruct.ap.internal.util.NativeTypes; @@ -67,7 +68,6 @@ public class Type extends ModelElement implements Comparable { private final ImplementationType implementationType; private final Type componentType; - private final BuilderType builderType; private final String packageName; private final String name; @@ -89,9 +89,10 @@ public class Type extends ModelElement implements Comparable { private Boolean isToBeImported; private Map readAccessors = null; - private Map presenceCheckers = null; + private Map presenceCheckers = null; - private List allAccessors = null; + private List allMethods = null; + private List allFields = null; private List setters = null; private List adders = null; private List alternativeTargetAccessors = null; @@ -100,12 +101,13 @@ public class Type extends ModelElement implements Comparable { private Boolean hasEmptyAccessibleContructor; + private final Filters filters; + //CHECKSTYLE:OFF public Type(Types typeUtils, Elements elementUtils, TypeFactory typeFactory, AccessorNamingUtils accessorNaming, TypeMirror typeMirror, TypeElement typeElement, List typeParameters, ImplementationType implementationType, Type componentType, - BuilderInfo builderInfo, String packageName, String name, String qualifiedName, boolean isInterface, boolean isEnumType, boolean isIterableType, boolean isCollectionType, boolean isMapType, boolean isStreamType, @@ -157,7 +159,7 @@ public Type(Types typeUtils, Elements elementUtils, TypeFactory typeFactory, this.isToBeImported = isToBeImported; this.toBeImportedTypes = toBeImportedTypes; this.notToBeImportedTypes = notToBeImportedTypes; - this.builderType = BuilderType.create( builderInfo, this, this.typeFactory, this.typeUtils ); + this.filters = new Filters( accessorNaming, typeUtils, typeMirror ); } //CHECKSTYLE:ON @@ -200,18 +202,6 @@ public Type getComponentType() { return componentType; } - public BuilderType getBuilderType() { - return builderType; - } - - /** - * The effective type that should be used when searching for getters / setters, creating new types etc - * @return the effective type for mappings - */ - public Type getEffectiveType() { - return builderType != null ? builderType.getBuilder() : this; - } - public boolean isPrimitive() { return typeMirror.getKind().isPrimitive(); } @@ -419,7 +409,6 @@ public Type erasure() { typeParameters, implementationType, componentType, - builderType == null ? null : builderType.asBuilderInfo(), packageName, name, qualifiedName, @@ -462,7 +451,6 @@ public Type withoutBounds() { bounds, implementationType, componentType, - builderType == null ? null : builderType.asBuilderInfo(), packageName, name, qualifiedName, @@ -505,30 +493,30 @@ public boolean isAssignableTo(Type other) { */ public Map getPropertyReadAccessors() { if ( readAccessors == null ) { - List getterList = Filters.getterMethodsIn( accessorNaming, getAllAccessors() ); + List getterList = filters.getterMethodsIn( getAllMethods() ); Map modifiableGetters = new LinkedHashMap<>(); for ( Accessor getter : getterList ) { - String propertyName = accessorNaming.getPropertyName( getter ); + String propertyName = getPropertyName( getter ); if ( modifiableGetters.containsKey( propertyName ) ) { // In the DefaultAccessorNamingStrategy, this can only be the case for Booleans: isFoo() and // getFoo(); The latter is preferred. if ( !getter.getSimpleName().toString().startsWith( "is" ) ) { - modifiableGetters.put( accessorNaming.getPropertyName( getter ), getter ); + modifiableGetters.put( getPropertyName( getter ),getter ); } } else { - modifiableGetters.put( accessorNaming.getPropertyName( getter ), getter ); + modifiableGetters.put( getPropertyName( getter ), getter ); } } - List fieldsList = Filters.fieldsIn( getAllAccessors() ); + List fieldsList = filters.fieldsIn( getAllFields() ); for ( Accessor field : fieldsList ) { - String propertyName = accessorNaming.getPropertyName( field ); + String propertyName = getPropertyName( field ); if ( !modifiableGetters.containsKey( propertyName ) ) { // If there was no getter or is method for booleans, then resort to the field. // If a field was already added do not add it again. - modifiableGetters.put( propertyName, field ); + modifiableGetters.put( propertyName, field ); } } readAccessors = Collections.unmodifiableMap( modifiableGetters ); @@ -541,15 +529,12 @@ public Map getPropertyReadAccessors() { * * @return an unmodifiable map of all presence checkers, indexed by property name */ - public Map getPropertyPresenceCheckers() { + public Map getPropertyPresenceCheckers() { if ( presenceCheckers == null ) { - List checkerList = Filters.presenceCheckMethodsIn( - accessorNaming, - getAllAccessors() - ); - Map modifiableCheckers = new LinkedHashMap<>(); - for ( ExecutableElementAccessor checker : checkerList ) { - modifiableCheckers.put( accessorNaming.getPropertyName( checker ), checker ); + List checkerList = filters.presenceCheckMethodsIn( getAllMethods() ); + Map modifiableCheckers = new LinkedHashMap<>(); + for ( Accessor checker : checkerList ) { + modifiableCheckers.put( getPropertyName( checker ), checker ); } presenceCheckers = Collections.unmodifiableMap( modifiableCheckers ); } @@ -577,7 +562,7 @@ public Map getPropertyWriteAccessors( CollectionMappingStrateg Map result = new LinkedHashMap<>(); for ( Accessor candidate : candidates ) { - String targetPropertyName = accessorNaming.getPropertyName( candidate ); + String targetPropertyName = getPropertyName( candidate ); Accessor readAccessor = getPropertyReadAccessors().get( targetPropertyName ); @@ -593,12 +578,12 @@ public Map getPropertyWriteAccessors( CollectionMappingStrateg // first check if there's a setter method. Accessor adderMethod = null; - if ( accessorNaming.isSetterMethod( candidate ) + if ( candidate.getAccessorType() == AccessorType.SETTER // ok, the current accessor is a setter. So now the strategy determines what to use && cmStrategy == CollectionMappingStrategyPrism.ADDER_PREFERRED ) { adderMethod = getAdderForType( targetType, targetPropertyName ); } - else if ( accessorNaming.isGetterMethod( candidate ) ) { + else if ( candidate.getAccessorType() == AccessorType.GETTER ) { // the current accessor is a getter (no setter available). But still, an add method is according // to the above strategy (SETTER_PREFERRED || ADDER_PREFERRED) preferred over the getter. adderMethod = getAdderForType( targetType, targetPropertyName ); @@ -608,7 +593,7 @@ else if ( accessorNaming.isGetterMethod( candidate ) ) { candidate = adderMethod; } } - else if ( Executables.isFieldAccessor( candidate ) && ( Executables.isFinal( candidate ) || + else if ( candidate.getAccessorType() == AccessorType.FIELD && ( Executables.isFinal( candidate ) || result.containsKey( targetPropertyName ) ) ) { // if the candidate is a field and a mapping already exists, then use that one, skip it. continue; @@ -636,18 +621,36 @@ private Type determineTargetType(Accessor candidate) { if ( parameter != null ) { return parameter.getType(); } - else if ( accessorNaming.isGetterMethod( candidate ) || Executables.isFieldAccessor( candidate ) ) { + else if ( candidate.getAccessorType() == AccessorType.GETTER + || candidate.getAccessorType() == AccessorType.FIELD ) { return typeFactory.getReturnType( (DeclaredType) typeMirror, candidate ); } return null; } - private List getAllAccessors() { - if ( allAccessors == null ) { - allAccessors = Executables.getAllEnclosedAccessors( elementUtils, typeElement ); + private List getAllMethods() { + if ( allMethods == null ) { + allMethods = Executables.getAllEnclosedExecutableElements( elementUtils, typeElement ); + } + + return allMethods; + } + + private List getAllFields() { + if ( allFields == null ) { + allFields = Fields.getAllEnclosedFields( elementUtils, typeElement ); } - return allAccessors; + return allFields; + } + + private String getPropertyName(Accessor accessor ) { + if ( accessor.getAccessorType() == AccessorType.FIELD ) { + return accessorNaming.getPropertyName( (VariableElement) accessor.getElement() ); + } + else { + return accessorNaming.getPropertyName( (ExecutableElement) accessor.getElement() ); + } } /** @@ -709,19 +712,14 @@ else if ( collectionProperty.isStreamType() ) { * @return accessor candidates */ private List getAccessorCandidates(Type property, Class superclass) { - TypeMirror typeArg = first( property.determineTypeArguments( superclass ) ).getTypeBound() - .getTypeMirror(); + TypeMirror typeArg = first( property.determineTypeArguments( superclass ) ).getTypeBound().getTypeMirror(); // now, look for a method that // 1) starts with add, // 2) and has typeArg as one and only arg List adderList = getAdders(); List candidateList = new ArrayList<>(); for ( Accessor adder : adderList ) { - ExecutableElement executable = adder.getExecutable(); - if ( executable == null ) { - // it should not be null, but to be safe - continue; - } + ExecutableElement executable = (ExecutableElement) adder.getElement(); VariableElement arg = executable.getParameters().get( 0 ); if ( typeUtils.isSameType( boxed( arg.asType() ), boxed( typeArg ) ) ) { candidateList.add( adder ); @@ -746,7 +744,7 @@ private TypeMirror boxed(TypeMirror possiblePrimitive) { */ private List getSetters() { if ( setters == null ) { - setters = Collections.unmodifiableList( Filters.setterMethodsIn( accessorNaming, getAllAccessors() ) ); + setters = Collections.unmodifiableList( filters.setterMethodsIn( getAllMethods() ) ); } return setters; } @@ -761,7 +759,7 @@ private List getSetters() { */ private List getAdders() { if ( adders == null ) { - adders = Collections.unmodifiableList( Filters.adderMethodsIn( accessorNaming, getAllAccessors() ) ); + adders = Collections.unmodifiableList( filters.adderMethodsIn( getAllMethods() ) ); } return adders; } @@ -781,10 +779,9 @@ private List getAlternativeTargetAccessors() { List result = new ArrayList<>(); List setterMethods = getSetters(); - List readAccessors = - new ArrayList<>( getPropertyReadAccessors().values() ); + List readAccessors = new ArrayList<>( getPropertyReadAccessors().values() ); // All the fields are also alternative accessors - readAccessors.addAll( Filters.fieldsIn( getAllAccessors() ) ); + readAccessors.addAll( filters.fieldsIn( getAllFields() ) ); // there could be a read accessor (field or method) for a list/map that is not present as setter. // an accessor could substitute the setter in that case and act as setter. @@ -794,7 +791,7 @@ private List getAlternativeTargetAccessors() { !correspondingSetterMethodExists( readAccessor, setterMethods ) ) { result.add( readAccessor ); } - else if ( Executables.isFieldAccessor( readAccessor ) && + else if ( readAccessor.getAccessorType() == AccessorType.FIELD && !correspondingSetterMethodExists( readAccessor, setterMethods ) ) { result.add( readAccessor ); } @@ -807,10 +804,10 @@ else if ( Executables.isFieldAccessor( readAccessor ) && private boolean correspondingSetterMethodExists(Accessor getterMethod, List setterMethods) { - String getterPropertyName = accessorNaming.getPropertyName( getterMethod ); + String getterPropertyName = getPropertyName( getterMethod ); for ( Accessor setterMethod : setterMethods ) { - String setterPropertyName = accessorNaming.getPropertyName( setterMethod ); + String setterPropertyName = getPropertyName( setterMethod ); if ( getterPropertyName.equals( setterPropertyName ) ) { return true; } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/common/TypeFactory.java b/processor/src/main/java/org/mapstruct/ap/internal/model/common/TypeFactory.java index 567053986e..003582777a 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/common/TypeFactory.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/common/TypeFactory.java @@ -49,6 +49,7 @@ import org.mapstruct.ap.internal.util.RoundContext; import org.mapstruct.ap.internal.util.Strings; import org.mapstruct.ap.internal.util.accessor.Accessor; +import org.mapstruct.ap.internal.util.accessor.AccessorType; import org.mapstruct.ap.spi.AstModifyingAnnotationProcessor; import org.mapstruct.ap.spi.BuilderInfo; import org.mapstruct.ap.spi.MoreThanOneBuilderCreationMethodException; @@ -354,10 +355,10 @@ public TypeMirror getMethodType(DeclaredType includingType, Element method) { } public Parameter getSingleParameter(DeclaredType includingType, Accessor method) { - ExecutableElement executable = method.getExecutable(); - if ( executable == null ) { + if ( method.getAccessorType() == AccessorType.FIELD ) { return null; } + ExecutableElement executable = (ExecutableElement) method.getElement(); List parameters = executable.getParameters(); if ( parameters.size() != 1 ) { @@ -369,7 +370,7 @@ public Parameter getSingleParameter(DeclaredType includingType, Accessor method) } public List getParameters(DeclaredType includingType, Accessor accessor) { - ExecutableElement method = accessor.getExecutable(); + ExecutableElement method = (ExecutableElement) accessor.getElement(); TypeMirror methodType = getMethodType( includingType, accessor.getElement() ); if ( method == null || methodType.getKind() != TypeKind.EXECUTABLE ) { return new ArrayList<>(); @@ -427,10 +428,10 @@ public List getThrownTypes(ExecutableType method) { } public List getThrownTypes(Accessor accessor) { - if (accessor.getExecutable() == null) { + if (accessor.getAccessorType() == AccessorType.FIELD) { return new ArrayList<>(); } - return extractTypes( accessor.getExecutable().getThrownTypes() ); + return extractTypes( ( (ExecutableElement) accessor.getElement() ).getThrownTypes() ); } private List extractTypes(List typeMirrors) { diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/PropertyEntry.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/PropertyEntry.java index 03bfe25c65..e6212db685 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/PropertyEntry.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/PropertyEntry.java @@ -7,6 +7,7 @@ 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; @@ -22,8 +23,9 @@ public class PropertyEntry { private final String[] fullName; private final Accessor readAccessor; private final Accessor writeAccessor; - private final ExecutableElementAccessor presenceChecker; + private final Accessor presenceChecker; private final Type type; + private final BuilderType builderType; /** * Constructor used to create {@link TargetReference} property entries from a mapping @@ -34,12 +36,13 @@ public class PropertyEntry { * @param type */ private PropertyEntry(String[] fullName, Accessor readAccessor, Accessor writeAccessor, - ExecutableElementAccessor presenceChecker, Type type) { + Accessor presenceChecker, Type type, BuilderType builderType) { this.fullName = fullName; this.readAccessor = readAccessor; this.writeAccessor = writeAccessor; this.presenceChecker = presenceChecker; this.type = type; + this.builderType = builderType; } /** @@ -49,11 +52,12 @@ private PropertyEntry(String[] fullName, Accessor readAccessor, Accessor writeAc * @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) { - return new PropertyEntry( fullName, readAccessor, writeAccessor, null, type ); + Accessor writeAccessor, Type type, BuilderType builderType ) { + return new PropertyEntry( fullName, readAccessor, writeAccessor, null, type, null ); } /** @@ -66,8 +70,8 @@ public static PropertyEntry forTargetReference(String[] fullName, Accessor readA * @return the property entry for given parameters. */ public static PropertyEntry forSourceReference(String name, Accessor readAccessor, - ExecutableElementAccessor presenceChecker, Type type) { - return new PropertyEntry( new String[]{name}, readAccessor, null, presenceChecker, type ); + Accessor presenceChecker, Type type) { + return new PropertyEntry( new String[]{name}, readAccessor, null, presenceChecker, type, null ); } public String getName() { @@ -82,7 +86,7 @@ public Accessor getWriteAccessor() { return writeAccessor; } - public ExecutableElementAccessor getPresenceChecker() { + public Accessor getPresenceChecker() { return presenceChecker; } @@ -90,6 +94,10 @@ public Type getType() { return type; } + public BuilderType getBuilderType() { + return builderType; + } + public String getFullName() { return Strings.join( Arrays.asList( fullName ), "." ); } @@ -97,7 +105,7 @@ public String getFullName() { public PropertyEntry pop() { if ( fullName.length > 1 ) { String[] newFullName = Arrays.copyOfRange( fullName, 1, fullName.length ); - return new PropertyEntry(newFullName, readAccessor, writeAccessor, presenceChecker, type ); + return new PropertyEntry(newFullName, readAccessor, writeAccessor, presenceChecker, type, builderType ); } else { return null; diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/SourceReference.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/SourceReference.java index 30b80c8b2f..9688c2f0b7 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/SourceReference.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/SourceReference.java @@ -261,7 +261,7 @@ private List matchWithSourceAccessorTypes(Type type, String[] ent for ( String entryName : entryNames ) { boolean matchFound = false; Map sourceReadAccessors = newType.getPropertyReadAccessors(); - Map sourcePresenceCheckers = newType.getPropertyPresenceCheckers(); + Map sourcePresenceCheckers = newType.getPropertyPresenceCheckers(); for ( Map.Entry getter : sourceReadAccessors.entrySet() ) { if ( getter.getKey().equals( entryName ) ) { @@ -297,7 +297,7 @@ public static class BuilderFromProperty { private String name; private Accessor readAccessor; - private ExecutableElementAccessor presenceChecker; + private Accessor presenceChecker; private Type type; private Parameter sourceParameter; @@ -311,7 +311,7 @@ public BuilderFromProperty readAccessor(Accessor readAccessor) { return this; } - public BuilderFromProperty presenceChecker(ExecutableElementAccessor presenceChecker) { + public BuilderFromProperty presenceChecker(Accessor presenceChecker) { this.presenceChecker = presenceChecker; return this; } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/TargetReference.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/TargetReference.java index 0edfb362d0..787d11f7da 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/TargetReference.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/TargetReference.java @@ -23,6 +23,7 @@ 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; /** * This class describes the target side of a property mapping. @@ -198,8 +199,8 @@ private List getTargetEntries(Type type, String[] entryNames) { break; } - if ( isLast || ( accessorNaming.isSetterMethod( targetWriteAccessor ) - || Executables.isFieldAccessor( targetWriteAccessor ) ) ) { + 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). @@ -228,8 +229,7 @@ private List getTargetEntries(Type type, String[] entryNames) { private Type findNextType(Type initial, Accessor targetWriteAccessor, Accessor targetReadAccessor) { Type nextType; Accessor toUse = targetWriteAccessor != null ? targetWriteAccessor : targetReadAccessor; - if ( accessorNaming.isGetterMethod( toUse ) || - Executables.isFieldAccessor( toUse ) ) { + if ( toUse.getAccessorType() == AccessorType.GETTER || toUse.getAccessorType() == AccessorType.FIELD ) { nextType = typeFactory.getReturnType( (DeclaredType) typeBasedOnMethod( initial ).getTypeMirror(), toUse diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/AccessorNamingUtils.java b/processor/src/main/java/org/mapstruct/ap/internal/util/AccessorNamingUtils.java index 3d0d11686b..6cdb83152e 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/util/AccessorNamingUtils.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/util/AccessorNamingUtils.java @@ -7,6 +7,7 @@ import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.TypeElement; +import javax.lang.model.element.VariableElement; import javax.lang.model.type.DeclaredType; import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeMirror; @@ -14,6 +15,7 @@ import javax.lang.model.util.SimpleTypeVisitor6; import org.mapstruct.ap.internal.util.accessor.Accessor; +import org.mapstruct.ap.internal.util.accessor.AccessorType; import org.mapstruct.ap.internal.util.accessor.ExecutableElementAccessor; import org.mapstruct.ap.spi.AccessorNamingStrategy; import org.mapstruct.ap.spi.MethodType; @@ -33,46 +35,42 @@ public AccessorNamingUtils(AccessorNamingStrategy accessorNamingStrategy) { this.accessorNamingStrategy = accessorNamingStrategy; } - public boolean isGetterMethod(Accessor method) { - ExecutableElement executable = method.getExecutable(); - return executable != null && isPublicNotStatic( method ) && + public boolean isGetterMethod(ExecutableElement executable) { + return executable != null && isPublicNotStatic( executable ) && executable.getParameters().isEmpty() && accessorNamingStrategy.getMethodType( executable ) == MethodType.GETTER; } - public boolean isPresenceCheckMethod(Accessor method) { - if ( !( method instanceof ExecutableElementAccessor ) ) { - return false; - } - ExecutableElement executable = method.getExecutable(); + public boolean isPresenceCheckMethod(ExecutableElement executable) { + return executable != null - && isPublicNotStatic( method ) + && isPublicNotStatic( executable ) && executable.getParameters().isEmpty() && ( executable.getReturnType().getKind() == TypeKind.BOOLEAN || "java.lang.Boolean".equals( getQualifiedName( executable.getReturnType() ) ) ) && accessorNamingStrategy.getMethodType( executable ) == MethodType.PRESENCE_CHECKER; } - public boolean isSetterMethod(Accessor method) { - ExecutableElement executable = method.getExecutable(); + public boolean isSetterMethod(ExecutableElement executable) { return executable != null - && isPublicNotStatic( method ) + && isPublicNotStatic( executable ) && executable.getParameters().size() == 1 && accessorNamingStrategy.getMethodType( executable ) == MethodType.SETTER; } - public boolean isAdderMethod(Accessor method) { - ExecutableElement executable = method.getExecutable(); + public boolean isAdderMethod(ExecutableElement executable) { return executable != null - && isPublicNotStatic( method ) + && isPublicNotStatic( executable ) && executable.getParameters().size() == 1 && accessorNamingStrategy.getMethodType( executable ) == MethodType.ADDER; } - public String getPropertyName(Accessor accessor) { - ExecutableElement executable = accessor.getExecutable(); - return executable != null ? accessorNamingStrategy.getPropertyName( executable ) : - accessor.getSimpleName().toString(); + public String getPropertyName(ExecutableElement executable) { + return accessorNamingStrategy.getPropertyName( executable ); + } + + public String getPropertyName(VariableElement variable) { + return variable.getSimpleName().toString(); } /** @@ -82,8 +80,12 @@ public String getPropertyName(Accessor accessor) { * {@code addChild(Child v)}, the element name would be 'Child'. */ public String getElementNameForAdder(Accessor adderMethod) { - ExecutableElement executable = adderMethod.getExecutable(); - return executable != null ? accessorNamingStrategy.getElementName( executable ) : null; + if ( adderMethod.getAccessorType() == AccessorType.ADDER ) { + return accessorNamingStrategy.getElementName( (ExecutableElement) adderMethod.getElement() ); + } + else { + return null; + } } private static String getQualifiedName(TypeMirror type) { diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/Executables.java b/processor/src/main/java/org/mapstruct/ap/internal/util/Executables.java index 6c16b6a8a0..3d01138f72 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/util/Executables.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/util/Executables.java @@ -5,20 +5,14 @@ */ package org.mapstruct.ap.internal.util; -import static javax.lang.model.util.ElementFilter.fieldsIn; -import static javax.lang.model.util.ElementFilter.methodsIn; -import static org.mapstruct.ap.internal.util.workarounds.SpecificCompilerWorkarounds.replaceTypeElementIfNecessary; - import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.List; import java.util.ListIterator; - import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.Modifier; import javax.lang.model.element.TypeElement; -import javax.lang.model.element.VariableElement; import javax.lang.model.type.DeclaredType; import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeMirror; @@ -27,10 +21,11 @@ import org.mapstruct.ap.internal.prism.AfterMappingPrism; import org.mapstruct.ap.internal.prism.BeforeMappingPrism; import org.mapstruct.ap.internal.util.accessor.Accessor; -import org.mapstruct.ap.internal.util.accessor.ExecutableElementAccessor; -import org.mapstruct.ap.internal.util.accessor.VariableElementAccessor; import org.mapstruct.ap.spi.TypeHierarchyErroneousException; +import static javax.lang.model.util.ElementFilter.methodsIn; +import static org.mapstruct.ap.internal.util.workarounds.SpecificCompilerWorkarounds.replaceTypeElementIfNecessary; + /** * Provides functionality around {@link ExecutableElement}s. * @@ -54,29 +49,16 @@ public class Executables { private Executables() { } - /** - * An {@link Accessor} is a field accessor, if it doesn't have an executable element, is public and it is not - * static. - * - * @param accessor the accessor to ber checked - * - * @return {@code true} if the {@code accessor} is for a {@code public} non {@code static} field. - */ - public static boolean isFieldAccessor(Accessor accessor) { - ExecutableElement executable = accessor.getExecutable(); - return executable == null && isPublic( accessor ) && isNotStatic( accessor ); + static boolean isPublicNotStatic(ExecutableElement method) { + return isPublic( method ) && isNotStatic( method ); } - static boolean isPublicNotStatic(Accessor accessor) { - return isPublic( accessor ) && isNotStatic( accessor ); - } - - static boolean isPublic(Accessor method) { + static boolean isPublic(ExecutableElement method) { return method.getModifiers().contains( Modifier.PUBLIC ); } - private static boolean isNotStatic(Accessor accessor) { - return !accessor.getModifiers().contains( Modifier.STATIC ); + private static boolean isNotStatic(ExecutableElement method) { + return !method.getModifiers().contains( Modifier.STATIC ); } public static boolean isFinal(Accessor accessor) { @@ -112,34 +94,14 @@ private static TypeElement asTypeElement(TypeMirror mirror) { * @return the executable elements usable in the type */ public static List getAllEnclosedExecutableElements(Elements elementUtils, TypeElement element) { - List executables = new ArrayList<>(); - for ( Accessor accessor : getAllEnclosedAccessors( elementUtils, element ) ) { - if ( accessor.getExecutable() != null ) { - executables.add( accessor.getExecutable() ); - } - } - return executables; - } - - /** - * Finds all executable elements/variable elements within the given type element, including executable/variable - * elements defined in super classes and implemented interfaces and including the fields in the . Methods defined - * in {@link java.lang.Object} are ignored, as well as implementations of {@link java.lang.Object#equals(Object)}. - * - * @param elementUtils element helper - * @param element the element to inspect - * - * @return the executable elements usable in the type - */ - public static List getAllEnclosedAccessors(Elements elementUtils, TypeElement element) { - List enclosedElements = new ArrayList<>(); + List enclosedElements = new ArrayList<>(); element = replaceTypeElementIfNecessary( elementUtils, element ); addEnclosedElementsInHierarchy( elementUtils, enclosedElements, element, element ); return enclosedElements; } - private static void addEnclosedElementsInHierarchy(Elements elementUtils, List alreadyAdded, + private static void addEnclosedElementsInHierarchy(Elements elementUtils, List alreadyAdded, TypeElement element, TypeElement parentType) { if ( element != parentType ) { // otherwise the element was already checked for replacement element = replaceTypeElementIfNecessary( elementUtils, element ); @@ -150,23 +112,22 @@ private static void addEnclosedElementsInHierarchy(Elements elementUtils, List alreadyCollected, + private static void addNotYetOverridden(Elements elementUtils, List alreadyCollected, List methodsToAdd, TypeElement parentType) { - List safeToAdd = new ArrayList<>( methodsToAdd.size() ); + List safeToAdd = new ArrayList<>( methodsToAdd.size() ); for ( ExecutableElement toAdd : methodsToAdd ) { if ( isNotObjectEquals( toAdd ) - && wasNotYetOverridden( elementUtils, alreadyCollected, toAdd, parentType ) ) { - safeToAdd.add( new ExecutableElementAccessor( toAdd ) ); + && wasNotYetOverridden( elementUtils, alreadyCollected, toAdd, parentType ) ) { + safeToAdd.add( toAdd ); } } alreadyCollected.addAll( 0, safeToAdd ); } - private static void addFields(List alreadyCollected, List variablesToAdd) { - List safeToAdd = new ArrayList<>( variablesToAdd.size() ); - for ( VariableElement toAdd : variablesToAdd ) { - safeToAdd.add( new VariableElementAccessor( toAdd ) ); - } - - alreadyCollected.addAll( 0, safeToAdd ); - } - - /** * @param executable the executable to check * @@ -209,8 +160,8 @@ private static void addFields(List alreadyCollected, List alreadyCollected, + private static boolean wasNotYetOverridden(Elements elementUtils, List alreadyCollected, ExecutableElement executable, TypeElement parentType) { - for ( ListIterator it = alreadyCollected.listIterator(); it.hasNext(); ) { - ExecutableElement executableInSubtype = it.next().getExecutable(); + for ( ListIterator it = alreadyCollected.listIterator(); it.hasNext(); ) { + ExecutableElement executableInSubtype = it.next(); if ( executableInSubtype == null ) { continue; } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/Fields.java b/processor/src/main/java/org/mapstruct/ap/internal/util/Fields.java new file mode 100644 index 0000000000..0804f1735b --- /dev/null +++ b/processor/src/main/java/org/mapstruct/ap/internal/util/Fields.java @@ -0,0 +1,107 @@ +/* + * 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.util; + +import java.util.ArrayList; +import java.util.List; +import javax.lang.model.element.ExecutableElement; +import javax.lang.model.element.Modifier; +import javax.lang.model.element.TypeElement; +import javax.lang.model.element.VariableElement; +import javax.lang.model.type.DeclaredType; +import javax.lang.model.type.TypeKind; +import javax.lang.model.type.TypeMirror; +import javax.lang.model.util.Elements; + +import org.mapstruct.ap.spi.TypeHierarchyErroneousException; + +import static javax.lang.model.util.ElementFilter.fieldsIn; +import static org.mapstruct.ap.internal.util.workarounds.SpecificCompilerWorkarounds.replaceTypeElementIfNecessary; + +/** + * Provides functionality around {@link VariableElement}s. + * + * @author Sjaak Derksen + */ +public class Fields { + + private Fields() { + } + + public static boolean isFieldAccessor(VariableElement method) { + return isPublic( method ) && isNotStatic( method ); + } + + static boolean isPublic(VariableElement method) { + return method.getModifiers().contains( Modifier.PUBLIC ); + } + + private static boolean isNotStatic(VariableElement method) { + return !method.getModifiers().contains( Modifier.STATIC ); + } + + /** + * Finds all variable elements within the given type element, including variable + * elements defined in super classes and implemented interfaces and including the fields in the . + * + * @param elementUtils element helper + * @param element the element to inspect + * + * @return the executable elements usable in the type + */ + public static List getAllEnclosedFields(Elements elementUtils, TypeElement element) { + List enclosedElements = new ArrayList<>(); + element = replaceTypeElementIfNecessary( elementUtils, element ); + addEnclosedElementsInHierarchy( elementUtils, enclosedElements, element, element ); + + return enclosedElements; + } + + private static void addEnclosedElementsInHierarchy(Elements elementUtils, List alreadyAdded, + TypeElement element, TypeElement parentType) { + if ( element != parentType ) { // otherwise the element was already checked for replacement + element = replaceTypeElementIfNecessary( elementUtils, element ); + } + + if ( element.asType().getKind() == TypeKind.ERROR ) { + throw new TypeHierarchyErroneousException( element ); + } + + addFields( alreadyAdded, fieldsIn( element.getEnclosedElements() ) ); + + + if ( hasNonObjectSuperclass( element ) ) { + addEnclosedElementsInHierarchy( + elementUtils, + alreadyAdded, + asTypeElement( element.getSuperclass() ), + parentType + ); + } + } + + private static void addFields(List alreadyCollected, List variablesToAdd) { + List safeToAdd = new ArrayList<>( variablesToAdd.size() ); + for ( VariableElement toAdd : variablesToAdd ) { + safeToAdd.add( toAdd ); + } + + alreadyCollected.addAll( 0, safeToAdd ); + } + + private static TypeElement asTypeElement(TypeMirror mirror) { + return (TypeElement) ( (DeclaredType) mirror ).asElement(); + } + + private static boolean hasNonObjectSuperclass(TypeElement element) { + if ( element.getSuperclass().getKind() == TypeKind.ERROR ) { + throw new TypeHierarchyErroneousException( element ); + } + + return element.getSuperclass().getKind() == TypeKind.DECLARED + && !asTypeElement( element.getSuperclass() ).getQualifiedName().toString().equals( "java.lang.Object" ); + } +} diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/Filters.java b/processor/src/main/java/org/mapstruct/ap/internal/util/Filters.java index 2b784785a5..ec69b7131a 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/util/Filters.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/util/Filters.java @@ -7,11 +7,23 @@ import java.util.LinkedList; import java.util.List; - import javax.lang.model.element.Element; +import javax.lang.model.element.ExecutableElement; +import javax.lang.model.element.VariableElement; +import javax.lang.model.type.DeclaredType; +import javax.lang.model.type.ExecutableType; +import javax.lang.model.type.TypeMirror; +import javax.lang.model.util.Types; import org.mapstruct.ap.internal.util.accessor.Accessor; import org.mapstruct.ap.internal.util.accessor.ExecutableElementAccessor; +import org.mapstruct.ap.internal.util.accessor.VariableElementAccessor; + +import static org.mapstruct.ap.internal.util.Collections.first; +import static org.mapstruct.ap.internal.util.accessor.AccessorType.ADDER; +import static org.mapstruct.ap.internal.util.accessor.AccessorType.GETTER; +import static org.mapstruct.ap.internal.util.accessor.AccessorType.PRESENCE_CHECKER; +import static org.mapstruct.ap.internal.util.accessor.AccessorType.SETTER; /** * Filter methods for working with {@link Element} collections. @@ -20,66 +32,88 @@ */ public class Filters { - private Filters() { + private final AccessorNamingUtils accessorNaming; + private final Types typeUtils; + private final TypeMirror typeMirror; + + public Filters(AccessorNamingUtils accessorNaming, Types typeUtils, TypeMirror typeMirror) { + this.accessorNaming = accessorNaming; + this.typeUtils = typeUtils; + this.typeMirror = typeMirror; } - public static List getterMethodsIn(AccessorNamingUtils accessorNaming, List elements) { + public List getterMethodsIn(List elements) { List getterMethods = new LinkedList<>(); - for ( Accessor method : elements ) { + for ( ExecutableElement method : elements ) { if ( accessorNaming.isGetterMethod( method ) ) { - getterMethods.add( method ); + getterMethods.add( new ExecutableElementAccessor( method, getReturnType( method ), GETTER ) ); } } return getterMethods; } - public static List fieldsIn(List accessors) { + public List fieldsIn(List accessors) { List fieldAccessors = new LinkedList<>(); - for ( Accessor accessor : accessors ) { - if ( Executables.isFieldAccessor( accessor ) ) { - fieldAccessors.add( accessor ); + for ( VariableElement accessor : accessors ) { + if ( Fields.isFieldAccessor( accessor ) ) { + fieldAccessors.add( new VariableElementAccessor( accessor ) ); } } return fieldAccessors; } - public static List presenceCheckMethodsIn(AccessorNamingUtils accessorNaming, - List elements) { - List presenceCheckMethods = new LinkedList<>(); + public List presenceCheckMethodsIn(List elements) { + List presenceCheckMethods = new LinkedList<>(); - for ( Accessor method : elements ) { + for ( ExecutableElement method : elements ) { if ( accessorNaming.isPresenceCheckMethod( method ) ) { - presenceCheckMethods.add( (ExecutableElementAccessor) method ); + presenceCheckMethods.add( new ExecutableElementAccessor( + method, + getReturnType( method ), + PRESENCE_CHECKER + ) ); } } return presenceCheckMethods; } - public static List setterMethodsIn(AccessorNamingUtils accessorNaming, List elements) { + public List setterMethodsIn(List elements) { List setterMethods = new LinkedList<>(); - for ( Accessor method : elements ) { + for ( ExecutableElement method : elements ) { if ( accessorNaming.isSetterMethod( method ) ) { - setterMethods.add( method ); + setterMethods.add( new ExecutableElementAccessor( method, getFirstParameter( method ), SETTER ) ); } } return setterMethods; } - public static List adderMethodsIn(AccessorNamingUtils accessorNaming, List elements) { + public List adderMethodsIn( List elements) { List adderMethods = new LinkedList<>(); - for ( Accessor method : elements ) { + for ( ExecutableElement method : elements ) { if ( accessorNaming.isAdderMethod( method ) ) { - adderMethods.add( method ); + adderMethods.add( new ExecutableElementAccessor( method, getFirstParameter( method ), ADDER ) ); } } return adderMethods; } + + private TypeMirror getReturnType(ExecutableElement executableElement) { + return getWithinContext( executableElement ).getReturnType(); + } + + private TypeMirror getFirstParameter(ExecutableElement executableElement) { + return first( getWithinContext( executableElement ).getParameterTypes() ); + } + + private ExecutableType getWithinContext( ExecutableElement executableElement ) { + return (ExecutableType) typeUtils.asMemberOf( (DeclaredType) typeMirror, executableElement ); + } } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/ValueProvider.java b/processor/src/main/java/org/mapstruct/ap/internal/util/ValueProvider.java index 28c706bef0..31a3a9c6cd 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/util/ValueProvider.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/util/ValueProvider.java @@ -6,6 +6,7 @@ package org.mapstruct.ap.internal.util; import org.mapstruct.ap.internal.util.accessor.Accessor; +import org.mapstruct.ap.internal.util.accessor.AccessorType; /** * This a wrapper class which provides the value that needs to be used in the models. @@ -45,7 +46,7 @@ public static ValueProvider of(Accessor accessor) { return null; } String value = accessor.getSimpleName().toString(); - if ( accessor.getExecutable() != null ) { + if ( accessor.getAccessorType() != AccessorType.FIELD ) { value += "()"; } return new ValueProvider( value ); diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/AbstractAccessor.java b/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/AbstractAccessor.java index b5b671c2dd..5a16e45fa8 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/AbstractAccessor.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/AbstractAccessor.java @@ -38,4 +38,5 @@ public Set getModifiers() { public T getElement() { return element; } + } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/Accessor.java b/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/Accessor.java index 9fb0969807..f9dd435d5f 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/Accessor.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/Accessor.java @@ -8,6 +8,7 @@ import java.util.Set; import javax.lang.model.element.Element; import javax.lang.model.element.ExecutableElement; +import javax.lang.model.element.VariableElement; import javax.lang.model.element.Modifier; import javax.lang.model.element.Name; import javax.lang.model.type.TypeMirror; @@ -23,7 +24,7 @@ public interface Accessor { * This returns the type that this accessor gives as a return. * * e.g. The {@link ExecutableElement#getReturnType()} if this is a method accessor, - * or {@link javax.lang.model.element.VariableElement#asType()} for field accessors. + * or {@link VariableElement#asType()} for field accessors. * * @return the type that the accessor gives as a return */ @@ -40,12 +41,14 @@ public interface Accessor { Set getModifiers(); /** - * @return the {@link ExecutableElement}, or {@code null} if the accessor does not have one + * @return the underlying {@link Element}, {@link VariableElement} or {@link ExecutableElement} */ - ExecutableElement getExecutable(); + Element getElement(); /** - * @return the underlying {@link Element} + * The accessor type + * + * @return */ - Element getElement(); + AccessorType getAccessorType(); } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/AccessorType.java b/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/AccessorType.java new file mode 100644 index 0000000000..9348de878c --- /dev/null +++ b/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/AccessorType.java @@ -0,0 +1,15 @@ +/* + * 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.util.accessor; + +public enum AccessorType { + + FIELD, + GETTER, + SETTER, + ADDER, + PRESENCE_CHECKER; +} diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/ExecutableElementAccessor.java b/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/ExecutableElementAccessor.java index effd137b04..af37acbce1 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/ExecutableElementAccessor.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/ExecutableElementAccessor.java @@ -15,22 +15,27 @@ */ public class ExecutableElementAccessor extends AbstractAccessor { - public ExecutableElementAccessor(ExecutableElement element) { + private final TypeMirror accessedType; + private final AccessorType accessorType; + + public ExecutableElementAccessor(ExecutableElement element, TypeMirror accessedType, AccessorType accessorType) { super( element ); + this.accessedType = accessedType; + this.accessorType = accessorType; } @Override public TypeMirror getAccessedType() { - return element.getReturnType(); + return accessedType; } @Override - public ExecutableElement getExecutable() { - return element; + public String toString() { + return element.toString(); } @Override - public String toString() { - return element.toString(); + public AccessorType getAccessorType() { + return accessorType; } } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/VariableElementAccessor.java b/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/VariableElementAccessor.java index cdbdf1f09f..0b82fdd42f 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/VariableElementAccessor.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/VariableElementAccessor.java @@ -26,12 +26,13 @@ public TypeMirror getAccessedType() { } @Override - public ExecutableElement getExecutable() { - return null; + public String toString() { + return element.toString(); } @Override - public String toString() { - return element.toString(); + public AccessorType getAccessorType() { + return AccessorType.FIELD; } + } From 6990b69b8cf5f9f3355263a02ba63e7eab7df628 Mon Sep 17 00:00:00 2001 From: sjaakd Date: Sat, 20 Apr 2019 17:29:53 +0200 Subject: [PATCH 2/9] #1742 move BuilderType out of Type --- .../internal/model/AbstractBaseBuilder.java | 4 +- .../model/AbstractMappingMethodBuilder.java | 2 +- .../ap/internal/model/BeanMappingMethod.java | 307 +++++++++++------- .../model/ContainerMappingMethodBuilder.java | 2 +- .../ap/internal/model/HelperMethod.java | 1 - .../model/LifecycleMethodResolver.java | 53 ++- .../ap/internal/model/MapMappingMethod.java | 2 +- .../ap/internal/model/MethodReference.java | 5 +- .../model/ObjectFactoryMethodResolver.java | 34 +- .../ap/internal/model/PropertyMapping.java | 12 +- .../ap/internal/model/common/Type.java | 4 +- .../ap/internal/model/common/TypeFactory.java | 36 +- .../internal/model/source/ForgedMethod.java | 17 +- .../internal/model/source/MappingOptions.java | 2 +- .../ap/internal/model/source/Method.java | 10 +- .../internal/model/source/PropertyEntry.java | 4 +- .../internal/model/source/SourceMethod.java | 9 +- .../model/source/SourceReference.java | 1 - .../model/source/TargetReference.java | 12 +- .../processor/MapperCreationProcessor.java | 1 + .../processor/MethodRetrievalProcessor.java | 7 +- .../ap/internal/util/AccessorNamingUtils.java | 1 - .../ap/internal/util/Executables.java | 4 +- .../mapstruct/ap/internal/util/Fields.java | 3 +- .../accessor/VariableElementAccessor.java | 1 - .../ap/internal/model/BeanMappingMethod.ftl | 4 +- .../DateFormatValidatorFactoryTest.java | 1 - .../common/DefaultConversionContextTest.java | 1 - .../multiple/MultipleBuilderMapperTest.java | 11 - .../selection/generics/ConversionTest.java | 28 +- 30 files changed, 350 insertions(+), 229 deletions(-) diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/AbstractBaseBuilder.java b/processor/src/main/java/org/mapstruct/ap/internal/model/AbstractBaseBuilder.java index 8d8750b9fd..220176c06d 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/AbstractBaseBuilder.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/AbstractBaseBuilder.java @@ -7,6 +7,7 @@ import javax.lang.model.element.AnnotationMirror; import org.mapstruct.ap.internal.model.common.Assignment; +import org.mapstruct.ap.internal.model.common.BuilderType; import org.mapstruct.ap.internal.model.common.ParameterBinding; import org.mapstruct.ap.internal.model.common.SourceRHS; import org.mapstruct.ap.internal.model.common.Type; @@ -73,7 +74,7 @@ private boolean isDisableSubMappingMethodsGeneration() { * * @return See above */ - Assignment createForgedAssignment(SourceRHS sourceRHS, ForgedMethod forgedMethod) { + Assignment createForgedAssignment(SourceRHS sourceRHS, BuilderType builderType, ForgedMethod forgedMethod) { if ( ctx.getForgedMethodsUnderCreation().containsKey( forgedMethod ) ) { return createAssignment( sourceRHS, ctx.getForgedMethodsUnderCreation().get( forgedMethod ) ); @@ -93,6 +94,7 @@ Assignment createForgedAssignment(SourceRHS sourceRHS, ForgedMethod forgedMethod else { forgedMappingMethod = new BeanMappingMethod.Builder() .forgedMethod( forgedMethod ) + .returnTypeBuilder( builderType ) .mappingContext( ctx ) .build(); } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/AbstractMappingMethodBuilder.java b/processor/src/main/java/org/mapstruct/ap/internal/model/AbstractMappingMethodBuilder.java index 759f22ca73..139ba8129d 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/AbstractMappingMethodBuilder.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/AbstractMappingMethodBuilder.java @@ -63,7 +63,7 @@ Assignment forgeMapping(SourceRHS sourceRHS, Type sourceType, Type targetType) { true ); - return createForgedAssignment( sourceRHS, forgedMethod ); + return createForgedAssignment( sourceRHS, ctx.getTypeFactory().builderTypeFor( targetType ), forgedMethod ); } private String getName(Type sourceType, Type targetType) { 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 a0e4975e7b..f8cbde9757 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 @@ -6,6 +6,9 @@ package org.mapstruct.ap.internal.model; import static org.mapstruct.ap.internal.util.Collections.first; +import static org.mapstruct.ap.internal.util.Message.BEANMAPPING_ABSTRACT; +import static org.mapstruct.ap.internal.util.Message.BEANMAPPING_NOT_ASSIGNABLE; +import static org.mapstruct.ap.internal.util.Message.GENERAL_ABSTRACT_RETURN_TYPE; import java.text.MessageFormat; import java.util.ArrayList; @@ -27,6 +30,7 @@ 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.common.BuilderType; import org.mapstruct.ap.internal.model.common.Parameter; import org.mapstruct.ap.internal.model.common.Type; import org.mapstruct.ap.internal.model.dependency.GraphAnalyzer; @@ -50,7 +54,6 @@ 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.ExecutableElementAccessor; /** * A {@link MappingMethod} implemented by a {@link Mapper} class which maps one bean type to another, optionally @@ -63,20 +66,23 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { private final List propertyMappings; private final Map> mappingsByParameter; private final List constantMappings; - private final Type resultType; + private final Type returnTypeToConstruct; + private final BuilderType returnTypeBuilder; private final MethodReference finalizerMethod; public static class Builder { private MappingBuilderContext ctx; private Method method; + + /* returnType to construct can have a builder */ + private BuilderType returnTypeBuilder; + private Map unprocessedTargetProperties; private Map unprocessedSourceProperties; private Set targetProperties; private final List propertyMappings = new ArrayList<>(); private final Set unprocessedSourceParameters = new HashSet<>(); - private NullValueMappingStrategyPrism nullValueMappingStrategy; - private SelectionParameters selectionParameters; private final Set existingVariableNames = new HashSet<>(); private Map> methodMappings; private SingleMappingByTargetPropertyNameFunction singleMapping; @@ -87,27 +93,69 @@ public Builder mappingContext(MappingBuilderContext mappingContext) { return this; } + public Builder returnTypeBuilder( BuilderType returnTypeBuilder ) { + this.returnTypeBuilder = returnTypeBuilder; + return this; + } + public Builder sourceMethod(SourceMethod sourceMethod) { singleMapping = new SourceMethodSingleMapping( sourceMethod ); - return setupMethodWithMapping( sourceMethod ); + this.method = sourceMethod; + return this; } public Builder forgedMethod(Method method) { singleMapping = new EmptySingleMapping(); - return setupMethodWithMapping( method ); + this.method = method; + return this; } - private Builder setupMethodWithMapping(Method sourceMethod) { - this.method = sourceMethod; - this.methodMappings = sourceMethod.getMappingOptions().getMappings(); - CollectionMappingStrategyPrism cms = sourceMethod.getMapperConfiguration().getCollectionMappingStrategy(); - Type mappingType = method.getResultType(); - if ( !method.isUpdateMethod() ) { - mappingType = mappingType.getEffectiveType(); + public BeanMappingMethod build() { + + BeanMapping beanMapping = method.getMappingOptions().getBeanMapping(); + SelectionParameters selectionParameters = beanMapping != null ? beanMapping.getSelectionParameters() : null; + + /* the return type that needs to be constructed (new or factorized), so for instance: */ + /* 1) the return type of a non-update method */ + /* 2) or the implementation type that needs to be used when the return type is abstract */ + /* 3) or the builder whenever the return type is immutable */ + Type returnTypeToConstruct = null; + + /* factory or builder method to construct the returnTypeToConstruct */ + MethodReference factoryMethod = null; + + // determine which return type to construct + if ( !method.getReturnType().isVoid() ) { + Type returnTypeImpl = getReturnTypeToConstructFromSelectionParameters( selectionParameters ); + if ( returnTypeImpl != null ) { + factoryMethod = getFactoryMethod( returnTypeImpl, selectionParameters ); + if ( factoryMethod != null || canBeConstructed( returnTypeImpl ) ) { + returnTypeToConstruct = returnTypeImpl; + } + else { + reportResultTypeFromBeanMappingNotConstructableError( returnTypeImpl ); + } + } + else { + returnTypeImpl = isBuilderRequired() ? returnTypeBuilder.getBuilder() : method.getReturnType(); + factoryMethod = getFactoryMethod( returnTypeImpl, selectionParameters ); + if ( factoryMethod != null || canBeConstructed( returnTypeImpl ) ) { + returnTypeToConstruct = returnTypeImpl; + } + else { + reportReturnTypeNotConstructableError( returnTypeImpl ); + } + } } - Map accessors = mappingType - .getPropertyWriteAccessors( cms ); + /* the type that needs to be used in the mapping process as target */ + Type resultTypeToMap = returnTypeToConstruct == null ? method.getResultType() : returnTypeToConstruct; + + this.methodMappings = this.method.getMappingOptions().getMappings(); + CollectionMappingStrategyPrism cms = this.method.getMapperConfiguration().getCollectionMappingStrategy(); + + // determine accessors + Map accessors = resultTypeToMap.getPropertyWriteAccessors( cms ); this.targetProperties = accessors.keySet(); this.unprocessedTargetProperties = new LinkedHashMap<>( accessors ); @@ -125,17 +173,13 @@ private Builder setupMethodWithMapping(Method sourceMethod) { } existingVariableNames.addAll( method.getParameterNames() ); - BeanMapping beanMapping = method.getMappingOptions().getBeanMapping(); + // get bean mapping (when specified as annotation ) if ( beanMapping != null ) { for ( String ignoreUnmapped : beanMapping.getIgnoreUnmappedSourceProperties() ) { unprocessedSourceProperties.remove( ignoreUnmapped ); } } - return this; - } - - public BeanMappingMethod build() { // map properties with mapping boolean mappingErrorOccured = handleDefinedMappings(); if ( mappingErrorOccured ) { @@ -158,87 +202,29 @@ public BeanMappingMethod build() { reportErrorForUnmappedTargetPropertiesIfRequired(); reportErrorForUnmappedSourcePropertiesIfRequired(); - // get bean mapping (when specified as annotation ) - BeanMapping beanMapping = method.getMappingOptions().getBeanMapping(); - BeanMappingPrism beanMappingPrism = BeanMappingPrism.getInstanceOn( method.getExecutable() ); - // mapNullToDefault NullValueMappingStrategyPrism nullValueMappingStrategy = beanMapping != null ? beanMapping.getNullValueMappingStrategy() : null; boolean mapNullToDefault = method.getMapperConfiguration().isMapToDefault( nullValueMappingStrategy ); - - // selectionParameters - SelectionParameters selectionParameters = beanMapping != null ? beanMapping.getSelectionParameters() : null; - - // check if there's a factory method for the result type - MethodReference factoryMethod = null; - if ( !method.isUpdateMethod() ) { - factoryMethod = ObjectFactoryMethodResolver.getFactoryMethod( - method, - method.getResultType(), - selectionParameters, - ctx - ); - } - - // if there's no factory method, try the resultType in the @BeanMapping - Type resultType = null; - if ( factoryMethod == null ) { - if ( selectionParameters != null && selectionParameters.getResultType() != null ) { - resultType = ctx.getTypeFactory().getType( selectionParameters.getResultType() ).getEffectiveType(); - if ( resultType.isAbstract() ) { - ctx.getMessager().printMessage( - method.getExecutable(), - beanMappingPrism.mirror, - Message.BEANMAPPING_ABSTRACT, - resultType, - method.getResultType() - ); - } - else if ( !resultType.isAssignableTo( method.getResultType() ) ) { - ctx.getMessager().printMessage( - method.getExecutable(), - beanMappingPrism.mirror, - Message.BEANMAPPING_NOT_ASSIGNABLE, resultType, method.getResultType() - ); - } - else if ( !resultType.hasEmptyAccessibleContructor() ) { - ctx.getMessager().printMessage( - method.getExecutable(), - beanMappingPrism.mirror, - Message.GENERAL_NO_SUITABLE_CONSTRUCTOR, - resultType - ); - } - } - else if ( !method.isUpdateMethod() && method.getReturnType().getEffectiveType().isAbstract() ) { - ctx.getMessager().printMessage( - method.getExecutable(), - Message.GENERAL_ABSTRACT_RETURN_TYPE, - method.getReturnType().getEffectiveType() - ); - } - else if ( !method.isUpdateMethod() && - !method.getReturnType().getEffectiveType().hasEmptyAccessibleContructor() ) { - ctx.getMessager().printMessage( - method.getExecutable(), - Message.GENERAL_NO_SUITABLE_CONSTRUCTOR, - method.getReturnType().getEffectiveType() - ); - } - } - + // sort sortPropertyMappingsByDependencies(); + // before / after mappings List beforeMappingMethods = LifecycleMethodResolver.beforeMappingMethods( - method, - selectionParameters, - ctx, - existingVariableNames + method, + resultTypeToMap, + selectionParameters, + ctx, + existingVariableNames + ); + List afterMappingMethods = LifecycleMethodResolver.afterMappingMethods( + method, + resultTypeToMap, + selectionParameters, + ctx, + existingVariableNames ); - List afterMappingMethods = - LifecycleMethodResolver.afterMappingMethods( method, selectionParameters, ctx, existingVariableNames ); if (factoryMethod != null && method instanceof ForgedMethod ) { ( (ForgedMethod) method ).addThrownTypes( factoryMethod.getThrownTypes() ); @@ -246,7 +232,7 @@ else if ( !method.isUpdateMethod() && MethodReference finalizeMethod = null; - if ( shouldCallFinalizerMethod( resultType == null ? method.getResultType() : resultType ) ) { + if ( shouldCallFinalizerMethod( returnTypeToConstruct ) ) { finalizeMethod = getFinalizerMethod(); } @@ -256,32 +242,41 @@ else if ( !method.isUpdateMethod() && propertyMappings, factoryMethod, mapNullToDefault, - resultType, + returnTypeToConstruct, + returnTypeBuilder, beforeMappingMethods, afterMappingMethods, finalizeMethod ); } - private boolean shouldCallFinalizerMethod(Type resultType) { - Type returnType = method.getReturnType(); - if ( returnType.isVoid() ) { + /** + * @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 + * builder is not assignable to the return type (so without building). + */ + private boolean isBuilderRequired() { + return returnTypeBuilder != null + && ( !method.isUpdateMethod() || !method.isMappingTargetAssignableToReturnType() ); + } + + private boolean shouldCallFinalizerMethod(Type returnTypeToConstruct ) { + if ( returnTypeToConstruct == null ) { return false; } - Type mappingType = method.isUpdateMethod() ? resultType : resultType.getEffectiveType(); - if ( mappingType.isAssignableTo( returnType ) ) { + else if ( returnTypeToConstruct.isAssignableTo( method.getReturnType() ) ) { // If the mapping type can be assigned to the return type then we // don't need a finalizer method return false; } - return returnType.getBuilderType() != null; + return returnTypeBuilder != null; } private MethodReference getFinalizerMethod() { return BuilderFinisherMethodResolver.getBuilderFinisherMethod( method, - method.getReturnType().getBuilderType(), + returnTypeBuilder, ctx ); } @@ -372,6 +367,87 @@ public int compare(PropertyMapping o1, PropertyMapping o2) { } } + private Type getReturnTypeToConstructFromSelectionParameters(SelectionParameters selectionParams) { + if ( selectionParams != null && selectionParams.getResultType() != null ) { + return ctx.getTypeFactory().getType( selectionParams.getResultType() ); + } + return null; + } + + private boolean canBeConstructed(Type typeToBeConstructed) { + return !typeToBeConstructed.isAbstract() + && typeToBeConstructed.isAssignableTo( this.method.getResultType() ) + && typeToBeConstructed.hasEmptyAccessibleContructor(); + } + + private void reportResultTypeFromBeanMappingNotConstructableError(Type resultType) { + + if ( resultType.isAbstract() ) { + ctx.getMessager().printMessage( + method.getExecutable(), + BeanMappingPrism.getInstanceOn( method.getExecutable() ).mirror, + BEANMAPPING_ABSTRACT, + resultType, + method.getResultType() + ); + } + else if ( !resultType.isAssignableTo( method.getResultType() ) ) { + ctx.getMessager().printMessage( + method.getExecutable(), + BeanMappingPrism.getInstanceOn( method.getExecutable() ).mirror, + BEANMAPPING_NOT_ASSIGNABLE, + resultType, + method.getResultType() + ); + } + else if ( !resultType.hasEmptyAccessibleContructor() ) { + ctx.getMessager().printMessage( + method.getExecutable(), + BeanMappingPrism.getInstanceOn( method.getExecutable() ).mirror, + Message.GENERAL_NO_SUITABLE_CONSTRUCTOR, + resultType + ); + } + } + + private void reportReturnTypeNotConstructableError(Type returnType) { + if ( returnType.isAbstract() ) { + ctx.getMessager().printMessage( + method.getExecutable(), + GENERAL_ABSTRACT_RETURN_TYPE, + returnType + ); + } + else if ( !returnType.hasEmptyAccessibleContructor() ) { + ctx.getMessager().printMessage( + method.getExecutable(), + Message.GENERAL_NO_SUITABLE_CONSTRUCTOR, + returnType + ); + } + } + + /** + * Find a factory method for a return type or for a builder. + * @param returnTypeImpl the return type implementation to construct + * @param selectionParameters + * @return + */ + private MethodReference getFactoryMethod(Type returnTypeImpl, SelectionParameters selectionParameters) { + MethodReference factoryMethod = ObjectFactoryMethodResolver.getFactoryMethod( method, + returnTypeImpl, + selectionParameters, + ctx + ); + if ( factoryMethod == null && returnTypeBuilder != null ) { + factoryMethod = ObjectFactoryMethodResolver.getBuilderFactoryMethod( method, returnTypeBuilder ); + } + + return factoryMethod; + } + + + /** * Iterates over all defined mapping methods ({@code @Mapping(s)}), either directly given or inherited from the * inverse mapping method. @@ -840,7 +916,8 @@ private BeanMappingMethod(Method method, List propertyMappings, MethodReference factoryMethod, boolean mapNullToDefault, - Type resultType, + Type returnTypeToConstruct, + BuilderType returnTypeBuilder, List beforeMappingReferences, List afterMappingReferences, MethodReference finalizerMethod) { @@ -854,6 +931,7 @@ private BeanMappingMethod(Method method, ); this.propertyMappings = propertyMappings; + this.returnTypeBuilder = returnTypeBuilder; this.finalizerMethod = finalizerMethod; // intialize constant mappings as all mappings, but take out the ones that can be contributed to a @@ -870,11 +948,7 @@ private BeanMappingMethod(Method method, } } } - this.resultType = resultType; - } - - public List getPropertyMappings() { - return propertyMappings; + this.returnTypeToConstruct = returnTypeToConstruct; } public List getConstantMappings() { @@ -886,14 +960,8 @@ public List propertyMappingsByParameter(Parameter parameter) { return mappingsByParameter.get( parameter.getName() ); } - @Override - public Type getResultType() { - if ( resultType == null ) { - return super.getResultType(); - } - else { - return resultType; - } + public Type getReturnTypeToConstruct() { + return returnTypeToConstruct; } public MethodReference getFinalizerMethod() { @@ -908,12 +976,11 @@ public Set getImportTypes() { types.addAll( propertyMapping.getImportTypes() ); } - if ( !isExistingInstanceMapping() ) { - types.addAll( getResultType().getEffectiveType().getImportTypes() ); + if ( returnTypeToConstruct != null ) { + types.addAll( returnTypeToConstruct.getImportTypes() ); } - - if ( getResultType().getBuilderType() != null ) { - types.add( getResultType().getBuilderType().getOwningType() ); + if ( returnTypeBuilder != null ) { + types.add( returnTypeBuilder.getOwningType() ); } return types; 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 a9a501a49e..c4dc73f611 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 @@ -139,7 +139,7 @@ public final M build() { MethodReference factoryMethod = null; if ( !method.isUpdateMethod() ) { - factoryMethod = ObjectFactoryMethodResolver.getFactoryMethod( method, method.getResultType(), null, ctx ); + factoryMethod = ObjectFactoryMethodResolver.getFactoryMethod( method, null, ctx ); } Set existingVariables = new HashSet<>( method.getParameterNames() ); diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/HelperMethod.java b/processor/src/main/java/org/mapstruct/ap/internal/model/HelperMethod.java index 72e062cca8..5921d15179 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/HelperMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/HelperMethod.java @@ -10,7 +10,6 @@ import java.util.Collections; import java.util.List; import java.util.Set; - import javax.lang.model.element.ExecutableElement; import org.mapstruct.ap.internal.model.common.Accessibility; diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/LifecycleMethodResolver.java b/processor/src/main/java/org/mapstruct/ap/internal/model/LifecycleMethodResolver.java index 64eee60517..0448be5d44 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/LifecycleMethodResolver.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/LifecycleMethodResolver.java @@ -32,16 +32,60 @@ private LifecycleMethodResolver() { /** * @param method the method to obtain the beforeMapping methods for + * @param alternativeTarget alternative to {@link Method#getResultType()} e.g. when target is abstract * @param selectionParameters method selectionParameters * @param ctx the builder context * @param existingVariableNames the existing variable names in the mapping method * @return all applicable {@code @BeforeMapping} methods for the given method */ public static List beforeMappingMethods(Method method, + Type alternativeTarget, SelectionParameters selectionParameters, MappingBuilderContext ctx, Set existingVariableNames) { return collectLifecycleCallbackMethods( method, + alternativeTarget, + selectionParameters, + filterBeforeMappingMethods( getAllAvailableMethods( method, ctx.getSourceModel() ) ), + ctx, + existingVariableNames ); + } + + /** + * @param method the method to obtain the afterMapping methods for + * @param alternativeTarget alternative to {@link Method#getResultType()} e.g. when target is abstract + * @param selectionParameters method selectionParameters + * @param ctx the builder context + * @param existingVariableNames list of already used variable names + * @return all applicable {@code @AfterMapping} methods for the given method + */ + public static List afterMappingMethods(Method method, + Type alternativeTarget, + SelectionParameters selectionParameters, + MappingBuilderContext ctx, + Set existingVariableNames) { + return collectLifecycleCallbackMethods( method, + alternativeTarget, + selectionParameters, + filterAfterMappingMethods( getAllAvailableMethods( method, ctx.getSourceModel() ) ), + ctx, + existingVariableNames ); + } + + + /** + * @param method the method to obtain the beforeMapping methods for + * @param selectionParameters method selectionParameters + * @param ctx the builder context + * @param existingVariableNames the existing variable names in the mapping method + * @return all applicable {@code @BeforeMapping} methods for the given method + */ + public static List beforeMappingMethods(Method method, + SelectionParameters selectionParameters, + MappingBuilderContext ctx, + Set existingVariableNames) { + return collectLifecycleCallbackMethods( method, + method.getResultType(), selectionParameters, filterBeforeMappingMethods( getAllAvailableMethods( method, ctx.getSourceModel() ) ), ctx, @@ -60,6 +104,7 @@ public static List afterMappingMethods(Method MappingBuilderContext ctx, Set existingVariableNames) { return collectLifecycleCallbackMethods( method, + method.getResultType(), selectionParameters, filterAfterMappingMethods( getAllAvailableMethods( method, ctx.getSourceModel() ) ), ctx, @@ -87,18 +132,12 @@ private static List getAllAvailableMethods(Method method, List collectLifecycleCallbackMethods( - Method method, SelectionParameters selectionParameters, List callbackMethods, + Method method, Type targetType, SelectionParameters selectionParameters, List callbackMethods, MappingBuilderContext ctx, Set existingVariableNames) { MethodSelectors selectors = new MethodSelectors( ctx.getTypeUtils(), ctx.getElementUtils(), ctx.getTypeFactory(), ctx.getMessager() ); - Type targetType = method.getResultType(); - - if ( !method.isUpdateMethod() ) { - targetType = targetType.getEffectiveType(); - } - List> matchingMethods = selectors.getMatchingMethods( method, callbackMethods, 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 b08fea5775..81026c5ec2 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 @@ -198,7 +198,7 @@ public MapMappingMethod build() { MethodReference factoryMethod = null; if ( !method.isUpdateMethod() ) { factoryMethod = ObjectFactoryMethodResolver - .getFactoryMethod( method, method.getResultType(), null, ctx ); + .getFactoryMethod( method, null, ctx ); } keyAssignment = new LocalVarWrapper( keyAssignment, method.getThrownTypes(), keyTargetType, false ); diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/MethodReference.java b/processor/src/main/java/org/mapstruct/ap/internal/model/MethodReference.java index b95ef372d7..81d75a80e4 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/MethodReference.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/MethodReference.java @@ -157,7 +157,7 @@ public void setAssignment( Assignment assignment ) { @Override public String getSourceReference() { - return assignment.getSourceReference(); + return assignment != null ? assignment.getSourceReference() : null; } @Override @@ -353,7 +353,8 @@ public static MethodReference forMethodCall(String methodName) { @Override public String toString() { String mapper = declaringMapper != null ? declaringMapper.getType().getName().toString() : ""; - String argument = getAssignment() != null ? getAssignment().toString() : getSourceReference(); + String argument = getAssignment() != null ? getAssignment().toString() : + ( getSourceReference() != null ? getSourceReference() : "" ); String returnTypeAsString = returnType != null ? returnType.toString() : ""; List arguments = sourceParameters.stream() .map( p -> p.isMappingContext() || p.isMappingTarget() || p.isTargetType() ? p.getName() : argument ) diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/ObjectFactoryMethodResolver.java b/processor/src/main/java/org/mapstruct/ap/internal/model/ObjectFactoryMethodResolver.java index 17d5135b00..5a0f72fb50 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/ObjectFactoryMethodResolver.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/ObjectFactoryMethodResolver.java @@ -38,7 +38,6 @@ private ObjectFactoryMethodResolver() { * returns a no arg factory method * * @param method target mapping method - * @param targetType return type to match * @param selectionParameters parameters used in the selection process * @param ctx * @@ -46,9 +45,29 @@ private ObjectFactoryMethodResolver() { * */ public static MethodReference getFactoryMethod( Method method, - Type targetType, SelectionParameters selectionParameters, MappingBuilderContext ctx) { + return getFactoryMethod( method, method.getResultType(), selectionParameters, ctx ); + } + + + + /** + * returns a no arg factory method + * + * @param method target mapping method + * @param alternativeTarget alternative to {@link Method#getResultType()} e.g. when target is abstract + * @param selectionParameters parameters used in the selection process + * @param ctx + * + * @return a method reference to the factory method, or null if no suitable, or ambiguous method found + * + */ + public static MethodReference getFactoryMethod( Method method, + Type alternativeTarget, + SelectionParameters selectionParameters, + MappingBuilderContext ctx) { + MethodSelectors selectors = new MethodSelectors( ctx.getTypeUtils(), ctx.getElementUtils(), ctx.getTypeFactory(), ctx.getMessager() ); @@ -58,18 +77,18 @@ public static MethodReference getFactoryMethod( Method method, method, getAllAvailableMethods( method, ctx.getSourceModel() ), java.util.Collections. emptyList(), - targetType.getEffectiveType(), + alternativeTarget, SelectionCriteria.forFactoryMethods( selectionParameters ) ); if (matchingFactoryMethods.isEmpty()) { - return findBuilderFactoryMethod( targetType ); + return null; } if ( matchingFactoryMethods.size() > 1 ) { ctx.getMessager().printMessage( method.getExecutable(), Message.GENERAL_AMBIGIOUS_FACTORY_METHOD, - targetType.getEffectiveType(), + alternativeTarget, Strings.join( matchingFactoryMethods, ", " ) ); return null; @@ -98,8 +117,7 @@ java.util.Collections. emptyList(), } } - private static MethodReference findBuilderFactoryMethod(Type targetType) { - BuilderType builder = targetType.getBuilderType(); + public static MethodReference getBuilderFactoryMethod(Method method, BuilderType builder ) { if ( builder == null ) { return null; } @@ -110,7 +128,7 @@ private static MethodReference findBuilderFactoryMethod(Type targetType) { return null; } - if ( !builder.getBuildingType().isAssignableTo( targetType ) ) { + if ( !builder.getBuildingType().isAssignableTo( method.getReturnType() ) ) { //TODO print error message return null; } 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 57cb84bb80..a2ba31752a 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 @@ -12,7 +12,6 @@ import java.util.Set; import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.ExecutableElement; -import javax.lang.model.type.DeclaredType; import org.mapstruct.ap.internal.model.assignment.AdderWrapper; import org.mapstruct.ap.internal.model.assignment.ArrayCopyWrapper; @@ -41,8 +40,6 @@ import org.mapstruct.ap.internal.prism.NullValueCheckStrategyPrism; import org.mapstruct.ap.internal.prism.NullValueMappingStrategyPrism; import org.mapstruct.ap.internal.prism.NullValuePropertyMappingStrategyPrism; -import org.mapstruct.ap.internal.util.AccessorNamingUtils; -import org.mapstruct.ap.internal.util.Executables; import org.mapstruct.ap.internal.util.MapperConfiguration; import org.mapstruct.ap.internal.util.Message; import org.mapstruct.ap.internal.util.NativeTypes; @@ -75,7 +72,6 @@ public class PropertyMapping extends ModelElement { private final List dependsOn; private final Assignment defaultValueAssignment; - @SuppressWarnings("unchecked") private static class MappingBuilderBase> extends AbstractBaseBuilder { @@ -115,9 +111,9 @@ public T targetReadAccessor(Accessor targetReadAccessor) { public T targetWriteAccessor(Accessor targetWriteAccessor) { this.targetWriteAccessor = targetWriteAccessor; + this.targetType = ctx.getTypeFactory().getType( targetWriteAccessor.getAccessedType() ); + this.targetBuilderType = ctx.getTypeFactory().builderTypeFor( this.targetType ); this.targetWriteAccessorType = targetWriteAccessor.getAccessorType(); - this.targetType = determineTargetType(); - return (T) this; } @@ -126,10 +122,6 @@ T mirror(AnnotationMirror mirror) { return (T) this; } - private Type determineTargetType() { - return ctx.getTypeFactory().getType( targetWriteAccessor.getAccessedType() ); - } - public T targetPropertyName(String targetPropertyName) { this.targetPropertyName = targetPropertyName; return (T) this; diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java b/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java index 1b8f233106..bc0170645b 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java @@ -13,7 +13,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; import java.util.stream.Stream; import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.Element; @@ -93,6 +92,7 @@ public class Type extends ModelElement implements Comparable { private List allMethods = null; private List allFields = null; + private List setters = null; private List adders = null; private List alternativeTargetAccessors = null; @@ -501,7 +501,7 @@ public Map getPropertyReadAccessors() { // In the DefaultAccessorNamingStrategy, this can only be the case for Booleans: isFoo() and // getFoo(); The latter is preferred. if ( !getter.getSimpleName().toString().startsWith( "is" ) ) { - modifiableGetters.put( getPropertyName( getter ),getter ); + modifiableGetters.put( getPropertyName( getter ), getter ); } } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/common/TypeFactory.java b/processor/src/main/java/org/mapstruct/ap/internal/model/common/TypeFactory.java index 003582777a..8f3da0b915 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/common/TypeFactory.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/common/TypeFactory.java @@ -187,7 +187,6 @@ private Type getType(TypeMirror mirror, boolean isLiteral) { } ImplementationType implementationType = getImplementationType( mirror ); - BuilderInfo builderInfo = findBuilder( mirror ); boolean isIterableType = typeUtils.isSubtype( mirror, iterableType ); boolean isCollectionType = typeUtils.isSubtype( mirror, collectionType ); @@ -281,7 +280,6 @@ else if (componentTypeMirror.getKind().isPrimitive()) { getTypeParameters( mirror, false ), implementationType, componentType, - builderInfo, packageName, name, qualifiedName, @@ -503,7 +501,6 @@ private ImplementationType getImplementationType(TypeMirror mirror) { getTypeParameters( mirror, true ), null, null, - null, implementationType.getPackageName(), implementationType.getName(), implementationType.getFullyQualifiedName(), @@ -524,19 +521,21 @@ private ImplementationType getImplementationType(TypeMirror mirror) { return null; } - private BuilderInfo findBuilder(TypeMirror type) { + private BuilderInfo findBuilder(TypeMirror type, boolean report) { try { return roundContext.getAnnotationProcessorContext() .getBuilderProvider() .findBuilderInfo( type ); } catch ( MoreThanOneBuilderCreationMethodException ex ) { - messager.printMessage( - typeUtils.asElement( type ), - Message.BUILDER_MORE_THAN_ONE_BUILDER_CREATION_METHOD, - type, - Strings.join( ex.getBuilderInfo(), ", ", BUILDER_INFO_CREATION_METHOD_EXTRACTOR ) - ); + if ( report ) { + messager.printMessage( + typeUtils.asElement( type ), + Message.BUILDER_MORE_THAN_ONE_BUILDER_CREATION_METHOD, + type, + Strings.join( ex.getBuilderInfo(), ", ", BUILDER_INFO_CREATION_METHOD_EXTRACTOR ) + ); + } } return null; @@ -664,4 +663,21 @@ private boolean canBeProcessed(TypeMirror type) { return true; } + + public BuilderType builderTypeFor( Type type ) { + if ( type != null ) { + BuilderInfo builderInfo = findBuilder( type.getTypeMirror(), true ); + return BuilderType.create( builderInfo, type, this, this.typeUtils ); + } + return null; + } + + public Type effectiveResultTypeFor( Type type ) { + if ( type != null ) { + BuilderInfo builderInfo = findBuilder( type.getTypeMirror(), false ); + BuilderType builderType = BuilderType.create( builderInfo, type, this, this.typeUtils ); + return builderType != null ? builderType.getBuilder() : type; + } + return type; + } } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/ForgedMethod.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/ForgedMethod.java index 4aeb2b5d59..01dbdedb1b 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/ForgedMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/ForgedMethod.java @@ -8,7 +8,6 @@ import java.util.ArrayList; import java.util.Iterator; import java.util.List; - import javax.lang.model.element.ExecutableElement; import org.mapstruct.ap.internal.model.common.Accessibility; @@ -51,9 +50,9 @@ public class ForgedMethod implements Method { * @param parameterProvidedMethods additional factory/lifecycle methods to consider that are provided by context * parameters */ - public ForgedMethod(String name, Type sourceType, Type returnType, MapperConfiguration mapperConfiguration, - ExecutableElement positionHintElement, List additionalParameters, - ParameterProvidedMethods parameterProvidedMethods) { + public ForgedMethod(String name, Type sourceType, Type returnType, + MapperConfiguration mapperConfiguration, ExecutableElement positionHintElement, + List additionalParameters, ParameterProvidedMethods parameterProvidedMethods) { this( name, sourceType, @@ -82,11 +81,11 @@ public ForgedMethod(String name, Type sourceType, Type returnType, MapperConfigu * @param mappingOptions the mapping options for this method * @param forgedNameBased forges a name based (matched) mapping method */ - public ForgedMethod(String name, Type sourceType, Type returnType, MapperConfiguration mapperConfiguration, - ExecutableElement positionHintElement, List additionalParameters, - ParameterProvidedMethods parameterProvidedMethods, ForgedMethodHistory history, - MappingOptions mappingOptions, boolean forgedNameBased) { - String sourceParamName = Strings.decapitalize( sourceType.getName() ); + public ForgedMethod(String name, Type sourceType, Type returnType, MapperConfiguration mapperConfiguration, + ExecutableElement positionHintElement, List additionalParameters, + ParameterProvidedMethods parameterProvidedMethods, ForgedMethodHistory history, + MappingOptions mappingOptions, boolean forgedNameBased) { + String sourceParamName = Strings.decapitalize( sourceType.getName() ); String sourceParamSafeName = Strings.getSafeVariableName( sourceParamName ); this.parameters = new ArrayList<>( 1 + additionalParameters.size() ); 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 3d5bf302e1..b89192fedd 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 @@ -291,7 +291,7 @@ public void applyIgnoreAll(MappingOptions inherited, SourceMethod method, Format CollectionMappingStrategyPrism cms = method.getMapperConfiguration().getCollectionMappingStrategy(); Type writeType = method.getResultType(); if ( !method.isUpdateMethod() ) { - writeType = writeType.getEffectiveType(); + writeType = typeFactory.effectiveResultTypeFor( writeType ); } Map writeAccessors = writeType.getPropertyWriteAccessors( cms ); List mappedPropertyNames = new ArrayList<>(); diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/Method.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/Method.java index 46810e0d4d..98f27b8334 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/Method.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/Method.java @@ -129,7 +129,6 @@ public interface Method { */ Type getResultType(); - /** * * @return the names of the parameters of this mapping method @@ -187,4 +186,13 @@ public interface Method { * @return the mapping options for this method */ MappingOptions getMappingOptions(); + + /** + * + * @return true when @MappingTarget annotated parameter is the same type as the return type. The method has + * to be an update method in order for this to be true. + */ + default boolean isMappingTargetAssignableToReturnType() { + return isUpdateMethod() ? getResultType().isAssignableTo( getReturnType() ) : false; + } } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/PropertyEntry.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/PropertyEntry.java index e6212db685..1eb4bc80b2 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/PropertyEntry.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/PropertyEntry.java @@ -11,8 +11,6 @@ import org.mapstruct.ap.internal.model.common.Type; import org.mapstruct.ap.internal.util.Strings; import org.mapstruct.ap.internal.util.accessor.Accessor; -import org.mapstruct.ap.internal.util.accessor.ExecutableElementAccessor; - /** * A PropertyEntry contains information on the name, readAccessor (for source), readAccessor and writeAccessor @@ -57,7 +55,7 @@ private PropertyEntry(String[] fullName, Accessor readAccessor, Accessor writeAc */ public static PropertyEntry forTargetReference(String[] fullName, Accessor readAccessor, Accessor writeAccessor, Type type, BuilderType builderType ) { - return new PropertyEntry( fullName, readAccessor, writeAccessor, null, type, null ); + return new PropertyEntry( fullName, readAccessor, writeAccessor, null, type, builderType ); } /** diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/SourceMethod.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/SourceMethod.java index efe38231d0..87302dce3c 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/SourceMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/SourceMethod.java @@ -5,20 +5,18 @@ */ package org.mapstruct.ap.internal.model.source; -import static org.mapstruct.ap.internal.util.Collections.first; - import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; - import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.Modifier; import javax.lang.model.util.Types; import org.mapstruct.ap.internal.model.common.Accessibility; +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; @@ -29,6 +27,8 @@ import org.mapstruct.ap.internal.util.MapperConfiguration; import org.mapstruct.ap.internal.util.Strings; +import static org.mapstruct.ap.internal.util.Collections.first; + /** * Represents a mapping method with source and target type and the mappings between the properties of source and target * type. @@ -50,6 +50,7 @@ public class SourceMethod implements Method { private final Parameter targetTypeParameter; private final boolean isObjectFactory; private final Type returnType; + private final BuilderType builderType; private final Accessibility accessibility; private final List exceptionTypes; private final MapperConfiguration config; @@ -80,6 +81,7 @@ public static class Builder { private ExecutableElement executable; private List parameters; private Type returnType = null; + private BuilderType builderType = null; private List exceptionTypes; private Map> mappings; private IterableMapping iterableMapping = null; @@ -207,6 +209,7 @@ private SourceMethod(Builder builder, MappingOptions mappingOptions) { this.executable = builder.executable; this.parameters = builder.parameters; this.returnType = builder.returnType; + this.builderType = builder.builderType; this.exceptionTypes = builder.exceptionTypes; this.accessibility = Accessibility.fromModifiers( builder.executable.getModifiers() ); diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/SourceReference.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/SourceReference.java index 9688c2f0b7..cd76dfff26 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/SourceReference.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/SourceReference.java @@ -24,7 +24,6 @@ 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.ExecutableElementAccessor; /** * This class describes the source side of a property mapping. diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/TargetReference.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/TargetReference.java index 787d11f7da..1d6e6d3f90 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/TargetReference.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/TargetReference.java @@ -13,12 +13,12 @@ import javax.lang.model.element.AnnotationMirror; 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.prism.CollectionMappingStrategyPrism; import org.mapstruct.ap.internal.util.AccessorNamingUtils; -import org.mapstruct.ap.internal.util.Executables; import org.mapstruct.ap.internal.util.FormattingMessager; import org.mapstruct.ap.internal.util.Message; import org.mapstruct.ap.internal.util.Strings; @@ -147,8 +147,7 @@ public TargetReference build() { Parameter parameter = method.getMappingTargetParameter(); boolean foundEntryMatch; - Type resultType = method.getResultType(); - resultType = typeBasedOnMethod( resultType ); + Type resultType = typeBasedOnMethod( method.getResultType() ); // there can be 4 situations // 1. Return type @@ -208,8 +207,9 @@ private List getTargetEntries(Type type, String[] entryNames) { // check if an entry alread exists, otherwise create String[] fullName = Arrays.copyOfRange( entryNames, 0, i + 1 ); + BuilderType builderType = method.isUpdateMethod() ? null : typeFactory.builderTypeFor( nextType ); PropertyEntry propertyEntry = PropertyEntry.forTargetReference( fullName, targetReadAccessor, - targetWriteAccessor, nextType ); + targetWriteAccessor, nextType, builderType ); targetEntries.add( propertyEntry ); } @@ -264,11 +264,11 @@ else if ( targetWriteAccessor == null ) { * search for setters and getters within the updating type. */ private Type typeBasedOnMethod(Type type) { - if ( method.isUpdateMethod() ) { + if ( method.isUpdateMethod() ) { return type; } else { - return type.getEffectiveType(); + return typeFactory.effectiveResultTypeFor( type ); } } 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 1b31bcf4cb..4ce9bff5b8 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 @@ -373,6 +373,7 @@ else if ( method.isStreamMapping() ) { BeanMappingMethod beanMappingMethod = builder .mappingContext( mappingContext ) .sourceMethod( method ) + .returnTypeBuilder( typeFactory.builderTypeFor( method.getReturnType() ) ) .build(); if ( beanMappingMethod != null ) { diff --git a/processor/src/main/java/org/mapstruct/ap/internal/processor/MethodRetrievalProcessor.java b/processor/src/main/java/org/mapstruct/ap/internal/processor/MethodRetrievalProcessor.java index 9ba8d54d54..0d88d97da6 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/processor/MethodRetrievalProcessor.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/processor/MethodRetrievalProcessor.java @@ -387,8 +387,7 @@ private Type selectResultType(Type returnType, Parameter targetParameter) { private boolean checkParameterAndReturnType(ExecutableElement method, List sourceParameters, Parameter targetParameter, List contextParameters, - Type resultType, Type returnType, - boolean containsTargetTypeParameter) { + Type resultType, Type returnType, boolean containsTargetTypeParameter) { if ( sourceParameters.isEmpty() ) { messager.printMessage( method, Message.RETRIEVAL_NO_INPUT_ARGS ); return false; @@ -406,8 +405,8 @@ private boolean checkParameterAndReturnType(ExecutableElement method, List${accessibility.keyword} <@includeModel object=returnType/> ${name}(<#list parameters as param><@includeModel object=param/><#if param_has_next>, )<@throws/> { <#assign targetType = resultType /> <#if !existingInstanceMapping> - <#assign targetType = resultType.effectiveType /> + <#assign targetType = returnTypeToConstruct /> <#list beforeMappingReferencesWithoutMappingTarget as callback> <@includeModel object=callback targetBeanName=resultName targetType=targetType/> @@ -25,7 +25,7 @@ <#if !existingInstanceMapping> - <@includeModel object=resultType.effectiveType/> ${resultName} = <#if factoryMethod??><@includeModel object=factoryMethod targetType=resultType.effectiveType/><#else>new <@includeModel object=resultType.effectiveType/>(); + <@includeModel object=returnTypeToConstruct/> ${resultName} = <#if factoryMethod??><@includeModel object=factoryMethod targetType=returnTypeToConstruct/><#else>new <@includeModel object=returnTypeToConstruct/>(); <#list beforeMappingReferencesWithMappingTarget as callback> diff --git a/processor/src/test/java/org/mapstruct/ap/internal/model/common/DateFormatValidatorFactoryTest.java b/processor/src/test/java/org/mapstruct/ap/internal/model/common/DateFormatValidatorFactoryTest.java index 064aabfd85..9552e881bd 100755 --- a/processor/src/test/java/org/mapstruct/ap/internal/model/common/DateFormatValidatorFactoryTest.java +++ b/processor/src/test/java/org/mapstruct/ap/internal/model/common/DateFormatValidatorFactoryTest.java @@ -167,7 +167,6 @@ private Type typeWithFQN(String fullQualifiedName) { null, null, null, - null, fullQualifiedName, false, false, diff --git a/processor/src/test/java/org/mapstruct/ap/internal/model/common/DefaultConversionContextTest.java b/processor/src/test/java/org/mapstruct/ap/internal/model/common/DefaultConversionContextTest.java index 76ceac40df..bc90a7c3c3 100755 --- a/processor/src/test/java/org/mapstruct/ap/internal/model/common/DefaultConversionContextTest.java +++ b/processor/src/test/java/org/mapstruct/ap/internal/model/common/DefaultConversionContextTest.java @@ -115,7 +115,6 @@ private Type typeWithFQN(String fullQualifiedName) { null, null, null, - null, fullQualifiedName, false, false, diff --git a/processor/src/test/java/org/mapstruct/ap/test/builder/multiple/MultipleBuilderMapperTest.java b/processor/src/test/java/org/mapstruct/ap/test/builder/multiple/MultipleBuilderMapperTest.java index 2d0fe0f78a..ac3e0fc6f6 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/builder/multiple/MultipleBuilderMapperTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/builder/multiple/MultipleBuilderMapperTest.java @@ -113,18 +113,7 @@ public void builderMappingMapperConfigDefined() { TooManyBuilderCreationMethodsMapper.class }) @ExpectedCompilationOutcome(value = CompilationResult.SUCCEEDED, - // We have 2 diagnostics, as we don't do caching of the types, so a type is processed multiple types diagnostics = { - @Diagnostic( - type = Case.class, - kind = javax.tools.Diagnostic.Kind.WARNING, - line = 11, - messageRegExp = "More than one builder creation method for \".*\\.multiple\\.builder.Case\"\\. " + - "Found methods: " + - "\".*wrongBuilder\\(\\) ?, " + - ".*builder\\(\\) ?\"\\. " + - "Builder will not be used\\. Consider implementing a custom BuilderProvider SPI\\." - ), @Diagnostic( type = Case.class, kind = javax.tools.Diagnostic.Kind.WARNING, diff --git a/processor/src/test/java/org/mapstruct/ap/test/selection/generics/ConversionTest.java b/processor/src/test/java/org/mapstruct/ap/test/selection/generics/ConversionTest.java index d46a4b6d06..8f493bb6ca 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/selection/generics/ConversionTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/selection/generics/ConversionTest.java @@ -160,22 +160,18 @@ public void shouldFailOnSuperBounds2() { ErroneousSourceTargetMapper6.class, NoProperties.class }) - @ExpectedCompilationOutcome(value = CompilationResult.FAILED, - diagnostics = { - @Diagnostic(type = ErroneousSourceTargetMapper6.class, - kind = javax.tools.Diagnostic.Kind.ERROR, - line = 16, - messageRegExp = "No target bean properties found: can't map property \".*NoProperties " - + "foo\\.wrapped\" to" - + " \"org.mapstruct.ap.test.selection.generics.TypeA " + - "foo\\.wrapped\""), - @Diagnostic(type = ErroneousSourceTargetMapper6.class, - kind = javax.tools.Diagnostic.Kind.ERROR, - line = 16, - messageRegExp = ".*\\.generics\\.WildCardSuperWrapper<.*\\.generics\\.TypeA> does not have an " + - "accessible parameterless constructor\\.") - - }) + @ExpectedCompilationOutcome( value = CompilationResult.FAILED, diagnostics = { + @Diagnostic( type = ErroneousSourceTargetMapper6.class, kind = javax.tools.Diagnostic.Kind.ERROR, + line = 16, messageRegExp = + ".*\\.generics\\.WildCardSuperWrapper<.*\\.generics\\.TypeA> does not have an " + + "accessible parameterless constructor\\." ), + @Diagnostic( type = ErroneousSourceTargetMapper6.class, kind = javax.tools.Diagnostic.Kind.ERROR, + line = 16, messageRegExp = + "No target bean properties found: can't map property \".*NoProperties " + + "foo\\.wrapped\" to" + + " \"org.mapstruct.ap.test.selection.generics.TypeA " + + "foo\\.wrapped\"" ) + } ) public void shouldFailOnNonMatchingWildCards() { } } From c1a0c50996f9305b11ce6f7d44a896a4b10da32e Mon Sep 17 00:00:00 2001 From: sjaakd Date: Fri, 10 May 2019 21:46:19 +0200 Subject: [PATCH 3/9] #1742 API and UnitTest --- core/src/main/java/org/mapstruct/Builder.java | 8 ++++ .../ap/test/builder/off/SimpleMapper.java | 20 +++++++++ .../test/builder/off/SimpleMutablePerson.java | 21 +++++++++ .../SimpleNotRealyImmutableBuilderTest.java | 42 ++++++++++++++++++ .../off/SimpleNotRealyImmutablePerson.java | 44 +++++++++++++++++++ 5 files changed, 135 insertions(+) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/builder/off/SimpleMapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/builder/off/SimpleMutablePerson.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/builder/off/SimpleNotRealyImmutableBuilderTest.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/builder/off/SimpleNotRealyImmutablePerson.java diff --git a/core/src/main/java/org/mapstruct/Builder.java b/core/src/main/java/org/mapstruct/Builder.java index ec26ab37ab..9909b3b49e 100644 --- a/core/src/main/java/org/mapstruct/Builder.java +++ b/core/src/main/java/org/mapstruct/Builder.java @@ -29,4 +29,12 @@ * @return the method that needs to tbe invoked on the builder */ String buildMethod() default "build"; + + /** + * Toggling builders on / off. Builders are sometimes used solely for unit testing (fluent testdata) + * MapStruct will need to use the regular getters /setters in that case. + * + * @return when true, no builder patterns will be applied + */ + boolean noBuilder() default false; } diff --git a/processor/src/test/java/org/mapstruct/ap/test/builder/off/SimpleMapper.java b/processor/src/test/java/org/mapstruct/ap/test/builder/off/SimpleMapper.java new file mode 100644 index 0000000000..6ab5497e20 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/builder/off/SimpleMapper.java @@ -0,0 +1,20 @@ +/* + * 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.builder.off; + +import org.mapstruct.BeanMapping; +import org.mapstruct.Builder; +import org.mapstruct.CollectionMappingStrategy; +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; + +@Mapper(collectionMappingStrategy = CollectionMappingStrategy.ADDER_PREFERRED) +public interface SimpleMapper { + + @BeanMapping( builder = @Builder( noBuilder = true ) ) + @Mapping(target = "name", source = "fullName") + SimpleNotRealyImmutablePerson toNotRealyImmutable(SimpleMutablePerson source); +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/builder/off/SimpleMutablePerson.java b/processor/src/test/java/org/mapstruct/ap/test/builder/off/SimpleMutablePerson.java new file mode 100644 index 0000000000..c6bb3d6d56 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/builder/off/SimpleMutablePerson.java @@ -0,0 +1,21 @@ +/* + * 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.builder.off; + +import java.util.List; + +public class SimpleMutablePerson { + private String fullName; + + public String getFullName() { + return fullName; + } + + public void setFullName(String fullName) { + this.fullName = fullName; + } + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/builder/off/SimpleNotRealyImmutableBuilderTest.java b/processor/src/test/java/org/mapstruct/ap/test/builder/off/SimpleNotRealyImmutableBuilderTest.java new file mode 100644 index 0000000000..a98be84d22 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/builder/off/SimpleNotRealyImmutableBuilderTest.java @@ -0,0 +1,42 @@ +/* + * 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.builder.off; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mapstruct.ap.testutil.IssueKey; +import org.mapstruct.ap.testutil.WithClasses; +import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner; +import org.mapstruct.ap.testutil.runner.GeneratedSource; +import org.mapstruct.factory.Mappers; + +import static org.assertj.core.api.Assertions.assertThat; + +@IssueKey("1743") +@WithClasses({ + SimpleMutablePerson.class, + SimpleNotRealyImmutablePerson.class +}) +@RunWith(AnnotationProcessorTestRunner.class) +public class SimpleNotRealyImmutableBuilderTest { + + @Rule + public final GeneratedSource generatedSource = new GeneratedSource(); + + @Test + @WithClasses({ SimpleMapper.class }) + public void testSimpleImmutableBuilderHappyPath() { + SimpleMapper mapper = Mappers.getMapper( SimpleMapper.class ); + SimpleMutablePerson source = new SimpleMutablePerson(); + source.setFullName( "Bob" ); + + SimpleNotRealyImmutablePerson targetObject = mapper.toNotRealyImmutable( source ); + + assertThat( targetObject.getName() ).isEqualTo( "Bob" ); + + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/builder/off/SimpleNotRealyImmutablePerson.java b/processor/src/test/java/org/mapstruct/ap/test/builder/off/SimpleNotRealyImmutablePerson.java new file mode 100644 index 0000000000..4caba6b016 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/builder/off/SimpleNotRealyImmutablePerson.java @@ -0,0 +1,44 @@ +/* + * 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.builder.off; + +public class SimpleNotRealyImmutablePerson { + + private String name; + + public static Builder builder() { + return new Builder(); + } + + public SimpleNotRealyImmutablePerson() { + } + + SimpleNotRealyImmutablePerson(Builder builder) { + this.name = builder.name; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public static class Builder { + + private String name; + + public Builder name(String name) { + throw new IllegalStateException( "name should not be called on builder" ); + } + + public SimpleNotRealyImmutablePerson build() { + return new SimpleNotRealyImmutablePerson( this ); + } + + } +} From 02d51d3c76823e916a3e677977a7191a6ff39e5b Mon Sep 17 00:00:00 2001 From: sjaakd Date: Sat, 11 May 2019 09:39:11 +0200 Subject: [PATCH 4/9] #1742 Fix escalation & using Optional for (optional) annotations --- .../model/BuilderFinisherMethodResolver.java | 25 +++++++++---------- .../ap/internal/model/source/BeanMapping.java | 5 ++-- .../ap/internal/util/MapperConfiguration.java | 14 +++++++---- 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/BuilderFinisherMethodResolver.java b/processor/src/main/java/org/mapstruct/ap/internal/model/BuilderFinisherMethodResolver.java index 78375d8e33..91fcea90ca 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/BuilderFinisherMethodResolver.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/BuilderFinisherMethodResolver.java @@ -6,10 +6,12 @@ package org.mapstruct.ap.internal.model; import java.util.Collection; +import java.util.Optional; import javax.lang.model.element.ExecutableElement; import org.mapstruct.ap.internal.model.common.BuilderType; import org.mapstruct.ap.internal.model.source.BeanMapping; +import org.mapstruct.ap.internal.model.source.MappingOptions; import org.mapstruct.ap.internal.model.source.Method; import org.mapstruct.ap.internal.prism.BuilderPrism; import org.mapstruct.ap.internal.util.MapperConfiguration; @@ -36,14 +38,18 @@ public static MethodReference getBuilderFinisherMethod(Method method, BuilderTyp return null; } - BuilderPrism builderMapping = builderMappingPrism( method, ctx ); - if ( builderMapping == null && buildMethods.size() == 1 ) { + Optional beanMethodBuilder = Optional.ofNullable( method.getMappingOptions() ) + .map( MappingOptions::getBeanMapping ) + .flatMap( BeanMapping::getBuilder ); + Optional builderMapping = method.getMapperConfiguration().getBuilderPrism( beanMethodBuilder ); + + if ( !builderMapping.isPresent() && buildMethods.size() == 1 ) { return MethodReference.forMethodCall( first( buildMethods ).getSimpleName().toString() ); } else { String buildMethodPattern = DEFAULT_BUILD_METHOD_NAME; - if ( builderMapping != null ) { - buildMethodPattern = builderMapping.buildMethod(); + if ( builderMapping.isPresent() ) { + buildMethodPattern = builderMapping.get().buildMethod(); } for ( ExecutableElement buildMethod : buildMethods ) { String methodName = buildMethod.getSimpleName().toString(); @@ -52,7 +58,7 @@ public static MethodReference getBuilderFinisherMethod(Method method, BuilderTyp } } - if ( builderMapping == null ) { + if ( !builderMapping.isPresent() ) { ctx.getMessager().printMessage( method.getExecutable(), Message.BUILDER_NO_BUILD_METHOD_FOUND_DEFAULT, @@ -65,7 +71,7 @@ public static MethodReference getBuilderFinisherMethod(Method method, BuilderTyp else { ctx.getMessager().printMessage( method.getExecutable(), - builderMapping.mirror, + builderMapping.get().mirror, Message.BUILDER_NO_BUILD_METHOD_FOUND, buildMethodPattern, builderType.getBuilder(), @@ -78,11 +84,4 @@ public static MethodReference getBuilderFinisherMethod(Method method, BuilderTyp return null; } - private static BuilderPrism builderMappingPrism(Method method, MappingBuilderContext ctx) { - BeanMapping beanMapping = method.getMappingOptions().getBeanMapping(); - if ( beanMapping != null && beanMapping.getBuilder() != null ) { - return beanMapping.getBuilder(); - } - return MapperConfiguration.getInstanceOn( ctx.getMapperTypeElement() ).getBuilderPrism(); - } } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/BeanMapping.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/BeanMapping.java index f2dc9527d1..c77ac3561a 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/BeanMapping.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/BeanMapping.java @@ -7,6 +7,7 @@ import java.util.Collections; import java.util.List; +import java.util.Optional; import javax.lang.model.element.ExecutableElement; import javax.lang.model.type.TypeKind; import javax.lang.model.util.Types; @@ -174,7 +175,7 @@ public List getIgnoreUnmappedSourceProperties() { return ignoreUnmappedSourceProperties; } - public BuilderPrism getBuilder() { - return builder; + public Optional getBuilder() { + return Optional.ofNullable( builder ); } } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/MapperConfiguration.java b/processor/src/main/java/org/mapstruct/ap/internal/util/MapperConfiguration.java index cb812b53b7..2d20b3ab86 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/util/MapperConfiguration.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/util/MapperConfiguration.java @@ -8,6 +8,7 @@ import java.util.ArrayList; import java.util.LinkedHashSet; import java.util.List; +import java.util.Optional; import java.util.Set; import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.Element; @@ -271,15 +272,18 @@ public boolean isDisableSubMappingMethodsGeneration() { return mapperPrism.disableSubMappingMethodsGeneration(); // fall back to default defined in the annotation } - public BuilderPrism getBuilderPrism() { - if ( mapperPrism.values.builder() != null ) { - return mapperPrism.builder(); + public Optional getBuilderPrism(Optional beanMappingBuilderPrism) { + if ( beanMappingBuilderPrism != null && beanMappingBuilderPrism.isPresent() ) { + return beanMappingBuilderPrism; + } + else if ( mapperPrism.values.builder() != null ) { + return Optional.ofNullable( mapperPrism.builder() ); } else if ( mapperConfigPrism != null && mapperConfigPrism.values.builder() != null ) { - return mapperConfigPrism.builder(); + return Optional.ofNullable( mapperConfigPrism.builder() ); } else { - return null; + return Optional.empty(); } } From e0d7e0bdef3c1b75fbee13b8d015d369d03837ce Mon Sep 17 00:00:00 2001 From: sjaakd Date: Sat, 11 May 2019 10:18:50 +0200 Subject: [PATCH 5/9] #1742 Fix builder optional --- .../model/AbstractMappingMethodBuilder.java | 8 ++++- .../model/BuilderFinisherMethodResolver.java | 8 +---- .../ap/internal/model/PropertyMapping.java | 4 ++- .../ap/internal/model/common/TypeFactory.java | 14 +++++---- .../ap/internal/model/source/BeanMapping.java | 11 +++++-- .../internal/model/source/MappingOptions.java | 5 +++- .../model/source/TargetReference.java | 30 +++++++++++++++---- .../processor/MapperCreationProcessor.java | 5 +++- .../processor/MethodRetrievalProcessor.java | 2 +- .../test/builder/off/SimpleMutablePerson.java | 2 -- 10 files changed, 63 insertions(+), 26 deletions(-) diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/AbstractMappingMethodBuilder.java b/processor/src/main/java/org/mapstruct/ap/internal/model/AbstractMappingMethodBuilder.java index 139ba8129d..6e6b3e0e97 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/AbstractMappingMethodBuilder.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/AbstractMappingMethodBuilder.java @@ -8,6 +8,7 @@ import org.mapstruct.ap.internal.model.common.Assignment; import org.mapstruct.ap.internal.model.common.SourceRHS; import org.mapstruct.ap.internal.model.common.Type; +import org.mapstruct.ap.internal.model.source.BeanMapping; import org.mapstruct.ap.internal.model.source.ForgedMethod; import org.mapstruct.ap.internal.model.source.ForgedMethodHistory; import org.mapstruct.ap.internal.util.Strings; @@ -63,7 +64,12 @@ Assignment forgeMapping(SourceRHS sourceRHS, Type sourceType, Type targetType) { true ); - return createForgedAssignment( sourceRHS, ctx.getTypeFactory().builderTypeFor( targetType ), forgedMethod ); + return createForgedAssignment( + sourceRHS, + ctx.getTypeFactory() + .builderTypeFor( targetType, BeanMapping.builderPrismFor( method ).orElse( null ) ), + forgedMethod + ); } private String getName(Type sourceType, Type targetType) { diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/BuilderFinisherMethodResolver.java b/processor/src/main/java/org/mapstruct/ap/internal/model/BuilderFinisherMethodResolver.java index 91fcea90ca..9f44085809 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/BuilderFinisherMethodResolver.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/BuilderFinisherMethodResolver.java @@ -11,10 +11,8 @@ import org.mapstruct.ap.internal.model.common.BuilderType; import org.mapstruct.ap.internal.model.source.BeanMapping; -import org.mapstruct.ap.internal.model.source.MappingOptions; import org.mapstruct.ap.internal.model.source.Method; import org.mapstruct.ap.internal.prism.BuilderPrism; -import org.mapstruct.ap.internal.util.MapperConfiguration; import org.mapstruct.ap.internal.util.Message; import org.mapstruct.ap.internal.util.Strings; @@ -38,11 +36,7 @@ public static MethodReference getBuilderFinisherMethod(Method method, BuilderTyp return null; } - Optional beanMethodBuilder = Optional.ofNullable( method.getMappingOptions() ) - .map( MappingOptions::getBeanMapping ) - .flatMap( BeanMapping::getBuilder ); - Optional builderMapping = method.getMapperConfiguration().getBuilderPrism( beanMethodBuilder ); - + Optional builderMapping = BeanMapping.builderPrismFor( method ); if ( !builderMapping.isPresent() && buildMethods.size() == 1 ) { return MethodReference.forMethodCall( first( buildMethods ).getSimpleName().toString() ); } 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 a2ba31752a..a439f69178 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 @@ -37,6 +37,7 @@ import org.mapstruct.ap.internal.model.source.SelectionParameters; import org.mapstruct.ap.internal.model.source.SourceReference; import org.mapstruct.ap.internal.model.source.selector.SelectionCriteria; +import org.mapstruct.ap.internal.prism.BuilderPrism; import org.mapstruct.ap.internal.prism.NullValueCheckStrategyPrism; import org.mapstruct.ap.internal.prism.NullValueMappingStrategyPrism; import org.mapstruct.ap.internal.prism.NullValuePropertyMappingStrategyPrism; @@ -112,7 +113,8 @@ public T targetReadAccessor(Accessor targetReadAccessor) { public T targetWriteAccessor(Accessor targetWriteAccessor) { this.targetWriteAccessor = targetWriteAccessor; this.targetType = ctx.getTypeFactory().getType( targetWriteAccessor.getAccessedType() ); - this.targetBuilderType = ctx.getTypeFactory().builderTypeFor( this.targetType ); + BuilderPrism builderPrism = BeanMapping.builderPrismFor( method ).orElse( null ); + this.targetBuilderType = ctx.getTypeFactory().builderTypeFor( this.targetType, builderPrism ); this.targetWriteAccessorType = targetWriteAccessor.getAccessorType(); return (T) this; } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/common/TypeFactory.java b/processor/src/main/java/org/mapstruct/ap/internal/model/common/TypeFactory.java index 8f3da0b915..33747c6e2d 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/common/TypeFactory.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/common/TypeFactory.java @@ -39,6 +39,7 @@ import javax.lang.model.util.Elements; import javax.lang.model.util.Types; +import org.mapstruct.ap.internal.prism.BuilderPrism; import org.mapstruct.ap.internal.util.AnnotationProcessingException; import org.mapstruct.ap.internal.util.Collections; import org.mapstruct.ap.internal.util.Extractor; @@ -521,7 +522,10 @@ private ImplementationType getImplementationType(TypeMirror mirror) { return null; } - private BuilderInfo findBuilder(TypeMirror type, boolean report) { + private BuilderInfo findBuilder(TypeMirror type, BuilderPrism builderPrism, boolean report) { + if ( builderPrism != null && builderPrism.noBuilder() ) { + return null; + } try { return roundContext.getAnnotationProcessorContext() .getBuilderProvider() @@ -664,17 +668,17 @@ private boolean canBeProcessed(TypeMirror type) { return true; } - public BuilderType builderTypeFor( Type type ) { + public BuilderType builderTypeFor( Type type, BuilderPrism builderPrism ) { if ( type != null ) { - BuilderInfo builderInfo = findBuilder( type.getTypeMirror(), true ); + BuilderInfo builderInfo = findBuilder( type.getTypeMirror(), builderPrism, true ); return BuilderType.create( builderInfo, type, this, this.typeUtils ); } return null; } - public Type effectiveResultTypeFor( Type type ) { + public Type effectiveResultTypeFor( Type type, BuilderPrism builderPrism ) { if ( type != null ) { - BuilderInfo builderInfo = findBuilder( type.getTypeMirror(), false ); + BuilderInfo builderInfo = findBuilder( type.getTypeMirror(), builderPrism, false ); BuilderType builderType = BuilderType.create( builderInfo, type, this, this.typeUtils ); return builderType != null ? builderType.getBuilder() : type; } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/BeanMapping.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/BeanMapping.java index c77ac3561a..a180264f80 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/BeanMapping.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/BeanMapping.java @@ -175,7 +175,14 @@ public List getIgnoreUnmappedSourceProperties() { return ignoreUnmappedSourceProperties; } - public Optional getBuilder() { - return Optional.ofNullable( builder ); + /** + * derives the builder prism given the options and configuration + * @param method containing mandatory configuration and the mapping options (optionally containing a beanmapping) + * @return a BuilderPrism as optional + */ + public static Optional builderPrismFor(Method method) { + return method.getMapperConfiguration() + .getBuilderPrism( Optional.ofNullable( method.getMappingOptions().getBeanMapping() ) + .map( b -> b.builder ) ); } } 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 b89192fedd..3e6811af9e 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 @@ -291,7 +291,10 @@ public void applyIgnoreAll(MappingOptions inherited, SourceMethod method, Format CollectionMappingStrategyPrism cms = method.getMapperConfiguration().getCollectionMappingStrategy(); Type writeType = method.getResultType(); if ( !method.isUpdateMethod() ) { - writeType = typeFactory.effectiveResultTypeFor( writeType ); + writeType = typeFactory.effectiveResultTypeFor( + writeType, + BeanMapping.builderPrismFor( method ).orElse( null ) + ); } Map writeAccessors = writeType.getPropertyWriteAccessors( cms ); List mappedPropertyNames = new ArrayList<>(); diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/TargetReference.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/TargetReference.java index 1d6e6d3f90..7c30144371 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/TargetReference.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/TargetReference.java @@ -17,6 +17,7 @@ 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.prism.BuilderPrism; import org.mapstruct.ap.internal.prism.CollectionMappingStrategyPrism; import org.mapstruct.ap.internal.util.AccessorNamingUtils; import org.mapstruct.ap.internal.util.FormattingMessager; @@ -207,9 +208,27 @@ private List getTargetEntries(Type type, String[] entryNames) { // check if an entry alread exists, otherwise create String[] fullName = Arrays.copyOfRange( entryNames, 0, i + 1 ); - BuilderType builderType = method.isUpdateMethod() ? null : typeFactory.builderTypeFor( nextType ); - PropertyEntry propertyEntry = PropertyEntry.forTargetReference( fullName, targetReadAccessor, - targetWriteAccessor, nextType, builderType ); + BuilderType builderType; + PropertyEntry propertyEntry = null; + if ( method.isUpdateMethod() ) { + propertyEntry = PropertyEntry.forTargetReference( fullName, + targetReadAccessor, + targetWriteAccessor, + nextType, + null + ); + } + else { + BuilderPrism builderPrism = BeanMapping.builderPrismFor( method ).orElse( null ); + builderType = typeFactory.builderTypeFor( nextType, builderPrism ); + propertyEntry = PropertyEntry.forTargetReference( fullName, + targetReadAccessor, + targetWriteAccessor, + nextType, + builderType + ); + + } targetEntries.add( propertyEntry ); } @@ -264,11 +283,12 @@ else if ( targetWriteAccessor == null ) { * search for setters and getters within the updating type. */ private Type typeBasedOnMethod(Type type) { - if ( method.isUpdateMethod() ) { + if ( method.isUpdateMethod() ) { return type; } else { - return typeFactory.effectiveResultTypeFor( type ); + BuilderPrism builderPrism = BeanMapping.builderPrismFor( method ).orElse( null ); + return typeFactory.effectiveResultTypeFor( type, builderPrism ); } } 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 4ce9bff5b8..3ee6adabdb 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 @@ -41,11 +41,13 @@ import org.mapstruct.ap.internal.model.common.FormattingParameters; import org.mapstruct.ap.internal.model.common.Type; import org.mapstruct.ap.internal.model.common.TypeFactory; +import org.mapstruct.ap.internal.model.source.BeanMapping; 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.option.Options; +import org.mapstruct.ap.internal.prism.BuilderPrism; import org.mapstruct.ap.internal.prism.DecoratedWithPrism; import org.mapstruct.ap.internal.prism.InheritConfigurationPrism; import org.mapstruct.ap.internal.prism.InheritInverseConfigurationPrism; @@ -369,11 +371,12 @@ else if ( method.isStreamMapping() ) { } else { this.messager.note( 1, Message.BEANMAPPING_CREATE_NOTE, method ); + BuilderPrism builderPrism = BeanMapping.builderPrismFor( method ).orElse( null ); BeanMappingMethod.Builder builder = new BeanMappingMethod.Builder(); BeanMappingMethod beanMappingMethod = builder .mappingContext( mappingContext ) .sourceMethod( method ) - .returnTypeBuilder( typeFactory.builderTypeFor( method.getReturnType() ) ) + .returnTypeBuilder( typeFactory.builderTypeFor( method.getReturnType(), builderPrism ) ) .build(); if ( beanMappingMethod != null ) { diff --git a/processor/src/main/java/org/mapstruct/ap/internal/processor/MethodRetrievalProcessor.java b/processor/src/main/java/org/mapstruct/ap/internal/processor/MethodRetrievalProcessor.java index 0d88d97da6..b796945f80 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/processor/MethodRetrievalProcessor.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/processor/MethodRetrievalProcessor.java @@ -406,7 +406,7 @@ private boolean checkParameterAndReturnType(ExecutableElement method, List Date: Sat, 11 May 2019 10:27:52 +0200 Subject: [PATCH 6/9] #1742 removing Optional as argument --- .../org/mapstruct/ap/internal/model/source/BeanMapping.java | 3 ++- .../org/mapstruct/ap/internal/util/MapperConfiguration.java | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/BeanMapping.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/BeanMapping.java index a180264f80..9c05e382a7 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/BeanMapping.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/BeanMapping.java @@ -183,6 +183,7 @@ public List getIgnoreUnmappedSourceProperties() { public static Optional builderPrismFor(Method method) { return method.getMapperConfiguration() .getBuilderPrism( Optional.ofNullable( method.getMappingOptions().getBeanMapping() ) - .map( b -> b.builder ) ); + .map( b -> b.builder ) + .orElse( null ) ); } } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/MapperConfiguration.java b/processor/src/main/java/org/mapstruct/ap/internal/util/MapperConfiguration.java index 2d20b3ab86..909152b6a0 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/util/MapperConfiguration.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/util/MapperConfiguration.java @@ -272,9 +272,9 @@ public boolean isDisableSubMappingMethodsGeneration() { return mapperPrism.disableSubMappingMethodsGeneration(); // fall back to default defined in the annotation } - public Optional getBuilderPrism(Optional beanMappingBuilderPrism) { - if ( beanMappingBuilderPrism != null && beanMappingBuilderPrism.isPresent() ) { - return beanMappingBuilderPrism; + public Optional getBuilderPrism(BuilderPrism beanMappingBuilderPrism) { + if ( beanMappingBuilderPrism != null ) { + return Optional.ofNullable( beanMappingBuilderPrism ); } else if ( mapperPrism.values.builder() != null ) { return Optional.ofNullable( mapperPrism.builder() ); From c3e8d4e2d6801413931a36b3c39f52bdae806328 Mon Sep 17 00:00:00 2001 From: sjaakd Date: Fri, 24 May 2019 22:43:04 +0200 Subject: [PATCH 7/9] #1742 rework --- core/src/main/java/org/mapstruct/Builder.java | 2 +- .../org/mapstruct/ap/internal/model/common/TypeFactory.java | 2 +- .../mapstruct/ap/internal/model/source/ForgedMethod.java | 6 +++--- .../org/mapstruct/ap/test/builder/off/SimpleMapper.java | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/mapstruct/Builder.java b/core/src/main/java/org/mapstruct/Builder.java index 9909b3b49e..2f3c9b2e90 100644 --- a/core/src/main/java/org/mapstruct/Builder.java +++ b/core/src/main/java/org/mapstruct/Builder.java @@ -36,5 +36,5 @@ * * @return when true, no builder patterns will be applied */ - boolean noBuilder() default false; + boolean disableBuilder() default false; } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/common/TypeFactory.java b/processor/src/main/java/org/mapstruct/ap/internal/model/common/TypeFactory.java index 33747c6e2d..c94986e822 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/common/TypeFactory.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/common/TypeFactory.java @@ -523,7 +523,7 @@ private ImplementationType getImplementationType(TypeMirror mirror) { } private BuilderInfo findBuilder(TypeMirror type, BuilderPrism builderPrism, boolean report) { - if ( builderPrism != null && builderPrism.noBuilder() ) { + if ( builderPrism != null && builderPrism.disableBuilder() ) { return null; } try { diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/ForgedMethod.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/ForgedMethod.java index 01dbdedb1b..0298dcbf24 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/ForgedMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/ForgedMethod.java @@ -50,9 +50,9 @@ public class ForgedMethod implements Method { * @param parameterProvidedMethods additional factory/lifecycle methods to consider that are provided by context * parameters */ - public ForgedMethod(String name, Type sourceType, Type returnType, - MapperConfiguration mapperConfiguration, ExecutableElement positionHintElement, - List additionalParameters, ParameterProvidedMethods parameterProvidedMethods) { + public ForgedMethod(String name, Type sourceType, Type returnType, MapperConfiguration mapperConfiguration, + ExecutableElement positionHintElement, List additionalParameters, + ParameterProvidedMethods parameterProvidedMethods) { this( name, sourceType, diff --git a/processor/src/test/java/org/mapstruct/ap/test/builder/off/SimpleMapper.java b/processor/src/test/java/org/mapstruct/ap/test/builder/off/SimpleMapper.java index 6ab5497e20..94e5101720 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/builder/off/SimpleMapper.java +++ b/processor/src/test/java/org/mapstruct/ap/test/builder/off/SimpleMapper.java @@ -14,7 +14,7 @@ @Mapper(collectionMappingStrategy = CollectionMappingStrategy.ADDER_PREFERRED) public interface SimpleMapper { - @BeanMapping( builder = @Builder( noBuilder = true ) ) + @BeanMapping( builder = @Builder( disableBuilder = true ) ) @Mapping(target = "name", source = "fullName") SimpleNotRealyImmutablePerson toNotRealyImmutable(SimpleMutablePerson source); } From 64bd2c7b18dfd3299f1a2c3251ba44c3ddaf696e Mon Sep 17 00:00:00 2001 From: sjaakd Date: Fri, 24 May 2019 22:59:18 +0200 Subject: [PATCH 8/9] #1742 documentation --- .../src/main/asciidoc/mapstruct-reference-guide.asciidoc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/documentation/src/main/asciidoc/mapstruct-reference-guide.asciidoc b/documentation/src/main/asciidoc/mapstruct-reference-guide.asciidoc index 467dc910c1..0f53069fb4 100644 --- a/documentation/src/main/asciidoc/mapstruct-reference-guide.asciidoc +++ b/documentation/src/main/asciidoc/mapstruct-reference-guide.asciidoc @@ -684,6 +684,11 @@ In case of a `MoreThanOneBuilderCreationMethodException` MapStruct will write a If such type is found then MapStruct will use that type to perform the mapping to (i.e. it will look for setters into that type). To finish the mapping MapStruct generates code that will invoke the build method of the builder. +[NOTE] +====== +Builder detection can be switched off by means of `@Builder#disableBuilder`. MapStruct will fall back on regular getters / setters in case builders are disabled. +====== + [NOTE] ====== The <> are also considered for the builder type. From a2ab6b5b0a1341223ed463a7020d65a164b6e350 Mon Sep 17 00:00:00 2001 From: sjaakd Date: Fri, 24 May 2019 23:18:35 +0200 Subject: [PATCH 9/9] #1742 rework ii --- .../mapstruct/ap/internal/model/common/BuilderType.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/common/BuilderType.java b/processor/src/main/java/org/mapstruct/ap/internal/model/common/BuilderType.java index 94f406b1f9..bae5c161b8 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/common/BuilderType.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/common/BuilderType.java @@ -82,13 +82,6 @@ public Collection getBuildMethods() { return buildMethods; } - public BuilderInfo asBuilderInfo() { - return new BuilderInfo.Builder() - .builderCreationMethod( this.builderCreationMethod ) - .buildMethod( this.buildMethods ) - .build(); - } - public static BuilderType create(BuilderInfo builderInfo, Type typeToBuild, TypeFactory typeFactory, Types typeUtils) { if ( builderInfo == null ) {