#151 rework inheritance and support auto inheritance #152
Conversation
thunderhook
force-pushed
the
151-auto-inheritance
branch
from
1d50fb8
to
1aed6ba
Compare
There was a problem hiding this comment.
Thanks for your work on this @thunderhook.
I've been away on holidays, so only getting the time now to have a look at this.
The code looks OK to me. I've left some questions I had as comments. I'll play around a bit to see how it behaves in an actual project as well.
src/main/java/org/mapstruct/intellij/util/MapstructAnnotationUtils.java
Outdated
Show resolved
Hide resolved
src/test/java/org/mapstruct/intellij/bugs/_148/AutoInheritConfigurationTest.java
Outdated
Show resolved
Hide resolved
| @@ -51,6 +51,8 @@ public void testWrongUsageOfMappersFactory() { | |||
| myFixture.launchAction( allQuickFixes.get( 6 ) ); // Remove jsr330 componentModel | |||
| myFixture.launchAction( allQuickFixes.get( 8 ) ); // Remove custom componentModel | |||
|
|
|||
| assertThat( myFixture.getAllQuickFixes() ).isEmpty(); | |||
There was a problem hiding this comment.
I forgot to mention it, sorry, could've left a comment. This is a workaround for the latest EAP Intellij. The test assertion just fails and only there, and only this test. I have no Idea why.
During inspecting it inside the debugger, and after evaluating myFixture.getAllQuickFixes() the assertion suddenly worked. So this is a workaround for that flaky test (or flaky fixture/assertion) - I will leave a comment with a TODO there, I don't think you have the time desire and to investigate this? 😅
There was a problem hiding this comment.
I will leave a comment with a TODO there, I don't think you have the time desire and to investigate this?
I am a bit busy yes, but I would prefer to figure out why is it happening, since it might be happening in the actual EAP as well. Can you please extract this change into a different PR so I can investigate it?
There was a problem hiding this comment.
The error is only happening in the LATEST-EAP-SNAPSHOT. See: https://github.com/mapstruct/mapstruct-idea/actions/runs/6129983187
And this is also happening in the master branch. The test WrongUsageOfMappersFactoryInspectionTest.java has nothing to do with my changes.
So you can just reproduce this by changing the ideaVersion (see git patch, no separate PR needed 🙂 )
I applied this workaround to get the build green. I can of course revert the workaround and we can merge the PR although the build is failing (when all issues are resolved ofc).
Subject: [PATCH] change current build to LATEST-EAP-SNAPSHOT
---
Index: gradle.properties
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>ISO-8859-1
===================================================================
diff --git a/gradle.properties b/gradle.properties
--- a/gradle.properties (revision 5168fbab259c968771f2944a98eb0187b55f5ade)
+++ b/gradle.properties (date 1694897260461)
@@ -2,10 +2,10 @@
# https://www.jetbrains.com/intellij-repository/releases
# https://www.jetbrains.com/intellij-repository/snapshots
-ideaVersion = 2021.2
+ideaVersion = LATEST-EAP-SNAPSHOT
ideaType = IC
sources = true
-isEAP = false
+isEAP = true
runGenerators = true
pluginGroup = org.mapstruct
src/main/java/org/mapstruct/intellij/inspection/UnmappedTargetPropertiesInspection.java
Outdated
Show resolved
Hide resolved
|
Thank you for having a look at the PR @filiphr. I hope you had a great and relaxing vacation! I will resolve your comments and questions in the evening. 👍 |
There was a problem hiding this comment.
Thanks for your additional changes @thunderhook.
I've replied to some of the comments and there is one more change that I think we should address.
src/main/java/org/mapstruct/intellij/util/MapstructAnnotationUtils.java
Outdated
Show resolved
Hide resolved
|
@filiphr You can now have another look at the PR when you find time. Thanks alot! Sidenote: Reached out to you by mail. Maybe check your spam folder, because I used an emoji in the subject... 🤦 😅 |
|
Thanks a lot for the improvements you did for the configuration inheritance. |
See #151. This took me a while to figure out how the inheritance is handled by MapStruct.
Fixes #151