Skip to content

use diamond operator for collections and maps on SetterWrapperForCollectionsAndMapsWithNullCheck#4026

Closed
hduelme wants to merge 2 commits into
mapstruct:mainfrom
hduelme:use-diamond-operator-for-collections-and-maps
Closed

use diamond operator for collections and maps on SetterWrapperForCollectionsAndMapsWithNullCheck#4026
hduelme wants to merge 2 commits into
mapstruct:mainfrom
hduelme:use-diamond-operator-for-collections-and-maps

Conversation

@hduelme
Copy link
Copy Markdown
Contributor

@hduelme hduelme commented Mar 29, 2026

In #3994 I found that MapStruct currently generates invalide copy constructors when trying to map from Collection/Map with <? extends T> or <? super T> bounds to itself.

I fixed the issue by using the diamond operator for SetterWrapperForCollectionsAndMapsWithNullCheck on the constructor.

I found that the same issue is present in ExistingInstanceSetterWrapperForCollectionsAndMaps.ftl. I didn't include that in this PR, as MapStruct tries to use addAll(Collection<? extends E> c)/putAll(Map<? extends K, ? extends V> m). It is impossible to add a super bounded to to an extend bounded one, example

if ( extendTypes.getSimpleObjectsCollection() != null ) {
        Collection<? super SimpleObject> collection = types.getSimpleObjectsCollection();
        if ( collection != null ) {
            extendTypes.getSimpleObjectsCollection().clear();
            extendTypes.getSimpleObjectsCollection().addAll( collection ); // Error
        }
    }
    else {
        Collection<? super SimpleObject> collection = types.getSimpleObjectsCollection();
        if ( collection != null ) {
            extendTypes.setSimpleObjectsCollection( new ArrayList<SimpleObject>( collection ) );
        }
    }
}

I am not sure what the correct solution would be:

  • Only override the values with a copy constructor (linke in the else branch)
  • Reject the complete mapping as impossible

@filiphr
Copy link
Copy Markdown
Member

filiphr commented May 1, 2026

@hduelme I went down a rabbit hole while reviewing his one and I came up with #4045, which finally makes #1018 possible. I'll close this one in favor of the new PR. Please feel free to review the PR and let me know what you think

@filiphr filiphr closed this May 1, 2026
@filiphr filiphr removed this from the 1.7.0.Beta2 milestone May 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants