From 05a132357b50688650a436bdf3f936f59c45ec0f Mon Sep 17 00:00:00 2001 From: Toshiaki Kameyama Date: Tue, 4 Jun 2019 11:56:58 +0900 Subject: [PATCH] Lift assignment: suggest for assignment of null #KT-30191 Fixed #KT-28595 Fixed --- .../BranchedFoldingUtils.kt | 30 ++++++++++++------- .../liftOut/ifToAssignment/hasNull.kt | 11 +++++++ .../liftOut/ifToAssignment/hasNull.kt.after | 11 +++++++ .../liftOut/ifToAssignment/hasNull2.kt | 11 +++++++ .../liftOut/ifToAssignment/hasNull2.kt.after | 11 +++++++ .../liftOut/ifToAssignment/hasNull3.kt | 11 +++++++ .../liftOut/ifToAssignment/hasNull3.kt.after | 11 +++++++ .../liftOut/ifToAssignment/hasNull4.kt | 13 ++++++++ .../liftOut/ifToAssignment/hasNull5.kt | 13 ++++++++ .../liftOut/ifToAssignment/subTypes.kt | 12 ++++++++ .../liftOut/ifToAssignment/subTypes.kt.after | 12 ++++++++ .../LocalInspectionTestGenerated.java | 30 +++++++++++++++++++ 12 files changed, 166 insertions(+), 10 deletions(-) create mode 100644 idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull.kt create mode 100644 idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull.kt.after create mode 100644 idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull2.kt create mode 100644 idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull2.kt.after create mode 100644 idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull3.kt create mode 100644 idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull3.kt.after create mode 100644 idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull4.kt create mode 100644 idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull5.kt create mode 100644 idea/testData/inspectionsLocal/liftOut/ifToAssignment/subTypes.kt create mode 100644 idea/testData/inspectionsLocal/liftOut/ifToAssignment/subTypes.kt.after diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/BranchedFoldingUtils.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/BranchedFoldingUtils.kt index 5b0c2bb48e6..c6e69627f17 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/BranchedFoldingUtils.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/BranchedFoldingUtils.kt @@ -28,8 +28,11 @@ import org.jetbrains.kotlin.psi.psiUtil.getNonStrictParentOfType import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType import org.jetbrains.kotlin.psi.psiUtil.lastBlockStatementOrThis import org.jetbrains.kotlin.resolve.calls.callUtil.getType +import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode +import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.types.TypeConstructor import org.jetbrains.kotlin.types.typeUtil.isNothing +import org.jetbrains.kotlin.types.typeUtil.isSubtypeOf object BranchedFoldingUtils { private fun getFoldableBranchedAssignment(branch: KtExpression?): KtBinaryExpression? { @@ -56,19 +59,25 @@ object BranchedFoldingUtils { it.getTargetLabel() == null } - private fun KtBinaryExpression.checkAssignmentsMatch(other: KtBinaryExpression, rightTypeConstructor: TypeConstructor): Boolean { - return left?.text == other.left?.text - && operationToken == other.operationToken - && rightTypeConstructor == other.rightTypeConstructor() + private fun KtBinaryExpression.checkAssignmentsMatch( + other: KtBinaryExpression, + leftType: KotlinType, + rightTypeConstructor: TypeConstructor + ): Boolean { + val left = this.left ?: return false + val otherLeft = other.left ?: return false + if (left.text != otherLeft.text || operationToken != other.operationToken) return false + val rightType = other.rightType() ?: return false + return rightType.constructor == rightTypeConstructor || (operationToken == KtTokens.EQ && rightType.isSubtypeOf(leftType)) } - private fun KtBinaryExpression.rightTypeConstructor(): TypeConstructor? { + private fun KtBinaryExpression.rightType(): KotlinType? { val right = this.right ?: return null val context = this.analyze() val diagnostics = context.diagnostics fun hasTypeMismatchError(e: KtExpression) = diagnostics.forElement(e).any { it.factory == Errors.TYPE_MISMATCH } if (hasTypeMismatchError(this) || hasTypeMismatchError(right)) return null - return right.getType(context)?.constructor + return right.getType(context) } internal fun getFoldableAssignmentNumber(expression: KtExpression?): Int { @@ -104,16 +113,17 @@ object BranchedFoldingUtils { else -> false } if (!collectAssignmentsAndCheck(expression)) return -1 - val firstAssignment = assignments.firstOrNull() ?: return 0 - val rightTypeConstructor = firstAssignment.rightTypeConstructor() ?: return -1 - if (assignments.any { !firstAssignment.checkAssignmentsMatch(it, rightTypeConstructor) }) { + val firstAssignment = assignments.firstOrNull { !it.right.isNullExpression() } ?: assignments.firstOrNull() ?: return 0 + val leftType = firstAssignment.left?.let { it.getType(it.analyze(BodyResolveMode.PARTIAL)) } ?: return 0 + val rightTypeConstructor = firstAssignment.rightType()?.constructor ?: return -1 + if (assignments.any { !firstAssignment.checkAssignmentsMatch(it, leftType, rightTypeConstructor) }) { return -1 } if (expression.anyDescendantOfType( predicate = { if (it.operationToken in KtTokens.ALL_ASSIGNMENTS) if (it.getNonStrictParentOfType() != null) - firstAssignment.checkAssignmentsMatch(it, rightTypeConstructor) + firstAssignment.checkAssignmentsMatch(it, leftType, rightTypeConstructor) else it !in assignments else diff --git a/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull.kt b/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull.kt new file mode 100644 index 00000000000..b8c2c6983af --- /dev/null +++ b/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull.kt @@ -0,0 +1,11 @@ +fun bar(i: Int) { + var str: String? = null + + if (i == 1) { + str = null + } else if (i == 2) { + str = "2" + } else { + str = "3" + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull.kt.after b/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull.kt.after new file mode 100644 index 00000000000..28098e18a03 --- /dev/null +++ b/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull.kt.after @@ -0,0 +1,11 @@ +fun bar(i: Int) { + var str: String? = null + + str = if (i == 1) { + null + } else if (i == 2) { + "2" + } else { + "3" + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull2.kt b/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull2.kt new file mode 100644 index 00000000000..de7ae326cfe --- /dev/null +++ b/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull2.kt @@ -0,0 +1,11 @@ +fun bar(i: Int) { + var str: String? = null + + if (i == 1) { + str = "1" + } else if (i == 2) { + str = "2" + } else { + str = null + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull2.kt.after b/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull2.kt.after new file mode 100644 index 00000000000..8e883301e00 --- /dev/null +++ b/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull2.kt.after @@ -0,0 +1,11 @@ +fun bar(i: Int) { + var str: String? = null + + str = if (i == 1) { + "1" + } else if (i == 2) { + "2" + } else { + null + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull3.kt b/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull3.kt new file mode 100644 index 00000000000..682b20e2bee --- /dev/null +++ b/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull3.kt @@ -0,0 +1,11 @@ +fun bar(i: Int) { + var str: String? = null + + if (i == 1) { + str = null + } else if (i == 2) { + str = null + } else { + str = null + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull3.kt.after b/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull3.kt.after new file mode 100644 index 00000000000..55c593bd06a --- /dev/null +++ b/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull3.kt.after @@ -0,0 +1,11 @@ +fun bar(i: Int) { + var str: String? = null + + str = if (i == 1) { + null + } else if (i == 2) { + null + } else { + null + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull4.kt b/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull4.kt new file mode 100644 index 00000000000..57770450a38 --- /dev/null +++ b/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull4.kt @@ -0,0 +1,13 @@ +// PROBLEM: none +// DISABLE-ERRORS +fun bar(i: Int) { + var str: String? = null + + if (i == 1) { + str = null + } else if (i == 2) { + str = "2" + } else { + str = 3 + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull5.kt b/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull5.kt new file mode 100644 index 00000000000..60b365c0c2e --- /dev/null +++ b/idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull5.kt @@ -0,0 +1,13 @@ +// PROBLEM: none +// DISABLE-ERRORS +fun bar(i: Int) { + var str: String = "" + + if (i == 1) { + str = null + } else if (i == 2) { + str = "2" + } else { + str = "3" + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/liftOut/ifToAssignment/subTypes.kt b/idea/testData/inspectionsLocal/liftOut/ifToAssignment/subTypes.kt new file mode 100644 index 00000000000..d0bb887df38 --- /dev/null +++ b/idea/testData/inspectionsLocal/liftOut/ifToAssignment/subTypes.kt @@ -0,0 +1,12 @@ +open class A +class B : A() +class C : A() + +fun liftClass(boolean: Boolean) { + val a1: A + if (boolean) { + a1 = B() + } else { + a1 = C() + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/liftOut/ifToAssignment/subTypes.kt.after b/idea/testData/inspectionsLocal/liftOut/ifToAssignment/subTypes.kt.after new file mode 100644 index 00000000000..80b2d4fda9e --- /dev/null +++ b/idea/testData/inspectionsLocal/liftOut/ifToAssignment/subTypes.kt.after @@ -0,0 +1,12 @@ +open class A +class B : A() +class C : A() + +fun liftClass(boolean: Boolean) { + val a1: A + a1 = if (boolean) { + B() + } else { + C() + } +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java index ab0e6a9001f..129f8b12b0a 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -4575,6 +4575,31 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { runTest("idea/testData/inspectionsLocal/liftOut/ifToAssignment/elseIfOnly.kt"); } + @TestMetadata("hasNull.kt") + public void testHasNull() throws Exception { + runTest("idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull.kt"); + } + + @TestMetadata("hasNull2.kt") + public void testHasNull2() throws Exception { + runTest("idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull2.kt"); + } + + @TestMetadata("hasNull3.kt") + public void testHasNull3() throws Exception { + runTest("idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull3.kt"); + } + + @TestMetadata("hasNull4.kt") + public void testHasNull4() throws Exception { + runTest("idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull4.kt"); + } + + @TestMetadata("hasNull5.kt") + public void testHasNull5() throws Exception { + runTest("idea/testData/inspectionsLocal/liftOut/ifToAssignment/hasNull5.kt"); + } + @TestMetadata("ifElseIf.kt") public void testIfElseIf() throws Exception { runTest("idea/testData/inspectionsLocal/liftOut/ifToAssignment/ifElseIf.kt"); @@ -4655,6 +4680,11 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { runTest("idea/testData/inspectionsLocal/liftOut/ifToAssignment/simpleIfWithoutTerminatingAssignment.kt"); } + @TestMetadata("subTypes.kt") + public void testSubTypes() throws Exception { + runTest("idea/testData/inspectionsLocal/liftOut/ifToAssignment/subTypes.kt"); + } + @TestMetadata("typeMismatch.kt") public void testTypeMismatch() throws Exception { runTest("idea/testData/inspectionsLocal/liftOut/ifToAssignment/typeMismatch.kt");