mapstruct / mapstruct-idea

An IntelliJ IDEA plugin for working with MapStruct
Other
141 stars 38 forks source link

prevent null as fix for WrongUsageOfMappersFactory #176

Closed hduelme closed 7 months ago

hduelme commented 7 months ago

I found a working fix for 2023.3. Sadly it doesn't work with older versions. Some things to note:

hduelme commented 7 months ago

See also #162

hduelme commented 7 months ago

One way to fix it would be to be to just delete the Instance variable.

    private static class RemoveMappersFix extends LocalQuickFixOnPsiElement {

        private final String myText;
        private final String myFamilyName;

        private RemoveMappersFix(@NotNull PsiVariable element) {
            super( element );
            this.myFamilyName = MapStructBundle.message( "inspection.wrong.usage.mappers.factory" );
            this.myText = MapStructBundle.message( "inspection.wrong.usage.mappers.factory.remove.mappers.usage" );
        }

        @NotNull
        @Override
        public String getText() {
            return myText;
        }

        @Override
        public void invoke(@NotNull Project project, @NotNull PsiFile psiFile, @NotNull PsiElement psiElement, @NotNull PsiElement psiElement1) {
           psiElement.delete();
        }

        @Nls
        @NotNull
        @Override
        public String getFamilyName() {
            return myFamilyName;
        }
    }

This would drop the usage search

filiphr commented 7 months ago

Thanks for your work on this @hduelme. I've been dabbling with this a bit as well. Perhaps we need to stop using the internal API from IntelliJ and just do the deletion on our side.

I didn't know that we could just do psiElement.delete();. Does https://github.com/mapstruct/mapstruct-idea/pull/176#issuecomment-1987337090 work on other versions as well? How does it behave if the instance field is used somewhere?

hduelme commented 7 months ago

@filiphr the delete option would work on all version. The problem is that delete()does not check for usage.

hduelme commented 7 months ago

The other solution would be to set the min supported version to 2023.3 and hope that the internal API does not change to much in the future.

hduelme commented 7 months ago

I did a bit of testing and it seems that using the RemoveUnusedVariableFix also ignores if the variable is used or not and just deletes it. So no benfit from this anymore. What is RemoveUnusedVariableFix currently does is giving you a choose grafik

But I think we don't want the extract option, so I removed it in my code and the result is that the variable is deleted ignoring if it is used or not.

filiphr commented 7 months ago

Thanks a lot again for your work on this @hduelme. I think that we are converging on the solution in #179. Therefore, I'll close this one.