#1719 strange error message for selecting collection update methods (#1724)

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

@@ -724,7 +724,13 @@ public class PropertyMapping extends ModelElement {
         }
 
         private Assignment forgeMapping(SourceRHS sourceRHS) {
-            Type sourceType = sourceRHS.getSourceType();
+            Type sourceType;
+            if ( targetWriteAccessorType == TargetWriteAccessorType.ADDER ) {
+                sourceType = sourceRHS.getSourceTypeForMatching();
+            }
+            else {
+                 sourceType = sourceRHS.getSourceType();
+            }
             if ( forgedNamedBased && !canGenerateAutoSubMappingBetween( sourceType, targetType ) ) {
                 return null;
             }
@@ -744,7 +750,8 @@ public class PropertyMapping extends ModelElement {
             // They should forge an update method only if we set the forceUpdateMethod. This is set to true,
             // 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 ) ) {
+            if ( !targetType.isEnumType() && ( method.isUpdateMethod() || forceUpdateMethod )
+                && targetWriteAccessorType != TargetWriteAccessorType.ADDER) {
                 parameters.add( Parameter.forForgedMappingTarget( targetType ) );
                 returnType = ctx.getTypeFactory().createVoidType();
             }

processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/Issue1719Mapper.java

@@ -0,0 +1,22 @@
+/*
+ * 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.bugs._1719;
+
+import org.mapstruct.CollectionMappingStrategy;
+import org.mapstruct.Mapper;
+import org.mapstruct.Mapping;
+import org.mapstruct.MappingTarget;
+import org.mapstruct.factory.Mappers;
+
+@Mapper(collectionMappingStrategy = CollectionMappingStrategy.ADDER_PREFERRED)
+public interface Issue1719Mapper {
+
+    Issue1719Mapper INSTANCE = Mappers.getMapper( Issue1719Mapper.class );
+
+    @Mapping(target = "targetElements", source = "sourceElements")
+    void map(Source source, @MappingTarget Target target);
+
+}

processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/Issue1719Test.java

@@ -0,0 +1,57 @@
+/*
+ * 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.bugs._1719;
+
+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 static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.tuple;
+
+@RunWith(AnnotationProcessorTestRunner.class)
+@IssueKey("1719")
+@WithClasses({
+    Source.class,
+    SourceElement.class,
+    Target.class,
+    TargetElement.class
+})
+public class Issue1719Test {
+
+    /**
+     * For adder methods MapStuct cannot generate an update method. MapStruct would cannot know how to remove objects
+     * from the child-parent relation. It cannot even assume that the the collection can be cleared at forehand.
+     * Therefore the only sensible choice is for MapStruct to create a create method for the target elements.
+     */
+    @Test
+    @WithClasses(Issue1719Mapper.class)
+    public void testShouldGiveNoErrorMessage() {
+        Source source = new Source();
+        source.getSourceElements().add( new SourceElement( 1, "jim" ) );
+        source.getSourceElements().add( new SourceElement( 2, "alice" ) );
+
+        Target target = new Target();
+        TargetElement bob = new TargetElement( 1, "bob" );
+        target.addTargetElement( bob );
+        TargetElement louise = new TargetElement( 3, "louise" );
+        target.addTargetElement( louise );
+
+        Issue1719Mapper.INSTANCE.map( source, target );
+
+        assertThat( target.getTargetElements() ).hasSize( 3 );
+        assertThat( target.getTargetElements() )
+            .extracting( TargetElement::getId, TargetElement::getName )
+            .containsOnly(
+                tuple( 1, "bob" ),
+                tuple( 2, "alice" ),
+                tuple( 3, "louise" )
+            );
+    }
+
+}

processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/Source.java

@@ -0,0 +1,23 @@
+/*
+ * 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.bugs._1719;
+
+import java.util.HashSet;
+import java.util.Set;
+
+public class Source {
+
+    private Set<SourceElement> sourceElements = new HashSet<>();
+
+    public Set<SourceElement> getSourceElements() {
+        return sourceElements;
+    }
+
+    public void setSourceElements(Set<SourceElement> sourceElements) {
+        this.sourceElements = sourceElements;
+    }
+
+}

processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/SourceElement.java

@@ -0,0 +1,52 @@
+/*
+ * 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.bugs._1719;
+
+import java.util.Objects;
+
+public class SourceElement {
+
+    private int id;
+    private String name;
+
+    public SourceElement(int id, String name) {
+        this.id = id;
+        this.name = name;
+    }
+
+    public int getId() {
+        return id;
+    }
+
+    public void setId(int id) {
+        this.id = id;
+    }
+
+    public String getName() {
+        return name;
+    }
+
+    public void setName(String name) {
+        this.name = name;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if ( this == o ) {
+            return true;
+        }
+        if ( o == null || getClass() != o.getClass() ) {
+            return false;
+        }
+        SourceElement that = (SourceElement) o;
+        return id == that.id;
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash( id );
+    }
+}

processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/Target.java

@@ -0,0 +1,35 @@
+/*
+ * 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.bugs._1719;
+
+import java.util.HashSet;
+import java.util.Set;
+
+public class Target {
+
+    private Set<TargetElement> targetElements = new HashSet<>();
+
+    public Set<TargetElement> getTargetElements() {
+        return targetElements;
+    }
+
+    public void setTargetElements(Set<TargetElement> targetElements) {
+        this.targetElements = targetElements;
+    }
+
+    public TargetElement addTargetElement(TargetElement element) {
+        element.updateTarget( this );
+        getTargetElements().add( element );
+        return element;
+    }
+
+    public TargetElement removeTargetElement(TargetElement element) {
+        element.updateTarget( null );
+        getTargetElements().remove( element );
+        return element;
+    }
+
+}

processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/TargetElement.java

@@ -0,0 +1,70 @@
+/*
+ * 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.bugs._1719;
+
+import java.util.Objects;
+
+public class TargetElement {
+
+    private int id;
+    private String name;
+    private Target target;
+
+    public TargetElement() {
+    }
+
+    public TargetElement(int id, String name) {
+        this.id = id;
+        this.name = name;
+    }
+
+    public int getId() {
+        return id;
+    }
+
+    public void setId(int id) {
+        this.id = id;
+    }
+
+    public String getName() {
+        return name;
+    }
+
+    public void setName(String name) {
+        this.name = name;
+    }
+
+    public Target getTarget() {
+        return target;
+    }
+
+    /**
+     * intentionally not a setter, to not further complicate this test case.
+     *
+     * @param target
+     */
+    public void updateTarget(Target target) {
+        this.target = target;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if ( this == o ) {
+            return true;
+        }
+        if ( o == null || getClass() != o.getClass() ) {
+            return false;
+        }
+        TargetElement that = (TargetElement) o;
+        return id == that.id;
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash( id );
+    }
+
+}