diff --git a/processor/src/main/java/org/mapstruct/ap/model/BeanMappingMethod.java b/processor/src/main/java/org/mapstruct/ap/model/BeanMappingMethod.java index 2f56ece04..725fbe9cc 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/BeanMappingMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/model/BeanMappingMethod.java @@ -92,7 +92,7 @@ public class BeanMappingMethod extends MappingMethod { String targetPropertyName = Executables.getPropertyName( targetAccessor ); - Mapping mapping = method.getMappingByTargetPropertyName( targetPropertyName ); + Mapping mapping = method.getSingleMappingByTargetPropertyName( targetPropertyName ); if ( mapping != null && mapping.isIgnored() ) { ignoredTargetProperties.add( targetPropertyName ); @@ -146,7 +146,7 @@ public class BeanMappingMethod extends MappingMethod { else if ( Executables.isSetterMethod( targetAccessor ) || Executables.isGetterMethod( targetAccessor ) ) { - if ( !mapping.getConstant().isEmpty() ) { + if ( mapping.getConstant() != null ) { // its a constant PropertyMapping.ConstantMappingBuilder builder = new PropertyMapping.ConstantMappingBuilder(); @@ -160,7 +160,7 @@ public class BeanMappingMethod extends MappingMethod { .build(); } - else if ( !mapping.getJavaExpression().isEmpty() ) { + else if ( mapping.getJavaExpression() != null ) { // its an expression PropertyMapping.JavaExpressionMappingBuilder builder = new PropertyMapping.JavaExpressionMappingBuilder(); diff --git a/processor/src/main/java/org/mapstruct/ap/model/EnumMappingMethod.java b/processor/src/main/java/org/mapstruct/ap/model/EnumMappingMethod.java index 6efdd1518..9c04b63b0 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/EnumMappingMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/model/EnumMappingMethod.java @@ -20,8 +20,7 @@ package org.mapstruct.ap.model; import java.util.ArrayList; import java.util.List; -import java.util.Map; -import java.util.Set; + import javax.tools.Diagnostic; import org.mapstruct.ap.model.common.Parameter; @@ -67,12 +66,11 @@ public class EnumMappingMethod extends MappingMethod { List sourceEnumConstants = method.getSourceParameters().iterator().next().getType().getEnumConstants(); - Map> mappings = method.getMappings(); for ( String enumConstant : sourceEnumConstants ) { - List mappedConstants = mappings.get( enumConstant ); + List mappedConstants = method.getMappingBySourcePropertyName( enumConstant ); - if ( mappedConstants == null ) { + if ( mappedConstants.isEmpty() ) { enumMappings.add( new EnumMapping( enumConstant, enumConstant ) ); } else if ( mappedConstants.size() == 1 ) { @@ -172,12 +170,11 @@ public class EnumMappingMethod extends MappingMethod { List sourceEnumConstants = method.getSourceParameters().iterator().next().getType().getEnumConstants(); List targetEnumConstants = method.getReturnType().getEnumConstants(); - Set mappedSourceEnumConstants = method.getMappings().keySet(); List unmappedSourceEnumConstants = new ArrayList(); for ( String sourceEnumConstant : sourceEnumConstants ) { if ( !targetEnumConstants.contains( sourceEnumConstant ) - && !mappedSourceEnumConstants.contains( sourceEnumConstant ) ) { + && method.getMappingBySourcePropertyName( sourceEnumConstant ).isEmpty() ) { unmappedSourceEnumConstants.add( sourceEnumConstant ); } } diff --git a/processor/src/main/java/org/mapstruct/ap/model/PropertyMapping.java b/processor/src/main/java/org/mapstruct/ap/model/PropertyMapping.java index 22e5f92b2..1b05e5064 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/PropertyMapping.java +++ b/processor/src/main/java/org/mapstruct/ap/model/PropertyMapping.java @@ -104,7 +104,7 @@ public class PropertyMapping extends ModelElement { public PropertyMapping build() { // check if there's a mapping defined - Mapping mapping = method.getMappingByTargetPropertyName( targetPropertyName ); + Mapping mapping = method.getSingleMappingByTargetPropertyName( targetPropertyName ); String dateFormat = null; List qualifiers = null; String sourcePropertyName; diff --git a/processor/src/main/java/org/mapstruct/ap/model/source/Mapping.java b/processor/src/main/java/org/mapstruct/ap/model/source/Mapping.java index f64341429..c5ff78d0c 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/source/Mapping.java +++ b/processor/src/main/java/org/mapstruct/ap/model/source/Mapping.java @@ -28,8 +28,13 @@ import javax.annotation.processing.Messager; import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.AnnotationValue; import javax.lang.model.element.Element; +import javax.lang.model.element.ElementKind; +import javax.lang.model.element.ExecutableElement; +import javax.lang.model.type.DeclaredType; +import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeMirror; import javax.tools.Diagnostic; +import javax.tools.Diagnostic.Kind; import org.mapstruct.ap.prism.MappingPrism; import org.mapstruct.ap.prism.MappingsPrism; @@ -59,24 +64,42 @@ public class Mapping { private final AnnotationValue targetAnnotationValue; - public static Map> fromMappingsPrism(MappingsPrism mappingsAnnotation, Element element, + public static Map> fromMappingsPrism(MappingsPrism mappingsAnnotation, + ExecutableElement method, Messager messager) { Map> mappings = new HashMap>(); for ( MappingPrism mappingPrism : mappingsAnnotation.value() ) { - if ( !mappings.containsKey( mappingPrism.source() ) ) { - mappings.put( mappingPrism.source(), new ArrayList() ); - } - Mapping mapping = fromMappingPrism( mappingPrism, element, messager ); + Mapping mapping = fromMappingPrism( mappingPrism, method, messager ); if ( mapping != null ) { - mappings.get( mappingPrism.source() ).add( mapping ); + List mappingsOfProperty = mappings.get( mappingPrism.target() ); + if ( mappingsOfProperty == null ) { + mappingsOfProperty = new ArrayList(); + mappings.put( mappingPrism.target(), mappingsOfProperty ); + } + + mappingsOfProperty.add( mapping ); + + if ( mappingsOfProperty.size() > 1 && !isEnumType( method.getReturnType() ) ) { + messager.printMessage( + Kind.ERROR, + "Target property \"" + mappingPrism.target() + "\" must not be mapped more than once.", + method + ); + } } } return mappings; } - public static Mapping fromMappingPrism(MappingPrism mappingPrism, Element element, Messager messager) { + private static boolean isEnumType(TypeMirror mirror) { + return mirror.getKind() == TypeKind.DECLARED && + ( (DeclaredType) mirror ).asElement().getKind() == ElementKind.ENUM; + } + + + public static Mapping fromMappingPrism(MappingPrism mappingPrism, ExecutableElement element, Messager messager) { String[] sourceNameParts = getSourceNameParts( mappingPrism.source(), element, @@ -84,10 +107,21 @@ public class Mapping { mappingPrism.values.source() ); + if ( mappingPrism.target().isEmpty() ) { + messager.printMessage( + Diagnostic.Kind.ERROR, + "Target must not be empty in @Mapping", + element, + mappingPrism.mirror, + mappingPrism.values.target() + ); + return null; + } + if ( !mappingPrism.source().isEmpty() && !mappingPrism.constant().isEmpty() ) { messager.printMessage( Diagnostic.Kind.ERROR, - "Source and constant are both defined in Mapping, either define a source or a constant", + "Source and constant are both defined in @Mapping, either define a source or a constant", element ); return null; @@ -95,7 +129,7 @@ public class Mapping { else if ( !mappingPrism.source().isEmpty() && !mappingPrism.expression().isEmpty() ) { messager.printMessage( Diagnostic.Kind.ERROR, - "Source and expression are both defined in Mapping, either define a source or an expression", + "Source and expression are both defined in @Mapping, either define a source or an expression", element ); return null; @@ -103,21 +137,22 @@ public class Mapping { else if ( !mappingPrism.expression().isEmpty() && !mappingPrism.constant().isEmpty() ) { messager.printMessage( Diagnostic.Kind.ERROR, - "Expression and constant are both defined in Mapping, either define an expression or a constant", + "Expression and constant are both defined in @Mapping, either define an expression or a constant", element ); return null; } String source = mappingPrism.source().isEmpty() ? null : mappingPrism.source(); + return new Mapping( source, sourceNameParts != null ? sourceNameParts[0] : null, sourceNameParts != null ? sourceNameParts[1] : source, - mappingPrism.constant(), - mappingPrism.expression(), + mappingPrism.constant().isEmpty() ? null : mappingPrism.constant(), + mappingPrism.expression().isEmpty() ? null : mappingPrism.expression(), mappingPrism.target(), - mappingPrism.dateFormat(), + mappingPrism.dateFormat().isEmpty() ? null : mappingPrism.dateFormat(), mappingPrism.qualifiedBy(), mappingPrism.ignore(), mappingPrism.mirror, @@ -155,8 +190,13 @@ public class Mapping { this.sourcePropertyName = sourcePropertyName; this.constant = constant; this.expression = expression; - Matcher javaExpressionMatcher = JAVA_EXPRESSION.matcher( expression ); - this.javaExpression = javaExpressionMatcher.matches() ? javaExpressionMatcher.group( 1 ).trim() : ""; + if ( expression != null ) { + Matcher javaExpressionMatcher = JAVA_EXPRESSION.matcher( expression ); + this.javaExpression = javaExpressionMatcher.matches() ? javaExpressionMatcher.group( 1 ).trim() : null; + } + else { + this.javaExpression = null; + } this.targetName = targetName; this.dateFormat = dateFormat; this.qualifiers = qualifiers; @@ -234,7 +274,7 @@ public class Mapping { public Mapping reverse() { Mapping reverse = null; // mapping can only be reversed if the source was not a constant nor an expression - if ( constant.isEmpty() && expression.isEmpty() ) { + if ( constant == null && expression == null ) { reverse = new Mapping( sourceName != null ? targetName : null, null, diff --git a/processor/src/main/java/org/mapstruct/ap/model/source/SourceMethod.java b/processor/src/main/java/org/mapstruct/ap/model/source/SourceMethod.java index fa2310a6a..c334566a6 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/source/SourceMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/model/source/SourceMethod.java @@ -146,6 +146,7 @@ public class SourceMethod implements Method { return declaringMapper; } + @Override public ExecutableElement getExecutable() { return executable; } @@ -212,12 +213,18 @@ public class SourceMethod implements Method { } /** - * @return the {@link Mapping}s configured for this method, keyed by source property name. + * @return the {@link Mapping}s configured for this method, keyed by target property name. Only for enum mapping + * methods a target will be mapped by several sources. */ public Map> getMappings() { return mappings; } + public Mapping getSingleMappingByTargetPropertyName(String targetPropertyName) { + List all = mappings.get( targetPropertyName ); + return all != null ? all.iterator().next() : null; + } + public void setMappings(Map> mappings) { this.mappings = mappings; this.configuredByReverseMappingMethod = true; @@ -298,17 +305,20 @@ public class SourceMethod implements Method { } /** - * Returns the {@link Mapping} for the given target property. May return {@code null}. + * Returns the {@link Mapping}s for the given source property. */ - public Mapping getMappingByTargetPropertyName(String targetPropertyName) { - for ( Map.Entry> entry : mappings.entrySet() ) { - for ( Mapping mapping : entry.getValue() ) { - if ( mapping.getTargetName().equals( targetPropertyName ) ) { - return mapping; + public List getMappingBySourcePropertyName(String sourcePropertyName) { + List mappingsOfSourceProperty = new ArrayList(); + + for ( List mappingOfProperty : mappings.values() ) { + for ( Mapping mapping : mappingOfProperty ) { + if ( mapping.getSourcePropertyName().equals( sourcePropertyName ) ) { + mappingsOfSourceProperty.add( mapping ); } } } - return null; + + return mappingsOfSourceProperty; } public Parameter getSourceParameter(String sourceParameterName) { diff --git a/processor/src/main/java/org/mapstruct/ap/processor/MapperCreationProcessor.java b/processor/src/main/java/org/mapstruct/ap/processor/MapperCreationProcessor.java index 05804980f..0317a518c 100644 --- a/processor/src/main/java/org/mapstruct/ap/processor/MapperCreationProcessor.java +++ b/processor/src/main/java/org/mapstruct/ap/processor/MapperCreationProcessor.java @@ -361,40 +361,23 @@ public class MapperCreationProcessor implements ModelElementProcessor> reverse(Map> mappings) { - Map> reversed = new HashMap>(); - - for ( List mappingList : mappings.values() ) { - for ( Mapping mapping : mappingList ) { - - Mapping reverseMapping = mapping.reverse(); - if ( reverseMapping != null ) { - if ( !reversed.containsKey( mapping.getTargetName() ) ) { - reversed.put( mapping.getTargetName(), new ArrayList() ); - } - reversed.get( mapping.getTargetName() ).add( reverseMapping ); - } - } - } - return reversed; - } - - private void mergeWithReverseMappings(SourceMethod reverseMappingMethod, SourceMethod method) { + private void mergeWithReverseMappings(SourceMethod forwardMappingMethod, SourceMethod method) { Map> newMappings = new HashMap>(); - if ( reverseMappingMethod != null && !reverseMappingMethod.getMappings().isEmpty() ) { - // define all the base mappings based on its forward counterpart. - // however, remove the mappings that are designated as constant, expression or ignore. - // They are characterized by the key "" - Map> reverseMappings = new HashMap>(); - reverseMappings.putAll( reverseMappingMethod.getMappings() ); - List nonSourceMappings = method.getMappings().get( "" ); - if (nonSourceMappings != null ) { - for (Mapping nonSourceMapping : nonSourceMappings) { - reverseMappings.remove( nonSourceMapping.getTargetName() ); + if ( forwardMappingMethod != null && !forwardMappingMethod.getMappings().isEmpty() ) { + for ( List mappings : forwardMappingMethod.getMappings().values() ) { + for ( Mapping forwardMapping : mappings ) { + Mapping reversed = forwardMapping.reverse(); + if ( reversed != null ) { + List mappingsOfProperty = newMappings.get( reversed.getTargetName() ); + if ( mappingsOfProperty == null ) { + mappingsOfProperty = new ArrayList(); + newMappings.put( reversed.getTargetName(), mappingsOfProperty ); + } + mappingsOfProperty.add( reversed ); + } } } - newMappings.putAll( reverse( reverseMappings ) ); } if ( method.getMappings().isEmpty() ) { diff --git a/processor/src/main/java/org/mapstruct/ap/processor/MethodRetrievalProcessor.java b/processor/src/main/java/org/mapstruct/ap/processor/MethodRetrievalProcessor.java index 84083d49a..1df7de87e 100644 --- a/processor/src/main/java/org/mapstruct/ap/processor/MethodRetrievalProcessor.java +++ b/processor/src/main/java/org/mapstruct/ap/processor/MethodRetrievalProcessor.java @@ -22,7 +22,6 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; - import javax.annotation.processing.Messager; import javax.lang.model.element.Element; import javax.lang.model.element.ExecutableElement; @@ -121,8 +120,9 @@ public class MethodRetrievalProcessor implements ModelElementProcessortrue, iff the executable does not represent {@link java.lang.Object#equals(Object)} or an - * overridden version of it + * overridden version of it */ private boolean isNotObjectEquals(ExecutableElement executable) { if ( "equals".equals( executable.getSimpleName().toString() ) @@ -135,8 +135,9 @@ public class MethodRetrievalProcessor implements ModelElementProcessortrue, iff the given executable was not yet overridden by a method in the given list. */ private boolean wasNotYetOverridden(List methods, ExecutableElement executable) { @@ -412,7 +413,7 @@ public class MethodRetrievalProcessor implements ModelElementProcessor> getMappings(ExecutableElement method) { Map> mappings = new HashMap>(); @@ -421,12 +422,12 @@ public class MethodRetrievalProcessor implements ModelElementProcessor() ); + if ( !mappings.containsKey( mappingAnnotation.target() ) ) { + mappings.put( mappingAnnotation.target(), new ArrayList() ); } - Mapping mapping = Mapping.fromMappingPrism( mappingAnnotation, method, messager ); + Mapping mapping = Mapping.fromMappingPrism( mappingAnnotation, method, messager ); if ( mapping != null ) { - mappings.get( mappingAnnotation.source() ).add( mapping ); + mappings.get( mappingAnnotation.target() ).add( mapping ); } } diff --git a/processor/src/test/java/org/mapstruct/ap/test/erroneous/attributereference/ErroneousMappingsTest.java b/processor/src/test/java/org/mapstruct/ap/test/erroneous/attributereference/ErroneousMappingsTest.java index dff48112e..72a4b8b92 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/erroneous/attributereference/ErroneousMappingsTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/erroneous/attributereference/ErroneousMappingsTest.java @@ -55,6 +55,10 @@ public class ErroneousMappingsTest { kind = Kind.ERROR, line = 31, messageRegExp = "Unknown property \"bar\" in return type"), + @Diagnostic(type = ErroneousMapper.class, + kind = Kind.ERROR, + line = 34, + messageRegExp = "Target property \"foo\" must not be mapped more than once"), @Diagnostic(type = ErroneousMapper.class, kind = Kind.ERROR, line = 32, diff --git a/processor/src/test/java/org/mapstruct/ap/test/source/constants/SourceConstantsTest.java b/processor/src/test/java/org/mapstruct/ap/test/source/constants/SourceConstantsTest.java index 8a6140c2b..9d37d8f94 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/source/constants/SourceConstantsTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/source/constants/SourceConstantsTest.java @@ -99,7 +99,7 @@ public class SourceConstantsTest { @Diagnostic(type = ErroneousMapper1.class, kind = Kind.ERROR, line = 41, - messageRegExp = "Source and constant are both defined in Mapping, either define a source or a " + messageRegExp = "Source and constant are both defined in @Mapping, either define a source or a " + "constant"), @Diagnostic(type = ErroneousMapper1.class, kind = Kind.WARNING, @@ -124,7 +124,7 @@ public class SourceConstantsTest { @Diagnostic(type = ErroneousMapper3.class, kind = Kind.ERROR, line = 41, - messageRegExp = "Expression and constant are both defined in Mapping, either define an expression or a " + messageRegExp = "Expression and constant are both defined in @Mapping, either define an expression or a " + "constant"), @Diagnostic(type = ErroneousMapper3.class, kind = Kind.WARNING, @@ -149,7 +149,7 @@ public class SourceConstantsTest { @Diagnostic(type = ErroneousMapper4.class, kind = Kind.ERROR, line = 41, - messageRegExp = "Source and expression are both defined in Mapping, either define a source or an " + messageRegExp = "Source and expression are both defined in @Mapping, either define a source or an " + "expression"), @Diagnostic(type = ErroneousMapper4.class, kind = Kind.WARNING,