-
-
Notifications
You must be signed in to change notification settings - Fork 981
#3617 support for target this enum mapping. #3616
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -235,7 +235,7 @@ else if ( !method.isUpdateMethod() ) { | |
|
||
this.unprocessedTargetProperties = new LinkedHashMap<>( accessors ); | ||
|
||
if ( !method.isUpdateMethod() && !hasFactoryMethod ) { | ||
if ( !method.isUpdateMethod() && !hasFactoryMethod && !isCorrectlyDefinedTargetThisForEnumTarget() ) { | ||
ConstructorAccessor constructorAccessor = getConstructorAccessor( resultTypeToMap ); | ||
if ( constructorAccessor != null ) { | ||
|
||
|
@@ -310,6 +310,8 @@ else if ( !method.isUpdateMethod() ) { | |
// map parameters without a mapping | ||
applyParameterNameBasedMapping(); | ||
|
||
// map this enum mapping | ||
applyTargetThisMappingForEnumTarget(); | ||
} | ||
|
||
// Process the unprocessed defined targets | ||
|
@@ -514,7 +516,8 @@ private SubclassMapping createSubclassMapping(SubclassMappingOptions subclassMap | |
private boolean isAbstractReturnTypeAllowed() { | ||
return !method.getOptions().getSubclassMappings().isEmpty() | ||
&& ( method.getOptions().getBeanMapping().getSubclassExhaustiveStrategy().isAbstractReturnTypeAllowed() | ||
|| isCorrectlySealed() ); | ||
|| isCorrectlySealed() ) | ||
|| isCorrectlyDefinedTargetThisForEnumTarget(); | ||
} | ||
|
||
private boolean isCorrectlySealed() { | ||
|
@@ -756,7 +759,9 @@ else if ( !returnType.hasAccessibleConstructor() ) { | |
|
||
private boolean isReturnTypeAbstractOrCanBeConstructed(Type returnType) { | ||
boolean error = true; | ||
if ( !returnType.isAbstract() && !returnType.hasAccessibleConstructor() ) { | ||
if ( !returnType.isAbstract() | ||
&& !returnType.hasAccessibleConstructor() | ||
&& !isCorrectlyDefinedTargetThisForEnumTarget() ) { | ||
ctx | ||
.getMessager() | ||
.printMessage( | ||
|
@@ -1532,6 +1537,38 @@ private void applyTargetThisMapping() { | |
} | ||
} | ||
|
||
/** | ||
* When a single target this mapping is present, and the current target type is an enum. | ||
*/ | ||
private void applyTargetThisMappingForEnumTarget() { | ||
if ( !isCorrectlyDefinedTargetThisForEnumTarget() ) { | ||
return; | ||
} | ||
|
||
for ( MappingReference targetThis : this.targetThisReferences ) { | ||
PropertyMapping propertyMapping = new PropertyMappingBuilder().mappingContext( ctx ) | ||
.sourceMethod( method ) | ||
.sourcePropertyName( "test" ) | ||
.sourceReference( targetThis.getSourceReference() ) | ||
.targetType( this.method.getReturnType() ) | ||
.forgedNamedBased( false ) | ||
.existingVariableNames( existingVariableNames ) | ||
.forgeMethodWithMappingReferences( MappingReferences.empty() ) | ||
.options( method.getOptions().getBeanMapping() ) | ||
.build(); | ||
|
||
if ( propertyMapping != null ) { | ||
propertyMappings.add( propertyMapping ); | ||
} | ||
} | ||
} | ||
|
||
private boolean isCorrectlyDefinedTargetThisForEnumTarget() { | ||
return this.method.getOptions().getMappings().size() == 1 | ||
&& this.method.getOptions().getMappings().iterator().next().getTargetName().equals( "." ) | ||
&& this.method.getReturnType().isEnumType(); | ||
} | ||
|
||
/** | ||
* Iterates over all target properties and all source parameters. | ||
* <p> | ||
|
@@ -2115,6 +2152,10 @@ private boolean doesNotNeedPresenceCheckForSourceParameter(PropertyMapping mappi | |
return mapping.getAssignment().isSourceReferenceParameter(); | ||
} | ||
|
||
public boolean shouldDirectlyReturnOnlyMappingResult() { | ||
return this.returnTypeToConstruct.isEnumType() && this.propertyMappings.size() == 1; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we could come up with more tests. I have no idea what scenarios could be tested, but after checking the coverage report we could provide a test where Not quite sure though, my coverage report seems to be a bit buggy. |
||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
//Needed for Checkstyle, otherwise it fails due to EqualsHashCode rule | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
/* | ||
* 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.assignment; | ||
|
||
import org.mapstruct.ap.internal.model.common.Assignment; | ||
|
||
/** | ||
* Decorates an assignment as a return variable. | ||
* | ||
* @author Ben Zegveld | ||
*/ | ||
public class ResultWrapper extends AssignmentWrapper { | ||
|
||
public ResultWrapper(Assignment decoratedAssignment) { | ||
super( decoratedAssignment, false ); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
<#-- | ||
|
||
Copyright MapStruct Authors. | ||
|
||
Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
--> | ||
<#-- @ftlvariable name="" type="org.mapstruct.ap.internal.model.assignment.ReturnWrapper" --> | ||
${ext.targetBeanName} = <@_assignment/>; | ||
<#macro _assignment> | ||
<@includeModel object=assignment | ||
targetBeanName=ext.targetBeanName | ||
existingInstanceMapping=ext.existingInstanceMapping | ||
targetReadAccessorName=ext.targetReadAccessorName | ||
targetWriteAccessorName=ext.targetWriteAccessorName | ||
targetPropertyName=ext.targetPropertyName | ||
targetType=ext.targetType/> | ||
</#macro> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
/* | ||
* 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.test.bugs._2421; | ||
|
||
import org.mapstruct.Mapper; | ||
import org.mapstruct.Mapping; | ||
import org.mapstruct.MappingConstants; | ||
import org.mapstruct.ValueMapping; | ||
|
||
@Mapper | ||
abstract class Issue2421Mapper { | ||
|
||
@Mapping( target = ".", source = "value" ) | ||
@ValueMapping( target = "C", source = MappingConstants.ANY_REMAINING ) | ||
public abstract EnumExample dtoTestToEnum(InnerDtoTest innerDtoTest); | ||
|
||
} | ||
|
||
enum EnumExample { | ||
A, B, C | ||
} | ||
|
||
class InnerDtoTest { | ||
private String value; | ||
|
||
public String getValue() { | ||
return value; | ||
} | ||
|
||
public void setValue(String value) { | ||
this.value = value; | ||
} | ||
} | ||
|
||
class TargetTest { | ||
private EnumExample enumExampleValue; | ||
|
||
public EnumExample getEnumExampleValue() { | ||
return enumExampleValue; | ||
} | ||
|
||
public void setEnumExampleValue(EnumExample enumExampleValue) { | ||
this.enumExampleValue = enumExampleValue; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
/* | ||
* 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.test.bugs._2421; | ||
|
||
import org.mapstruct.ap.testutil.ProcessorTest; | ||
import org.mapstruct.ap.testutil.WithClasses; | ||
|
||
public class Issue2421Test { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, we would need to change the issue number to 3617 - might aswell add @IssueKey("3617") here. |
||
|
||
@ProcessorTest | ||
@WithClasses( Issue2421Mapper.class ) | ||
void shouldCompile() { | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to have no effect in this case, however this seems fishy - maybe just not set
sourcePropertyName
at all?