User: joe Date: 09 Aug 22 17:05 Revision: e7b5e59cdd099776ada6953acf209ba599a483d9 Summary: Fix implicit modify constant inspection TeamCity URL: https://ci.denwav.dev/viewModification.html?tab=vcsModificationFiles&modId=8051&personal=false Index: src/main/kotlin/platform/mixin/handlers/ModifyConstantHandler.kt =================================================================== --- src/main/kotlin/platform/mixin/handlers/ModifyConstantHandler.kt (revision 20b77530cb09cf51d8fb0ea6a90252e301428c6d) +++ src/main/kotlin/platform/mixin/handlers/ModifyConstantHandler.kt (revision e7b5e59cdd099776ada6953acf209ba599a483d9) @@ -173,13 +173,20 @@ canDecompile = true ) ?: return emptyList() + val constantInjectionPoint = InjectionPoint.byAtCode("CONSTANT") as? ConstantInjectionPoint + ?: return emptyList() + return constantInfos.asSequence().flatMap { modifyConstantInfo -> val collectVisitor = ConstantInjectionPoint.MyCollectVisitor( annotation.project, CollectVisitor.Mode.MATCH_ALL, modifyConstantInfo.constantInfo ) - InjectionPoint.addStandardFilters(modifyConstantInfo.constantAnnotation, targetClass, collectVisitor) + constantInjectionPoint.addStandardFilters( + modifyConstantInfo.constantAnnotation, + targetClass, + collectVisitor + ) collectVisitor.visit(targetMethod) val bytecodeResults = collectVisitor.result @@ -199,13 +206,19 @@ mode: CollectVisitor.Mode ): List> { val constantInfos = getConstantInfos(annotation) ?: return emptyList() + val constantInjectionPoint = InjectionPoint.byAtCode("CONSTANT") as? ConstantInjectionPoint + ?: return emptyList() return constantInfos.asSequence().flatMap { modifyConstantInfo -> val collectVisitor = ConstantInjectionPoint.MyCollectVisitor( annotation.project, mode, modifyConstantInfo.constantInfo ) - InjectionPoint.addStandardFilters(modifyConstantInfo.constantAnnotation, targetClass, collectVisitor) + constantInjectionPoint.addStandardFilters( + modifyConstantInfo.constantAnnotation, + targetClass, + collectVisitor + ) collectVisitor.visit(targetMethod) collectVisitor.result.asSequence() }.sortedBy { targetMethod.instructions.indexOf(it.insn) }.toList() @@ -217,13 +230,19 @@ targetMethod: MethodNode ): InsnResolutionInfo.Failure? { val constantInfos = getConstantInfos(annotation) ?: return InsnResolutionInfo.Failure() + val constantInjectionPoint = InjectionPoint.byAtCode("CONSTANT") as? ConstantInjectionPoint + ?: return null return constantInfos.asSequence().mapNotNull { modifyConstantInfo -> val collectVisitor = ConstantInjectionPoint.MyCollectVisitor( annotation.project, CollectVisitor.Mode.MATCH_FIRST, modifyConstantInfo.constantInfo ) - InjectionPoint.addStandardFilters(modifyConstantInfo.constantAnnotation, targetClass, collectVisitor) + constantInjectionPoint.addStandardFilters( + modifyConstantInfo.constantAnnotation, + targetClass, + collectVisitor + ) collectVisitor.visit(targetMethod) if (collectVisitor.result.isEmpty()) { collectVisitor.filterToBlame Index: src/main/kotlin/platform/mixin/handlers/ModifyVariableHandler.kt =================================================================== --- src/main/kotlin/platform/mixin/handlers/ModifyVariableHandler.kt (revision 20b77530cb09cf51d8fb0ea6a90252e301428c6d) +++ src/main/kotlin/platform/mixin/handlers/ModifyVariableHandler.kt (revision e7b5e59cdd099776ada6953acf209ba599a483d9) @@ -132,7 +132,7 @@ } val ordinal = ordinals[local.desc] ?: 0 ordinals[local.desc!!] = ordinal + 1 - if (ordinal == ordinal && (!matchType || typeDesc == null || local.desc == typeDesc)) { + if (ordinal == this.ordinal && (!matchType || typeDesc == null || local.desc == typeDesc)) { result += local } } Index: src/main/kotlin/platform/mixin/handlers/injectionPoint/InjectionPoint.kt =================================================================== --- src/main/kotlin/platform/mixin/handlers/injectionPoint/InjectionPoint.kt (revision 20b77530cb09cf51d8fb0ea6a90252e301428c6d) +++ src/main/kotlin/platform/mixin/handlers/injectionPoint/InjectionPoint.kt (revision e7b5e59cdd099776ada6953acf209ba599a483d9) @@ -58,132 +58,132 @@ fun byAtCode(atCode: String): InjectionPoint<*>? { return COLLECTOR.findSingle(atCode) } + } + open fun usesMemberReference() = false + + abstract fun createNavigationVisitor( + at: PsiAnnotation, + target: MixinSelector?, + targetClass: PsiClass + ): NavigationVisitor? + + abstract fun doCreateCollectVisitor( + at: PsiAnnotation, + target: MixinSelector?, + targetClass: ClassNode, + mode: CollectVisitor.Mode + ): CollectVisitor? + + fun createCollectVisitor( + at: PsiAnnotation, + target: MixinSelector?, + targetClass: ClassNode, + mode: CollectVisitor.Mode + ): CollectVisitor? { + return doCreateCollectVisitor(at, target, targetClass, mode)?.also { + addFilters(at, targetClass, it) + } + } + + protected open fun addFilters(at: PsiAnnotation, targetClass: ClassNode, collectVisitor: CollectVisitor) { + addStandardFilters(at, targetClass, collectVisitor) + } + - fun addStandardFilters(at: PsiAnnotation, targetClass: ClassNode, collectVisitor: CollectVisitor<*>) { + fun addStandardFilters(at: PsiAnnotation, targetClass: ClassNode, collectVisitor: CollectVisitor<*>) { - addShiftSupport(at, collectVisitor) + addShiftSupport(at, targetClass, collectVisitor) - addSliceFilter(at, targetClass, collectVisitor) - // make sure the ordinal filter is last, so that the ordinal only increments once the other filters have passed + addSliceFilter(at, targetClass, collectVisitor) + // make sure the ordinal filter is last, so that the ordinal only increments once the other filters have passed - addOrdinalFilter(at, collectVisitor) + addOrdinalFilter(at, targetClass, collectVisitor) - } + } - private fun addShiftSupport(at: PsiAnnotation, collectVisitor: CollectVisitor<*>) { + protected open fun addShiftSupport(at: PsiAnnotation, targetClass: ClassNode, collectVisitor: CollectVisitor<*>) { - val shiftAttr = at.findDeclaredAttributeValue("shift") as? PsiExpression ?: return - val shiftReference = PsiUtil.skipParenthesizedExprDown(shiftAttr) as? PsiReferenceExpression ?: return - val shift = shiftReference.resolve() as? PsiEnumConstant ?: return - val containingClass = shift.containingClass ?: return - val shiftClass = JavaPsiFacade.getInstance(at.project).findClass(SHIFT, at.resolveScope) ?: return - if (!(containingClass equivalentTo shiftClass)) return - when (shift.name) { - "BEFORE" -> collectVisitor.shiftBy = -1 - "AFTER" -> collectVisitor.shiftBy = 1 - "BY" -> { - val by = at.findDeclaredAttributeValue("by")?.constantValue as? Int ?: return - collectVisitor.shiftBy = by - } - } - } + val shiftAttr = at.findDeclaredAttributeValue("shift") as? PsiExpression ?: return + val shiftReference = PsiUtil.skipParenthesizedExprDown(shiftAttr) as? PsiReferenceExpression ?: return + val shift = shiftReference.resolve() as? PsiEnumConstant ?: return + val containingClass = shift.containingClass ?: return + val shiftClass = JavaPsiFacade.getInstance(at.project).findClass(SHIFT, at.resolveScope) ?: return + if (!(containingClass equivalentTo shiftClass)) return + when (shift.name) { + "BEFORE" -> collectVisitor.shiftBy = -1 + "AFTER" -> collectVisitor.shiftBy = 1 + "BY" -> { + val by = at.findDeclaredAttributeValue("by")?.constantValue as? Int ?: return + collectVisitor.shiftBy = by + } + } + } - private fun addSliceFilter(at: PsiAnnotation, targetClass: ClassNode, collectVisitor: CollectVisitor<*>) { + protected open fun addSliceFilter(at: PsiAnnotation, targetClass: ClassNode, collectVisitor: CollectVisitor<*>) { - // resolve slice annotation, take into account slice id if present - val sliceId = at.findDeclaredAttributeValue("slice")?.constantStringValue - val parentAnnotation = at.parentOfType() ?: return - val slices = parentAnnotation.findDeclaredAttributeValue("slice")?.findAnnotations() ?: return - val slice = if (sliceId != null) { - slices.singleOrNull { aSlice -> - val aSliceId = aSlice.findDeclaredAttributeValue("id")?.constantStringValue - ?: return@singleOrNull false - aSliceId == sliceId - } - } else { - slices.singleOrNull() - } ?: return + // resolve slice annotation, take into account slice id if present + val sliceId = at.findDeclaredAttributeValue("slice")?.constantStringValue + val parentAnnotation = at.parentOfType() ?: return + val slices = parentAnnotation.findDeclaredAttributeValue("slice")?.findAnnotations() ?: return + val slice = if (sliceId != null) { + slices.singleOrNull { aSlice -> + val aSliceId = aSlice.findDeclaredAttributeValue("id")?.constantStringValue + ?: return@singleOrNull false + aSliceId == sliceId + } + } else { + slices.singleOrNull() + } ?: return - // precompute what we can - val from = slice.findDeclaredAttributeValue("from") as? PsiAnnotation - val to = slice.findDeclaredAttributeValue("to") as? PsiAnnotation - if (from == null && to == null) { - return - } - val fromSelector = from?.findDeclaredAttributeValue("value")?.constantStringValue?.let { atCode -> - SliceSelector.values().firstOrNull { atCode.endsWith(":${it.name}") } - } ?: SliceSelector.FIRST - val toSelector = to?.findDeclaredAttributeValue("value")?.constantStringValue?.let { atCode -> - SliceSelector.values().firstOrNull { atCode.endsWith(":${it.name}") } - } ?: SliceSelector.FIRST + // precompute what we can + val from = slice.findDeclaredAttributeValue("from") as? PsiAnnotation + val to = slice.findDeclaredAttributeValue("to") as? PsiAnnotation + if (from == null && to == null) { + return + } + val fromSelector = from?.findDeclaredAttributeValue("value")?.constantStringValue?.let { atCode -> + SliceSelector.values().firstOrNull { atCode.endsWith(":${it.name}") } + } ?: SliceSelector.FIRST + val toSelector = to?.findDeclaredAttributeValue("value")?.constantStringValue?.let { atCode -> + SliceSelector.values().firstOrNull { atCode.endsWith(":${it.name}") } + } ?: SliceSelector.FIRST - fun resolveSliceIndex( - sliceAt: PsiAnnotation?, - selector: SliceSelector, - insns: InsnList, - method: MethodNode - ): Int? { - return sliceAt?.let { - val results = AtResolver(sliceAt, targetClass, method).resolveInstructions() - val insn = if (selector == SliceSelector.LAST) { - results.lastOrNull()?.insn - } else { - results.firstOrNull()?.insn - } - insn?.let { insns.indexOf(it) } - } - } + fun resolveSliceIndex( + sliceAt: PsiAnnotation?, + selector: SliceSelector, + insns: InsnList, + method: MethodNode + ): Int? { + return sliceAt?.let { + val results = AtResolver(sliceAt, targetClass, method).resolveInstructions() + val insn = if (selector == SliceSelector.LAST) { + results.lastOrNull()?.insn + } else { + results.firstOrNull()?.insn + } + insn?.let { insns.indexOf(it) } + } + } - // allocate lazy indexes so we don't have to re-run the at resolver for the slices each time - var fromInsnIndex: Int? = null - var toInsnIndex: Int? = null + // allocate lazy indexes so we don't have to re-run the at resolver for the slices each time + var fromInsnIndex: Int? = null + var toInsnIndex: Int? = null - collectVisitor.addResultFilter("slice") { result, method -> - val insns = method.instructions ?: return@addResultFilter true - if (fromInsnIndex == null) { - fromInsnIndex = resolveSliceIndex(from, fromSelector, insns, method) ?: 0 - } - if (toInsnIndex == null) { - toInsnIndex = resolveSliceIndex(to, toSelector, insns, method) ?: insns.size() - } + collectVisitor.addResultFilter("slice") { result, method -> + val insns = method.instructions ?: return@addResultFilter true + if (fromInsnIndex == null) { + fromInsnIndex = resolveSliceIndex(from, fromSelector, insns, method) ?: 0 + } + if (toInsnIndex == null) { + toInsnIndex = resolveSliceIndex(to, toSelector, insns, method) ?: insns.size() + } - insns.indexOf(result.insn) in fromInsnIndex!!..toInsnIndex!! - } - } + insns.indexOf(result.insn) in fromInsnIndex!!..toInsnIndex!! + } + } - private fun addOrdinalFilter(at: PsiAnnotation, collectVisitor: CollectVisitor<*>) { + protected open fun addOrdinalFilter(at: PsiAnnotation, targetClass: ClassNode, collectVisitor: CollectVisitor<*>) { - val ordinal = at.findDeclaredAttributeValue("ordinal")?.constantValue as? Int ?: return - if (ordinal < 0) return - collectVisitor.addResultFilter("ordinal") { _, _ -> - collectVisitor.ordinal++ == ordinal - } - } + val ordinal = at.findDeclaredAttributeValue("ordinal")?.constantValue as? Int ?: return + if (ordinal < 0) return + collectVisitor.addResultFilter("ordinal") { _, _ -> + collectVisitor.ordinal++ == ordinal + } + } - } - open fun usesMemberReference() = false - - abstract fun createNavigationVisitor( - at: PsiAnnotation, - target: MixinSelector?, - targetClass: PsiClass - ): NavigationVisitor? - - abstract fun doCreateCollectVisitor( - at: PsiAnnotation, - target: MixinSelector?, - targetClass: ClassNode, - mode: CollectVisitor.Mode - ): CollectVisitor? - - fun createCollectVisitor( - at: PsiAnnotation, - target: MixinSelector?, - targetClass: ClassNode, - mode: CollectVisitor.Mode - ): CollectVisitor? { - return doCreateCollectVisitor(at, target, targetClass, mode)?.also { - addFilters(at, targetClass, it) - } - } - - protected open fun addFilters(at: PsiAnnotation, targetClass: ClassNode, collectVisitor: CollectVisitor) { - addStandardFilters(at, targetClass, collectVisitor) - } - abstract fun createLookup(targetClass: ClassNode, result: CollectVisitor.Result): LookupElementBuilder? protected fun LookupElementBuilder.setBoldIfInClass(member: PsiMember, clazz: ClassNode): LookupElementBuilder { @@ -371,7 +371,7 @@ } } - val result = Result(nextIndex++, shiftedInsn ?: return, element, qualifier) + val result = Result(nextIndex++, insn, shiftedInsn ?: return, element, qualifier) var isFiltered = false for ((name, filter) in resultFilters) { if (!filter(result, method)) { @@ -402,6 +402,7 @@ data class Result( val index: Int, + val originalInsn: AbstractInsnNode, val insn: AbstractInsnNode, val target: T, val qualifier: String? = null Index: src/main/kotlin/platform/mixin/handlers/injectionPoint/LoadInjectionPoint.kt =================================================================== --- src/main/kotlin/platform/mixin/handlers/injectionPoint/LoadInjectionPoint.kt (revision 20b77530cb09cf51d8fb0ea6a90252e301428c6d) +++ src/main/kotlin/platform/mixin/handlers/injectionPoint/LoadInjectionPoint.kt (revision e7b5e59cdd099776ada6953acf209ba599a483d9) @@ -12,6 +12,7 @@ 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.LocalVariables import com.demonwav.mcdev.platform.mixin.util.MixinConstants.Annotations.MODIFY_VARIABLE import com.demonwav.mcdev.util.constantValue @@ -77,6 +78,32 @@ return null } + override fun addOrdinalFilter(at: PsiAnnotation, targetClass: ClassNode, collectVisitor: CollectVisitor<*>) { + val ordinal = at.findDeclaredAttributeValue("ordinal")?.constantValue as? Int ?: return + if (ordinal < 0) return + + // Replace the ordinal filter with one that takes into account the type of the local variable being modified. + // Fixes otherwise incorrect results for completion. + val project = at.project + val ordinals = mutableMapOf() + collectVisitor.addResultFilter("ordinal") { result, method -> + result.originalInsn as? VarInsnNode + ?: throw IllegalStateException("AbstractLoadInjectionPoint returned non-var insn") + val localInsn = if (store) { result.originalInsn.next } else { result.originalInsn } + val localType = AsmDfaUtil.getLocalVariableType( + project, + targetClass, + method, + localInsn, + result.originalInsn.`var` + ) ?: return@addResultFilter true + val desc = localType.descriptor + val ord = ordinals[desc] ?: 0 + ordinals[desc] = ord + 1 + ord == ordinal + } + } + private class MyNavigationVisitor( private val info: ModifyVariableInfo, private val store: Boolean