User: joe
Date: 02 Jan 24 02:14
Revision: 2ffce66ee88ba80f6ff5a02cee20ba3b33e8c6aa
Summary:
Prevent @Inject being cancellable before a superconstructor call


TeamCity URL: http://ci.mcdev.io:80/viewModification.html?tab=vcsModificationFiles&modId=8943&personal=false

Index: src/main/kotlin/platform/mixin/handlers/InjectorAnnotationHandler.kt
===================================================================
--- src/main/kotlin/platform/mixin/handlers/InjectorAnnotationHandler.kt	(revision 7de13ba5f87c9b2d4950641cb3fca57919cc1840)
+++ src/main/kotlin/platform/mixin/handlers/InjectorAnnotationHandler.kt	(revision 2ffce66ee88ba80f6ff5a02cee20ba3b33e8c6aa)
@@ -34,6 +34,7 @@
 import com.demonwav.mcdev.platform.mixin.util.hasAccess
 import com.demonwav.mcdev.platform.mixin.util.mixinTargets
 import com.demonwav.mcdev.util.Parameter
+import com.demonwav.mcdev.util.cached
 import com.demonwav.mcdev.util.computeStringArray
 import com.demonwav.mcdev.util.findAnnotations
 import com.demonwav.mcdev.util.findContainingClass
@@ -44,6 +45,8 @@
 import com.intellij.psi.PsiElement
 import com.intellij.psi.PsiEllipsisType
 import com.intellij.psi.PsiType
+import com.intellij.psi.util.PsiModificationTracker
+import java.util.concurrent.ConcurrentHashMap
 import org.objectweb.asm.Opcodes
 import org.objectweb.asm.tree.AbstractInsnNode
 import org.objectweb.asm.tree.ClassNode
@@ -110,9 +113,9 @@
             .flatMap { AtResolver(it, targetClass, targetMethod).resolveNavigationTargets() }
     }
 
