User: joe
Date: 23 Jan 24 13:42
Revision: 11870b464c1bde4f6ee793cc6ba284a20e5464c0
Summary:
Add inspections for unnecessary unsafe and CTOR_HEAD over HEAD


TeamCity URL: https://ci.mcdev.io/viewModification.html?tab=vcsModificationFiles&modId=9033&personal=false

Index: src/main/kotlin/platform/mixin/inspection/fix/AnnotationAttributeFix.kt
===================================================================
--- src/main/kotlin/platform/mixin/inspection/fix/AnnotationAttributeFix.kt	(revision 11870b464c1bde4f6ee793cc6ba284a20e5464c0)
+++ src/main/kotlin/platform/mixin/inspection/fix/AnnotationAttributeFix.kt	(revision 11870b464c1bde4f6ee793cc6ba284a20e5464c0)
@@ -0,0 +1,112 @@
+/*
+ * 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 <https://www.gnu.org/licenses/>.
+ */
+
+package com.demonwav.mcdev.platform.mixin.inspection.fix
+
+import com.demonwav.mcdev.util.createLiteralExpression
+import com.intellij.codeInsight.intention.FileModifier.SafeFieldForPreview
+import com.intellij.codeInspection.LocalQuickFixOnPsiElement
+import com.intellij.openapi.project.Project
+import com.intellij.psi.JavaPsiFacade
+import com.intellij.psi.PsiAnnotation
+import com.intellij.psi.PsiAnnotationMemberValue
+import com.intellij.psi.PsiElement
+import com.intellij.psi.PsiFile
+import com.intellij.psi.codeStyle.CodeStyleManager
+
+open class AnnotationAttributeFix(
+    annotation: PsiAnnotation,
+    vararg attributes: Pair<String, Any?>,
+) : LocalQuickFixOnPsiElement(annotation) {
+    @SafeFieldForPreview
+    private val attributes = attributes.map { (key, value) ->
+        key to value?.let {
+            if (it !is PsiAnnotationMemberValue) {
+                JavaPsiFacade.getElementFactory(annotation.project).createLiteralExpression(it)
+            } else {
+                it
+            }
+        }
+    }
+
+    private val description = run {
+        val added = this.attributes
+            .filter { (_, value) -> value != null }
+            .map { (key, value) -> "$key = ${value!!.text}" }
+        val removed = this.attributes
+            .filter { (_, value) -> value == null }
+            .map { (key, _) -> key }
+
+        buildString {
+            if (added.isNotEmpty()) {
+                append("Add ")
+                for ((i, add) in added.withIndex()) {
+                    if (i != 0) {
+                        if (i == added.size - 1) {
+                            append(" and ")
+                        } else {
+                            append(", ")
+                        }
+                    }
+                    append(add)
+                }
+                if (removed.isNotEmpty()) {
+                    append(", and remove ")
+                }
+            } else if (removed.isNotEmpty()) {
+                append("Remove ")
+            }
+            if (removed.isNotEmpty()) {
+                for ((i, rem) in removed.withIndex()) {
+                    if (i != 0) {
+                        if (i == removed.size - 1) {
+                            append(" and ")
+                        } else {
+                            append(", ")
+                        }
+                    }
+                    append(rem)
+                }
+            }
+        }
+    }
+
+    override fun getFamilyName() = description
+    override fun getText() = description
+
+    override fun invoke(project: Project, file: PsiFile, startElement: PsiElement, endElement: PsiElement) {
+        val annotation = startElement as? PsiAnnotation ?: return
+        for ((key, value) in attributes) {
+            annotation.setDeclaredAttributeValue(key, value)
+        }
+
+        // replace single "value = foo" with "foo"
+        val attrs = annotation.parameterList.attributes
+        if (attrs.size == 1 && attrs[0].name == "value") {
+            attrs[0].value?.let { value ->
+                val fakeAnnotation = JavaPsiFacade.getElementFactory(project).createAnnotationFromText("@Foo(0)", null)
+                fakeAnnotation.parameterList.attributes[0].value!!.replace(value)
+                annotation.parameterList.replace(fakeAnnotation.parameterList)
+            }
+        }
+
+        CodeStyleManager.getInstance(project).reformat(annotation.parameterList)
+    }
+}
Index: src/main/kotlin/platform/mixin/inspection/injector/CtorHeadNoUnsafeInspection.kt
===================================================================
--- src/main/kotlin/platform/mixin/inspection/injector/CtorHeadNoUnsafeInspection.kt	(revision ab985497ef6095c7c2dcb11a253606fa6da72e96)
+++ src/main/kotlin/platform/mixin/inspection/injector/CtorHeadNoUnsafeInspection.kt	(revision 11870b464c1bde4f6ee793cc6ba284a20e5464c0)
@@ -23,6 +23,7 @@
 import com.demonwav.mcdev.facet.MinecraftFacet
 import com.demonwav.mcdev.platform.fabric.FabricModuleType
 import com.demonwav.mcdev.platform.mixin.inspection.MixinInspection
