Skip to content

Commit e6ddd88

Browse files
EarthcomputerLlamaLad7
Earthcomputer
and
LlamaLad7
committed
Add unnecessary mutable reference to captured local inspection.
Co-authored-by: LlamaLad7 <[email protected]>
1 parent 3a6b32f commit e6ddd88

File tree

3 files changed

+207
-0
lines changed

3 files changed

+207
-0
lines changed

‎src/main/kotlin/platform/mixin/inspection/mixinextras/UnnecessaryMutableLocalInspection.kt

+197
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
/*
2+
* Minecraft Development for IntelliJ
3+
*
4+
* https://mcdev.io/
5+
*
6+
* Copyright (C) 2023 minecraft-dev
7+
*
8+
* This program is free software: you can redistribute it and/or modify
9+
* it under the terms of the GNU Lesser General Public License as published
10+
* by the Free Software Foundation, version 3.0 only.
11+
*
12+
* This program is distributed in the hope that it will be useful,
13+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15+
* GNU General Public License for more details.
16+
*
17+
* You should have received a copy of the GNU Lesser General Public License
18+
* along with this program. If not, see <https://www.gnu.org/licenses/>.
19+
*/
20+
21+
package com.demonwav.mcdev.platform.mixin.inspection.mixinextras
22+
23+
import com.demonwav.mcdev.platform.mixin.handlers.InjectorAnnotationHandler
24+
import com.demonwav.mcdev.platform.mixin.handlers.MixinAnnotationHandler
25+
import com.demonwav.mcdev.platform.mixin.handlers.mixinextras.WrapOperationHandler
26+
import com.demonwav.mcdev.platform.mixin.inspection.MixinInspection
27+
import com.demonwav.mcdev.platform.mixin.util.MixinConstants
28+
import com.demonwav.mcdev.util.findContainingMethod
29+
import com.intellij.codeInspection.LocalQuickFixOnPsiElement
30+
import com.intellij.codeInspection.ProblemsHolder
31+
import com.intellij.openapi.project.Project
32+
import com.intellij.psi.JavaElementVisitor
33+
import com.intellij.psi.PsiClass
34+
import com.intellij.psi.PsiClassType
35+
import com.intellij.psi.PsiElement
36+
import com.intellij.psi.PsiElementFactory
37+
import com.intellij.psi.PsiExpressionList
38+
import com.intellij.psi.PsiFile
39+
import com.intellij.psi.PsiMethod
40+
import com.intellij.psi.PsiMethodCallExpression
41+
import com.intellij.psi.PsiParameter
42+
import com.intellij.psi.PsiReferenceExpression
43+
import com.intellij.psi.PsiType
44+
import com.intellij.psi.search.searches.OverridingMethodsSearch
45+
import com.intellij.psi.search.searches.ReferencesSearch
46+
import com.intellij.psi.util.PsiUtil
47+
import com.intellij.psi.util.parentOfType
48+
import com.siyeh.ig.psiutils.MethodCallUtils
49+
import org.jetbrains.plugins.groovy.intentions.style.inference.resolve
50+
51+
class UnnecessaryMutableLocalInspection : MixinInspection() {
52+
override fun getStaticDescription() = "Unnecessary mutable reference to captured local"
53+
54+
override fun buildVisitor(holder: ProblemsHolder) = object : JavaElementVisitor() {
55+
override fun visitMethod(method: PsiMethod) {
56+
val project = method.project
57+
val hasValidMixinAnnotation = method.annotations.any { ann ->
58+
ann.qualifiedName?.let { MixinAnnotationHandler.forMixinAnnotation(it, project) }
59+
// Mutable Local references do have different semantics inside a WrapOperation.
60+
?.let { it is InjectorAnnotationHandler && it !is WrapOperationHandler } == true
61+
}
62+
if (hasValidMixinAnnotation) {
63+
return
64+
}
65+
66+
// ignore if method has any references
67+
val hasReferences = ReferencesSearch.search(method)
68+
.mapNotNull { PsiUtil.skipParenthesizedExprUp(it.element).parent as? PsiMethodCallExpression }
69+
.any { !MethodCallUtils.hasSuperQualifier(it) }
70+
if (hasReferences) {
71+
return
72+
}
73+
74+
for ((i, param) in method.parameterList.parameters.withIndex()) {
75+
if (!param.hasAnnotation(MixinConstants.MixinExtras.LOCAL)) {
76+
continue
77+
}
78+
val paramType = param.type.resolve()
79+
if (paramType?.qualifiedName?.startsWith(MixinConstants.MixinExtras.LOCAL_REF_PACKAGE) != true) {
80+
continue
81+
}
82+
83+
checkParameter(holder, method, param, i, paramType)
84+
}
85+
}
86+
}
87+
88+
private fun checkParameter(
89+
holder: ProblemsHolder,
90+
originalMethod: PsiMethod,
91+
originalParam: PsiParameter,
92+
paramIndex: Int,
93+
paramType: PsiClass
94+
) {
95+
var hasAnyGets = false
96+
for (method in OverridingMethodsSearch.search(originalMethod).findAll() + listOf(originalMethod)) {
97+
val param = method.parameterList.getParameter(paramIndex) ?: return
98+
val getMethod = paramType.findMethodsByName("get", false).firstOrNull() ?: return
99+
for (ref in ReferencesSearch.search(param)) {
100+
if (isDelegationToSuper(ref.element, paramIndex)) {
101+
continue
102+
}
103+
val parent = PsiUtil.skipParenthesizedExprUp(ref.element.parent) as? PsiReferenceExpression ?: return
104+
if (parent.references.any { it.isReferenceTo(getMethod) }) {
105+
hasAnyGets = true
106+
} else {
107+
return
108+
}
109+
}
110+
}
111+
if (!hasAnyGets) {
112+
// Don't annoy them if they've just made the parameter
113+
return
114+
}
115+
holder.registerProblem(
116+
originalParam.typeElement ?: originalParam,
117+
"@Local could be captured immutably",
118+
SwitchToImmutableCaptureFix(originalParam)
119+
)
120+
}
121+
122+
// Ignore super delegations in subclasses. super.foo(myLocalRef) has no effect on whether the local can be converted
123+
private fun isDelegationToSuper(ref: PsiElement, paramIndex: Int): Boolean {
124+
val method = ref.findContainingMethod() ?: return false
125+
val superMethod = method.findSuperMethods().firstOrNull { it.containingClass?.isInterface == false }
126+
?: return false
127+
128+
// For some reason ref is sometimes the identifier rather than the reference expression. Get the reference expr
129+
val actualRef = if (ref is PsiReferenceExpression) {
130+
ref
131+
} else {
132+
PsiUtil.skipParenthesizedExprUp(ref.parent) as? PsiReferenceExpression ?: return false
133+
}
134+
val param = PsiUtil.skipParenthesizedExprUp(actualRef)
135+
val paramList = param.parent as? PsiExpressionList ?: return false
136+
val methodCall = paramList.parent as? PsiMethodCallExpression ?: return false
137+
138+
// Check that the method call is a super call
139+
if (!MethodCallUtils.hasSuperQualifier(methodCall)) {
140+
return false
141+
}
142+
143+
// Check that our reference is in the correct parameter index
144+
if (paramList.expressions.getOrNull(paramIndex) != param) {
145+
return false
146+
}
147+
148+
// Check that the super call is referencing the correct super method.
149+
return methodCall.resolveMethod() == superMethod
150+
}
151+
152+
private class SwitchToImmutableCaptureFix(param: PsiParameter) : LocalQuickFixOnPsiElement(param) {
153+
override fun getFamilyName() = "Switch to immutable capture"
154+
override fun getText() = "Switch to immutable capture"
155+
156+
override fun invoke(project: Project, file: PsiFile, startElement: PsiElement, endElement: PsiElement) {
157+
val param = startElement as? PsiParameter ?: return
158+
val method = param.parentOfType<PsiMethod>() ?: return
159+
val paramIndex = method.parameterList.getParameterIndex(param)
160+
val methods = mutableListOf(method)
161+
if (file.isPhysical) {
162+
methods.addAll(OverridingMethodsSearch.search(method))
163+
}
164+
for (impl in methods) {
165+
fixMethod(impl, paramIndex)
166+
}
167+
}
168+
169+
private fun fixMethod(method: PsiMethod, paramIndex: Int) {
170+
val param = method.parameterList.getParameter(paramIndex) ?: return
171+
val paramType = param.type as? PsiClassType ?: return
172+
val innerType = paramType.innerRefType ?: return
173+
val factory = PsiElementFactory.getInstance(method.project)
174+
param.typeElement?.replace(factory.createTypeElement(innerType))
175+
for (ref in ReferencesSearch.search(param)) {
176+
val refExpression = PsiUtil.skipParenthesizedExprUp(ref.element.parent) as? PsiReferenceExpression
177+
?: continue
178+
val call = refExpression.parent as? PsiMethodCallExpression ?: continue
179+
call.replace(ref.element)
180+
}
181+
}
182+
183+
private val PsiClassType.innerRefType: PsiType?
184+
get() =
185+
when (resolve()?.qualifiedName?.substringAfterLast('.')) {
186+
"LocalBooleanRef" -> PsiType.BOOLEAN
187+
"LocalCharRef" -> PsiType.CHAR
188+
"LocalDoubleRef" -> PsiType.DOUBLE
189+
"LocalFloatRef" -> PsiType.FLOAT
190+
"LocalIntRef" -> PsiType.INT
191+
"LocalLongRef" -> PsiType.LONG
192+
"LocalShortRef" -> PsiType.SHORT
193+
"LocalRef" -> parameters.getOrNull(0)
194+
else -> null
195+
}
196+
}
197+
}

