#1018 Use diamond operator for new expressions in generated code#4045
Conversation
…d code Adds a `NewInstanceCreation` ModelElement that renders `new T<>` and contributes only the raw class to imports, so type parameters used solely in the new-expression are no longer orphaned (Issue mapstruct#955). Also fixes mapstruct#3994's follow-up wildcard cases where explicit type arguments produced uncompilable Java.
hduelme
left a comment
There was a problem hiding this comment.
@filiphr this is a really nice solution. Handling new instance creation in one place is definitely the better approach.
I found one small potential improvement: Simplification by using a ternary expression.
It might also be worth adding a safeguard/check to ensure that an effective target can always be constructed. This isn’t introduced by this change (it existed before), but this could be a good place to catch or handle it more explicitly.
for example, attempting to instantiate an interface or a type without an accessible constructor:
import org.mapstruct.BeanMapping;
import org.mapstruct.Mapper;
import org.mapstruct.MappingTarget;
import org.mapstruct.NullValuePropertyMappingStrategy;
@Mapper
public interface EasyMap {
class To {
private Comparable Comparable;
public Comparable getComparable() {
return Comparable;
}
public void setComparable(Comparable comparable) {
Comparable = comparable;
}
}
class Fromm {
private Comparable Comparable;
public Comparable getComparable() {
return Comparable;
}
public void setComparable(Comparable comparable) {
Comparable = comparable;
}
}
@BeanMapping(nullValuePropertyMappingStrategy = NullValuePropertyMappingStrategy.SET_TO_DEFAULT)
void update(@MappingTarget To to, Fromm from);
}
generates:
import javax.annotation.processing.Generated;
@Generated(
value = "org.mapstruct.ap.MappingProcessor",
date = "2026-05-04T21:34:26+0200",
comments = "version: 1.6.3, compiler: javac, environment: Java 21.0.5 (Eclipse Adoptium)"
)
public class EasyMapImpl implements EasyMap {
@Override
public void update(To to, Fromm from) {
if ( from == null ) {
return;
}
if ( from.getComparable() != null ) {
to.setComparable( from.getComparable() );
}
else {
to.setComparable( new Comparable() );
}
}
}
| if ( newInstance != null ) { | ||
| types.addAll( newInstance.getImportTypes() ); | ||
| } | ||
| else { | ||
| types.addAll( returnTypeToConstruct.getImportTypes() ); | ||
| } |
There was a problem hiding this comment.
Could be written with a ternary expression like
types.addAll( newInstance != null ? newInstance.getImportTypes() : returnTypeToConstruct.getImportTypes() );
or even shorter
types.addAll( (newInstance != null ? newInstance : returnTypeToConstruct).getImportTypes() );
| public static NewInstanceCreation forType(Type targetType) { | ||
| Type effective = targetType.getImplementationType() != null | ||
| ? targetType.getImplementationType() | ||
| : targetType; | ||
| return new NewInstanceCreation( effective ); | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
|
Thanks for the review @hduelme, I would apply the simplification. Regarding the check you mentioned, yes it would be good to add that, but I am not sure that this new place is the right place to do it. I would expose this to be done way earlier in order to provide a proper error message like the line number. |
Adds a
NewInstanceCreationModelElement that rendersnew T<>and contributes only the raw class to imports, so type parameters used solely in the new-expression are no longer orphaned (Issue #955). Also fixes #3994's follow-up wildcard cases where explicit type arguments produced uncompilable Java.Fixes #1018.
@hduelme I went down a rabbit hole with Claude Code while reviewing your PR #4026, so I finally have #1018. Let me know what you think