+import com.demonwav.mcdev.platform.mixin.inspection.fix.AnnotationAttributeFix
 import com.demonwav.mcdev.platform.mixin.util.MixinConstants
 import com.demonwav.mcdev.util.constantStringValue
 import com.demonwav.mcdev.util.constantValue
@@ -76,7 +77,7 @@
                 holder.registerProblem(
                     valueElement,
                     "CTOR_HEAD is missing unsafe = true",
-                    InjectIntoConstructorInspection.AddUnsafeFix(annotation),
+                    AnnotationAttributeFix(annotation, "unsafe" to true),
                 )
             }
         }
Index: src/main/kotlin/platform/mixin/inspection/injector/CtorHeadUsedForNonConstructorInspection.kt
===================================================================
--- src/main/kotlin/platform/mixin/inspection/injector/CtorHeadUsedForNonConstructorInspection.kt	(revision 11870b464c1bde4f6ee793cc6ba284a20e5464c0)
+++ src/main/kotlin/platform/mixin/inspection/injector/CtorHeadUsedForNonConstructorInspection.kt	(revision 11870b464c1bde4f6ee793cc6ba284a20e5464c0)
@@ -0,0 +1,59 @@
+/*
+ * 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 <https://www.gnu.org/licenses/>.
+ */
+
+package com.demonwav.mcdev.platform.mixin.inspection.injector
+
+import com.demonwav.mcdev.platform.mixin.inspection.MixinInspection
+import com.demonwav.mcdev.platform.mixin.inspection.fix.AnnotationAttributeFix
+import com.demonwav.mcdev.platform.mixin.util.MixinConstants
+import com.demonwav.mcdev.util.constantValue
+import com.intellij.codeInspection.ProblemsHolder
+import com.intellij.psi.JavaElementVisitor
+import com.intellij.psi.PsiAnnotation
+import com.intellij.psi.PsiElementVisitor
+
+class CtorHeadUsedForNonConstructorInspection : MixinInspection() {
+    override fun getStaticDescription() = "Reports when CTOR_HEAD is used without targeting a constructor method"
+
+    override fun buildVisitor(holder: ProblemsHolder): PsiElementVisitor = object : JavaElementVisitor() {
+        override fun visitAnnotation(annotation: PsiAnnotation) {
+            if (!annotation.hasQualifiedName(MixinConstants.Annotations.AT)) {
+                return
+            }
+            val atValue = annotation.findDeclaredAttributeValue("value") ?: return
+            if (atValue.constantValue != "CTOR_HEAD") {
+                return
+            }
+            if (!UnnecessaryUnsafeInspection.mightTargetConstructor(holder.project, annotation)) {
+                holder.registerProblem(
+                    atValue,
+                    "CTOR_HEAD used without targeting a constructor",
+                    ReplaceWithHeadFix(annotation),
+                )
+            }
+        }
+    }
+
+    private class ReplaceWithHeadFix(at: PsiAnnotation) :
+        AnnotationAttributeFix(at, "value" to "HEAD", "unsafe" to null) {
+        override fun getFamilyName() = "Replace with \"HEAD\""
+        override fun getText() = "Replace with \"HEAD\""
+    }
+}
Index: src/main/kotlin/platform/mixin/inspection/injector/InjectIntoConstructorInspection.kt
===================================================================
--- src/main/kotlin/platform/mixin/inspection/injector/InjectIntoConstructorInspection.kt	(revision ab985497ef6095c7c2dcb11a253606fa6da72e96)
+++ src/main/kotlin/platform/mixin/inspection/injector/InjectIntoConstructorInspection.kt	(revision 11870b464c1bde4f6ee793cc6ba284a20e5464c0)
@@ -26,25 +26,19 @@
 import com.demonwav.mcdev.platform.mixin.handlers.MixinAnnotationHandler
 import com.demonwav.mcdev.platform.mixin.handlers.injectionPoint.AtResolver
 import com.demonwav.mcdev.platform.mixin.inspection.MixinInspection
