User: joe Date: 17 Jan 24 21:43 Revision: 8da4a4e36fcb2388f1872f8c9c07654fc247c868 Summary: Add inspection for unresolved @Local TeamCity URL: https://ci.mcdev.io/viewModification.html?tab=vcsModificationFiles&modId=9004&personal=false Index: src/main/kotlin/platform/mixin/handlers/ModifyVariableHandler.kt =================================================================== --- src/main/kotlin/platform/mixin/handlers/ModifyVariableHandler.kt (revision 29dce3223c7fd87da775bdda9e33c0fb119f7b3f) +++ src/main/kotlin/platform/mixin/handlers/ModifyVariableHandler.kt (revision 8da4a4e36fcb2388f1872f8c9c07654fc247c868) @@ -25,22 +25,14 @@ import com.demonwav.mcdev.platform.mixin.handlers.injectionPoint.InjectionPoint import com.demonwav.mcdev.platform.mixin.inspection.injector.MethodSignature import com.demonwav.mcdev.platform.mixin.inspection.injector.ParameterGroup -import com.demonwav.mcdev.platform.mixin.util.LocalVariables -import com.demonwav.mcdev.platform.mixin.util.hasAccess +import com.demonwav.mcdev.platform.mixin.util.LocalInfo import com.demonwav.mcdev.platform.mixin.util.toPsiType -import com.demonwav.mcdev.util.computeStringArray import com.demonwav.mcdev.util.constantStringValue -import com.demonwav.mcdev.util.constantValue -import com.demonwav.mcdev.util.descriptor import com.demonwav.mcdev.util.findContainingMethod import com.demonwav.mcdev.util.findModule -import com.intellij.openapi.module.Module import com.intellij.psi.JavaPsiFacade import com.intellij.psi.PsiAnnotation -import com.intellij.psi.PsiType -import org.objectweb.asm.Opcodes import org.objectweb.asm.Type -import org.objectweb.asm.tree.AbstractInsnNode import org.objectweb.asm.tree.ClassNode import org.objectweb.asm.tree.MethodNode @@ -64,8 +56,9 @@ isVarargs = true, ) - val info = ModifyVariableInfo.getModifyVariableInfo(annotation, CollectVisitor.Mode.COMPLETION) - ?: return null + val method = annotation.findContainingMethod() ?: return null + val localType = method.parameterList.getParameter(0)?.type + val info = LocalInfo.fromAnnotation(localType, annotation) val possibleTypes = mutableSetOf() for (insn in targets) { @@ -93,116 +86,3 @@ return result } } - -class ModifyVariableInfo( - val type: PsiType?, - val argsOnly: Boolean, - val index: Int?, - val ordinal: Int?, - val names: Set, -) { - fun getLocals( - module: Module, - targetClass: ClassNode, - methodNode: MethodNode, - insn: AbstractInsnNode, - ): Array? { - return if (argsOnly) { - val args = mutableListOf() - if (!methodNode.hasAccess(Opcodes.ACC_STATIC)) { - val thisDesc = Type.getObjectType(targetClass.name).descriptor - args.add(LocalVariables.LocalVariable("this", thisDesc, null, null, null, 0)) - } - for (argType in Type.getArgumentTypes(methodNode.desc)) { - args.add( - LocalVariables.LocalVariable("arg${args.size}", argType.descriptor, null, null, null, args.size), - ) - if (argType.size == 2) { - args.add(null) - } - } - args.toTypedArray() - } else { - LocalVariables.getLocals(module, targetClass, methodNode, insn) - } - } - - fun matchLocals( - locals: Array, - mode: CollectVisitor.Mode, - matchType: Boolean = true, - ): List { - val typeDesc = type?.descriptor - if (ordinal != null) { - val ordinals = mutableMapOf() - val result = mutableListOf() - for (local in locals) { - if (local == null) { - continue - } - val ordinal = ordinals[local.desc] ?: 0 - ordinals[local.desc!!] = ordinal + 1 - if (ordinal == this.ordinal && (!matchType || typeDesc == null || local.desc == typeDesc)) { - result += local - } - } - return result - } - - if (index != null) { - val local = locals.firstOrNull { it?.index == index } - if (local != null) { - if (!matchType || typeDesc == null || local.desc == typeDesc) { - return listOf(local) - } - } - return emptyList() - } - - if (names.isNotEmpty()) { - val result = mutableListOf() - for (local in locals) { - if (local == null) { - continue - } - if (names.contains(local.name)) { - if (!matchType || typeDesc == null || local.desc == typeDesc) { - result += local - } - } - } - return result - } - - // implicit mode - if (mode == CollectVisitor.Mode.COMPLETION) { - return locals.asSequence() - .filterNotNull() - .filter { local -> locals.count { it?.desc == local.desc } == 1 } - .toList() - } - - return if (matchType && typeDesc != null) { - locals.singleOrNull { it?.desc == typeDesc }?.let { listOf(it) } ?: emptyList() - } else { - emptyList() - } - } - - companion object { - fun getModifyVariableInfo(modifyVariable: PsiAnnotation, mode: CollectVisitor.Mode?): ModifyVariableInfo? { - val method = modifyVariable.findContainingMethod() ?: return null - val type = method.parameterList.getParameter(0)?.type - if (type == null && mode != CollectVisitor.Mode.COMPLETION) { - return null - } - val argsOnly = modifyVariable.findDeclaredAttributeValue("argsOnly")?.constantValue as? Boolean ?: false - val index = (modifyVariable.findDeclaredAttributeValue("index")?.constantValue as? Int) - ?.takeIf { it != -1 } - val ordinal = (modifyVariable.findDeclaredAttributeValue("ordinal")?.constantValue as? Int) - ?.takeIf { it != -1 } - val names = modifyVariable.findDeclaredAttributeValue("name")?.computeStringArray()?.toSet() ?: emptySet() - return ModifyVariableInfo(type, argsOnly, index, ordinal, names) - } - } -} Index: src/main/kotlin/platform/mixin/handlers/injectionPoint/LoadInjectionPoint.kt =================================================================== --- src/main/kotlin/platform/mixin/handlers/injectionPoint/LoadInjectionPoint.kt (revision 29dce3223c7fd87da775bdda9e33c0fb119f7b3f) +++ src/main/kotlin/platform/mixin/handlers/injectionPoint/LoadInjectionPoint.kt (revision 8da4a4e36fcb2388f1872f8c9c07654fc247c868) @@ -20,12 +20,13 @@ package com.demonwav.mcdev.platform.mixin.handlers.injectionPoint -import com.demonwav.mcdev.platform.mixin.handlers.ModifyVariableInfo import com.demonwav.mcdev.platform.mixin.reference.MixinSelector import com.demonwav.mcdev.platform.mixin.util.AsmDfaUtil +import com.demonwav.mcdev.platform.mixin.util.LocalInfo import com.demonwav.mcdev.platform.mixin.util.LocalVariables import com.demonwav.mcdev.platform.mixin.util.MixinConstants.Annotations.MODIFY_VARIABLE import com.demonwav.mcdev.util.constantValue +import com.demonwav.mcdev.util.findContainingMethod import com.demonwav.mcdev.util.findModule import com.demonwav.mcdev.util.isErasureEquivalentTo import com.intellij.codeInsight.lookup.LookupElementBuilder @@ -53,13 +54,19 @@ import org.objectweb.asm.tree.VarInsnNode abstract class AbstractLoadInjectionPoint(private val store: Boolean) : InjectionPoint() { - private fun getModifyVariableInfo(at: PsiAnnotation, mode: CollectVisitor.Mode?): ModifyVariableInfo? { + private fun getModifyVariableInfo(at: PsiAnnotation, mode: CollectVisitor.Mode?): LocalInfo? { val modifyVariable = at.parentOfType() ?: return null if (!modifyVariable.hasQualifiedName(MODIFY_VARIABLE)) { return null } - return ModifyVariableInfo.getModifyVariableInfo(modifyVariable, mode) + + val method = modifyVariable.findContainingMethod() ?: return null + val localType = method.parameterList.getParameter(0)?.type + if (localType == null && mode != CollectVisitor.Mode.COMPLETION) { + return null - } + } + return LocalInfo.fromAnnotation(localType, modifyVariable) + } override fun createNavigationVisitor( at: PsiAnnotation, @@ -115,7 +122,7 @@ } private class MyNavigationVisitor( - private val info: ModifyVariableInfo, + private val info: LocalInfo, private val store: Boolean, ) : NavigationVisitor() { override fun visitThisExpression(expression: PsiThisExpression) { @@ -276,7 +283,7 @@ private val module: Module, private val targetClass: ClassNode, mode: Mode, - private val info: ModifyVariableInfo, + private val info: LocalInfo, private val store: Boolean, ) : CollectVisitor(mode) { override fun accept(methodNode: MethodNode) { Index: src/main/kotlin/platform/mixin/inspection/mixinextras/UnnecessaryMutableLocalInspection.kt =================================================================== --- src/main/kotlin/platform/mixin/inspection/mixinextras/UnnecessaryMutableLocalInspection.kt (revision 29dce3223c7fd87da775bdda9e33c0fb119f7b3f) +++ src/main/kotlin/platform/mixin/inspection/mixinextras/UnnecessaryMutableLocalInspection.kt (revision 8da4a4e36fcb2388f1872f8c9c07654fc247c868) @@ -25,13 +25,13 @@ import com.demonwav.mcdev.platform.mixin.handlers.mixinextras.WrapOperationHandler import com.demonwav.mcdev.platform.mixin.inspection.MixinInspection import com.demonwav.mcdev.platform.mixin.util.MixinConstants +import com.demonwav.mcdev.platform.mixin.util.MixinConstants.MixinExtras.unwrapLocalRef import com.demonwav.mcdev.util.findContainingMethod import com.intellij.codeInspection.LocalQuickFixOnPsiElement import com.intellij.codeInspection.ProblemsHolder import com.intellij.openapi.project.Project import com.intellij.psi.JavaElementVisitor import com.intellij.psi.PsiClass -import com.intellij.psi.PsiClassType import com.intellij.psi.PsiElement import com.intellij.psi.PsiElementFactory import com.intellij.psi.PsiExpressionList @@ -40,8 +40,6 @@ import com.intellij.psi.PsiMethodCallExpression import com.intellij.psi.PsiParameter import com.intellij.psi.PsiReferenceExpression -import com.intellij.psi.PsiType -import com.intellij.psi.PsiTypes import com.intellij.psi.search.searches.OverridingMethodsSearch import com.intellij.psi.search.searches.ReferencesSearch import com.intellij.psi.util.PsiUtil @@ -169,8 +167,7 @@ private fun fixMethod(method: PsiMethod, paramIndex: Int) { val param = method.parameterList.getParameter(paramIndex) ?: return - val paramType = param.type as? PsiClassType ?: return - val innerType = paramType.innerRefType ?: return + val innerType = param.type.unwrapLocalRef() val factory = PsiElementFactory.getInstance(method.project) param.typeElement?.replace(factory.createTypeElement(innerType)) for (ref in ReferencesSearch.search(param)) { @@ -180,19 +177,5 @@ call.replace(ref.element) } } - - private val PsiClassType.innerRefType: PsiType? - get() = - when (resolve()?.qualifiedName?.substringAfterLast('.')) { - "LocalBooleanRef" -> PsiTypes.booleanType() - "LocalCharRef" -> PsiTypes.charType() - "LocalDoubleRef" -> PsiTypes.doubleType() - "LocalFloatRef" -> PsiTypes.floatType() - "LocalIntRef" -> PsiTypes.intType() - "LocalLongRef" -> PsiTypes.longType() - "LocalShortRef" -> PsiTypes.shortType() - "LocalRef" -> parameters.getOrNull(0) - else -> null - } - } + } +} -} Index: src/main/kotlin/platform/mixin/inspection/mixinextras/UnresolvedLocalCaptureInspection.kt =================================================================== --- src/main/kotlin/platform/mixin/inspection/mixinextras/UnresolvedLocalCaptureInspection.kt (revision 8da4a4e36fcb2388f1872f8c9c07654fc247c868) +++ src/main/kotlin/platform/mixin/inspection/mixinextras/UnresolvedLocalCaptureInspection.kt (revision 8da4a4e36fcb2388f1872f8c9c07654fc247c868) @@ -0,0 +1,76 @@ +/* + * Minecraft Development for IntelliJ + * + * https://mcdev.io/ + * + * Copyright (C) 2024 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 . + */ + +package com.demonwav.mcdev.platform.mixin.inspection.mixinextras + +import com.demonwav.mcdev.platform.mixin.handlers.InjectorAnnotationHandler +import com.demonwav.mcdev.platform.mixin.handlers.MixinAnnotationHandler +import com.demonwav.mcdev.platform.mixin.handlers.injectionPoint.CollectVisitor +import com.demonwav.mcdev.platform.mixin.inspection.MixinInspection +import com.demonwav.mcdev.platform.mixin.util.LocalInfo +import com.demonwav.mcdev.platform.mixin.util.MixinConstants +import com.demonwav.mcdev.platform.mixin.util.MixinConstants.MixinExtras.unwrapLocalRef +import com.demonwav.mcdev.util.findContainingMethod +import com.demonwav.mcdev.util.findModule +import com.demonwav.mcdev.util.mapFirstNotNull +import com.intellij.codeInspection.ProblemsHolder +import com.intellij.psi.JavaElementVisitor +import com.intellij.psi.PsiAnnotation +import com.intellij.psi.PsiElementVisitor +import com.intellij.psi.PsiParameter +import com.intellij.psi.util.parentOfType + +class UnresolvedLocalCaptureInspection : MixinInspection() { + override fun getStaticDescription() = "Verifies targets of @Local annotations" + + override fun buildVisitor(holder: ProblemsHolder): PsiElementVisitor = object : JavaElementVisitor() { + override fun visitAnnotation(localAnnotation: PsiAnnotation) { + if (localAnnotation.qualifiedName != MixinConstants.MixinExtras.LOCAL) { + return + } + + val parameter = localAnnotation.parentOfType() ?: return + val method = parameter.findContainingMethod() ?: return + val targets = method.annotations.mapFirstNotNull { annotation -> + val qName = annotation.qualifiedName ?: return@mapFirstNotNull null + val handler = + MixinAnnotationHandler.forMixinAnnotation(qName, holder.project) as? InjectorAnnotationHandler + ?: return@mapFirstNotNull null + handler.resolveInstructions(annotation) + } ?: return + val module = method.findModule() ?: return + + val localInfo = LocalInfo.fromAnnotation(parameter.type.unwrapLocalRef(), localAnnotation) + + for (target in targets) { + val locals = localInfo.getLocals(module, target.method.clazz, target.method.method, target.result.insn) + ?: continue + val matchingLocals = localInfo.matchLocals(locals, CollectVisitor.Mode.MATCH_ALL) + if (matchingLocals.size != 1) { + holder.registerProblem( + localAnnotation.nameReferenceElement ?: localAnnotation, + "@Local does not match any or matched multiple local variables in the target method" + ) + return + } + } + } + } +} Index: src/main/kotlin/platform/mixin/util/LocalInfo.kt =================================================================== --- src/main/kotlin/platform/mixin/util/LocalInfo.kt (revision 8da4a4e36fcb2388f1872f8c9c07654fc247c868) +++ src/main/kotlin/platform/mixin/util/LocalInfo.kt (revision 8da4a4e36fcb2388f1872f8c9c07654fc247c868) @@ -0,0 +1,151 @@ +/* + * Minecraft Development for IntelliJ + * + * https://mcdev.io/ + * + * Copyright (C) 2024 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 . + */ + +package com.demonwav.mcdev.platform.mixin.util + +import com.demonwav.mcdev.platform.mixin.handlers.injectionPoint.CollectVisitor +import com.demonwav.mcdev.util.computeStringArray +import com.demonwav.mcdev.util.constantValue +import com.demonwav.mcdev.util.descriptor +import com.intellij.openapi.module.Module +import com.intellij.psi.PsiAnnotation +import com.intellij.psi.PsiType +import org.objectweb.asm.Opcodes +import org.objectweb.asm.Type +import org.objectweb.asm.tree.AbstractInsnNode +import org.objectweb.asm.tree.ClassNode +import org.objectweb.asm.tree.MethodNode + +class LocalInfo( + val type: PsiType?, + val argsOnly: Boolean, + val index: Int?, + val ordinal: Int?, + val names: Set, +) { + fun getLocals( + module: Module, + targetClass: ClassNode, + methodNode: MethodNode, + insn: AbstractInsnNode, + ): Array? { + return if (argsOnly) { + val args = mutableListOf() + if (!methodNode.hasAccess(Opcodes.ACC_STATIC)) { + val thisDesc = Type.getObjectType(targetClass.name).descriptor + args.add(LocalVariables.LocalVariable("this", thisDesc, null, null, null, 0)) + } + for (argType in Type.getArgumentTypes(methodNode.desc)) { + args.add( + LocalVariables.LocalVariable("arg${args.size}", argType.descriptor, null, null, null, args.size), + ) + if (argType.size == 2) { + args.add(null) + } + } + args.toTypedArray() + } else { + LocalVariables.getLocals(module, targetClass, methodNode, insn) + } + } + + fun matchLocals( + locals: Array, + mode: CollectVisitor.Mode, + matchType: Boolean = true, + ): List { + val typeDesc = type?.descriptor + if (ordinal != null) { + val ordinals = mutableMapOf() + val result = mutableListOf() + for (local in locals) { + if (local == null) { + continue + } + val ordinal = ordinals[local.desc] ?: 0 + ordinals[local.desc!!] = ordinal + 1 + if (ordinal == this.ordinal && (!matchType || typeDesc == null || local.desc == typeDesc)) { + result += local + } + } + return result + } + + if (index != null) { + val local = locals.firstOrNull { it?.index == index } + if (local != null) { + if (!matchType || typeDesc == null || local.desc == typeDesc) { + return listOf(local) + } + } + return emptyList() + } + + if (names.isNotEmpty()) { + val result = mutableListOf() + for (local in locals) { + if (local == null) { + continue + } + if (names.contains(local.name)) { + if (!matchType || typeDesc == null || local.desc == typeDesc) { + result += local + } + } + } + return result + } + + // implicit mode + if (mode == CollectVisitor.Mode.COMPLETION) { + return locals.asSequence() + .filterNotNull() + .filter { local -> locals.count { it?.desc == local.desc } == 1 } + .toList() + } + + return if (matchType && typeDesc != null) { + locals.singleOrNull { it?.desc == typeDesc }?.let { listOf(it) } ?: emptyList() + } else { + emptyList() + } + } + + companion object { + /** + * Gets a [LocalInfo] from an annotation which declares the following attributes: + * - `argsOnly` to only match the target method arguments + * - `index` to match local variables by index + * - `ordinal` to match local variables by type then index + * - `name` to match local variables by name + * + * The `ModifyVariable` and `Local` annotations match this description. + */ + fun fromAnnotation(localType: PsiType?, annotation: PsiAnnotation): LocalInfo { + val argsOnly = annotation.findDeclaredAttributeValue("argsOnly")?.constantValue as? Boolean ?: false + val index = (annotation.findDeclaredAttributeValue("index")?.constantValue as? Int) + ?.takeIf { it != -1 } + val ordinal = (annotation.findDeclaredAttributeValue("ordinal")?.constantValue as? Int) + ?.takeIf { it != -1 } + val names = annotation.findDeclaredAttributeValue("name")?.computeStringArray()?.toSet() ?: emptySet() + return LocalInfo(localType, argsOnly, index, ordinal, names) + } + } +} Index: src/main/kotlin/platform/mixin/util/MixinConstants.kt =================================================================== --- src/main/kotlin/platform/mixin/util/MixinConstants.kt (revision 29dce3223c7fd87da775bdda9e33c0fb119f7b3f) +++ src/main/kotlin/platform/mixin/util/MixinConstants.kt (revision 8da4a4e36fcb2388f1872f8c9c07654fc247c868) @@ -20,6 +20,10 @@ package com.demonwav.mcdev.platform.mixin.util +import com.intellij.psi.PsiClassType +import com.intellij.psi.PsiType +import com.intellij.psi.PsiTypes + @Suppress("MemberVisibilityCanBePrivate") object MixinConstants { const val PACKAGE = "org.spongepowered.asm.mixin." @@ -81,5 +85,26 @@ const val WRAP_OPERATION = "com.llamalad7.mixinextras.injector.wrapoperation.WrapOperation" const val LOCAL = "com.llamalad7.mixinextras.sugar.Local" const val LOCAL_REF_PACKAGE = "com.llamalad7.mixinextras.sugar.ref." + + fun PsiType.unwrapLocalRef(): PsiType { + if (this !is PsiClassType) { + return this - } + } + val qName = resolve()?.qualifiedName ?: return this + if (!qName.startsWith(LOCAL_REF_PACKAGE)) { + return this -} + } + return when (qName.substringAfterLast('.')) { + "LocalBooleanRef" -> PsiTypes.booleanType() + "LocalCharRef" -> PsiTypes.charType() + "LocalDoubleRef" -> PsiTypes.doubleType() + "LocalFloatRef" -> PsiTypes.floatType() + "LocalIntRef" -> PsiTypes.intType() + "LocalLongRef" -> PsiTypes.longType() + "LocalShortRef" -> PsiTypes.shortType() + "LocalRef" -> parameters.getOrNull(0) ?: this + else -> this + } + } + } +} Index: src/main/resources/META-INF/plugin.xml =================================================================== --- src/main/resources/META-INF/plugin.xml (revision 29dce3223c7fd87da775bdda9e33c0fb119f7b3f) +++ src/main/resources/META-INF/plugin.xml (revision 8da4a4e36fcb2388f1872f8c9c07654fc247c868) @@ -1003,6 +1003,14 @@ level="WARNING" hasStaticDescription="true" implementationClass="com.demonwav.mcdev.platform.mixin.inspection.mixinextras.UnnecessaryMutableLocalInspection"/> +