From 65c272286f012c850e2e9e4ec33c7d279b8943a4 Mon Sep 17 00:00:00 2001 From: sjaakd Date: Tue, 16 Dec 2014 23:33:15 +0100 Subject: [PATCH] #385 Refactoring, factory methods should use selectors, preparing selector api for more criteria --- .../mapstruct/ap/model/AssignmentFactory.java | 48 ------------------- .../mapstruct/ap/model/BeanMappingMethod.java | 2 +- .../ap/model/IterableMappingMethod.java | 3 +- .../mapstruct/ap/model/MapMappingMethod.java | 2 +- .../ap/model/MappingBuilderContext.java | 12 +++++ .../source/selector/InheritanceSelector.java | 14 +++--- .../model/source/selector/MethodSelector.java | 13 ++--- .../source/selector/MethodSelectors.java | 20 ++++---- .../source/selector/QualifierSelector.java | 6 +-- .../source/selector/SelectionCriteria.java | 47 ++++++++++++++++++ .../model/source/selector/TypeSelector.java | 12 ++--- .../selector/XmlElementDeclSelector.java | 7 ++- .../creation/MappingResolverImpl.java | 31 ++++++++++-- .../ambiguousfactorymethod/FactoryTest.java | 13 +++-- 14 files changed, 130 insertions(+), 100 deletions(-) create mode 100644 processor/src/main/java/org/mapstruct/ap/model/source/selector/SelectionCriteria.java diff --git a/processor/src/main/java/org/mapstruct/ap/model/AssignmentFactory.java b/processor/src/main/java/org/mapstruct/ap/model/AssignmentFactory.java index 3cb763a8c..1c346a8f5 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/AssignmentFactory.java +++ b/processor/src/main/java/org/mapstruct/ap/model/AssignmentFactory.java @@ -21,15 +21,12 @@ package org.mapstruct.ap.model; import java.util.List; import java.util.Set; -import javax.tools.Diagnostic; import org.mapstruct.ap.model.assignment.Assignment; import org.mapstruct.ap.model.common.ConversionContext; import org.mapstruct.ap.model.common.Type; import org.mapstruct.ap.model.source.Method; -import org.mapstruct.ap.model.source.SourceMethod; import org.mapstruct.ap.model.source.builtin.BuiltInMethod; -import org.mapstruct.ap.model.source.selector.MethodSelectors; /** * Factory class for creating all types of assignments @@ -57,50 +54,5 @@ public class AssignmentFactory { public static Direct createDirect(String sourceRef) { return new Direct( sourceRef ); } - - public static MethodReference createFactoryMethod( Type returnType, MappingBuilderContext ctx ) { - MethodReference result = null; - for ( SourceMethod method : ctx.getSourceModel() ) { - if ( !method.overridesMethod() && !method.isIterableMapping() && !method.isMapMapping() - && method.getSourceParameters().isEmpty() ) { - - List parameterTypes = MethodSelectors.getParameterTypes( - ctx.getTypeFactory(), - method.getParameters(), - null, - returnType - ); - - if ( method.matches( parameterTypes, returnType ) ) { - if ( result == null ) { - MapperReference mapperReference = findMapperReference( ctx.getMapperReferences(), method ); - result = new MethodReference( method, mapperReference, null ); - } - else { - ctx.getMessager().printMessage( - Diagnostic.Kind.ERROR, - String.format( - "Ambiguous factory methods: \"%s\" conflicts with \"%s\".", - result, - method - ), - method.getExecutable() - ); - } - } - } - } - return result; - } - - private static MapperReference findMapperReference( List mapperReferences, SourceMethod method ) { - for ( MapperReference ref : mapperReferences ) { - if ( ref.getType().equals( method.getDeclaringMapper() ) ) { - ref.setUsed( !method.isStatic() ); - return ref; - } - } - return null; - } } diff --git a/processor/src/main/java/org/mapstruct/ap/model/BeanMappingMethod.java b/processor/src/main/java/org/mapstruct/ap/model/BeanMappingMethod.java index a8fe614fe..0a811dc63 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/BeanMappingMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/model/BeanMappingMethod.java @@ -103,7 +103,7 @@ public class BeanMappingMethod extends MappingMethod { boolean mapNullToDefault = MapperConfig.getInstanceOn( ctx.getMapperTypeElement() ).isMapToDefault( prism ); - MethodReference factoryMethod = AssignmentFactory.createFactoryMethod( method.getReturnType(), ctx ); + MethodReference factoryMethod = ctx.getMappingResolver().getFactoryMethod( method, method.getResultType() ); return new BeanMappingMethod( method, propertyMappings, factoryMethod, mapNullToDefault ); } diff --git a/processor/src/main/java/org/mapstruct/ap/model/IterableMappingMethod.java b/processor/src/main/java/org/mapstruct/ap/model/IterableMappingMethod.java index 9f2b1251b..08bbb1d21 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/IterableMappingMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/model/IterableMappingMethod.java @@ -124,8 +124,7 @@ public class IterableMappingMethod extends MappingMethod { boolean mapNullToDefault = MapperConfig.getInstanceOn( ctx.getMapperTypeElement() ).isMapToDefault( prism ); - MethodReference factoryMethod = AssignmentFactory.createFactoryMethod( method.getReturnType(), ctx ); - + MethodReference factoryMethod = ctx.getMappingResolver().getFactoryMethod( method, method.getResultType() ); return new IterableMappingMethod( method, assignment, diff --git a/processor/src/main/java/org/mapstruct/ap/model/MapMappingMethod.java b/processor/src/main/java/org/mapstruct/ap/model/MapMappingMethod.java index 4069eb1c0..12dc8650d 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/MapMappingMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/model/MapMappingMethod.java @@ -141,7 +141,7 @@ public class MapMappingMethod extends MappingMethod { boolean mapNullToDefault = MapperConfig.getInstanceOn( ctx.getMapperTypeElement() ).isMapToDefault( prism ); - MethodReference factoryMethod = AssignmentFactory.createFactoryMethod( method.getReturnType(), ctx ); + MethodReference factoryMethod = ctx.getMappingResolver().getFactoryMethod( method, method.getResultType() ); keyAssignment = new LocalVarWrapper( keyAssignment, method.getThrownTypes() ); valueAssignment = new LocalVarWrapper( valueAssignment, method.getThrownTypes() ); diff --git a/processor/src/main/java/org/mapstruct/ap/model/MappingBuilderContext.java b/processor/src/main/java/org/mapstruct/ap/model/MappingBuilderContext.java index d8d581f10..40b6b80ee 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/MappingBuilderContext.java +++ b/processor/src/main/java/org/mapstruct/ap/model/MappingBuilderContext.java @@ -94,6 +94,17 @@ public class MappingBuilderContext { String targetPropertyName, String dateFormat, List qualifiers, String sourceReference); + /** + * returns a no arg factory method + * + * @param mappingMethod target mapping method + * @param targetType return type to match + * + * @return a method reference to the factory method, or null if no suitable, or ambiguous method found + * + */ + MethodReference getFactoryMethod(Method mappingMethod, Type targetType); + Set getUsedVirtualMappings(); } @@ -190,4 +201,5 @@ public class MappingBuilderContext { public Set getUsedVirtualMappings() { return mappingResolver.getUsedVirtualMappings(); } + } diff --git a/processor/src/main/java/org/mapstruct/ap/model/source/selector/InheritanceSelector.java b/processor/src/main/java/org/mapstruct/ap/model/source/selector/InheritanceSelector.java index 84eadd3ba..ea889a622 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/source/selector/InheritanceSelector.java +++ b/processor/src/main/java/org/mapstruct/ap/model/source/selector/InheritanceSelector.java @@ -20,7 +20,6 @@ package org.mapstruct.ap.model.source.selector; import java.util.ArrayList; import java.util.List; -import javax.lang.model.type.TypeMirror; import org.mapstruct.ap.model.common.Parameter; import org.mapstruct.ap.model.common.Type; @@ -37,12 +36,15 @@ public class InheritanceSelector implements MethodSelector { public List getMatchingMethods( Method mappingMethod, List methods, - Type parameterType, - Type returnType, - List qualifiers, - String targetPropertyName + Type sourceType, + Type targetType, + SelectionCriteria criteria ) { + if ( sourceType == null ) { + return methods; + } + List candidatesWithBestMatchingSourceType = new ArrayList(); int bestMatchingSourceTypeDistance = Integer.MAX_VALUE; @@ -50,7 +52,7 @@ public class InheritanceSelector implements MethodSelector { for ( T method : methods ) { Parameter singleSourceParam = method.getSourceParameters().iterator().next(); - int sourceTypeDistance = parameterType.distanceTo( singleSourceParam.getType() ); + int sourceTypeDistance = sourceType.distanceTo( singleSourceParam.getType() ); bestMatchingSourceTypeDistance = addToCandidateListIfMinimal( candidatesWithBestMatchingSourceType, diff --git a/processor/src/main/java/org/mapstruct/ap/model/source/selector/MethodSelector.java b/processor/src/main/java/org/mapstruct/ap/model/source/selector/MethodSelector.java index 07814101b..186f44e42 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/source/selector/MethodSelector.java +++ b/processor/src/main/java/org/mapstruct/ap/model/source/selector/MethodSelector.java @@ -19,7 +19,6 @@ package org.mapstruct.ap.model.source.selector; import java.util.List; -import javax.lang.model.type.TypeMirror; import org.mapstruct.ap.model.common.Type; import org.mapstruct.ap.model.source.Method; @@ -39,14 +38,12 @@ public interface MethodSelector { * @param either SourceMethod or BuiltInMethod * @param mappingMethod mapping method, defined in Mapper for which this selection is carried out * @param methods list of available methods - * @param parameterType parameter type that should be matched - * @param returnType return type that should be matched - * @param qualifiers list of custom annotations, used in the qualifying process - * @param targetPropertyName some information can be derived from the target property + * @param sourceType parameter type that should be matched + * @param targetType return type that should be matched + * @param criteria criteria used in the selection process * * @return list of methods that passes the matching process */ - List getMatchingMethods(Method mappingMethod, List methods, Type parameterType, - Type returnType, List qualifiers, - String targetPropertyName); + List getMatchingMethods(Method mappingMethod, List methods, Type sourceType, + Type targetType, SelectionCriteria criteria); } diff --git a/processor/src/main/java/org/mapstruct/ap/model/source/selector/MethodSelectors.java b/processor/src/main/java/org/mapstruct/ap/model/source/selector/MethodSelectors.java index 2af3c833a..bdf313b69 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/source/selector/MethodSelectors.java +++ b/processor/src/main/java/org/mapstruct/ap/model/source/selector/MethodSelectors.java @@ -21,7 +21,6 @@ package org.mapstruct.ap.model.source.selector; import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import javax.lang.model.type.TypeMirror; import javax.lang.model.util.Elements; import javax.lang.model.util.Types; @@ -51,9 +50,8 @@ public class MethodSelectors implements MethodSelector { @Override public List getMatchingMethods(Method mappingMethod, List methods, - Type parameterType, Type returnType, - List qualifiers, - String targetPropertyName) { + Type sourceType, Type targetType, + SelectionCriteria criteria) { List candidates = new ArrayList( methods ); @@ -61,10 +59,9 @@ public class MethodSelectors implements MethodSelector { candidates = selector.getMatchingMethods( mappingMethod, candidates, - parameterType, - returnType, - qualifiers, - targetPropertyName + sourceType, + targetType, + criteria ); } return candidates; @@ -80,13 +77,16 @@ public class MethodSelectors implements MethodSelector { */ public static List getParameterTypes(TypeFactory typeFactory, List parameters, Type sourceType, Type returnType) { - List result = new ArrayList( parameters.size() ); + List result = new ArrayList(); for ( Parameter param : parameters ) { if ( param.isTargetType() ) { result.add( typeFactory.classTypeOf( returnType ) ); } else { - result.add( sourceType ); + if ( sourceType != null ) { + /* for factory methods (sourceType==null), no parameter must be added */ + result.add( sourceType ); + } } } diff --git a/processor/src/main/java/org/mapstruct/ap/model/source/selector/QualifierSelector.java b/processor/src/main/java/org/mapstruct/ap/model/source/selector/QualifierSelector.java index bea0fcdab..504021b56 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/source/selector/QualifierSelector.java +++ b/processor/src/main/java/org/mapstruct/ap/model/source/selector/QualifierSelector.java @@ -59,10 +59,10 @@ public class QualifierSelector implements MethodSelector { @Override public List getMatchingMethods(Method mappingMethod, List methods, - Type parameterType, Type returnType, - List qualifiers, - String targetPropertyName) { + Type sourceType, Type targetType, + SelectionCriteria criteria) { + List qualifiers = criteria.getQualifiers(); if ( qualifiers == null || qualifiers.isEmpty() ) { // remove the method marked as qualifier from the list List nonQualiferAnnotatedMethods = new ArrayList(); diff --git a/processor/src/main/java/org/mapstruct/ap/model/source/selector/SelectionCriteria.java b/processor/src/main/java/org/mapstruct/ap/model/source/selector/SelectionCriteria.java new file mode 100644 index 000000000..d51d5de1e --- /dev/null +++ b/processor/src/main/java/org/mapstruct/ap/model/source/selector/SelectionCriteria.java @@ -0,0 +1,47 @@ +/** + * Copyright 2012-2015 Gunnar Morling (http://www.gunnarmorling.de/) + * and/or other contributors as indicated by the @authors tag. See the + * copyright.txt file in the distribution for a full listing of all + * contributors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.mapstruct.ap.model.source.selector; + +import java.util.List; +import javax.lang.model.type.TypeMirror; + +/** + * This class groups the selection criteria in one class + * + * @author Sjaak Derksen + */ +public class SelectionCriteria { + + private final List qualifiers; + private final String targetPropertyName; + + public SelectionCriteria(List qualifiers, String targetPropertyName) { + this.qualifiers = qualifiers; + this.targetPropertyName = targetPropertyName; + } + + public List getQualifiers() { + return qualifiers; + } + + public String getTargetPropertyName() { + return targetPropertyName; + } + +} diff --git a/processor/src/main/java/org/mapstruct/ap/model/source/selector/TypeSelector.java b/processor/src/main/java/org/mapstruct/ap/model/source/selector/TypeSelector.java index 0436d05d9..52be353d1 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/source/selector/TypeSelector.java +++ b/processor/src/main/java/org/mapstruct/ap/model/source/selector/TypeSelector.java @@ -20,7 +20,6 @@ package org.mapstruct.ap.model.source.selector; import java.util.ArrayList; import java.util.List; -import javax.lang.model.type.TypeMirror; import org.mapstruct.ap.model.common.Type; import org.mapstruct.ap.model.common.TypeFactory; @@ -43,19 +42,18 @@ public class TypeSelector implements MethodSelector { @Override public List getMatchingMethods(Method mappingMethod, List methods, - Type parameterType, Type returnType, - List qualifiers, - String targetPropertyName) { + Type sourceType, Type targetType, + SelectionCriteria criteria) { List result = new ArrayList(); for ( T method : methods ) { - if ( method.getSourceParameters().size() != 1 ) { + if ( method.getSourceParameters().size() > 1 ) { continue; } List parameterTypes = - MethodSelectors.getParameterTypes( typeFactory, method.getParameters(), parameterType, returnType ); - if ( method.matches( parameterTypes, returnType ) ) { + MethodSelectors.getParameterTypes( typeFactory, method.getParameters(), sourceType, targetType ); + if ( method.matches( parameterTypes, targetType ) ) { result.add( method ); } } diff --git a/processor/src/main/java/org/mapstruct/ap/model/source/selector/XmlElementDeclSelector.java b/processor/src/main/java/org/mapstruct/ap/model/source/selector/XmlElementDeclSelector.java index 9b9bddece..d82e9c3e5 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/source/selector/XmlElementDeclSelector.java +++ b/processor/src/main/java/org/mapstruct/ap/model/source/selector/XmlElementDeclSelector.java @@ -52,9 +52,8 @@ public class XmlElementDeclSelector implements MethodSelector { @Override public List getMatchingMethods(Method mappingMethod, List methods, - Type parameterType, Type returnType, - List qualifiers, - String targetPropertyName) { + Type sourceType, Type targetType, + SelectionCriteria criteria) { // only true source methods are qualifying if ( !(mappingMethod instanceof SourceMethod) ) { @@ -83,7 +82,7 @@ public class XmlElementDeclSelector implements MethodSelector { TypeMirror scope = xmlElememtDecl.scope(); TypeMirror target = sourceMappingMethod.getExecutable().getReturnType(); - boolean nameIsSetAndMatches = name != null && name.equals( targetPropertyName ); + boolean nameIsSetAndMatches = name != null && name.equals( criteria.getTargetPropertyName() ); boolean scopeIsSetAndMatches = scope != null && typeUtils.isSameType( scope, target ); if ( nameIsSetAndMatches ) { diff --git a/processor/src/main/java/org/mapstruct/ap/processor/creation/MappingResolverImpl.java b/processor/src/main/java/org/mapstruct/ap/processor/creation/MappingResolverImpl.java index a25bde816..5cd6abc5f 100755 --- a/processor/src/main/java/org/mapstruct/ap/processor/creation/MappingResolverImpl.java +++ b/processor/src/main/java/org/mapstruct/ap/processor/creation/MappingResolverImpl.java @@ -34,6 +34,7 @@ import org.mapstruct.ap.conversion.Conversions; import org.mapstruct.ap.model.AssignmentFactory; import org.mapstruct.ap.model.MapperReference; import org.mapstruct.ap.model.MappingBuilderContext.MappingResolver; +import org.mapstruct.ap.model.MethodReference; import org.mapstruct.ap.model.VirtualMappingMethod; import org.mapstruct.ap.model.assignment.Assignment; import org.mapstruct.ap.model.common.ConversionContext; @@ -45,6 +46,7 @@ import org.mapstruct.ap.model.source.SourceMethod; import org.mapstruct.ap.model.source.builtin.BuiltInMappingMethods; import org.mapstruct.ap.model.source.builtin.BuiltInMethod; import org.mapstruct.ap.model.source.selector.MethodSelectors; +import org.mapstruct.ap.model.source.selector.SelectionCriteria; import org.mapstruct.ap.util.Strings; /** @@ -140,6 +142,28 @@ public class MappingResolverImpl implements MappingResolver { return usedVirtualMappings; } + @Override + public MethodReference getFactoryMethod( Method mappingMethod, Type targetType ) { + + ResolvingAttempt attempt = new ResolvingAttempt( + sourceModel, + mappingMethod, + null, + null, + null, + null, + null + ); + + SourceMethod matchingSourceMethod = attempt.getBestMatch( sourceModel, null, targetType ); + if ( matchingSourceMethod != null ) { + MapperReference ref = attempt.findMapperReference( matchingSourceMethod ); + return new MethodReference( matchingSourceMethod, ref, null ); + } + return null; + + } + private class ResolvingAttempt { private final Method mappingMethod; @@ -416,13 +440,13 @@ public class MappingResolverImpl implements MappingResolver { private T getBestMatch(List methods, Type sourceType, Type returnType) { + SelectionCriteria criteria = new SelectionCriteria( qualifiers, targetPropertyName ); List candidates = methodSelectors.getMatchingMethods( mappingMethod, methods, sourceType, returnType, - qualifiers, - targetPropertyName + criteria ); // raise an error if more than one mapping method is suitable to map the given source type @@ -430,7 +454,8 @@ public class MappingResolverImpl implements MappingResolver { if ( candidates.size() > 1 ) { String errorMsg = String.format( - "Ambiguous mapping methods found for mapping " + mappedElement + " to %s: %s.", + "Ambiguous mapping methods found for %s %s: %s.", + mappedElement != null ? "mapping " + mappedElement + " to" : "factorizing", returnType, Strings.join( candidates, ", " ) ); diff --git a/processor/src/test/java/org/mapstruct/ap/test/erroneous/ambiguousfactorymethod/FactoryTest.java b/processor/src/test/java/org/mapstruct/ap/test/erroneous/ambiguousfactorymethod/FactoryTest.java index 03ff347df..a71559fa9 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/erroneous/ambiguousfactorymethod/FactoryTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/erroneous/ambiguousfactorymethod/FactoryTest.java @@ -44,14 +44,13 @@ public class FactoryTest { @ExpectedCompilationOutcome( value = CompilationResult.FAILED, diagnostics = { - @Diagnostic(type = BarFactory.class, + @Diagnostic(type = SourceTargetMapperAndBarFactory.class, kind = javax.tools.Diagnostic.Kind.ERROR, - line = 29, - messageRegExp = "Ambiguous factory methods: \"org\\.mapstruct\\.ap\\.test\\.erroneous\\." - + "ambiguousfactorymethod\\.Bar createBar\\(\\)\" conflicts with " - + "\"org\\.mapstruct\\.ap\\.test\\.erroneous\\.ambiguousfactorymethod\\.Bar " - + "org\\.mapstruct\\.ap\\.test\\.erroneous\\.ambiguousfactorymethod" - + "\\.a\\.BarFactory\\.createBar\\(\\)\"\\") + line = 35, + messageRegExp = "Ambiguous mapping methods found for factorizing " + + "org.mapstruct.ap.test.erroneous.ambiguousfactorymethod.Bar: " + + "org.mapstruct.ap.test.erroneous.ambiguousfactorymethod.Bar createBar\\(\\), " + + "org.mapstruct.ap.test.erroneous.ambiguousfactorymethod.Bar .*BarFactory.createBar\\(\\)." ) } ) public void shouldUseTwoFactoryMethods() {