Skip to content

Commit

Permalink
feature: inspection on redundant dropReturn call (#127)
Browse files Browse the repository at this point in the history
  • Loading branch information
SirYwell authored Sep 6, 2024
1 parent f296ac7 commit 7d5bf51
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ abstract class ProblemEmitter(protected val typeData: TypeData) {

protected fun emitRedundant(element: PsiElement, message: String, vararg quickFixes: LocalQuickFix) {
typeData.reportProblem(element) {
it.registerProblem(element, message, ProblemHighlightType.LIKE_UNUSED_SYMBOL, *quickFixes)
it.registerProblem(element, message, ProblemHighlightType.GENERIC_ERROR_OR_WARNING, *quickFixes)
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,17 @@ package de.sirywell.handlehints.inspection

import com.intellij.codeInspection.LocalQuickFix
import com.intellij.codeInspection.ProblemDescriptor
import com.intellij.lang.jvm.JvmModifier
import com.intellij.openapi.project.Project
import com.intellij.psi.PsiMethodCallExpression
import com.siyeh.ig.PsiReplacementUtil
import com.siyeh.ig.psiutils.CommentTracker
import de.sirywell.handlehints.MethodHandleBundle

/**
* Convert calls of the form Class.method(p0, ..., pn) into p0,
* and calls of the form object.method(p0, ..., pn) into object
*/
class RedundantInvocationFix : LocalQuickFix {
override fun getFamilyName(): String {
return MethodHandleBundle.message("problem.general.invocation.redundant.remove")
Expand All @@ -17,9 +22,14 @@ class RedundantInvocationFix : LocalQuickFix {
val commentTracker = CommentTracker()
when (val psiElement = descriptor.psiElement) {
is PsiMethodCallExpression -> {
val replacement = if (psiElement.resolveMethod()?.hasModifier(JvmModifier.STATIC) ?: return) {

Check warning on line 25 in src/main/kotlin/de/sirywell/handlehints/inspection/RedundantInvocationFix.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Unstable API Usage

'hasModifier([email protected] JvmModifier)' is unstable because its signature references unstable enum 'com.intellij.lang.jvm.JvmModifier' marked with @ApiStatus.Experimental

Check warning on line 25 in src/main/kotlin/de/sirywell/handlehints/inspection/RedundantInvocationFix.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Unstable API Usage

'STATIC' is declared in unstable package 'com.intellij.lang.jvm' marked with @ApiStatus.Experimental

Check warning on line 25 in src/main/kotlin/de/sirywell/handlehints/inspection/RedundantInvocationFix.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Unstable API Usage

'com.intellij.lang.jvm.JvmModifier' is declared in unstable package 'com.intellij.lang.jvm' marked with @ApiStatus.Experimental
psiElement.argumentList.expressions.first()
} else {
psiElement.methodExpression.qualifierExpression
}
PsiReplacementUtil.replaceExpression(
psiElement,
psiElement.methodExpression.qualifierExpression?.text!!,
replacement?.text!!,
commentTracker
)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package de.sirywell.handlehints.mhtype

import com.intellij.psi.PsiExpression
import com.intellij.psi.PsiMethodCallExpression
import com.intellij.psi.PsiTypes
import com.intellij.psi.util.parentOfType
import de.sirywell.handlehints.*
import de.sirywell.handlehints.MethodHandleBundle.message
import de.sirywell.handlehints.dfa.SsaAnalyzer
import de.sirywell.handlehints.dfa.SsaConstruction
import de.sirywell.handlehints.inspection.ProblemEmitter
import de.sirywell.handlehints.inspection.RedundantInvocationFix
import de.sirywell.handlehints.type.*

/**
Expand Down Expand Up @@ -237,11 +240,15 @@ class MethodHandlesMerger(private val ssaAnalyzer: SsaAnalyzer) : ProblemEmitter
// fun dropCoordinates() no VarHandle support, preview

fun dropReturn(targetExpr: PsiExpression, block: SsaConstruction.Block): MethodHandleType {
val target = ssaAnalyzer.methodHandleType(targetExpr, block) ?: bottomType
if (target is CompleteMethodHandleType) {
return target.withReturnType(ExactType.voidType)
val target = ssaAnalyzer.methodHandleType(targetExpr, block) ?: topType
if (target.returnType == ExactType.voidType) {
emitRedundant(
targetExpr.parentOfType<PsiMethodCallExpression>()!!,
message("problem.transforming.dropReturn.redundant"),
RedundantInvocationFix()
)
}
return target
return target.withReturnType(ExactType.voidType)
}

fun explicitCastArguments(
Expand Down
3 changes: 2 additions & 1 deletion src/main/resources/messages/MethodHandleMessages.properties
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,12 @@ problem.merging.tableSwitch.leadingInt=The passed MethodHandle requires a leadin
problem.merging.tableSwitch.notIdentical=The passed MethodHandle type is not compatible with the previous handle(s).
problem.merging.withVarargs.arrayTypeExpected=Last parameter type must be an array but was {0}.
problem.merging.withVarargs.noParameters=MethodHandle does not have any parameters.
problem.transforming.dropReturn.redundant=MethodHandle to drop return from already has return type 'void'.
problem.invocation.returnType.mustBeVoid=MethodHandle returns void but is called in a non-void context.
problem.general.argument.notAPowerOfTwo=Argument must be a power of 2 but was {0}.
problem.general.argument.numericConditionMismatch=Argument must be {0} but is {1}.
problem.general.invocation.redundant=Redundant method invocation.
problem.general.invocation.redundant.remove=Remove redundant method invocation.
problem.general.invocation.redundant.remove=Remove redundant method invocation
problem.foreign.memory.layoutSizeOverflow=Resulting layout byte size overflows.
problem.foreign.memory.layoutMismatch=Layout requires a byte alignment of {0} but is inserted at an offset of {1}.
problem.foreign.memory.alignmentMismatch=Layout must be {0} byte aligned but is {1} byte aligned.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class MethodHandleInspectionsTest : TypeAnalysisTestBase() {

fun testMethodHandlesDropArguments() = doTypeCheckingTest()

fun testMethodHandlesDropReturn() = doTypeCheckingTest()
fun testMethodHandlesDropReturn() = doInspectionAndTypeCheckingTest()

fun testMethodHandlesEmpty() = doTypeCheckingTest()

Expand Down
12 changes: 8 additions & 4 deletions src/test/testData/MethodHandlesDropReturn.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,13 @@
import java.lang.invoke.MethodType;

class MethodHandlesDropReturn {
static {
try {
<info descr="(int)void">MethodHandle mh = <info descr="(int)void">MethodHandles.dropReturn(<info descr="(int)int">MethodHandles.identity(int.class)</info>)</info>;</info>
} catch (Throwable ignored) {}
void droppingReturn(MethodHandle mh10) {
<info descr="(int)int">MethodHandle mh00 = <info descr="(int)int">MethodHandles.identity(int.class)</info>;</info>
// dropping makes sense
<info descr="(int)void">MethodHandle mh01 = <info descr="(int)void">MethodHandles.dropReturn(mh00)</info>;</info>
// dropping doesn't make sense, but type stays!
<info descr="(int)void">MethodHandle mh02 = <warning descr="MethodHandle to drop return from already has return type 'void'."><info descr="(int)void">MethodHandles.dropReturn(mh01)</info></warning>;</info>
// we don't know if dropping makes sense - assume it does
<info descr="⊤">MethodHandle mh11 = <info descr="⊤">MethodHandles.dropReturn(mh10)</info>;</info>
}
}

0 comments on commit 7d5bf51

Please sign in to comment.