Skip to content

[#1140] adds warning when the target has no target properties - #3595

Merged
filiphr merged 11 commits into
mapstruct:mainfrom
zyberzebra:#1140-add-warning-for-target-without-target-properties
May 11, 2025
Merged

[#1140] adds warning when the target has no target properties#3595
filiphr merged 11 commits into
mapstruct:mainfrom
zyberzebra:#1140-add-warning-for-target-without-target-properties

Conversation

@zyberzebra

@zyberzebra zyberzebra commented May 6, 2024 •
edited by filiphr
Loading

Copy link
Copy Markdown
Contributor

Fixes: #1140
TODO:

  • should no accessible properties also lead to the warning?
    • I guess so
  • remove reportErrorForUnmappedTargetPropertiesIfRequired, I guess?
    • realized unmappedTarget is a different case.
  • Any other possibilities or edge cases to test?

@zyberzebra
zyberzebra marked this pull request as draft May 6, 2024 19:16
@zyberzebra
[mapstruct#1140] add test for target without accessible properties
632966c 
- warning that no actual target properties are targeted/written to should be given when compiling
@zyberzebra
zyberzebra marked this pull request as ready for review May 7, 2024 16:45
zyberzebra
zyberzebra commented May 7, 2024
View reviewed changes
Comment thread processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java Outdated
filiphr
filiphr reviewed May 18, 2024
View reviewed changes

@filiphr filiphr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for your work on this @zyberzebra. I've left some comments.

Comment thread processor/src/main/java/org/mapstruct/ap/internal/util/Message.java Outdated
Comment thread processor/src/main/java/org/mapstruct/ap/internal/util/Message.java Outdated
Comment thread processor/src/test/java/org/mapstruct/ap/test/bugs/_1140/AirplaneWithNoAccessors.java Outdated
Comment thread processor/src/test/java/org/mapstruct/ap/test/bugs/_1140/JarMapperTest.java Outdated
@zyberzebra

zyberzebra commented May 27, 2024

Copy link
Copy Markdown
Contributor Author

@filiphr I'm not 100% what the correct workflow is. Should I request reviews when I'm ready or not?

@filiphr

filiphr commented May 27, 2024

Copy link
Copy Markdown
Member

The workflow is me trying to get time and going through all the emails and updated PRs 😀. I usually go through PRs that have been updated as well

@zyberzebra

zyberzebra commented May 27, 2024

Copy link
Copy Markdown
Contributor Author

The workflow is me trying to get time and going through all the emails and updated PRs 😀. I usually go through PRs that have been updated as well

Thanks for the clarification ! I just scrolled through some other pull request here and saw that some request reviews and because it's my first time working open source I was unsure. ;)
At work I just ping my mates 😁

@zyberzebra
zyberzebra requested a review from filiphr August 25, 2024 09:29
@zyberzebra

zyberzebra commented Aug 25, 2024

Copy link
Copy Markdown
Contributor Author

@filiphr Sorry for the ping. Just thought I give you a ping, because my PRs are probably way down on the pile now.

With more time passing, it gets harder for me to remember what we fixed. Which is not a problem, put it might take longer to for me to respond if you come around for a review. :)

filiphr
filiphr requested changes Sep 2, 2024
View reviewed changes

@filiphr filiphr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry for the waiting @zyberzebra. I've done another pass here. Have a look at my comments

Comment thread processor/src/main/java/org/mapstruct/ap/internal/util/Message.java Outdated
Comment thread processor/src/test/java/org/mapstruct/ap/test/emptytarget/JarMapperTest.java Outdated
Comment thread processor/src/test/java/org/mapstruct/ap/test/emptytarget/JarMapperTest.java Outdated
@zyberzebra
refactor: remove method from warning
fc5b091 
- remove method name
- assert that line is reported on warning
@zyberzebra
zyberzebra requested a review from filiphr September 6, 2024 20:17
filiphr
filiphr requested changes Sep 13, 2024
View reviewed changes

@filiphr filiphr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good work @zyberzebra. I've done some small polishing and added a test case for the comment I left.

Have a look at it and let me know what you think

Comment thread processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java Outdated
zyberzebra and others added 2 commits September 14, 2024 17:51
@zyberzebra
fix: move targetProperty check after constructor parameters computation
86b26c7 
- the check needs to be after constructor parameters were evaluated
- otherwise we would produce incorrect warning if the constructor is used for setting target properties
@filiphr
Fix tests
49267ad
@filiphr
filiphr merged commit 602e290 into mapstruct:main May 11, 2025
@zyberzebra
zyberzebra deleted the #1140-add-warning-for-target-without-target-properties branch November 20, 2025 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@filiphr filiphr filiphr requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Warning when the target has no target properties

2 participants

@zyberzebra @filiphr