Skip to content

#1018 Use diamond operator for new expressions in generated code #4045

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

Merged
filiphr merged 2 commits into from
May 4, 2026
+470 −203
Merged

#1018 Use diamond operator for new expressions in generated code #4045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
79aca2e
#1018 Use diamond operator for `new` expressions in generated code
filiphr May 1, 2026
5651a8a
Apply review feedback
filiphr May 4, 2026
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
11 changes: 10 additions & 1 deletion processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import org.mapstruct.ap.internal.model.common.Assignment;
import org.mapstruct.ap.internal.model.common.BuilderType;
import org.mapstruct.ap.internal.model.common.FormattingParameters;
import org.mapstruct.ap.internal.model.common.NewInstanceCreation;
import org.mapstruct.ap.internal.model.common.Parameter;
import org.mapstruct.ap.internal.model.common.ParameterBinding;
import org.mapstruct.ap.internal.model.common.PresenceCheck;
Expand Down Expand Up @@ -97,6 +98,7 @@ public class BeanMappingMethod extends NormalTypeMappingMethod {
private final List<PropertyMapping> constructorConstantMappings;
private final List<SubclassMapping> subclassMappings;
private final Type returnTypeToConstruct;
private final NewInstanceCreation newInstance;
private final BuilderType returnTypeBuilder;
private final MethodReference finalizerMethod;
private final String finalizedResultName;
Expand Down Expand Up @@ -2184,6 +2186,9 @@ else if ( sourceParameterNames.contains( mapping.getSourceBeanName() ) ) {
}
}
this.returnTypeToConstruct = returnTypeToConstruct;
this.newInstance = ( returnTypeToConstruct != null && getFactoryMethod() == null )
? NewInstanceCreation.forType( returnTypeToConstruct )
: null;
this.subclassMappings = subclassMappings;
this.sourceParametersReassignments = sourceParametersReassignments;
}
Expand Down Expand Up @@ -2246,6 +2251,10 @@ public Type getReturnTypeToConstruct() {
return returnTypeToConstruct;
}

public NewInstanceCreation getNewInstance() {
return newInstance;
}

public boolean hasSubclassMappings() {
return !subclassMappings.isEmpty();
}
Expand Down Expand Up @@ -2279,7 +2288,7 @@ public Set<Type> getImportTypes() {
}

