Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: inspection on redundant dropReturn call #127

Merged
merged 1 commit into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@

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 @@
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 @@ -224,7 +227,7 @@
): MethodHandleType {
val types = valueTypes.mapToTypes()
val target = ssaAnalyzer.methodHandleType(targetExpr, block) ?: bottomType
val signature = target

Check notice on line 230 in src/main/kotlin/de/sirywell/handlehints/mhtype/MethodHandlesMerger.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Unnecessary local variable

Variable is same as 'target' and can be inlined
if (signature.parameterTypes.compareSize(pos) == PartialOrder.LT) {
return emitOutOfBounds(signature.parameterTypes.sizeOrNull()?.toLong(), targetExpr, pos.toLong(), false)
}
Expand All @@ -237,11 +240,15 @@
// 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 All @@ -264,7 +271,7 @@
// TODO proper handling of bottom/top
if (target !is CompleteMethodHandleType) return target
if (filters.any { it !is CompleteMethodHandleType }) return topType
val targetSignature = target

Check notice on line 274 in src/main/kotlin/de/sirywell/handlehints/mhtype/MethodHandlesMerger.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Unnecessary local variable

Variable is same as 'target' and can be inlined
val targetParameterList = targetSignature.parameterTypes
if (targetParameterList.sizeMatches { pos + filters.size > it } == TriState.YES) return topType
val filtersCast = filters.map { it as CompleteMethodHandleType }
Expand Down Expand Up @@ -374,10 +381,10 @@
return target.withParameterTypes(new)
}

fun iteratedLoop(iterator: MethodHandleType, init: MethodHandleType, body: MethodHandleType): MethodHandleType =

Check warning on line 384 in src/main/kotlin/de/sirywell/handlehints/mhtype/MethodHandlesMerger.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Unused symbol

Function "iteratedLoop" is never used
TODO()

fun loop(clauses: List<List<MethodHandleType>>): MethodHandleType =

Check warning on line 387 in src/main/kotlin/de/sirywell/handlehints/mhtype/MethodHandlesMerger.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Unused symbol

Function "loop" is never used
TODO() // not sure if we can do anything about this

fun permuteArguments(
Expand Down Expand Up @@ -493,14 +500,14 @@
return target
}

fun whileLoop(

Check warning on line 503 in src/main/kotlin/de/sirywell/handlehints/mhtype/MethodHandlesMerger.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Unused symbol

Function "whileLoop" is never used
initExpr: PsiExpression,
bodyExpr: PsiExpression,
predExpr: PsiExpression,
block: SsaConstruction.Block
) = doWhileLoop(initExpr, bodyExpr, predExpr, block) // same type behavior

fun PsiExpression.nonNegativeInt(): Int? {

Check notice on line 510 in src/main/kotlin/de/sirywell/handlehints/mhtype/MethodHandlesMerger.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Class member can have 'private' visibility

Function 'nonNegativeInt' could be private
return this.getConstantOfType<Int>()?.let {
if (it < 0) {
emitProblem<MethodHandleType>(this, message("problem.general.position.invalidIndexNegative", it))
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>
}
}
Loading