From a02ad76b16cc5627cea20f6c41883ad26588ac06 Mon Sep 17 00:00:00 2001 From: Dereck Bridie Date: Mon, 4 Mar 2019 14:41:41 +0100 Subject: [PATCH] KT-25272: Unused expression as last expression of normal function should have quickfix to add "return" Fixed --- .../AddReturnToLastExpressionInFunctionFix.kt | 52 +++++++++++++++ ...turnToUnusedLastExpressionInFunctionFix.kt | 56 ++++++++++++++++ .../kotlin/idea/quickfix/QuickFixRegistrar.kt | 3 + .../notSubtype.kt | 8 +++ .../nothing.kt | 5 ++ .../simpleBoolean.kt | 4 ++ .../simpleBoolean.kt.after | 4 ++ .../subtype.kt | 6 ++ .../subtype.kt.after | 6 ++ .../notSubtype.kt | 7 ++ .../nothing.kt | 6 ++ .../simpleBoolean.kt | 4 ++ .../simpleBoolean.kt.after | 4 ++ .../subtype.kt | 6 ++ .../subtype.kt.after | 6 ++ .../QuickFixMultiFileTestGenerated.java | 26 ++++++++ .../idea/quickfix/QuickFixTestGenerated.java | 66 +++++++++++++++++++ 17 files changed, 269 insertions(+) create mode 100644 idea/src/org/jetbrains/kotlin/idea/quickfix/AddReturnToLastExpressionInFunctionFix.kt create mode 100644 idea/src/org/jetbrains/kotlin/idea/quickfix/AddReturnToUnusedLastExpressionInFunctionFix.kt create mode 100644 idea/testData/quickfix/addReturnToLastExpressionInFunction/notSubtype.kt create mode 100644 idea/testData/quickfix/addReturnToLastExpressionInFunction/nothing.kt create mode 100644 idea/testData/quickfix/addReturnToLastExpressionInFunction/simpleBoolean.kt create mode 100644 idea/testData/quickfix/addReturnToLastExpressionInFunction/simpleBoolean.kt.after create mode 100644 idea/testData/quickfix/addReturnToLastExpressionInFunction/subtype.kt create mode 100644 idea/testData/quickfix/addReturnToLastExpressionInFunction/subtype.kt.after create mode 100644 idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/notSubtype.kt create mode 100644 idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/nothing.kt create mode 100644 idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/simpleBoolean.kt create mode 100644 idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/simpleBoolean.kt.after create mode 100644 idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/subtype.kt create mode 100644 idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/subtype.kt.after diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/AddReturnToLastExpressionInFunctionFix.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/AddReturnToLastExpressionInFunctionFix.kt new file mode 100644 index 00000000000..7303abfe2e0 --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/AddReturnToLastExpressionInFunctionFix.kt @@ -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(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) + } + } +} diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/AddReturnToUnusedLastExpressionInFunctionFix.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/AddReturnToUnusedLastExpressionInFunctionFix.kt new file mode 100644 index 00000000000..36c58355c7d --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/AddReturnToUnusedLastExpressionInFunctionFix.kt @@ -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(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) + } + } +} diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt index 22300595dc1..6856307758d 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt @@ -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) diff --git a/idea/testData/quickfix/addReturnToLastExpressionInFunction/notSubtype.kt b/idea/testData/quickfix/addReturnToLastExpressionInFunction/notSubtype.kt new file mode 100644 index 00000000000..82848b1817d --- /dev/null +++ b/idea/testData/quickfix/addReturnToLastExpressionInFunction/notSubtype.kt @@ -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 +} diff --git a/idea/testData/quickfix/addReturnToLastExpressionInFunction/nothing.kt b/idea/testData/quickfix/addReturnToLastExpressionInFunction/nothing.kt new file mode 100644 index 00000000000..517270bdc2b --- /dev/null +++ b/idea/testData/quickfix/addReturnToLastExpressionInFunction/nothing.kt @@ -0,0 +1,5 @@ +// "Add 'return' to last expression" "false" + +fun test(): Nothing { + throw RuntimeException("test") +} diff --git a/idea/testData/quickfix/addReturnToLastExpressionInFunction/simpleBoolean.kt b/idea/testData/quickfix/addReturnToLastExpressionInFunction/simpleBoolean.kt new file mode 100644 index 00000000000..5449c709c06 --- /dev/null +++ b/idea/testData/quickfix/addReturnToLastExpressionInFunction/simpleBoolean.kt @@ -0,0 +1,4 @@ +// "Add 'return' to last expression" "true" +fun test(): Boolean { + true +} diff --git a/idea/testData/quickfix/addReturnToLastExpressionInFunction/simpleBoolean.kt.after b/idea/testData/quickfix/addReturnToLastExpressionInFunction/simpleBoolean.kt.after new file mode 100644 index 00000000000..9d8fe6b142b --- /dev/null +++ b/idea/testData/quickfix/addReturnToLastExpressionInFunction/simpleBoolean.kt.after @@ -0,0 +1,4 @@ +// "Add 'return' to last expression" "true" +fun test(): Boolean { + return true +} diff --git a/idea/testData/quickfix/addReturnToLastExpressionInFunction/subtype.kt b/idea/testData/quickfix/addReturnToLastExpressionInFunction/subtype.kt new file mode 100644 index 00000000000..01af39a668e --- /dev/null +++ b/idea/testData/quickfix/addReturnToLastExpressionInFunction/subtype.kt @@ -0,0 +1,6 @@ +// "Add 'return' to last expression" "true" +// WITH_RUNTIME + +fun foo(): Any { + true +} diff --git a/idea/testData/quickfix/addReturnToLastExpressionInFunction/subtype.kt.after b/idea/testData/quickfix/addReturnToLastExpressionInFunction/subtype.kt.after new file mode 100644 index 00000000000..ed62f049069 --- /dev/null +++ b/idea/testData/quickfix/addReturnToLastExpressionInFunction/subtype.kt.after @@ -0,0 +1,6 @@ +// "Add 'return' to last expression" "true" +// WITH_RUNTIME + +fun foo(): Any { + return true +} diff --git a/idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/notSubtype.kt b/idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/notSubtype.kt new file mode 100644 index 00000000000..1d70265644f --- /dev/null +++ b/idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/notSubtype.kt @@ -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 { + 5 +} diff --git a/idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/nothing.kt b/idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/nothing.kt new file mode 100644 index 00000000000..f0263086518 --- /dev/null +++ b/idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/nothing.kt @@ -0,0 +1,6 @@ +// "Add 'return' before the expression" "false" +// ACTION: Add '@Throws' annotation + +fun test(): Nothing { + throw RuntimeException("test") +} diff --git a/idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/simpleBoolean.kt b/idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/simpleBoolean.kt new file mode 100644 index 00000000000..2a6e243248e --- /dev/null +++ b/idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/simpleBoolean.kt @@ -0,0 +1,4 @@ +// "Add 'return' before the expression" "true" +fun test(): Boolean { + true +} diff --git a/idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/simpleBoolean.kt.after b/idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/simpleBoolean.kt.after new file mode 100644 index 00000000000..14747681f22 --- /dev/null +++ b/idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/simpleBoolean.kt.after @@ -0,0 +1,4 @@ +// "Add 'return' before the expression" "true" +fun test(): Boolean { + return true +} diff --git a/idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/subtype.kt b/idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/subtype.kt new file mode 100644 index 00000000000..68763fca4c4 --- /dev/null +++ b/idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/subtype.kt @@ -0,0 +1,6 @@ +// "Add 'return' before the expression" "true" +// WITH_RUNTIME + +fun foo(): Any { + true +} diff --git a/idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/subtype.kt.after b/idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/subtype.kt.after new file mode 100644 index 00000000000..9aa1852a4ce --- /dev/null +++ b/idea/testData/quickfix/addReturnToUnusedLastExpressionInFunction/subtype.kt.after @@ -0,0 +1,6 @@ +// "Add 'return' before the expression" "true" +// WITH_RUNTIME + +fun foo(): Any { + return true +} diff --git a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixMultiFileTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixMultiFileTestGenerated.java index b39081869c8..d3c4740920d 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixMultiFileTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixMultiFileTestGenerated.java @@ -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) diff --git a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java index 97c99c15837..a5efab23eab 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java @@ -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)