[NI] Fix smartcasts for conventional contains in when

Call argument for conventional `contains` after expanding `in` may come from a `when` subject during its branch analysis. In this case data flow info from a previous when branch was not considered, because data flow info for subject had been used instead of data flow before argument. Use of the latter one for the conventional `contains` solves the issue. The old FE uses `isExternal` property of value arguments to skip smartcast reporting on `when` subject, if they come from branches. To prevent undesired smartcasts on `when` subject after branch analysis in the new FE, `isExternal` arguments are skipped in diagnostic reporter and during recorded type update. Also, the new FE interprets `isExternal` completely differently from the old FE. In the old FE this property is used exclusively by `when` with subject. In the new FE it is also used for parially resolved calls, lambda return arguments and receivers. This may be preventing the use of data flow info before argument in the first place, but this assumption requires additional investigation. ^KT-36818 Fixed
2020-02-19 20:22:57 +03:00
parent afceec71a4
commit 07ca355af8
17 changed files with 183 additions and 22 deletions
@@ -1360,6 +1360,11 @@ public class FirOldFrontendDiagnosticsTestWithStdlibGenerated extends AbstractFi
                runTest("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/extensionReceiver_after.kt");
            }

+            @TestMetadata("externalArguments.kt")
+            public void testExternalArguments() throws Exception {
+                runTest("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/externalArguments.kt");
+            }
+
            @TestMetadata("intersectingInfo.kt")
            public void testIntersectingInfo() throws Exception {
                runTest("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/intersectingInfo.kt");
@@ -1508,6 +1513,11 @@ public class FirOldFrontendDiagnosticsTestWithStdlibGenerated extends AbstractFi
                    KotlinTestUtils.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/when"), Pattern.compile("^(.+)\\.kt$"), Pattern.compile("^(.+)\\.fir\\.kts?$"), true);
                }

+                @TestMetadata("kt36818.kt")
+                public void testKt36818() throws Exception {
+                    runTest("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/when/kt36818.kt");
+                }
+
                @TestMetadata("withSubject.kt")
                public void testWithSubject() throws Exception {
                    runTest("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/when/withSubject.kt");
@@ -251,10 +251,12 @@ class DiagnosticReporterByTrackingStrategy(
                )
                val dataFlowValue = dataFlowValueFactory.createDataFlowValue(expressionArgument.receiver.receiverValue, context)
                val call = if (call.callElement is KtBinaryExpression) null else call
-                smartCastManager.checkAndRecordPossibleCast(
-                    dataFlowValue, smartCastDiagnostic.smartCastType, argumentExpression, context, call,
-                    recordExpressionType = false
-                )
+                if (!expressionArgument.valueArgument.isExternal()) {
+                    smartCastManager.checkAndRecordPossibleCast(
+                        dataFlowValue, smartCastDiagnostic.smartCastType, argumentExpression, context, call,
+                        recordExpressionType = false
+                    )
+                } else null
            }
            is ReceiverExpressionKotlinCallArgument -> {
                trace.markAsReported()
@@ -130,7 +130,7 @@ class KotlinResolutionCallbacksImpl(
            return createSimplePSICallArgument(
                trace.bindingContext, outerCallContext.statementFilter, outerCallContext.scope.ownerDescriptor,
                CallMaker.makeExternalValueArgument(ktExpression), DataFlowInfo.EMPTY, typeInfo, languageVersionSettings,
-                dataFlowValueFactory
+                dataFlowValueFactory, outerCallContext.call
            )
        }

@@ -318,13 +318,15 @@ class KotlinToResolvedCallTransformer(
                )
            }

-            updateRecordedType(
-                argumentExpression,
-                parameter,
-                newContext,
-                constantConvertedArgument?.unknownIntegerType?.unwrap(),
-                resolvedCall.isReallySuccess()
-            )
+            if (!valueArgument.isExternal()) {
+                updateRecordedType(
+                    argumentExpression,
+                    parameter,
+                    newContext,
+                    constantConvertedArgument?.unknownIntegerType?.unwrap(),
+                    resolvedCall.isReallySuccess()
+                )
+            }
        }
    }

