From 3d43d022f49f52b70f5c35b9b086d163bd5110bd Mon Sep 17 00:00:00 2001 From: sjaakd Date: Wed, 20 Jul 2016 21:55:04 +0200 Subject: [PATCH] #838 check for suitable constructors when reversing --- .../ap/internal/model/common/Type.java | 20 ++++++++ .../internal/model/source/SourceMethod.java | 46 ++++++++++++++++--- .../processor/MapperCreationProcessor.java | 19 ++++++++ .../mapstruct/ap/internal/util/Message.java | 3 +- .../NestedSourcePropertiesTest.java | 8 ++-- 5 files changed, 84 insertions(+), 12 deletions(-) diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java b/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java index 7159d6200..2fadbfc03 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java @@ -39,6 +39,7 @@ import javax.lang.model.type.DeclaredType; import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeMirror; import javax.lang.model.type.WildcardType; +import javax.lang.model.util.ElementFilter; import javax.lang.model.util.Elements; import javax.lang.model.util.Types; @@ -95,6 +96,7 @@ public class Type extends ModelElement implements Comparable { private Type boundingBase = null; + private Boolean hasEmptyAccessibleContructor; //CHECKSTYLE:OFF public Type(Types typeUtils, Elements elementUtils, TypeFactory typeFactory, @@ -770,4 +772,22 @@ public class Type extends ModelElement implements Comparable { return boundingBase; } + + + public boolean hasEmptyAccessibleContructor() { + + if ( this.hasEmptyAccessibleContructor == null ) { + hasEmptyAccessibleContructor = false; + List constructors = ElementFilter.constructorsIn( typeElement.getEnclosedElements() ); + for ( ExecutableElement constructor : constructors ) { + if ( (constructor.getModifiers().contains( Modifier.PUBLIC ) + || constructor.getModifiers().contains( Modifier.PROTECTED ) ) + && constructor.getParameters().isEmpty() ) { + hasEmptyAccessibleContructor = true; + break; + } + } + } + return hasEmptyAccessibleContructor; + } } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/SourceMethod.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/SourceMethod.java index de219f3a4..4f0c5dfc8 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/SourceMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/SourceMethod.java @@ -73,6 +73,12 @@ public class SourceMethod implements Method { private List applicablePrototypeMethods; + private Boolean isBeanMapping; + private Boolean isEnumMapping; + private Boolean isValueMapping; + private Boolean isIterableMapping; + private Boolean isMapMapping; + public static class Builder { private Type declaringMapper = null; @@ -367,18 +373,40 @@ public class SourceMethod implements Method { } public boolean isIterableMapping() { - return getSourceParameters().size() == 1 && first( getSourceParameters() ).getType().isIterableType() - && getResultType().isIterableType(); + if ( isIterableMapping == null ) { + isIterableMapping = getSourceParameters().size() == 1 + && first( getSourceParameters() ).getType().isIterableType() + && getResultType().isIterableType(); + } + return isIterableMapping; } public boolean isMapMapping() { - return getSourceParameters().size() == 1 && first( getSourceParameters() ).getType().isMapType() - && getResultType().isMapType(); + if ( isMapMapping == null ) { + isMapMapping = getSourceParameters().size() == 1 + && first( getSourceParameters() ).getType().isMapType() + && getResultType().isMapType(); + } + return isMapMapping; } public boolean isEnumMapping() { - return getSourceParameters().size() == 1 && first( getSourceParameters() ).getType().isEnumType() - && getResultType().isEnumType(); + if ( isEnumMapping == null ) { + isEnumMapping = getSourceParameters().size() == 1 + && first( getSourceParameters() ).getType().isEnumType() + && getResultType().isEnumType(); + } + return isEnumMapping; + } + + public boolean isBeanMapping() { + if ( isBeanMapping == null ) { + isBeanMapping = !isIterableMapping() + && !isMapMapping() + && !isEnumMapping() + && !isValueMapping(); + } + return isBeanMapping; } /** @@ -388,7 +416,11 @@ public class SourceMethod implements Method { * @return whether (true) or not (false) to execute value mappings */ public boolean isValueMapping() { - return isEnumMapping() && mappingOptions.getMappings().isEmpty(); + + if ( isValueMapping == null ) { + isValueMapping = isEnumMapping() && mappingOptions.getMappings().isEmpty(); + } + return isValueMapping; } private boolean equals(Object o1, Object o2) { diff --git a/processor/src/main/java/org/mapstruct/ap/internal/processor/MapperCreationProcessor.java b/processor/src/main/java/org/mapstruct/ap/internal/processor/MapperCreationProcessor.java index bde48781d..c1bdd4056 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/processor/MapperCreationProcessor.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/processor/MapperCreationProcessor.java @@ -458,6 +458,14 @@ public class MapperCreationProcessor implements ModelElementProcessor candidates = new ArrayList(); for ( SourceMethod oneMethod : rawMethods ) { @@ -621,6 +629,17 @@ public class MapperCreationProcessor implements ModelElementProcessor candidates, SourceMethod method, InheritInverseConfigurationPrism reversePrism) { diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/Message.java b/processor/src/main/java/org/mapstruct/ap/internal/util/Message.java index dc5646f7d..8752ece2c 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/util/Message.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/util/Message.java @@ -48,7 +48,7 @@ public enum Message { PROPERTYMAPPING_EXPRESSION_AND_DEFAULT_VALUE_BOTH_DEFINED( "Expression and default value are both defined in @Mapping, either define a defaultValue or an expression." ), PROPERTYMAPPING_CONSTANT_AND_DEFAULT_VALUE_BOTH_DEFINED( "Constant and default value are both defined in @Mapping, either define a defaultValue or a constant." ), PROPERTYMAPPING_INVALID_EXPRESSION( "Value must be given in the form \"java()\"." ), - PROPERTYMAPPING_REVERSAL_PROBLEM( "Parameter %s cannot be reversed." ), + PROPERTYMAPPING_REVERSAL_PROBLEM( "Parameter %s cannot be reversed.", Diagnostic.Kind.NOTE ), PROPERTYMAPPING_INVALID_PARAMETER_NAME( "Method has no parameter named \"%s\"." ), PROPERTYMAPPING_NO_PROPERTY_IN_PARAMETER( "The type of parameter \"%s\" has no property named \"%s\"." ), PROPERTYMAPPING_INVALID_PROPERTY_NAME( "No property named \"%s\" exists in source parameter(s)." ), @@ -102,6 +102,7 @@ public enum Message { INHERITINVERSECONFIGURATION_DUPLICATES( "Several matching inverse methods exist: %s(). Specify a name explicitly." ), INHERITINVERSECONFIGURATION_INVALID_NAME( "None of the candidates %s() matches given name: \"%s\"." ), INHERITINVERSECONFIGURATION_DUPLICATE_MATCHES( "Given name \"%s\" matches several candidate methods: %s." ), + INHERITINVERSECONFIGURATION_NO_SUITABLE_CONSTRUCTOR( "There is no suitable result type constructor for reversing this method." ), INHERITINVERSECONFIGURATION_NO_NAME_MATCH( "Given name \"%s\" does not match the only candidate. Did you mean: \"%s\"." ), INHERITCONFIGURATION_DUPLICATES( "Several matching methods exist: %s(). Specify a name explicitly." ), INHERITCONFIGURATION_INVALIDNAME( "None of the candidates %s() matches given name: \"%s\"." ), diff --git a/processor/src/test/java/org/mapstruct/ap/test/nestedsourceproperties/NestedSourcePropertiesTest.java b/processor/src/test/java/org/mapstruct/ap/test/nestedsourceproperties/NestedSourcePropertiesTest.java index fd12d2927..70b79d11a 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/nestedsourceproperties/NestedSourcePropertiesTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/nestedsourceproperties/NestedSourcePropertiesTest.java @@ -171,17 +171,17 @@ public class NestedSourcePropertiesTest { } @Test - @IssueKey( "337" ) + @IssueKey( "838" ) @ExpectedCompilationOutcome( value = CompilationResult.FAILED, diagnostics = { @Diagnostic( type = ArtistToChartEntryErroneous.class, kind = javax.tools.Diagnostic.Kind.ERROR, - line = 47, - messageRegExp = "Parameter java.lang.Integer position cannot be reversed" ) + line = 46, + messageRegExp = "There is no suitable result type constructor for reversing this method\\." ) } ) @WithClasses({ ArtistToChartEntryErroneous.class }) - public void reverseShouldIgnoreParameter() { + public void reverseShouldRaiseErrorForEmptyContructor() { } }