diff --git a/idea/resources/META-INF/plugin-common.xml b/idea/resources/META-INF/plugin-common.xml index ac4b83c3e7e..f47d8737596 100644 --- a/idea/resources/META-INF/plugin-common.xml +++ b/idea/resources/META-INF/plugin-common.xml @@ -3145,6 +3145,15 @@ language="kotlin" /> + + diff --git a/idea/resources/inspectionDescriptions/RedundantElseInIf.html b/idea/resources/inspectionDescriptions/RedundantElseInIf.html new file mode 100644 index 00000000000..354ed5e899b --- /dev/null +++ b/idea/resources/inspectionDescriptions/RedundantElseInIf.html @@ -0,0 +1,16 @@ + + +This inspection reports redundant else in if with return: + +
+fun foo(arg: Boolean): Int {
+    if (arg) return 0
+    // This else is redundant, code in braces could be just shifted left
+    else {
+        ...
+    }
+}
+
+ + + diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantElseInIfInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantElseInIfInspection.kt new file mode 100644 index 00000000000..adc53380876 --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantElseInIfInspection.kt @@ -0,0 +1,116 @@ +/* + * Copyright 2010-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license + * that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.idea.inspections + +import com.intellij.codeInspection.LocalQuickFix +import com.intellij.codeInspection.ProblemDescriptor +import com.intellij.codeInspection.ProblemHighlightType +import com.intellij.codeInspection.ProblemsHolder +import com.intellij.openapi.fileEditor.FileDocumentManager +import com.intellij.openapi.project.Project +import com.intellij.openapi.util.TextRange +import com.intellij.psi.PsiDocumentManager +import com.intellij.psi.PsiElement +import com.intellij.psi.PsiElementVisitor +import com.intellij.psi.PsiFile +import com.intellij.psi.codeStyle.CodeStyleManager +import org.jetbrains.kotlin.KtNodeTypes +import org.jetbrains.kotlin.idea.caches.resolve.analyze +import org.jetbrains.kotlin.idea.intentions.branchedTransformations.isElseIf +import org.jetbrains.kotlin.idea.refactoring.getLineNumber +import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.psiUtil.* +import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.types.typeUtil.isNothing + +class RedundantElseInIfInspection : AbstractKotlinInspection() { + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor = + ifExpressionVisitor(fun(ifExpression) { + if (ifExpression.elseKeyword == null || ifExpression.isElseIf()) return + val elseKeyword = ifExpression.lastSingleElseKeyword() ?: return + if (!ifExpression.hasRedundantElse()) return + val rangeInElement = elseKeyword.textRange?.shiftRight(-ifExpression.startOffset) ?: return + holder.registerProblem( + holder.manager.createProblemDescriptor( + ifExpression, + rangeInElement, + "Redundant 'else'", + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, + isOnTheFly, + RemoveRedundantElseFix() + ) + ) + }) +} + +private class RemoveRedundantElseFix : LocalQuickFix { + override fun getName() = "Remove redundant 'else'" + + override fun getFamilyName() = name + + override fun applyFix(project: Project, descriptor: ProblemDescriptor) { + val ifExpression = descriptor.psiElement as? KtIfExpression ?: return + val elseKeyword = ifExpression.lastSingleElseKeyword() ?: return + val elseExpression = elseKeyword.getStrictParentOfType()?.`else` ?: return + + val copy = elseExpression.copy() + if (copy is KtBlockExpression) { + copy.lBrace?.delete() + copy.rBrace?.delete() + } + val parent = ifExpression.parent + val added = parent.addAfter(copy, ifExpression) + val elseKeywordLineNumber = elseKeyword.getLineNumber() + val lastThenEndLine = elseKeyword.getPrevSiblingIgnoringWhitespaceAndComments()?.takeIf { + it is KtContainerNodeForControlStructureBody && it.node.elementType == KtNodeTypes.THEN + }?.getLineNumber(start = false) + val elseStartLine = ((elseExpression as? KtBlockExpression)?.statements?.firstOrNull() ?: elseExpression).getLineNumber() + if (elseKeywordLineNumber == lastThenEndLine && elseKeywordLineNumber == elseStartLine) { + parent.addAfter(KtPsiFactory(ifExpression).createNewLine(), ifExpression) + } + elseExpression.delete() + elseKeyword.delete() + + ifExpression.containingFile.adjustLineIndent( + ifExpression.endOffset, + (added.getNextSiblingIgnoringWhitespace() ?: added.parent).endOffset + ) + } + + fun PsiFile.adjustLineIndent(startOffset: Int, endOffset: Int) { + val virtualFile = this.virtualFile ?: return + val document = FileDocumentManager.getInstance().getDocument(virtualFile) ?: return + val documentManager = PsiDocumentManager.getInstance(project) + val psiFile = documentManager.getPsiFile(document) ?: return + documentManager.commitDocument(document) + documentManager.doPostponedOperationsAndUnblockDocument(document) + CodeStyleManager.getInstance(project).adjustLineIndent(psiFile, TextRange(startOffset, endOffset)) + } +} + +private fun KtIfExpression.lastSingleElseKeyword(): PsiElement? { + var ifExpression = this + while (true) { + ifExpression = ifExpression.`else` as? KtIfExpression ?: break + } + return ifExpression.elseKeyword +} + +private fun KtIfExpression.hasRedundantElse(): Boolean { + val context = analyze() + if (context[BindingContext.USED_AS_EXPRESSION, this] == true) return false + var ifExpression = this + while (true) { + if ((ifExpression.then)?.isReturnOrNothing(context) != true) return false + ifExpression = ifExpression.`else` as? KtIfExpression ?: break + } + return true +} + +private fun KtExpression.isReturnOrNothing(context: BindingContext): Boolean { + val lastExpression = (this as? KtBlockExpression)?.statements?.lastOrNull() ?: this + return lastExpression is KtReturnExpression || context.getType(lastExpression)?.isNothing() == true +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantElseInIf/.inspection b/idea/testData/inspectionsLocal/redundantElseInIf/.inspection new file mode 100644 index 00000000000..7939618d310 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantElseInIf/.inspection @@ -0,0 +1 @@ +org.jetbrains.kotlin.idea.inspections.RedundantElseInIfInspection \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantElseInIf/empty.kt b/idea/testData/inspectionsLocal/redundantElseInIf/empty.kt new file mode 100644 index 00000000000..20feebaff14 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantElseInIf/empty.kt @@ -0,0 +1,14 @@ +// PROBLEM: none +// WITH_RUNTIME +class SomeException : RuntimeException() +fun foo(): Int = 1 + +fun test(x: Boolean, y: Boolean) { + if (x) { + throw SomeException() + } else if (y) { + // empty + } else { + foo() + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantElseInIf/inLambda.kt b/idea/testData/inspectionsLocal/redundantElseInIf/inLambda.kt new file mode 100644 index 00000000000..96e8f7a86f7 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantElseInIf/inLambda.kt @@ -0,0 +1,15 @@ +// WITH_RUNTIME +fun foo() {} +fun bar() {} + +fun test(s: String?, b: Boolean) { + s?.also { + if (b) { + foo() + return + } else { + bar() + return + } + } +} diff --git a/idea/testData/inspectionsLocal/redundantElseInIf/inLambda.kt.after b/idea/testData/inspectionsLocal/redundantElseInIf/inLambda.kt.after new file mode 100644 index 00000000000..e26b7fccd89 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantElseInIf/inLambda.kt.after @@ -0,0 +1,14 @@ +// WITH_RUNTIME +fun foo() {} +fun bar() {} + +fun test(s: String?, b: Boolean) { + s?.also { + if (b) { + foo() + return + } + bar() + return + } +} diff --git a/idea/testData/inspectionsLocal/redundantElseInIf/noElse.kt b/idea/testData/inspectionsLocal/redundantElseInIf/noElse.kt new file mode 100644 index 00000000000..f6e22b0342c --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantElseInIf/noElse.kt @@ -0,0 +1,11 @@ +// PROBLEM: none +// WITH_RUNTIME +class SomeException : RuntimeException() + +fun test(x: Boolean, y: Boolean) { + if (x) { + throw SomeException() + } else if (y) { + return + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantElseInIf/notNothing.kt b/idea/testData/inspectionsLocal/redundantElseInIf/notNothing.kt new file mode 100644 index 00000000000..43d0639512a --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantElseInIf/notNothing.kt @@ -0,0 +1,9 @@ +// PROBLEM: none +fun foo(): Int = 1 +fun bar(): Int = 2 + +fun test(x: Boolean, y: Boolean) { + if (x) foo() + else if (y) return + else bar() +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantElseInIf/notReturn.kt b/idea/testData/inspectionsLocal/redundantElseInIf/notReturn.kt new file mode 100644 index 00000000000..278c1155a6c --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantElseInIf/notReturn.kt @@ -0,0 +1,16 @@ +// PROBLEM: none +// WITH_RUNTIME +class SomeException : RuntimeException() +fun foo(): Int = 1 +fun bar(): Int = 2 + +fun test(x: Boolean, y: Boolean) { + if (x) { + foo() + } else if (y) { + throw SomeException() + } else { + foo() + bar() + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantElseInIf/redundant.kt b/idea/testData/inspectionsLocal/redundantElseInIf/redundant.kt new file mode 100644 index 00000000000..f286c148d97 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantElseInIf/redundant.kt @@ -0,0 +1,8 @@ +// WITH_RUNTIME +class SomeException : RuntimeException() +fun foo(): Int = 1 + +fun test(x: Boolean) { + if (x) throw SomeException() + else foo() +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantElseInIf/redundant.kt.after b/idea/testData/inspectionsLocal/redundantElseInIf/redundant.kt.after new file mode 100644 index 00000000000..eeea5d5b722 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantElseInIf/redundant.kt.after @@ -0,0 +1,8 @@ +// WITH_RUNTIME +class SomeException : RuntimeException() +fun foo(): Int = 1 + +fun test(x: Boolean) { + if (x) throw SomeException() + foo() +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantElseInIf/redundant2.kt b/idea/testData/inspectionsLocal/redundantElseInIf/redundant2.kt new file mode 100644 index 00000000000..14a513977a9 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantElseInIf/redundant2.kt @@ -0,0 +1,11 @@ +fun foo(): Int = 1 +fun bar(): Int = 2 + +fun test(x: Boolean) { + if (x) { + return + } else { + foo() + bar() + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantElseInIf/redundant2.kt.after b/idea/testData/inspectionsLocal/redundantElseInIf/redundant2.kt.after new file mode 100644 index 00000000000..0e0f228dcad --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantElseInIf/redundant2.kt.after @@ -0,0 +1,10 @@ +fun foo(): Int = 1 +fun bar(): Int = 2 + +fun test(x: Boolean) { + if (x) { + return + } + foo() + bar() +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantElseInIf/redundant3.kt b/idea/testData/inspectionsLocal/redundantElseInIf/redundant3.kt new file mode 100644 index 00000000000..9f6dd642cd0 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantElseInIf/redundant3.kt @@ -0,0 +1,9 @@ +// WITH_RUNTIME +class SomeException : RuntimeException() +fun foo(): Int = 1 + +fun test(x: Boolean, y: Boolean) { + if (x) throw SomeException() + else if (y) return + else foo() +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantElseInIf/redundant3.kt.after b/idea/testData/inspectionsLocal/redundantElseInIf/redundant3.kt.after new file mode 100644 index 00000000000..3af08fb48ba --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantElseInIf/redundant3.kt.after @@ -0,0 +1,9 @@ +// WITH_RUNTIME +class SomeException : RuntimeException() +fun foo(): Int = 1 + +fun test(x: Boolean, y: Boolean) { + if (x) throw SomeException() + else if (y) return + foo() +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantElseInIf/redundant4.kt b/idea/testData/inspectionsLocal/redundantElseInIf/redundant4.kt new file mode 100644 index 00000000000..220e2268966 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantElseInIf/redundant4.kt @@ -0,0 +1,17 @@ +// WITH_RUNTIME +class SomeException : RuntimeException() +fun foo(): Int = 1 +fun bar(): Int = 2 + +fun test(x: Boolean, y: Boolean) { + if (x) { + return + } else if (y) { + throw SomeException() + } else { + // comment1 + foo() + // comment2 + bar() + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantElseInIf/redundant4.kt.after b/idea/testData/inspectionsLocal/redundantElseInIf/redundant4.kt.after new file mode 100644 index 00000000000..fefb793be8e --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantElseInIf/redundant4.kt.after @@ -0,0 +1,16 @@ +// WITH_RUNTIME +class SomeException : RuntimeException() +fun foo(): Int = 1 +fun bar(): Int = 2 + +fun test(x: Boolean, y: Boolean) { + if (x) { + return + } else if (y) { + throw SomeException() + } + // comment1 + foo() + // comment2 + bar() +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantElseInIf/redundantSingleLine.kt b/idea/testData/inspectionsLocal/redundantElseInIf/redundantSingleLine.kt new file mode 100644 index 00000000000..6453445d9f6 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantElseInIf/redundantSingleLine.kt @@ -0,0 +1,7 @@ +// WITH_RUNTIME +class SomeException : RuntimeException() +fun foo(): Int = 1 + +fun test(x: Boolean): Int { + if (x) throw SomeException() else return foo() +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantElseInIf/redundantSingleLine.kt.after b/idea/testData/inspectionsLocal/redundantElseInIf/redundantSingleLine.kt.after new file mode 100644 index 00000000000..6c5ae253cb8 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantElseInIf/redundantSingleLine.kt.after @@ -0,0 +1,8 @@ +// WITH_RUNTIME +class SomeException : RuntimeException() +fun foo(): Int = 1 + +fun test(x: Boolean): Int { + if (x) throw SomeException() + return foo() +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantElseInIf/redundantSingleLine2.kt b/idea/testData/inspectionsLocal/redundantElseInIf/redundantSingleLine2.kt new file mode 100644 index 00000000000..227403fa41c --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantElseInIf/redundantSingleLine2.kt @@ -0,0 +1,11 @@ +// WITH_RUNTIME +class SomeException : RuntimeException() +fun foo(): Int = 1 + +fun test(x: Int): Int { + if (x == 1) { + throw SomeException() + } else if (x == 2) { + throw SomeException() + } else return foo() +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantElseInIf/redundantSingleLine2.kt.after b/idea/testData/inspectionsLocal/redundantElseInIf/redundantSingleLine2.kt.after new file mode 100644 index 00000000000..026d550da90 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantElseInIf/redundantSingleLine2.kt.after @@ -0,0 +1,12 @@ +// WITH_RUNTIME +class SomeException : RuntimeException() +fun foo(): Int = 1 + +fun test(x: Int): Int { + if (x == 1) { + throw SomeException() + } else if (x == 2) { + throw SomeException() + } + return foo() +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantElseInIf/usedAsExpression.kt b/idea/testData/inspectionsLocal/redundantElseInIf/usedAsExpression.kt new file mode 100644 index 00000000000..e31c0d76e06 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantElseInIf/usedAsExpression.kt @@ -0,0 +1,10 @@ +// PROBLEM: none +// WITH_RUNTIME +class SomeException : RuntimeException() +fun foo(): Int = 1 + +fun test(x: Boolean, y: Boolean) { + val i: Int = if (x) throw SomeException() + else if (y) return + else foo() +} \ 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 3572eac8b27..68fa009ae81 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -4216,6 +4216,79 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { } } + @TestMetadata("idea/testData/inspectionsLocal/redundantElseInIf") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class RedundantElseInIf extends AbstractLocalInspectionTest { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath); + } + + public void testAllFilesPresentInRedundantElseInIf() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/redundantElseInIf"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), TargetBackend.ANY, true); + } + + @TestMetadata("empty.kt") + public void testEmpty() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantElseInIf/empty.kt"); + } + + @TestMetadata("inLambda.kt") + public void testInLambda() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantElseInIf/inLambda.kt"); + } + + @TestMetadata("noElse.kt") + public void testNoElse() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantElseInIf/noElse.kt"); + } + + @TestMetadata("notNothing.kt") + public void testNotNothing() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantElseInIf/notNothing.kt"); + } + + @TestMetadata("notReturn.kt") + public void testNotReturn() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantElseInIf/notReturn.kt"); + } + + @TestMetadata("redundant.kt") + public void testRedundant() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantElseInIf/redundant.kt"); + } + + @TestMetadata("redundant2.kt") + public void testRedundant2() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantElseInIf/redundant2.kt"); + } + + @TestMetadata("redundant3.kt") + public void testRedundant3() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantElseInIf/redundant3.kt"); + } + + @TestMetadata("redundant4.kt") + public void testRedundant4() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantElseInIf/redundant4.kt"); + } + + @TestMetadata("redundantSingleLine.kt") + public void testRedundantSingleLine() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantElseInIf/redundantSingleLine.kt"); + } + + @TestMetadata("redundantSingleLine2.kt") + public void testRedundantSingleLine2() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantElseInIf/redundantSingleLine2.kt"); + } + + @TestMetadata("usedAsExpression.kt") + public void testUsedAsExpression() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantElseInIf/usedAsExpression.kt"); + } + } + @TestMetadata("idea/testData/inspectionsLocal/redundantExplicitType") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)