@@ -303,7 +303,8 @@ internal fun createSimplePSICallArgument(
    contextForArgument.scope.ownerDescriptor, valueArgument,
    contextForArgument.dataFlowInfo, typeInfoForArgument,
    contextForArgument.languageVersionSettings,
-    contextForArgument.dataFlowValueFactory
+    contextForArgument.dataFlowValueFactory,
+    contextForArgument.call,
 )

 internal fun createSimplePSICallArgument(
@@ -314,7 +315,8 @@ internal fun createSimplePSICallArgument(
    dataFlowInfoBeforeThisArgument: DataFlowInfo,
    typeInfoForArgument: KotlinTypeInfo,
    languageVersionSettings: LanguageVersionSettings,
-    dataFlowValueFactory: DataFlowValueFactory
+    dataFlowValueFactory: DataFlowValueFactory,
+    call: Call
 ): SimplePSIKotlinCallArgument? {

    val ktExpression = KtPsiUtil.getLastElementDeparenthesized(valueArgument.getArgumentExpression(), statementFilter) ?: return null
@@ -331,10 +333,10 @@ internal fun createSimplePSICallArgument(
            // so we use a fast-path here to avoid calling transformToReceiverWithSmartCastInfo function
            ReceiverValueWithSmartCastInfo(expressionReceiver, emptySet(), isStable = true)
        } else {
+            val useDataFlowInfoBeforeArgument = call.callType == Call.CallType.CONTAINS
            transformToReceiverWithSmartCastInfo(
                ownerDescriptor, bindingContext,
-                // dataFlowInfoBeforeThisArgument cannot be used here, because of if() { if (x != null) return; x }
-                typeInfoForArgument.dataFlowInfo,
+                if (useDataFlowInfoBeforeArgument) dataFlowInfoBeforeThisArgument else typeInfoForArgument.dataFlowInfo,
                expressionReceiver,
                languageVersionSettings,
                dataFlowValueFactory
@@ -1402,9 +1402,14 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor {
        ExpressionReceiver receiver = safeGetExpressionReceiver(facade, right, contextWithNoExpectedType);
        ExpressionTypingContext contextWithDataFlow = context.replaceDataFlowInfo(dataFlowInfo);

+        Call containsCall = CallMaker.makeCall(
+                callElement, receiver, null, operationSign,
+                Collections.singletonList(leftArgument), Call.CallType.CONTAINS
+        );
+
        OverloadResolutionResults<FunctionDescriptor> resolutionResult = components.callResolver.resolveCallWithGivenName(
                contextWithDataFlow,
-                CallMaker.makeCall(callElement, receiver, null, operationSign, Collections.singletonList(leftArgument)),
+                containsCall,
                operationSign,
                OperatorNameConventions.CONTAINS);
        KotlinType containsType = OverloadResolutionResultsUtil.getResultingType(resolutionResult, context);
@@ -67,7 +67,7 @@ public interface Call {
    KtElement getCallElement();

    enum CallType {
-        DEFAULT, ARRAY_GET_METHOD, ARRAY_SET_METHOD, INVOKE
+        DEFAULT, ARRAY_GET_METHOD, ARRAY_SET_METHOD, INVOKE, CONTAINS
    }

    @NotNull
@@ -0,0 +1,37 @@
+// !DIAGNOSTICS: -UNUSED_VARIABLE
+// !WITH_NEW_INFERENCE
+
+import kotlin.reflect.KProperty
+
+fun testLambdaArgumentSmartCast(foo: Int?) {
+    val v = run {
+        if (foo != null)
+            return@run foo
+        15
+    }
+}
+
+class D {
+    operator fun getValue(ref: Any?, property: KProperty<*>): Int = 42
+}
+
+fun testSmartCastInDelegate(d: D?) {
+    if (d == null) return
+    val v: Int by d
+}
+
+fun testFunctionCallSmartcast(fn: (() -> Unit)?) {
+    if (fn == null) return
+
+    fn()
+}
+
+fun testCallableRefernceSmartCast() {
+    fun forReference() {}
+
+    val refernece = if (true) ::forReference else null
+    if (refernece == null)
+        return
+
+    refernece()
+}
@@ -0,0 +1,37 @@
+// !DIAGNOSTICS: -UNUSED_VARIABLE
+// !WITH_NEW_INFERENCE
+
+import kotlin.reflect.KProperty
+
+fun testLambdaArgumentSmartCast(foo: Int?) {
+    val v = run {
+        if (foo != null)
+            return@run <!NI;DEBUG_INFO_SMARTCAST!>foo<!>
+        15
+    }
+}
+
+class D {
+    operator fun getValue(ref: Any?, property: KProperty<*>): Int = 42
+}
+
+fun testSmartCastInDelegate(d: D?) {
+    if (d == null) return
+    val v: Int by <!DEBUG_INFO_SMARTCAST!>d<!>
+}
+
+fun testFunctionCallSmartcast(fn: (() -> Unit)?) {
+    if (fn == null) return
+
+    <!DEBUG_INFO_SMARTCAST!>fn<!>()
+}
+
+fun testCallableRefernceSmartCast() {
+    fun forReference() {}
+
+    val refernece = if (true) ::forReference else null
+    if (refernece == null)
+        return
+
+    <!DEBUG_INFO_SMARTCAST!>refernece<!>()
+}
@@ -0,0 +1,14 @@
+package
+
+public fun testCallableRefernceSmartCast(): kotlin.Unit
+public fun testFunctionCallSmartcast(/*0*/ fn: (() -> kotlin.Unit)?): kotlin.Unit
+public fun testLambdaArgumentSmartCast(/*0*/ foo: kotlin.Int?): kotlin.Unit
+public fun testSmartCastInDelegate(/*0*/ d: D?): kotlin.Unit
+
+public final class D {
+    public constructor D()
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public final operator fun getValue(/*0*/ ref: kotlin.Any?, /*1*/ property: kotlin.reflect.KProperty<*>): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
@@ -0,0 +1,11 @@
+fun main(x1: Double?, range: ClosedRange<Double>) {
+    when (x1) {
+        null -> throw Exception()
+        <!INAPPLICABLE_CANDIDATE!>in<!> range -> {} // error, no smartcast from previous branch, OK in OI
+    }
+
+    when {
+        x1 == null -> throw Exception()
+        x1 in range -> {}
+    }
+}
@@ -0,0 +1,11 @@
+fun main(x1: Double?, range: ClosedRange<Double>) {
+    when (x1) {
+        null -> throw Exception()
+        in range -> {} // error, no smartcast from previous branch, OK in OI
+    }
+
+    when {
+        x1 == null -> throw Exception()
+        <!DEBUG_INFO_SMARTCAST!>x1<!> in range -> {}
+    }
+}
@@ -0,0 +1,3 @@
+package
+
+public fun main(/*0*/ x1: kotlin.Double?, /*1*/ range: kotlin.ranges.ClosedRange<kotlin.Double>): kotlin.Unit
@@ -47,10 +47,11 @@ FILE fqName:<root> fileName:/when.kt
              then: CONST String type=kotlin.String value="!Number"
            BRANCH
              if: CALL 'public final fun contains <T> (element: T of kotlin.collections.contains): kotlin.Boolean [operator] declared in kotlin.collections' type=kotlin.Boolean origin=IN
-                <T>: kotlin.Any?
+                <T>: kotlin.Number
                $receiver: CALL 'public final fun setOf <T> (): kotlin.collections.Set<T of kotlin.collections.setOf> [inline] declared in kotlin.collections' type=kotlin.collections.Set<kotlin.Nothing> origin=null
                  <T>: kotlin.Nothing
-                element: GET_VAR 'val tmp_0: kotlin.Any? [val] declared in <root>.testWithSubject' type=kotlin.Any? origin=null
+                element: TYPE_OP type=kotlin.Number origin=IMPLICIT_CAST typeOperand=kotlin.Number
+                  GET_VAR 'val tmp_0: kotlin.Any? [val] declared in <root>.testWithSubject' type=kotlin.Any? origin=null
              then: CONST String type=kotlin.String value="nothingness?"
            BRANCH
              if: CONST Boolean type=kotlin.Boolean value=true
@@ -1361,6 +1361,11 @@ public class DiagnosticsTestWithStdLibGenerated extends AbstractDiagnosticsTestW
                runTest("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/extensionReceiver_after.kt");
            }

+            @TestMetadata("externalArguments.kt")
+            public void testExternalArguments() throws Exception {
+                runTest("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/externalArguments.kt");
+            }
+
            @TestMetadata("intersectingInfo.kt")
            public void testIntersectingInfo() throws Exception {
                runTest("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/intersectingInfo.kt");
@@ -1509,6 +1514,11 @@ public class DiagnosticsTestWithStdLibGenerated extends AbstractDiagnosticsTestW
                    KotlinTestUtils.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/when"), Pattern.compile("^(.+)\\.kt$"), Pattern.compile("^(.+)\\.fir\\.kts?$"), true);
                }

+                @TestMetadata("kt36818.kt")
+                public void testKt36818() throws Exception {
+                    runTest("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/when/kt36818.kt");
+                }
+
                @TestMetadata("withSubject.kt")
                public void testWithSubject() throws Exception {
                    runTest("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/when/withSubject.kt");
@@ -1361,6 +1361,11 @@ public class DiagnosticsTestWithStdLibUsingJavacGenerated extends AbstractDiagno
                runTest("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/extensionReceiver_after.kt");
            }

+            @TestMetadata("externalArguments.kt")
+            public void testExternalArguments() throws Exception {
+                runTest("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/externalArguments.kt");
+            }
+
            @TestMetadata("intersectingInfo.kt")
            public void testIntersectingInfo() throws Exception {
                runTest("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/intersectingInfo.kt");
@@ -1509,6 +1514,11 @@ public class DiagnosticsTestWithStdLibUsingJavacGenerated extends AbstractDiagno
                    KotlinTestUtils.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/when"), Pattern.compile("^(.+)\\.kt$"), Pattern.compile("^(.+)\\.fir\\.kts?$"), true);
                }

+                @TestMetadata("kt36818.kt")
+                public void testKt36818() throws Exception {
+                    runTest("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/when/kt36818.kt");
+                }
+
                @TestMetadata("withSubject.kt")
                public void testWithSubject() throws Exception {
                    runTest("compiler/testData/diagnostics/testsWithStdLib/contracts/smartcasts/when/withSubject.kt");
@@ -257,8 +257,14 @@ object SuperCallCase : FunctionCallCase() {
 }

 object DynamicInvokeAndBracketAccessCallCase : FunctionCallCase() {
-    fun canApply(callInfo: FunctionCallInfo): Boolean =
-            callInfo.resolvedCall.call.callType != Call.CallType.DEFAULT && callInfo.callableDescriptor.isDynamic()
+    fun canApply(callInfo: FunctionCallInfo): Boolean {
+        if (!callInfo.callableDescriptor.isDynamic())
+            return false
+        val callType = callInfo.resolvedCall.call.callType
+        return callType == Call.CallType.ARRAY_GET_METHOD
+                || callType == Call.CallType.ARRAY_SET_METHOD
+                || callType == Call.CallType.INVOKE
+    }

    override fun FunctionCallInfo.dispatchReceiver(): JsExpression {
        val arguments = argumentsInfo.translateArguments