Merge branch '2024.2' into 2024.3

Author: Earthcomputer

changelog.md

@@ -1,5 +1,43 @@
 # Minecraft Development for IntelliJ
 
+## [1.8.6]
+
+## Added
+
+- Debug flow diagrams for MixinExtras expressions.
+- An inspection to replace `INVOKE_ASSIGN` with MixinExtras expressions where possible.
+  - Expressions are preferred because `INVOKE_ASSIGN` doesn't fail if there's no assignment.
+
+## Changed
+
+- Performance improvements for shadow completions.
+- Smarter warnings for discouraged shifting
+  - Shifting is now always discouraged on injectors that can't inject on any instruction in the method (such as `@Redirect`).
+  - When MixinExtras expressions are being used, the discourage shifting logic now takes into account the type of expression.
+- Mixin navigation to lambdas has been improved. MinecraftDev will now properly detect that a synthetic method will be
+  generated for method references in the following additional cases:
+  - All array method references.
+  - Method references to a superclass method (`super::foo`).
+  - Constructor method references for a non-static inner class.
+    - Previously, all constructor method references were incorrectly assumed to generate a synthetic method.
+  - Varargs parameter expansions.
+  - Access to private methods in an outer or inner class before Java 9.
+  - Access to protected methods in the superclass of an outer class, if the superclass is in a different package.
+  - Cases where the functional interface method being implemented has a parameter which is an intersection type (a type
+    parameter extending multiple things).
+
+## Fixed
+
+- Fixed "generate accessor/invoker" action not doing anything.
+- Fixed Mixin version detection in legacy/non-standard Minecraft environments. The Mixin version is now taken from the
+  `MixinBootstrap` class, rather than attempting to decipher it from the library artifact.
+- Fixed `@Coerce` on return type wanting subtypes not supertypes.
+- Fixed the "add definition" quick-fix for MixinExtras expressions creating a "dummy" definition since 2025.1
+- Fixed incorrect unused warnings on MixinExtras expression definitions
+- Fixed "find usages" on MixinExtras expression definitions
+- Fixed IDE error that sometimes occurs when navigating to mixin target
+- Fixed mixin navigation to field assignments.
+
 ## [1.8.5]
 
 ### Added

gradle.properties

@@ -23,7 +23,7 @@ org.gradle.jvmargs=-Xmx1g
 
 ideaVersionName = 2024.3
 
-coreVersion = 1.8.5
+coreVersion = 1.8.6
 
 # Silences a build-time warning because we are bundling our own kotlin library
 kotlin.stdlib.default.dependency = false

gradle/libs.versions.toml

@@ -60,7 +60,7 @@ fuel-coroutines = { module = "com.github.kittinunf.fuel:fuel-coroutines", versio
 # Testing
 test-mixin = "org.spongepowered:mixin:0.8.5"
 test-spigotapi = "org.spigotmc:spigot-api:1.21-R0.1-SNAPSHOT"
-test-bungeecord = "net.md-5:bungeecord-api:1.21-R0.3-SNAPSHOT"
+test-bungeecord = "net.md-5:bungeecord-api:1.21-R0.3"
 test-spongeapi = "org.spongepowered:spongeapi:7.4.0"
 test-fabricloader = "net.fabricmc:fabric-loader:0.15.11"
 test-nbt = "com.demonwav.mcdev:all-types-nbt:1.0"

src/main/kotlin/platform/mixin/expression/gui/DiagramStyles.kt

@@ -64,21 +64,29 @@ object DiagramStyles {
     )
     val FAILED
         get() = mapOf(
-            mxConstants.STYLE_STROKECOLOR to JBColor.red.hexString,
+            mxConstants.STYLE_STROKECOLOR to FlowMatchStatus.FAIL.hexColor,
             mxConstants.STYLE_STROKEWIDTH to "3.5",
         )
     val PARTIAL_MATCH
         get() = mapOf(
-            mxConstants.STYLE_STROKECOLOR to JBColor.orange.hexString,
+            mxConstants.STYLE_STROKECOLOR to FlowMatchStatus.PARTIAL.hexColor,
             mxConstants.STYLE_STROKEWIDTH to "2.5",
         )
     val SUCCESS
         get() = mapOf(
-            mxConstants.STYLE_STROKECOLOR to JBColor.green.hexString,
+            mxConstants.STYLE_STROKECOLOR to FlowMatchStatus.SUCCESS.hexColor,
             mxConstants.STYLE_STROKEWIDTH to "1.5",
         )
     val CURRENT_EDITOR_FONT
         get() = EditorColorsManager.getInstance().globalScheme.getFont(EditorFontType.PLAIN)
 }
 
-private val Color.hexString get() = "#%06X".format(rgb)
+private val Color.hexString get() = "#%06X".format(rgb and 0xFFFFFF)
+
+val FlowMatchStatus.hexColor
+    get() = when (this) {
+        FlowMatchStatus.SUCCESS -> JBColor.green
+        FlowMatchStatus.PARTIAL -> JBColor.orange
+        FlowMatchStatus.FAIL -> JBColor.red
+        FlowMatchStatus.IGNORED -> UIUtil.getLabelForeground()
+    }.hexString

