From e7575599eb838595bf30166d10893cec6ece79bc Mon Sep 17 00:00:00 2001 From: shiraji Date: Sun, 30 Oct 2016 06:27:47 +0900 Subject: [PATCH] Correct handling of "Redundant if" for negation case #KT-14575 Fixed --- .../idea/inspections/RedundantIfInspection.kt | 84 +++++++++++-------- idea/testData/quickfix/redundantIf/negate.kt | 9 ++ .../quickfix/redundantIf/negate.kt.after | 4 + .../idea/quickfix/QuickFixTestGenerated.java | 6 ++ 4 files changed, 70 insertions(+), 33 deletions(-) create mode 100644 idea/testData/quickfix/redundantIf/negate.kt create mode 100644 idea/testData/quickfix/redundantIf/negate.kt.after diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantIfInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantIfInspection.kt index 82c8de26555..30c91a74404 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantIfInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantIfInspection.kt @@ -19,6 +19,7 @@ package org.jetbrains.kotlin.idea.inspections import com.intellij.codeInspection.* import com.intellij.openapi.project.Project import com.intellij.psi.PsiElementVisitor +import org.jetbrains.kotlin.idea.intentions.negate import org.jetbrains.kotlin.psi.* class RedundantIfInspection : AbstractKotlinInspection(), CleanupLocalInspectionTool { @@ -27,50 +28,67 @@ class RedundantIfInspection : AbstractKotlinInspection(), CleanupLocalInspection return object : KtVisitorVoid() { override fun visitIfExpression(expression: KtIfExpression) { super.visitIfExpression(expression) + if (expression.condition == null) return + val type = RedundantType.of(expression) + if (type == RedundantType.NONE) return + holder.registerProblem(expression, + "Redundant 'if' statement", + ProblemHighlightType.WEAK_WARNING, + RemoveRedundantIf(type)) + } + } + } - if (expression.condition != null && isRedundant(expression)) { - holder.registerProblem(expression, - "Redundant 'if' statement", - ProblemHighlightType.LIKE_UNUSED_SYMBOL, - RemoveRedundantIf) + private enum class RedundantType { + NONE, + THEN_TRUE, + ELSE_TRUE; + + companion object { + internal fun of(expression: KtIfExpression): RedundantType { + val thenReturn = getReturnedExpression(expression.then) ?: return NONE + val elseReturn = getReturnedExpression(expression.`else`) ?: return NONE + + return if (KtPsiUtil.isTrueConstant(thenReturn) && KtPsiUtil.isFalseConstant(elseReturn)) { + THEN_TRUE + } + else if (KtPsiUtil.isFalseConstant(thenReturn) && KtPsiUtil.isTrueConstant(elseReturn)) { + ELSE_TRUE + } + else { + NONE + } + } + + private fun getReturnedExpression(expression: KtExpression?) : KtExpression? { + return when (expression) { + is KtReturnExpression -> { + expression.returnedExpression + } + is KtBlockExpression -> { + val statement = expression.statements.singleOrNull() as? KtReturnExpression ?: return null + statement.returnedExpression + } + else -> { + null + } } } } } - private fun isRedundant(expression: KtIfExpression): Boolean { - val thenReturn = getReturnedExpression(expression.then) ?: return false - val elseReturn = getReturnedExpression(expression.`else`) ?: return false - - if (KtPsiUtil.isTrueConstant(thenReturn) && KtPsiUtil.isFalseConstant(elseReturn)) { - return true - } - - if (KtPsiUtil.isFalseConstant(thenReturn) && KtPsiUtil.isTrueConstant(elseReturn)) { - return true - } - - return false - } - - private fun getReturnedExpression(expression: KtExpression?) : KtExpression? { - when(expression) { - is KtReturnExpression -> return expression.returnedExpression - is KtBlockExpression -> { - val statement = expression.statements.singleOrNull() as? KtReturnExpression ?: return null - return statement.returnedExpression - } - else -> return null - } - } - - private object RemoveRedundantIf : LocalQuickFix { + private class RemoveRedundantIf(private val redundantType: RedundantType) : LocalQuickFix { override fun getName() = "Remove redundant 'if' statement" override fun getFamilyName() = name override fun applyFix(project: Project, descriptor: ProblemDescriptor) { val element = descriptor.psiElement as KtIfExpression - element.replace(KtPsiFactory(element).createExpressionByPattern("return $0", element.condition!!.text)); + val condition = when (redundantType) { + RedundantType.NONE -> return + RedundantType.THEN_TRUE -> element.condition!! + RedundantType.ELSE_TRUE -> element.condition!!.negate() + } + element.replace(KtPsiFactory(element).createExpressionByPattern("return $0", condition.text)) } } diff --git a/idea/testData/quickfix/redundantIf/negate.kt b/idea/testData/quickfix/redundantIf/negate.kt new file mode 100644 index 00000000000..23cf67a9ef6 --- /dev/null +++ b/idea/testData/quickfix/redundantIf/negate.kt @@ -0,0 +1,9 @@ +// "Remove redundant 'if' statement" "true" +fun bar(value: Int): Boolean { + if (value % 2 == 0) { + return false + } + else { + return true + } +} \ No newline at end of file diff --git a/idea/testData/quickfix/redundantIf/negate.kt.after b/idea/testData/quickfix/redundantIf/negate.kt.after new file mode 100644 index 00000000000..e88dda9b4ad --- /dev/null +++ b/idea/testData/quickfix/redundantIf/negate.kt.after @@ -0,0 +1,4 @@ +// "Remove redundant 'if' statement" "true" +fun bar(value: Int): Boolean { + return value % 2 != 0 +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java index 6dd8a02d5e9..c11eb62d347 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java @@ -7148,6 +7148,12 @@ public class QuickFixTestGenerated extends AbstractQuickFixTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/quickfix/redundantIf"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), true); } + @TestMetadata("negate.kt") + public void testNegate() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/redundantIf/negate.kt"); + doTest(fileName); + } + @TestMetadata("simple.kt") public void testSimple() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/redundantIf/simple.kt");