From 9b11b5300ce7dd279945327646027815384ad38f Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Wed, 9 Sep 2015 15:59:06 +0300 Subject: [PATCH] Elvis DFA: now only left part is taken into account #KT-9100 Fixed I had to fix a few incorrect tests using something like x!! in Elvis right part, and also one bug in our code --- .../BasicExpressionTypingVisitor.java | 14 +++++++--- .../specialConstructions/elvisAsCall.kt | 6 +++-- .../tests/smartCasts/elvisExclExcl.kt | 9 +++++++ .../tests/smartCasts/elvisExclExcl.txt | 3 +++ .../tests/smartCasts/elvisExclExclMerge.kt | 6 +++++ .../tests/smartCasts/elvisExclExclMerge.txt | 3 +++ .../tests/smartCasts/elvisExclExclPlatform.kt | 27 +++++++++++++++++++ .../smartCasts/elvisExclExclPlatform.txt | 16 +++++++++++ .../assignElvisIfBreakInsideWhileTrue.kt | 4 +-- .../loops/elvisIfBreakInsideWhileTrue.kt | 4 +-- .../loops/elvisLeftBreakInsideWhileTrue.kt | 4 +-- .../checkers/JetDiagnosticsTestGenerated.java | 18 +++++++++++++ .../kotlin/idea/stubs/DebugTextByStubTest.kt | 2 +- 13 files changed, 103 insertions(+), 13 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/smartCasts/elvisExclExcl.kt create mode 100644 compiler/testData/diagnostics/tests/smartCasts/elvisExclExcl.txt create mode 100644 compiler/testData/diagnostics/tests/smartCasts/elvisExclExclMerge.kt create mode 100644 compiler/testData/diagnostics/tests/smartCasts/elvisExclExclMerge.txt create mode 100644 compiler/testData/diagnostics/tests/smartCasts/elvisExclExclPlatform.kt create mode 100644 compiler/testData/diagnostics/tests/smartCasts/elvisExclExclPlatform.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/BasicExpressionTypingVisitor.java b/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/BasicExpressionTypingVisitor.java index 3dc5b5e7596..be3296ebc4c 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/BasicExpressionTypingVisitor.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/BasicExpressionTypingVisitor.java @@ -1173,10 +1173,16 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor { boolean loopBreakContinuePossible = leftTypeInfo.getJumpOutPossible() || rightTypeInfo.getJumpOutPossible(); JetType rightType = rightTypeInfo.getType(); - DataFlowInfo dataFlowInfo = resolvedCall.getDataFlowInfoForArguments().getResultInfo(); - if (leftType != null && rightType != null && KotlinBuiltIns.isNothingOrNullableNothing(rightType) && !rightType.isMarkedNullable()) { - DataFlowValue value = createDataFlowValue(left, leftType, context); - dataFlowInfo = dataFlowInfo.disequate(value, DataFlowValue.NULL); + // Only left argument DFA is taken into account here: we cannot be sure that right argument is executed + DataFlowInfo dataFlowInfo = resolvedCall.getDataFlowInfoForArguments().getInfo(call.getValueArguments().get(1)); + if (leftType != null) { + DataFlowValue leftValue = createDataFlowValue(left, leftType, context); + DataFlowInfo rightDataFlowInfo = resolvedCall.getDataFlowInfoForArguments().getResultInfo(); + // left argument is considered not-null if it's not-null also in right part or if we have jump in right part + if ((rightType != null && KotlinBuiltIns.isNothingOrNullableNothing(rightType) && !rightType.isMarkedNullable()) + || !rightDataFlowInfo.getNullability(leftValue).canBeNull()) { + dataFlowInfo = dataFlowInfo.disequate(leftValue, DataFlowValue.NULL); + } } JetType type = resolvedCall.getResultingDescriptor().getReturnType(); if (type == null || rightType == null) return TypeInfoFactoryPackage.noTypeInfo(dataFlowInfo); diff --git a/compiler/testData/diagnostics/tests/resolve/specialConstructions/elvisAsCall.kt b/compiler/testData/diagnostics/tests/resolve/specialConstructions/elvisAsCall.kt index c087d7ac479..efb8a58241c 100644 --- a/compiler/testData/diagnostics/tests/resolve/specialConstructions/elvisAsCall.kt +++ b/compiler/testData/diagnostics/tests/resolve/specialConstructions/elvisAsCall.kt @@ -21,12 +21,14 @@ fun testElvis(a: Int?, b: Int?) { fun testDataFlowInfo1(a: Int?, b: Int?) { val c: Int = a ?: b!! doInt(c) - b + 1 + // b is nullable if a != null + b + 1 } fun testDataFlowInfo2(a: Int?, b: Int?) { doInt(a ?: b!!) - b + 1 + // b is nullable if a != null + b + 1 } fun testTypeMismatch(a: String?, b: Any) { diff --git a/compiler/testData/diagnostics/tests/smartCasts/elvisExclExcl.kt b/compiler/testData/diagnostics/tests/smartCasts/elvisExclExcl.kt new file mode 100644 index 00000000000..ac6084bbe2c --- /dev/null +++ b/compiler/testData/diagnostics/tests/smartCasts/elvisExclExcl.kt @@ -0,0 +1,9 @@ +// Based on KT-9100 +fun test(x: Any?, y: Any?): Any { + val z = x ?: y!! + y.hashCode() + // !! / ?. is necessary here, because y!! above may not be executed + y?.hashCode() + y!!.hashCode() + return z +} diff --git a/compiler/testData/diagnostics/tests/smartCasts/elvisExclExcl.txt b/compiler/testData/diagnostics/tests/smartCasts/elvisExclExcl.txt new file mode 100644 index 00000000000..efa574054be --- /dev/null +++ b/compiler/testData/diagnostics/tests/smartCasts/elvisExclExcl.txt @@ -0,0 +1,3 @@ +package + +public fun test(/*0*/ x: kotlin.Any?, /*1*/ y: kotlin.Any?): kotlin.Any diff --git a/compiler/testData/diagnostics/tests/smartCasts/elvisExclExclMerge.kt b/compiler/testData/diagnostics/tests/smartCasts/elvisExclExclMerge.kt new file mode 100644 index 00000000000..543e7d75075 --- /dev/null +++ b/compiler/testData/diagnostics/tests/smartCasts/elvisExclExclMerge.kt @@ -0,0 +1,6 @@ +fun test(x: Any?): Any { + val z = x ?: x!! + // x is not null in both branches + x.hashCode() + return z +} diff --git a/compiler/testData/diagnostics/tests/smartCasts/elvisExclExclMerge.txt b/compiler/testData/diagnostics/tests/smartCasts/elvisExclExclMerge.txt new file mode 100644 index 00000000000..6cefb3fd1fb --- /dev/null +++ b/compiler/testData/diagnostics/tests/smartCasts/elvisExclExclMerge.txt @@ -0,0 +1,3 @@ +package + +public fun test(/*0*/ x: kotlin.Any?): kotlin.Any diff --git a/compiler/testData/diagnostics/tests/smartCasts/elvisExclExclPlatform.kt b/compiler/testData/diagnostics/tests/smartCasts/elvisExclExclPlatform.kt new file mode 100644 index 00000000000..69606a8a9aa --- /dev/null +++ b/compiler/testData/diagnostics/tests/smartCasts/elvisExclExclPlatform.kt @@ -0,0 +1,27 @@ +// FILE: p/My.java + +package p; + +import org.jetbrains.annotations.*; + +class My { + @Nullable static String create() { + return ""; + } +} + +// FILE: test.kt + +package p + +fun bar(x: String) = x + +fun test(x: String?): Any { + val y = My.create() + val z = x ?: y!! + bar(y) + // !! / ?. is necessary here, because y!! above may not be executed + y?.hashCode() + y!!.hashCode() + return z +} diff --git a/compiler/testData/diagnostics/tests/smartCasts/elvisExclExclPlatform.txt b/compiler/testData/diagnostics/tests/smartCasts/elvisExclExclPlatform.txt new file mode 100644 index 00000000000..3ceef9b54e4 --- /dev/null +++ b/compiler/testData/diagnostics/tests/smartCasts/elvisExclExclPlatform.txt @@ -0,0 +1,16 @@ +package + +package p { + public fun bar(/*0*/ x: kotlin.String): kotlin.String + public fun test(/*0*/ x: kotlin.String?): kotlin.Any + + public/*package*/ open class My { + public/*package*/ constructor My() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + // Static members + org.jetbrains.annotations.Nullable() public/*package*/ open fun create(): kotlin.String? + } +} diff --git a/compiler/testData/diagnostics/tests/smartCasts/loops/assignElvisIfBreakInsideWhileTrue.kt b/compiler/testData/diagnostics/tests/smartCasts/loops/assignElvisIfBreakInsideWhileTrue.kt index 2f8a7167440..8eff15ae074 100644 --- a/compiler/testData/diagnostics/tests/smartCasts/loops/assignElvisIfBreakInsideWhileTrue.kt +++ b/compiler/testData/diagnostics/tests/smartCasts/loops/assignElvisIfBreakInsideWhileTrue.kt @@ -3,8 +3,8 @@ public fun foo(x: String?, y: String?): Int { val z = x ?: if (y == null) break else y // z is not null in both branches z.length() - // y is not null in both branches - y.length() + // y is nullable if x != null + y.length() } // y is null because of the break return y.length() diff --git a/compiler/testData/diagnostics/tests/smartCasts/loops/elvisIfBreakInsideWhileTrue.kt b/compiler/testData/diagnostics/tests/smartCasts/loops/elvisIfBreakInsideWhileTrue.kt index 312fff11a94..e48a55cabeb 100644 --- a/compiler/testData/diagnostics/tests/smartCasts/loops/elvisIfBreakInsideWhileTrue.kt +++ b/compiler/testData/diagnostics/tests/smartCasts/loops/elvisIfBreakInsideWhileTrue.kt @@ -1,8 +1,8 @@ public fun foo(x: String?, y: String?): Int { while (true) { x ?: if (y == null) break - // y is not null in both branches - y.length() + // y is nullable if x != null + y.length() } // y is null because of the break return y.length() diff --git a/compiler/testData/diagnostics/tests/smartCasts/loops/elvisLeftBreakInsideWhileTrue.kt b/compiler/testData/diagnostics/tests/smartCasts/loops/elvisLeftBreakInsideWhileTrue.kt index bf043a200dd..68f8d3c9318 100644 --- a/compiler/testData/diagnostics/tests/smartCasts/loops/elvisLeftBreakInsideWhileTrue.kt +++ b/compiler/testData/diagnostics/tests/smartCasts/loops/elvisLeftBreakInsideWhileTrue.kt @@ -1,8 +1,8 @@ public fun foo(x: String?, y: String?): Int { while (true) { (if (x != null) break else y) ?: y!! - // x is not null in both branches - y.length() + // y is not null in both branches but it's hard to determine + y.length() } // y can be null because of the break return y.length() diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java index 503851af02d..712b1d24ad5 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java @@ -13167,6 +13167,24 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest { doTest(fileName); } + @TestMetadata("elvisExclExcl.kt") + public void testElvisExclExcl() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/smartCasts/elvisExclExcl.kt"); + doTest(fileName); + } + + @TestMetadata("elvisExclExclMerge.kt") + public void testElvisExclExclMerge() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/smartCasts/elvisExclExclMerge.kt"); + doTest(fileName); + } + + @TestMetadata("elvisExclExclPlatform.kt") + public void testElvisExclExclPlatform() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/smartCasts/elvisExclExclPlatform.kt"); + doTest(fileName); + } + @TestMetadata("elvisNothingRHS.kt") public void testElvisNothingRHS() throws Exception { String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/smartCasts/elvisNothingRHS.kt"); diff --git a/idea/tests/org/jetbrains/kotlin/idea/stubs/DebugTextByStubTest.kt b/idea/tests/org/jetbrains/kotlin/idea/stubs/DebugTextByStubTest.kt index 15b8c351bf3..d90a79bd9f7 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/stubs/DebugTextByStubTest.kt +++ b/idea/tests/org/jetbrains/kotlin/idea/stubs/DebugTextByStubTest.kt @@ -81,7 +81,7 @@ public class DebugTextByStubTest : LightCodeInsightFixtureTestCase() { val toCheckAgainst = "STUB: " + (expectedText ?: classByPsi!!.getText()) Assert.assertEquals(toCheckAgainst, psiFromStub.getDebugText()) if (expectedText != null) { - Assert.assertNotEquals("Expected text should not be specified", classByPsi.getDebugText(), psiFromStub.getDebugText()) + Assert.assertNotEquals("Expected text should not be specified", classByPsi!!.getDebugText(), psiFromStub.getDebugText()) } }