+import com.demonwav.mcdev.platform.mixin.inspection.fix.AnnotationAttributeFix
 import com.demonwav.mcdev.platform.mixin.util.MethodTargetMember
 import com.demonwav.mcdev.platform.mixin.util.MixinConstants.Annotations.INJECT
 import com.demonwav.mcdev.platform.mixin.util.findSuperConstructorCall
 import com.demonwav.mcdev.platform.mixin.util.isConstructor
 import com.demonwav.mcdev.util.constantValue
-import com.demonwav.mcdev.util.createLiteralExpression
 import com.demonwav.mcdev.util.findAnnotation
 import com.demonwav.mcdev.util.findAnnotations
 import com.demonwav.mcdev.util.findModule
-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.JavaPsiFacade
-import com.intellij.psi.PsiAnnotation
 import com.intellij.psi.PsiClass
-import com.intellij.psi.PsiElement
 import com.intellij.psi.PsiElementVisitor
-import com.intellij.psi.PsiFile
 import com.intellij.psi.PsiMethod
 import java.awt.FlowLayout
 import javax.swing.JCheckBox
@@ -96,7 +90,7 @@
                             val atHasUnsafe = !atClass?.findMethodsByName("unsafe", false).isNullOrEmpty()
 
                             val quickFixes = if (atHasUnsafe) {
-                                arrayOf(AddUnsafeFix(at))
+                                arrayOf(AnnotationAttributeFix(at, "unsafe" to true))
                             } else {
                                 emptyArray()
                             }
@@ -130,15 +124,4 @@
     }
 
     override fun getStaticDescription() = "@Inject into Constructor"
-
-    class AddUnsafeFix(at: PsiAnnotation) : LocalQuickFixOnPsiElement(at) {
-        override fun getFamilyName() = "Add unsafe = true"
-        override fun getText() = "Add unsafe = 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("unsafe", trueExpr)
-        }
+}
-    }
-}
Index: src/main/kotlin/platform/mixin/inspection/injector/ModifyVariableArgsOnlyInspection.kt
===================================================================
--- src/main/kotlin/platform/mixin/inspection/injector/ModifyVariableArgsOnlyInspection.kt	(revision ab985497ef6095c7c2dcb11a253606fa6da72e96)
+++ src/main/kotlin/platform/mixin/inspection/injector/ModifyVariableArgsOnlyInspection.kt	(revision 11870b464c1bde4f6ee793cc6ba284a20e5464c0)
@@ -22,24 +22,19 @@
 
 import com.demonwav.mcdev.platform.mixin.handlers.MixinAnnotationHandler
 import com.demonwav.mcdev.platform.mixin.inspection.MixinInspection
+import com.demonwav.mcdev.platform.mixin.inspection.fix.AnnotationAttributeFix
 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
 import com.demonwav.mcdev.util.constantValue
-import com.demonwav.mcdev.util.createLiteralExpression
 import com.demonwav.mcdev.util.descriptor
 import com.demonwav.mcdev.util.findAnnotation
 import com.demonwav.mcdev.util.ifEmpty
