#892 If multiple setters exist, don't override the setter if it doesn't match the property getters type.

This commit is contained in:
Andreas Gudian 2016-10-28 20:14:51 +02:00 committed by Gunnar Morling
parent a928ff56d7
commit 4832369148
3 changed files with 142 additions and 14 deletions

View File

@ -414,10 +414,8 @@ public class Type extends ModelElement implements Comparable<Type> {
* @return an unmodifiable map of all write accessors indexed by property name
*/
public Map<String, ExecutableElement> getPropertyWriteAccessors( CollectionMappingStrategyPrism cmStrategy ) {
// collect all candidate target accessors
List<ExecutableElement> candidates = new ArrayList<ExecutableElement>();
candidates.addAll( getSetters() );
List<ExecutableElement> candidates = new ArrayList<ExecutableElement>( getSetters() );
candidates.addAll( getAlternativeTargetAccessors() );
Map<String, ExecutableElement> result = new LinkedHashMap<String, ExecutableElement>();
@ -425,27 +423,27 @@ public class Type extends ModelElement implements Comparable<Type> {
for ( ExecutableElement candidate : candidates ) {
String targetPropertyName = Executables.getPropertyName( candidate );
ExecutableElement readAccessor = getPropertyReadAccessors().get( targetPropertyName );
Type preferredType = determinePreferredType( readAccessor );
Type targetType = determineTargetType( candidate );
// A target access is in general a setter method on the target object. However, in case of collections,
// the current target accessor can also be a getter method.
// The following if block, checks if the target accessor should be overruled by an add method.
if ( cmStrategy == CollectionMappingStrategyPrism.SETTER_PREFERRED
|| cmStrategy == CollectionMappingStrategyPrism.ADDER_PREFERRED ) {
|| cmStrategy == CollectionMappingStrategyPrism.ADDER_PREFERRED ) {
// first check if there's a setter method.
ExecutableElement adderMethod = null;
if ( Executables.isSetterMethod( candidate ) ) {
Type targetType = typeFactory.getSingleParameter( (DeclaredType) typeMirror, candidate ).getType();
if ( Executables.isSetterMethod( candidate )
// ok, the current accessor is a setter. So now the strategy determines what to use
if ( cmStrategy == CollectionMappingStrategyPrism.ADDER_PREFERRED ) {
adderMethod = getAdderForType( targetType, targetPropertyName );
}
&& cmStrategy == CollectionMappingStrategyPrism.ADDER_PREFERRED ) {
adderMethod = getAdderForType( targetType, targetPropertyName );
}
else if ( Executables.isGetterMethod( candidate ) ) {
// the current accessor is a getter (no setter available). But still, an add method is according
// the current accessor is a getter (no setter available). But still, an add method is according
// to the above strategy (SETTER_PREFERRED || ADDER_PREFERRED) preferred over the getter.
Type targetType = typeFactory.getReturnType(
typeFactory.getMethodType( (DeclaredType) typeMirror, candidate )
);
adderMethod = getAdderForType( targetType, targetPropertyName );
}
if ( adderMethod != null ) {
@ -454,11 +452,36 @@ public class Type extends ModelElement implements Comparable<Type> {
}
}
result.put( targetPropertyName, candidate );
ExecutableElement previousCandidate = result.get( targetPropertyName );
if ( previousCandidate == null || preferredType == null || ( targetType != null
&& typeUtils.isAssignable( preferredType.getTypeMirror(), targetType.getTypeMirror() ) ) ) {
result.put( targetPropertyName, candidate );
}
}
return result;
}
private Type determinePreferredType(ExecutableElement readAccessor) {
if ( readAccessor != null ) {
return typeFactory.getReturnType(
typeFactory.getMethodType( (DeclaredType) typeMirror, readAccessor ) );
}
return null;
}
private Type determineTargetType(ExecutableElement candidate) {
Parameter parameter = typeFactory.getSingleParameter( (DeclaredType) typeMirror, candidate );
if ( parameter != null ) {
return parameter.getType();
}
else if ( Executables.isGetterMethod( candidate ) ) {
return typeFactory.getReturnType(
typeFactory.getMethodType( (DeclaredType) typeMirror, candidate ) );
}
return null;
}
private List<ExecutableElement> getAllExecutables() {
if ( allExecutables == null ) {
allExecutables = Executables.getAllEnclosedExecutableElements( elementUtils, typeElement );

View File

@ -0,0 +1,58 @@
/**
* Copyright 2012-2016 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.test.bugs._892;
import org.mapstruct.Mapper;
/**
* @author Andreas Gudian
*
*/
@Mapper
public interface Issue892Mapper {
Target toTarget(Source source);
class Source {
private int type;
public int getType() {
return type;
}
public void setType(int type) {
this.type = type;
}
}
class Target {
private int type;
public int getType() {
return type;
}
public void setType(int type) {
this.type = type;
}
public void setType(String typeInHex) {
this.type = Integer.parseInt( typeInHex, 16 );
}
}
}

View File

@ -0,0 +1,47 @@
/**
* Copyright 2012-2016 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.test.bugs._892;
import static org.assertj.core.api.Assertions.assertThat;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mapstruct.ap.test.bugs._892.Issue892Mapper.Source;
import org.mapstruct.ap.test.bugs._892.Issue892Mapper.Target;
import org.mapstruct.ap.testutil.WithClasses;
import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner;
import org.mapstruct.factory.Mappers;
/**
* When having two setter methods with the same name, choose the one with the argument type matching the getter method.
*
* @author Andreas Gudian
*/
@RunWith(AnnotationProcessorTestRunner.class)
@WithClasses({ Issue892Mapper.class })
public class Issue892Test {
@Test
public void compiles() {
Source src = new Source();
src.setType( 42 );
Target target = Mappers.getMapper( Issue892Mapper.class ).toTarget( src );
assertThat( target.getType() ).isEqualTo( 42 );
}
}