From 11f50186fa526e7949decf87b09c65bbaa18acd0 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Tue, 19 Jul 2016 11:17:16 +0300 Subject: [PATCH] x?.y != null and x?.call() != null provoke now x != null, a set of tests #KT-2127 Fixed --- .../calls/smartcasts/DataFlowValueFactory.kt | 31 +++-- .../smartcasts/DelegatingDataFlowInfo.kt | 16 ++- .../calls/smartcasts/IdentifierInfo.kt | 21 ++- .../nestedCalls/makeNullableIfSafeCall.kt | 10 +- .../tests/inference/regressions/kt702.kt | 2 +- .../diagnostics/tests/regressions/Jet72.kt | 3 +- .../diagnostics/tests/regressions/kt701.kt | 2 +- .../smartCasts/safecalls/propertyChain.kt | 4 +- .../safecalls/safeAccessReceiverNotNull.kt | 125 ++++++++++++++++++ .../safecalls/safeAccessReceiverNotNull.txt | 61 +++++++++ .../checkers/DiagnosticsTestGenerated.java | 6 + 11 files changed, 249 insertions(+), 32 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/smartCasts/safecalls/safeAccessReceiverNotNull.kt create mode 100644 compiler/testData/diagnostics/tests/smartCasts/safecalls/safeAccessReceiverNotNull.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DataFlowValueFactory.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DataFlowValueFactory.kt index f1653c264cb..77ddd4abe96 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DataFlowValueFactory.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DataFlowValueFactory.kt @@ -180,10 +180,11 @@ object DataFlowValueFactory { is KtQualifiedExpression -> { val receiverExpression = expression.receiverExpression val selectorExpression = expression.selectorExpression - val receiverId = getIdForStableIdentifier(receiverExpression, bindingContext, containingDeclarationOrModule) - val selectorId = getIdForStableIdentifier(selectorExpression, bindingContext, containingDeclarationOrModule) + val receiverInfo = getIdForStableIdentifier(receiverExpression, bindingContext, containingDeclarationOrModule) + val selectorInfo = getIdForStableIdentifier(selectorExpression, bindingContext, containingDeclarationOrModule) - IdentifierInfo.qualified(receiverId, selectorId, expression.operationSign === KtTokens.SAFE_ACCESS) + IdentifierInfo.qualified(receiverInfo, bindingContext.getType(receiverExpression), + selectorInfo, expression.operationSign === KtTokens.SAFE_ACCESS) } is KtSimpleNameExpression -> getIdForSimpleNameExpression(expression, bindingContext, containingDeclarationOrModule) @@ -219,13 +220,25 @@ object DataFlowValueFactory { // for now it fails for resolving 'invoke' convention, return it after 'invoke' algorithm changes // assert resolvedCall != null : "Cannot create right identifier info if the resolved call is not known yet for val usageModuleDescriptor = DescriptorUtils.getContainingModuleOrNull(containingDeclarationOrModule) - val receiverInfo = resolvedCall?.let { getIdForImplicitReceiver(it.dispatchReceiver, simpleNameExpression) } + val selectorInfo = IdentifierInfo.Variable(declarationDescriptor, + variableKind(declarationDescriptor, usageModuleDescriptor, + bindingContext, simpleNameExpression)) - IdentifierInfo.qualified(receiverInfo, - IdentifierInfo.Variable(declarationDescriptor, - variableKind(declarationDescriptor, usageModuleDescriptor, - bindingContext, simpleNameExpression)), - resolvedCall?.call?.isSafeCall() ?: false) + val implicitReceiver = resolvedCall?.dispatchReceiver + if (implicitReceiver == null) { + selectorInfo + } + else { + val receiverInfo = getIdForImplicitReceiver(implicitReceiver, simpleNameExpression) + + if (receiverInfo == null) { + selectorInfo + } + else { + IdentifierInfo.qualified(receiverInfo, implicitReceiver.type, + selectorInfo, resolvedCall?.call?.isSafeCall() ?: false) + } + } } is PackageViewDescriptor, is ClassDescriptor -> IdentifierInfo.PackageOrClass(declarationDescriptor) else -> IdentifierInfo.NO diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DelegatingDataFlowInfo.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DelegatingDataFlowInfo.kt index fadc1e42efb..d884ff9cdf0 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DelegatingDataFlowInfo.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DelegatingDataFlowInfo.kt @@ -90,8 +90,18 @@ internal class DelegatingDataFlowInfo private constructor( } } - private fun putNullability(map: MutableMap, value: DataFlowValue, nullability: Nullability): Boolean { + private fun putNullability(map: MutableMap, value: DataFlowValue, + nullability: Nullability, affectReceiver: Boolean = true): Boolean { map.put(value, nullability) + + val identifierInfo = value.identifierInfo + if (affectReceiver && !nullability.canBeNull() && identifierInfo is IdentifierInfo.Qualified) { + val receiverType = identifierInfo.receiverType + if (identifierInfo.safe && receiverType != null) { + putNullability(map, DataFlowValue(identifierInfo.receiverInfo, receiverType), nullability) + } + } + return nullability != getCollectedNullability(value) } @@ -135,7 +145,7 @@ internal class DelegatingDataFlowInfo private constructor( override fun assign(a: DataFlowValue, b: DataFlowValue): DataFlowInfo { val nullability = Maps.newHashMap() val nullabilityOfB = getPredictableNullability(b) - putNullability(nullability, a, nullabilityOfB) + putNullability(nullability, a, nullabilityOfB, affectReceiver = false) val newTypeInfo = newTypeInfo() var typesForB = getPredictableTypes(b) @@ -212,7 +222,7 @@ internal class DelegatingDataFlowInfo private constructor( val nullabilityOfA = getPredictableNullability(a) val nullabilityOfB = getPredictableNullability(b) - var changed = putNullability(builder, a, nullabilityOfA.refine(nullabilityOfB.invert())) or + val changed = putNullability(builder, a, nullabilityOfA.refine(nullabilityOfB.invert())) or putNullability(builder, b, nullabilityOfB.refine(nullabilityOfA.invert())) return if (changed) create(this, ImmutableMap.copyOf(builder), EMPTY_TYPE_INFO) else this } diff --git a/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/smartcasts/IdentifierInfo.kt b/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/smartcasts/IdentifierInfo.kt index 41372cb104f..ec9198f7e4e 100644 --- a/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/smartcasts/IdentifierInfo.kt +++ b/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/smartcasts/IdentifierInfo.kt @@ -26,6 +26,7 @@ import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowValue.Kind.STABLE_V import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe import org.jetbrains.kotlin.resolve.scopes.receivers.ImplicitReceiver import org.jetbrains.kotlin.resolve.scopes.receivers.ReceiverValue +import org.jetbrains.kotlin.types.KotlinType interface IdentifierInfo { @@ -72,7 +73,8 @@ interface IdentifierInfo { } class Qualified( - val receiverInfo: IdentifierInfo, val selectorInfo: IdentifierInfo, val safe: Boolean + val receiverInfo: IdentifierInfo, val selectorInfo: IdentifierInfo, + val safe: Boolean, val receiverType: KotlinType? ) : IdentifierInfo { override val kind: DataFlowValue.Kind get() = if (receiverInfo.kind.isStable()) selectorInfo.kind else OTHER @@ -89,16 +91,13 @@ interface IdentifierInfo { companion object { - fun qualified(receiverInfo: IdentifierInfo?, selectorInfo: IdentifierInfo, safe: Boolean): IdentifierInfo { - return if (selectorInfo == NO || receiverInfo === NO) { - NO - } - else if (receiverInfo == null || receiverInfo is PackageOrClass) { - selectorInfo - } - else { - Qualified(receiverInfo, selectorInfo, safe) - } + fun qualified( + receiverInfo: IdentifierInfo, receiverType: KotlinType?, + selectorInfo: IdentifierInfo, safe: Boolean + ) = when (receiverInfo) { + NO -> NO + is PackageOrClass -> selectorInfo + else -> Qualified(receiverInfo, selectorInfo, safe, receiverType) } } } \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/inference/nestedCalls/makeNullableIfSafeCall.kt b/compiler/testData/diagnostics/tests/inference/nestedCalls/makeNullableIfSafeCall.kt index 247b064a19a..79d463d9d7e 100644 --- a/compiler/testData/diagnostics/tests/inference/nestedCalls/makeNullableIfSafeCall.kt +++ b/compiler/testData/diagnostics/tests/inference/nestedCalls/makeNullableIfSafeCall.kt @@ -11,14 +11,16 @@ interface B { fun test(u: A?, x: A?, y: A?, z: A?, w: A, v: A?) { u?.b?.foo()!! // was UNNECESSARY_SAFE_CALL everywhere, because result type (of 'foo()') wasn't made nullable - u!!.b?.foo()!! + u!!.b?.foo()!! x?.b!!.foo()!! - x!!.b!!.foo()!! + // x?.b is not null + x!!.b!!.foo()!! y?.nb?.foo()!! - y!!.nb?.foo()!! + y!!.nb?.foo()!! z?.nb!!.foo()!! - z!!.nb!!.foo()!! + // z?.nb is not null + z!!.nb!!.foo()!! w.b?.foo()!! w.b!!.foo()!! diff --git a/compiler/testData/diagnostics/tests/inference/regressions/kt702.kt b/compiler/testData/diagnostics/tests/inference/regressions/kt702.kt index 656f9bba3ad..71f5142a0f6 100644 --- a/compiler/testData/diagnostics/tests/inference/regressions/kt702.kt +++ b/compiler/testData/diagnostics/tests/inference/regressions/kt702.kt @@ -9,7 +9,7 @@ public class Throwables() { public fun propagateIfInstanceOf(throwable : Throwable?, declaredType : Class?) : Unit { if (((throwable != null) && declaredType?.isInstance(throwable)!!)) { - throw declaredType?.cast(throwable)!! + throw declaredType?.cast(throwable)!! } } public fun propagateIfPossible(throwable : Throwable?) : Unit { diff --git a/compiler/testData/diagnostics/tests/regressions/Jet72.kt b/compiler/testData/diagnostics/tests/regressions/Jet72.kt index 0943fd5a2d8..86a39f57ba0 100644 --- a/compiler/testData/diagnostics/tests/regressions/Jet72.kt +++ b/compiler/testData/diagnostics/tests/regressions/Jet72.kt @@ -11,7 +11,8 @@ val items: ArrayList = ArrayList() fun test(room : Object) { for(item: Item? in items) { if (item?.room === room) { - System.out.println("You see " + item?.name) + // item?.room is not null + System.out.println("You see " + item?.name) } } } \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/regressions/kt701.kt b/compiler/testData/diagnostics/tests/regressions/kt701.kt index 019099348bb..5be7b22e089 100644 --- a/compiler/testData/diagnostics/tests/regressions/kt701.kt +++ b/compiler/testData/diagnostics/tests/regressions/kt701.kt @@ -6,7 +6,7 @@ public class Throwables() { public fun propagateIfInstanceOf(throwable : Throwable?, declaredType : Class?) { if (((throwable != null) && declaredType?.isInstance(throwable)!!)) { - throw declaredType?.cast(throwable)!! + throw declaredType?.cast(throwable)!! } } public fun propagateIfPossible(throwable : Throwable?) { diff --git a/compiler/testData/diagnostics/tests/smartCasts/safecalls/propertyChain.kt b/compiler/testData/diagnostics/tests/smartCasts/safecalls/propertyChain.kt index 43bf6b3472f..89728b8497a 100644 --- a/compiler/testData/diagnostics/tests/smartCasts/safecalls/propertyChain.kt +++ b/compiler/testData/diagnostics/tests/smartCasts/safecalls/propertyChain.kt @@ -9,6 +9,6 @@ fun foo(y: MyClass?): Int { } fun bar(y: MyClass?) { y?.x!!.length - // !! is necessary here - y!!.x + // !! is NOT necessary here, because y?.x != null + y!!.x } \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/smartCasts/safecalls/safeAccessReceiverNotNull.kt b/compiler/testData/diagnostics/tests/smartCasts/safecalls/safeAccessReceiverNotNull.kt new file mode 100644 index 00000000000..04a619206a8 --- /dev/null +++ b/compiler/testData/diagnostics/tests/smartCasts/safecalls/safeAccessReceiverNotNull.kt @@ -0,0 +1,125 @@ +// A set of examples for +// "If the result of a safe call is not null, understand that its receiver is not null" +// and some other improvements for nullability detection + +fun kt6840_1(s: String?) { + val hash = s?.hashCode() + if (hash != null) { + // To be supported + s.length + } +} + +fun kt6840_2(s: String?) { + if (s?.hashCode() != null) { + s.length + } +} + +fun kt1635(s: String?) { + s?.hashCode()!! + s.hashCode() +} + +fun kt2127() { + val s: String? = "" + if (s?.length != null) { + s.hashCode() + } +} + +fun kt3356(s: String?): Int { + if (s?.length != 1) return 0 + return s.length +} + +open class SomeClass(val data: Any) + +class SubClass(val extra: Any, data: Any) : SomeClass(data) + +fun kt4565_1(a: SomeClass?) { + val data = a?.data + if (data != null) { + data.hashCode() + // To be supported + a.hashCode() + a.data.hashCode() + } + if (a?.data != null) { + // To be supported + data.hashCode() + a.hashCode() + a.data.hashCode() + } + if (a?.data is String) { + // To be supported + a.data.length + data.length + } +} + +fun kt4565_2(a: SomeClass?) { + // To be supported + if (a as? SubClass != null) { + a.extra.hashCode() + } + val extra = (a as? SubClass)?.extra + if (extra != null) { + a.extra.hashCode() + } +} + +class A(val y: Int) + +fun kt7491_1() { + val x: A? = A(42) + val z = x?.y ?: return + x.y +} + +fun getA(): A? = null +fun useA(a: A): Int = a.hashCode() + +fun kt7491_2() { + val a = getA() + (a?.let { useA(a) } ?: a.y ).toString() +} + +fun String.correct() = true + +fun kt8492(s: String?) { + if (s?.correct() ?: false) { + // To be supported + s.length + } +} + +fun kt11085(prop: String?) { + when (prop?.hashCode()) { + 1 -> prop.length + } +} + +class HttpExchange(val code: String) + +fun kt11313(arg: HttpExchange?) { + when (arg?.code) { + "GET" -> handleGet(arg) + "POST" -> handlePost(arg) + } +} + +fun handleGet(arg: HttpExchange) = arg + +fun handlePost(arg: HttpExchange) = arg + +class Wrapper { + fun unwrap(): String? = "Something not consistent" +} + +fun falsePositive(w: Wrapper) { + if (w.unwrap() != null) { + // Here we should NOT have smart cast + w.unwrap().length + } +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/smartCasts/safecalls/safeAccessReceiverNotNull.txt b/compiler/testData/diagnostics/tests/smartCasts/safecalls/safeAccessReceiverNotNull.txt new file mode 100644 index 00000000000..75a9a529cec --- /dev/null +++ b/compiler/testData/diagnostics/tests/smartCasts/safecalls/safeAccessReceiverNotNull.txt @@ -0,0 +1,61 @@ +package + +public fun falsePositive(/*0*/ w: Wrapper): kotlin.Unit +public fun getA(): A? +public fun handleGet(/*0*/ arg: HttpExchange): HttpExchange +public fun handlePost(/*0*/ arg: HttpExchange): HttpExchange +public fun kt11085(/*0*/ prop: kotlin.String?): kotlin.Unit +public fun kt11313(/*0*/ arg: HttpExchange?): kotlin.Unit +public fun kt1635(/*0*/ s: kotlin.String?): kotlin.Unit +public fun kt2127(): kotlin.Unit +public fun kt3356(/*0*/ s: kotlin.String?): kotlin.Int +public fun kt4565_1(/*0*/ a: SomeClass?): kotlin.Unit +public fun kt4565_2(/*0*/ a: SomeClass?): kotlin.Unit +public fun kt6840_1(/*0*/ s: kotlin.String?): kotlin.Unit +public fun kt6840_2(/*0*/ s: kotlin.String?): kotlin.Unit +public fun kt7491_1(): kotlin.Unit +public fun kt7491_2(): kotlin.Unit +public fun kt8492(/*0*/ s: kotlin.String?): kotlin.Unit +public fun useA(/*0*/ a: A): kotlin.Int +public fun kotlin.String.correct(): kotlin.Boolean + +public final class A { + public constructor A(/*0*/ y: kotlin.Int) + public final val y: kotlin.Int + 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 +} + +public final class HttpExchange { + public constructor HttpExchange(/*0*/ code: kotlin.String) + public final val code: kotlin.String + 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 +} + +public open class SomeClass { + public constructor SomeClass(/*0*/ data: kotlin.Any) + public final val data: kotlin.Any + 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 +} + +public final class SubClass : SomeClass { + public constructor SubClass(/*0*/ extra: kotlin.Any, /*1*/ data: kotlin.Any) + public final override /*1*/ /*fake_override*/ val data: kotlin.Any + public final val extra: kotlin.Any + 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 +} + +public final class Wrapper { + public constructor Wrapper() + 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 + public final fun unwrap(): kotlin.String? +} diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index cf0ff71016f..92f9acdd1c6 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -18356,6 +18356,12 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { doTest(fileName); } + @TestMetadata("safeAccessReceiverNotNull.kt") + public void testSafeAccessReceiverNotNull() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/smartCasts/safecalls/safeAccessReceiverNotNull.kt"); + doTest(fileName); + } + @TestMetadata("simple.kt") public void testSimple() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/smartCasts/safecalls/simple.kt");