#3949 Support SET_TO_NULL for overloaded target methods, requiring a cast#3988
#3949 Support SET_TO_NULL for overloaded target methods, requiring a cast#3988hduelme wants to merge 4 commits intomapstruct:mainfrom
Conversation
| public long settersWithName(String name) { | ||
| return getSetters().stream().filter( setter -> setter.getSimpleName().equals( name ) ).count(); | ||
| } |
There was a problem hiding this comment.
I would not expose this in the Type. This check should be in the PropertyMapping. We can actually implement this a bit better and stop the iteration if the count reaches 2. No need to iterate through all setters.
There was a problem hiding this comment.
Moved to PropertyMapping and break if the second one is found.
| if (propertyMapping.getAssignment() instanceof SetterWrapper | ||
| && ((SetterWrapper) propertyMapping.getAssignment()).isSetExplicitlyToNull() | ||
| && ((SetterWrapper) propertyMapping.getAssignment()).isMustCastForNull()) { | ||
| // Setting to null with cast might require an import | ||
| types.add( propertyMapping.getTargetType() ); | ||
| } |
There was a problem hiding this comment.
This is not the responsibility of the BeanMappingMethod. The getImportTypes() of the SetterWrapper should do this instead. We should pass the targetType to the SetterWrapper, in a similar way like we are doing for the UpdateWrapper
There was a problem hiding this comment.
Moved to SetterWrapper like you suggested
| includeSourceNullCheck && nvpms == SET_TO_NULL && !targetType.isPrimitive(), | ||
| nvpms == SET_TO_DEFAULT ); | ||
| nvpms == SET_TO_DEFAULT, | ||
| this.method.getResultType().settersWithName( this.targetWriteAccessor.getSimpleName() ) > 1 |
There was a problem hiding this comment.
We cannot implement this like this, because the method resultType is not what has the setters. If the resultType has a builder then the setters we are using are actually in the builder, not in the result type.
There was a problem hiding this comment.
Changed to look at the enclosing class.
|
@filiphr as you suggested, I moved the import logic to the In addition I added the tests where sub-mapping is generated. |
2 participants
Currently, invalid code is generated when
SET_TO_NULLis used, as described in #3949.This PR changes the generated code to include an explicit cast when more than two setters with the same name are present.
MapStruct does not fully support overloaded target methods yet (see #1135). To account for this limitation, the tests use a workaround: the preferred target type is determined via the getter with the same name to verify that the correct setter is invoked when passing
null.