diff --git a/src/main/kotlin/platform/mixin/handlers/injectionPoint/AtResolver.kt b/src/main/kotlin/platform/mixin/handlers/injectionPoint/AtResolver.kt index ec61f4a4c..c35525bd1 100644 --- a/src/main/kotlin/platform/mixin/handlers/injectionPoint/AtResolver.kt +++ b/src/main/kotlin/platform/mixin/handlers/injectionPoint/AtResolver.kt @@ -25,24 +25,29 @@ import com.demonwav.mcdev.platform.mixin.reference.isMiscDynamicSelector import com.demonwav.mcdev.platform.mixin.reference.parseMixinSelector import com.demonwav.mcdev.platform.mixin.reference.target.TargetReference import com.demonwav.mcdev.platform.mixin.util.MixinConstants.Annotations.SLICE +import com.demonwav.mcdev.platform.mixin.util.MixinConstants.Classes.SHIFT import com.demonwav.mcdev.platform.mixin.util.findSourceElement import com.demonwav.mcdev.util.computeStringArray import com.demonwav.mcdev.util.constantStringValue import com.demonwav.mcdev.util.constantValue +import com.demonwav.mcdev.util.equivalentTo import com.demonwav.mcdev.util.fullQualifiedName import com.intellij.codeInsight.lookup.LookupElementBuilder +import com.intellij.psi.JavaPsiFacade import com.intellij.psi.PsiAnnotation import com.intellij.psi.PsiAnnotationMemberValue import com.intellij.psi.PsiArrayInitializerMemberValue import com.intellij.psi.PsiClass import com.intellij.psi.PsiClassType import com.intellij.psi.PsiElement +import com.intellij.psi.PsiEnumConstant import com.intellij.psi.PsiExpression import com.intellij.psi.PsiModifierList import com.intellij.psi.PsiQualifiedReference import com.intellij.psi.PsiReference import com.intellij.psi.PsiReferenceExpression import com.intellij.psi.search.GlobalSearchScope +import com.intellij.psi.util.PsiUtil import com.intellij.psi.util.parentOfType import com.intellij.psi.util.parents import org.objectweb.asm.tree.ClassNode @@ -142,6 +147,21 @@ class AtResolver( .filterIsInstance() .firstOrNull { it.parent is PsiModifierList } } + + fun getShift(at: PsiAnnotation): Int { + val shiftAttr = at.findDeclaredAttributeValue("shift") as? PsiExpression ?: return 0 + val shiftReference = PsiUtil.skipParenthesizedExprDown(shiftAttr) as? PsiReferenceExpression ?: return 0 + val shift = shiftReference.resolve() as? PsiEnumConstant ?: return 0 + val containingClass = shift.containingClass ?: return 0 + val shiftClass = JavaPsiFacade.getInstance(at.project).findClass(SHIFT, at.resolveScope) ?: return 0 + if (!(containingClass equivalentTo shiftClass)) return 0 + return when (shift.name) { + "BEFORE" -> -1 + "AFTER" -> 1 + "BY" -> at.findDeclaredAttributeValue("by")?.constantValue as? Int ?: 0 + else -> 0 + } + } } fun isUnresolved(): InsnResolutionInfo.Failure? { diff --git a/src/main/kotlin/platform/mixin/handlers/injectionPoint/ConstantStringMethodInjectionPoint.kt b/src/main/kotlin/platform/mixin/handlers/injectionPoint/ConstantStringMethodInjectionPoint.kt index f2a0a48ba..5a8dc1d7e 100644 --- a/src/main/kotlin/platform/mixin/handlers/injectionPoint/ConstantStringMethodInjectionPoint.kt +++ b/src/main/kotlin/platform/mixin/handlers/injectionPoint/ConstantStringMethodInjectionPoint.kt @@ -86,6 +86,11 @@ class ConstantStringMethodInjectionPoint : AbstractMethodInjectionPoint() { editor.caretModel.moveToOffset(cursorElement.textRange.endOffset - 1) } + override fun isShiftDiscouraged(shift: Int): Boolean { + // allow shifting after the INVOKE_STRING + return shift != 0 && shift != 1 + } + override fun getArgsKeys(at: PsiAnnotation) = ARGS_KEYS override fun getArgsValues(at: PsiAnnotation, key: String): Array { diff --git a/src/main/kotlin/platform/mixin/handlers/injectionPoint/FieldInjectionPoint.kt b/src/main/kotlin/platform/mixin/handlers/injectionPoint/FieldInjectionPoint.kt index 10cada293..576374721 100644 --- a/src/main/kotlin/platform/mixin/handlers/injectionPoint/FieldInjectionPoint.kt +++ b/src/main/kotlin/platform/mixin/handlers/injectionPoint/FieldInjectionPoint.kt @@ -59,6 +59,11 @@ class FieldInjectionPoint : QualifiedInjectionPoint() { completeExtraStringAtAttribute(editor, reference, "target") } + override fun isShiftDiscouraged(shift: Int): Boolean { + // allow shift after the field access + return shift != 0 && shift != 1 + } + override fun getArgsKeys(at: PsiAnnotation) = ARGS_KEYS override fun getArgsValues(at: PsiAnnotation, key: String): Array = diff --git a/src/main/kotlin/platform/mixin/handlers/injectionPoint/InjectionPoint.kt b/src/main/kotlin/platform/mixin/handlers/injectionPoint/InjectionPoint.kt index 903b129d3..f4b29cb52 100644 --- a/src/main/kotlin/platform/mixin/handlers/injectionPoint/InjectionPoint.kt +++ b/src/main/kotlin/platform/mixin/handlers/injectionPoint/InjectionPoint.kt @@ -22,13 +22,11 @@ package com.demonwav.mcdev.platform.mixin.handlers.injectionPoint import com.demonwav.mcdev.platform.mixin.reference.MixinSelector import com.demonwav.mcdev.platform.mixin.reference.toMixinString -import com.demonwav.mcdev.platform.mixin.util.MixinConstants.Classes.SHIFT import com.demonwav.mcdev.platform.mixin.util.fakeResolve import com.demonwav.mcdev.platform.mixin.util.findOrConstructSourceMethod import com.demonwav.mcdev.util.constantStringValue import com.demonwav.mcdev.util.constantValue import com.demonwav.mcdev.util.createLiteralExpression -import com.demonwav.mcdev.util.equivalentTo import com.demonwav.mcdev.util.findAnnotations import com.demonwav.mcdev.util.fullQualifiedName import com.demonwav.mcdev.util.getQualifiedMemberReference @@ -47,17 +45,13 @@ import com.intellij.psi.PsiAnnotation import com.intellij.psi.PsiAnonymousClass import com.intellij.psi.PsiClass import com.intellij.psi.PsiElement -import com.intellij.psi.PsiEnumConstant -import com.intellij.psi.PsiExpression import com.intellij.psi.PsiLambdaExpression import com.intellij.psi.PsiLiteral import com.intellij.psi.PsiMember import com.intellij.psi.PsiMethod import com.intellij.psi.PsiMethodReferenceExpression -import com.intellij.psi.PsiReferenceExpression import com.intellij.psi.PsiSubstitutor import com.intellij.psi.codeStyle.CodeStyleManager -import com.intellij.psi.util.PsiUtil import com.intellij.psi.util.parentOfType import com.intellij.serviceContainer.BaseKeyedLazyInstance import com.intellij.util.ArrayUtilRt @@ -108,7 +102,9 @@ abstract class InjectionPoint { open fun isArgValueList(at: PsiAnnotation, key: String) = false - open val isDiscouraged: Boolean = false + open val discouragedMessage: String? = null + + open fun isShiftDiscouraged(shift: Int): Boolean = shift != 0 abstract fun createNavigationVisitor( at: PsiAnnotation, @@ -146,20 +142,7 @@ abstract class InjectionPoint { } protected open fun addShiftSupport(at: PsiAnnotation, targetClass: ClassNode, collectVisitor: CollectVisitor<*>) { - val shiftAttr = at.findDeclaredAttributeValue("shift") as? PsiExpression ?: return - val shiftReference = PsiUtil.skipParenthesizedExprDown(shiftAttr) as? PsiReferenceExpression ?: return - val shift = shiftReference.resolve() as? PsiEnumConstant ?: return - val containingClass = shift.containingClass ?: return - val shiftClass = JavaPsiFacade.getInstance(at.project).findClass(SHIFT, at.resolveScope) ?: return - if (!(containingClass equivalentTo shiftClass)) return - when (shift.name) { - "BEFORE" -> collectVisitor.shiftBy = -1 - "AFTER" -> collectVisitor.shiftBy = 1 - "BY" -> { - val by = at.findDeclaredAttributeValue("by")?.constantValue as? Int ?: return - collectVisitor.shiftBy = by - } - } + collectVisitor.shiftBy = AtResolver.getShift(at) } protected open fun addSliceFilter(at: PsiAnnotation, targetClass: ClassNode, collectVisitor: CollectVisitor<*>) { diff --git a/src/main/kotlin/platform/mixin/handlers/injectionPoint/InvokeInjectionPoint.kt b/src/main/kotlin/platform/mixin/handlers/injectionPoint/InvokeInjectionPoint.kt index 6448dfca3..594ed5aff 100644 --- a/src/main/kotlin/platform/mixin/handlers/injectionPoint/InvokeInjectionPoint.kt +++ b/src/main/kotlin/platform/mixin/handlers/injectionPoint/InvokeInjectionPoint.kt @@ -44,6 +44,24 @@ abstract class AbstractInvokeInjectionPoint(private val assign: Boolean) : Abstr completeExtraStringAtAttribute(editor, reference, "target") } + override fun isShiftDiscouraged(shift: Int): Boolean { + if (shift == 0) { + return false + } + if (assign) { + // allow shifting before the INVOKE_ASSIGN + if (shift == -1) { + return false + } + } else { + // allow shifting after the INVOKE + if (shift == 1) { + return false + } + } + return true + } + override fun createNavigationVisitor( at: PsiAnnotation, target: MixinSelector?, diff --git a/src/main/kotlin/platform/mixin/handlers/injectionPoint/JumpInjectionPoint.kt b/src/main/kotlin/platform/mixin/handlers/injectionPoint/JumpInjectionPoint.kt index 770db81bd..ce09d906c 100644 --- a/src/main/kotlin/platform/mixin/handlers/injectionPoint/JumpInjectionPoint.kt +++ b/src/main/kotlin/platform/mixin/handlers/injectionPoint/JumpInjectionPoint.kt @@ -57,7 +57,7 @@ class JumpInjectionPoint : InjectionPoint() { ) } - override val isDiscouraged = true + override val discouragedMessage = "Usage of JUMP is discouraged because it is brittle" override fun createNavigationVisitor( at: PsiAnnotation, diff --git a/src/main/kotlin/platform/mixin/handlers/injectionPoint/LoadInjectionPoint.kt b/src/main/kotlin/platform/mixin/handlers/injectionPoint/LoadInjectionPoint.kt index 7ad828179..f99689366 100644 --- a/src/main/kotlin/platform/mixin/handlers/injectionPoint/LoadInjectionPoint.kt +++ b/src/main/kotlin/platform/mixin/handlers/injectionPoint/LoadInjectionPoint.kt @@ -67,6 +67,24 @@ abstract class AbstractLoadInjectionPoint(private val store: Boolean) : Injectio return LocalInfo.fromAnnotation(localType, modifyVariable) } + override fun isShiftDiscouraged(shift: Int): Boolean { + if (shift == 0) { + return false + } + if (store) { + // allow shift before the STORE + if (shift == -1) { + return false + } + } else { + // allow shift after the LOAD + if (shift == 1) { + return false + } + } + return true + } + override fun createNavigationVisitor( at: PsiAnnotation, target: MixinSelector?, diff --git a/src/main/kotlin/platform/mixin/inspection/injector/DiscouragedInjectionPointInspection.kt b/src/main/kotlin/platform/mixin/inspection/injector/DiscouragedInjectionPointInspection.kt index 6989fa9a5..d75bb9ab2 100644 --- a/src/main/kotlin/platform/mixin/inspection/injector/DiscouragedInjectionPointInspection.kt +++ b/src/main/kotlin/platform/mixin/inspection/injector/DiscouragedInjectionPointInspection.kt @@ -39,8 +39,9 @@ class DiscouragedInjectionPointInspection : MixinInspection() { } val atValue = annotation.findDeclaredAttributeValue("value") ?: return val atCode = atValue.constantStringValue ?: return - if (InjectionPoint.byAtCode(atCode)?.isDiscouraged == true) { - holder.registerProblem(atValue, "Usage of $atCode is discouraged") + val discouragedMessage = InjectionPoint.byAtCode(atCode)?.discouragedMessage + if (discouragedMessage != null) { + holder.registerProblem(atValue, discouragedMessage) } } } diff --git a/src/main/kotlin/platform/mixin/inspection/injector/DiscouragedShiftInspection.kt b/src/main/kotlin/platform/mixin/inspection/injector/DiscouragedShiftInspection.kt new file mode 100644 index 000000000..e9145d1ca --- /dev/null +++ b/src/main/kotlin/platform/mixin/inspection/injector/DiscouragedShiftInspection.kt @@ -0,0 +1,50 @@ +/* + * 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.injector + +import com.demonwav.mcdev.platform.mixin.handlers.injectionPoint.AtResolver +import com.demonwav.mcdev.platform.mixin.handlers.injectionPoint.InjectionPoint +import com.demonwav.mcdev.platform.mixin.inspection.MixinInspection +import com.demonwav.mcdev.platform.mixin.util.MixinConstants +import com.demonwav.mcdev.util.constantStringValue +import com.intellij.codeInspection.ProblemsHolder +import com.intellij.psi.JavaElementVisitor +import com.intellij.psi.PsiAnnotation +import com.intellij.psi.PsiElementVisitor + +class DiscouragedShiftInspection : MixinInspection() { + override fun getStaticDescription() = "Reports discouraged usages of shifting in injection points" + + override fun buildVisitor(holder: ProblemsHolder): PsiElementVisitor = object : JavaElementVisitor() { + override fun visitAnnotation(annotation: PsiAnnotation) { + if (!annotation.hasQualifiedName(MixinConstants.Annotations.AT)) { + return + } + val atValue = annotation.findDeclaredAttributeValue("value") ?: return + val atCode = atValue.constantStringValue ?: return + val shift = AtResolver.getShift(annotation) + if (InjectionPoint.byAtCode(atCode)?.isShiftDiscouraged(shift) == true) { + val shiftElement = annotation.findDeclaredAttributeValue("shift") ?: return + holder.registerProblem(shiftElement, "Shifting like this is discouraged because it's brittle") + } + } + } +} diff --git a/src/main/kotlin/platform/mixin/reference/InjectionPointReference.kt b/src/main/kotlin/platform/mixin/reference/InjectionPointReference.kt index 2059034fb..06a427e58 100644 --- a/src/main/kotlin/platform/mixin/reference/InjectionPointReference.kt +++ b/src/main/kotlin/platform/mixin/reference/InjectionPointReference.kt @@ -88,7 +88,7 @@ object InjectionPointReference : ReferenceResolver(), MixinReference { return ( getAllAtCodes(context.project).keys.asSequence() .filter { - InjectionPoint.byAtCode(it)?.isDiscouraged != true + InjectionPoint.byAtCode(it)?.discouragedMessage == null } .map { PrioritizedLookupElement.withPriority( diff --git a/src/main/resources/META-INF/plugin.xml b/src/main/resources/META-INF/plugin.xml index 562127f59..c6105cec4 100644 --- a/src/main/resources/META-INF/plugin.xml +++ b/src/main/resources/META-INF/plugin.xml @@ -1026,6 +1026,14 @@ level="WARNING" hasStaticDescription="true" implementationClass="com.demonwav.mcdev.platform.mixin.inspection.injector.DiscouragedInjectionPointInspection"/> +