Skip to content

#3695 Use retainAll() for Set and Map instead of clear() #3696

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

#3695 Use retainAll() for Set and Map instead of clear() #3696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ public class Type extends ModelElement implements Comparable<Type> {
private final boolean isIterableType;
private final boolean isCollectionType;
private final boolean isMapType;
private final boolean isSetType;
private final boolean isVoid;
private final boolean isStream;
private final boolean isLiteral;
Expand Down Expand Up @@ -145,7 +146,7 @@ public Type(TypeUtils typeUtils, ElementUtils elementUtils, TypeFactory typeFact
List<Type> typeParameters, ImplementationType implementationType, Type componentType,
String packageName, String name, String qualifiedName,
boolean isInterface, boolean isEnumType, boolean isIterableType,
boolean isCollectionType, boolean isMapType, boolean isStreamType,
boolean isCollectionType, boolean isMapType, boolean isSetType, boolean isStreamType,
Map<String, String> toBeImportedTypes,
Map<String, String> notToBeImportedTypes,
Boolean isToBeImported,
Expand All @@ -171,6 +172,7 @@ public Type(TypeUtils typeUtils, ElementUtils elementUtils, TypeFactory typeFact
this.isIterableType = isIterableType;
this.isCollectionType = isCollectionType;
this.isMapType = isMapType;
this.isSetType = isSetType;
this.isStream = isStreamType;
this.isVoid = typeMirror.getKind() == TypeKind.VOID;
this.isLiteral = isLiteral;
Expand Down Expand Up @@ -348,6 +350,10 @@ public boolean isMapType() {
return isMapType;
}

public boolean isSetType() {
return isSetType;
}

private boolean hasStringMapSignature() {
if ( isMapType() ) {
List<Type> typeParameters = getTypeParameters();
Expand Down Expand Up @@ -553,6 +559,7 @@ public Type erasure() {
isIterableType,
isCollectionType,
isMapType,
isSetType,
isStream,
toBeImportedTypes,
notToBeImportedTypes,
Expand Down Expand Up @@ -596,6 +603,7 @@ public Type withoutBounds() {
isIterableType,
isCollectionType,
isMapType,
isSetType,
isStream,
toBeImportedTypes,
notToBeImportedTypes,
Expand Down
5 changes: 5 additions & 0 deletions processor/src/main/java/org/mapstruct/ap/internal/model/common/TypeFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ public class TypeFactory {
private final TypeMirror iterableType;
private final TypeMirror collectionType;
private final TypeMirror mapType;
private final TypeMirror setType;
private final TypeMirror streamType;

private final Map<String, ImplementationType> implementationTypes = new HashMap<>();
Expand All @@ -116,6 +117,7 @@ public TypeFactory(ElementUtils elementUtils, TypeUtils typeUtils, FormattingMes
collectionType =
typeUtils.erasure( elementUtils.getTypeElement( Collection.class.getCanonicalName() ).asType() );
mapType = typeUtils.erasure( elementUtils.getTypeElement( Map.class.getCanonicalName() ).asType() );
setType = typeUtils.erasure( elementUtils.getTypeElement( Set.class.getCanonicalName() ).asType() );
TypeElement streamTypeElement = elementUtils.getTypeElement( JavaStreamConstants.STREAM_FQN );
streamType = streamTypeElement == null ? null : typeUtils.erasure( streamTypeElement.asType() );

Expand Down Expand Up @@ -236,6 +238,7 @@ private Type getType(TypeMirror mirror, boolean isLiteral, Boolean alwaysImport)
boolean isIterableType = typeUtils.isSubtypeErased( mirror, iterableType );
boolean isCollectionType = typeUtils.isSubtypeErased( mirror, collectionType );
boolean isMapType = typeUtils.isSubtypeErased( mirror, mapType );
boolean isSetType = typeUtils.isSubtypeErased( mirror, setType );
boolean isStreamType = streamType != null && typeUtils.isSubtypeErased( mirror, streamType );

boolean isEnumType;
Expand Down Expand Up @@ -345,6 +348,7 @@ else if ( mirror.getKind() == TypeKind.TYPEVAR ) {
isIterableType,
isCollectionType,
isMapType,
isSetType,
isStreamType,
toBeImportedTypes,
notToBeImportedTypes,
Expand Down Expand Up @@ -571,6 +575,7 @@ private ImplementationType getImplementationType(TypeMirror mirror) {
implementationType.isIterableType(),
implementationType.isCollectionType(),
implementationType.isMapType(),
implementationType.isSetType(),
implementationType.isStreamType(),
toBeImportedTypes,
notToBeImportedTypes,
Expand Down
6 changes: 5 additions & 1 deletion processor/src/main/resources/org/mapstruct/ap/internal/model/IterableMappingMethod.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@
</#if>
<#else>
<#if existingInstanceMapping>
${resultName}.clear();
<#if resultType.setType>
${resultName}.retainAll( java.util.stream.StreamSupport.stream( ${sourceParameter.name}.spliterator(), false ).collect( java.util.stream.Collectors.toSet() ) );
<#else>
${resultName}.clear();
</#if>
<#else>
<#-- Use the interface type on the left side, except it is java.lang.Iterable; use the implementation type - if present - on the right side -->
<@iterableLocalVarDef/> ${resultName} = <@includeModel object=iterableCreation useSizeIfPossible=true/>;
Expand Down
2 changes: 1 addition & 1 deletion processor/src/main/resources/org/mapstruct/ap/internal/model/MapMappingMethod.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
}

<#if existingInstanceMapping>
${resultName}.clear();
${resultName}.entrySet().retainAll( ${sourceParameter.name}.entrySet() );
<#else>
<@includeModel object=resultType /> ${resultName} = <@includeModel object=iterableCreation useSizeIfPossible=true/>;
</#if>
Expand Down
21 changes: 13 additions & 8 deletions ...truct/ap/internal/model/assignment/ExistingInstanceSetterWrapperForCollectionsAndMaps.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,19 @@
<@lib.sourceLocalVarAssignment/>
<@lib.handleExceptions>
if ( ${ext.targetBeanName}.${ext.targetReadAccessorName} != null ) {
<@lib.handleLocalVarNullCheck needs_explicit_local_var=false>
${ext.targetBeanName}.${ext.targetReadAccessorName}.clear();
${ext.targetBeanName}.${ext.targetReadAccessorName}.<#if ext.targetType.collectionType>addAll<#else>putAll</#if>( <@lib.handleWithAssignmentOrNullCheckVar/> );
</@lib.handleLocalVarNullCheck>
<#if !ext.defaultValueAssignment?? && !sourcePresenceCheckerReference?? && includeElseBranch>else {<#-- the opposite (defaultValueAssignment) case is handeld inside lib.handleLocalVarNullCheck -->
${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWrite><#if mapNullToDefault><@lib.initTargetObject/><#else>null</#if></@lib.handleWrite>;
}
<@lib.handleLocalVarNullCheck needs_explicit_local_var=false cleanBefore=true ; sourcePresent>
<#if !sourcePresent??>
${ext.targetBeanName}.${ext.targetReadAccessorName}.<#if ext.targetType.mapType>putAll<#else>addAll</#if>( <@lib.handleWithAssignmentOrNullCheckVar/> );
<#elseif !sourcePresent>
<#if !ext.defaultValueAssignment?? && !sourcePresenceCheckerReference?? && includeElseBranch><#-- the opposite (defaultValueAssignment) case is handeld inside lib.handleLocalVarNullCheck -->
${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWrite><#if mapNullToDefault><@lib.initTargetObject/><#else>null</#if></@lib.handleWrite>;
</#if>
<#elseif ext.targetType.setType || ext.targetType.mapType>
${ext.targetBeanName}.${ext.targetReadAccessorName}.<#if ext.targetType.mapType>entrySet().</#if>retainAll( <@lib.handleWithAssignmentOrNullCheckVar/><#if sourceType.mapType>.entrySet()</#if> );
<#else>
${ext.targetBeanName}.${ext.targetReadAccessorName}.clear();
</#if>
</@lib.handleLocalVarNullCheck>
}
else {
<@callTargetWriteAccessor/>
Expand All @@ -27,7 +32,7 @@
assigns the target via the regular target write accessor (usually the setter)
-->
<#macro callTargetWriteAccessor>
<@lib.handleLocalVarNullCheck needs_explicit_local_var=directAssignment>
<@lib.handleLocalVarNullCheck needs_explicit_local_var=directAssignment cleanBefore=false>
${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWrite><#if directAssignment><@wrapLocalVarInCollectionInitializer/><#else><@lib.handleWithAssignmentOrNullCheckVar/></#if></@lib.handleWrite>;
</@lib.handleLocalVarNullCheck>
</#macro>
Expand Down
17 changes: 12 additions & 5 deletions ...sources/org/mapstruct/ap/internal/model/assignment/GetterWrapperForCollectionsAndMaps.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,18 @@
<@lib.sourceLocalVarAssignment/>
if ( ${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWriteAccesing /> != null ) {
<@lib.handleExceptions>
<#if ext.existingInstanceMapping>
${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWriteAccesing />.clear();
</#if>
<@lib.handleLocalVarNullCheck needs_explicit_local_var=false>
${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWriteAccesing />.<#if ext.targetType.collectionType>addAll<#else>putAll</#if>( <@lib.handleWithAssignmentOrNullCheckVar/> );
<@lib.handleLocalVarNullCheck needs_explicit_local_var=false cleanBefore=true ; sourcePresent>
<#if sourcePresent??>
<#if ext.existingInstanceMapping>
<#if sourcePresent && (ext.targetType.setType || ext.targetType.mapType)>
${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWriteAccesing />.<#if ext.targetType.mapType>entrySet().</#if>retainAll( <@lib.handleWithAssignmentOrNullCheckVar/><#if sourceType.mapType>.entrySet()</#if> );
<#else>
${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWriteAccesing />.clear();
</#if>
</#if>
<#else>
${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWriteAccesing />.<#if ext.targetType.mapType>putAll<#else>addAll</#if>( <@lib.handleWithAssignmentOrNullCheckVar/> );
</#if>
</@lib.handleLocalVarNullCheck>
</@lib.handleExceptions>
}
2 changes: 1 addition & 1 deletion .../mapstruct/ap/internal/model/assignment/NewInstanceSetterWrapperForCollectionsAndMaps.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
assigns the target via the regular target write accessor (usually the setter)
-->
<#macro callTargetWriteAccessor>
<@lib.handleLocalVarNullCheck needs_explicit_local_var=directAssignment>
<@lib.handleLocalVarNullCheck needs_explicit_local_var=directAssignment cleanBefore=false>
<#if ext.targetType.implementationType??><@includeModel object=ext.targetType.implementationType/><#else><@includeModel object=ext.targetType/></#if> ${instanceVar} = new <#if ext.targetType.implementationType??><@includeModel object=ext.targetType.implementationType/><#else><@includeModel object=ext.targetType/></#if>();
${instanceVar}.<#if ext.targetType.collectionType>addAll<#else>putAll</#if>( ${nullCheckLocalVarName} );
<#if ext.targetBeanName?has_content>${ext.targetBeanName}.</#if>${ext.targetWriteAccessorName}<@lib.handleWrite>${instanceVar}</@lib.handleWrite>;
Expand Down
2 changes: 1 addition & 1 deletion ...apstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMapsWithNullCheck.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
assigns the target via the regular target write accessor (usually the setter)
-->
<#macro callTargetWriteAccessor>
<@lib.handleLocalVarNullCheck needs_explicit_local_var=directAssignment>
<@lib.handleLocalVarNullCheck needs_explicit_local_var=directAssignment cleanBefore=false>
<#if ext.targetBeanName?has_content>${ext.targetBeanName}.</#if>${ext.targetWriteAccessorName}<@lib.handleWrite><#if directAssignment><@wrapLocalVarInCollectionInitializer/><#else><@lib.handleWithAssignmentOrNullCheckVar/></#if></@lib.handleWrite>;
</@lib.handleLocalVarNullCheck>
</#macro>
Expand Down
18 changes: 12 additions & 6 deletions processor/src/main/resources/org/mapstruct/ap/internal/model/macro/CommonMacros.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,29 +58,35 @@
requires: caller to implement String:getNullCheckLocalVarName()
caller to implement Type:getNullCheckLocalVarType()
-->
<#macro handleLocalVarNullCheck needs_explicit_local_var>
<#macro handleLocalVarNullCheck needs_explicit_local_var cleanBefore>
<#if sourcePresenceCheckerReference??>
if ( <@includeModel object=sourcePresenceCheckerReference
targetType=ext.targetType
sourcePropertyName=ext.sourcePropertyName
targetPropertyName=ext.targetPropertyName /> ) {
<#if needs_explicit_local_var>
<@includeModel object=nullCheckLocalVarType/> ${nullCheckLocalVarName} = <@lib.handleAssignment/>;
<#nested>
<#else>
<#nested>
<@includeModel object=nullCheckLocalVarType/> ${nullCheckLocalVarName} = <@lib.handleAssignment/>;
</#if>
<#if cleanBefore><#nested true></#if>
<#nested>
}
<#else>
<@includeModel object=nullCheckLocalVarType/> ${nullCheckLocalVarName} = <@lib.handleAssignment/>;
<@includeModel object=nullCheckLocalVarType/> ${nullCheckLocalVarName} = <@lib.handleAssignment/>;
if ( ${nullCheckLocalVarName} != null ) {
<#if cleanBefore><#nested true></#if>
<#nested>
}
</#if>
<#local nestedContent><#nested false></#local>
<#if ext.defaultValueAssignment?? >
else {
<#if cleanBefore><#nested false></#if>
<@handeDefaultAssigment/>
}
<#elseif cleanBefore && nestedContent?trim?has_content>
else {
<#nested false>
}
</#if>
</#macro>
<#--
Expand Down
1 change: 1 addition & 0 deletions .../src/test/java/org/mapstruct/ap/internal/model/common/DateFormatValidatorFactoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ private Type typeWithFQN(String fullQualifiedName) {
false,
false,
false,
false,
new HashMap<>( ),
new HashMap<>( ),
false,
Expand Down
1 change: 1 addition & 0 deletions ...or/src/test/java/org/mapstruct/ap/internal/model/common/DefaultConversionContextTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ private Type typeWithFQN(String fullQualifiedName) {
false,
false,
false,
false,
new HashMap<>( ),
new HashMap<>( ),
false,
Expand Down
2 changes: 1 addition & 1 deletion ...src/test/resources/fixtures/org/mapstruct/ap/test/bugs/_3591/ContainerBeanMapperImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public ContainerBeanDto mapWithMapMapping(ContainerBean containerBean, Container
if ( containerBeanDto.getBeanMap() != null ) {
Map<String, ContainerBeanDto> map = stringContainerBeanMapToStringContainerBeanDtoMap( containerBean.getBeanMap() );
if ( map != null ) {
containerBeanDto.getBeanMap().clear();
containerBeanDto.getBeanMap().entrySet().retainAll( map.entrySet() );
containerBeanDto.getBeanMap().putAll( map );
}
else {
Expand Down
16 changes: 8 additions & 8 deletions ...resources/fixtures/org/mapstruct/ap/test/bugs/_913/DomainDtoWithNcvsAlwaysMapperImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public void update(DtoWithPresenceCheck source, Domain target) {

if ( target.getStrings() != null ) {
if ( source.hasStrings() ) {
target.getStrings().clear();
target.getStrings().retainAll( source.getStrings() );
target.getStrings().addAll( source.getStrings() );
}
}
Expand All @@ -73,7 +73,7 @@ public void update(DtoWithPresenceCheck source, Domain target) {
}
if ( target.getLongs() != null ) {
if ( source.hasStrings() ) {
target.getLongs().clear();
target.getLongs().retainAll( stringListToLongSet( source.getStrings() ) );
target.getLongs().addAll( stringListToLongSet( source.getStrings() ) );
}
}
Expand All @@ -84,7 +84,7 @@ public void update(DtoWithPresenceCheck source, Domain target) {
}
if ( target.getStringsInitialized() != null ) {
if ( source.hasStringsInitialized() ) {
target.getStringsInitialized().clear();
target.getStringsInitialized().retainAll( source.getStringsInitialized() );
target.getStringsInitialized().addAll( source.getStringsInitialized() );
}
}
Expand All @@ -96,7 +96,7 @@ public void update(DtoWithPresenceCheck source, Domain target) {
}
if ( target.getLongsInitialized() != null ) {
if ( source.hasStringsInitialized() ) {
target.getLongsInitialized().clear();
target.getLongsInitialized().retainAll( stringListToLongSet( source.getStringsInitialized() ) );
target.getLongsInitialized().addAll( stringListToLongSet( source.getStringsInitialized() ) );
}
}
Expand Down Expand Up @@ -133,7 +133,7 @@ public Domain updateWithReturn(DtoWithPresenceCheck source, Domain target) {

if ( target.getStrings() != null ) {
if ( source.hasStrings() ) {
target.getStrings().clear();
target.getStrings().retainAll( source.getStrings() );
target.getStrings().addAll( source.getStrings() );
}
}
Expand All @@ -145,7 +145,7 @@ public Domain updateWithReturn(DtoWithPresenceCheck source, Domain target) {
}
if ( target.getLongs() != null ) {
if ( source.hasStrings() ) {
target.getLongs().clear();
target.getLongs().retainAll( stringListToLongSet( source.getStrings() ) );
target.getLongs().addAll( stringListToLongSet( source.getStrings() ) );
}
}
Expand All @@ -156,7 +156,7 @@ public Domain updateWithReturn(DtoWithPresenceCheck source, Domain target) {
}
if ( target.getStringsInitialized() != null ) {
if ( source.hasStringsInitialized() ) {
target.getStringsInitialized().clear();
target.getStringsInitialized().retainAll( source.getStringsInitialized() );
target.getStringsInitialized().addAll( source.getStringsInitialized() );
}
}
Expand All @@ -168,7 +168,7 @@ public Domain updateWithReturn(DtoWithPresenceCheck source, Domain target) {
}
if ( target.getLongsInitialized() != null ) {
if ( source.hasStringsInitialized() ) {
target.getLongsInitialized().clear();
target.getLongsInitialized().retainAll( stringListToLongSet( source.getStringsInitialized() ) );
target.getLongsInitialized().addAll( stringListToLongSet( source.getStringsInitialized() ) );
}
}
Expand Down
Loading