Skip to content

Commit 5cc5385

Browse files
EarthcomputerEarthcomputer
committed
Downgrade non-void WrapWithCondition to warning if there is a pop instruction afterwards, with a quick-fix to replace with WrapOperation
1 parent 6fc242f commit 5cc5385

File tree

3 files changed

+239
-1
lines changed

3 files changed

+239
-1
lines changed

‎src/main/kotlin/platform/mixin/handlers/mixinextras/WrapWithConditionHandler.kt‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,12 @@
2121
package com.demonwav.mcdev.platform.mixin.handlers.mixinextras
2222

2323
import com.demonwav.mcdev.platform.mixin.inspection.injector.ParameterGroup
24+
import com.demonwav.mcdev.platform.mixin.util.nextRealInsn
2425
import com.intellij.psi.PsiAnnotation
2526
import com.intellij.psi.PsiType
2627
import com.intellij.psi.PsiTypes
2728
import com.llamalad7.mixinextras.expression.impl.point.ExpressionContext
29+
import org.objectweb.asm.Opcodes
2830
import org.objectweb.asm.Type
2931
import org.objectweb.asm.tree.AbstractInsnNode
3032
import org.objectweb.asm.tree.ClassNode
@@ -37,10 +39,33 @@ class WrapWithConditionHandler : MixinExtrasInjectorAnnotationHandler() {
3739
InstructionType.METHOD_CALL, InstructionType.FIELD_SET
3840
)
3941

40-
override fun isInsnAllowed(insn: AbstractInsnNode, decorations: Map<String, Any?>) = super.isInsnAllowed(insn, decorations) && getInsnReturnType(insn) == Type.VOID_TYPE
42+
override fun isInsnAllowed(insn: AbstractInsnNode, decorations: Map<String, Any?>): Boolean {
43+
if (!super.isInsnAllowed(insn, decorations)) {
44+
return false
45+
}
46+
47+
return when (val size = getInsnReturnType(insn)?.size) {
48+
0 -> true
49+
1 -> insn.nextRealInsn?.opcode == Opcodes.POP
50+
2 -> insn.nextRealInsn?.opcode == Opcodes.POP2
51+
else -> throw IllegalStateException("Unexpected return type size: $size")
52+
}
53+
}
4154

4255
override val allowedInsnDescription = "void method invocations and field assignments"
4356

