DFA: take into account if / else and when expression assignments #KT-10002 Fixed

Also #KT-7479 Fixed

compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DataFlowValue.kt

@@ -31,6 +31,8 @@ class DataFlowValue(val id: Any?, val type: KotlinType, val kind: DataFlowValue.
         // or protected / public member value from the same module without open / custom getter
         // Smart casts are completely safe
         STABLE_VALUE("stable"),
+        // Block, or if / else, or when, or (in future) some other complex expression
+        STABLE_COMPLEX_EXPRESSION("complex expression", ""),
         // Member value with open / custom getter
         // Smart casts are not safe
         PROPERTY_WITH_GETTER("custom getter", "property that has open or custom getter"),
@@ -60,7 +62,7 @@ class DataFlowValue(val id: Any?, val type: KotlinType, val kind: DataFlowValue.
      * Predictable means here we do not expect some sudden change of their values,
      * like accessing mutable properties in another thread, so smart casts can be used safely.
      */
-    val isPredictable = (kind == Kind.STABLE_VALUE || kind == Kind.PREDICTABLE_VARIABLE)
+    val isPredictable = (kind == Kind.STABLE_VALUE || kind == Kind.STABLE_COMPLEX_EXPRESSION || kind == Kind.PREDICTABLE_VARIABLE)
         @JvmName("isPredictable") get
 
     override fun equals(other: Any?): Boolean {

compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DataFlowValueFactory.java

@@ -96,6 +96,10 @@ public class DataFlowValueFactory {
                                      Nullability.NOT_NULL);
         }
 
+        if (expression instanceof KtBlockExpression || expression instanceof KtIfExpression || expression instanceof KtWhenExpression) {
+            return createDataFlowValueForComplexExpression(expression, type);
+        }
+
         IdentifierInfo result = getIdForStableIdentifier(expression, bindingContext, containingDeclarationOrModule);
         return new DataFlowValue(result == NO_IDENTIFIER_INFO ? expression : result.id,
                                  type,
@@ -155,6 +159,14 @@ public class DataFlowValueFactory {
                                  getImmanentNullability(type));
     }
 
+    @NotNull
+    private static DataFlowValue createDataFlowValueForComplexExpression(
+            @NotNull KtExpression expression,
+            @NotNull KotlinType type
+    ) {
+        return new DataFlowValue(expression, type, Kind.STABLE_COMPLEX_EXPRESSION, getImmanentNullability(type));
+    }
+
     @NotNull
     private static Nullability getImmanentNullability(@NotNull KotlinType type) {
         return TypeUtils.isNullableType(type) ? Nullability.UNKNOWN : Nullability.NOT_NULL;

compiler/frontend/src/org/jetbrains/kotlin/types/expressions/ControlStructureTypingVisitor.java

@@ -32,6 +32,8 @@ import org.jetbrains.kotlin.resolve.ModifiersChecker;
 import org.jetbrains.kotlin.resolve.calls.model.MutableDataFlowInfoForArguments;
 import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall;
 import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowInfo;
+import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowValue;
+import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowValueFactory;
 import org.jetbrains.kotlin.resolve.descriptorUtil.DescriptorUtilsKt;
 import org.jetbrains.kotlin.resolve.inline.InlineUtil;
 import org.jetbrains.kotlin.resolve.scopes.LexicalScope;
@@ -137,12 +139,21 @@ public class ControlStructureTypingVisitor extends ExpressionTypingVisitor {
         KotlinTypeInfo elseTypeInfo = BindingContextUtils.getRecordedTypeInfo(elseBranch, bindingContext);
         assert thenTypeInfo != null : "'Then' branch of if expression  was not processed: " + ifExpression;
         assert elseTypeInfo != null : "'Else' branch of if expression  was not processed: " + ifExpression;
-        boolean loopBreakContinuePossible = thenTypeInfo.getJumpOutPossible() || elseTypeInfo.getJumpOutPossible();
 
+        KotlinType resultType = resolvedCall.getResultingDescriptor().getReturnType();
         KotlinType thenType = thenTypeInfo.getType();
         KotlinType elseType = elseTypeInfo.getType();
         DataFlowInfo thenDataFlowInfo = thenTypeInfo.getDataFlowInfo();
         DataFlowInfo elseDataFlowInfo = elseTypeInfo.getDataFlowInfo();
+        if (resultType != null && thenType != null && elseType != null) {
+            DataFlowValue resultValue = DataFlowValueFactory.createDataFlowValue(ifExpression, resultType, context);
+            DataFlowValue thenValue = DataFlowValueFactory.createDataFlowValue(thenBranch, thenType, context);
+            thenDataFlowInfo = thenDataFlowInfo.assign(resultValue, thenValue);
+            DataFlowValue elseValue = DataFlowValueFactory.createDataFlowValue(elseBranch, elseType, context);
+            elseDataFlowInfo = elseDataFlowInfo.assign(resultValue, elseValue);
+        }
+
+        boolean loopBreakContinuePossible = thenTypeInfo.getJumpOutPossible() || elseTypeInfo.getJumpOutPossible();
 
         boolean jumpInThen = thenType != null && KotlinBuiltIns.isNothing(thenType);
         boolean jumpInElse = elseType != null && KotlinBuiltIns.isNothing(elseType);
@@ -161,7 +172,6 @@ public class ControlStructureTypingVisitor extends ExpressionTypingVisitor {
             resultDataFlowInfo = thenDataFlowInfo.or(elseDataFlowInfo);
         }
 
-        KotlinType resultType = resolvedCall.getResultingDescriptor().getReturnType();
         // If break or continue was possible, take condition check info as the jump info
         return TypeInfoFactoryKt
                 .createTypeInfo(components.dataFlowAnalyzer.checkImplicitCast(resultType, ifExpression, contextWithExpectedType, isStatement),

compiler/frontend/src/org/jetbrains/kotlin/types/expressions/ExpressionTypingServices.java

@@ -29,6 +29,8 @@ import org.jetbrains.kotlin.resolve.*;
 import org.jetbrains.kotlin.resolve.calls.context.ContextDependency;
 import org.jetbrains.kotlin.resolve.calls.context.ResolutionContext;
 import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowInfo;
+import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowValue;
+import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowValueFactory;
 import org.jetbrains.kotlin.resolve.scopes.LexicalScope;
 import org.jetbrains.kotlin.resolve.scopes.LexicalScopeKind;
 import org.jetbrains.kotlin.resolve.scopes.LexicalWritableScope;
@@ -241,6 +243,13 @@ public class ExpressionTypingServices {
                 result = getTypeOfLastExpressionInBlock(
                         statementExpression, newContext.replaceExpectedType(context.expectedType), coercionStrategyForLastExpression,
                         blockLevelVisitor);
+                if (result.getType() != null && statementExpression.getParent() instanceof KtBlockExpression) {
+                    DataFlowValue lastExpressionValue = DataFlowValueFactory.createDataFlowValue(
+                            statementExpression, result.getType(), context);
+                    DataFlowValue blockExpressionValue = DataFlowValueFactory.createDataFlowValue(
+                            (KtBlockExpression) statementExpression.getParent(), result.getType(), context);
+                    result = result.replaceDataFlowInfo(result.getDataFlowInfo().assign(blockExpressionValue, lastExpressionValue));
+                }
             }
             else {
                 result = blockLevelVisitor

compiler/frontend/src/org/jetbrains/kotlin/types/expressions/PatternMatchingTypingVisitor.java

@@ -114,6 +114,7 @@ public class PatternMatchingTypingVisitor extends ExpressionTypingVisitor {
         Set<KotlinType> expressionTypes = Sets.newHashSet();
         DataFlowInfo commonDataFlowInfo = null;
         DataFlowInfo elseDataFlowInfo = context.dataFlowInfo;
+        DataFlowValue whenValue = DataFlowValueFactory.createDataFlowValue(expression, components.builtIns.getNullableAnyType(), context);
         for (KtWhenEntry whenEntry : expression.getEntries()) {
             DataFlowInfos infosForCondition = getDataFlowInfosForEntryCondition(
                     whenEntry, context.replaceDataFlowInfo(elseDataFlowInfo), subjectExpression, subjectType, subjectDataFlowValue);
@@ -131,6 +132,8 @@ public class PatternMatchingTypingVisitor extends ExpressionTypingVisitor {
                 KotlinType type = typeInfo.getType();
                 if (type != null) {
                     expressionTypes.add(type);
+                    DataFlowValue entryValue = DataFlowValueFactory.createDataFlowValue(bodyExpression, type, context);
+                    typeInfo = typeInfo.replaceDataFlowInfo(typeInfo.getDataFlowInfo().assign(whenValue, entryValue));
                 }
                 if (commonDataFlowInfo == null) {
                     commonDataFlowInfo = typeInfo.getDataFlowInfo();
@@ -150,9 +153,14 @@ public class PatternMatchingTypingVisitor extends ExpressionTypingVisitor {
             commonDataFlowInfo = commonDataFlowInfo.or(context.dataFlowInfo);
         }
 
+        KotlinType resultType = expressionTypes.isEmpty() ? null : CommonSupertypes.commonSupertype(expressionTypes);
+        if (resultType != null) {
+            DataFlowValue resultValue = DataFlowValueFactory.createDataFlowValue(expression, resultType, context);
+            commonDataFlowInfo = commonDataFlowInfo.assign(resultValue, whenValue);
+        }
         return TypeInfoFactoryKt.createTypeInfo(expressionTypes.isEmpty() ? null : components.dataFlowAnalyzer.checkType(
                                                              components.dataFlowAnalyzer.checkImplicitCast(
-                                                                     CommonSupertypes.commonSupertype(expressionTypes), expression,
+                                                                     resultType, expression,
                                                                      contextWithExpectedType, isStatement),
                                                              expression, contextWithExpectedType),
                                                 commonDataFlowInfo,

compiler/testData/diagnostics/tests/smartCasts/ifExprNonNull.kt

@@ -0,0 +1,19 @@
+fun baz(s: String?): String {
+    // If String type is given explicitly, problem disappears
+    val t = if (s == null) {
+        ""
+    }
+    else {
+        val u: String? = null
+        if (u == null) return ""
+        // !! is detected as unnecessary here
+        u
+    }
+    return <!DEBUG_INFO_SMARTCAST!>t<!>
+}
+
+fun foo(s: String?): String {
+    if (s == null) return ""
+    val t = if (s == "abc") <!DEBUG_INFO_SMARTCAST!>s<!> else "xyz"
+    return t
+}

compiler/testData/diagnostics/tests/smartCasts/ifExprNonNull.txt

@@ -0,0 +1,4 @@
+package
+
+public fun baz(/*0*/ s: kotlin.String?): kotlin.String
+public fun foo(/*0*/ s: kotlin.String?): kotlin.String

compiler/testData/diagnostics/tests/smartCasts/whenExprNonNull.kt

@@ -0,0 +1,18 @@
+fun baz(s: String?): String {
+    if (s == null) return ""
+    // if explicit type String is given for t, problem disappears
+    val t = when(<!DEBUG_INFO_SMARTCAST!>s<!>) {
+        // !! is detected as unnecessary here
+        "abc" -> s
+        else -> "xyz"
+    }
+    return <!DEBUG_INFO_SMARTCAST!>t<!>
+}
+
+fun foo(s: String?): String {
+    val t = when {
+        s != null -> s
+        else -> ""
+    }
+    return <!DEBUG_INFO_SMARTCAST!>t<!>
+}

compiler/testData/diagnostics/tests/smartCasts/whenExprNonNull.txt

@@ -0,0 +1,4 @@
+package
+
+public fun baz(/*0*/ s: kotlin.String?): kotlin.String
+public fun foo(/*0*/ s: kotlin.String?): kotlin.String

compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java

@@ -14961,6 +14961,12 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest {
                 doTest(fileName);
             }
 
+            @TestMetadata("ifExprNonNull.kt")
+            public void testIfExprNonNull() throws Exception {
+                String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/smartCasts/ifExprNonNull.kt");
+                doTest(fileName);
+            }
+
             @TestMetadata("implicitReceiver.kt")
             public void testImplicitReceiver() throws Exception {
                 String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/smartCasts/implicitReceiver.kt");
@@ -15291,6 +15297,12 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest {
                 doTest(fileName);
             }
 
+            @TestMetadata("whenExprNonNull.kt")
+            public void testWhenExprNonNull() throws Exception {
+                String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/smartCasts/whenExprNonNull.kt");
+                doTest(fileName);
+            }
+
             @TestMetadata("whenSubjectImpossible.kt")
             public void testWhenSubjectImpossible() throws Exception {
                 String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/smartCasts/whenSubjectImpossible.kt");