if ( returnTypeToConstruct != null ) {
types.addAll( returnTypeToConstruct.getImportTypes() );
types.addAll( newInstance != null ? newInstance.getImportTypes() : returnTypeToConstruct.getImportTypes() );
}
if ( returnTypeBuilder != null ) {
types.add( returnTypeBuilder.getOwningType() );
Expand Down
11 changes: 9 additions & 2 deletions processor/src/main/java/org/mapstruct/ap/internal/model/IterableCreation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import java.util.Set;

import org.mapstruct.ap.internal.model.common.ModelElement;
import org.mapstruct.ap.internal.model.common.NewInstanceCreation;
import org.mapstruct.ap.internal.model.common.Parameter;
import org.mapstruct.ap.internal.model.common.Type;

Expand All @@ -27,13 +28,15 @@ public class IterableCreation extends ModelElement {
private final Type resultType;
private final Parameter sourceParameter;
private final MethodReference factoryMethod;
private final NewInstanceCreation newInstance;
private final boolean canUseSize;
private final boolean loadFactorAdjustment;

private IterableCreation(Type resultType, Parameter sourceParameter, MethodReference factoryMethod) {
this.resultType = resultType;
this.sourceParameter = sourceParameter;
this.factoryMethod = factoryMethod;
this.newInstance = factoryMethod == null ? NewInstanceCreation.forType( resultType ) : null;
this.canUseSize = ( sourceParameter.getType().isCollectionOrMapType() ||
sourceParameter.getType().isArrayType() )
&& resultType.getImplementation() != null && resultType.getImplementation().hasInitialCapacityConstructor();
Expand All @@ -57,6 +60,10 @@ public MethodReference getFactoryMethod() {
return this.factoryMethod;
}

public NewInstanceCreation getNewInstance() {
return newInstance;
}

public boolean isCanUseSize() {
return canUseSize;
}
Expand All @@ -68,8 +75,8 @@ public boolean isLoadFactorAdjustment() {
@Override
public Set<Type> getImportTypes() {
Set<Type> types = new HashSet<>();
if ( factoryMethod == null && resultType.getImplementationType() != null ) {
types.addAll( resultType.getImplementationType().getImportTypes() );
if ( newInstance != null ) {
types.addAll( newInstance.getImportTypes() );
}

if ( isEnumSet() ) {
Expand Down
2 changes: 1 addition & 1 deletion ...ruct/ap/internal/model/assignment/ExistingInstanceSetterWrapperForCollectionsAndMaps.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public ExistingInstanceSetterWrapperForCollectionsAndMaps(Assignment decoratedAs
public Set<Type> getImportTypes() {
Set<Type> imported = new HashSet<>( super.getImportTypes() );
if ( isMapNullToDefault() && ( targetType.getImplementationType() != null ) ) {
imported.add( targetType.getImplementationType() );
imported.addAll( getNewInstance().getImportTypes() );
}
return imported;
}
Expand Down
15 changes: 8 additions & 7 deletions ...pstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMapsWithNullCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.util.Set;

import org.mapstruct.ap.internal.model.common.Assignment;
import org.mapstruct.ap.internal.model.common.NewInstanceCreation;
import org.mapstruct.ap.internal.model.common.Type;
import org.mapstruct.ap.internal.model.common.TypeFactory;

Expand All @@ -26,6 +27,7 @@ public class SetterWrapperForCollectionsAndMapsWithNullCheck extends WrapperForC

private final Type targetType;
private final TypeFactory typeFactory;
private final NewInstanceCreation newInstance;

public SetterWrapperForCollectionsAndMapsWithNullCheck(Assignment decoratedAssignment,
List<Type> thrownTypesToExclude,
Expand All @@ -40,19 +42,14 @@ public SetterWrapperForCollectionsAndMapsWithNullCheck(Assignment decoratedAssig
);
this.targetType = targetType;
this.typeFactory = typeFactory;
this.newInstance = NewInstanceCreation.forType( targetType );
}

@Override
public Set<Type> getImportTypes() {
Set<Type> imported = new HashSet<>( super.getImportTypes() );
if ( isDirectAssignment() ) {
if ( targetType.getImplementationType() != null ) {
imported.addAll( targetType.getImplementationType().getImportTypes() );
}
else {
imported.addAll( targetType.getImportTypes() );
}

imported.addAll( newInstance.getImportTypes() );
if ( isEnumSet() ) {
imported.add( typeFactory.getType( EnumSet.class ) );
}
Expand All @@ -63,6 +60,10 @@ public Set<Type> getImportTypes() {
return imported;
}

public NewInstanceCreation getNewInstance() {
return newInstance;
}

public boolean isDirectAssignment() {
return getType() == DIRECT;
}
Expand Down
28 changes: 9 additions & 19 deletions processor/src/main/java/org/mapstruct/ap/internal/model/assignment/UpdateWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.util.Set;

import org.mapstruct.ap.internal.model.common.Assignment;
import org.mapstruct.ap.internal.model.common.NewInstanceCreation;
import org.mapstruct.ap.internal.model.common.Type;

/**
Expand All @@ -22,7 +23,7 @@ public class UpdateWrapper extends AssignmentWrapper {

private final List<Type> thrownTypesToExclude;
private final Assignment factoryMethod;
private final Type targetImplementationType;
private final NewInstanceCreation newInstance;
private final boolean includeSourceNullCheck;
private final boolean setExplicitlyToNull;
private final boolean setExplicitlyToDefault;
Expand All @@ -40,27 +41,13 @@ public UpdateWrapper( Assignment decoratedAssignment,
super( decoratedAssignment, fieldAssignment );
this.thrownTypesToExclude = thrownTypesToExclude;
this.factoryMethod = factoryMethod;
this.targetImplementationType = determineImplType( factoryMethod, targetType );
this.newInstance = ( factoryMethod == null ) ? NewInstanceCreation.forType( targetType ) : null;
this.includeSourceNullCheck = includeSourceNullCheck;
this.setExplicitlyToDefault = setExplicitlyToDefault;
this.setExplicitlyToNull = setExplicitlyToNull;
this.mustCastForNull = mustCastForNull;
}

private static Type determineImplType(Assignment factoryMethod, Type targetType) {
if ( factoryMethod != null ) {
//If we have factory method then we won't use the targetType
return null;
}
if ( targetType.getImplementationType() != null ) {
// it's probably a collection or something
return targetType.getImplementationType();
}

// no factory method means we create a new instance ourselves and thus need to import the type
return targetType;
}

@Override
public List<Type> getThrownTypes() {
List<Type> parentThrownTypes = super.getThrownTypes();
Expand All @@ -81,9 +68,8 @@ public Set<Type> getImportTypes() {
if ( factoryMethod != null ) {
imported.addAll( factoryMethod.getImportTypes() );
}
if ( targetImplementationType != null ) {
imported.add( targetImplementationType );
imported.addAll( targetImplementationType.getTypeParameters() );
if ( newInstance != null ) {
imported.addAll( newInstance.getImportTypes() );
}
return imported;
}
Expand All @@ -92,6 +78,10 @@ public Assignment getFactoryMethod() {
return factoryMethod;
}

public NewInstanceCreation getNewInstance() {
return newInstance;
}

public boolean isIncludeSourceNullCheck() {
return includeSourceNullCheck;
}
Expand Down
63 changes: 63 additions & 0 deletions processor/src/main/java/org/mapstruct/ap/internal/model/common/NewInstanceCreation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* 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.common;

import java.util.Set;

/**
* Model element representing the head of a {@code new T<>(...)} expression. It always renders the diamond
* operator for generic types, leaving any type-argument inference to the surrounding Java context.
* <p>
* The caller is responsible for emitting the parenthesised argument list after the model.
* <p>
* Imports contributed by this element only include the raw constructor type, not its type parameters: the
* generated source never references the parameter classes through this expression, so they are not needed
* here. If the surrounding code references a type parameter elsewhere (e.g. in a variable declaration or a
* method signature), that reference contributes its own imports through its own model element.
*
* @author Filip Hrisafov
*/
public class NewInstanceCreation extends ModelElement {

private final Type type;
private final Type rawType;

private NewInstanceCreation(Type type) {
this.type = type;
this.rawType = type.asRawType();
}

/**
* Creates a {@link NewInstanceCreation} for the given target type. If the target has an implementation
* type (e.g. {@code Collection} -> {@code ArrayList}), the implementation type is used.
*
* @param targetType the target type to be instantiated; must not be {@code null}
* @return a new model
*/
public static NewInstanceCreation forType(Type targetType) {
Type effective = targetType.getImplementationType() != null
? targetType.getImplementationType()
: targetType;
return new NewInstanceCreation( effective );
}
Comment on lines +40 to +45

@hduelme hduelme May 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A check could be added to verify whether an effective target can be constructed.

This isn’t a new issue—it existed before this change—but this might be a good place to catch or handle it.

@filiphr filiphr May 4, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice catch. However, I do not believe that this is the right place for this, we need to do this way earlier in order to provide a proper error message, like the line of the @Mapping


public Type getType() {
return type;
}

public Type getRawType() {
return rawType;
}

public boolean isGeneric() {
return !type.getTypeParameters().isEmpty();
}

@Override
public Set<Type> getImportTypes() {
return rawType.getImportTypes();
}
}
2 changes: 1 addition & 1 deletion processor/src/main/resources/org/mapstruct/ap/internal/model/BeanMappingMethod.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@

<@includeModel object=returnTypeToConstruct/> ${resultName} = <@includeModel object=factoryMethod targetType=returnTypeToConstruct/>;
<#else >
<@includeModel object=returnTypeToConstruct/> ${resultName} = <#if factoryMethod??><@includeModel object=factoryMethod targetType=returnTypeToConstruct/><#else>new <@includeModel object=returnTypeToConstruct/>()</#if>;
<@includeModel object=returnTypeToConstruct/> ${resultName} = <#if factoryMethod??><@includeModel object=factoryMethod targetType=returnTypeToConstruct/><#else><@includeModel object=newInstance/>()</#if>;
</#if>

</#if>
Expand Down
4 changes: 2 additions & 2 deletions processor/src/main/resources/org/mapstruct/ap/internal/model/IterableCreation.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@
<#if resultType.implementation.factoryMethodName?? && ext.useSizeIfPossible?? && ext.useSizeIfPossible && canUseSize>
<@includeModel object=resultType.implementationType raw=true />.${resultType.implementation.factoryMethodName}( <@sizeForCreation /> )
<#else>
new <@includeModel object=resultType.implementationType/><#if ext.useSizeIfPossible?? && ext.useSizeIfPossible && canUseSize>( <@sizeForCreation /> )<#else>()</#if>
<@includeModel object=newInstance/><#if ext.useSizeIfPossible?? && ext.useSizeIfPossible && canUseSize>( <@sizeForCreation /> )<#else>()</#if>
</#if>
<#else>
new <@includeModel object=resultType/>()
<@includeModel object=newInstance/>()
</#if>
</@compress>
<#macro sizeForCreation>
Expand Down
2 changes: 1 addition & 1 deletion ...truct/ap/internal/model/assignment/ExistingInstanceSetterWrapperForCollectionsAndMaps.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,6 @@
<#if enumSet>
EnumSet.copyOf( ${nullCheckLocalVarName} )
<#else>
new <#if ext.targetType.implementationType??><@includeModel object=ext.targetType.implementationType/><#else><@includeModel object=ext.targetType/></#if>( ${nullCheckLocalVarName} )
<@includeModel object=newInstance/>( ${nullCheckLocalVarName} )
</#if>
</@compress></#macro>
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 @@ -16,7 +16,7 @@
-->
<#macro callTargetWriteAccessor>
<@lib.handleLocalVarNullCheck needs_explicit_local_var=directAssignment>
<#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>();
<#if ext.targetType.implementationType??><@includeModel object=ext.targetType.implementationType/><#else><@includeModel object=ext.targetType/></#if> ${instanceVar} = <@includeModel object=newInstance/>();
${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>;
</@lib.handleLocalVarNullCheck>
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 @@ -26,6 +26,6 @@
<#if enumSet>
EnumSet.copyOf( ${nullCheckLocalVarName} )
<#else>
new <#if ext.targetType.implementationType??><@includeModel object=ext.targetType.implementationType/><#else><@includeModel object=ext.targetType/></#if>( ${nullCheckLocalVarName} )
<@includeModel object=newInstance/>( ${nullCheckLocalVarName} )
</#if>
</@compress></#macro>
11 changes: 11 additions & 0 deletions processor/src/main/resources/org/mapstruct/ap/internal/model/common/NewInstanceCreation.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<#--

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.common.NewInstanceCreation" -->
<@compress single_line=true>
new <@includeModel object=rawType/><#if generic><></#if>
</@compress>
6 changes: 4 additions & 2 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 @@ -170,19 +170,21 @@ Performs a default assignment with a default value.
macro: constructTargetObject

purpose: Either call the constructor of the target object directly or of the implementing type.
Emits the diamond operator for generic types so type-arguments are inferred from the
assignment context.
-->
<#-- @ftlvariable name="targetType" type="org.mapstruct.ap.internal.model.common.Type" -->
<#macro constructTargetObject targetType><@compress single_line=true>
<#if targetType.implementationType??>
new <@includeModel object=targetType.implementationType/>()
new <@includeModel object=targetType.implementationType raw=true/><#if targetType.implementationType.typeParameters?size != 0><></#if>()
<#elseif targetType.arrayType>
new <@includeModel object=targetType.componentType/>[0]
<#elseif targetType.sensibleDefault??>
${targetType.sensibleDefault}
<#elseif targetType.optionalType>
<@includeModel object=targetType.asRawType()/>.of( <@constructTargetObject targetType=targetType.optionalBaseType/> )
<#else>
new <@includeModel object=targetType/>()
new <@includeModel object=targetType raw=true/><#if targetType.typeParameters?size != 0><></#if>()
</#if>
</@compress></#macro>
<#--
Expand Down
20 changes: 20 additions & 0 deletions processor/src/test/java/org/mapstruct/ap/test/generics/wildcard/BoundCopyMapper.java
View file
Open in desktop
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.test.generics.wildcard;

import org.mapstruct.Mapper;

@Mapper
public interface BoundCopyMapper {

CollectionSuperTypes copySuperCollection(CollectionSuperTypes collectionSuperTypes);

CollectionExtendTypes copyExtendsCollection(CollectionExtendTypes collectionExtendTypes);

MapSuperType copySuperMap(MapSuperType mapSuperType);

MapExtendType copyExtendMap(MapExtendType mapExtendType);
}
Loading
Loading