Fix: Handle quantifiers and staticness properly when resolving Mixin target methods. (#2645)

We should only take as many matches as is allowed, and additionally static methods should not be considered unless our handler is also static or we are trying to select exactly 1 method.

Author: LlamaLad7

src/main/kotlin/platform/mixin/handlers/InjectorAnnotationHandler.kt

@@ -31,6 +31,7 @@ import com.demonwav.mcdev.platform.mixin.reference.parseMixinSelector
 import com.demonwav.mcdev.platform.mixin.util.ClassAndMethodNode
 import com.demonwav.mcdev.platform.mixin.util.MethodTargetMember
 import com.demonwav.mcdev.platform.mixin.util.MixinTargetMember
+import com.demonwav.mcdev.platform.mixin.util.findMethods
 import com.demonwav.mcdev.platform.mixin.util.getGenericParameterTypes
 import com.demonwav.mcdev.platform.mixin.util.hasAccess
 import com.demonwav.mcdev.platform.mixin.util.mixinTargets
@@ -46,8 +47,10 @@ import com.intellij.psi.PsiAnnotation
 import com.intellij.psi.PsiElement
 import com.intellij.psi.PsiEllipsisType
 import com.intellij.psi.PsiMethod
+import com.intellij.psi.PsiModifier
 import com.intellij.psi.PsiType
 import com.intellij.psi.util.PsiModificationTracker
+import com.intellij.psi.util.findParentOfType
 import com.llamalad7.mixinextras.expression.impl.point.ExpressionContext
 import java.util.concurrent.ConcurrentHashMap
 import org.objectweb.asm.Opcodes
@@ -63,21 +66,19 @@ abstract class InjectorAnnotationHandler : MixinAnnotationHandler {
         val selectors = method.mapNotNull { parseMixinSelector(it, methodAttr!!) } +
             desc.mapNotNull { DescSelectorParser.Util.descSelectorFromAnnotation(it) }
 
-        val targetClassMethods = selectors.associateWith { selector ->
-            val actualTarget = selector.getCustomOwner(targetClass)
-            (actualTarget to actualTarget.methods)
+        val targetsBySelector = selectors.associateWith { selector ->
+            selector.getCustomOwner(targetClass)
         }
+        val allowStatic = annotation.findParentOfType<PsiMethod>()?.hasModifierProperty(PsiModifier.STATIC) ?: true
 
-        return targetClassMethods.flatMap { (selector, pair) ->
-            val (clazz, methods) = pair
-            methods.mapNotNull { method ->
-                if (selector.matchMethod(method, clazz)) {
-                    MethodTargetMember(clazz, method)
-                } else {
-                    null
-                }
+        return targetsBySelector.asSequence()
+            .flatMap { (selector, targetClass) ->
+                targetClass.findMethods(selector, allowStatic)
+                    .map { ClassAndMethodNode(targetClass, it) }
             }
-        }
+            .distinct()
+            .map { MethodTargetMember(it) }
+            .toList()
     }
 
     override fun isUnresolved(annotation: PsiAnnotation, targetClass: ClassNode): InsnResolutionInfo.Failure? {

src/main/kotlin/platform/mixin/handlers/injectionPoint/NewInsnInjectionPoint.kt

@@ -143,7 +143,7 @@ class NewInsnInjectionPoint : InjectionPoint<PsiMember>() {
             val anonymousName = anonymousClass?.fullQualifiedName?.replace('.', '/')
             if (anonymousName != null) {
                 val methods = findClassNodeByPsiClass(anonymousClass)
-                    ?.findMethods(selector.withQuantifier(Quantifier.Default))
+                    ?.findMethods(selector.withQuantifier(Quantifier.Any), allowStatic = true)
                     .orEmpty()
 
                 if (methods.any { selector.matchMethod(anonymousName, it.name, it.desc) }) {

src/main/kotlin/platform/mixin/reference/AbstractMethodReference.kt

@@ -48,6 +48,8 @@ import com.intellij.codeInsight.lookup.LookupElementBuilder
 import com.intellij.psi.PsiAnnotation
 import com.intellij.psi.PsiArrayInitializerMemberValue
 import com.intellij.psi.PsiElement
+import com.intellij.psi.PsiMethod
+import com.intellij.psi.PsiModifier
 import com.intellij.psi.PsiSubstitutor
 import com.intellij.psi.ResolveResult
 import com.intellij.psi.util.parentOfType
@@ -82,13 +84,15 @@ abstract class AbstractMethodReference : PolyReferenceResolver(), MixinReference
             return false
         }
 
+        val allowStatic = context.parentOfType<PsiMethod>()?.hasModifierProperty(PsiModifier.STATIC) ?: true
         val stringValue = context.constantStringValue ?: return false
         val targetMethodInfo = parseSelector(stringValue, context) ?: return false
         val minMatches = targetMethodInfo.quantifier.min(Quantifier.Context.MEMBER).coerceAtLeast(1)
         val targets = getTargets(context) ?: return false
 
         return targets.any {
-            targetMethodInfo.getCustomOwner(it).findMethods(targetMethodInfo).countIsLessThan(minMatches)
+            targetMethodInfo.getCustomOwner(it).findMethods(targetMethodInfo, allowStatic)
+                .countIsLessThan(minMatches)
         }
     }
 
@@ -104,10 +108,13 @@ abstract class AbstractMethodReference : PolyReferenceResolver(), MixinReference
     }
 
     private fun isAmbiguous(targets: Collection<ClassNode>, targetReference: MemberInfo): Boolean {
-        return targets.any { it.findMethods(targetReference.withQuantifier(Quantifier.Any)).countIsAtLeast(2) }
+        return targets.any {
+            it.findMethods(targetReference.withQuantifier(Quantifier.Any), allowStatic = true).countIsAtLeast(2)
+        }
     }
 
     fun resolve(context: PsiElement): Sequence<ClassAndMethodNode>? {
+        val allowStatic = context.parentOfType<PsiMethod>()?.hasModifierProperty(PsiModifier.STATIC) ?: true
         val targets = getTargets(context) ?: return null
         val targetedMethods = when (context) {
             is PsiArrayInitializerMemberValue -> context.initializers.mapNotNull { it.constantStringValue }
@@ -116,18 +123,19 @@ abstract class AbstractMethodReference : PolyReferenceResolver(), MixinReference
 
         return targetedMethods.asSequence().flatMap { method ->
             val targetReference = parseSelector(method, context) ?: return@flatMap emptySequence()
-            return@flatMap resolve(targets, targetReference)
+            return@flatMap resolve(targets, targetReference, allowStatic)
         }
     }
 
     private fun resolve(
         targets: Collection<ClassNode>,
         selector: MixinSelector,
+        allowStatic: Boolean,
     ): Sequence<ClassAndMethodNode> {
         return targets.asSequence()
             .flatMap { target ->
                 val actualTarget = selector.getCustomOwner(target)
-                actualTarget.findMethods(selector).map { ClassAndMethodNode(actualTarget, it) }
+                actualTarget.findMethods(selector, allowStatic).map { ClassAndMethodNode(actualTarget, it) }
             }
     }
 

src/main/kotlin/platform/mixin/util/AsmUtil.kt

@@ -472,9 +472,11 @@ fun ClassNode.findField(ref: MemberMatcher): FieldNode? {
     return findFields(ref).firstOrNull()
 }
 
-fun ClassNode.findMethods(ref: MixinSelector): Sequence<MethodNode> {
+fun ClassNode.findMethods(ref: MixinSelector, allowStatic: Boolean): Sequence<MethodNode> {
     val maxMatches = ref.quantifier.max(Quantifier.Context.MEMBER)
-    return methods?.asSequence()?.filter { ref.matchMethod(it, this) }?.take(maxMatches).orEmpty()
+    return methods?.asSequence()?.filter {
+        ref.matchMethod(it, this) && (maxMatches <= 1 || allowStatic || !it.hasAccess(Opcodes.ACC_STATIC))
+    }?.take(maxMatches).orEmpty()
 }
 
 fun ClassNode.findMethod(ref: MemberReference): MethodNode? {