Introduce "Redundant else in if" inspection #KT-19668 Fixed

2018-11-29 15:23:59 +03:00
parent ca87e53f04
commit 7cbc8e8b76
24 changed files with 430 additions and 0 deletions
@@ -3145,6 +3145,15 @@
                     language="kotlin"
    />

+    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.RedundantElseInIfInspection"
+                     displayName="Redundant 'else' in 'if'"
+                     groupPath="Kotlin"
+                     groupName="Style issues"
+                     enabledByDefault="true"
+                     level="INFORMATION"
+                     language="kotlin"
+    />
+
    <referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>

    <fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
@@ -0,0 +1,16 @@
+<html>
+<body>
+This inspection reports redundant <b>else</b> in <b>if</b> with <b>return</b>:
+
+<pre>
+fun foo(arg: Boolean): Int {
+    if (arg) return 0
+    // This else is redundant, code in braces could be just shifted left
+    else {
+        ...
+    }
+}
+</pre>
+
+</body>
+</html>
@@ -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<KtIfExpression>()?.`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
+}
@@ -0,0 +1 @@
+org.jetbrains.kotlin.idea.inspections.RedundantElseInIfInspection
@@ -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<caret> {
+        foo()
+    }
+}
@@ -0,0 +1,15 @@
+// WITH_RUNTIME
+fun foo() {}
+fun bar() {}
+
+fun test(s: String?, b: Boolean) {
+    s?.also {
+        if (b) {
+            foo()
+            return
+        } <caret>else {
+            bar()
+            return
+        }
+    }
+}
@@ -0,0 +1,14 @@
+// WITH_RUNTIME
+fun foo() {}
+fun bar() {}
+
+fun test(s: String?, b: Boolean) {
+    s?.also {
+        if (b) {
+            foo()
+            return
+        }
+        bar()
+        return
+    }
+}
@@ -0,0 +1,11 @@
+// PROBLEM: none
+// WITH_RUNTIME
+class SomeException : RuntimeException()
+
+fun test(x: Boolean, y: Boolean) {
+    if (x) {
+        throw SomeException()
+    } else<caret> if (y) {
+        return
+    }
+}
@@ -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<caret> bar()
+}
@@ -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<caret> {
+        foo()
+        bar()
+    }
+}
@@ -0,0 +1,8 @@
+// WITH_RUNTIME
+class SomeException : RuntimeException()
+fun foo(): Int = 1
+
+fun test(x: Boolean) {
+    if (x) throw SomeException()
+    else<caret> foo()
+}
@@ -0,0 +1,8 @@
+// WITH_RUNTIME
+class SomeException : RuntimeException()
+fun foo(): Int = 1
+
+fun test(x: Boolean) {
+    if (x) throw SomeException()
+    foo()
+}
@@ -0,0 +1,11 @@
+fun foo(): Int = 1
+fun bar(): Int = 2
+
+fun test(x: Boolean) {
+    if (x) {
+        return
+    } else<caret> {
+        foo()
+        bar()
+    }
+}
@@ -0,0 +1,10 @@
+fun foo(): Int = 1
+fun bar(): Int = 2
+
+fun test(x: Boolean) {
+    if (x) {
+        return
+    }
+    foo()
+    bar()
+}
@@ -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<caret> foo()
+}
@@ -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()
+}
@@ -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<caret> {
+        // comment1
+        foo()
+        // comment2
+        bar()
+    }
+}
@@ -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()
+}
@@ -0,0 +1,7 @@
+// WITH_RUNTIME
+class SomeException : RuntimeException()
+fun foo(): Int = 1
+
+fun test(x: Boolean): Int {
+    if (x) throw SomeException() else<caret> return foo()
+}
@@ -0,0 +1,8 @@
+// WITH_RUNTIME
+class SomeException : RuntimeException()
+fun foo(): Int = 1
+
+fun test(x: Boolean): Int {
+    if (x) throw SomeException()
+    return foo()
+}
@@ -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()
+    } <caret>else return foo()
+}
@@ -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()
+}
@@ -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<caret> foo()
+}
@@ -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)
				`@@ -0,0 +1 @@`
				`org.jetbrains.kotlin.idea.inspections.RedundantElseInIfInspection`