mapstruct#1751 Fix handling of possible builder creation methods with generic signature

filiphr · filiphr · commit 211c627c25a8 · 2019-09-22T10:08:01.000+02:00
When a method has a generic signature and the builder type is generic then the method return type does not match the builder type.
Therefore check only for raw types.
Add extra check for void method since a void method can't be a builder creation method
diff --git a/processor/src/main/java/org/mapstruct/ap/spi/DefaultBuilderProvider.java b/processor/src/main/java/org/mapstruct/ap/spi/DefaultBuilderProvider.java
@@ -204,7 +204,14 @@ protected boolean isPossibleBuilderCreationMethod(ExecutableElement method, Type
         return method.getParameters().isEmpty()
             && method.getModifiers().contains( Modifier.PUBLIC )
             && method.getModifiers().contains( Modifier.STATIC )
-            && !typeUtils.isSameType( method.getReturnType(), typeElement.asType() );
+            && method.getReturnType().getKind() != TypeKind.VOID
+            // Only compare raw elements
+            // Reason: if the method is a generic method (<T> Holder<T> build()) and the type element is (Holder<T>)
+            // then the return type of the method does not match the type of the type element
+            && !typeUtils.isSameType(
+            typeUtils.erasure( method.getReturnType() ),
+            typeUtils.erasure( typeElement.asType() )
+        );
     }
 
     /**
diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1751/Holder.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1751/Holder.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._1751;
+
+/**
+ * @author Filip Hrisafov
+ */
+public class Holder<T> {
+
+    private final T value;
+
+    public Holder(T value) {
+        this.value = value;
+    }
+
+    public T getValue() {
+        return value;
+    }
+
+    // If empty is considered as a builder creation method, this method would be the build method and would
+    // lead to a stackoverflow
+    @SuppressWarnings("unused")
+    public Holder<T> duplicate() {
+        return new Holder<>( value );
+    }
+
+    // This method should not be considered as builder creation method
+    @SuppressWarnings("unused")
+    public static <V> Holder<V> empty() {
+        return new Holder<>( null );
+    }
+}
diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1751/Issue1751Mapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1751/Issue1751Mapper.java
@@ -0,0 +1,24 @@
+/*
+ * 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._1751;
+
+import org.mapstruct.Mapper;
+import org.mapstruct.factory.Mappers;
+
+/**
+ * @author Filip Hrisafov
+ */
+@Mapper
+public interface Issue1751Mapper {
+
+    Issue1751Mapper INSTANCE = Mappers.getMapper( Issue1751Mapper.class );
+
+    Target map(Source source);
+
+    default Holder<Target> mapToHolder(Source source) {
+        return new Holder<>( this.map( source ) );
+    }
+}
diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1751/Issue1751Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1751/Issue1751Test.java
@@ -0,0 +1,40 @@
+/*
+ * 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._1751;
+
+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;
+
+/**
+ * @author Filip Hrisafov
+ */
+@IssueKey("1772")
+@RunWith(AnnotationProcessorTestRunner.class)
+@WithClasses({
+    Holder.class,
+    Issue1751Mapper.class,
+    Source.class,
+    Target.class
+})
+public class Issue1751Test {
+
+    @Test
+    public void name() {
+        Source source = new Source();
+        source.setValue( "some value" );
+
+        Holder<Target> targetHolder = Issue1751Mapper.INSTANCE.mapToHolder( source );
+
+        assertThat( targetHolder.getValue() )
+            .extracting( Target::getValue )
+            .isEqualTo( "some value" );
+    }
+}
diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1751/Source.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1751/Source.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._1751;
+
+/**
+ * @author Filip Hrisafov
+ */
+public class Source {
+
+    private String value;
+
+    public String getValue() {
+        return value;
+    }
+
+    public void setValue(String value) {
+        this.value = value;
+    }
+}
diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1751/Target.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1751/Target.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._1751;
+
+/**
+ * @author Filip Hrisafov
+ */
+public class Target {
+
+    private String value;
+
+    public String getValue() {
+        return value;
+    }
+
+    public void setValue(String value) {
+        this.value = value;
+    }
+}