-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.JavaPsiFacade
 import com.intellij.psi.PsiAnnotation
-import com.intellij.psi.PsiElement
 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
@@ -64,23 +59,16 @@
 
                 if (shouldReport(modifyVariable, wantedType, methodTargets)) {
                     val description = "@ModifyVariable may be argsOnly = true"
-                    holder.registerProblem(problemElement, description, AddArgsOnlyFix(modifyVariable))
+                    holder.registerProblem(
+                        problemElement,
+                        description,
+                        AnnotationAttributeFix(modifyVariable, "argsOnly" to true),
+                    )
                 }
             }
         }
     }
 
-    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,
Index: src/main/kotlin/platform/mixin/inspection/injector/UnnecessaryUnsafeInspection.kt
===================================================================
--- src/main/kotlin/platform/mixin/inspection/injector/UnnecessaryUnsafeInspection.kt	(revision 11870b464c1bde4f6ee793cc6ba284a20e5464c0)
+++ src/main/kotlin/platform/mixin/inspection/injector/UnnecessaryUnsafeInspection.kt	(revision 11870b464c1bde4f6ee793cc6ba284a20e5464c0)
@@ -0,0 +1,114 @@
+/*
+ * 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 <https://www.gnu.org/licenses/>.
+ */
+
+package com.demonwav.mcdev.platform.mixin.inspection.injector
+
+import com.demonwav.mcdev.facet.MinecraftFacet
+import com.demonwav.mcdev.platform.fabric.FabricModuleType
+import com.demonwav.mcdev.platform.mixin.handlers.MixinAnnotationHandler
+import com.demonwav.mcdev.platform.mixin.inspection.MixinInspection
+import com.demonwav.mcdev.platform.mixin.inspection.fix.AnnotationAttributeFix
+import com.demonwav.mcdev.platform.mixin.util.MethodTargetMember
+import com.demonwav.mcdev.platform.mixin.util.MixinConstants
+import com.demonwav.mcdev.platform.mixin.util.isConstructor
+import com.demonwav.mcdev.util.constantValue
+import com.demonwav.mcdev.util.findModule
+import com.demonwav.mcdev.util.ifEmpty
+import com.intellij.codeInspection.ProblemsHolder
+import com.intellij.openapi.project.Project
+import com.intellij.psi.JavaElementVisitor
+import com.intellij.psi.PsiAnnotation
+import com.intellij.psi.PsiClass
+import com.intellij.psi.PsiElementVisitor
+import com.intellij.psi.PsiModifierList
+import com.intellij.psi.util.parents
+import java.awt.FlowLayout
+import javax.swing.JCheckBox
+import javax.swing.JComponent
+import javax.swing.JPanel
+
+class UnnecessaryUnsafeInspection : MixinInspection() {
+    @JvmField
+    var alwaysUnnecessaryOnFabric = true
+
+    override fun getStaticDescription() = "Reports unnecessary unsafe = true"
+
+    override fun createOptionsPanel(): JComponent {
+        val panel = JPanel(FlowLayout(FlowLayout.LEFT))
+        val checkbox = JCheckBox("Always unnecessary in Fabric mods", alwaysUnnecessaryOnFabric)
+        checkbox.addActionListener {
+            alwaysUnnecessaryOnFabric = checkbox.isSelected
+        }
+        panel.add(checkbox)
+        return panel
+    }
+
+    override fun buildVisitor(holder: ProblemsHolder): PsiElementVisitor {
+        val isFabric = holder.file.findModule()?.let { MinecraftFacet.getInstance(it) }?.isOfType(FabricModuleType)
+            ?: false
+        val alwaysUnnecessary = isFabric && alwaysUnnecessaryOnFabric
+
+        return object : JavaElementVisitor() {
+            override fun visitAnnotation(annotation: PsiAnnotation) {
+                if (!annotation.hasQualifiedName(MixinConstants.Annotations.AT)) {
+                    return
+                }
+                if (!alwaysUnnecessary &&
+                    annotation.findDeclaredAttributeValue("value")?.constantValue == "CTOR_HEAD"
+                ) {
+                    // this case is handled by a specific inspection for CTOR_HEAD
+                    return
+                }
+
+                val unsafeValue = annotation.findDeclaredAttributeValue("unsafe") ?: return
+                if (unsafeValue.constantValue != true) {
+                    return
+                }
+
+                if (alwaysUnnecessary || !mightTargetConstructor(holder.project, annotation)) {
+                    holder.registerProblem(
+                        unsafeValue,
+                        "Unnecessary unsafe = true",
+                        AnnotationAttributeFix(annotation, "unsafe" to null)
+                    )
+                }
+            }
+        }
+    }
+
+    companion object {
+        fun mightTargetConstructor(project: Project, at: PsiAnnotation): Boolean {
+            val injectorAnnotation = at.parents(false)
+                .takeWhile { it !is PsiClass }
+                .filterIsInstance<PsiAnnotation>()
+                .firstOrNull { it.parent is PsiModifierList }
+                ?: return true
+            val handler = injectorAnnotation.qualifiedName?.let {
+                MixinAnnotationHandler.forMixinAnnotation(it, project)
+            } ?: return true
+
+            val targets = handler.resolveTarget(injectorAnnotation)
+                .filterIsInstance<MethodTargetMember>()
+                .ifEmpty { return true }
+
+            return targets.any { it.classAndMethod.method.isConstructor }
+        }
+    }
+}
Index: src/main/kotlin/platform/mixin/inspection/mixinextras/LocalArgsOnlyInspection.kt
===================================================================
--- src/main/kotlin/platform/mixin/inspection/mixinextras/LocalArgsOnlyInspection.kt	(revision ab985497ef6095c7c2dcb11a253606fa6da72e96)
+++ src/main/kotlin/platform/mixin/inspection/mixinextras/LocalArgsOnlyInspection.kt	(revision 11870b464c1bde4f6ee793cc6ba284a20e5464c0)
@@ -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.inspection.fix.AnnotationAttributeFix
 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
