diff --git a/plugins/kotlin/code-insight/inspections-k2/resources/kotlin.code-insight.inspections-k2.xml b/plugins/kotlin/code-insight/inspections-k2/resources/kotlin.code-insight.inspections-k2.xml index a39832c3fbd8..2d404069142c 100644 --- a/plugins/kotlin/code-insight/inspections-k2/resources/kotlin.code-insight.inspections-k2.xml +++ b/plugins/kotlin/code-insight/inspections-k2/resources/kotlin.code-insight.inspections-k2.xml @@ -191,5 +191,12 @@ language="kotlin" key="inspection.enum.values.method.soft.deprecate.migration.display.name" bundle="messages.KotlinBundle"/> + diff --git a/plugins/kotlin/code-insight/inspections-k2/src/org/jetbrains/kotlin/idea/k2/codeinsight/inspections/expressions/WhenWithOnlyElseInspection.kt b/plugins/kotlin/code-insight/inspections-k2/src/org/jetbrains/kotlin/idea/k2/codeinsight/inspections/expressions/WhenWithOnlyElseInspection.kt new file mode 100644 index 000000000000..47b3e201e8c2 --- /dev/null +++ b/plugins/kotlin/code-insight/inspections-k2/src/org/jetbrains/kotlin/idea/k2/codeinsight/inspections/expressions/WhenWithOnlyElseInspection.kt @@ -0,0 +1,208 @@ +// Copyright 2000-2022 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package org.jetbrains.kotlin.idea.k2.codeinsight.inspections.expressions + +import com.intellij.openapi.util.TextRange +import com.intellij.psi.PsiElement +import com.intellij.psi.search.searches.ReferencesSearch +import com.intellij.refactoring.suggested.startOffset +import org.jetbrains.kotlin.analysis.api.KtAnalysisSession +import org.jetbrains.kotlin.analysis.api.components.KtConstantEvaluationMode +import org.jetbrains.kotlin.idea.base.analysis.api.utils.shortenReferencesInRange +import org.jetbrains.kotlin.idea.base.resources.KotlinBundle +import org.jetbrains.kotlin.idea.codeinsight.api.applicators.* +import org.jetbrains.kotlin.idea.codeinsights.impl.base.applicators.ApplicabilityRanges +import org.jetbrains.kotlin.psi.* + +/** + * This inspection finds and replaces When expressions containing only a single + * `else` branch by the body of that branch, accounting for the presence of a + * subject variable. In general it rewrites: + * ```kotlin + * when (val x = e1) { + * else -> e2 + * } + * ``` + * into: + * ``` + * run { + * val x = e1 + * e2 + * } + * ``` + * contingent on a few complications. See Steps 3.1 and 3.2 below. + */ +internal class WhenWithOnlyElseInspection: AbstractKotlinApplicatorBasedInspection(KtWhenExpression::class) { + + data class WhenSubjectVariableInfo( + val subjectVariable: KtProperty, + val initializer: KtExpression?, + val isInitializerPure: Boolean + ) + + class Input( + val isWhenUsedAsExpression: Boolean, + val elseExpression: KtExpression, + val subjectVariableInfo: WhenSubjectVariableInfo? + ) : KotlinApplicatorInput + + override fun getApplicabilityRange(): KotlinApplicabilityRange = ApplicabilityRanges.SELF + + /** + * STEP 2: + * Gather the necessary semantic information for the transformation: + * - whether the when expression itself is used as an expression + * - for the subject variable, if present, whether the initializer is pure. + */ + override fun getInputProvider(): KotlinApplicatorInputProvider = inputProvider { whenExpression -> + val singleEntry = whenExpression.entries.singleOrNull() ?: return@inputProvider null + val elseExpression = singleEntry.takeIf { it.isElse }?.expression ?: return@inputProvider null + val isWhenUsedAsExpression = whenExpression.isUsedAsExpression() + val subjectVariableInfo = whenExpression.subjectVariable?.let { + val initializer = it.initializer + WhenSubjectVariableInfo( + subjectVariable = it, + initializer = initializer, + isInitializerPure = initializer?.isPure() == true + ) + } + + Input(isWhenUsedAsExpression, elseExpression, subjectVariableInfo) + } + + /** + * Over-approximates if the expression has side-effects. + * @return `true` if `this` is _definitely_ pure. + */ + context(KtAnalysisSession) + private fun KtExpression.isPure(): Boolean = when (this) { + is KtStringTemplateExpression -> !hasInterpolation() + is KtConstantExpression -> true + is KtIsExpression -> true + is KtThisExpression -> true + is KtObjectLiteralExpression -> true + else -> + evaluate(KtConstantEvaluationMode.CONSTANT_EXPRESSION_EVALUATION) != null + } + + override fun getApplicator(): KotlinApplicator = applicator { + familyAndActionName(KotlinBundle.lazyMessage("inspection.when.with.only.else.display.name")) + + /** + * STEP 1: + * Discard when expressions that are not of the form + *```kotlin + * when (...) { else -> ... } + * ``` + */ + isApplicableByPsi { whenExpression -> + whenExpression.entries.singleOrNull()?.isElse == true + } + + applyToWithEditorRequired { whenExpression, input, _, editor -> + val factory = KtPsiFactory(whenExpression) + val newCaretPosition = whenExpression.startOffset + + /** + * STEP 3.1: + * Rewrite the branch body to include the subject variable, if + * present: + * + * - if the variable is unused and the initializer is pure, elide + * the variable entirely + * - if the variable is only used once and the initializer is + * pure, inline the variable in the branch body + * - otherwise, preface the branch body with the subject variable + * declaration and wrap it in a call to `kotlin.run`. + * + * Additionally, track if _we_ inserted a call to `kotlin.run`. In + * that case, we will attempt to shorten the reference to `run`, + * once we have done the actual transformation in Step 3.2 below. + * This preserves explicit, user-supplied calls to `kotlin.run`. + */ + val (rewrittenBranch, insertedCallToKotlinDotRun) = if (input.subjectVariableInfo?.initializer == null) { + input.elseExpression to false + } else { + val info = input.subjectVariableInfo + val isInitializerPure = info.isInitializerPure + val references = ReferencesSearch.search(info.subjectVariable).findAll() + val occurrences = references.size + when { + occurrences == 0 && isInitializerPure -> + input.elseExpression to false + + occurrences == 1 && isInitializerPure -> { + references.single().element.replace(info.initializer!!) + input.elseExpression to false + } + + else -> { + val branch = input.elseExpression + if (branch is KtBlockExpression) { + branch.apply { + val subjectVariable = addBefore(info.subjectVariable, statements.firstOrNull()) + addAfter(factory.createNewLine(), subjectVariable) + } + factory.createExpressionByPattern("kotlin.run $0", branch.text) to true + } else { + factory.createExpressionByPattern("kotlin.run { $0\n$1 }", info.subjectVariable.text, branch.text) to true + } + } + } + } + + /** + * Step 3.2: + * Replace the when expression with the (possibly rewritten) branch: + * + * - If the branch is a single expression, either originally, or + * due to the rewrite to a `kotlin.run { ... }` by step 3.1, replace + * the when by the expression. + * - Otherwise, the else branch is of the form `else -> { ... }` + * and we have to decide + * + If the when is used as an expression, replace the when by + * a call to `kotlin.run { ... }` + * + Otherwise, inline the statements of `{ ... }` in place of + * the when expression. + * + * Generated calls to `kotlin.run` are shortened to `run`, if possible. + */ + when { + rewrittenBranch !is KtBlockExpression -> + whenExpression.replace(rewrittenBranch).also { + if (insertedCallToKotlinDotRun) it.shortenKotlinDotRun() + } + + input.isWhenUsedAsExpression -> + whenExpression.replace(factory.createExpressionByPattern("kotlin.run $0", rewrittenBranch.text)).also { + it.shortenKotlinDotRun() + } + + else -> { + val firstChildSibling = rewrittenBranch.firstChild.nextSibling + val lastChild = rewrittenBranch.lastChild + whenExpression.parent.addRangeAfter(firstChildSibling, lastChild.prevSibling, whenExpression).also { + whenExpression.delete() + } + } + } + + editor.caretModel.moveToOffset(newCaretPosition) + } + } + + /** + * Shortens calls to `kotlin.run`, as generated by the transformation in + * this inspection, to `run` if possible, cf. the logic of + * [shortenReferencesInRange]. + */ + private fun PsiElement.shortenKotlinDotRun() { + if (this !is KtDotQualifiedExpression) return + shortenReferencesInRange( + this.containingKtFile, + TextRange( + this.startOffset, + this.startOffset + "kotlin.run".length + ) + ) + } +} diff --git a/plugins/kotlin/code-insight/inspections-k2/tests/test/org/jetbrains/kotlin/idea/k2/inspections/tests/K2LocalInspectionTestGenerated.java b/plugins/kotlin/code-insight/inspections-k2/tests/test/org/jetbrains/kotlin/idea/k2/inspections/tests/K2LocalInspectionTestGenerated.java index 96e39f1efe18..6b353812677e 100644 --- a/plugins/kotlin/code-insight/inspections-k2/tests/test/org/jetbrains/kotlin/idea/k2/inspections/tests/K2LocalInspectionTestGenerated.java +++ b/plugins/kotlin/code-insight/inspections-k2/tests/test/org/jetbrains/kotlin/idea/k2/inspections/tests/K2LocalInspectionTestGenerated.java @@ -1391,6 +1391,189 @@ public abstract class K2LocalInspectionTestGenerated extends AbstractK2LocalInsp } } + @RunWith(JUnit3RunnerWithInners.class) + @TestMetadata("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse") + public abstract static class WhenWithOnlyElse extends AbstractK2LocalInspectionTest { + @RunWith(JUnit3RunnerWithInners.class) + @TestMetadata("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable") + public abstract static class SubjectVariable extends AbstractK2LocalInspectionTest { + @RunWith(JUnit3RunnerWithInners.class) + @TestMetadata("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/blockElse") + public static class BlockElse extends AbstractK2LocalInspectionTest { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTest, this, testDataFilePath); + } + + @TestMetadata("multiReference.kt") + public void testMultiReference() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/blockElse/multiReference.kt"); + } + + @TestMetadata("noReference.kt") + public void testNoReference() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/blockElse/noReference.kt"); + } + + @TestMetadata("noReference2.kt") + public void testNoReference2() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/blockElse/noReference2.kt"); + } + + @TestMetadata("singleReference.kt") + public void testSingleReference() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/blockElse/singleReference.kt"); + } + } + + @RunWith(JUnit3RunnerWithInners.class) + @TestMetadata("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/blockElseUsedAsExpression") + public static class BlockElseUsedAsExpression extends AbstractK2LocalInspectionTest { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTest, this, testDataFilePath); + } + + @TestMetadata("multiReference.kt") + public void testMultiReference() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/blockElseUsedAsExpression/multiReference.kt"); + } + + @TestMetadata("noReference.kt") + public void testNoReference() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/blockElseUsedAsExpression/noReference.kt"); + } + + @TestMetadata("noReference2.kt") + public void testNoReference2() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/blockElseUsedAsExpression/noReference2.kt"); + } + + @TestMetadata("singleReference.kt") + public void testSingleReference() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/blockElseUsedAsExpression/singleReference.kt"); + } + } + + @RunWith(JUnit3RunnerWithInners.class) + @TestMetadata("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/singleElse") + public static class SingleElse extends AbstractK2LocalInspectionTest { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTest, this, testDataFilePath); + } + + @TestMetadata("multiReference.kt") + public void testMultiReference() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/singleElse/multiReference.kt"); + } + + @TestMetadata("multiReferenceUsedAsExpression.kt") + public void testMultiReferenceUsedAsExpression() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/singleElse/multiReferenceUsedAsExpression.kt"); + } + + @TestMetadata("noReference.kt") + public void testNoReference() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/singleElse/noReference.kt"); + } + + @TestMetadata("noReference2.kt") + public void testNoReference2() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/singleElse/noReference2.kt"); + } + + @TestMetadata("noReferenceUsedAsExpression.kt") + public void testNoReferenceUsedAsExpression() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/singleElse/noReferenceUsedAsExpression.kt"); + } + + @TestMetadata("noReferenceUsedAsExpression2.kt") + public void testNoReferenceUsedAsExpression2() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/singleElse/noReferenceUsedAsExpression2.kt"); + } + + @TestMetadata("singleReference.kt") + public void testSingleReference() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/singleElse/singleReference.kt"); + } + + @TestMetadata("singleReferenceUsedAsExpression.kt") + public void testSingleReferenceUsedAsExpression() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/singleElse/singleReferenceUsedAsExpression.kt"); + } + } + + @RunWith(JUnit3RunnerWithInners.class) + @TestMetadata("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable") + public static class Uncategorized extends AbstractK2LocalInspectionTest { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTest, this, testDataFilePath); + } + + @TestMetadata("hasAnnotation.kt") + public void testHasAnnotation() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/hasAnnotation.kt"); + } + + @TestMetadata("hasSameNameVariable.kt") + public void testHasSameNameVariable() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/hasSameNameVariable.kt"); + } + + @TestMetadata("hasSameNameVariable2.kt") + public void testHasSameNameVariable2() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/hasSameNameVariable2.kt"); + } + + @TestMetadata("hasSameNameVariable3.kt") + public void testHasSameNameVariable3() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/hasSameNameVariable3.kt"); + } + + @TestMetadata("hasSameNameVariable4.kt") + public void testHasSameNameVariable4() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/subjectVariable/hasSameNameVariable4.kt"); + } + } + } + + @RunWith(JUnit3RunnerWithInners.class) + @TestMetadata("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse") + public static class Uncategorized extends AbstractK2LocalInspectionTest { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTest, this, testDataFilePath); + } + + @TestMetadata("complexExpression.kt") + public void testComplexExpression() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/complexExpression.kt"); + } + + @TestMetadata("emptyElseBranchBlock.kt") + public void testEmptyElseBranchBlock() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/emptyElseBranchBlock.kt"); + } + + @TestMetadata("hasOtherBranches.kt") + public void testHasOtherBranches() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/hasOtherBranches.kt"); + } + + @TestMetadata("notExpression.kt") + public void testNotExpression() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/notExpression.kt"); + } + + @TestMetadata("simpleExpression.kt") + public void testSimpleExpression() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/simpleExpression.kt"); + } + + @TestMetadata("userKotlinDotRun.kt") + public void testUserKotlinDotRun() throws Exception { + runTest("../../../idea/tests/testData/inspectionsLocal/whenWithOnlyElse/userKotlinDotRun.kt"); + } + } + } + @RunWith(JUnit3RunnerWithInners.class) @TestMetadata("testData/inspectionsLocal") public abstract static class InspectionsLocal extends AbstractK2LocalInspectionTest { diff --git a/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java b/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java index a61ad9af2b4d..f75d76607846 100644 --- a/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -17141,6 +17141,11 @@ public abstract class LocalInspectionTestGenerated extends AbstractLocalInspecti runTest("testData/inspectionsLocal/whenWithOnlyElse/complexExpression.kt"); } + @TestMetadata("emptyElseBranchBlock.kt") + public void testEmptyElseBranchBlock() throws Exception { + runTest("testData/inspectionsLocal/whenWithOnlyElse/emptyElseBranchBlock.kt"); + } + @TestMetadata("hasOtherBranches.kt") public void testHasOtherBranches() throws Exception { runTest("testData/inspectionsLocal/whenWithOnlyElse/hasOtherBranches.kt"); @@ -17155,6 +17160,11 @@ public abstract class LocalInspectionTestGenerated extends AbstractLocalInspecti public void testSimpleExpression() throws Exception { runTest("testData/inspectionsLocal/whenWithOnlyElse/simpleExpression.kt"); } + + @TestMetadata("userKotlinDotRun.kt") + public void testUserKotlinDotRun() throws Exception { + runTest("testData/inspectionsLocal/whenWithOnlyElse/userKotlinDotRun.kt"); + } } } } diff --git a/plugins/kotlin/idea/tests/testData/inspectionsLocal/whenWithOnlyElse/.k2Inspection b/plugins/kotlin/idea/tests/testData/inspectionsLocal/whenWithOnlyElse/.k2Inspection new file mode 100644 index 000000000000..774702de7c01 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/inspectionsLocal/whenWithOnlyElse/.k2Inspection @@ -0,0 +1 @@ +org.jetbrains.kotlin.idea.k2.codeinsight.inspections.expressions.WhenWithOnlyElseInspection \ No newline at end of file diff --git a/plugins/kotlin/idea/tests/testData/inspectionsLocal/whenWithOnlyElse/emptyElseBranchBlock.kt b/plugins/kotlin/idea/tests/testData/inspectionsLocal/whenWithOnlyElse/emptyElseBranchBlock.kt new file mode 100644 index 000000000000..bd5d854c7967 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/inspectionsLocal/whenWithOnlyElse/emptyElseBranchBlock.kt @@ -0,0 +1,9 @@ + +fun foo(a: Boolean, b: Boolean) { + val x = a + when ("") { + else -> { } + + } + val y = b +} \ No newline at end of file diff --git a/plugins/kotlin/idea/tests/testData/inspectionsLocal/whenWithOnlyElse/emptyElseBranchBlock.kt.after b/plugins/kotlin/idea/tests/testData/inspectionsLocal/whenWithOnlyElse/emptyElseBranchBlock.kt.after new file mode 100644 index 000000000000..c72a69d2afd1 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/inspectionsLocal/whenWithOnlyElse/emptyElseBranchBlock.kt.after @@ -0,0 +1,6 @@ + +fun foo(a: Boolean, b: Boolean) { + val x = a + + val y = b +} \ No newline at end of file diff --git a/plugins/kotlin/idea/tests/testData/inspectionsLocal/whenWithOnlyElse/userKotlinDotRun.kt b/plugins/kotlin/idea/tests/testData/inspectionsLocal/whenWithOnlyElse/userKotlinDotRun.kt new file mode 100644 index 000000000000..da02d96cfef5 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/inspectionsLocal/whenWithOnlyElse/userKotlinDotRun.kt @@ -0,0 +1,7 @@ +// WITH_STDLIB + +fun foo() { + val a = when ("") { + else -> kotlin.run { 1 } + } +} diff --git a/plugins/kotlin/idea/tests/testData/inspectionsLocal/whenWithOnlyElse/userKotlinDotRun.kt.after b/plugins/kotlin/idea/tests/testData/inspectionsLocal/whenWithOnlyElse/userKotlinDotRun.kt.after new file mode 100644 index 000000000000..d5b1478ffd3c --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/inspectionsLocal/whenWithOnlyElse/userKotlinDotRun.kt.after @@ -0,0 +1,5 @@ +// WITH_STDLIB + +fun foo() { + val a = kotlin.run { 1 } +} diff --git a/plugins/kotlin/util/test-generator-fir/test/org/jetbrains/kotlin/fir/testGenerator/codeinsight/GenerateK2InspectionTests.kt b/plugins/kotlin/util/test-generator-fir/test/org/jetbrains/kotlin/fir/testGenerator/codeinsight/GenerateK2InspectionTests.kt index bac9d8eae004..1f01506f4ad2 100644 --- a/plugins/kotlin/util/test-generator-fir/test/org/jetbrains/kotlin/fir/testGenerator/codeinsight/GenerateK2InspectionTests.kt +++ b/plugins/kotlin/util/test-generator-fir/test/org/jetbrains/kotlin/fir/testGenerator/codeinsight/GenerateK2InspectionTests.kt @@ -34,6 +34,7 @@ internal fun MutableTWorkspace.generateK2InspectionTests() { model("${idea}/inspectionsLocal/liftOut/tryToReturn") model("${idea}/inspectionsLocal/liftOut/whenToReturn") model("${idea}/inspectionsLocal/inconsistentCommentForJavaParameter") + model("${idea}/inspectionsLocal/whenWithOnlyElse") model("code-insight/inspections-k2/tests/testData/inspectionsLocal", pattern = pattern) }