From 211c627c25a8f59fc92f192f49559ca4f4665cdf Mon Sep 17 00:00:00 2001 From: Filip Hrisafov Date: Fri, 24 May 2019 23:26:25 +0200 Subject: [PATCH] #1751 Fix handling of possible builder creation methods with generic signature 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 --- .../ap/spi/DefaultBuilderProvider.java | 9 ++++- .../mapstruct/ap/test/bugs/_1751/Holder.java | 35 ++++++++++++++++ .../ap/test/bugs/_1751/Issue1751Mapper.java | 24 +++++++++++ .../ap/test/bugs/_1751/Issue1751Test.java | 40 +++++++++++++++++++ .../mapstruct/ap/test/bugs/_1751/Source.java | 22 ++++++++++ .../mapstruct/ap/test/bugs/_1751/Target.java | 22 ++++++++++ 6 files changed, 151 insertions(+), 1 deletion(-) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1751/Holder.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1751/Issue1751Mapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1751/Issue1751Test.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1751/Source.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1751/Target.java diff --git a/processor/src/main/java/org/mapstruct/ap/spi/DefaultBuilderProvider.java b/processor/src/main/java/org/mapstruct/ap/spi/DefaultBuilderProvider.java index 9921ad2f0..b992e305b 100644 --- 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 @@ public class DefaultBuilderProvider implements BuilderProvider { 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 ( Holder build()) and the type element is (Holder) + // 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 new file mode 100644 index 000000000..cd35297d7 --- /dev/null +++ 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 { + + 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 duplicate() { + return new Holder<>( value ); + } + + // This method should not be considered as builder creation method + @SuppressWarnings("unused") + public static Holder 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 new file mode 100644 index 000000000..62fb66dc0 --- /dev/null +++ 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 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 new file mode 100644 index 000000000..5061ba598 --- /dev/null +++ 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 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 new file mode 100644 index 000000000..d69e688d7 --- /dev/null +++ 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 new file mode 100644 index 000000000..a6b2ca91b --- /dev/null +++ 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; + } +}