From af24ce5e0328222d612c00500b6f5fd280d9aa09 Mon Sep 17 00:00:00 2001 From: Dmitry Gridin Date: Mon, 22 Jun 2020 22:07:29 +0700 Subject: [PATCH] RedundantUnitExpressionInspection: support lambdas #KT-39772 Fixed --- .../RedundantUnitExpressionInspection.kt | 7 +-- .../atLastAfterFunInIf.kt | 1 - .../atLastAfterFunInIf.kt.after | 4 ++ .../atLastAfterFunInWhen.kt | 1 - .../atLastAfterFunInWhen.kt.after | 7 +++ .../redundantUnitExpression/notRedundant4.kt | 28 ++++++++++++ .../redundantUnitExpression/notRedundant5.kt | 28 ++++++++++++ .../redundantUnitExpression/notRedundant6.kt | 16 +++++++ .../redundantUnitExpression/notRedundant7.kt | 17 +++++++ .../redundantUnitExpression/unitReturnType.kt | 12 +++++ .../unitReturnType.kt.after | 12 +++++ .../unitReturnType2.kt | 12 +++++ .../unitReturnType2.kt.after | 12 +++++ .../unitReturnType3.kt | 9 ++++ .../unitReturnType4.kt | 14 ++++++ .../unitReturnType4.kt.after | 13 ++++++ .../unitReturnType5.kt | 15 +++++++ .../unitReturnType5.kt.after | 14 ++++++ .../LocalInspectionTestGenerated.java | 45 +++++++++++++++++++ 19 files changed, 260 insertions(+), 7 deletions(-) create mode 100644 idea/testData/inspectionsLocal/redundantUnitExpression/atLastAfterFunInIf.kt.after create mode 100644 idea/testData/inspectionsLocal/redundantUnitExpression/atLastAfterFunInWhen.kt.after create mode 100644 idea/testData/inspectionsLocal/redundantUnitExpression/notRedundant4.kt create mode 100644 idea/testData/inspectionsLocal/redundantUnitExpression/notRedundant5.kt create mode 100644 idea/testData/inspectionsLocal/redundantUnitExpression/notRedundant6.kt create mode 100644 idea/testData/inspectionsLocal/redundantUnitExpression/notRedundant7.kt create mode 100644 idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType.kt create mode 100644 idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType.kt.after create mode 100644 idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType2.kt create mode 100644 idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType2.kt.after create mode 100644 idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType3.kt create mode 100644 idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType4.kt create mode 100644 idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType4.kt.after create mode 100644 idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType5.kt create mode 100644 idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType5.kt.after diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantUnitExpressionInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantUnitExpressionInspection.kt index 815936f2f3b..2cba184b040 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantUnitExpressionInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantUnitExpressionInspection.kt @@ -15,7 +15,6 @@ import org.jetbrains.kotlin.idea.caches.resolve.resolveToCall import org.jetbrains.kotlin.idea.intentions.loopToCallChain.previousStatement import org.jetbrains.kotlin.js.descriptorUtils.nameIfStandardType import org.jetbrains.kotlin.psi.* -import org.jetbrains.kotlin.psi.psiUtil.getParentOfType import org.jetbrains.kotlin.psi.psiUtil.getParentOfTypesAndPredicate import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType import org.jetbrains.kotlin.psi.psiUtil.lastBlockStatementOrThis @@ -48,16 +47,14 @@ class RedundantUnitExpressionInspection : AbstractKotlinInspection(), CleanupLoc } if (parent is KtBlockExpression) { - // Do not report just 'Unit' in function literals (return@label Unit is OK even in literals) - if (parent.getParentOfType(strict = true) != null) return false - if (referenceExpression == parent.lastBlockStatementOrThis()) { val prev = referenceExpression.previousStatement() ?: return true if (prev.isUnitLiteral()) return true - val prevType = prev.resolveToCall(BodyResolveMode.FULL)?.resultingDescriptor?.returnType + val prevType = prev.analyze(BodyResolveMode.PARTIAL).getType(prev) if (prevType != null) { return prevType.isUnit() } + if (prev !is KtDeclaration) return false if (prev !is KtFunction) return true return parent.getParentOfTypesAndPredicate( diff --git a/idea/testData/inspectionsLocal/redundantUnitExpression/atLastAfterFunInIf.kt b/idea/testData/inspectionsLocal/redundantUnitExpression/atLastAfterFunInIf.kt index 06c94826a58..925863cc7eb 100644 --- a/idea/testData/inspectionsLocal/redundantUnitExpression/atLastAfterFunInIf.kt +++ b/idea/testData/inspectionsLocal/redundantUnitExpression/atLastAfterFunInIf.kt @@ -1,4 +1,3 @@ -// PROBLEM: none fun test(b: Boolean): Unit = if (b) { fun a() = 1 Unit diff --git a/idea/testData/inspectionsLocal/redundantUnitExpression/atLastAfterFunInIf.kt.after b/idea/testData/inspectionsLocal/redundantUnitExpression/atLastAfterFunInIf.kt.after new file mode 100644 index 00000000000..2e0a5c8db3a --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantUnitExpression/atLastAfterFunInIf.kt.after @@ -0,0 +1,4 @@ +fun test(b: Boolean): Unit = if (b) { + fun a() = 1 +} else { +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantUnitExpression/atLastAfterFunInWhen.kt b/idea/testData/inspectionsLocal/redundantUnitExpression/atLastAfterFunInWhen.kt index 1e7c0fce247..5425b7100e8 100644 --- a/idea/testData/inspectionsLocal/redundantUnitExpression/atLastAfterFunInWhen.kt +++ b/idea/testData/inspectionsLocal/redundantUnitExpression/atLastAfterFunInWhen.kt @@ -1,4 +1,3 @@ -// PROBLEM: none fun test(b: Boolean): Unit = when (b) { true -> { fun a() {} diff --git a/idea/testData/inspectionsLocal/redundantUnitExpression/atLastAfterFunInWhen.kt.after b/idea/testData/inspectionsLocal/redundantUnitExpression/atLastAfterFunInWhen.kt.after new file mode 100644 index 00000000000..d661d64e8f1 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantUnitExpression/atLastAfterFunInWhen.kt.after @@ -0,0 +1,7 @@ +fun test(b: Boolean): Unit = when (b) { + true -> { + fun a() {} + } + else -> { + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantUnitExpression/notRedundant4.kt b/idea/testData/inspectionsLocal/redundantUnitExpression/notRedundant4.kt new file mode 100644 index 00000000000..bfd46648a07 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantUnitExpression/notRedundant4.kt @@ -0,0 +1,28 @@ +// PROBLEM: none +// WITH_RUNTIME +fun nonUnit(p: Int): Int = p + +fun doIt(p: () -> T): T = TODO() + +fun g(p: String?) { + println(1) + nonUnit(2) + + p?.let { + println(3) + nonUnit(4) + Unit + } +} + +fun h() { + println(5) + nonUnit(6) +} + +fun x() = doIt { + println(7) + nonUnit(8) + Unit +} + diff --git a/idea/testData/inspectionsLocal/redundantUnitExpression/notRedundant5.kt b/idea/testData/inspectionsLocal/redundantUnitExpression/notRedundant5.kt new file mode 100644 index 00000000000..81b0da83781 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantUnitExpression/notRedundant5.kt @@ -0,0 +1,28 @@ +// PROBLEM: none +// WITH_RUNTIME +fun nonUnit(p: Int): Int = p + +fun doIt(p: () -> T): T = TODO() + +fun g(p: String?) { + println(1) + nonUnit(2) + + p?.let { + println(3) + nonUnit(4) + Unit + } +} + +fun h() { + println(5) + nonUnit(6) +} + +fun x() = doIt { + println(7) + nonUnit(8) + Unit +} + diff --git a/idea/testData/inspectionsLocal/redundantUnitExpression/notRedundant6.kt b/idea/testData/inspectionsLocal/redundantUnitExpression/notRedundant6.kt new file mode 100644 index 00000000000..15ef8d86cd2 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantUnitExpression/notRedundant6.kt @@ -0,0 +1,16 @@ +// PROBLEM: none + +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 + } +} + diff --git a/idea/testData/inspectionsLocal/redundantUnitExpression/notRedundant7.kt b/idea/testData/inspectionsLocal/redundantUnitExpression/notRedundant7.kt new file mode 100644 index 00000000000..827c1dd2469 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantUnitExpression/notRedundant7.kt @@ -0,0 +1,17 @@ +// PROBLEM: none +// WITH_RUNTIME + +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 + } +} + diff --git a/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType.kt b/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType.kt new file mode 100644 index 00000000000..0caec992e2e --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType.kt @@ -0,0 +1,12 @@ +// WITH_RUNTIME + +fun doIt(p: () -> T): T = TODO() + +fun g(p: String?) { + + p?.let { Unit } +} + +fun h() = Unit + +fun x() = doIt { Unit } diff --git a/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType.kt.after b/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType.kt.after new file mode 100644 index 00000000000..896dbfa2524 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType.kt.after @@ -0,0 +1,12 @@ +// WITH_RUNTIME + +fun doIt(p: () -> T): T = TODO() + +fun g(p: String?) { + + p?.let { } +} + +fun h() = Unit + +fun x() = doIt { Unit } diff --git a/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType2.kt b/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType2.kt new file mode 100644 index 00000000000..bad18e778c7 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType2.kt @@ -0,0 +1,12 @@ +// WITH_RUNTIME + +fun doIt(p: () -> T): T = TODO() + +fun g(p: String?) { + + p?.let { Unit } +} + +fun h() = Unit + +fun x() = doIt { Unit } diff --git a/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType2.kt.after b/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType2.kt.after new file mode 100644 index 00000000000..015c5f3f31f --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType2.kt.after @@ -0,0 +1,12 @@ +// WITH_RUNTIME + +fun doIt(p: () -> T): T = TODO() + +fun g(p: String?) { + + p?.let { Unit } +} + +fun h() = Unit + +fun x() = doIt { } diff --git a/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType3.kt b/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType3.kt new file mode 100644 index 00000000000..928e3222425 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType3.kt @@ -0,0 +1,9 @@ +// WITH_RUNTIME +// PROBLEM: none +// not yet supported +fun doIt(p: () -> T): T = TODO() + +fun x() = doIt { + 4 + Unit +} diff --git a/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType4.kt b/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType4.kt new file mode 100644 index 00000000000..44df1633362 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType4.kt @@ -0,0 +1,14 @@ +// WITH_RUNTIME +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() } } } } + Unit + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType4.kt.after b/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType4.kt.after new file mode 100644 index 00000000000..1eb90c5e0dc --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType4.kt.after @@ -0,0 +1,13 @@ +// WITH_RUNTIME +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/testData/inspectionsLocal/redundantUnitExpression/unitReturnType5.kt b/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType5.kt new file mode 100644 index 00000000000..f6eeec4facc --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType5.kt @@ -0,0 +1,15 @@ +// WITH_RUNTIME +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() } } } } + Unit + } +} + diff --git a/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType5.kt.after b/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType5.kt.after new file mode 100644 index 00000000000..5cc4e33319b --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType5.kt.after @@ -0,0 +1,14 @@ +// WITH_RUNTIME +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() } } } } + } +} + diff --git a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java index 963b2d52a9d..cb704c58fc8 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -8768,6 +8768,26 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { runTest("idea/testData/inspectionsLocal/redundantUnitExpression/notRedundant3.kt"); } + @TestMetadata("notRedundant4.kt") + public void testNotRedundant4() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantUnitExpression/notRedundant4.kt"); + } + + @TestMetadata("notRedundant5.kt") + public void testNotRedundant5() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantUnitExpression/notRedundant5.kt"); + } + + @TestMetadata("notRedundant6.kt") + public void testNotRedundant6() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantUnitExpression/notRedundant6.kt"); + } + + @TestMetadata("notRedundant7.kt") + public void testNotRedundant7() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantUnitExpression/notRedundant7.kt"); + } + @TestMetadata("redundant1.kt") public void testRedundant1() throws Exception { runTest("idea/testData/inspectionsLocal/redundantUnitExpression/redundant1.kt"); @@ -8797,6 +8817,31 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { public void testReturnAsNullableUnit() throws Exception { runTest("idea/testData/inspectionsLocal/redundantUnitExpression/returnAsNullableUnit.kt"); } + + @TestMetadata("unitReturnType.kt") + public void testUnitReturnType() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType.kt"); + } + + @TestMetadata("unitReturnType2.kt") + public void testUnitReturnType2() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType2.kt"); + } + + @TestMetadata("unitReturnType3.kt") + public void testUnitReturnType3() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType3.kt"); + } + + @TestMetadata("unitReturnType4.kt") + public void testUnitReturnType4() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType4.kt"); + } + + @TestMetadata("unitReturnType5.kt") + public void testUnitReturnType5() throws Exception { + runTest("idea/testData/inspectionsLocal/redundantUnitExpression/unitReturnType5.kt"); + } } @TestMetadata("idea/testData/inspectionsLocal/redundantVisibilityModifier")