mapstruct#1103 Fix issue with tree automapping

processor/src/main/java/org/mapstruct/ap/internal/model/AbstractBaseBuilder.java

@@ -56,12 +56,23 @@ public B method(Method sourceMethod) {
      * @return See above
      */
     Assignment createForgedBeanAssignment(SourceRHS sourceRHS, ForgedMethod forgedMethod) {
+
+        if ( ctx.getForgedMethodsUnderCreation().containsKey( forgedMethod ) ) {
+            return createAssignment( sourceRHS, ctx.getForgedMethodsUnderCreation().get( forgedMethod ) );
+        }
+        else {
+            ctx.getForgedMethodsUnderCreation().put( forgedMethod, forgedMethod );
+        }
+
+
         BeanMappingMethod forgedMappingMethod = new BeanMappingMethod.Builder()
             .forgedMethod( forgedMethod )
             .mappingContext( ctx )
             .build();
 
-        return createForgedAssignment( sourceRHS, forgedMethod, forgedMappingMethod );
+        Assignment forgedAssignment = createForgedAssignment( sourceRHS, forgedMethod, forgedMappingMethod );
+        ctx.getForgedMethodsUnderCreation().remove( forgedMethod );
+        return forgedAssignment;
     }
 
     Assignment createForgedAssignment(SourceRHS source, ForgedMethod methodRef, MappingMethod mappingMethod) {
@@ -76,9 +87,14 @@ Assignment createForgedAssignment(SourceRHS source, ForgedMethod methodRef, Mapp
             methodRef = new ForgedMethod( existingName, methodRef );
         }
 
+        return createAssignment( source, methodRef );
+    }
+
+    private Assignment createAssignment(SourceRHS source, ForgedMethod methodRef) {
         Assignment assignment = MethodReference.forForgedMethod(
             methodRef,
-            ParameterBinding.fromParameters( methodRef.getParameters() ) );
+            ParameterBinding.fromParameters( methodRef.getParameters() )
+        );
         assignment.setAssignment( source );
 
         return assignment;

processor/src/main/java/org/mapstruct/ap/internal/model/MappingBuilderContext.java

@@ -19,8 +19,9 @@
 package org.mapstruct.ap.internal.model;
 
 import java.util.ArrayList;
-import java.util.HashSet;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 import javax.lang.model.element.TypeElement;
@@ -122,7 +123,8 @@ Assignment getTargetAssignment(Method mappingMethod, Type targetType, String tar
     private final List<MapperReference> mapperReferences;
     private final MappingResolver mappingResolver;
     private final List<MappingMethod> mappingsToGenerate = new ArrayList<MappingMethod>();
-    private Set<ForgedMethod> forgedMethods = new HashSet<ForgedMethod>(  );
+    private final Map<ForgedMethod, ForgedMethod> forgedMethodsUnderCreation =
+        new HashMap<ForgedMethod, ForgedMethod>(  );
 
     public MappingBuilderContext(TypeFactory typeFactory,
                           Elements elementUtils,
@@ -144,8 +146,17 @@ public MappingBuilderContext(TypeFactory typeFactory,
         this.mapperReferences = mapperReferences;
     }
 
-    public Set<ForgedMethod> getForgedMethods() {
-        return forgedMethods;
+    /**
+     * Returns a map which is used to track which forged methods are under creation.
+     * Used for cutting the possible infinite recursion of forged method creation.
+     *
+     * Map is used instead of set because not all fields of ForgedMethods are used in equals/hashCode and we are
+     * interested only in the first created ForgedMethod
+     *
+     * @return map of forged methods
+     */
+    public Map<ForgedMethod, ForgedMethod> getForgedMethodsUnderCreation() {
+        return forgedMethodsUnderCreation;
     }
 
     public TypeElement getMapperTypeElement() {

processor/src/main/java/org/mapstruct/ap/internal/model/PropertyMapping.java

@@ -18,17 +18,11 @@
  */
 package org.mapstruct.ap.internal.model;
 
-import static org.mapstruct.ap.internal.model.assignment.Assignment.AssignmentType.DIRECT;
-import static org.mapstruct.ap.internal.prism.NullValueCheckStrategyPrism.ALWAYS;
-import static org.mapstruct.ap.internal.util.Collections.first;
-import static org.mapstruct.ap.internal.util.Collections.last;
-
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Set;
-
 import javax.lang.model.element.ExecutableElement;
 import javax.lang.model.type.DeclaredType;
 
@@ -42,7 +36,6 @@
 import org.mapstruct.ap.internal.model.assignment.UpdateWrapper;
 import org.mapstruct.ap.internal.model.common.ModelElement;
 import org.mapstruct.ap.internal.model.common.Parameter;
-import org.mapstruct.ap.internal.model.common.ParameterBinding;
 import org.mapstruct.ap.internal.model.common.Type;
 import org.mapstruct.ap.internal.model.source.ForgedMethod;
 import org.mapstruct.ap.internal.model.source.ForgedMethodHistory;
@@ -60,6 +53,11 @@
 import org.mapstruct.ap.internal.util.ValueProvider;
 import org.mapstruct.ap.internal.util.accessor.Accessor;
 
+import static org.mapstruct.ap.internal.model.assignment.Assignment.AssignmentType.DIRECT;
+import static org.mapstruct.ap.internal.prism.NullValueCheckStrategyPrism.ALWAYS;
+import static org.mapstruct.ap.internal.util.Collections.first;
+import static org.mapstruct.ap.internal.util.Collections.last;
+
 /**
  * Represents the mapping between a source and target property, e.g. from {@code String Source#foo} to
  * {@code int Target#bar}. Name and type of source and target property can differ. If they have different types, the
@@ -655,14 +653,6 @@ private Assignment forgeMapping(SourceRHS sourceRHS) {
                 forgedNamedBased
             );
 
-            if (ctx.getForgedMethods().contains( forgedMethod )) {
-                Assignment assignment = MethodReference.forForgedMethod(
-                    forgedMethod,
-                    ParameterBinding.fromParameters( forgedMethod.getParameters() ) );
-                assignment.setAssignment( sourceRHS );
-                return assignment;
-            }
-            ctx.getForgedMethods().add( forgedMethod );
             return createForgedBeanAssignment( sourceRHS, forgedMethod );
         }
 

processor/src/main/java/org/mapstruct/ap/internal/model/common/Parameter.java

@@ -95,25 +95,28 @@ public boolean isMappingContext() {
     }
 
     @Override
-    public int hashCode() {
-        int hash = 5;
-        hash = 23 * hash + (this.name != null ? this.name.hashCode() : 0);
-        return hash;
-    }
-
-    @Override
-    public boolean equals(Object obj) {
-        if ( obj == null ) {
-            return false;
+    public boolean equals(Object o) {
+        if ( this == o ) {
+            return true;
         }
-        if ( getClass() != obj.getClass() ) {
+        if ( o == null || getClass() != o.getClass() ) {
             return false;
         }
-        final Parameter other = (Parameter) obj;
-        if ( (this.name == null) ? (other.name != null) : !this.name.equals( other.name ) ) {
+
+        Parameter parameter = (Parameter) o;
+
+        if ( name != null ? !name.equals( parameter.name ) : parameter.name != null ) {
             return false;
         }
-        return true;
+        return type != null ? type.equals( parameter.type ) : parameter.type == null;
+
+    }
+
+    @Override
+    public int hashCode() {
+        int result = name != null ? name.hashCode() : 0;
+        result = 31 * result + ( type != null ? type.hashCode() : 0 );
+        return result;
     }
 
     public static Parameter forElementAndType(VariableElement element, Type parameterType) {

processor/src/main/java/org/mapstruct/ap/internal/model/source/ForgedMethod.java

@@ -335,18 +335,14 @@ public boolean equals(Object o) {
         if ( parameters != null ? !parameters.equals( that.parameters ) : that.parameters != null ) {
             return false;
         }
-        if ( returnType != null ? !returnType.equals( that.returnType ) : that.returnType != null ) {
-            return false;
-        }
-        return name != null ? name.equals( that.name ) : that.name == null;
+        return returnType != null ? returnType.equals( that.returnType ) : that.returnType == null;
 
     }
 
     @Override
     public int hashCode() {
         int result = parameters != null ? parameters.hashCode() : 0;
         result = 31 * result + ( returnType != null ? returnType.hashCode() : 0 );
-        result = 31 * result + ( name != null ? name.hashCode() : 0 );
         return result;
     }
 }

processor/src/main/java/org/mapstruct/ap/internal/model/source/FormattingParameters.java

@@ -40,4 +40,28 @@ public String getNumber() {
         return number;
     }
 
+    @Override
+    public boolean equals(Object o) {
+        if ( this == o ) {
+            return true;
+        }
+        if ( o == null || getClass() != o.getClass() ) {
+            return false;
+        }
+
+        FormattingParameters that = (FormattingParameters) o;
+
+        if ( date != null ? !date.equals( that.date ) : that.date != null ) {
+            return false;
+        }
+        return number != null ? number.equals( that.number ) : that.number == null;
+
+    }
+
+    @Override
+    public int hashCode() {
+        int result = date != null ? date.hashCode() : 0;
+        result = 31 * result + ( number != null ? number.hashCode() : 0 );
+        return result;
+    }
 }

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

@@ -168,6 +168,11 @@ private Mapper getMapper(TypeElement element, MapperConfiguration mapperConfig,
             .implPackage( mapperConfig.implementationPackage() )
             .build();
 
+        if ( !mappingContext.getForgedMethodsUnderCreation().isEmpty() ) {
+            messager.printMessage( element, Message.GENERAL_NOT_ALL_FORGED_CREATED,
+                mappingContext.getForgedMethodsUnderCreation().keySet() );
+        }
+
         return mapper;
     }
 

processor/src/main/java/org/mapstruct/ap/internal/util/Message.java

@@ -84,6 +84,7 @@ public enum Message {
     GENERAL_UNSUPPORTED_DATE_FORMAT_CHECK( "No dateFormat check is supported for types %s, %s" ),
     GENERAL_VALID_DATE( "Given date format \"%s\" is valid.", Diagnostic.Kind.NOTE ),
     GENERAL_INVALID_DATE( "Given date format \"%s\" is invalid. Message: \"%s\"." ),
+    GENERAL_NOT_ALL_FORGED_CREATED( "Internal Error in e creation of Forged Methods, it was expected all Forged Methods to finished with creation, but %s did not" ),
 
     RETRIEVAL_NO_INPUT_ARGS( "Can't generate mapping method with no input arguments." ),
     RETRIEVAL_DUPLICATE_MAPPING_TARGETS( "Can't generate mapping method with more than one @MappingTarget parameter." ),
@@ -100,8 +101,8 @@ public enum Message {
     RETRIEVAL_TYPE_VAR_RESULT( "Can't generate mapping method for a generic type variable target." ),
     RETRIEVAL_WILDCARD_SUPER_BOUND_SOURCE( "Can't generate mapping method for a wildcard super bound source." ),
     RETRIEVAL_WILDCARD_EXTENDS_BOUND_RESULT( "Can't generate mapping method for a wildcard extends bound result." ),
-    RETRIEVAL_CONTEXT_PARAMS_WITH_SAME_TYPE( "The types of @Context parameters must be unique." ),
 
+    RETRIEVAL_CONTEXT_PARAMS_WITH_SAME_TYPE( "The types of @Context parameters must be unique." ),
     INHERITCONFIGURATION_BOTH( "Method cannot be annotated with both a @InheritConfiguration and @InheritInverseConfiguration." ),
     INHERITINVERSECONFIGURATION_DUPLICATES( "Several matching inverse methods exist: %s(). Specify a name explicitly." ),
     INHERITINVERSECONFIGURATION_INVALID_NAME( "None of the candidates %s() matches given name: \"%s\"." ),
@@ -114,8 +115,8 @@ public enum Message {
     INHERITCONFIGURATION_NO_NAME_MATCH( "Given name \"%s\" does not match the only candidate. Did you mean: \"%s\"." ),
     INHERITCONFIGURATION_MULTIPLE_PROTOTYPE_METHODS_MATCH( "More than one configuration prototype method is applicable. Use @InheritConfiguration to select one of them explicitly: %s." ),
     INHERITINVERSECONFIGURATION_MULTIPLE_PROTOTYPE_METHODS_MATCH( "More than one configuration prototype method is applicable. Use @InheritInverseConfiguration to select one of them explicitly: %s." ),
-    INHERITCONFIGURATION_CYCLE( "Cycle detected while evaluating inherited configurations. Inheritance path: %s" ),
 
+    INHERITCONFIGURATION_CYCLE( "Cycle detected while evaluating inherited configurations. Inheritance path: %s" ),
     VALUEMAPPING_DUPLICATE_SOURCE( "Source value mapping: \"%s\" cannot be mapped more than once." ),
     VALUEMAPPING_ANY_AREADY_DEFINED( "Source = \"<ANY_REMAINING>\" or \"<ANY_UNMAPPED>\" can only be used once." ),
     VALUE_MAPPING_UNMAPPED_SOURCES( "The following constants from the source enum have no corresponding constant in the target enum and must be be mapped via adding additional mappings: %s." ),

processor/src/test/java/org/mapstruct/ap/test/nestedbeans/RecursionTest.java

@@ -0,0 +1,88 @@
+/**
+ *  Copyright 2012-2017 Gunnar Morling (http://www.gunnarmorling.de/)
+ *  and/or other contributors as indicated by the @authors tag. See the
+ *  copyright.txt file in the distribution for a full listing of all
+ *  contributors.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.mapstruct.ap.test.nestedbeans;
+
+import java.util.Collections;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mapstruct.ap.test.nestedbeans.recursive.RecursionMapper;
+import org.mapstruct.ap.test.nestedbeans.recursive.TreeRecursionMapper;
+import org.mapstruct.ap.testutil.IssueKey;
+import org.mapstruct.ap.testutil.WithClasses;
+import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner;
+
+@RunWith(AnnotationProcessorTestRunner.class)
+public class RecursionTest {
+
+    @WithClasses({
+        RecursionMapper.class
+    })
+    @Test
+    @IssueKey("1103")
+    public void testRecursiveAutoMap() {
+        RecursionMapper.Root root = RecursionMapper.INSTANCE.mapRoot(
+            new RecursionMapper.RootDto(
+                new RecursionMapper.ChildDto( "Sub Root", new RecursionMapper.ChildDto(
+                    "Sub child", null
+                ) )
+            ) );
+
+        Assert.assertEquals(
+            new RecursionMapper.Root(
+                new RecursionMapper.Child( "Sub Root", new RecursionMapper.Child(
+                    "Sub child", null
+                ) )
+            ),
+            root
+        );
+    }
+
+    @WithClasses({
+        TreeRecursionMapper.class
+    })
+    @Test
+    @IssueKey("1103")
+    public void testRecursiveAutoMap2() {
+        TreeRecursionMapper.Root root = TreeRecursionMapper.INSTANCE.mapRoot(
+            new TreeRecursionMapper.RootDto(
+                Collections.singletonList( new TreeRecursionMapper.ChildDto(
+                    "Sub Root",
+                    Collections.singletonList( new TreeRecursionMapper.ChildDto(
+                        "Sub child", null
+                    ) )
+                ) )
+            ) );
+
+        Assert.assertEquals(
+            new TreeRecursionMapper.Root(
+                Collections.singletonList(
+                    new TreeRecursionMapper.Child(
+                        "Sub Root",
+                        Collections.singletonList( new TreeRecursionMapper.Child(
+                            "Sub child", null
+                        ) )
+                    ) )
+            ),
+            root
+        );
+    }
+
+}