@@ -66,7 +67,7 @@
                 holder.registerProblem(
                     localAnnotation.nameReferenceElement ?: localAnnotation,
                     "@Local may be argsOnly = true",
-                    ModifyVariableArgsOnlyInspection.AddArgsOnlyFix(localAnnotation)
+                    AnnotationAttributeFix(localAnnotation, "argsOnly" to true)
                 )
             }
         }
Index: src/main/resources/META-INF/plugin.xml
===================================================================
--- src/main/resources/META-INF/plugin.xml	(revision ab985497ef6095c7c2dcb11a253606fa6da72e96)
+++ src/main/resources/META-INF/plugin.xml	(revision 11870b464c1bde4f6ee793cc6ba284a20e5464c0)
@@ -896,6 +896,22 @@
                          level="ERROR"
                          hasStaticDescription="true"
                          implementationClass="com.demonwav.mcdev.platform.mixin.inspection.injector.CtorHeadNoUnsafeInspection"/>
+        <localInspection displayName="CTOR_HEAD is not targeting a constructor"
+                         shortName="CtorHeadUsedForNonConstructor"
+                         groupName="Mixin"
+                         language="JAVA"
+                         enabledByDefault="true"
+                         level="WARNING"
+                         hasStaticDescription="true"
+                         implementationClass="com.demonwav.mcdev.platform.mixin.inspection.injector.CtorHeadUsedForNonConstructorInspection"/>
+        <localInspection displayName="Unnecessary unsafe = true"
+                         shortName="UnnecessaryUnsafe"
+                         groupName="Mixin"
+                         language="JAVA"
+                         enabledByDefault="true"
+                         level="WARNING"
+                         hasStaticDescription="true"
+                         implementationClass="com.demonwav.mcdev.platform.mixin.inspection.injector.UnnecessaryUnsafeInspection"/>
         <!--endregion-->
 
         <!--region Overwrite Inspections -->