KT-25272: Unused expression as last expression of normal function should have quickfix to add "return" Fixed

2019-03-04 14:41:41 +01:00
parent e02877d1dc
commit a02ad76b16
17 changed files with 269 additions and 0 deletions
@@ -0,0 +1,52 @@
+/*
+ * Copyright 2010-2019 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.quickfix
+
+import com.intellij.codeInsight.intention.IntentionAction
+import com.intellij.openapi.editor.Editor
+import com.intellij.openapi.project.Project
+import org.jetbrains.kotlin.diagnostics.Diagnostic
+import org.jetbrains.kotlin.diagnostics.Errors
+import org.jetbrains.kotlin.idea.caches.resolve.analyze
+import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptorIfAny
+import org.jetbrains.kotlin.psi.KtDeclarationWithBody
+import org.jetbrains.kotlin.psi.KtFile
+import org.jetbrains.kotlin.psi.KtNamedFunction
+import org.jetbrains.kotlin.psi.KtPsiFactory
+import org.jetbrains.kotlin.resolve.calls.callUtil.getType
+import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
+import org.jetbrains.kotlin.types.typeUtil.isSubtypeOf
+
+class AddReturnToLastExpressionInFunctionFix(element: KtDeclarationWithBody) : KotlinQuickFixAction<KtDeclarationWithBody>(element) {
+    override fun getText() = "Add 'return' to last expression"
+    override fun getFamilyName() = text
+
+    override fun isAvailable(project: Project, editor: Editor?, file: KtFile): Boolean {
+        val element = element as? KtNamedFunction ?: return false
+        val block = element.bodyBlockExpression ?: return false
+        val last = block.statements.lastOrNull() ?: return false
+
+        val context = last.analyze(BodyResolveMode.PARTIAL)
+        val lastType = last.getType(context) ?: return false
+        val expectedType = element.resolveToDescriptorIfAny()?.returnType ?: return false
+        if (!lastType.isSubtypeOf(expectedType)) return false
+
+        return true
+    }
+
+    override fun invoke(project: Project, editor: Editor?, file: KtFile) {
+        val element = element as? KtNamedFunction ?: return
+        val last = element.bodyBlockExpression?.statements?.lastOrNull() ?: return
+        last.replace(KtPsiFactory(project).createExpression("return ${last.text}"))
+    }
+
+    companion object Factory : KotlinSingleIntentionActionFactory() {
+        override fun createAction(diagnostic: Diagnostic): IntentionAction {
+            val casted = Errors.NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY.cast(diagnostic)
+            return AddReturnToLastExpressionInFunctionFix(casted.psiElement)
+        }
+    }
+}
@@ -0,0 +1,56 @@
+/*
+ * Copyright 2010-2019 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.quickfix
+
+import com.intellij.codeInsight.intention.IntentionAction
+import com.intellij.openapi.editor.Editor
+import com.intellij.openapi.project.Project
+import org.jetbrains.kotlin.diagnostics.Diagnostic
+import org.jetbrains.kotlin.diagnostics.Errors
+import org.jetbrains.kotlin.idea.caches.resolve.analyze
+import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptorIfAny
+import org.jetbrains.kotlin.psi.*
+import org.jetbrains.kotlin.resolve.BindingContext
+import org.jetbrains.kotlin.resolve.bindingContextUtil.isUsedAsStatement
+import org.jetbrains.kotlin.resolve.calls.callUtil.getType
+import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
+import org.jetbrains.kotlin.types.typeUtil.isSubtypeOf
+
+class AddReturnToUnusedLastExpressionInFunctionFix(element: KtElement) : KotlinQuickFixAction<KtElement>(element) {
+    override fun getText() = "Add 'return' before the expression"
+    override fun getFamilyName() = text
+
+    override fun isAvailable(project: Project, editor: Editor?, file: KtFile): Boolean {
+        val expr = element as? KtExpression ?: return false
+        val context = expr.analyze(BodyResolveMode.PARTIAL)
+        if (!expr.isLastStatementInFunctionBody()) return false
+
+        val exprType = expr.getType(context) ?: return false
+        val function = expr.parent.parent as? KtNamedFunction ?: return false
+        val functionReturnType = function.resolveToDescriptorIfAny()?.returnType ?: return false
+        if (!exprType.isSubtypeOf(functionReturnType)) return false
+
+        return true
+    }
+
+    private fun KtExpression.isLastStatementInFunctionBody(): Boolean {
+        val body = this.parent as? KtBlockExpression ?: return false
+        val last = body.statements.lastOrNull() ?: return false
+        return last === this
+    }
+
+    override fun invoke(project: Project, editor: Editor?, file: KtFile) {
+        val element = element ?: return
+        element.replace(KtPsiFactory(project).createExpression("return ${element.text}"))
+    }
+
+    companion object Factory : KotlinSingleIntentionActionFactory() {
+        override fun createAction(diagnostic: Diagnostic): IntentionAction {
+            val casted = Errors.UNUSED_EXPRESSION.cast(diagnostic)
+            return AddReturnToUnusedLastExpressionInFunctionFix(casted.psiElement)
+        }
+    }
+}
@@ -223,6 +223,9 @@ class QuickFixRegistrar : QuickFixContributor {
        UNUSED_VARIABLE.registerFactory(RemovePsiElementSimpleFix.RemoveVariableFactory)
        UNUSED_VARIABLE.registerFactory(RenameToUnderscoreFix.Factory)

+        NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY.registerFactory(AddReturnToLastExpressionInFunctionFix)
+        UNUSED_EXPRESSION.registerFactory(AddReturnToUnusedLastExpressionInFunctionFix)
+
        UNUSED_DESTRUCTURED_PARAMETER_ENTRY.registerFactory(RenameToUnderscoreFix.Factory)

        SENSELESS_COMPARISON.registerFactory(SimplifyComparisonFix)
@@ -0,0 +1,8 @@
+// "Add 'return' to last expression" "false"
+// ERROR: A 'return' expression required in a function with a block body ('{...}')
+// ACTION: Introduce local variable
+// ACTION: Remove explicitly specified return type of enclosing function 'test'
+
+fun test(): Boolean {
+    5
+}<caret>
@@ -0,0 +1,5 @@
+// "Add 'return' to last expression" "false"
+
+fun test(): Nothing {
+    throw RuntimeException("test")
+}<caret>
@@ -0,0 +1,4 @@
+// "Add 'return' to last expression" "true"
+fun test(): Boolean {
+    true
+}<caret>
@@ -0,0 +1,4 @@
+// "Add 'return' to last expression" "true"
+fun test(): Boolean {
+    return true
+}
@@ -0,0 +1,6 @@
+// "Add 'return' to last expression" "true"
+// WITH_RUNTIME
+
+fun foo(): Any {
+    true
+}<caret>
@@ -0,0 +1,6 @@
+// "Add 'return' to last expression" "true"
+// WITH_RUNTIME
+
+fun foo(): Any {
+    return true
+}
@@ -0,0 +1,7 @@
+// "Add 'return' before the expression" "false"
+// ERROR: A 'return' expression required in a function with a block body ('{...}')
+// ACTION: Introduce local variable
+
+fun test(): Boolean {
+    <caret>5
+}
@@ -0,0 +1,6 @@
+// "Add 'return' before the expression" "false"
+// ACTION: Add '@Throws' annotation
+
+fun test(): Nothing {
+    <caret>throw RuntimeException("test")
+}
@@ -0,0 +1,4 @@
+// "Add 'return' before the expression" "true"
+fun test(): Boolean {
+    <caret>true
+}
@@ -0,0 +1,4 @@
+// "Add 'return' before the expression" "true"
+fun test(): Boolean {
+    return true
+}
@@ -0,0 +1,6 @@
+// "Add 'return' before the expression" "true"
+// WITH_RUNTIME
+
+fun foo(): Any {
+    <caret>true
+}
@@ -0,0 +1,6 @@
+// "Add 'return' before the expression" "true"
+// WITH_RUNTIME
+
+fun foo(): Any {
+    return true
+}
@@ -247,6 +247,32 @@ public class QuickFixMultiFileTestGenerated extends AbstractQuickFixMultiFileTes
        }
    }

+    @TestMetadata("idea/testData/quickfix/addReturnToLastExpressionInFunction")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class AddReturnToLastExpressionInFunction extends AbstractQuickFixMultiFileTest {
+        private void runTest(String testDataFilePath) throws Exception {
+            KotlinTestUtils.runTest(this::doTestWithExtraFile, TargetBackend.ANY, testDataFilePath);
+        }
+
+        public void testAllFilesPresentInAddReturnToLastExpressionInFunction() throws Exception {
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/quickfix/addReturnToLastExpressionInFunction"), Pattern.compile("^(\\w+)\\.((before\\.Main\\.\\w+)|(test))$"), TargetBackend.ANY, true);
+        }
+    }
+
+    @TestMetadata("idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class AddReturnToUnusedLastExpressionInFunction extends AbstractQuickFixMultiFileTest {
+        private void runTest(String testDataFilePath) throws Exception {
+            KotlinTestUtils.runTest(this::doTestWithExtraFile, TargetBackend.ANY, testDataFilePath);
+        }
+
+        public void testAllFilesPresentInAddReturnToUnusedLastExpressionInFunction() throws Exception {
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction"), Pattern.compile("^(\\w+)\\.((before\\.Main\\.\\w+)|(test))$"), TargetBackend.ANY, true);
+        }
+    }
+
    @TestMetadata("idea/testData/quickfix/addRunBeforeLambda")
    @TestDataPath("$PROJECT_ROOT")
    @RunWith(JUnit3RunnerWithInners.class)
@@ -902,6 +902,72 @@ public class QuickFixTestGenerated extends AbstractQuickFixTest {
        }
    }

+    @TestMetadata("idea/testData/quickfix/addReturnToLastExpressionInFunction")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class AddReturnToLastExpressionInFunction extends AbstractQuickFixTest {
+        private void runTest(String testDataFilePath) throws Exception {
+            KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath);
+        }
+
+        public void testAllFilesPresentInAddReturnToLastExpressionInFunction() throws Exception {
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/quickfix/addReturnToLastExpressionInFunction"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), TargetBackend.ANY, true);
+        }
+
+        @TestMetadata("notSubtype.kt")
+        public void testNotSubtype() throws Exception {
+            runTest("idea/testData/quickfix/addReturnToLastExpressionInFunction/notSubtype.kt");
+        }
+
+        @TestMetadata("nothing.kt")
+        public void testNothing() throws Exception {
+            runTest("idea/testData/quickfix/addReturnToLastExpressionInFunction/nothing.kt");
+        }
+
+        @TestMetadata("simpleBoolean.kt")
+        public void testSimpleBoolean() throws Exception {
+            runTest("idea/testData/quickfix/addReturnToLastExpressionInFunction/simpleBoolean.kt");
+        }
+
+        @TestMetadata("subtype.kt")
+        public void testSubtype() throws Exception {
+            runTest("idea/testData/quickfix/addReturnToLastExpressionInFunction/subtype.kt");
+        }
+    }
+
+    @TestMetadata("idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class AddReturnToUnusedLastExpressionInFunction extends AbstractQuickFixTest {
+        private void runTest(String testDataFilePath) throws Exception {
+            KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath);
+        }
+
+        public void testAllFilesPresentInAddReturnToUnusedLastExpressionInFunction() throws Exception {
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), TargetBackend.ANY, true);
+        }
+
+        @TestMetadata("notSubtype.kt")
+        public void testNotSubtype() throws Exception {
+            runTest("idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/notSubtype.kt");
+        }
+
+        @TestMetadata("nothing.kt")
+        public void testNothing() throws Exception {
+            runTest("idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/nothing.kt");
+        }
+
+        @TestMetadata("simpleBoolean.kt")
+        public void testSimpleBoolean() throws Exception {
+            runTest("idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/simpleBoolean.kt");
+        }
+
+        @TestMetadata("subtype.kt")
+        public void testSubtype() throws Exception {
+            runTest("idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/subtype.kt");
+        }
+    }
+
    @TestMetadata("idea/testData/quickfix/addRunBeforeLambda")
    @TestDataPath("$PROJECT_ROOT")
    @RunWith(JUnit3RunnerWithInners.class)