#3119 Add qualifiedBy and qualifiedByName to SubclassMapping annotation by EvaristeGalois11 · Pull Request #3120

#3119 Add qualifiedBy and qualifiedByName to SubclassMapping annotation by EvaristeGalois11 · Pull Request #3120 · mapstruct/mapstruct

Close #3119

It's a simple fix that just passes along the qualifiers present on the BeanMapping annotation to aid the resolution of the subclass specific method.

I don't know if this is the right approach or you would prefer two specific new attributes on the SubclassMapping annotation. For my needs it is enough.

EvaristeGalois11 changed the title ~~#3119 Pass qualifiers to subclass mapping resolving attempt~~ #3119 Add qualifiedBy and qualifiedByName to SubclassMapping annotation

Dec 25, 2022

As pointed out in #3119 (comment) using the qualifiers present in the BeanMapping annotation wasn't the right thing to do, so i expanded the SubclassMapping annotation with qualifiedBy and qualifiedByName.

Choose a reason for hiding this comment

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

This looks great @EvaristeGalois11. Thanks a lot for the PR and sorry that it took so long to merge it.

I've left one comment, but I'll still merge the PR without it. I'll create an issue for the improvement of the error line location

		diagnostics = @Diagnostic(
		type = ErroneousSubclassQualifiedByMapper.class,
		kind = javax.tools.Diagnostic.Kind.ERROR,
		line = 15,

Choose a reason for hiding this comment

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

Ideally this should be the line with the @SubclassMapping, i.e. line 13

		diagnostics = @Diagnostic(
		type = ErroneousSubclassQualifiedByNameMapper.class,
		kind = javax.tools.Diagnostic.Kind.ERROR,
		line = 15,

Choose a reason for hiding this comment

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

Ideally this should be the line with the @SubclassMapping, i.e. line 13

@EvaristeGalois11 I've created #3202 for the line location. In case you are interested in adding that, you have the first dibs on the issue :).

This looks great @EvaristeGalois11. Thanks a lot for the PR and sorry that it took so long to merge it.

Thank you instead for accepting the PR, I had a really great time working on this project and i plan to contribute a bit more if i can in the future!

And no need to apologize about the wait, we all have personal stuff to take care :D

Thank you instead for accepting the PR, I had a really great time working on this project and i plan to contribute a bit more if i can in the future!

I am happy to hear that it was fun for you to work on this project. We appreciate any help we can get. So I won't say no for any help :)