57+
fun getValidTargetType(insn: AbstractInsnNode): Type? {
58+
if (!isInsnAllowed(insn, emptyMap())) {
59+
return null
60+
}
61+
62+
return getInsnReturnType(insn)
63+
}
64+
65+
fun getRequiredParameterCount(insn: AbstractInsnNode, targetClass: ClassNode, annotation: PsiAnnotation): Int {
66+
return getPsiParameters(insn, targetClass, annotation)?.size ?: 0
67+
}
68+
4469
override fun expectedMethodSignature(
4570
annotation: PsiAnnotation,
4671
targetClass: ClassNode,

‎src/main/kotlin/platform/mixin/inspection/mixinextras/WrapWithConditionValidNonVoidInspection.kt‎

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
/*
2+
* Minecraft Development for IntelliJ
3+
*
4+
* https://mcdev.io/
5+
*
6+
* Copyright (C) 2025 minecraft-dev
7+
*
8+
* This program is free software: you can redistribute it and/or modify
9+
* it under the terms of the GNU Lesser General Public License as published
10+
* by the Free Software Foundation, version 3.0 only.
11+
*
12+
* This program is distributed in the hope that it will be useful,
13+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15+
* GNU General Public License for more details.
16+
*
17+
* You should have received a copy of the GNU Lesser General Public License
18+
* along with this program. If not, see <https://www.gnu.org/licenses/>.
19+
*/
20+
21+
package com.demonwav.mcdev.platform.mixin.inspection.mixinextras
22+
23+
import com.demonwav.mcdev.platform.mixin.handlers.MixinAnnotationHandler
24+
import com.demonwav.mcdev.platform.mixin.handlers.mixinextras.WrapWithConditionHandler
25+
import com.demonwav.mcdev.platform.mixin.inspection.MixinInspection
26+
import com.demonwav.mcdev.platform.mixin.util.MethodTargetMember
27+
import com.demonwav.mcdev.platform.mixin.util.MixinConstants
28+
import com.demonwav.mcdev.platform.mixin.util.toPsiType
29+
import com.demonwav.mcdev.util.findContainingMethod
30+
import com.intellij.codeInspection.LocalQuickFix
31+
import com.intellij.codeInspection.LocalQuickFixOnPsiElement
32+
import com.intellij.codeInspection.ProblemsHolder
33+
import com.intellij.openapi.project.Project
34+
import com.intellij.psi.JavaElementVisitor
35+
import com.intellij.psi.JavaPsiFacade
36+
import com.intellij.psi.PsiAnnotation
37+
import com.intellij.psi.PsiBlockStatement
38+
import com.intellij.psi.PsiElement
39+
import com.intellij.psi.PsiElementFactory
40+
import com.intellij.psi.PsiExpression
41+
import com.intellij.psi.PsiFile
42+
import com.intellij.psi.PsiIfStatement
43+
import com.intellij.psi.PsiLiteralExpression
44+
import com.intellij.psi.PsiMethod
45+
import com.intellij.psi.codeStyle.CodeStyleManager
46+
import com.intellij.psi.codeStyle.JavaCodeStyleManager
47+
import com.intellij.psi.codeStyle.VariableKind
48+
import com.intellij.psi.util.PsiTypesUtil
49+
import com.intellij.psi.util.PsiUtil
50+
import com.siyeh.ig.psiutils.VariableNameGenerator
51+
import org.objectweb.asm.Type
52+
53+
class WrapWithConditionValidNonVoidInspection : MixinInspection() {
54+
override fun getStaticDescription() = "Reports when @WrapWithCondition targets a non-void instruction followed by a pop"
55+
56+
override fun buildVisitor(holder: ProblemsHolder) = object : JavaElementVisitor() {
57+
override fun visitAnnotation(annotation: PsiAnnotation) {
58+
val handler = MixinAnnotationHandler.forMixinAnnotation(annotation, holder.project) as? WrapWithConditionHandler ?: return
59+
60+
val hasMatchingInstructions = handler.resolveInstructions(annotation).any { insnResult ->
61+
val validTargetType = handler.getValidTargetType(insnResult.result.insn)
62+
validTargetType != null && validTargetType != Type.VOID_TYPE
63+
}
64+
65+
if (hasMatchingInstructions) {
66+
val fixes = mutableListOf<LocalQuickFix>()
67+
if (handler.getReplaceWithWrapOpInfo(annotation) != null) {
68+
fixes += ReplaceWithWrapOpFix(annotation)
69+
}
70+
71+
holder.registerProblem(
72+
annotation.nameReferenceElement ?: annotation,
73+
"@WrapWithCondition is targeting a non-void instruction",
74+
*fixes.toTypedArray()
75+
)
76+
}
77+
}
78+
}
79+
80+
companion object {
81+
private fun WrapWithConditionHandler.getReplaceWithWrapOpInfo(annotation: PsiAnnotation): ReplaceWithWrapOpInfo? {
82+
var targetType: Type? = null
83+
var requiredParameterCount: Int? = null
84+
85+
for (target in MixinAnnotationHandler.resolveTarget(annotation)) {
86+
if (target !is MethodTargetMember) {
87+
continue
88+
}
89+
90+
for (insn in resolveInstructions(annotation, target.classAndMethod.clazz, target.classAndMethod.method)) {
91+
val newTargetType = getValidTargetType(insn.insn) ?: continue
92+
if (targetType != null && targetType != newTargetType) {
93+
return null
94+
}
95+
targetType = newTargetType
96+
97+
val newRequiredParameterCount = getRequiredParameterCount(insn.insn, target.classAndMethod.clazz, annotation)
98+
if (requiredParameterCount != null && requiredParameterCount != newRequiredParameterCount) {
99+
return null
100+
}
101+
requiredParameterCount = newRequiredParameterCount
102+
}
103+
}
104+
105+
if (targetType == null || requiredParameterCount == null) {
106+
return null
107+
}
108+
109+
return ReplaceWithWrapOpInfo(targetType, requiredParameterCount)
110+
}
111+
}
112+
113+
private class ReplaceWithWrapOpInfo(val targetType: Type, val requiredParameterCount: Int)
114+
115+
private class ReplaceWithWrapOpFix(annotation: PsiAnnotation) : LocalQuickFixOnPsiElement(annotation) {
116+
override fun getFamilyName() = "Replace with @WrapOperation"
117+
override fun getText() = "Replace with @WrapOperation"
118+
119+
override fun invoke(project: Project, file: PsiFile, startElement: PsiElement, endElement: PsiElement) {
120+
val annotation = startElement as? PsiAnnotation ?: return
121+
val handler = MixinAnnotationHandler.forMixinAnnotation(annotation, project) as? WrapWithConditionHandler ?: return
122+
val info = handler.getReplaceWithWrapOpInfo(annotation) ?: return
123+
val factory = JavaPsiFacade.getElementFactory(project)
124+
125+
annotation.nameReferenceElement?.replace(factory.createReferenceElementByFQClassName(MixinConstants.MixinExtras.WRAP_OPERATION, annotation.resolveScope))
126+
127+
val method = annotation.findContainingMethod() ?: return
128+
val operationName = addOperationParameter(method, factory, info)
129+
replaceReturnType(method, factory, info)
130+
replaceReturnStatements(method, factory, info, operationName)
131+
132+
JavaCodeStyleManager.getInstance(project).shortenClassReferences(method)
133+
CodeStyleManager.getInstance(project).reformat(method)
134+
}
135+
136+
private fun addOperationParameter(method: PsiMethod, factory: PsiElementFactory, info: ReplaceWithWrapOpInfo): String {
137+
val operationName = VariableNameGenerator(method, VariableKind.PARAMETER)
138+
.byName("original")
139+
.generate(true)
140+
val parameterType = factory.createTypeFromText(
141+
"${MixinConstants.MixinExtras.OPERATION}<${
142+
PsiTypesUtil.boxIfPossible(info.targetType.toPsiType(
143+
factory,
144+
method
145+
).canonicalText)
146+
}>",
147+
method
148+
)
149+
val parameter = factory.createParameter(operationName, parameterType)
150+
method.parameterList.addBefore(
151+
parameter,
152+
method.parameterList.parameters.getOrNull(info.requiredParameterCount)
153+
)
154+
return operationName
155+
}
156+
157+
private fun replaceReturnType(method: PsiMethod, factory: PsiElementFactory, info: ReplaceWithWrapOpInfo) {
158+
method.returnTypeElement?.replace(factory.createTypeElement(info.targetType.toPsiType(factory, method)))
159+
}
160+
161+
private fun replaceReturnStatements(method: PsiMethod, factory: PsiElementFactory, info: ReplaceWithWrapOpInfo, operationName: String) {
162+
val returnType = info.targetType.toPsiType(factory, method)
163+
164+
for (returnStatement in PsiUtil.findReturnStatements(method)) {
165+
val returnValue = returnStatement.returnValue ?: continue
166+
if (returnValue is PsiLiteralExpression) {
167+
val literalValue = returnValue.value
168+
if (literalValue == true) {
169+
returnValue.replace(createOperationCall(method, factory, info, operationName))
170+
continue
171+
} else if (literalValue == false) {
172+
returnValue.replace(factory.createExpressionFromText(PsiTypesUtil.getDefaultValueOfType(returnType, true), method))
173+
continue
174+
}
175+
}
176+
177+
val ifStatement = factory.createStatementFromText(
178+
"if (cond) { return x; } else { return ${PsiTypesUtil.getDefaultValueOfType(returnType, true)}; }",
179+
method
180+
) as PsiIfStatement
181+
ifStatement.condition!!.replace(returnValue)
182+
PsiUtil.findReturnStatements((ifStatement.thenBranch as PsiBlockStatement).codeBlock)
183+
.first()
184+
.returnValue!!
185+
.replace(createOperationCall(method, factory, info, operationName))
186+
returnStatement.replace(ifStatement)
187+
}
188+
}
189+
190+
private fun createOperationCall(method: PsiMethod, factory: PsiElementFactory, info: ReplaceWithWrapOpInfo, operationName: String): PsiExpression {
191+
val expressionText = buildString {
192+
append(operationName)
193+
append(".call(")
194+
for ((i, param) in method.parameterList.parameters.take(info.requiredParameterCount).withIndex()) {
195+
if (i != 0) {
196+
append(", ")
197+
}
198+
append(param.name)
199+
}
200+
append(")")
201+
}
202+
return factory.createExpressionFromText(expressionText, method)
203+
}
204+
}
205+
}

‎src/main/resources/META-INF/plugin.xml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1502,6 +1502,14 @@
15021502
level="ERROR"
15031503
hasStaticDescription="true"
15041504
implementationClass="com.demonwav.mcdev.platform.mixin.inspection.mixinextras.ShareNonLocalRefInspection"/>
1505+
<localInspection displayName="@WrapWithCondition targets non-void instruction followed by pop"
1506+
shortName="WrapWithConditionTargetsNonVoid"
1507+
groupName="Mixin"
1508+
language="JAVA"
1509+
enabledByDefault="true"
1510+
level="WARNING"
1511+
hasStaticDescription="true"
1512+
implementationClass="com.demonwav.mcdev.platform.mixin.inspection.mixinextras.WrapWithConditionValidNonVoidInspection"/>
15051513
<!--endregion-->
15061514

15071515
<!--region Configuration -->

0 commit comments

Comments
 (0)