GeezerVison is showing github.com — #1018 Use diamond operator for `new` expressions in generated code by filiphr · Pull Request #4045 · mapstruct/mapstruct
Conversation
filiphr
commented
May 1, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
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 #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
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
hduelme reacted with thumbs up emoji
…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.
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.
@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() );
}
}
}
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Comment on lines
+2291
to
+2296
|
|
if ( newInstance != null ) {
|
|
|
types.addAll( newInstance.getImportTypes() );
|
|
|
}
|
|
|
else {
|
|
|
types.addAll( returnTypeToConstruct.getImportTypes() );
|
|
|
}
|
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.
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() );
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
filiphr reacted with thumbs up emoji
Comment on lines
+40
to
+45
|
|
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.
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.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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 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
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
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.
|
hduelme reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Reviewers
hduelme
hduelme left review comments
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|