User: joe Date: 17 Jan 24 22:54 Revision: 88db0318dcf70f7ee98a0759bebb1bb8b328664b Summary: Add inspection for when @Local may be argsOnly = true TeamCity URL: https://ci.mcdev.io/viewModification.html?tab=vcsModificationFiles&modId=9006&personal=false Index: src/main/kotlin/platform/mixin/inspection/injector/ModifyVariableArgsOnlyInspection.kt =================================================================== --- src/main/kotlin/platform/mixin/inspection/injector/ModifyVariableArgsOnlyInspection.kt (revision 8da4a4e36fcb2388f1872f8c9c07654fc247c868) +++ src/main/kotlin/platform/mixin/inspection/injector/ModifyVariableArgsOnlyInspection.kt (revision 88db0318dcf70f7ee98a0759bebb1bb8b328664b) @@ -22,6 +22,7 @@ import com.demonwav.mcdev.platform.mixin.handlers.MixinAnnotationHandler import com.demonwav.mcdev.platform.mixin.inspection.MixinInspection +import com.demonwav.mcdev.platform.mixin.util.ClassAndMethodNode import com.demonwav.mcdev.platform.mixin.util.MethodTargetMember import com.demonwav.mcdev.platform.mixin.util.MixinConstants.Annotations.MODIFY_VARIABLE import com.demonwav.mcdev.platform.mixin.util.hasAccess @@ -40,25 +41,19 @@ import com.intellij.psi.PsiElementVisitor import com.intellij.psi.PsiFile import com.intellij.psi.PsiMethod +import com.intellij.psi.PsiType import org.objectweb.asm.Opcodes import org.objectweb.asm.Type class ModifyVariableArgsOnlyInspection : MixinInspection() { + override fun getStaticDescription() = + "Checks that @ModifyVariable has argsOnly if it targets arguments, which improves performance of the mixin" + override fun buildVisitor(holder: ProblemsHolder): PsiElementVisitor { return object : JavaElementVisitor() { override fun visitMethod(method: PsiMethod) { val modifyVariable = method.findAnnotation(MODIFY_VARIABLE) ?: return - if (modifyVariable.findDeclaredAttributeValue("argsOnly")?.constantValue == true) { - return - } - val ordinal = (modifyVariable.findDeclaredAttributeValue("ordinal")?.constantValue as? Int?) - ?.takeIf { it != -1 } - val index = (modifyVariable.findDeclaredAttributeValue("index")?.constantValue as? Int?) - ?.takeIf { it != -1 } - if (ordinal == null && index == null && modifyVariable.findDeclaredAttributeValue("name") != null) { - return - } - val wantedType = method.parameterList.getParameter(0)?.type?.descriptor ?: return + val wantedType = method.parameterList.getParameter(0)?.type ?: return val problemElement = modifyVariable.nameReferenceElement ?: return val handler = MixinAnnotationHandler.forMixinAnnotation(MODIFY_VARIABLE) ?: return @@ -66,50 +61,74 @@ val methodTargets = targets.asSequence() .filterIsInstance() .map { it.classAndMethod } + + if (shouldReport(modifyVariable, wantedType, methodTargets)) { + val description = "@ModifyVariable may be argsOnly = true" + holder.registerProblem(problemElement, description, AddArgsOnlyFix(modifyVariable)) + } + } + } + } + + class AddArgsOnlyFix(annotation: PsiAnnotation) : LocalQuickFixOnPsiElement(annotation) { + override fun getFamilyName() = "Add argsOnly = true" + override fun getText() = "Add argsOnly = true" + + override fun invoke(project: Project, file: PsiFile, startElement: PsiElement, endElement: PsiElement) { + val annotation = startElement as? PsiAnnotation ?: return + val trueExpr = JavaPsiFacade.getElementFactory(project).createLiteralExpression(true) + annotation.setDeclaredAttributeValue("argsOnly", trueExpr) + } + } + + companion object { + fun shouldReport( + annotation: PsiAnnotation, + wantedType: PsiType, + methodTargets: Sequence, + ): Boolean { + if (annotation.findDeclaredAttributeValue("argsOnly")?.constantValue == true) { + return false + } + + val ordinal = (annotation.findDeclaredAttributeValue("ordinal")?.constantValue as? Int?) + ?.takeIf { it != -1 } + val index = (annotation.findDeclaredAttributeValue("index")?.constantValue as? Int?) + ?.takeIf { it != -1 } + if (ordinal == null && index == null && annotation.findDeclaredAttributeValue("name") != null) { + return false + } + + val wantedDesc = wantedType.descriptor + - for ((targetClass, targetMethod) in methodTargets) { - val argTypes = mutableListOf() - if (!targetMethod.hasAccess(Opcodes.ACC_STATIC)) { - argTypes += "L${targetClass.name};" - } - for (arg in Type.getArgumentTypes(targetMethod.desc)) { - argTypes += arg.descriptor - if (arg.size == 2) { - argTypes += null - } - } + for ((targetClass, targetMethod) in methodTargets) { + val argTypes = mutableListOf() + if (!targetMethod.hasAccess(Opcodes.ACC_STATIC)) { + argTypes += "L${targetClass.name};" + } + for (arg in Type.getArgumentTypes(targetMethod.desc)) { + argTypes += arg.descriptor + if (arg.size == 2) { + argTypes += null + } + } - if (ordinal != null) { + if (ordinal != null) { - if (argTypes.asSequence().filter { it == wantedType }.count() <= ordinal) { - return + if (argTypes.asSequence().filter { it == wantedDesc }.count() <= ordinal) { + return false - } - } else if (index != null) { - if (argTypes.size <= index) { + } + } else if (index != null) { + if (argTypes.size <= index) { - return + return false - } - } else { + } + } else { - if (argTypes.asSequence().filter { it == wantedType }.count() != 1) { - return + if (argTypes.asSequence().filter { it == wantedDesc }.count() != 1) { + return false - } - } - } + } + } + } - val description = "ModifyVariable may be argsOnly = true" - holder.registerProblem(problemElement, description, AddArgsOnlyFix(modifyVariable)) + return true - } - } - } + } + } +} - - override fun getStaticDescription() = - "Checks that ModifyVariable has argsOnly if it targets arguments, which improves performance of the mixin" - - private class AddArgsOnlyFix(annotation: PsiAnnotation) : LocalQuickFixOnPsiElement(annotation) { - override fun getFamilyName() = "Add argsOnly = true" - override fun getText() = "Add argsOnly = true" - - override fun invoke(project: Project, file: PsiFile, startElement: PsiElement, endElement: PsiElement) { - val annotation = startElement as? PsiAnnotation ?: return - val trueExpr = JavaPsiFacade.getElementFactory(project).createLiteralExpression(true) - annotation.setDeclaredAttributeValue("argsOnly", trueExpr) - } - } -} Index: src/main/kotlin/platform/mixin/inspection/mixinextras/LocalArgsOnlyInspection.kt =================================================================== --- src/main/kotlin/platform/mixin/inspection/mixinextras/LocalArgsOnlyInspection.kt (revision 88db0318dcf70f7ee98a0759bebb1bb8b328664b) +++ src/main/kotlin/platform/mixin/inspection/mixinextras/LocalArgsOnlyInspection.kt (revision 88db0318dcf70f7ee98a0759bebb1bb8b328664b) @@ -0,0 +1,74 @@ +/* + * 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.MixinAnnotationHandler +import com.demonwav.mcdev.platform.mixin.inspection.MixinInspection +import com.demonwav.mcdev.platform.mixin.inspection.injector.ModifyVariableArgsOnlyInspection +import com.demonwav.mcdev.platform.mixin.util.MethodTargetMember +import com.demonwav.mcdev.platform.mixin.util.MixinConstants +import com.demonwav.mcdev.platform.mixin.util.MixinConstants.MixinExtras.unwrapLocalRef +import com.demonwav.mcdev.util.constantValue +import com.demonwav.mcdev.util.findContainingMethod +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 LocalArgsOnlyInspection : MixinInspection() { + override fun getStaticDescription() = + "Checks that @Local has argsOnly if it targets arguments, which improves performance of the mixin" + + override fun buildVisitor(holder: ProblemsHolder): PsiElementVisitor = object : JavaElementVisitor() { + override fun visitAnnotation(localAnnotation: PsiAnnotation) { + if (localAnnotation.qualifiedName != MixinConstants.MixinExtras.LOCAL) { + return + } + if (localAnnotation.findDeclaredAttributeValue("argsOnly")?.constantValue == true) { + 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) + ?: return@mapFirstNotNull null + handler.resolveTarget(annotation).asSequence() + .filterIsInstance() + .map { it.classAndMethod } + } ?: return + + val localType = parameter.type.unwrapLocalRef() + + if (ModifyVariableArgsOnlyInspection.shouldReport(localAnnotation, localType, targets)) { + holder.registerProblem( + localAnnotation.nameReferenceElement ?: localAnnotation, + "@Local may be argsOnly = true", + ModifyVariableArgsOnlyInspection.AddArgsOnlyFix(localAnnotation) + ) + } + } + } +} Index: src/main/resources/META-INF/plugin.xml =================================================================== --- src/main/resources/META-INF/plugin.xml (revision 8da4a4e36fcb2388f1872f8c9c07654fc247c868) +++ src/main/resources/META-INF/plugin.xml (revision 88db0318dcf70f7ee98a0759bebb1bb8b328664b) @@ -871,7 +871,7 @@ level="ERROR" hasStaticDescription="true" implementationClass="com.demonwav.mcdev.platform.mixin.inspection.injector.InvalidInjectorMethodSignatureInspection"/> - +