-    fun resolveInstructions(annotation: PsiAnnotation): List<InsnResult> {
-        val containingClass = annotation.findContainingClass() ?: return emptyList()
-        return containingClass.mixinTargets.flatMap { resolveInstructions(annotation, it) }
+    fun resolveInstructions(annotation: PsiAnnotation) = annotation.cached(PsiModificationTracker.MODIFICATION_COUNT) {
+        val containingClass = annotation.findContainingClass() ?: return@cached emptyList()
+        containingClass.mixinTargets.flatMap { resolveInstructions(annotation, it) }
     }
 
     fun resolveInstructions(annotation: PsiAnnotation, targetClass: ClassNode): List<InsnResult> {
@@ -131,10 +134,15 @@
         targetMethod: MethodNode,
         mode: CollectVisitor.Mode = CollectVisitor.Mode.MATCH_ALL,
     ): List<CollectVisitor.Result<*>> {
-        return annotation.findAttributeValue(getAtKey(annotation))?.findAnnotations()
-            .ifNullOrEmpty { return emptyList() }!!
+        val cache = annotation.cached(PsiModificationTracker.MODIFICATION_COUNT) {
+            ConcurrentHashMap<Pair<ClassAndMethodNode, CollectVisitor.Mode>, List<CollectVisitor.Result<*>>>()
+        }
+        return cache.computeIfAbsent(ClassAndMethodNode(targetClass, targetMethod) to mode) {
+            annotation.findAttributeValue(getAtKey(annotation))?.findAnnotations()
+                .ifNullOrEmpty { return@computeIfAbsent emptyList() }!!
-            .flatMap { AtResolver(it, targetClass, targetMethod).resolveInstructions(mode) }
-    }
+                .flatMap { AtResolver(it, targetClass, targetMethod).resolveInstructions(mode) }
+        }
+    }
 
     /**
      * Returns a list of valid method signatures for the injector.
Index: src/main/kotlin/platform/mixin/handlers/MixinAnnotationHandler.kt
===================================================================
--- src/main/kotlin/platform/mixin/handlers/MixinAnnotationHandler.kt	(revision 7de13ba5f87c9b2d4950641cb3fca57919cc1840)
+++ src/main/kotlin/platform/mixin/handlers/MixinAnnotationHandler.kt	(revision 2ffce66ee88ba80f6ff5a02cee20ba3b33e8c6aa)
@@ -24,6 +24,7 @@
 import com.demonwav.mcdev.platform.mixin.util.MixinConstants
 import com.demonwav.mcdev.platform.mixin.util.MixinTargetMember
 import com.demonwav.mcdev.platform.mixin.util.mixinTargets
+import com.demonwav.mcdev.util.cached
 import com.demonwav.mcdev.util.findAnnotation
 import com.demonwav.mcdev.util.findContainingClass
 import com.demonwav.mcdev.util.resolveClass
@@ -45,9 +46,9 @@
 import org.objectweb.asm.tree.ClassNode
 
 interface MixinAnnotationHandler {
-    fun resolveTarget(annotation: PsiAnnotation): List<MixinTargetMember> {
-        val containingClass = annotation.findContainingClass() ?: return emptyList()
-        return containingClass.mixinTargets.flatMap { resolveTarget(annotation, it) }
+    fun resolveTarget(annotation: PsiAnnotation) = annotation.cached(PsiModificationTracker.MODIFICATION_COUNT) {
+        val containingClass = annotation.findContainingClass() ?: return@cached emptyList()
+        containingClass.mixinTargets.flatMap { resolveTarget(annotation, it) }
     }
 
     fun resolveTarget(annotation: PsiAnnotation, targetClass: ClassNode): List<MixinTargetMember>
Index: src/main/kotlin/platform/mixin/inspection/MixinCancellableInspection.kt
===================================================================
--- src/main/kotlin/platform/mixin/inspection/MixinCancellableInspection.kt	(revision 7de13ba5f87c9b2d4950641cb3fca57919cc1840)
+++ src/main/kotlin/platform/mixin/inspection/MixinCancellableInspection.kt	(revision 2ffce66ee88ba80f6ff5a02cee20ba3b33e8c6aa)
@@ -20,10 +20,12 @@
 
 package com.demonwav.mcdev.platform.mixin.inspection
 
+import com.demonwav.mcdev.platform.mixin.inspection.injector.CancellableBeforeSuperCallInspection
 import com.demonwav.mcdev.platform.mixin.util.MixinConstants.Annotations.INJECT
 import com.demonwav.mcdev.platform.mixin.util.MixinConstants.Classes.CALLBACK_INFO
 import com.demonwav.mcdev.platform.mixin.util.MixinConstants.Classes.CALLBACK_INFO_RETURNABLE
 import com.demonwav.mcdev.util.fullQualifiedName
+import com.intellij.codeInspection.LocalQuickFix
 import com.intellij.codeInspection.LocalQuickFixAndIntentionActionOnPsiElement
 import com.intellij.codeInspection.ProblemHighlightType
 import com.intellij.codeInspection.ProblemsHolder
@@ -83,11 +85,16 @@
             }
 
             if (definitelyUsesCancel && !isCancellable) {
+                val fixes = mutableListOf<LocalQuickFix>()
+                if (!CancellableBeforeSuperCallInspection.doesInjectBeforeSuperConstructorCall(injectAnnotation)) {
+                    fixes += MakeInjectCancellableFix(injectAnnotation)
+                }
+
                 holder.registerProblem(
                     method.nameIdentifier ?: method,
                     "@Inject must be marked as cancellable in order to be cancelled",
                     ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
-                    MakeInjectCancellableFix(injectAnnotation),
+                    *fixes.toTypedArray(),
                 )
             } else if (!definitelyUsesCancel && !mayUseCancel && isCancellable) {
                 holder.registerProblem(
@@ -120,10 +127,10 @@
         }
     }
 
-    private class RemoveInjectCancellableFix(element: PsiAnnotation) :
+    class RemoveInjectCancellableFix(element: PsiAnnotation) :
         LocalQuickFixAndIntentionActionOnPsiElement(element) {
 
-        override fun getFamilyName(): String = "Remove unused cancellable attribute"
+        override fun getFamilyName(): String = "Remove cancellable attribute"
 
         override fun getText(): String = familyName
 
Index: src/main/kotlin/platform/mixin/inspection/injector/CancellableBeforeSuperCallInspection.kt
===================================================================
--- src/main/kotlin/platform/mixin/inspection/injector/CancellableBeforeSuperCallInspection.kt	(revision 2ffce66ee88ba80f6ff5a02cee20ba3b33e8c6aa)
+++ src/main/kotlin/platform/mixin/inspection/injector/CancellableBeforeSuperCallInspection.kt	(revision 2ffce66ee88ba80f6ff5a02cee20ba3b33e8c6aa)
@@ -0,0 +1,86 @@
+/*
+ * Minecraft Development for IntelliJ
+ *
+ * https://mcdev.io/
+ *
+ * Copyright (C) 2023 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.inspection.MixinCancellableInspection
+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.findSuperConstructorCall
+import com.demonwav.mcdev.platform.mixin.util.isConstructor
+import com.demonwav.mcdev.util.constantValue
+import com.intellij.codeInspection.ProblemsHolder
+import com.intellij.psi.JavaElementVisitor
+import com.intellij.psi.PsiAnnotation
+import com.intellij.psi.PsiElementVisitor
+
+class CancellableBeforeSuperCallInspection : MixinInspection() {
+    override fun getStaticDescription() = "Reports when cancellable @Injects are used before a superconstructor call"
+
+    override fun buildVisitor(holder: ProblemsHolder): PsiElementVisitor = Visitor(holder)
+
+    private class Visitor(private val holder: ProblemsHolder) : JavaElementVisitor() {
+        override fun visitAnnotation(annotation: PsiAnnotation) {
+            if (annotation.qualifiedName != MixinConstants.Annotations.INJECT) {
+                return
+            }
+            val cancellableAttr =
+                annotation.parameterList.attributes.firstOrNull { it.attributeName == "cancellable" } ?: return
+            if (cancellableAttr.value?.constantValue != true) {
+                return
+            }
+            if (doesInjectBeforeSuperConstructorCall(annotation)) {
+                holder.registerProblem(
+                    cancellableAttr,
+                    "@Inject is cancellable before a superconstructor call",
+                    MixinCancellableInspection.RemoveInjectCancellableFix(annotation)
+                )
+            }
+        }
+    }
+
+    companion object {
+        fun doesInjectBeforeSuperConstructorCall(annotation: PsiAnnotation): Boolean {
+            val handler = MixinAnnotationHandler.forMixinAnnotation(MixinConstants.Annotations.INJECT)!!
+                as InjectorAnnotationHandler
+
+            for (target in handler.resolveTarget(annotation)) {
+                if (target !is MethodTargetMember) {
+                    continue
+                }
+                if (!target.classAndMethod.method.isConstructor) {
+                    continue
+                }
+                val methodInsns = target.classAndMethod.method.instructions ?: continue
+                val superCtorCall = target.classAndMethod.method.findSuperConstructorCall() ?: continue
+                val instructions =
+                    handler.resolveInstructions(annotation, target.classAndMethod.clazz, target.classAndMethod.method)
+                if (instructions.any { methodInsns.indexOf(it.insn) <= methodInsns.indexOf(superCtorCall) }) {
+                    return true
+                }
+            }
+
+            return false
+        }
+    }
+}
Index: src/main/kotlin/platform/mixin/inspection/injector/InvalidInjectorMethodSignatureInspection.kt
===================================================================
--- src/main/kotlin/platform/mixin/inspection/injector/InvalidInjectorMethodSignatureInspection.kt	(revision 7de13ba5f87c9b2d4950641cb3fca57919cc1840)
+++ src/main/kotlin/platform/mixin/inspection/injector/InvalidInjectorMethodSignatureInspection.kt	(revision 2ffce66ee88ba80f6ff5a02cee20ba3b33e8c6aa)
@@ -27,6 +27,7 @@
 import com.demonwav.mcdev.platform.mixin.reference.MethodReference
 import com.demonwav.mcdev.platform.mixin.util.MixinConstants
 import com.demonwav.mcdev.platform.mixin.util.MixinConstants.Annotations.COERCE
+import com.demonwav.mcdev.platform.mixin.util.findSuperConstructorCall
 import com.demonwav.mcdev.platform.mixin.util.hasAccess
 import com.demonwav.mcdev.platform.mixin.util.isAssignable
 import com.demonwav.mcdev.platform.mixin.util.isConstructor
@@ -56,9 +57,6 @@
 import com.intellij.psi.util.PsiUtil
 import com.intellij.psi.util.TypeConversionUtil
 import org.objectweb.asm.Opcodes
-import org.objectweb.asm.tree.AbstractInsnNode
-import org.objectweb.asm.tree.InsnList
-import org.objectweb.asm.tree.MethodInsnNode
 
 class InvalidInjectorMethodSignatureInspection : MixinInspection() {
 
@@ -88,8 +86,9 @@
 
                         if (!shouldBeStatic && targetMethod.method.isConstructor) {
                             // before the superclass constructor call, everything must be static
-                            targetMethod.method.instructions?.let { methodInsns ->
-                                val superCtorCall = findSuperConstructorCall(methodInsns)
+                            val methodInsns = targetMethod.method.instructions
+                            val superCtorCall = targetMethod.method.findSuperConstructorCall()
+                            if (methodInsns != null && superCtorCall != null) {
                                 val insns = handler.resolveInstructions(
                                     annotation,
                                     targetMethod.clazz,
@@ -199,27 +198,6 @@
             }
         }
 
-        private fun findSuperConstructorCall(methodInsns: InsnList): AbstractInsnNode? {
-            var superCtorCall = methodInsns.first
-            var newCount = 0
-            while (superCtorCall != null) {
-                if (superCtorCall.opcode == Opcodes.NEW) {
-                    newCount++
-                } else if (superCtorCall.opcode == Opcodes.INVOKESPECIAL) {
-                    val methodCall = superCtorCall as MethodInsnNode
-                    if (methodCall.name == "<init>") {
-                        if (newCount == 0) {
-                            return superCtorCall
-                        } else {
-                            newCount--
-                        }
-                    }
-                }
-                superCtorCall = superCtorCall.next
-            }
-            return null
-        }
-
         private fun checkReturnType(
             expectedReturnType: PsiType,
             methodReturnType: PsiType,
Index: src/main/kotlin/platform/mixin/util/AsmUtil.kt
===================================================================
--- src/main/kotlin/platform/mixin/util/AsmUtil.kt	(revision 7de13ba5f87c9b2d4950641cb3fca57919cc1840)
+++ src/main/kotlin/platform/mixin/util/AsmUtil.kt	(revision 2ffce66ee88ba80f6ff5a02cee20ba3b33e8c6aa)
@@ -79,6 +79,7 @@
 import org.objectweb.asm.Opcodes
 import org.objectweb.asm.Type
 import org.objectweb.asm.signature.SignatureReader
+import org.objectweb.asm.tree.AbstractInsnNode
 import org.objectweb.asm.tree.ClassNode
 import org.objectweb.asm.tree.FieldInsnNode
 import org.objectweb.asm.tree.FieldNode
@@ -577,6 +578,31 @@
 val MethodNode.isClinit
     get() = this.name == "<clinit>"
 
+/**
+ * Finds the super() call in this method node, assuming it is a constructor
+ */
+fun MethodNode.findSuperConstructorCall(): AbstractInsnNode? {
+    val insns = instructions ?: return null
+    var superCtorCall = insns.first
+    var newCount = 0
+    while (superCtorCall != null) {
+        if (superCtorCall.opcode == Opcodes.NEW) {
+            newCount++
+        } else if (superCtorCall.opcode == Opcodes.INVOKESPECIAL) {
+            val methodCall = superCtorCall as MethodInsnNode
+            if (methodCall.name == "<init>") {
+                if (newCount == 0) {
+                    return superCtorCall
+                } else {
+                    newCount--
+                }
+            }
+        }
+        superCtorCall = superCtorCall.next
+    }
+    return null
+}
+
 private fun findContainingMethod(clazz: ClassNode, lambdaMethod: MethodNode): Pair<MethodNode, Int>? {
     if (!lambdaMethod.hasAccess(Opcodes.ACC_SYNTHETIC)) {
         return null
Index: src/main/resources/META-INF/plugin.xml
===================================================================
--- src/main/resources/META-INF/plugin.xml	(revision 7de13ba5f87c9b2d4950641cb3fca57919cc1840)
+++ src/main/resources/META-INF/plugin.xml	(revision 2ffce66ee88ba80f6ff5a02cee20ba3b33e8c6aa)
@@ -878,6 +878,14 @@
                          level="WARNING"
                          hasStaticDescription="true"
                          implementationClass="com.demonwav.mcdev.platform.mixin.inspection.injector.ModifyVariableArgsOnlyInspection"/>
+        <localInspection displayName="@Inject before a superconstructor call is cancellable"
+                         shortName="InjectCancellableBeforeSuperConstructorCall"
+                         groupName="Mixin"
+                         language="JAVA"
+                         enabledByDefault="true"
+                         level="ERROR"
+                         hasStaticDescription="true"
+                         implementationClass="com.demonwav.mcdev.platform.mixin.inspection.injector.CancellableBeforeSuperCallInspection"/>
         <!--endregion-->
 
         <!--region Overwrite Inspections -->