Actually use InjectorAnnotationHandler.isInsnAllowed. Errors are reported, auto completion is filtered, and invalid injector inspection is disabled if there is an invalid insn. Closes #2290

Earthcomputer · Earthcomputer · commit 6f5a7955af87 · 2025-11-04T04:43:35.000Z
diff --git a/src/main/kotlin/platform/mixin/handlers/InjectorAnnotationHandler.kt b/src/main/kotlin/platform/mixin/handlers/InjectorAnnotationHandler.kt
@@ -164,10 +164,12 @@ abstract class InjectorAnnotationHandler : MixinAnnotationHandler {
         targetMethod: MethodNode,
     ): List<MethodSignature>?
 
-    open fun isInsnAllowed(insn: AbstractInsnNode): Boolean {
+    open fun isInsnAllowed(insn: AbstractInsnNode, decorations: Map<String, Any?>): Boolean {
         return true
     }
 
+    open val allowedInsnDescription = "all instructions"
+
     override fun createUnresolvedMessage(annotation: PsiAnnotation): String? {
         return "Cannot resolve any target instructions in target class"
     }
diff --git a/src/main/kotlin/platform/mixin/handlers/ModifyArgHandler.kt b/src/main/kotlin/platform/mixin/handlers/ModifyArgHandler.kt
@@ -38,10 +38,12 @@ import org.objectweb.asm.tree.MethodInsnNode
 import org.objectweb.asm.tree.MethodNode
 
 class ModifyArgHandler : InjectorAnnotationHandler() {
-    override fun isInsnAllowed(insn: AbstractInsnNode): Boolean {
+    override fun isInsnAllowed(insn: AbstractInsnNode, decorations: Map<String, Any?>): Boolean {
         return insn is MethodInsnNode
     }
 
+    override val allowedInsnDescription = "method invocations"
+
     override fun expectedMethodSignature(
         annotation: PsiAnnotation,
         targetClass: ClassNode,
diff --git a/src/main/kotlin/platform/mixin/handlers/ModifyArgsHandler.kt b/src/main/kotlin/platform/mixin/handlers/ModifyArgsHandler.kt
@@ -34,10 +34,12 @@ import org.objectweb.asm.tree.MethodInsnNode
 import org.objectweb.asm.tree.MethodNode
 
 class ModifyArgsHandler : InjectorAnnotationHandler() {
-    override fun isInsnAllowed(insn: AbstractInsnNode): Boolean {
+    override fun isInsnAllowed(insn: AbstractInsnNode, decorations: Map<String, Any?>): Boolean {
         return insn is MethodInsnNode
     }
 
+    override val allowedInsnDescription = "method invocations"
+
     override fun expectedMethodSignature(
         annotation: PsiAnnotation,
         targetClass: ClassNode,
diff --git a/src/main/kotlin/platform/mixin/handlers/ModifyConstantHandler.kt b/src/main/kotlin/platform/mixin/handlers/ModifyConstantHandler.kt
@@ -129,9 +129,11 @@ class ModifyConstantHandler : InjectorAnnotationHandler() {
         }.toList()
     }
 
-    override fun isInsnAllowed(insn: AbstractInsnNode): Boolean {
+    override fun isInsnAllowed(insn: AbstractInsnNode, decorations: Map<String, Any?>): Boolean {
         return insn.opcode in allowedOpcodes
     }
 
+    override val allowedInsnDescription = "constants"
+
     override val mixinExtrasExpressionContextType = ExpressionContext.Type.MODIFY_CONSTANT
 }
diff --git a/src/main/kotlin/platform/mixin/handlers/RedirectInjectorHandler.kt b/src/main/kotlin/platform/mixin/handlers/RedirectInjectorHandler.kt
@@ -78,10 +78,12 @@ class RedirectInjectorHandler : InjectorAnnotationHandler() {
         }
     }
 
-    override fun isInsnAllowed(insn: AbstractInsnNode): Boolean {
+    override fun isInsnAllowed(insn: AbstractInsnNode, decorations: Map<String, Any?>): Boolean {
         return getRedirectType(insn)?.isInsnAllowed(insn) ?: false
     }
 
+    override val allowedInsnDescription = "redirect targets (see docs)"
+
     override fun expectedMethodSignature(
         annotation: PsiAnnotation,
         targetClass: ClassNode,
diff --git a/src/main/kotlin/platform/mixin/handlers/injectionPoint/AtResolver.kt b/src/main/kotlin/platform/mixin/handlers/injectionPoint/AtResolver.kt
@@ -20,6 +20,8 @@
 
 package com.demonwav.mcdev.platform.mixin.handlers.injectionPoint
 
+import com.demonwav.mcdev.platform.mixin.handlers.InjectorAnnotationHandler
+import com.demonwav.mcdev.platform.mixin.handlers.MixinAnnotationHandler
 import com.demonwav.mcdev.platform.mixin.handlers.desugar.DesugarContext
 import com.demonwav.mcdev.platform.mixin.handlers.desugar.DesugarUtil
 import com.demonwav.mcdev.platform.mixin.reference.MixinSelector
@@ -276,6 +278,8 @@ class AtResolver(
         val injectionPoint = getInjectionPoint(at) ?: return emptyList()
         val targetAttr = at.findAttributeValue("target")
         val target = targetAttr?.let { parseMixinSelector(it) }
+        val injector = findInjectorAnnotation(at)?.let(MixinAnnotationHandler::forMixinAnnotation)
+            as? InjectorAnnotationHandler
 
         // Collect all possible targets
         fun <T : PsiElement> doCollectVariants(injectionPoint: InjectionPoint<T>): List<Any> {
@@ -287,6 +291,7 @@ class AtResolver(
 
             return visitor.visit(targetMethod)
                 .results
+                .filter { result -> injector?.isInsnAllowed(result.insn, result.decorations) != false }
                 .mapNotNull { result ->
                     injectionPoint.createLookup(getTargetClass(target), result)?.let { completionHandler(it) }
                 }
diff --git a/src/main/kotlin/platform/mixin/handlers/mixinextras/MixinExtrasInjectorAnnotationHandler.kt b/src/main/kotlin/platform/mixin/handlers/mixinextras/MixinExtrasInjectorAnnotationHandler.kt
@@ -95,7 +95,9 @@ abstract class MixinExtrasInjectorAnnotationHandler : InjectorAnnotationHandler(
 
     abstract val supportedInstructionTypes: Collection<InstructionType>
 
-    open fun extraTargetRestrictions(insn: AbstractInsnNode): Boolean = true
+    override fun isInsnAllowed(insn: AbstractInsnNode, decorations: Map<String, Any?>): Boolean {
+        return supportedInstructionTypes.any { it.matches(TargetInsn(insn, decorations)) }
+    }
 
     abstract fun expectedMethodSignature(
         annotation: PsiAnnotation,
@@ -118,7 +120,6 @@ abstract class MixinExtrasInjectorAnnotationHandler : InjectorAnnotationHandler(
         val insns = resolveInstructions(annotation, targetClass, targetMethod)
             .ifEmpty { return emptyList() }
             .map { TargetInsn(it.insn, it.decorations) }
-        if (insns.any { insn -> supportedInstructionTypes.none { it.matches(insn) } }) return emptyList()
         val signatures = insns.map { insn ->
             expectedMethodSignature(annotation, targetClass, targetMethod, insn)
         }
diff --git a/src/main/kotlin/platform/mixin/handlers/mixinextras/ModifyExpressionValueHandler.kt b/src/main/kotlin/platform/mixin/handlers/mixinextras/ModifyExpressionValueHandler.kt
@@ -41,11 +41,17 @@ class ModifyExpressionValueHandler : MixinExtrasInjectorAnnotationHandler() {
         InstructionType.SIMPLE_EXPRESSION, InstructionType.STRING_CONCAT_EXPRESSION
     )
 
-    override fun extraTargetRestrictions(insn: AbstractInsnNode): Boolean {
+    override fun isInsnAllowed(insn: AbstractInsnNode, decorations: Map<String, Any?>): Boolean {
+        if (!super.isInsnAllowed(insn, decorations)) {
+            return false
+        }
+
         val returnType = getInsnReturnType(insn) ?: return false
         return returnType != Type.VOID_TYPE
     }
 
+    override val allowedInsnDescription = "instructions that return a value"
+
     override fun expectedMethodSignature(
         annotation: PsiAnnotation,
         targetClass: ClassNode,
diff --git a/src/main/kotlin/platform/mixin/handlers/mixinextras/ModifyReceiverHandler.kt b/src/main/kotlin/platform/mixin/handlers/mixinextras/ModifyReceiverHandler.kt
@@ -34,13 +34,15 @@ class ModifyReceiverHandler : MixinExtrasInjectorAnnotationHandler() {
         InstructionType.METHOD_CALL, InstructionType.FIELD_GET, InstructionType.FIELD_SET
     )
 
-    override fun extraTargetRestrictions(insn: AbstractInsnNode) = when (insn.opcode) {
+    override fun isInsnAllowed(insn: AbstractInsnNode, decorations: Map<String, Any?>) = when (insn.opcode) {
         Opcodes.INVOKEVIRTUAL, Opcodes.INVOKESPECIAL, Opcodes.INVOKEINTERFACE,
         Opcodes.GETFIELD, Opcodes.PUTFIELD -> true
 
         else -> false
     }
 
+    override val allowedInsnDescription = "non-static method invocations and field references"
+
     override fun expectedMethodSignature(
         annotation: PsiAnnotation,
         targetClass: ClassNode,
diff --git a/src/main/kotlin/platform/mixin/handlers/mixinextras/ModifyReturnValueHandler.kt b/src/main/kotlin/platform/mixin/handlers/mixinextras/ModifyReturnValueHandler.kt
@@ -32,6 +32,8 @@ import org.objectweb.asm.tree.MethodNode
 class ModifyReturnValueHandler : MixinExtrasInjectorAnnotationHandler() {
     override val supportedInstructionTypes = listOf(InstructionType.RETURN)
 
+    override val allowedInsnDescription = "return instructions"
+
     override fun expectedMethodSignature(
         annotation: PsiAnnotation,
         targetClass: ClassNode,
diff --git a/src/main/kotlin/platform/mixin/handlers/mixinextras/WrapOperationHandler.kt b/src/main/kotlin/platform/mixin/handlers/mixinextras/WrapOperationHandler.kt
@@ -41,6 +41,8 @@ class WrapOperationHandler : MixinExtrasInjectorAnnotationHandler() {
         InstructionType.INSTANTIATION, InstructionType.SIMPLE_OPERATION
     )
 
+    override val allowedInsnDescription = "wrap operation targets (see docs)"
+
     override fun getAtKey(annotation: PsiAnnotation): String {
         return if (annotation.hasAttribute("constant")) "constant" else "at"
     }
diff --git a/src/main/kotlin/platform/mixin/handlers/mixinextras/WrapWithConditionHandler.kt b/src/main/kotlin/platform/mixin/handlers/mixinextras/WrapWithConditionHandler.kt
@@ -37,7 +37,9 @@ class WrapWithConditionHandler : MixinExtrasInjectorAnnotationHandler() {
         InstructionType.METHOD_CALL, InstructionType.FIELD_SET
     )
 
-    override fun extraTargetRestrictions(insn: AbstractInsnNode) = getInsnReturnType(insn) == Type.VOID_TYPE
+    override fun isInsnAllowed(insn: AbstractInsnNode, decorations: Map<String, Any?>) = super.isInsnAllowed(insn, decorations) && getInsnReturnType(insn) == Type.VOID_TYPE
+
+    override val allowedInsnDescription = "void method invocations and field assignments"
 
     override fun expectedMethodSignature(
         annotation: PsiAnnotation,
diff --git a/src/main/kotlin/platform/mixin/inspection/injector/DisallowedTargetInsnInspection.kt b/src/main/kotlin/platform/mixin/inspection/injector/DisallowedTargetInsnInspection.kt
@@ -0,0 +1,70 @@
+/*
+ * Minecraft Development for IntelliJ
+ *
+ * https://mcdev.io/
+ *
+ * Copyright (C) 2025 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 <https://www.gnu.org/licenses/>.
+ */
+
+package com.demonwav.mcdev.platform.mixin.inspection.injector
+
+import com.demonwav.mcdev.platform.mixin.handlers.InjectorAnnotationHandler
+import com.demonwav.mcdev.platform.mixin.handlers.MixinAnnotationHandler
+import com.demonwav.mcdev.platform.mixin.handlers.injectionPoint.AtResolver
+import com.demonwav.mcdev.platform.mixin.inspection.MixinInspection
+import com.demonwav.mcdev.platform.mixin.util.MethodTargetMember
+import com.demonwav.mcdev.platform.mixin.util.MixinConstants
+import com.demonwav.mcdev.platform.mixin.util.mixinTargets
+import com.demonwav.mcdev.util.findContainingClass
+import com.intellij.codeInspection.ProblemsHolder
+import com.intellij.psi.JavaElementVisitor
+import com.intellij.psi.PsiAnnotation
+
+class DisallowedTargetInsnInspection : MixinInspection() {
+    override fun getStaticDescription() = "Reports disallowed injector targets"
+
+    override fun buildVisitor(holder: ProblemsHolder) = object : JavaElementVisitor() {
+        override fun visitAnnotation(annotation: PsiAnnotation) {
+            if (!annotation.hasQualifiedName(MixinConstants.Annotations.AT)) {
+                return
+            }
+
+            val injectorAnnotation = AtResolver.findInjectorAnnotation(annotation) ?: return
+            val injector = MixinAnnotationHandler.forMixinAnnotation(injectorAnnotation, annotation.project)
+                as? InjectorAnnotationHandler ?: return
+            val containingClass = injectorAnnotation.findContainingClass() ?: return
+            val hasInvalidInstructions = containingClass.mixinTargets.any { targetClass ->
+                injector.resolveTarget(injectorAnnotation, targetClass).any { targetMember ->
+                    if (targetMember !is MethodTargetMember) {
+                        return@any false
+                    }
+
+                    AtResolver(annotation, targetMember.classAndMethod.clazz, targetMember.classAndMethod.method)
+                        .resolveInstructions()
+                        .any {
+                            !injector.isInsnAllowed(it.insn, it.decorations)
+                        }
+                }
+            }
+
+            if (hasInvalidInstructions) {
+                holder.registerProblem(
+                    annotation,
+                    "This injector can only target ${injector.allowedInsnDescription}"
+                )
+            }
+        }
+    }
+}
diff --git a/src/main/kotlin/platform/mixin/inspection/injector/InvalidInjectorMethodSignatureInspection.kt b/src/main/kotlin/platform/mixin/inspection/injector/InvalidInjectorMethodSignatureInspection.kt
@@ -22,6 +22,7 @@ package com.demonwav.mcdev.platform.mixin.inspection.injector
 
 import com.demonwav.mcdev.platform.mixin.handlers.InjectorAnnotationHandler
 import com.demonwav.mcdev.platform.mixin.handlers.MixinAnnotationHandler
+import com.demonwav.mcdev.platform.mixin.handlers.injectionPoint.AtResolver
 import com.demonwav.mcdev.platform.mixin.inspection.MixinInspection
 import com.demonwav.mcdev.platform.mixin.reference.MethodReference
 import com.demonwav.mcdev.platform.mixin.util.MixinConstants
@@ -100,6 +101,17 @@ class InvalidInjectorMethodSignatureInspection : MixinInspection() {
                 val methodAttribute = annotation.findDeclaredAttributeValue("method") ?: continue
                 val targetMethods = MethodReference.resolveAllIfNotAmbiguous(methodAttribute) ?: continue
 
+                val hasDisallowedInsns = targetMethods.any { classAndMethod ->
+                    handler.resolveInstructions(
+                        annotation,
+                        classAndMethod.clazz,
+                        classAndMethod.method
+                    ).any { !handler.isInsnAllowed(it.insn, it.decorations) }
+                }
+                if (hasDisallowedInsns) {
+                    continue
+                }
+
                 for (targetMethod in targetMethods) {
                     if (!reportedStatic) {
                         var shouldBeStatic = targetMethod.method.hasAccess(Opcodes.ACC_STATIC)
diff --git a/src/main/resources/META-INF/plugin.xml b/src/main/resources/META-INF/plugin.xml
@@ -1286,6 +1286,14 @@
                          level="WARNING"
                          hasStaticDescription="true"
                          implementationClass="com.demonwav.mcdev.platform.mixin.inspection.injector.DiscouragedShiftInspection"/>
+        <localInspection displayName="Usage of disallowed injector target"
+                         shortName="DisallowedTargetInsn"
+                         groupName="Mixin"
+                         language="JAVA"
+                         enabledByDefault="true"
+                         level="ERROR"
+                         hasStaticDescription="true"
+                         implementationClass="com.demonwav.mcdev.platform.mixin.inspection.injector.DisallowedTargetInsnInspection"/>
         <localInspection displayName="Usage of LocalCapture.CAPTURE_FAILEXCEPTION"
                          shortName="LocalCaptureFailException"
                          groupName="Mixin"

Original file line number	Diff line number	Diff line change
`@@ -164,10 +164,12 @@ abstract class InjectorAnnotationHandler : MixinAnnotationHandler {`
`164`	`164`	`targetMethod: MethodNode,`
`165`	`165`	`): List<MethodSignature>?`
`166`	`166`
`167`		`- open fun isInsnAllowed(insn: AbstractInsnNode): Boolean {`
	`167`	`+ open fun isInsnAllowed(insn: AbstractInsnNode, decorations: Map<String, Any?>): Boolean {`
`168`	`168`	`return true`
`169`	`169`	`}`
`170`	`170`
	`171`	`+ open val allowedInsnDescription = "all instructions"`
	`172`	`+`
`171`	`173`	`override fun createUnresolvedMessage(annotation: PsiAnnotation): String? {`
`172`	`174`	`return "Cannot resolve any target instructions in target class"`
`173`	`175`	`}`
Original file line number	Diff line number	Diff line change
`@@ -129,9 +129,11 @@ class ModifyConstantHandler : InjectorAnnotationHandler() {`
`129`	`129`	`}.toList()`
`130`	`130`	`}`
`131`	`131`
`132`		`- override fun isInsnAllowed(insn: AbstractInsnNode): Boolean {`
	`132`	`+ override fun isInsnAllowed(insn: AbstractInsnNode, decorations: Map<String, Any?>): Boolean {`
`133`	`133`	`return insn.opcode in allowedOpcodes`
`134`	`134`	`}`
`135`	`135`
	`136`	`+ override val allowedInsnDescription = "constants"`
	`137`	`+`
`136`	`138`	`override val mixinExtrasExpressionContextType = ExpressionContext.Type.MODIFY_CONSTANT`
`137`	`139`	`}`
Original file line number	Diff line number	Diff line change
`@@ -78,10 +78,12 @@ class RedirectInjectorHandler : InjectorAnnotationHandler() {`
`78`	`78`	`}`
`79`	`79`	`}`
`80`	`80`
`81`		`- override fun isInsnAllowed(insn: AbstractInsnNode): Boolean {`
	`81`	`+ override fun isInsnAllowed(insn: AbstractInsnNode, decorations: Map<String, Any?>): Boolean {`
`82`	`82`	`return getRedirectType(insn)?.isInsnAllowed(insn) ?: false`
`83`	`83`	`}`
`84`	`84`
	`85`	`+ override val allowedInsnDescription = "redirect targets (see docs)"`
	`86`	`+`
`85`	`87`	`override fun expectedMethodSignature(`
`86`	`88`	`annotation: PsiAnnotation,`
`87`	`89`	`targetClass: ClassNode,`
Original file line number	Diff line number	Diff line change
`@@ -34,13 +34,15 @@ class ModifyReceiverHandler : MixinExtrasInjectorAnnotationHandler() {`
`34`	`34`	`InstructionType.METHOD_CALL, InstructionType.FIELD_GET, InstructionType.FIELD_SET`
`35`	`35`	`)`
`36`	`36`
`37`		`- override fun extraTargetRestrictions(insn: AbstractInsnNode) = when (insn.opcode) {`
	`37`	`+ override fun isInsnAllowed(insn: AbstractInsnNode, decorations: Map<String, Any?>) = when (insn.opcode) {`
`38`	`38`	`Opcodes.INVOKEVIRTUAL, Opcodes.INVOKESPECIAL, Opcodes.INVOKEINTERFACE,`
`39`	`39`	`Opcodes.GETFIELD, Opcodes.PUTFIELD -> true`
`40`	`40`
`41`	`41`	`else -> false`
`42`	`42`	`}`
`43`	`43`
	`44`	`+ override val allowedInsnDescription = "non-static method invocations and field references"`
	`45`	`+`
`44`	`46`	`override fun expectedMethodSignature(`
`45`	`47`	`annotation: PsiAnnotation,`
`46`	`48`	`targetClass: ClassNode,`