From ade4f4d7e2ab87b2e0f113221d3fc9a6729f78cc Mon Sep 17 00:00:00 2001 From: Sjaak Derksen Date: Sun, 22 Sep 2019 19:25:43 +0200 Subject: [PATCH] #1821 nullpointer due to @BeanMapping via inheritance (#1822) --- .../ap/internal/model/BeanMappingMethod.java | 7 +- .../ap/internal/model/source/BeanMapping.java | 175 ++++++++++++------ .../ap/internal/model/source/EnumMapping.java | 36 ---- .../model/source/SelectionParameters.java | 17 ++ .../processor/MethodRetrievalProcessor.java | 9 +- .../ap/test/bugs/_1821/Issue1821Mapper.java | 32 ++++ .../ap/test/bugs/_1821/Issue1821Test.java | 24 +++ 7 files changed, 199 insertions(+), 101 deletions(-) delete mode 100644 processor/src/main/java/org/mapstruct/ap/internal/model/source/EnumMapping.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1821/Issue1821Mapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1821/Issue1821Test.java diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java b/processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java index 43fdfbc80..d9103ea64 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java @@ -42,7 +42,6 @@ import org.mapstruct.ap.internal.model.source.MappingOptions; import org.mapstruct.ap.internal.model.source.Method; import org.mapstruct.ap.internal.model.source.SelectionParameters; import org.mapstruct.ap.internal.model.source.SourceMethod; -import org.mapstruct.ap.internal.prism.BeanMappingPrism; import org.mapstruct.ap.internal.prism.CollectionMappingStrategyPrism; import org.mapstruct.ap.internal.prism.NullValueMappingStrategyPrism; import org.mapstruct.ap.internal.prism.ReportingPolicyPrism; @@ -393,7 +392,7 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { if ( resultType.isAbstract() ) { ctx.getMessager().printMessage( method.getExecutable(), - BeanMappingPrism.getInstanceOn( method.getExecutable() ).mirror, + method.getMappingOptions().getBeanMapping().getMirror(), BEANMAPPING_ABSTRACT, resultType, method.getResultType() @@ -403,7 +402,7 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { else if ( !resultType.isAssignableTo( method.getResultType() ) ) { ctx.getMessager().printMessage( method.getExecutable(), - BeanMappingPrism.getInstanceOn( method.getExecutable() ).mirror, + method.getMappingOptions().getBeanMapping().getMirror(), BEANMAPPING_NOT_ASSIGNABLE, resultType, method.getResultType() @@ -413,7 +412,7 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { else if ( !resultType.hasEmptyAccessibleConstructor() ) { ctx.getMessager().printMessage( method.getExecutable(), - BeanMappingPrism.getInstanceOn( method.getExecutable() ).mirror, + method.getMappingOptions().getBeanMapping().getMirror(), Message.GENERAL_NO_SUITABLE_CONSTRUCTOR, resultType ); diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/BeanMapping.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/BeanMapping.java index fd96c14c2..47729dbe7 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/BeanMapping.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/BeanMapping.java @@ -6,11 +6,14 @@ package org.mapstruct.ap.internal.model.source; import java.util.List; +import java.util.Objects; import java.util.Optional; +import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.ExecutableElement; import javax.lang.model.type.TypeKind; import javax.lang.model.util.Types; +import org.mapstruct.ap.internal.model.common.TypeFactory; import org.mapstruct.ap.internal.prism.BeanMappingPrism; import org.mapstruct.ap.internal.prism.BuilderPrism; import org.mapstruct.ap.internal.prism.NullValueCheckStrategyPrism; @@ -36,87 +39,134 @@ public class BeanMapping { private final BuilderPrism builder; private final NullValuePropertyMappingStrategyPrism nullValuePropertyMappingStrategy; + private final AnnotationMirror mirror; + /** - * creates a mapping for inheritance. Will set ignoreByDefault to false. + * creates a mapping for inheritance. Will set + * - ignoreByDefault to false. + * - resultType to null * - * @param map - * @return + * @return new mapping */ - public static BeanMapping forInheritance( BeanMapping map ) { + public static BeanMapping forInheritance(BeanMapping beanMapping) { return new BeanMapping( - map.selectionParameters, - map.nullValueMappingStrategy, - map.nullValuePropertyMappingStrategy, - map.nullValueCheckStrategy, - map.reportingPolicy, + SelectionParameters.forInheritance( beanMapping.selectionParameters ), + beanMapping.nullValueMappingStrategy, + beanMapping.nullValuePropertyMappingStrategy, + beanMapping.nullValueCheckStrategy, + beanMapping.reportingPolicy, false, - map.ignoreUnmappedSourceProperties, - map.builder + beanMapping.ignoreUnmappedSourceProperties, + beanMapping.builder, + beanMapping.mirror ); } - public static BeanMapping fromPrism(BeanMappingPrism beanMapping, ExecutableElement method, - FormattingMessager messager, Types typeUtils) { + public static class Builder { - if ( beanMapping == null ) { - return null; + private BeanMappingPrism prism; + private ExecutableElement method; + private FormattingMessager messager; + private Types typeUtils; + private TypeFactory typeFactory; + + public Builder beanMappingPrism(BeanMappingPrism beanMappingPrism) { + this.prism = beanMappingPrism; + return this; } - boolean resultTypeIsDefined = !TypeKind.VOID.equals( beanMapping.resultType().getKind() ); - - NullValueMappingStrategyPrism nullValueMappingStrategy = - null == beanMapping.values.nullValueMappingStrategy() - ? null - : NullValueMappingStrategyPrism.valueOf( beanMapping.nullValueMappingStrategy() ); - - NullValuePropertyMappingStrategyPrism nullValuePropertyMappingStrategy = - null == beanMapping.values.nullValuePropertyMappingStrategy() - ? null - : NullValuePropertyMappingStrategyPrism.valueOf( beanMapping.nullValuePropertyMappingStrategy() ); - - NullValueCheckStrategyPrism nullValueCheckStrategy = - null == beanMapping.values.nullValueCheckStrategy() - ? null - : NullValueCheckStrategyPrism.valueOf( beanMapping.nullValueCheckStrategy() ); - - boolean ignoreByDefault = beanMapping.ignoreByDefault(); - BuilderPrism builderMapping = null; - if ( beanMapping.values.builder() != null ) { - builderMapping = beanMapping.builder(); + public Builder method(ExecutableElement method) { + this.method = method; + return this; } - if ( !resultTypeIsDefined && beanMapping.qualifiedBy().isEmpty() && beanMapping.qualifiedByName().isEmpty() - && beanMapping.ignoreUnmappedSourceProperties().isEmpty() - && ( nullValueMappingStrategy == null ) && ( nullValuePropertyMappingStrategy == null ) - && ( nullValueCheckStrategy == null ) && !ignoreByDefault && builderMapping == null ) { - - messager.printMessage( method, Message.BEANMAPPING_NO_ELEMENTS ); + public Builder messager(FormattingMessager messager) { + this.messager = messager; + return this; } - SelectionParameters cmp = new SelectionParameters( - beanMapping.qualifiedBy(), - beanMapping.qualifiedByName(), - resultTypeIsDefined ? beanMapping.resultType() : null, - typeUtils - ); + public Builder typeUtils(Types typeUtils) { + this.typeUtils = typeUtils; + return this; + } - //TODO Do we want to add the reporting policy to the BeanMapping as well? To give more granular support? - return new BeanMapping( - cmp, - nullValueMappingStrategy, - nullValuePropertyMappingStrategy, - nullValueCheckStrategy, - null, - ignoreByDefault, - beanMapping.ignoreUnmappedSourceProperties(), - builderMapping - ); + public Builder typeFactory(TypeFactory typeFactory) { + this.typeFactory = typeFactory; + return this; + } + + public BeanMapping build() { + + if ( prism == null ) { + return null; + } + + Objects.requireNonNull( method ); + Objects.requireNonNull( messager ); + Objects.requireNonNull( method ); + Objects.requireNonNull( typeUtils ); + Objects.requireNonNull( typeFactory ); + + boolean resultTypeIsDefined = !TypeKind.VOID.equals( prism.resultType().getKind() ); + + NullValueMappingStrategyPrism nullValueMappingStrategy = + null == prism.values.nullValueMappingStrategy() + ? null + : NullValueMappingStrategyPrism.valueOf( prism.nullValueMappingStrategy() ); + + NullValuePropertyMappingStrategyPrism nullValuePropertyMappingStrategy = + null == prism.values.nullValuePropertyMappingStrategy() + ? null + : + NullValuePropertyMappingStrategyPrism.valueOf( prism.nullValuePropertyMappingStrategy() ); + + NullValueCheckStrategyPrism nullValueCheckStrategy = + null == prism.values.nullValueCheckStrategy() + ? null + : NullValueCheckStrategyPrism.valueOf( prism.nullValueCheckStrategy() ); + + boolean ignoreByDefault = prism.ignoreByDefault(); + BuilderPrism builderMapping = null; + if ( prism.values.builder() != null ) { + builderMapping = prism.builder(); + } + + if ( !resultTypeIsDefined && prism.qualifiedBy().isEmpty() && + prism.qualifiedByName().isEmpty() + && prism.ignoreUnmappedSourceProperties().isEmpty() + && ( nullValueMappingStrategy == null ) && ( nullValuePropertyMappingStrategy == null ) + && ( nullValueCheckStrategy == null ) && !ignoreByDefault && builderMapping == null ) { + + messager.printMessage( method, Message.BEANMAPPING_NO_ELEMENTS ); + } + + SelectionParameters cmp = new SelectionParameters( + prism.qualifiedBy(), + prism.qualifiedByName(), + resultTypeIsDefined ? prism.resultType() : null, + typeUtils + ); + + //TODO Do we want to add the reporting policy to the BeanMapping as well? To give more granular support? + return new BeanMapping( + cmp, + nullValueMappingStrategy, + nullValuePropertyMappingStrategy, + nullValueCheckStrategy, + null, + ignoreByDefault, + prism.ignoreUnmappedSourceProperties(), + builderMapping, + prism.mirror + ); + } } private BeanMapping(SelectionParameters selectionParameters, NullValueMappingStrategyPrism nvms, NullValuePropertyMappingStrategyPrism nvpms, NullValueCheckStrategyPrism nvcs, ReportingPolicyPrism reportingPolicy, boolean ignoreByDefault, - List ignoreUnmappedSourceProperties, BuilderPrism builder) { + List ignoreUnmappedSourceProperties, BuilderPrism builder, + AnnotationMirror mirror) { this.selectionParameters = selectionParameters; this.nullValueMappingStrategy = nvms; this.nullValuePropertyMappingStrategy = nvpms; @@ -125,6 +175,7 @@ public class BeanMapping { this.ignoreByDefault = ignoreByDefault; this.ignoreUnmappedSourceProperties = ignoreUnmappedSourceProperties; this.builder = builder; + this.mirror = mirror; } public SelectionParameters getSelectionParameters() { @@ -155,6 +206,10 @@ public class BeanMapping { return ignoreUnmappedSourceProperties; } + public AnnotationMirror getMirror() { + return mirror; + } + /** * derives the builder prism given the options and configuration * @param method containing mandatory configuration and the mapping options (optionally containing a beanmapping) diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/EnumMapping.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/EnumMapping.java deleted file mode 100644 index 2c13344d2..000000000 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/EnumMapping.java +++ /dev/null @@ -1,36 +0,0 @@ -/* - * 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.internal.model.source; - -/** - * Represents the mapping between one enum constant and another. - * - * @author Gunnar Morling - */ -public class EnumMapping { - - private final String source; - private final String target; - - public EnumMapping(String source, String target) { - this.source = source; - this.target = target; - } - - /** - * @return the name of the constant in the source enum. - */ - public String getSource() { - return source; - } - - /** - * @return the name of the constant in the target enum. - */ - public String getTarget() { - return target; - } -} diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/SelectionParameters.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/SelectionParameters.java index cdbf74560..34eb0a7d8 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/SelectionParameters.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/SelectionParameters.java @@ -26,6 +26,23 @@ public class SelectionParameters { private final Types typeUtils; private final SourceRHS sourceRHS; + /** + * Returns new selection parameters + * + * ResultType is not inherited. + * + * @param selectionParameters + * @return + */ + public static SelectionParameters forInheritance(SelectionParameters selectionParameters) { + return new SelectionParameters( + selectionParameters.qualifiers, + selectionParameters.qualifyingNames, + null, + selectionParameters.typeUtils + ); + } + public SelectionParameters(List qualifiers, List qualifyingNames, TypeMirror resultType, Types typeUtils) { this( qualifiers, qualifyingNames, resultType, typeUtils, null ); diff --git a/processor/src/main/java/org/mapstruct/ap/internal/processor/MethodRetrievalProcessor.java b/processor/src/main/java/org/mapstruct/ap/internal/processor/MethodRetrievalProcessor.java index ffef1656d..e7e2db260 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/processor/MethodRetrievalProcessor.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/processor/MethodRetrievalProcessor.java @@ -254,7 +254,14 @@ public class MethodRetrievalProcessor implements ModelElementProcessor