‎src/main/kotlin/platform/mixin/util/MixinConstants.kt

+2
Original file line numberDiff line numberDiff line change
@@ -79,5 +79,7 @@ object MixinConstants {
7979
object MixinExtras {
8080
const val OPERATION = "com.llamalad7.mixinextras.injector.wrapoperation.Operation"
8181
const val WRAP_OPERATION = "com.llamalad7.mixinextras.injector.wrapoperation.WrapOperation"
82+
const val LOCAL = "com.llamalad7.mixinextras.sugar.Local"
83+
const val LOCAL_REF_PACKAGE = "com.llamalad7.mixinextras.sugar.ref."
8284
}
8385
}

‎src/main/resources/META-INF/plugin.xml

+8
Original file line numberDiff line numberDiff line change
@@ -980,6 +980,14 @@
980980
level="ERROR"
981981
hasStaticDescription="true"
982982
implementationClass="com.demonwav.mcdev.platform.mixin.inspection.mixinextras.WrongOperationParametersInspection"/>
983+
<localInspection displayName="Unnecessary mutable reference to captured local"
984+
shortName="MixinExtrasUnnecessaryMutableLocal"
985+
groupName="Mixin"
986+
language="JAVA"
987+
enabledByDefault="true"
988+
level="WARNING"
989+
hasStaticDescription="true"
990+
implementationClass="com.demonwav.mcdev.platform.mixin.inspection.mixinextras.UnnecessaryMutableLocalInspection"/>
983991
<!--endregion-->
984992

985993
<!--region Configuration -->

0 commit comments

Comments
 (0)