Skip to content

Commit dec3e63

Browse files
Feature/mixin quantifiers (#2643)
* Refactor: Split MemberInfo and MemberReference. Mixin MemberInfo strings are only tangentially related to concrete MemberReferences. We also introduce a MemberMatcher superinterface for resolving things. * Refactor: Parse MemberInfos properly and represent Quantifiers. * Fix: Handle all cases properly in AmbiguousReferenceInspection. Really nothing is "ambiguous" (except perhaps to the reader), it's just that Mixin's default behaviour of matching the first method in a given class is unexpected, but I'll leave the terminology for now. Adding any explicit quantifier suppresses this inspection, including `{1}` if that's really what's desired. Additionally, adding a wildcard is almost never the correct fix. Choosing one of the available descriptors is what should be offered instead, but I'll leave that too for now. * New: Handle maximum values in quantifiers. * Fix: Fix incorrect filter stats emptiness check. Causes targets not to show as unresolved when they should. * New: Handle minimum values in quantifiers. * Fix: Account for individual Mixin `method` strings contributing multiple methods. * Fix: Don't report injector signature issues in methods where we find no targets. * Move insn resolution failure message to field in failure class, and add message for min quantifier not satisfied --------- Co-authored-by: joe <burtonjae@hotmail.co.uk>
1 parent b8a594f commit dec3e63

33 files changed

Lines changed: 769 additions & 387 deletions

mixin-test-data/src/main/java/com/demonwav/mcdev/mixintestdata/ambiguousReference/MixedIn.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*
44
* https://mcdev.io/
55
*
6-
* Copyright (C) 2025 minecraft-dev
6+
* Copyright (C) 2026 minecraft-dev
77
*
88
* This program is free software: you can redistribute it and/or modify
99
* it under the terms of the GNU Lesser General Public License as published
@@ -29,4 +29,7 @@ public void method(String string) {
2929

3030
public void uniqueMethod(String string) {
3131
}
32+
33+
public void uniqueDescMethod(int i) {
34+
}
3235
}

src/main/kotlin/platform/mixin/action/CopyMixinTargetReferenceAction.kt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*
44
* https://mcdev.io/
55
*
6-
* Copyright (C) 2025 minecraft-dev
6+
* Copyright (C) 2026 minecraft-dev
77
*
88
* This program is free software: you can redistribute it and/or modify
99
* it under the terms of the GNU Lesser General Public License as published
@@ -21,7 +21,6 @@
2121
package com.demonwav.mcdev.platform.mixin.action
2222

2323
import com.demonwav.mcdev.platform.mixin.handlers.injectionPoint.QualifiedMember
24-
import com.demonwav.mcdev.platform.mixin.reference.toMixinString
2524
import com.demonwav.mcdev.util.findReferencedMember
2625
import com.demonwav.mcdev.util.getQualifiedMemberReference
2726
import com.demonwav.mcdev.util.qualifiedMemberReference

src/main/kotlin/platform/mixin/expression/MEDefinitionFoldingBuilder.kt

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*
44
* https://mcdev.io/
55
*
6-
* Copyright (C) 2025 minecraft-dev
6+
* Copyright (C) 2026 minecraft-dev
77
*
88
* This program is free software: you can redistribute it and/or modify
99
* it under the terms of the GNU Lesser General Public License as published
@@ -24,6 +24,7 @@ import com.demonwav.mcdev.platform.mixin.MixinModuleType
2424
import com.demonwav.mcdev.platform.mixin.folding.MixinFoldingSettings
2525
import com.demonwav.mcdev.platform.mixin.reference.target.FieldDefinitionReference
2626
import com.demonwav.mcdev.platform.mixin.reference.target.MethodDefinitionReference
27+
import com.demonwav.mcdev.platform.mixin.util.MemberInfo
2728
import com.demonwav.mcdev.platform.mixin.util.MixinConstants
2829
import com.demonwav.mcdev.util.MemberReference
2930
import com.intellij.lang.ASTNode
@@ -49,8 +50,8 @@ class MEDefinitionFoldingBuilder : CustomFoldingBuilder() {
4950
val psi = node.psi
5051
if (psi is PsiLiteralExpression) {
5152
val value = psi.value as? String ?: return "..."
52-
val memberReference = MemberReference.parse(value) ?: return "..."
53-
return memberReference.presentableText
53+
val memberInfo = MemberInfo.parse(value) ?: return "..."
54+
return MemberReference(memberInfo.name ?: "*", memberInfo.descriptor, memberInfo.owner).presentableText
5455
}
5556
return "..."
5657
}
@@ -108,7 +109,7 @@ class MEDefinitionFoldingBuilder : CustomFoldingBuilder() {
108109
if (FieldDefinitionReference.ELEMENT_PATTERN.accepts(expression) ||
109110
MethodDefinitionReference.ELEMENT_PATTERN.accepts(expression)
110111
) {
111-
if (MemberReference.parse(expression.value as String) != null) {
112+
if (MemberInfo.parse(expression.value as String) != null) {
112113
descriptors.add(FoldingDescriptor(expression.node, expression.textRange))
113114
}
114115
}

src/main/kotlin/platform/mixin/expression/MEExpressionMatchUtil.kt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*
44
* https://mcdev.io/
55
*
6-
* Copyright (C) 2025 minecraft-dev
6+
* Copyright (C) 2026 minecraft-dev
77
*
88
* This program is free software: you can redistribute it and/or modify
99
* it under the terms of the GNU Lesser General Public License as published
@@ -24,8 +24,8 @@ import com.demonwav.mcdev.platform.mixin.handlers.InjectorAnnotationHandler
2424
import com.demonwav.mcdev.platform.mixin.handlers.MixinAnnotationHandler
2525
import com.demonwav.mcdev.platform.mixin.handlers.injectionPoint.CollectVisitor
2626
import com.demonwav.mcdev.platform.mixin.util.LocalInfo
27+
import com.demonwav.mcdev.platform.mixin.util.MemberInfo
2728
import com.demonwav.mcdev.platform.mixin.util.MixinConstants
28-
import com.demonwav.mcdev.util.MemberReference
2929
import com.demonwav.mcdev.util.computeStringArray
3030
import com.demonwav.mcdev.util.constantStringValue
3131
import com.demonwav.mcdev.util.descriptor
@@ -172,7 +172,7 @@ object MEExpressionMatchUtil {
172172

173173
val fields = annotation.findDeclaredAttributeValue("field")?.computeStringArray() ?: emptyList()
174174
for (field in fields) {
175-
val fieldRef = MemberReference.parse(field) ?: continue
175+
val fieldRef = MemberInfo.parse(field) ?: continue
176176
pool.addMember(
177177
definitionId,
178178
SimpleMemberDefinition {
@@ -183,7 +183,7 @@ object MEExpressionMatchUtil {
183183

184184
val methods = annotation.findDeclaredAttributeValue("method")?.computeStringArray() ?: emptyList()
185185
for (method in methods) {
186-
val methodRef = MemberReference.parse(method) ?: continue
186+
val methodRef = MemberInfo.parse(method) ?: continue
187187
pool.addMember(
188188
definitionId,
189189
object : SimpleMemberDefinition {

src/main/kotlin/platform/mixin/expression/MESourceMatchContext.kt

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*
44
* https://mcdev.io/
55
*
6-
* Copyright (C) 2025 minecraft-dev
6+
* Copyright (C) 2026 minecraft-dev
77
*
88
* This program is free software: you can redistribute it and/or modify
99
* it under the terms of the GNU Lesser General Public License as published
@@ -21,7 +21,7 @@
2121
package com.demonwav.mcdev.platform.mixin.expression
2222

2323
import com.demonwav.mcdev.platform.mixin.util.LocalInfo
24-
import com.demonwav.mcdev.util.MemberReference
24+
import com.demonwav.mcdev.platform.mixin.util.MemberInfo
2525
import com.intellij.openapi.project.Project
2626
import com.intellij.psi.PsiElement
2727

@@ -32,8 +32,8 @@ class MESourceMatchContext(val project: Project) {
3232
val captures: List<PsiElement> get() = capturesInternal
3333

3434
private val types = mutableMapOf<String, MutableList<String>>()
35-
private val fields = mutableMapOf<String, MutableList<MemberReference>>()
36-
private val methods = mutableMapOf<String, MutableList<MemberReference>>()
35+
private val fields = mutableMapOf<String, MutableList<MemberInfo>>()
36+
private val methods = mutableMapOf<String, MutableList<MemberInfo>>()
3737
private val localInfos = mutableMapOf<String, MutableList<LocalInfo>>()
3838

3939
init {
@@ -57,15 +57,15 @@ class MESourceMatchContext(val project: Project) {
5757
types.getOrPut(key, ::mutableListOf) += desc
5858
}
5959

60-
fun getFields(key: String): List<MemberReference> = fields[key] ?: emptyList()
60+
fun getFields(key: String): List<MemberInfo> = fields[key] ?: emptyList()
6161

62-
fun addField(key: String, field: MemberReference) {
62+
fun addField(key: String, field: MemberInfo) {
6363
fields.getOrPut(key, ::mutableListOf) += field
6464
}
6565

66-
fun getMethods(key: String): List<MemberReference> = methods[key] ?: emptyList()
66+
fun getMethods(key: String): List<MemberInfo> = methods[key] ?: emptyList()
6767

68-
fun addMethod(key: String, method: MemberReference) {
68+
fun addMethod(key: String, method: MemberInfo) {
6969
methods.getOrPut(key, ::mutableListOf) += method
7070
}
7171

src/main/kotlin/platform/mixin/folding/MixinTargetDescriptorFoldingBuilder.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*
44
* https://mcdev.io/
55
*
6-
* Copyright (C) 2025 minecraft-dev
6+
* Copyright (C) 2026 minecraft-dev
77
*
88
* This program is free software: you can redistribute it and/or modify
99
* it under the terms of the GNU Lesser General Public License as published
@@ -51,7 +51,7 @@ class MixinTargetDescriptorFoldingBuilder : CustomFoldingBuilder() {
5151

5252
override fun getLanguagePlaceholderText(node: ASTNode, range: TextRange): String? {
5353
val element = node.psi
54-
return TargetReference.resolveTarget(element)?.let { formatElement(it) }
54+
return TargetReference.resolveTargets(element).singleOrNull()?.let { formatElement(it) }
5555
}
5656

5757
private fun formatElement(element: PsiElement): String? {

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

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,9 @@ abstract class InjectorAnnotationHandler : MixinAnnotationHandler {
6868
(actualTarget to actualTarget.methods)
6969
}
7070

71-
return targetClassMethods.mapNotNull { (selector, pair) ->
71+
return targetClassMethods.flatMap { (selector, pair) ->
7272
val (clazz, methods) = pair
73-
methods.firstNotNullOfOrNull { method ->
73+
methods.mapNotNull { method ->
7474
if (selector.matchMethod(method, clazz)) {
7575
MethodTargetMember(clazz, method)
7676
} else {
@@ -88,9 +88,9 @@ abstract class InjectorAnnotationHandler : MixinAnnotationHandler {
8888
}
8989

9090
return resolveTarget(annotation, targetClass).map { targetMember ->
91-
val targetMethod = targetMember as? MethodTargetMember ?: return@map InsnResolutionInfo.Failure()
91+
val targetMethod = targetMember as? MethodTargetMember ?: return@map InsnResolutionInfo.Failure(AtResolver.DEFAULT_UNRESOLVED_MESSAGE)
9292
isUnresolved(annotation, targetClass, targetMethod.classAndMethod.method) ?: return@isUnresolved null
93-
}.reduceOrNull(InsnResolutionInfo.Failure::combine) ?: InsnResolutionInfo.Failure()
93+
}.reduceOrNull(InsnResolutionInfo.Failure::combine) ?: InsnResolutionInfo.Failure(AtResolver.DEFAULT_UNRESOLVED_MESSAGE)
9494
}
9595

9696
open fun getAtKey(annotation: PsiAnnotation): String = "at"
@@ -101,7 +101,7 @@ abstract class InjectorAnnotationHandler : MixinAnnotationHandler {
101101
targetMethod: MethodNode,
102102
): InsnResolutionInfo.Failure? {
103103
return annotation.findAttributeValue(getAtKey(annotation))?.findAnnotations()
104-
.ifNullOrEmpty { return InsnResolutionInfo.Failure() }!!
104+
.ifNullOrEmpty { return InsnResolutionInfo.Failure(AtResolver.DEFAULT_UNRESOLVED_MESSAGE) }!!
105105
.firstNotNullOfOrNull { AtResolver(it, targetClass, targetMethod).isUnresolved() }
106106
}
107107

@@ -170,10 +170,6 @@ abstract class InjectorAnnotationHandler : MixinAnnotationHandler {
170170

171171
open val allowedInsnDescription = "all instructions"
172172

173-
override fun createUnresolvedMessage(annotation: PsiAnnotation): String? {
174-
return "Cannot resolve any target instructions in target class"
175-
}
176-
177173
open fun canAlwaysBeStatic(method: PsiMethod): Boolean {
178174
return true
179175
}

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ interface MixinAnnotationHandler {
5252
fun resolveTarget(annotation: PsiAnnotation, targetClass: ClassNode): List<MixinTargetMember>
5353

5454
fun isUnresolved(annotation: PsiAnnotation): InsnResolutionInfo.Failure? {
55-
val containingClass = annotation.findContainingClass() ?: return InsnResolutionInfo.Failure()
55+
val containingClass = annotation.findContainingClass() ?: return null // no containing mixin, don't show unresolved error
5656
return containingClass.mixinTargets
5757
.mapNotNull { isUnresolved(annotation, it) }
5858
.reduceOrNull(InsnResolutionInfo.Failure::combine)
@@ -67,8 +67,6 @@ interface MixinAnnotationHandler {
6767

6868
fun resolveForNavigation(annotation: PsiAnnotation, targetClass: ClassNode): List<PsiElement>
6969

70-
fun createUnresolvedMessage(annotation: PsiAnnotation): String?
71-
7270
/**
7371
* Returns true if we don't actually know the implementation of the annotation, and we're just making
7472
* a guess. Prevents unresolved errors but still attempts navigation

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import org.objectweb.asm.tree.ClassNode
2828
interface MixinMemberAnnotationHandler : MixinAnnotationHandler {
2929
override fun isUnresolved(annotation: PsiAnnotation, targetClass: ClassNode): InsnResolutionInfo.Failure? {
3030
return if (resolveTarget(annotation, targetClass).isEmpty()) {
31-
InsnResolutionInfo.Failure()
31+
createUnresolvedMessage(annotation)?.let(InsnResolutionInfo<Nothing>::Failure)
3232
} else {
3333
null
3434
}
@@ -39,4 +39,6 @@ interface MixinMemberAnnotationHandler : MixinAnnotationHandler {
3939
val targets = resolveTarget(annotation, targetClass)
4040
return targets.mapNotNull { it.findSourceElement(project, annotation.resolveScope, canDecompile = true) }
4141
}
42+
43+
fun createUnresolvedMessage(annotation: PsiAnnotation): String?
4244
}

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

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,11 @@ import com.demonwav.mcdev.platform.mixin.util.findSourceClass
3636
import com.demonwav.mcdev.platform.mixin.util.findSourceElement
3737
import com.demonwav.mcdev.platform.mixin.util.isClinit
3838
import com.demonwav.mcdev.platform.mixin.util.memberReference
39+
import com.demonwav.mcdev.util.Quantifier
3940
import com.demonwav.mcdev.util.computeStringArray
4041
import com.demonwav.mcdev.util.constantStringValue
4142
import com.demonwav.mcdev.util.constantValue
43+
import com.demonwav.mcdev.util.countIsAtLeast
4244
import com.demonwav.mcdev.util.descriptor
4345
import com.demonwav.mcdev.util.equivalentTo
4446
import com.demonwav.mcdev.util.findMethods
@@ -174,6 +176,8 @@ class AtResolver(
174176
else -> 0
175177
}
176178
}
179+
180+
const val DEFAULT_UNRESOLVED_MESSAGE = "Cannot resolve any instructions in target"
177181
}
178182

179183
fun isUnresolved(): InsnResolutionInfo.Failure? {
@@ -190,14 +194,24 @@ class AtResolver(
190194
)
191195
if (collectVisitor == null) {
192196
// syntax error in target
193-
val stringValue = targetAttr?.constantStringValue ?: return InsnResolutionInfo.Failure()
197+
val stringValue = targetAttr?.constantStringValue ?: return InsnResolutionInfo.Failure(DEFAULT_UNRESOLVED_MESSAGE)
194198
return if (isMiscDynamicSelector(at.project, stringValue)) {
195199
null
196200
} else {
197-
InsnResolutionInfo.Failure()
201+
InsnResolutionInfo.Failure(DEFAULT_UNRESOLVED_MESSAGE)
202+
}
203+
}
204+
return when (val result = collectVisitor.visit(targetMethod)) {
205+
is InsnResolutionInfo.Failure -> result
206+
is InsnResolutionInfo.Success -> {
207+
val minMatches = collectVisitor.quantifier.min(Quantifier.Context.INSTRUCTION).coerceAtLeast(1)
208+
if (result.results.countIsAtLeast(minMatches)) {
209+
null
210+
} else {
211+
InsnResolutionInfo.Failure("Quantifier requires at least $minMatches matches")
212+
}
198213
}
199214
}
200-
return collectVisitor.visit(targetMethod) as? InsnResolutionInfo.Failure
201215
}
202216

203217
fun resolveInstructions(
@@ -207,12 +221,12 @@ class AtResolver(
207221
}
208222

209223
fun getInstructionResolutionInfo(mode: CollectVisitor.Mode = CollectVisitor.Mode.RESOLUTION): InsnResolutionInfo<*> {
210-
val injectionPoint = getInjectionPoint(at) ?: return InsnResolutionInfo.Failure()
224+
val injectionPoint = getInjectionPoint(at) ?: return InsnResolutionInfo.Failure(DEFAULT_UNRESOLVED_MESSAGE)
211225
val targetAttr = at.findAttributeValue("target")
212226
val target = targetAttr?.let { parseMixinSelector(it) }
213227

214228
val collectVisitor = injectionPoint.createCollectVisitor(at, target, getTargetClass(target), mode)
215-
?: return InsnResolutionInfo.Failure()
229+
?: return InsnResolutionInfo.Failure(DEFAULT_UNRESOLVED_MESSAGE)
216230

217231
return collectVisitor.visit(targetMethod)
218232
}
@@ -311,13 +325,19 @@ class AtResolver(
311325

312326
sealed class InsnResolutionInfo<out T : PsiElement>(val results: Sequence<CollectVisitor.Result<T>>) {
313327
class Success<T : PsiElement>(results: Sequence<CollectVisitor.Result<T>>) : InsnResolutionInfo<T>(results)
314-
class Failure(val filterStats: Map<String, Int> = emptyMap()) : InsnResolutionInfo<Nothing>(emptySequence()) {
328+
class Failure(val messages: Set<String>, val filterStats: Map<String, Int>) : InsnResolutionInfo<Nothing>(emptySequence()) {
329+
constructor(message: String, filterStats: Map<String, Int> = emptyMap()) : this(linkedSetOf(message), filterStats)
330+
315331
infix fun combine(other: Failure): Failure {
332+
val messages = linkedSetOf<String>()
333+
messages += this.messages
334+
messages += other.messages
335+
316336
val result = LinkedHashMap(this.filterStats)
317337
for ((key, value) in other.filterStats) {
318338
result[key] = (result[key] ?: 0) + value
319339
}
320-
return Failure(result)
340+
return Failure(messages, result)
321341
}
322342
}
323343
}

0 commit comments

Comments
 (0)