[kotlin] Keep comments on applying "Redundant 'if' statement" fix

#KTIJ-30443 Fixed

GitOrigin-RevId: b0d71560260552dd4056e3247b7bdd01eecb2a0e
This commit is contained in:
Vladimir Dolzhenko
2024-07-24 22:21:32 +00:00
committed by intellij-monorepo-bot
parent fc832724ee
commit 0887d20d20
11 changed files with 91 additions and 14 deletions
@@ -15,10 +15,10 @@ import org.jetbrains.kotlin.idea.base.psi.getLineNumber
import org.jetbrains.kotlin.idea.base.resources.KotlinBundle
import org.jetbrains.kotlin.idea.codeinsight.api.classic.inspections.AbstractKotlinInspection
import org.jetbrains.kotlin.idea.codeinsight.utils.negate
import org.jetbrains.kotlin.idea.util.CommentSaver
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.psi.*
import org.jetbrains.kotlin.psi.psiUtil.*
import kotlin.collections.dropLastWhile
/**
* A parent class for K1 and K2 RedundantIfInspection.
@@ -191,13 +191,13 @@ abstract class RedundantIfInspectionBase : AbstractKotlinInspection(), CleanupLo
returnAfterIf: KtExpression?,
private val mayChangeSemantics: Boolean,
) : LocalQuickFix {
val returnExpressionAfterIf: SmartPsiElementPointer<KtExpression>? = returnAfterIf?.let(SmartPointerManager::createPointer)
val returnExpressionAfterIfPointer: SmartPsiElementPointer<KtExpression>? = returnAfterIf?.let(SmartPointerManager::createPointer)
override fun getName(): String =
if (mayChangeSemantics) KotlinBundle.message("remove.redundant.if.may.change.semantics.with.floating.point.types")
else KotlinBundle.message("remove.redundant.if.text")
override fun getFamilyName() = name
override fun getFamilyName(): String = name
override fun startInWriteAction(): Boolean = false
@@ -218,21 +218,39 @@ abstract class RedundantIfInspectionBase : AbstractKotlinInspection(), CleanupLo
else -> condition
}
val commentSaver = CommentSaver(element)
val comments = element.comments().map {
// create a copy as all branches will be dropped
val text = it.text
if (it is PsiWhiteSpace) factory.createWhiteSpace(text) else factory.createComment(text)
}
runWriteAction {
/**
* This is the case that we used the next expression of the if expression as the else expression.
* See the code and comment in [RedundancyType.of].
*/
returnExpressionAfterIf?.element?.let {
val returnExpressionAfterIf = returnExpressionAfterIfPointer?.element
returnExpressionAfterIf?.let {
it.parent.deleteChildRange(it.prevSibling as? PsiWhiteSpace ?: it, it)
}
val replaced = element.replace(newExpressionOnlyWithCondition)
commentSaver.restore(replaced)
comments.reversed().forEach { replaced.parent.addAfter(it, replaced) }
}
}
private fun PsiElement.comments(): List<PsiElement> {
val comments = LinkedHashSet<PsiElement>()
accept(object : PsiRecursiveElementVisitor() {
override fun visitComment(comment: PsiComment) {
(comment.prevSibling as? PsiWhiteSpace)?.let { comments.add(it) }
comments.add(comment)
(comment.nextSibling as? PsiWhiteSpace)?.let { comments.add(it) }
}
})
return comments.toList().dropLastWhile { it is PsiWhiteSpace }
}
private fun negate(expression: KtExpression?): KtExpression? {
if (expression == null) return null
invertEmptinessCheck(expression)?.let { return it }
@@ -52,6 +52,11 @@ public abstract class K2QuickFixTestGenerated extends AbstractK2QuickFixTest {
runTest("../../../idea/tests/testData/quickfix/redundantIf/comment.kt");
}
@TestMetadata("commentLiftedUpReturn.kt")
public void testCommentLiftedUpReturn() throws Exception {
runTest("../../../idea/tests/testData/quickfix/redundantIf/commentLiftedUpReturn.kt");
}
@TestMetadata("expression.kt")
public void testExpression() throws Exception {
runTest("../../../idea/tests/testData/quickfix/redundantIf/expression.kt");
@@ -67,6 +72,11 @@ public abstract class K2QuickFixTestGenerated extends AbstractK2QuickFixTest {
runTest("../../../idea/tests/testData/quickfix/redundantIf/labeledReturn.kt");
}
@TestMetadata("multiLineCommentLiftedUpReturn.kt")
public void testMultiLineCommentLiftedUpReturn() throws Exception {
runTest("../../../idea/tests/testData/quickfix/redundantIf/multiLineCommentLiftedUpReturn.kt");
}
@TestMetadata("negate.kt")
public void testNegate() throws Exception {
runTest("../../../idea/tests/testData/quickfix/redundantIf/negate.kt");
@@ -13529,6 +13529,11 @@ public abstract class K1QuickFixTestGenerated extends AbstractK1QuickFixTest {
runTest("testData/quickfix/redundantIf/comment.kt");
}
@TestMetadata("commentLiftedUpReturn.kt")
public void testCommentLiftedUpReturn() throws Exception {
runTest("testData/quickfix/redundantIf/commentLiftedUpReturn.kt");
}
@TestMetadata("expression.kt")
public void testExpression() throws Exception {
runTest("testData/quickfix/redundantIf/expression.kt");
@@ -13544,6 +13549,11 @@ public abstract class K1QuickFixTestGenerated extends AbstractK1QuickFixTest {
runTest("testData/quickfix/redundantIf/labeledReturn.kt");
}
@TestMetadata("multiLineCommentLiftedUpReturn.kt")
public void testMultiLineCommentLiftedUpReturn() throws Exception {
runTest("testData/quickfix/redundantIf/multiLineCommentLiftedUpReturn.kt");
}
@TestMetadata("negate.kt")
public void testNegate() throws Exception {
runTest("testData/quickfix/redundantIf/negate.kt");
@@ -1,7 +1,7 @@
// HIGHLIGHT: INFORMATION
fun foo(): Boolean {
// comment explaining the `false` case
return !someComplexCondition() // comment explaining the `true` case
return !someComplexCondition() // comment explaining the `false` case
// comment explaining the `true` case
}
fun someComplexCondition(): Boolean = true
@@ -1,8 +1,8 @@
// HIGHLIGHT: INFORMATION
fun foo(): Boolean {
// comment explaining the `false` case
// comment explaining the `false` case
return !someComplexCondition()
// comment explaining the `false` case
// comment explaining the `false` case
// comment explaining the `true` case
}
@@ -1,7 +1,8 @@
// HIGHLIGHT: INFORMATION
fun foo(): Boolean {// comment explaining the `true` case
// comment explaining the `false` case
return !someComplexCondition()
fun foo(): Boolean {
return !someComplexCondition() // comment explaining the `false` case
// comment explaining the `true` case
}
fun someComplexCondition(): Boolean = true
@@ -1,8 +1,8 @@
// "Remove redundant 'if' statement" "true"
fun test(a: Boolean, b: Boolean): Boolean {
return !(!a && b)
// comment1
// comment2
return !(!a && b)
}
// FUS_K2_QUICKFIX_NAME: org.jetbrains.kotlin.idea.codeinsights.impl.base.RedundantIfInspectionBase$RemoveRedundantIf
// FUS_QUICKFIX_NAME: org.jetbrains.kotlin.idea.codeinsights.impl.base.RedundantIfInspectionBase$RemoveRedundantIf
@@ -0,0 +1,11 @@
// "Remove redundant 'if' statement" "true"
fun test(a: Boolean, b: Boolean): Boolean {
return <caret>if (!a && b) {
// comment
false
} else {
true
}
}
// FUS_K2_QUICKFIX_NAME: org.jetbrains.kotlin.idea.codeinsights.impl.base.RedundantIfInspectionBase$RemoveRedundantIf
// FUS_QUICKFIX_NAME: org.jetbrains.kotlin.idea.codeinsights.impl.base.RedundantIfInspectionBase$RemoveRedundantIf
@@ -0,0 +1,7 @@
// "Remove redundant 'if' statement" "true"
fun test(a: Boolean, b: Boolean): Boolean {
return <caret>!(!a && b)
// comment
}
// FUS_K2_QUICKFIX_NAME: org.jetbrains.kotlin.idea.codeinsights.impl.base.RedundantIfInspectionBase$RemoveRedundantIf
// FUS_QUICKFIX_NAME: org.jetbrains.kotlin.idea.codeinsights.impl.base.RedundantIfInspectionBase$RemoveRedundantIf
@@ -0,0 +1,12 @@
// "Remove redundant 'if' statement" "true"
fun test(a: Boolean, b: Boolean): Boolean {
return <caret>if (!a && b) {
// comment 1
// comment 2
false
} else {
true
}
}
// FUS_K2_QUICKFIX_NAME: org.jetbrains.kotlin.idea.codeinsights.impl.base.RedundantIfInspectionBase$RemoveRedundantIf
// FUS_QUICKFIX_NAME: org.jetbrains.kotlin.idea.codeinsights.impl.base.RedundantIfInspectionBase$RemoveRedundantIf
@@ -0,0 +1,8 @@
// "Remove redundant 'if' statement" "true"
fun test(a: Boolean, b: Boolean): Boolean {
return <caret>!(!a && b)
// comment 1
// comment 2
}
// FUS_K2_QUICKFIX_NAME: org.jetbrains.kotlin.idea.codeinsights.impl.base.RedundantIfInspectionBase$RemoveRedundantIf
// FUS_QUICKFIX_NAME: org.jetbrains.kotlin.idea.codeinsights.impl.base.RedundantIfInspectionBase$RemoveRedundantIf