src/main/kotlin/platform/mixin/expression/gui/FlowDiagram.kt

@@ -22,10 +22,8 @@ package com.demonwav.mcdev.platform.mixin.expression.gui
 
 import com.demonwav.mcdev.platform.mixin.expression.MEExpressionMatchUtil
 import com.demonwav.mcdev.util.constantStringValue
-import com.intellij.openapi.application.ApplicationManager
 import com.intellij.openapi.application.EDT
-import com.intellij.openapi.application.ModalityState
-import com.intellij.openapi.application.ReadAction
+import com.intellij.openapi.application.readAction
 import com.intellij.openapi.module.Module
 import com.intellij.openapi.progress.checkCanceled
 import com.intellij.openapi.project.Project
@@ -41,8 +39,9 @@ import com.mxgraph.util.mxRectangle
 import com.mxgraph.view.mxGraph
 import java.awt.Dimension
 import java.util.SortedMap
-import java.util.concurrent.Callable
+import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
 import org.objectweb.asm.tree.ClassNode
 import org.objectweb.asm.tree.MethodNode
@@ -53,15 +52,21 @@ private const val INTRA_GROUP_SPACING = 75
 private const val LINE_NUMBER_STYLE = "LINE_NUMBER"
 
 class FlowDiagram(
+    private val scope: CoroutineScope,
     val ui: FlowDiagramUi,
     private val flowGraph: FlowGraph,
     private val clazz: ClassNode,
     val method: MethodNode,
 ) {
     companion object {
-        suspend fun create(project: Project, clazz: ClassNode, method: MethodNode): FlowDiagram? {
+        suspend fun create(
+            project: Project,
+            scope: CoroutineScope,
+            clazz: ClassNode,
+            method: MethodNode
+        ): FlowDiagram? {
             val flowGraph = FlowGraph.parse(project, clazz, method) ?: return null
-            return buildDiagram(flowGraph, clazz, method)
+            return buildDiagram(scope, flowGraph, clazz, method)
         }
     }
 
@@ -91,6 +96,12 @@ class FlowDiagram(
             flowGraph.highlightMatches(node, soft)
             ui.refresh()
         }
+
+        flowGraph.onHighlightChanged { exprText, node ->
+            scope.launch(Dispatchers.EDT) {
+                ui.showExpr(exprText, node)
+            }
+        }
     }
 
     fun populateMatchStatuses(
@@ -105,41 +116,42 @@ class FlowDiagram(
             val oldHighlightRoot = flowGraph.highlightRoot
             ui.setMatchToolbarVisible(false)
             flowGraph.resetMatches()
-            ReadAction.nonBlocking(Callable<String?> run@{
-                val stringLit = stringRef.element ?: return@run null
-                val modifierList = modifierListRef.element ?: return@run null
-                val expression = stringLit.constantStringValue?.let(MEExpressionMatchUtil::createExpression)
-                    ?: return@run null
-                val pool = MEExpressionMatchUtil.createIdentifierPoolFactory(module, clazz, modifierList)(method)
-                for ((virtualInsn, root) in flowGraph.flowMap) {
-                    val node = flowGraph.allNodes.getValue(root)
-                    MEExpressionMatchUtil.findMatchingInstructions(
-                        clazz, method, pool, flowGraph.flowMap, expression, listOf(virtualInsn),
-                        ExpressionContext.Type.MODIFY_EXPRESSION_VALUE, // most permissive
-                        false,
-                        node::reportMatchStatus,
-                        node::reportPartialMatch
-                    ) {}
+            scope.launch(Dispatchers.Default) {
+                val success = readAction run@{
+                    val stringLit = stringRef.element ?: return@run false
+                    val modifierList = modifierListRef.element ?: return@run false
+                    val expression = stringLit.constantStringValue?.let(MEExpressionMatchUtil::createExpression)
+                        ?: return@run false
+                    val pool = MEExpressionMatchUtil.createIdentifierPoolFactory(module, clazz, modifierList)(method)
+                    for ((virtualInsn, root) in flowGraph.flowMap) {
+                        val node = flowGraph.allNodes.getValue(root)
+                        MEExpressionMatchUtil.findMatchingInstructions(
+                            clazz, method, pool, flowGraph.flowMap, expression, listOf(virtualInsn),
+                            ExpressionContext.Type.MODIFY_EXPRESSION_VALUE, // most permissive
+                            false,
+                            node::reportMatchStatus,
+                            node::reportPartialMatch
+                        ) {}
+                    }
+                    flowGraph.setExprText(expression.src.toString())
+                    flowGraph.highlightMatches(oldHighlightRoot, false)
+                    true
                 }
-                flowGraph.markHasMatchData()
-                flowGraph.highlightMatches(oldHighlightRoot, false)
-                StringUtil.escapeStringCharacters(expression.src.toString())
-            })
-                .finishOnUiThread(ModalityState.nonModal()) { exprText ->
-                    exprText ?: return@finishOnUiThread
+                if (success) {
                     if (jump) {
                         showBestNode()
                     }
-                    ui.refresh()
-                    ui.setExprText(exprText)
                 }
-                .submit(ApplicationManager.getApplication()::executeOnPooledThread)
+                ui.refresh()
+            }
         }
         this.jumpToExpression = {
-            ReadAction.run<Nothing> {
-                val target = stringRef.element
-                if (target is Navigatable && target.isValid && target.canNavigate()) {
-                    target.navigate(true)
+            scope.launch {
+                readAction {
+                    val target = stringRef.element
+                    if (target is Navigatable && target.isValid && target.canNavigate()) {
+                        target.navigate(true)
+                    }
                 }
             }
         }
@@ -161,7 +173,12 @@ class FlowDiagram(
     }
 }
 
-private suspend fun buildDiagram(flowGraph: FlowGraph, clazz: ClassNode, method: MethodNode): FlowDiagram {
+private suspend fun buildDiagram(
+    scope: CoroutineScope,
+    flowGraph: FlowGraph,
+    clazz: ClassNode,
+    method: MethodNode
+): FlowDiagram {
     val graph = MxFlowGraph(flowGraph)
     setupStyles(graph)
     val groupedCells = addGraphContent(graph, flowGraph)
@@ -171,7 +188,7 @@ private suspend fun buildDiagram(flowGraph: FlowGraph, clazz: ClassNode, method:
     val ui = withContext(Dispatchers.EDT) {
         FlowDiagramUi(graph, calculateBounds, lineNumberNodes)
     }
-    return FlowDiagram(ui, flowGraph, clazz, method)
+    return FlowDiagram(scope, ui, flowGraph, clazz, method)
 }
 
 private class MxFlowGraph(private val flowGraph: FlowGraph) : mxGraph() {

src/main/kotlin/platform/mixin/expression/gui/FlowDiagramUi.kt

@@ -22,6 +22,7 @@ package com.demonwav.mcdev.platform.mixin.expression.gui
 
 import com.intellij.icons.AllIcons
 import com.intellij.openapi.editor.colors.EditorColorsManager
+import com.intellij.openapi.util.text.StringUtil
 import com.intellij.ui.DocumentAdapter
 import com.mxgraph.model.mxCell
 import com.mxgraph.swing.mxGraphComponent
@@ -83,8 +84,8 @@ class FlowDiagramUi(
         fixBounds()
     }
 
-    fun setExprText(text: String) {
-        matchToolbar.setExprTest(text)
+    fun showExpr(text: String, highlightRoot: FlowNode?) {
+        matchToolbar.setExprText("<html>" + makeExprString(text, highlightRoot) + "</html>")
         matchToolbar.isVisible = true
     }
 
@@ -243,7 +244,7 @@ class FlowDiagramUi(
             add(buttonPanel, BorderLayout.EAST)
         }
 
-        fun setExprTest(text: String) {
+        fun setExprText(text: String) {
             exprText.text = text
             exprText.toolTipText = text
         }
@@ -277,3 +278,54 @@ private fun makeButton(icon: Icon, tooltip: String): JButton =
         toolTipText = tooltip
         preferredSize = Dimension(32, 32)
     }
+
+private sealed class HighlightChange : Comparable<HighlightChange> {
+    abstract val pos: Int
+
+    data class Start(override val pos: Int, val length: Int, val status: FlowMatchStatus) : HighlightChange()
+    data class End(override val pos: Int) : HighlightChange()
+
+    override fun compareTo(other: HighlightChange): Int =
+        compareValuesBy(
+            this, other,
+            { it.pos },
+            { if (it is Start) 1 else -1 },
+            { -((it as? Start)?.length ?: 0) },
+        )
+}
+
+private fun makeExprString(text: String, highlightRoot: FlowNode?): String {
+    fun escape(str: String) = StringUtil.escapeXmlEntities(StringUtil.escapeStringCharacters(str))
+
+    if (highlightRoot == null) {
+        return escape(text)
+    }
+
+    val changes = mutableListOf<HighlightChange>()
+    for ((status, src) in highlightRoot.matches) {
+        if (src == null) {
+            continue
+        }
+        changes.add(HighlightChange.Start(src.startIndex, src.endIndex - src.startIndex, status))
+        changes.add(HighlightChange.End(src.endIndex + 1))
+    }
+    changes.sort()
+
+    val result = StringBuilder()
+    var pos = 0
+    for (change in changes) {
+        result.append(escape(text.substring(pos, change.pos)))
+        pos = change.pos
+        when (change) {
+            is HighlightChange.Start -> {
+                result.append("<span style='color: ${change.status.hexColor}'>")
+            }
+            is HighlightChange.End -> {
+                result.append("</span>")
+            }
+        }
+    }
+    result.append(escape(text.substring(pos)))
+
+    return result.toString()
+}