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

Fix build with ideaVersion LATEST-EAP-SNAPSHOT #155

Closed
thunderhook opened this issue Oct 18, 2023 · 3 comments
Closed

Fix build with ideaVersion LATEST-EAP-SNAPSHOT #155

thunderhook opened this issue Oct 18, 2023 · 3 comments
Assignees
Milestone

Comments

@thunderhook
Copy link
Contributor

thunderhook commented Oct 18, 2023

In the LATEST-EAP-SNAPSHOT they changed the way how RemoveUnusedVariableFix works, see this commit

The org.mapstruct.intellij.inspection.WrongUsageOfMappersFactoryInspection.RemoveMappersFix won't compile anymore since it is expected to be an IntentionAction instead of a PsiBasedModCommandAction<PsiVariable>:

\IdeaProjects\mapstruct-idea\src\main\java\org\mapstruct\intellij\inspection\WrongUsageOfMappersFactoryInspection.java:132: error: method does not override or implement a method from a supertype
        @Override
        ^

\IdeaProjects\mapstruct-idea\src\main\java\org\mapstruct\intellij\inspection\WrongUsageOfMappersFactoryInspection.java:149: error: incompatible types: RemoveMappersFix cannot be converted to IntentionAction
                new RemoveMappersFix( (PsiVariable) parent ),
                ^

A quick-fix would be: copy the contents of the old code instead of extending RemoveUnusedVariableFix.
Additionally it would be good to know why they changed it, and if this is a better way of handling fixes. Therefore we might change our RemoveMappersFix respectively.

To change the version inside the IDE just change the version to LATEST-EAP-SNAPSHOT in gradle.properties

thunderhook added a commit that referenced this issue Nov 18, 2023
…to be downward compatible with WrongUsageOfMappersFactoryInspection
@thunderhook thunderhook self-assigned this Nov 18, 2023
@thunderhook
Copy link
Contributor Author

thunderhook commented Nov 18, 2023

I thought this would be easy. However, they changed a lot of code. My approach with copying the old classes did not work. They also removed the com.intellij.codeInsight.daemon.impl.quickfix.SideEffectWarningDialog class, see: https://github.com/mapstruct/mapstruct-idea/actions/runs/6916833362/job/18817324564

It is still only in the LATEST-EAP-SNAPSHOT, will be likely in main with 2023.3 though.

I think we should either:

  • ignore the failure in the github actions for now
  • when 2023.3 is released: maybe disable the WrongUsageOfMappersFactoryInspection (not the most important intention)

I am not sure what happens when we switch to 2023.3 as the main version for development. I think this intention cannot run on older versions then? Or does it just not build with that version? We'll see.

filiphr added a commit that referenced this issue Nov 26, 2023
@filiphr
Copy link
Member

filiphr commented Nov 26, 2023

I had a look at it. They didn't remove it. They just changed it. They are now doing:

return new RemoveUnusedVariableFix(variable).asIntention();

I've done PR #162 for this

@filiphr
Copy link
Member

filiphr commented Mar 16, 2024

Done in PR #179

@filiphr filiphr closed this as completed Mar 16, 2024
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

No branches or pull requests

2 participants