From 60c159a0a1150882a7279cb90e72ecef8bad259a 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 f5656e2e48..f89e99cd17 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 @@ 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 ( 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 0000000000..cd35297d73 --- /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 0000000000..62fb66dc0b --- /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 0000000000..5061ba598f --- /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 0000000000..d69e688d79 --- /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 0000000000..a6b2ca91b5 --- /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; + } +}