From 0887d20d206ff8e3619d647d494cfef35abd1281 Mon Sep 17 00:00:00 2001 From: Vladimir Dolzhenko Date: Wed, 24 Jul 2024 11:55:47 +0200 Subject: [PATCH] [kotlin] Keep comments on applying "Redundant 'if' statement" fix #KTIJ-30443 Fixed GitOrigin-RevId: b0d71560260552dd4056e3247b7bdd01eecb2a0e --- .../impl/base/RedundantIfInspectionBase.kt | 30 +++++++++++++++---- .../tests/K2QuickFixTestGenerated.java | 10 +++++++ .../quickfix/K1QuickFixTestGenerated.java | 10 +++++++ .../bothBranchesHaveComments2.kt.after | 4 +-- .../bothBranchesHaveComments3.kt.after | 4 +-- .../bothBranchesHaveComments6.kt.after | 7 +++-- .../quickfix/redundantIf/comment.kt.after | 2 +- .../redundantIf/commentLiftedUpReturn.kt | 11 +++++++ .../commentLiftedUpReturn.kt.after | 7 +++++ .../multiLineCommentLiftedUpReturn.kt | 12 ++++++++ .../multiLineCommentLiftedUpReturn.kt.after | 8 +++++ 11 files changed, 91 insertions(+), 14 deletions(-) create mode 100644 plugins/kotlin/idea/tests/testData/quickfix/redundantIf/commentLiftedUpReturn.kt create mode 100644 plugins/kotlin/idea/tests/testData/quickfix/redundantIf/commentLiftedUpReturn.kt.after create mode 100644 plugins/kotlin/idea/tests/testData/quickfix/redundantIf/multiLineCommentLiftedUpReturn.kt create mode 100644 plugins/kotlin/idea/tests/testData/quickfix/redundantIf/multiLineCommentLiftedUpReturn.kt.after diff --git a/plugins/kotlin/code-insight/impl-base/src/org/jetbrains/kotlin/idea/codeinsights/impl/base/RedundantIfInspectionBase.kt b/plugins/kotlin/code-insight/impl-base/src/org/jetbrains/kotlin/idea/codeinsights/impl/base/RedundantIfInspectionBase.kt index f2bdcdb354a6..7a33e49a00bb 100644 --- a/plugins/kotlin/code-insight/impl-base/src/org/jetbrains/kotlin/idea/codeinsights/impl/base/RedundantIfInspectionBase.kt +++ b/plugins/kotlin/code-insight/impl-base/src/org/jetbrains/kotlin/idea/codeinsights/impl/base/RedundantIfInspectionBase.kt @@ -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? = returnAfterIf?.let(SmartPointerManager::createPointer) + val returnExpressionAfterIfPointer: SmartPsiElementPointer? = 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 { + val comments = LinkedHashSet() + 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 } diff --git a/plugins/kotlin/code-insight/inspections-k2/tests/test/org/jetbrains/kotlin/idea/k2/quickfix/tests/K2QuickFixTestGenerated.java b/plugins/kotlin/code-insight/inspections-k2/tests/test/org/jetbrains/kotlin/idea/k2/quickfix/tests/K2QuickFixTestGenerated.java index 890a824789f3..eb107eee6adc 100644 --- a/plugins/kotlin/code-insight/inspections-k2/tests/test/org/jetbrains/kotlin/idea/k2/quickfix/tests/K2QuickFixTestGenerated.java +++ b/plugins/kotlin/code-insight/inspections-k2/tests/test/org/jetbrains/kotlin/idea/k2/quickfix/tests/K2QuickFixTestGenerated.java @@ -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"); diff --git a/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/quickfix/K1QuickFixTestGenerated.java b/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/quickfix/K1QuickFixTestGenerated.java index 3e48e03b599c..9864d409dffe 100644 --- a/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/quickfix/K1QuickFixTestGenerated.java +++ b/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/quickfix/K1QuickFixTestGenerated.java @@ -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"); diff --git a/plugins/kotlin/idea/tests/testData/inspectionsLocal/redundantIf/comment/bothBranchesHaveComments2.kt.after b/plugins/kotlin/idea/tests/testData/inspectionsLocal/redundantIf/comment/bothBranchesHaveComments2.kt.after index b3516087b233..67d2122d8d2b 100644 --- a/plugins/kotlin/idea/tests/testData/inspectionsLocal/redundantIf/comment/bothBranchesHaveComments2.kt.after +++ b/plugins/kotlin/idea/tests/testData/inspectionsLocal/redundantIf/comment/bothBranchesHaveComments2.kt.after @@ -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 diff --git a/plugins/kotlin/idea/tests/testData/inspectionsLocal/redundantIf/comment/bothBranchesHaveComments3.kt.after b/plugins/kotlin/idea/tests/testData/inspectionsLocal/redundantIf/comment/bothBranchesHaveComments3.kt.after index 0902ea7320b3..155e661e2df9 100644 --- a/plugins/kotlin/idea/tests/testData/inspectionsLocal/redundantIf/comment/bothBranchesHaveComments3.kt.after +++ b/plugins/kotlin/idea/tests/testData/inspectionsLocal/redundantIf/comment/bothBranchesHaveComments3.kt.after @@ -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 } diff --git a/plugins/kotlin/idea/tests/testData/inspectionsLocal/redundantIf/comment/bothBranchesHaveComments6.kt.after b/plugins/kotlin/idea/tests/testData/inspectionsLocal/redundantIf/comment/bothBranchesHaveComments6.kt.after index 14b95ec8552d..94a0f6ae1b54 100644 --- a/plugins/kotlin/idea/tests/testData/inspectionsLocal/redundantIf/comment/bothBranchesHaveComments6.kt.after +++ b/plugins/kotlin/idea/tests/testData/inspectionsLocal/redundantIf/comment/bothBranchesHaveComments6.kt.after @@ -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 diff --git a/plugins/kotlin/idea/tests/testData/quickfix/redundantIf/comment.kt.after b/plugins/kotlin/idea/tests/testData/quickfix/redundantIf/comment.kt.after index 073ad7dacfa9..902d7895025d 100644 --- a/plugins/kotlin/idea/tests/testData/quickfix/redundantIf/comment.kt.after +++ b/plugins/kotlin/idea/tests/testData/quickfix/redundantIf/comment.kt.after @@ -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 \ No newline at end of file diff --git a/plugins/kotlin/idea/tests/testData/quickfix/redundantIf/commentLiftedUpReturn.kt b/plugins/kotlin/idea/tests/testData/quickfix/redundantIf/commentLiftedUpReturn.kt new file mode 100644 index 000000000000..fc2b103a6fa2 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/quickfix/redundantIf/commentLiftedUpReturn.kt @@ -0,0 +1,11 @@ +// "Remove redundant 'if' statement" "true" +fun test(a: Boolean, b: Boolean): Boolean { + return 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 \ No newline at end of file diff --git a/plugins/kotlin/idea/tests/testData/quickfix/redundantIf/commentLiftedUpReturn.kt.after b/plugins/kotlin/idea/tests/testData/quickfix/redundantIf/commentLiftedUpReturn.kt.after new file mode 100644 index 000000000000..19d519256d02 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/quickfix/redundantIf/commentLiftedUpReturn.kt.after @@ -0,0 +1,7 @@ +// "Remove redundant 'if' statement" "true" +fun test(a: Boolean, b: Boolean): Boolean { + return !(!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 \ No newline at end of file diff --git a/plugins/kotlin/idea/tests/testData/quickfix/redundantIf/multiLineCommentLiftedUpReturn.kt b/plugins/kotlin/idea/tests/testData/quickfix/redundantIf/multiLineCommentLiftedUpReturn.kt new file mode 100644 index 000000000000..a5e377be0b8d --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/quickfix/redundantIf/multiLineCommentLiftedUpReturn.kt @@ -0,0 +1,12 @@ +// "Remove redundant 'if' statement" "true" +fun test(a: Boolean, b: Boolean): Boolean { + return 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 \ No newline at end of file diff --git a/plugins/kotlin/idea/tests/testData/quickfix/redundantIf/multiLineCommentLiftedUpReturn.kt.after b/plugins/kotlin/idea/tests/testData/quickfix/redundantIf/multiLineCommentLiftedUpReturn.kt.after new file mode 100644 index 000000000000..12e1d2e22d4b --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/quickfix/redundantIf/multiLineCommentLiftedUpReturn.kt.after @@ -0,0 +1,8 @@ +// "Remove redundant 'if' statement" "true" +fun test(a: Boolean, b: Boolean): Boolean { + return !(!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 \ No newline at end of file