#337 Moving mergeWithInverseMappings() to SourceMethod; Improving some names; Adding some docs

processor/src/main/java/org/mapstruct/ap/model/source/Mapping.java

@@ -91,12 +91,6 @@ public class Mapping {
         return mappings;
     }
 
-    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) {
 
         if ( mappingPrism.target().isEmpty() ) {
@@ -154,6 +148,22 @@ public class Mapping {
         );
     }
 
+    private Mapping(String sourceName, String constant, String javaExpression, String targetName,
+                    String dateFormat, List<TypeMirror> qualifiers,
+                    boolean isIgnored, AnnotationMirror mirror,
+                    AnnotationValue sourceAnnotationValue, AnnotationValue targetAnnotationValue) {
+        this.sourceName = sourceName;
+        this.constant = constant;
+        this.javaExpression = javaExpression;
+        this.targetName = targetName;
+        this.dateFormat = dateFormat;
+        this.qualifiers = qualifiers;
+        this.isIgnored = isIgnored;
+        this.mirror = mirror;
+        this.sourceAnnotationValue = sourceAnnotationValue;
+        this.targetAnnotationValue = targetAnnotationValue;
+    }
+
     private static String getExpression(MappingPrism mappingPrism, ExecutableElement element, Messager messager) {
         if ( mappingPrism.expression().isEmpty() ) {
             return null;
@@ -175,20 +185,9 @@ public class Mapping {
         return javaExpressionMatcher.group( 1 ).trim();
     }
 
-    private Mapping(String sourceName, String constant, String javaExpression, String targetName,
+    private static boolean isEnumType(TypeMirror mirror) {
-                    String dateFormat, List<TypeMirror> qualifiers,
+        return mirror.getKind() == TypeKind.DECLARED &&
-                    boolean isIgnored, AnnotationMirror mirror,
+            ( (DeclaredType) mirror ).asElement().getKind() == ElementKind.ENUM;
-                    AnnotationValue sourceAnnotationValue, AnnotationValue targetAnnotationValue) {
-        this.sourceName = sourceName;
-        this.constant = constant;
-        this.javaExpression = javaExpression;
-        this.targetName = targetName;
-        this.dateFormat = dateFormat;
-        this.qualifiers = qualifiers;
-        this.isIgnored = isIgnored;
-        this.mirror = mirror;
-        this.sourceAnnotationValue = sourceAnnotationValue;
-        this.targetAnnotationValue = targetAnnotationValue;
     }
 
     public void init(SourceMethod method, Messager messager, TypeFactory typeFactory) {
@@ -254,20 +253,19 @@ public class Mapping {
     }
 
     private boolean hasPropertyInReverseMethod(String name, SourceMethod method) {
-        boolean match = false;
         for ( ExecutableElement getter : method.getResultType().getGetters() ) {
             if ( Executables.getPropertyName( getter ).equals( name ) ) {
-                match = true;
+                return true;
-                break;
             }
         }
+
         for ( ExecutableElement getter : method.getResultType().getAlternativeTargetAccessors() ) {
             if ( Executables.getPropertyName( getter ).equals( name ) ) {
-                match = true;
+                return true;
-                break;
             }
         }
-        return match;
+
+        return false;
     }
 
     public Mapping reverse(SourceMethod method, Messager messager, TypeFactory typeFactory) {
@@ -291,7 +289,7 @@ public class Mapping {
 
         // should generate error when parameter
         if ( sourceReference != null && sourceReference.getPropertyEntries().isEmpty() ) {
-            // parameter mapping only, apparantly the @InheritReverseConfiguration is intentional
+            // parameter mapping only, apparently the @InheritReverseConfiguration is intentional
             // but erroneous. Lets raise an error to warn.
             messager.printMessage(
                 Diagnostic.Kind.ERROR,

processor/src/main/java/org/mapstruct/ap/model/source/SourceMethod.java

@@ -20,10 +20,11 @@ package org.mapstruct.ap.model.source;
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import javax.annotation.processing.Messager;
 
+import javax.annotation.processing.Messager;
 import javax.lang.model.element.ExecutableElement;
 import javax.lang.model.element.Modifier;
 import javax.lang.model.util.Types;
@@ -47,20 +48,22 @@ import org.mapstruct.ap.util.Strings;
  */
 public class SourceMethod implements Method {
 
+    private final Types typeUtils;
+    private final TypeFactory typeFactory;
+    private final Messager messager;
+
     private final Type declaringMapper;
     private final ExecutableElement executable;
     private final List<Parameter> parameters;
     private final Parameter targetParameter;
     private final Type returnType;
     private final Accessibility accessibility;
-    private final Types typeUtils;
     private final List<Type> exceptionTypes;
 
     private Map<String, List<Mapping>> mappings;
     private IterableMapping iterableMapping;
     private MapMapping mapMapping;
 
-
     public static SourceMethod forMethodRequiringImplementation(ExecutableElement executable,
                                                                 List<Parameter> parameters,
                                                                 Type returnType,
@@ -80,7 +83,11 @@ public class SourceMethod implements Method {
             mappings,
             iterableMapping,
             mapMapping,
-            typeUtils );
+            typeUtils,
+            typeFactory,
+            messager
+        );
+
         for ( Map.Entry<String, List<Mapping>> entry : sourceMethod.getMappings().entrySet() ) {
             for ( Mapping mapping : entry.getValue() ) {
                 mapping.init( sourceMethod, messager, typeFactory );
@@ -102,7 +109,9 @@ public class SourceMethod implements Method {
             Collections.<String, List<Mapping>>emptyMap(),
             null,
             null,
-            typeUtils
+            typeUtils,
+            null,
+            null
         );
     }
 
@@ -118,13 +127,17 @@ public class SourceMethod implements Method {
             Collections.<String, List<Mapping>>emptyMap(),
             null,
             null,
-            typeUtils
+            typeUtils,
+            null,
+            null
         );
     }
 
+    //CHECKSTYLE:OFF
     private SourceMethod(Type declaringMapper, ExecutableElement executable, List<Parameter> parameters,
                          Type returnType, List<Type> exceptionTypes, Map<String, List<Mapping>> mappings,
-                         IterableMapping iterableMapping, MapMapping mapMapping, Types typeUtils) {
+                         IterableMapping iterableMapping, MapMapping mapMapping, Types typeUtils,
+                         TypeFactory typeFactory, Messager messager) {
         this.declaringMapper = declaringMapper;
         this.executable = executable;
         this.parameters = parameters;
@@ -138,7 +151,10 @@ public class SourceMethod implements Method {
         this.targetParameter = determineTargetParameter( parameters );
 
         this.typeUtils = typeUtils;
+        this.typeFactory = typeFactory;
+        this.messager = messager;
     }
+    //CHECKSTYLE:ON
 
     private Parameter determineTargetParameter(Iterable<Parameter> parameters) {
         for ( Parameter parameter : parameters ) {
@@ -306,7 +322,9 @@ public class SourceMethod implements Method {
 
     /**
      * Returns the {@link Mapping}s for the given source property.
+     *
      * @param sourcePropertyName the source property name
+     *
      * @return list of mappings
      */
     public List<Mapping> getMappingBySourcePropertyName(String sourcePropertyName) {
@@ -367,7 +385,7 @@ public class SourceMethod implements Method {
     /**
      * @param parameters the parameter list to check
      *
-     * @return <code>true</code>, iff the parameter list contains a parameter annotated with {@code @TargetType}
+     * @return {@code true} if the parameter list contains a parameter annotated with {@code @TargetType}
      */
     public static boolean containsTargetTypeParameter(List<Parameter> parameters) {
         for ( Parameter param : parameters ) {
@@ -383,4 +401,40 @@ public class SourceMethod implements Method {
     public List<Type> getThrownTypes() {
         return exceptionTypes;
     }
+
+    /**
+     * Merges in all the mappings configured via the given inverse mapping method, giving the locally defined mappings
+     * precedence.
+     */
+    public void mergeWithInverseMappings(SourceMethod inverseMethod) {
+        Map<String, List<Mapping>> newMappings = new HashMap<String, List<Mapping>>();
+
+        if ( inverseMethod != null && !inverseMethod.getMappings().isEmpty() ) {
+            for ( List<Mapping> mappings : inverseMethod.getMappings().values() ) {
+                for ( Mapping inverseMapping : mappings ) {
+                    Mapping reversed = inverseMapping.reverse( this, messager, typeFactory );
+                    if ( reversed != null ) {
+                        List<Mapping> mappingsOfProperty = newMappings.get( reversed.getTargetName() );
+                        if ( mappingsOfProperty == null ) {
+                            mappingsOfProperty = new ArrayList<Mapping>();
+                            newMappings.put( reversed.getTargetName(), mappingsOfProperty );
+                        }
+                        mappingsOfProperty.add( reversed );
+                    }
+                }
+            }
+        }
+
+        if ( getMappings().isEmpty() ) {
+            // the mapping method is configuredByReverseMappingMethod, see SourceMethod#setMappings()
+            setMappings( newMappings );
+        }
+        else {
+            // now add all of its own mappings
+            newMappings.putAll( getMappings() );
+            getMappings().clear();
+            // the mapping method is NOT configuredByReverseMappingMethod,
+            getMappings().putAll( newMappings );
+        }
+    }
 }

processor/src/main/java/org/mapstruct/ap/model/source/SourceReference.java

@@ -21,6 +21,7 @@ package org.mapstruct.ap.model.source;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+
 import javax.annotation.processing.Messager;
 import javax.lang.model.element.ExecutableElement;
 import javax.tools.Diagnostic;
@@ -38,8 +39,6 @@ import org.mapstruct.ap.util.Strings;
  * mapping method:
  *
  * {@code
- *
- * @author Sjaak Derksen
  * @Mapping( source = "in.propA.propB" target = "propC" )
  * TypeB mappingMethod ( TypeA in );
  * }
@@ -52,6 +51,8 @@ import org.mapstruct.ap.util.Strings;
  * </ol>
  *
  * After building, {@link #isValid()} will return true when when no problems are detected during building.
+ *
+ * @author Sjaak Derksen
  */
 public class SourceReference {
 

processor/src/main/java/org/mapstruct/ap/processor/MapperCreationProcessor.java

@@ -19,10 +19,8 @@
 package org.mapstruct.ap.processor;
 
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.List;
-import java.util.Map;
 import java.util.SortedSet;
 import java.util.TreeSet;
 import javax.annotation.processing.Messager;
@@ -49,7 +47,6 @@ import org.mapstruct.ap.model.MappingBuilderContext;
 import org.mapstruct.ap.model.MappingMethod;
 import org.mapstruct.ap.model.common.Type;
 import org.mapstruct.ap.model.common.TypeFactory;
-import org.mapstruct.ap.model.source.Mapping;
 import org.mapstruct.ap.model.source.SourceMethod;
 import org.mapstruct.ap.option.Options;
 import org.mapstruct.ap.prism.DecoratedWithPrism;
@@ -249,15 +246,15 @@ public class MapperCreationProcessor implements ModelElementProcessor<List<Sourc
                 continue;
             }
 
-            SourceMethod reverseMappingMethod = getReverseMappingMethod( methods, method );
+            SourceMethod inverseMappingMethod = getInverseMappingMethod( methods, method );
 
             boolean hasFactoryMethod = false;
             if ( method.isIterableMapping() ) {
 
                 IterableMappingMethod.Builder builder = new IterableMappingMethod.Builder();
-                if ( method.getIterableMapping() == null && reverseMappingMethod != null &&
+                if ( method.getIterableMapping() == null && inverseMappingMethod != null &&
-                    reverseMappingMethod.getIterableMapping() != null ) {
+                    inverseMappingMethod.getIterableMapping() != null ) {
-                    method.setIterableMapping( reverseMappingMethod.getIterableMapping() );
+                    method.setIterableMapping( inverseMappingMethod.getIterableMapping() );
                 }
 
                 String dateFormat = null;
@@ -281,9 +278,9 @@ public class MapperCreationProcessor implements ModelElementProcessor<List<Sourc
 
                 MapMappingMethod.Builder builder = new MapMappingMethod.Builder();
 
-                if ( method.getMapMapping() == null && reverseMappingMethod != null &&
+                if ( method.getMapMapping() == null && inverseMappingMethod != null &&
-                    reverseMappingMethod.getMapMapping() != null ) {
+                    inverseMappingMethod.getMapMapping() != null ) {
-                    method.setMapMapping( reverseMappingMethod.getMapMapping() );
+                    method.setMapMapping( inverseMappingMethod.getMapMapping() );
                 }
                 String keyDateFormat = null;
                 String valueDateFormat = null;
@@ -311,7 +308,7 @@ public class MapperCreationProcessor implements ModelElementProcessor<List<Sourc
             else if ( method.isEnumMapping() ) {
 
                 EnumMappingMethod.Builder builder = new EnumMappingMethod.Builder();
-                mergeWithReverseMappings( reverseMappingMethod, method );
+                method.mergeWithInverseMappings( inverseMappingMethod );
                 MappingMethod enumMappingMethod = builder
                     .mappingContext( mappingContext )
                     .souceMethod( method )
@@ -324,7 +321,7 @@ public class MapperCreationProcessor implements ModelElementProcessor<List<Sourc
             else {
 
                 BeanMappingMethod.Builder builder = new BeanMappingMethod.Builder();
-                mergeWithReverseMappings( reverseMappingMethod, method );
+                method.mergeWithInverseMappings( inverseMappingMethod );
                 BeanMappingMethod beanMappingMethod = builder
                     .mappingContext( mappingContext )
                     .souceMethod( method )
@@ -361,39 +358,12 @@ public class MapperCreationProcessor implements ModelElementProcessor<List<Sourc
         }
     }
 
-    private void mergeWithReverseMappings(SourceMethod forwardMappingMethod, SourceMethod method) {
+    /**
-        Map<String, List<Mapping>> newMappings = new HashMap<String, List<Mapping>>();
+     * Returns the configuring inverse method in case the given method is annotated with
-
+     * {@code @InheritInverseConfiguration} and exactly one such configuring method can unambiguously be selected (as
-        if ( forwardMappingMethod != null && !forwardMappingMethod.getMappings().isEmpty() ) {
+     * per the source/target type and optionally the name given via {@code @InheritInverseConfiguration}).
-            for ( List<Mapping> mappings : forwardMappingMethod.getMappings().values() ) {
+     */
-                for ( Mapping forwardMapping : mappings ) {
+    private SourceMethod getInverseMappingMethod(List<SourceMethod> rawMethods, SourceMethod method) {
-                    Mapping reversed = forwardMapping.reverse( method, messager, typeFactory );
-                    if ( reversed != null ) {
-                        List<Mapping> mappingsOfProperty = newMappings.get( reversed.getTargetName() );
-                        if ( mappingsOfProperty == null ) {
-                            mappingsOfProperty = new ArrayList<Mapping>();
-                            newMappings.put( reversed.getTargetName(), mappingsOfProperty );
-                        }
-                        mappingsOfProperty.add( reversed );
-                    }
-                }
-            }
-        }
-
-        if ( method.getMappings().isEmpty() ) {
-            // the mapping method is configuredByReverseMappingMethod, see SourceMethod#setMappings()
-            method.setMappings( newMappings );
-        }
-        else {
-            // now add all of its own mappings
-            newMappings.putAll( method.getMappings() );
-            method.getMappings().clear();
-            // the mapping method is NOT configuredByReverseMappingMethod,
-            method.getMappings().putAll( newMappings );
-        }
-    }
-
-    private SourceMethod getReverseMappingMethod(List<SourceMethod> rawMethods, SourceMethod method) {
         SourceMethod result = null;
         InheritInverseConfigurationPrism reversePrism = InheritInverseConfigurationPrism.getInstanceOn(
             method.getExecutable()