Skip to content
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.

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

#129 improve support of mapping composition #134

Merged
merged 1 commit into from
May 7, 2023

Conversation

thunderhook
Copy link
Contributor

@thunderhook thunderhook commented May 1, 2023

This PR is about #129.

Mapping composition (or meta annotations) were already tacklet with the following commit (target version 1.2.1): 35f1712

However, the used utility method com.intellij.codeInsight.MetaAnnotationUtil#findMetaAnnotations returns only a single (more precise: the first) @Mapping annotation. Therefore this did not work as expected (but already worked with the simple test named UnmappedTargetPropertiesWithMetaAnnotationInspectionTest.

I implemented an own recursive method to find @Mapping annotations and provided a test case with an infinitve chained meta-annotation.
I also added a test with combined @Mapping and meta-annotation on the same mapping method. The current implementation did not merge them together but worked against each other.


Sidenote: The intellij utility method MetaAnnotationUtil#findMetaAnnotations used an internal cache. I chose against using one, because it felt like a premature optimization which made the code less readable. Let me know what you think, especially because of the TODO comments to use a cache in line 300 😆

If we/you really want this, we should do this in a separate issue/PR with a clean cache solution outsourced in a separate class to keep the code clean.

I can also improve this with the current PR. Let me know what you think.

@filiphr filiphr merged commit 9c6ab9c into mapstruct:main May 7, 2023
8 checks passed
@filiphr
Copy link
Member

filiphr commented May 7, 2023

Thanks @thunderhook

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants