diff --git a/mixin-test-data/src/main/java/com/demonwav/mcdev/mixintestdata/multipleTargetClasses/SimpleClass1.java b/mixin-test-data/src/main/java/com/demonwav/mcdev/mixintestdata/multipleTargetClasses/SimpleClass1.java new file mode 100644 index 000000000..99381ce46 --- /dev/null +++ b/mixin-test-data/src/main/java/com/demonwav/mcdev/mixintestdata/multipleTargetClasses/SimpleClass1.java @@ -0,0 +1,14 @@ +/* + * Minecraft Dev for IntelliJ + * + * https://minecraftdev.org + * + * Copyright (c) 2021 minecraft-dev + * + * MIT License + */ + +package com.demonwav.mcdev.mixintestdata.multipleTargetClasses; + +public class SimpleClass1 { +} diff --git a/mixin-test-data/src/main/java/com/demonwav/mcdev/mixintestdata/multipleTargetClasses/SimpleClass2.java b/mixin-test-data/src/main/java/com/demonwav/mcdev/mixintestdata/multipleTargetClasses/SimpleClass2.java new file mode 100644 index 000000000..e3d530ca4 --- /dev/null +++ b/mixin-test-data/src/main/java/com/demonwav/mcdev/mixintestdata/multipleTargetClasses/SimpleClass2.java @@ -0,0 +1,14 @@ +/* + * Minecraft Dev for IntelliJ + * + * https://minecraftdev.org + * + * Copyright (c) 2021 minecraft-dev + * + * MIT License + */ + +package com.demonwav.mcdev.mixintestdata.multipleTargetClasses; + +public class SimpleClass2 { +} diff --git a/mixin-test-data/src/main/java/com/demonwav/mcdev/mixintestdata/multipleTargetClasses/SimpleClass3.java b/mixin-test-data/src/main/java/com/demonwav/mcdev/mixintestdata/multipleTargetClasses/SimpleClass3.java new file mode 100644 index 000000000..c62650ffd --- /dev/null +++ b/mixin-test-data/src/main/java/com/demonwav/mcdev/mixintestdata/multipleTargetClasses/SimpleClass3.java @@ -0,0 +1,14 @@ +/* + * Minecraft Dev for IntelliJ + * + * https://minecraftdev.org + * + * Copyright (c) 2021 minecraft-dev + * + * MIT License + */ + +package com.demonwav.mcdev.mixintestdata.multipleTargetClasses; + +public class SimpleClass3 { +} diff --git a/src/main/kotlin/platform/mixin/inspection/MixinDuplicateTargetInspection.kt b/src/main/kotlin/platform/mixin/inspection/MixinDuplicateTargetInspection.kt new file mode 100644 index 000000000..e8ed272d0 --- /dev/null +++ b/src/main/kotlin/platform/mixin/inspection/MixinDuplicateTargetInspection.kt @@ -0,0 +1,178 @@ +/* + * Minecraft Development for IntelliJ + * + * https://mcdev.io/ + * + * Copyright (C) 2024 minecraft-dev + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published + * by the Free Software Foundation, version 3.0 only. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + */ + +package com.demonwav.mcdev.platform.mixin.inspection + +import com.demonwav.mcdev.platform.mixin.util.MixinConstants.Annotations.MIXIN +import com.demonwav.mcdev.platform.mixin.util.findClassNodeByPsiClass +import com.demonwav.mcdev.platform.mixin.util.findClassNodeByQualifiedName +import com.demonwav.mcdev.platform.mixin.util.mixinTargets +import com.demonwav.mcdev.util.computeStringArray +import com.demonwav.mcdev.util.constantStringValue +import com.demonwav.mcdev.util.findModule +import com.demonwav.mcdev.util.resolveClass +import com.intellij.codeInsight.intention.QuickFixFactory +import com.intellij.codeInspection.LocalQuickFix +import com.intellij.codeInspection.LocalQuickFixAndIntentionActionOnPsiElement +import com.intellij.codeInspection.ProblemsHolder +import com.intellij.openapi.editor.Editor +import com.intellij.openapi.project.Project +import com.intellij.psi.JavaElementVisitor +import com.intellij.psi.PsiAnnotation +import com.intellij.psi.PsiArrayInitializerMemberValue +import com.intellij.psi.PsiClass +import com.intellij.psi.PsiClassObjectAccessExpression +import com.intellij.psi.PsiElement +import com.intellij.psi.PsiElementVisitor +import com.intellij.psi.PsiExpression +import com.intellij.psi.PsiFile +import org.jetbrains.plugins.groovy.intentions.style.inference.resolve +import org.objectweb.asm.tree.ClassNode + +class MixinDuplicateTargetInspection : MixinInspection() { + + override fun getStaticDescription() = "Targeting the same class multiple times in a mixin is redundant" + + override fun buildVisitor(holder: ProblemsHolder): PsiElementVisitor = Visitor(holder) + + private class Visitor(private val holder: ProblemsHolder) : JavaElementVisitor() { + + override fun visitClass(psiClass: PsiClass) { + val mixinAnnotation = psiClass.modifierList?.findAnnotation(MIXIN) ?: return + + val mixinTargets = psiClass.mixinTargets + + if (mixinTargets.size != mixinTargets.distinct().size) { + val nonDuplicates = mutableListOf() + for (i in mixinTargets.indices) { + val mixinTarget = mixinTargets[i] + if (mixinTarget !in mixinTargets.subList( + i + 1, mixinTargets.size + ) && mixinTarget !in mixinTargets.subList(0, i) + ) { + nonDuplicates.add(mixinTarget) + } + } + + goOverValues(nonDuplicates, mixinAnnotation) + goOverTargets(psiClass, nonDuplicates, mixinAnnotation) + } + } + + private fun goOverValues(nonDuplicates: List, mixinAnnotation: PsiAnnotation) { + + // Read class targets (value) + val value = mixinAnnotation.findDeclaredAttributeValue("value") ?: return + if (value is PsiArrayInitializerMemberValue) { + val classElements = value.children.filterIsInstance() + for (classTargetIndex in classElements.indices) { + val expression = classElements[classTargetIndex] as? PsiClassObjectAccessExpression ?: continue + val targetPsiClass = expression.operand.type.resolve() ?: continue + val targetClassNode = findClassNodeByPsiClass(targetPsiClass) + + registerProblemIfProblematic( + nonDuplicates, + targetClassNode, + expression, + QuickFixFactory.getInstance().createDeleteFix(expression) + ) + } + } else { + val targetClass = value.resolveClass() ?: return + val classNode = findClassNodeByPsiClass(targetClass) + val possibleQuickFix = RemoveDuplicateTargetFix(mixinAnnotation, "value") + registerProblemIfProblematic(nonDuplicates, classNode, value, possibleQuickFix) + } + } + + private fun goOverTargets(psiClass: PsiClass, nonDuplicates: List, mixinAnnotation: PsiAnnotation) { + // Read string targets (targets) + val targets = mixinAnnotation.findDeclaredAttributeValue("targets") ?: return + val classTargetNames = targets.computeStringArray() + if (targets is PsiArrayInitializerMemberValue) { + val classChildren = targets.children.filterIsInstance() + + // TODO: may be misaligned if there are numbers for example in the class array because they will have different lengths + for (classTargetIndex in classTargetNames.indices) { + val targetName = classTargetNames[classTargetIndex] + + val classNode = findClassNodeByQualifiedName( + psiClass.project, + psiClass.findModule(), + targetName.replace('/', '.'), + ) + registerProblemIfProblematic( + nonDuplicates, + classNode, + classChildren[classTargetIndex], + QuickFixFactory.getInstance().createDeleteFix(classChildren[classTargetIndex]) + ) + } + } else { + val stringValue = targets.constantStringValue ?: return + val classNode = findClassNodeByQualifiedName( + psiClass.project, + psiClass.findModule(), + stringValue.replace('/', '.'), + ) + val removeDuplicateTargetFix = RemoveDuplicateTargetFix(mixinAnnotation, "targets") + registerProblemIfProblematic(nonDuplicates, classNode, targets, removeDuplicateTargetFix) + } + } + + private fun registerProblemForDuplicate(duplicateExpression: PsiElement, quickFix: LocalQuickFix) { + holder.registerProblem( + duplicateExpression, + "Duplicate target is redundant", + quickFix, + ) + } + + private fun registerProblemIfProblematic( + nonDuplicates: List, + classNode: ClassNode?, + possibleDuplicate: PsiElement, + possibleQuickFix: LocalQuickFix + ) { + if (classNode != null && classNode !in nonDuplicates) { + registerProblemForDuplicate(possibleDuplicate, possibleQuickFix) + } + } + } + + private class RemoveDuplicateTargetFix(annotation: PsiAnnotation, val annotationAttributeName: String) : + LocalQuickFixAndIntentionActionOnPsiElement(annotation) { + + override fun getFamilyName() = "Remove duplicate target" + + override fun getText() = familyName + + override fun invoke( + project: Project, + file: PsiFile, + editor: Editor?, + startElement: PsiElement, + endElement: PsiElement + ) { + val annotation = startElement as? PsiAnnotation ?: return + annotation.setDeclaredAttributeValue(annotationAttributeName, null) + } + } +} diff --git a/src/main/resources/META-INF/plugin.xml b/src/main/resources/META-INF/plugin.xml index 27a38c6a3..420595b23 100644 --- a/src/main/resources/META-INF/plugin.xml +++ b/src/main/resources/META-INF/plugin.xml @@ -830,6 +830,14 @@ level="ERROR" hasStaticDescription="true" implementationClass="com.demonwav.mcdev.platform.mixin.inspection.MixinTargetInspection"/> + . + */ + +package com.demonwav.mcdev.platform.mixin + +import com.demonwav.mcdev.framework.EdtInterceptor +import com.demonwav.mcdev.platform.mixin.inspection.MixinDuplicateTargetInspection +import org.intellij.lang.annotations.Language +import org.junit.jupiter.api.DisplayName +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.ExtendWith + +@ExtendWith(EdtInterceptor::class) +@DisplayName("Mixin Duplicate Target Inspection Test") +class MixinDuplicateTargetInspectionTest : BaseMixinTest() { + + private fun doTest(@Language("JAVA") code: String) { + buildProject { + dir("test") { + java("TestMixin.java", code) + } + } + + fixture.enableInspections(MixinDuplicateTargetInspection::class) + fixture.checkHighlighting(false, false, true) + } + + @Test + @DisplayName("Simple class target") + fun simpleClassTarget() { + doTest( + """ + package test; + + import com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass1; + import org.spongepowered.asm.mixin.Mixin; + + @Mixin(SimpleClass1.class) + public class TestMixin { + } + """, + ) + } + + @Test + @DisplayName("Simple class target in array") + fun simpleClassTargetInArray() { + doTest( + """ + package test; + + import com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass1; + import org.spongepowered.asm.mixin.Mixin; + + @Mixin({SimpleClass1.class}) + public class TestMixin { + } + """, + ) + } + + @Test + @DisplayName("Simple class target in array with attribute name") + fun simpleClassTargetInArrayWithAttributeName() { + doTest( + """ + package test; + + import com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass1; + import org.spongepowered.asm.mixin.Mixin; + + @Mixin(value = {SimpleClass1.class}) + public class TestMixin { + } + """, + ) + } + + @Test + @DisplayName("Simple class target with attribute name") + fun simpleClassTargetWithAttributeName() { + doTest( + """ + package test; + + import com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass1; + import org.spongepowered.asm.mixin.Mixin; + + @Mixin(value = SimpleClass1.class) + public class TestMixin { + } + """, + ) + } + + @Test + @DisplayName("Different targets in array") + fun differentTargetsInArray() { + doTest( + """ + package test; + + import com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass1; + import com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass2; + import com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass3; + import org.spongepowered.asm.mixin.Mixin; + + @Mixin({SimpleClass1.class, SimpleClass2.class, SimpleClass3.class}) + public class TestMixin { + } + """, + ) + } + + @Test + @DisplayName("Different targets in array with attribute name") + fun differentTargetsInArrayWithAttributeName() { + doTest( + """ + package test; + + import com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass1; + import com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass2; + import com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass3; + import org.spongepowered.asm.mixin.Mixin; + + @Mixin(value = {SimpleClass1.class, SimpleClass2.class, SimpleClass3.class}) + public class TestMixin { + } + """, + ) + } + + @Test + @DisplayName("Different targets in string targets") + fun differentTargetsInStringTargets() { + doTest( + """ + package test; + + import org.spongepowered.asm.mixin.Mixin; + + @Mixin(targets = { + "com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass1", + "com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass2", + "com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass3" + }) + public class TestMixin { + } + """, + ) + } + + @Test + @DisplayName("Unresolved duplicates") + fun unreslovedDuplicates() { + doTest( + """ + package test; + + import org.spongepowered.asm.mixin.Mixin; + + @Mixin( + value = { + UnresolvedClass1.class, + UnresolvedClass1.class, + UnresolvedClass2.class, + UnresolvedClass2.class + }, + targets = { + "UnresolvedClass1", + "UnresolvedClass1", + "UnresolvedClass2", + "UnresolvedClass2" + } + ) + public class TestMixin { + } + """, + ) + } + + @Test + @DisplayName("Target twice in array") + fun targetTwiceInArray() { + doTest( + """ + package test; + + import com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass1; + import org.spongepowered.asm.mixin.Mixin; + + @Mixin({SimpleClass1.class, SimpleClass1.class}) + public class TestMixin { + } + """, + ) + } + + @Test + @DisplayName("Target twice in array with attribute name") + fun targetTwiceInArrayWithAttributeName() { + doTest( + """ + package test; + + import com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass1; + import org.spongepowered.asm.mixin.Mixin; + + @Mixin(value = {SimpleClass1.class, SimpleClass1.class}) + public class TestMixin { + } + """, + ) + } + + @Test + @DisplayName("Target twice with unresolved targets next to it") + fun targetTwiceWithUnresolvedTargetsNextToIt() { + doTest( + """ + package test; + + import com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass1; + import org.spongepowered.asm.mixin.Mixin; + + @Mixin(value = { + UnresolvedClass1.class, + SimpleClass1.class, + UnresolvedClass2.class, + SimpleClass1.class, + UnresolvedClass3.class + }) + public class TestMixin { + } + """, + ) + } + + @Test + @DisplayName("Target three times with unresolved targets next to it") + fun targetThreeTimesWithUnresolvedTargetsNextToIt() { + doTest( + """ + package test; + + import com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass1; + import org.spongepowered.asm.mixin.Mixin; + + @Mixin(value = { + UnresolvedClass1.class, + SimpleClass1.class, + UnresolvedClass2.class, + SimpleClass1.class, + UnresolvedClass3.class, + SimpleClass1.class, + UnresolvedClass4.class + }) + public class TestMixin { + } + """, + )/* + UnresolvedClass4.class + is the expected + */ + } + + @Test + @DisplayName("Multiple targets multiple times") + fun multipleTargetsMultipleTimes() { + doTest( + """ + package test; + + import com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass1; + import com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass2; + import org.spongepowered.asm.mixin.Mixin; + + @Mixin(value = { + SimpleClass2.class, + UnresolvedClass1.class, + SimpleClass1.class, + UnresolvedClass2.class, + SimpleClass1.class, + SimpleClass2.class, + UnresolvedClass3.class + }) + public class TestMixin { + } + """, + ) + } + + @Test + @DisplayName("String target and class target") + fun stringTargetAndClassTarget() { + doTest( + """ + package test; + + import com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass1; + import org.spongepowered.asm.mixin.Mixin; + + @Mixin(value = SimpleClass1.class, + targets = "com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass1") + public class TestMixin { + } + """, + ) + } + + @Test + @DisplayName("Multiple string targets") + fun multipleStringTargets() { + doTest( + """ + package test; + + import org.spongepowered.asm.mixin.Mixin; + + @Mixin(targets = { + "com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass2", + "com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass1", + "com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass1", + "com.demonwav.mcdev.mixintestdata.multipleTargetClasses.SimpleClass3" + } + ) + public class TestMixin { + } + """, + ) + } +}