From 99f958c8c433cec6ee872e01cb8634f187f1386f Mon Sep 17 00:00:00 2001 From: Dmitry Gridin Date: Mon, 22 Jun 2020 22:09:26 +0700 Subject: [PATCH] Inline refactoring: should remove redundant Unit #KT-19443 Fixed --- .../kotlin/idea/codeInliner/CodeInliner.kt | 41 ++++++++++++------- .../inline/function/EmptyFunction.kt.after | 4 +- .../inline/function/NullableUnitReturnType.kt | 16 ++++++++ .../function/NullableUnitReturnType.kt.after | 13 ++++++ .../function/NullableUnitReturnType2.kt | 16 ++++++++ .../function/NullableUnitReturnType2.kt.after | 13 ++++++ .../function/NullableUnitReturnType3.kt | 14 +++++++ .../function/NullableUnitReturnType3.kt.after | 12 ++++++ .../inline/function/UnitReturnType.kt.after | 2 - .../inline/function/UnitReturnType3.kt | 12 ++++++ .../inline/function/UnitReturnType3.kt.after | 11 +++++ .../inline/function/UnitReturnType4.kt | 16 ++++++++ .../inline/function/UnitReturnType4.kt.after | 12 ++++++ .../inline/InlineTestGenerated.java | 25 +++++++++++ 14 files changed, 189 insertions(+), 18 deletions(-) create mode 100644 idea/testData/refactoring/inline/function/NullableUnitReturnType.kt create mode 100644 idea/testData/refactoring/inline/function/NullableUnitReturnType.kt.after create mode 100644 idea/testData/refactoring/inline/function/NullableUnitReturnType2.kt create mode 100644 idea/testData/refactoring/inline/function/NullableUnitReturnType2.kt.after create mode 100644 idea/testData/refactoring/inline/function/NullableUnitReturnType3.kt create mode 100644 idea/testData/refactoring/inline/function/NullableUnitReturnType3.kt.after create mode 100644 idea/testData/refactoring/inline/function/UnitReturnType3.kt create mode 100644 idea/testData/refactoring/inline/function/UnitReturnType3.kt.after create mode 100644 idea/testData/refactoring/inline/function/UnitReturnType4.kt create mode 100644 idea/testData/refactoring/inline/function/UnitReturnType4.kt.after diff --git a/idea/src/org/jetbrains/kotlin/idea/codeInliner/CodeInliner.kt b/idea/src/org/jetbrains/kotlin/idea/codeInliner/CodeInliner.kt index fa3dbaa5dad..1817b9da91f 100644 --- a/idea/src/org/jetbrains/kotlin/idea/codeInliner/CodeInliner.kt +++ b/idea/src/org/jetbrains/kotlin/idea/codeInliner/CodeInliner.kt @@ -15,6 +15,7 @@ import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade import org.jetbrains.kotlin.idea.caches.resolve.resolveImportReference import org.jetbrains.kotlin.idea.caches.resolve.resolveToCall import org.jetbrains.kotlin.idea.core.* +import org.jetbrains.kotlin.idea.inspections.RedundantUnitExpressionInspection import org.jetbrains.kotlin.idea.intentions.RemoveExplicitTypeArgumentsIntention import org.jetbrains.kotlin.idea.util.CommentSaver import org.jetbrains.kotlin.idea.util.IdeDescriptorRenderers @@ -407,27 +408,29 @@ class CodeInliner( } private fun postProcessInsertedCode(range: PsiChildRange, lexicalScope: LexicalScope?): PsiChildRange { - val elements = range.filterIsInstance().toList() - if (elements.isEmpty()) return PsiChildRange.EMPTY + val pointers = range.filterIsInstance().map { it.createSmartPointer() }.toList() + if (pointers.isEmpty()) return PsiChildRange.EMPTY lexicalScope?.let { - renameDuplicates(elements.dropLast(1).filterIsInstance(), it) + renameDuplicates(pointers.dropLast(1).mapNotNull { pointer -> pointer.element as? KtNamedDeclaration }, it) } - elements.forEach { - introduceNamedArguments(it) + for (pointer in pointers) { + val element = pointer.element ?: continue + introduceNamedArguments(element) - restoreFunctionLiteralArguments(it) + restoreFunctionLiteralArguments(element) //TODO: do this earlier - dropArgumentsForDefaultValues(it) + dropArgumentsForDefaultValues(element) - simplifySpreadArrayOfArguments(it) + simplifySpreadArrayOfArguments(element) - removeExplicitTypeArguments(it) + removeExplicitTypeArguments(element) + + removeRedundantUnitExpressions(element) } - val shortenFilter = { element: PsiElement -> if (element[USER_CODE_KEY]) { ShortenReferences.FilterResult.SKIP @@ -440,11 +443,13 @@ class CodeInliner( } } - val newElements = elements.map { - ShortenReferences { ShortenReferences.Options(removeThis = true) }.process(it, elementFilter = shortenFilter) + val newElements = pointers.mapNotNull { + it.element?.let { element -> + ShortenReferences { ShortenReferences.Options(removeThis = true) }.process(element, elementFilter = shortenFilter) + } } - newElements.forEach { element -> + for (element in newElements) { // clean up user data element.forEachDescendantOfType { it.clear(USER_CODE_KEY) @@ -460,7 +465,15 @@ class CodeInliner( } } - return PsiChildRange(newElements.first(), newElements.last()) + return if (newElements.isEmpty()) PsiChildRange.EMPTY else PsiChildRange(newElements.first(), newElements.last()) + } + + private fun removeRedundantUnitExpressions(result: KtElement) { + result.forEachDescendantOfType { + if (RedundantUnitExpressionInspection.isRedundantUnit(it)) { + it.delete() + } + } } private fun introduceNamedArguments(result: KtElement) { diff --git a/idea/testData/refactoring/inline/function/EmptyFunction.kt.after b/idea/testData/refactoring/inline/function/EmptyFunction.kt.after index c14e42f7ef1..f56a3ed7bb9 100644 --- a/idea/testData/refactoring/inline/function/EmptyFunction.kt.after +++ b/idea/testData/refactoring/inline/function/EmptyFunction.kt.after @@ -2,9 +2,9 @@ fun doIt(p: () -> T): T = TODO() fun g(p: String?) { - p?.let { Unit } + p?.let { } } fun h() = Unit -fun x() = doIt { Unit } +fun x() = doIt { } diff --git a/idea/testData/refactoring/inline/function/NullableUnitReturnType.kt b/idea/testData/refactoring/inline/function/NullableUnitReturnType.kt new file mode 100644 index 00000000000..27c6e870b98 --- /dev/null +++ b/idea/testData/refactoring/inline/function/NullableUnitReturnType.kt @@ -0,0 +1,16 @@ +fun doIt(p: () -> T): T = p() +fun Any.doDo() = Unit + +abstract class A { + abstract fun a() +} + +fun foo() { + null?.doDo() +} + +class B : A() { + override fun a() = doIt { + foo() + } +} \ No newline at end of file diff --git a/idea/testData/refactoring/inline/function/NullableUnitReturnType.kt.after b/idea/testData/refactoring/inline/function/NullableUnitReturnType.kt.after new file mode 100644 index 00000000000..d2ea191b538 --- /dev/null +++ b/idea/testData/refactoring/inline/function/NullableUnitReturnType.kt.after @@ -0,0 +1,13 @@ +fun doIt(p: () -> T): T = p() +fun Any.doDo() = Unit + +abstract class A { + abstract fun a() +} + +class B : A() { + override fun a() = doIt { + null?.doDo() + Unit + } +} \ No newline at end of file diff --git a/idea/testData/refactoring/inline/function/NullableUnitReturnType2.kt b/idea/testData/refactoring/inline/function/NullableUnitReturnType2.kt new file mode 100644 index 00000000000..f9b2cc098de --- /dev/null +++ b/idea/testData/refactoring/inline/function/NullableUnitReturnType2.kt @@ -0,0 +1,16 @@ +fun doIt(p: () -> T): T = p() +fun Any.doDo() = Unit + +abstract class A { + abstract fun a() +} + +fun foo() { + 1.let { it.let { it.let { it.let { null?.doDo() } } } } +} + +class B : A() { + override fun a() = doIt { + foo() + } +} \ No newline at end of file diff --git a/idea/testData/refactoring/inline/function/NullableUnitReturnType2.kt.after b/idea/testData/refactoring/inline/function/NullableUnitReturnType2.kt.after new file mode 100644 index 00000000000..75e41638503 --- /dev/null +++ b/idea/testData/refactoring/inline/function/NullableUnitReturnType2.kt.after @@ -0,0 +1,13 @@ +fun doIt(p: () -> T): T = p() +fun Any.doDo() = Unit + +abstract class A { + abstract fun a() +} + +class B : A() { + override fun a() = doIt { + 1.let { it.let { it.let { it.let { null?.doDo() } } } } + Unit + } +} \ No newline at end of file diff --git a/idea/testData/refactoring/inline/function/NullableUnitReturnType3.kt b/idea/testData/refactoring/inline/function/NullableUnitReturnType3.kt new file mode 100644 index 00000000000..1b7c8ac48a9 --- /dev/null +++ b/idea/testData/refactoring/inline/function/NullableUnitReturnType3.kt @@ -0,0 +1,14 @@ +fun doIt(p: () -> T): T = p() +fun Any.doDo() = Unit + +abstract class A { + abstract fun a() +} + +fun foo(): Unit = doIt { + 1.let { it.let { it.let { it.let { null?.doDo() } } } } +} + +class B : A() { + override fun a() = foo() +} \ No newline at end of file diff --git a/idea/testData/refactoring/inline/function/NullableUnitReturnType3.kt.after b/idea/testData/refactoring/inline/function/NullableUnitReturnType3.kt.after new file mode 100644 index 00000000000..3175392ce23 --- /dev/null +++ b/idea/testData/refactoring/inline/function/NullableUnitReturnType3.kt.after @@ -0,0 +1,12 @@ +fun doIt(p: () -> T): T = p() +fun Any.doDo() = Unit + +abstract class A { + abstract fun a() +} + +class B : A() { + override fun a() = doIt { + 1.let { it.let { it.let { it.let { null?.doDo() } } } } + } +} \ No newline at end of file diff --git a/idea/testData/refactoring/inline/function/UnitReturnType.kt.after b/idea/testData/refactoring/inline/function/UnitReturnType.kt.after index cba7e7e8117..2dfff0d52b7 100644 --- a/idea/testData/refactoring/inline/function/UnitReturnType.kt.after +++ b/idea/testData/refactoring/inline/function/UnitReturnType.kt.after @@ -9,7 +9,6 @@ class C { p?.let { println(3) println(4) - Unit } if (other != null) { @@ -26,6 +25,5 @@ class C { fun x() = doIt { println(9) println(10) - Unit } } diff --git a/idea/testData/refactoring/inline/function/UnitReturnType3.kt b/idea/testData/refactoring/inline/function/UnitReturnType3.kt new file mode 100644 index 00000000000..9f8641fccac --- /dev/null +++ b/idea/testData/refactoring/inline/function/UnitReturnType3.kt @@ -0,0 +1,12 @@ +class C { + fun f(p1: Int, p2: Int) { + println(p1) + nonUnit(p2) + } + + fun nonUnit(p: Int): Int = p + + fun doIt(p: () -> T): T = TODO() + + fun x() = doIt { f(9, 10) } +} diff --git a/idea/testData/refactoring/inline/function/UnitReturnType3.kt.after b/idea/testData/refactoring/inline/function/UnitReturnType3.kt.after new file mode 100644 index 00000000000..74386b7c633 --- /dev/null +++ b/idea/testData/refactoring/inline/function/UnitReturnType3.kt.after @@ -0,0 +1,11 @@ +class C { + + fun nonUnit(p: Int): Int = p + + fun doIt(p: () -> T): T = TODO() + + fun x() = doIt { + println(9) + nonUnit(10) + } +} diff --git a/idea/testData/refactoring/inline/function/UnitReturnType4.kt b/idea/testData/refactoring/inline/function/UnitReturnType4.kt new file mode 100644 index 00000000000..9110b6b50d6 --- /dev/null +++ b/idea/testData/refactoring/inline/function/UnitReturnType4.kt @@ -0,0 +1,16 @@ +fun doIt(p: () -> T): T = p() +fun Any.doDo() = Unit + +abstract class A { + abstract fun a() +} + +fun foo() { + 1.let { it.let { it.let { it.let { it?.doDo() } } } } +} + +class B : A() { + override fun a() = doIt { + foo() + } +} \ No newline at end of file diff --git a/idea/testData/refactoring/inline/function/UnitReturnType4.kt.after b/idea/testData/refactoring/inline/function/UnitReturnType4.kt.after new file mode 100644 index 00000000000..7b6beaf7013 --- /dev/null +++ b/idea/testData/refactoring/inline/function/UnitReturnType4.kt.after @@ -0,0 +1,12 @@ +fun doIt(p: () -> T): T = p() +fun Any.doDo() = Unit + +abstract class A { + abstract fun a() +} + +class B : A() { + override fun a() = doIt { + 1.let { it.let { it.let { it.let { it?.doDo() } } } } + } +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/refactoring/inline/InlineTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/refactoring/inline/InlineTestGenerated.java index f48e6d359e5..17a21e0f2ab 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/inline/InlineTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/refactoring/inline/InlineTestGenerated.java @@ -75,6 +75,21 @@ public class InlineTestGenerated extends AbstractInlineTest { runTest("idea/testData/refactoring/inline/function/MultipleReturns.kt"); } + @TestMetadata("NullableUnitReturnType.kt") + public void testNullableUnitReturnType() throws Exception { + runTest("idea/testData/refactoring/inline/function/NullableUnitReturnType.kt"); + } + + @TestMetadata("NullableUnitReturnType2.kt") + public void testNullableUnitReturnType2() throws Exception { + runTest("idea/testData/refactoring/inline/function/NullableUnitReturnType2.kt"); + } + + @TestMetadata("NullableUnitReturnType3.kt") + public void testNullableUnitReturnType3() throws Exception { + runTest("idea/testData/refactoring/inline/function/NullableUnitReturnType3.kt"); + } + @TestMetadata("OuterClassReceiver.kt") public void testOuterClassReceiver() throws Exception { runTest("idea/testData/refactoring/inline/function/OuterClassReceiver.kt"); @@ -135,6 +150,16 @@ public class InlineTestGenerated extends AbstractInlineTest { runTest("idea/testData/refactoring/inline/function/UnitReturnType2.kt"); } + @TestMetadata("UnitReturnType3.kt") + public void testUnitReturnType3() throws Exception { + runTest("idea/testData/refactoring/inline/function/UnitReturnType3.kt"); + } + + @TestMetadata("UnitReturnType4.kt") + public void testUnitReturnType4() throws Exception { + runTest("idea/testData/refactoring/inline/function/UnitReturnType4.kt"); + } + @TestMetadata("idea/testData/refactoring/inline/function/expressionBody") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)