diff --git a/idea/resources/inspectionDescriptions/WhenWithOnlyElse.html b/idea/resources/inspectionDescriptions/WhenWithOnlyElse.html new file mode 100644 index 00000000000..c062c5858b5 --- /dev/null +++ b/idea/resources/inspectionDescriptions/WhenWithOnlyElse.html @@ -0,0 +1,5 @@ + + +This inspection reports 'when' expressions with only an 'else' branch that can be simplified to the 'else' branch's expression. + + diff --git a/idea/src/META-INF/plugin.xml b/idea/src/META-INF/plugin.xml index 9654a838a88..6b449c86b47 100644 --- a/idea/src/META-INF/plugin.xml +++ b/idea/src/META-INF/plugin.xml @@ -2437,6 +2437,15 @@ language="kotlin" /> + + diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/ConstantConditionIfInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/ConstantConditionIfInspection.kt index 9d8b6b6525a..f38428e5c16 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/ConstantConditionIfInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/ConstantConditionIfInspection.kt @@ -72,34 +72,10 @@ class ConstantConditionIfInspection : AbstractKotlinInspection() { override fun applyFix(project: Project, descriptor: ProblemDescriptor) { val ifExpression = descriptor.psiElement.getParentOfType(strict = true) ?: return - val caretModel = ifExpression.findExistingEditor()?.caretModel val branch = ifExpression.branch(conditionValue)?.unwrapBlockOrParenthesis() ?: return - val lastExpression = when { - branch !is KtBlockExpression -> ifExpression.replaced(branch) - isUsedAsExpression -> { - val factory = KtPsiFactory(ifExpression) - ifExpression.replaced(factory.createExpressionByPattern("run $0", branch.text)) - } - else -> { - val firstChild = branch.firstChild.nextSibling - - if (firstChild == branch.lastChild) { - ifExpression.delete() - } - else { - val lastChild = branch.lastChild.prevSibling - val parent = ifExpression.parent - parent.addRangeAfter(firstChild, lastChild, ifExpression) - ifExpression.delete() - } - - null - } - } - - caretModel?.moveToOffset(lastExpression?.startOffset ?: return) + ifExpression.replaceWithBranch(branch, isUsedAsExpression) } } @@ -113,15 +89,41 @@ class ConstantConditionIfInspection : AbstractKotlinInspection() { ifExpression.delete() } } +} - private companion object { - private fun KtIfExpression.branch(thenBranch: Boolean) = if (thenBranch) then else `else` +private fun KtIfExpression.branch(thenBranch: Boolean) = if (thenBranch) then else `else` - private fun KtExpression.constantBooleanValue(context: BindingContext): Boolean? { - val type = getType(context) ?: return null +private fun KtExpression.constantBooleanValue(context: BindingContext): Boolean? { + val type = getType(context) ?: return null - val constantValue = ConstantExpressionEvaluator.getConstant(this, context)?.toConstantValue(type) - return constantValue?.value as? Boolean + val constantValue = ConstantExpressionEvaluator.getConstant(this, context)?.toConstantValue(type) + return constantValue?.value as? Boolean +} + +fun KtExpression.replaceWithBranch(branch: KtExpression, isUsedAsExpression: Boolean) { + val lastExpression = when { + branch !is KtBlockExpression -> replaced(branch) + isUsedAsExpression -> { + val factory = KtPsiFactory(this) + replaced(factory.createExpressionByPattern("run $0", branch.text)) + } + else -> { + val firstChild = branch.firstChild.nextSibling + + if (firstChild == branch.lastChild) { + delete() + } + else { + val lastChild = branch.lastChild.prevSibling + val parent = parent + parent.addRangeAfter(firstChild, lastChild, this) + delete() + } + + null } } + + val caretModel = branch.findExistingEditor()?.caretModel + caretModel?.moveToOffset(lastExpression?.startOffset ?: return) } \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/WhenWithOnlyElseInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/WhenWithOnlyElseInspection.kt new file mode 100644 index 00000000000..ed0f4b4218a --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/WhenWithOnlyElseInspection.kt @@ -0,0 +1,61 @@ +/* + * Copyright 2010-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jetbrains.kotlin.idea.inspections + +import com.intellij.codeInsight.FileModificationService +import com.intellij.codeInspection.LocalQuickFix +import com.intellij.codeInspection.ProblemDescriptor +import com.intellij.codeInspection.ProblemsHolder +import com.intellij.openapi.project.Project +import com.intellij.psi.PsiElementVisitor +import org.jetbrains.kotlin.idea.caches.resolve.analyze +import org.jetbrains.kotlin.psi.KtVisitorVoid +import org.jetbrains.kotlin.psi.KtWhenExpression +import org.jetbrains.kotlin.resolve.bindingContextUtil.isUsedAsExpression + +class WhenWithOnlyElseInspection : AbstractKotlinInspection() { + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor { + return object : KtVisitorVoid() { + override fun visitWhenExpression(expression: KtWhenExpression) { + val singleEntry = expression.entries.singleOrNull() + if (singleEntry?.isElse != true) return + + val usedAsExpression = expression.isUsedAsExpression(expression.analyze()) + + holder.registerProblem(expression, + "'when' has only 'else' branch and can be simplified", + SimplifyFix(usedAsExpression) + ) + } + } + } + + private class SimplifyFix( + private val isUsedAsExpression: Boolean + ) : LocalQuickFix { + override fun getFamilyName() = name + + override fun getName() = "Simplify expression" + + override fun applyFix(project: Project, descriptor: ProblemDescriptor) { + val whenExpression = descriptor.psiElement as? KtWhenExpression ?: return + FileModificationService.getInstance().preparePsiElementForWrite(whenExpression) + + whenExpression.replaceWithBranch(whenExpression.elseExpression ?: return, isUsedAsExpression) + } + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/whenWithOnlyElse/.inspection b/idea/testData/inspectionsLocal/whenWithOnlyElse/.inspection new file mode 100644 index 00000000000..c5299d36511 --- /dev/null +++ b/idea/testData/inspectionsLocal/whenWithOnlyElse/.inspection @@ -0,0 +1 @@ +org.jetbrains.kotlin.idea.inspections.WhenWithOnlyElseInspection \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/whenWithOnlyElse/complexExpression.kt b/idea/testData/inspectionsLocal/whenWithOnlyElse/complexExpression.kt new file mode 100644 index 00000000000..58e54c48462 --- /dev/null +++ b/idea/testData/inspectionsLocal/whenWithOnlyElse/complexExpression.kt @@ -0,0 +1,12 @@ +// WITH_RUNTIME + +fun println(s: String) {} + +fun foo() { + val a = when ("") { + else -> { + println("") + 1 + } + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/whenWithOnlyElse/complexExpression.kt.after b/idea/testData/inspectionsLocal/whenWithOnlyElse/complexExpression.kt.after new file mode 100644 index 00000000000..299a55a2e01 --- /dev/null +++ b/idea/testData/inspectionsLocal/whenWithOnlyElse/complexExpression.kt.after @@ -0,0 +1,10 @@ +// WITH_RUNTIME + +fun println(s: String) {} + +fun foo() { + val a = run { + println("") + 1 + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/whenWithOnlyElse/hasOtherBranches.kt b/idea/testData/inspectionsLocal/whenWithOnlyElse/hasOtherBranches.kt new file mode 100644 index 00000000000..20b9dbad25d --- /dev/null +++ b/idea/testData/inspectionsLocal/whenWithOnlyElse/hasOtherBranches.kt @@ -0,0 +1,11 @@ +// PROBLEM: none + +fun println(s: String) {} + +fun foo(a: Boolean, b: Boolean) { + when ("") { + "a" -> println("a") + else -> println("else") + + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/whenWithOnlyElse/notExpression.kt b/idea/testData/inspectionsLocal/whenWithOnlyElse/notExpression.kt new file mode 100644 index 00000000000..afefc6a02ea --- /dev/null +++ b/idea/testData/inspectionsLocal/whenWithOnlyElse/notExpression.kt @@ -0,0 +1,10 @@ +fun println(s: String) {} + +fun foo() { + when ("") { + else -> { + println("") + 1 + } + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/whenWithOnlyElse/notExpression.kt.after b/idea/testData/inspectionsLocal/whenWithOnlyElse/notExpression.kt.after new file mode 100644 index 00000000000..46086172e59 --- /dev/null +++ b/idea/testData/inspectionsLocal/whenWithOnlyElse/notExpression.kt.after @@ -0,0 +1,6 @@ +fun println(s: String) {} + +fun foo() { + println("") + 1 +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/whenWithOnlyElse/simpleExpression.kt b/idea/testData/inspectionsLocal/whenWithOnlyElse/simpleExpression.kt new file mode 100644 index 00000000000..7cda10472b4 --- /dev/null +++ b/idea/testData/inspectionsLocal/whenWithOnlyElse/simpleExpression.kt @@ -0,0 +1,5 @@ +fun foo() { + val a = when ("") { + else -> 1 + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/whenWithOnlyElse/simpleExpression.kt.after b/idea/testData/inspectionsLocal/whenWithOnlyElse/simpleExpression.kt.after new file mode 100644 index 00000000000..d75b8c8c1f5 --- /dev/null +++ b/idea/testData/inspectionsLocal/whenWithOnlyElse/simpleExpression.kt.after @@ -0,0 +1,3 @@ +fun foo() { + val a = 1 +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java index bb1999735de..f4e5c5a650b 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -1929,6 +1929,39 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { } } + @TestMetadata("idea/testData/inspectionsLocal/whenWithOnlyElse") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class WhenWithOnlyElse extends AbstractLocalInspectionTest { + public void testAllFilesPresentInWhenWithOnlyElse() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/whenWithOnlyElse"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), TargetBackend.ANY, true); + } + + @TestMetadata("complexExpression.kt") + public void testComplexExpression() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/whenWithOnlyElse/complexExpression.kt"); + doTest(fileName); + } + + @TestMetadata("hasOtherBranches.kt") + public void testHasOtherBranches() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/whenWithOnlyElse/hasOtherBranches.kt"); + doTest(fileName); + } + + @TestMetadata("notExpression.kt") + public void testNotExpression() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/whenWithOnlyElse/notExpression.kt"); + doTest(fileName); + } + + @TestMetadata("simpleExpression.kt") + public void testSimpleExpression() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/whenWithOnlyElse/simpleExpression.kt"); + doTest(fileName); + } + } + @TestMetadata("idea/testData/inspectionsLocal/wrapUnaryOperator") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)