More precise diagnostics of smart cast impossible #KT-7240 Fixed

compiler/frontend.java/src/org/jetbrains/kotlin/jvm/RuntimeAssertions.kt

@@ -92,7 +92,7 @@ public object RuntimeAssertionsTypeChecker : AdditionalTypeChecker {
                 expressionType,
                 object : RuntimeAssertionInfo.DataFlowExtras {
                     override val canBeNull: Boolean
-                        get() = c.dataFlowInfo.getNullability(dataFlowValue).canBeNull()
+                        get() = c.dataFlowInfo.getPredictableNullability(dataFlowValue).canBeNull()
                     override val possibleTypes: Set<KotlinType>
                         get() = c.dataFlowInfo.getPossibleTypes(dataFlowValue)
                     override val presentableText: String

compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JavaNullabilityWarningsChecker.kt

@@ -71,7 +71,7 @@ public class JavaNullabilityWarningsChecker : AdditionalTypeChecker {
         if (TypeUtils.noExpectedType(expectedType)) return
 
         val expectedMustNotBeNull = expectedType.mustNotBeNull()
-        if (dataFlowInfo.getNullability(dataFlowValue) == Nullability.NOT_NULL) return
+        if (dataFlowInfo.getPredictableNullability(dataFlowValue) == Nullability.NOT_NULL) return
 
         val actualMayBeNull = expressionType.mayBeNull()
 
@@ -155,7 +155,7 @@ public class JavaNullabilityWarningsChecker : AdditionalTypeChecker {
     }
 
     private fun <T: Any> doIfNotNull(dataFlowValue: DataFlowValue, c: ResolutionContext<*>, body: () -> T): T? {
-        if (c.dataFlowInfo.getNullability(dataFlowValue).canBeNull()
+        if (c.dataFlowInfo.getPredictableNullability(dataFlowValue).canBeNull()
             && dataFlowValue.type.mustNotBeNull() == ErrorsJvm.NullabilityInformationSource.JAVA) {
             return body()
         }

compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CandidateResolver.kt

@@ -475,7 +475,7 @@ public class CandidateResolver(
 
         val bindingContext = trace.bindingContext
         val receiverValue = DataFlowValueFactory.createDataFlowValue(receiverArgument, bindingContext, scope.ownerDescriptor)
-        if (safeAccess && !dataFlowInfo.getNullability(receiverValue).canBeNull()) {
+        if (safeAccess && !dataFlowInfo.getPredictableNullability(receiverValue).canBeNull()) {
             tracing.unnecessarySafeCall(trace, receiverArgument.type)
         }
 

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

@@ -37,9 +37,19 @@ public interface DataFlowInfo {
     @NotNull
     SetMultimap<DataFlowValue, KotlinType> getCompleteTypeInfo();
 
+    /**
+     * Returns collected nullability for the given value, NOT taking its predictability into account.
+     */
     @NotNull
     Nullability getNullability(@NotNull DataFlowValue key);
 
+    /**
+     * Returns collected nullability for the given value if it's predictable.
+     * Otherwise basic value nullability is returned
+     */
+    @NotNull
+    Nullability getPredictableNullability(@NotNull DataFlowValue key);
+
     /**
      * IMPORTANT: by default, the original (native) type for this value
      * are NOT included. So it's quite possible to get an empty set here.

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

@@ -111,7 +111,18 @@ import static org.jetbrains.kotlin.resolve.calls.smartcasts.Nullability.NOT_NULL
     @Override
     @NotNull
     public Nullability getNullability(@NotNull DataFlowValue key) {
-        if (!key.isPredictable()) return key.getImmanentNullability();
+        return getNullability(key, false);
+    }
+
+    @Override
+    @NotNull
+    public Nullability getPredictableNullability(@NotNull DataFlowValue key) {
+        return getNullability(key, true);
+    }
+
+    @NotNull
+    private Nullability getNullability(@NotNull DataFlowValue key, boolean predictableOnly) {
+        if (predictableOnly && !key.isPredictable()) return key.getImmanentNullability();
         Nullability nullability = nullabilityInfo.get(key);
         return nullability != null ? nullability :
                parent != null ? parent.getNullability(key) :
@@ -123,7 +134,6 @@ import static org.jetbrains.kotlin.resolve.calls.smartcasts.Nullability.NOT_NULL
             @NotNull DataFlowValue value,
             @NotNull Nullability nullability
     ) {
-        if (!value.isPredictable()) return false;
         map.put(value, nullability);
         return nullability != getNullability(value);
     }

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

@@ -892,7 +892,7 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor {
 
     private static boolean isKnownToBeNotNull(KtExpression expression, KotlinType jetType, ExpressionTypingContext context) {
         DataFlowValue dataFlowValue = createDataFlowValue(expression, jetType, context);
-        return !context.dataFlowInfo.getNullability(dataFlowValue).canBeNull();
+        return !context.dataFlowInfo.getPredictableNullability(dataFlowValue).canBeNull();
     }
 
     /**
@@ -1191,7 +1191,7 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor {
             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()) {
+                || !rightDataFlowInfo.getPredictableNullability(leftValue).canBeNull()) {
                 dataFlowInfo = dataFlowInfo.disequate(leftValue, DataFlowValue.nullValue(components.builtIns));
             }
         }
@@ -1298,7 +1298,7 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor {
                         new Function1<DataFlowValue, Nullability>() {
                             @Override
                             public Nullability invoke(DataFlowValue value) {
-                                return context.dataFlowInfo.getNullability(value);
+                                return context.dataFlowInfo.getPredictableNullability(value);
                             }
                         });
             }

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

@@ -6,7 +6,7 @@ fun foo(arg: Int?) {
     if (x == null) return
     run {
         // Not safe: x = null later in the owner
-        x<!UNSAFE_CALL!>.<!>hashCode()
+        <!SMARTCAST_IMPOSSIBLE!>x<!>.hashCode()
     }
     x = null  
 }

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

@@ -8,8 +8,8 @@ fun foo() {
                 i = null
             }
         }
-        i<!UNSAFE_CALL!>.<!>hashCode()
+        <!SMARTCAST_IMPOSSIBLE!>i<!>.hashCode()
         Changing().bar()
-        i<!UNSAFE_CALL!>.<!>hashCode()
+        <!SMARTCAST_IMPOSSIBLE!>i<!>.hashCode()
     }
 }

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

@@ -7,8 +7,8 @@ fun foo() {
             i = null
             return true
         }
-        i<!UNSAFE_CALL!>.<!>hashCode()
-        trans(<!TYPE_MISMATCH!>i<!>, ::can)
-        i<!UNSAFE_CALL!>.<!>hashCode()
+        <!SMARTCAST_IMPOSSIBLE!>i<!>.hashCode()
+        trans(<!SMARTCAST_IMPOSSIBLE!>i<!>, ::can)
+        <!SMARTCAST_IMPOSSIBLE!>i<!>.hashCode()
     }
 }

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

@@ -10,6 +10,6 @@ fun foo() {
                 i = null
             }
         }.bar()
-        i<!UNSAFE_CALL!>.<!>hashCode()
+        <!SMARTCAST_IMPOSSIBLE!>i<!>.hashCode()
     }
 }

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

@@ -6,10 +6,10 @@ fun foo(arg: Int?) {
     if (x == null) return
     run {
         // Unsafe because of owner modification
-        x<!UNSAFE_CALL!>.<!>hashCode()
+        <!SMARTCAST_IMPOSSIBLE!>x<!>.hashCode()
         x = null
     }
     if (x != null) x = 42
     // Unsafe because of lambda
-    x<!UNSAFE_CALL!>.<!>hashCode()
+    <!SMARTCAST_IMPOSSIBLE!>x<!>.hashCode()
 }

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

@@ -0,0 +1,13 @@
+class Immutable(val x: String?) {
+    fun foo(): String {
+        if (x != null) return <!DEBUG_INFO_SMARTCAST!>x<!>
+        return ""
+    }
+}
+
+class Mutable(var y: String?) {
+    fun foo(): String {
+        if (y != null) return <!SMARTCAST_IMPOSSIBLE!>y<!>
+        return ""
+    }
+}

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

@@ -0,0 +1,19 @@
+package
+
+public final class Immutable {
+    public constructor Immutable(/*0*/ x: kotlin.String?)
+    public final val x: kotlin.String?
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public final fun foo(): kotlin.String
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+public final class Mutable {
+    public constructor Mutable(/*0*/ y: kotlin.String?)
+    public final var y: kotlin.String?
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public final fun foo(): kotlin.String
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}

compiler/testData/diagnostics/tests/smartCasts/publicVals/customGetter.kt

@@ -5,7 +5,7 @@ public class X {
     public fun fn(): Int {
         if (y != null)
             // With non-default getter smartcast is not possible
-            return y<!UNSAFE_CALL!>.<!>length
+            return <!SMARTCAST_IMPOSSIBLE!>y<!>.length
         else
             return 0
     }

compiler/testData/diagnostics/tests/smartCasts/publicVals/delegate.kt

@@ -13,7 +13,7 @@ class Example {
 
     public fun foo(): String {
         // Smart cast is not possible if property is delegated
-        return if (p != null) <!TYPE_MISMATCH!>p<!> else ""
+        return if (p != null) <!SMARTCAST_IMPOSSIBLE!>p<!> else ""
     }
 
     public fun bar(): String {

compiler/testData/diagnostics/tests/smartCasts/publicVals/otherModule.kt

@@ -17,7 +17,7 @@ import a.X
 public fun X.gav(): Int {
     if (x != null)
         // Smart cast is not possible if definition is in another module
-        return x<!UNSAFE_CALL!>.<!>length
+        return <!SMARTCAST_IMPOSSIBLE!>x<!>.length
     else
         return 0
 }
@@ -29,7 +29,7 @@ package a
 public fun X.gav(): Int {
     if (x != null)
         // Even if it's in the same package
-        return x<!UNSAFE_CALL!>.<!>length
+        return <!SMARTCAST_IMPOSSIBLE!>x<!>.length
     else
         return 0
 }

compiler/testData/diagnostics/tests/smartCasts/publicVals/var.kt

@@ -4,10 +4,10 @@ public class X {
     public fun fn(): Int {
         if (x != null)
             // Smartcast is not possible for variable properties
-            return x<!UNSAFE_CALL!>.<!>length
+            return <!SMARTCAST_IMPOSSIBLE!>x<!>.length
         else if (y != null)
             // Even if they are private
-            return y<!UNSAFE_CALL!>.<!>length
+            return <!SMARTCAST_IMPOSSIBLE!>y<!>.length
         else
             return 0
     }

compiler/testData/diagnostics/tests/smartCasts/varnotnull/varCapturedInClosure.kt

@@ -7,11 +7,11 @@ public fun foo() {
         } else if (s == null) {
             return -2
         } else {
-            return s<!UNSAFE_CALL!>.<!>length // Here smartcast is possible, at least in principle
+            return <!SMARTCAST_IMPOSSIBLE!>s<!>.length // Here smartcast is possible, at least in principle
         }
     }
     if (s != null) {
         System.out.println(closure())
-        System.out.println(s<!UNSAFE_CALL!>.<!>length)   // Here smartcast is not possible due to a closure predecessor
+        System.out.println(<!SMARTCAST_IMPOSSIBLE!>s<!>.length)   // Here smartcast is not possible due to a closure predecessor
     }
 }

compiler/testData/diagnostics/tests/smartCasts/varnotnull/varCapturedInInlineClosure.kt

@@ -8,7 +8,7 @@ fun IntArray.forEachIndexed( op: (i: Int, value: Int) -> Unit) {
 fun max(a: IntArray): Int? {
     var maxI: Int? = null
     a.forEachIndexed { i, value ->
-        if (maxI == null || value >= a[<!TYPE_MISMATCH!>maxI<!>])
+        if (maxI == null || value >= a[<!SMARTCAST_IMPOSSIBLE!>maxI<!>])
             maxI = i
     }
     return maxI

compiler/testData/diagnostics/testsWithStdLib/smartcasts/forEachSafe.kt

@@ -3,7 +3,7 @@
 fun indexOfMax(a: IntArray): Int? {
     var maxI: Int? = null
     a.forEachIndexed { i, value ->
-        if (maxI == null || value >= a[<!TYPE_MISMATCH!>maxI<!>]) {
+        if (maxI == null || value >= a[<!SMARTCAST_IMPOSSIBLE!>maxI<!>]) {
             maxI = i
         }
     }

compiler/testData/diagnostics/testsWithStdLib/smartcasts/letChangesToNull.kt

@@ -4,6 +4,6 @@ fun foo(y: String?) {
     var x: String? = ""
     if (x != null) {
         y?.let { x = null }
-        x<!UNSAFE_CALL!>.<!>length // Smart cast is not possible
+        <!SMARTCAST_IMPOSSIBLE!>x<!>.length // Smart cast is not possible
     }
 }

compiler/testData/diagnostics/testsWithStdLib/smartcasts/letChangesToNullComplex.kt

@@ -6,6 +6,6 @@ fun foo(y: String?) {
     var x: String? = ""
     if (x != null) {
         bar(y?.let { x = null; it })<!UNSAFE_CALL!>.<!>length
-        x<!UNSAFE_CALL!>.<!>length // Smart cast is not possible
+        <!SMARTCAST_IMPOSSIBLE!>x<!>.length // Smart cast is not possible
     }
 }

compiler/testData/diagnostics/testsWithStdLib/smartcasts/letMergeNotNull.kt

@@ -4,6 +4,6 @@ fun foo(y: String?) {
     var x: String? = null
     if (x != null) {
         y?.let { x = it }
-        x<!UNSAFE_CALL!>.<!>length // not-null or not-null
+        <!SMARTCAST_IMPOSSIBLE!>x<!>.length // not-null or not-null
     }
 }

compiler/testData/diagnostics/testsWithStdLib/smartcasts/withChangesToNull.kt

@@ -3,8 +3,8 @@ fun foo(y: String?) {
     if (x != null) {
         with(y?.let { x = null; it }) {
             this<!UNSAFE_CALL!>.<!>length
-            x<!UNSAFE_CALL!>.<!>length
+            <!SMARTCAST_IMPOSSIBLE!>x<!>.length
         }
-        x<!UNSAFE_CALL!>.<!>length
+        <!SMARTCAST_IMPOSSIBLE!>x<!>.length
     }
 }

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

@@ -14619,6 +14619,12 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest {
                 doTest(fileName);
             }
 
+            @TestMetadata("propertyToNotNull.kt")
+            public void testPropertyToNotNull() throws Exception {
+                String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/smartCasts/propertyToNotNull.kt");
+                doTest(fileName);
+            }
+
             @TestMetadata("thisWithLabel.kt")
             public void testThisWithLabel() throws Exception {
                 String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/smartCasts/thisWithLabel.kt");

idea/testData/checker/infos/SmartCasts.kt

@@ -231,8 +231,7 @@ class Mutable(var <info descr="This property has a backing field">x</info>: Stri
             return <error descr="[SMARTCAST_IMPOSSIBLE] Smart cast to 'kotlin.String' is impossible, because 'x' is a member variable that can be changed from another thread">x</error>
         }
         if (x != null) {
-            // It would be better to have smart cast impossible also here
-            return <error descr="[TYPE_MISMATCH] Type mismatch: inferred type is kotlin.String? but kotlin.String was expected">x</error>
+            return <error descr="[SMARTCAST_IMPOSSIBLE] Smart cast to 'kotlin.String' is impossible, because 'x' is a member variable that can be changed from another thread">x</error>
         }
         if (xx is String) {
             return <error descr="[SMARTCAST_IMPOSSIBLE] Smart cast to 'kotlin.String' is impossible, because 'xx' is a member value that has open or custom getter">xx</error>

idea/testData/refactoring/extractFunction/basic/extensionValUnderSmartCast.kt

@@ -1,14 +0,0 @@
-// SUGGESTED_NAMES: i, getKm
-// PARAM_TYPES: A
-// PARAM_DESCRIPTOR: val a: A defined in test
-class A {
-
-}
-
-val A.meters: Int? get() = 1
-
-fun test() {
-    val a = A()
-    if (a.meters == null) return
-    val km = <selection>a.meters / 10</selection>
-}

idea/testData/refactoring/extractFunction/basic/extensionValUnderSmartCast.kt.after

@@ -1,16 +0,0 @@
-// SUGGESTED_NAMES: i, getKm
-// PARAM_TYPES: A
-// PARAM_DESCRIPTOR: val a: A defined in test
-class A {
-
-}
-
-val A.meters: Int? get() = 1
-
-fun test() {
-    val a = A()
-    if (a.meters == null) return
-    val km = i(a)
-}
-
-private fun i(a: A) = a.meters / 10

idea/tests/org/jetbrains/kotlin/idea/refactoring/introduce/JetExtractionTestGenerated.java

@@ -504,12 +504,6 @@ public class JetExtractionTestGenerated extends AbstractJetExtractionTest {
                 doExtractFunctionTest(fileName);
             }
 
-            @TestMetadata("extensionValUnderSmartCast.kt")
-            public void testExtensionValUnderSmartCast() throws Exception {
-                String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/extractFunction/basic/extensionValUnderSmartCast.kt");
-                doExtractFunctionTest(fileName);
-            }
-
             @TestMetadata("extractBlockContent.kt")
             public void testExtractBlockContent() throws Exception {
                 String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/extractFunction/basic/extractBlockContent.kt");

jps-plugin/testData/incremental/pureKotlin/valAddCustomAccessor/build.log

@@ -12,4 +12,4 @@ Compiling files:
 src/usage.kt
 End of files
 COMPILATION FAILED
-Only safe (?.) or non-null asserted (!!.) calls are allowed on a nullable receiver of type kotlin.Int?
+Smart cast to 'kotlin.Int' is impossible, because 'a.x' is a member value that has open or custom getter

jps-plugin/testData/incremental/pureKotlin/valRemoveCustomAccessor/build.log

@@ -6,7 +6,7 @@ Compiling files:
 src/usage.kt
 End of files
 COMPILATION FAILED
-Only safe (?.) or non-null asserted (!!.) calls are allowed on a nullable receiver of type kotlin.Int?
+Smart cast to 'kotlin.Int' is impossible, because 'a.x' is a member value that has open or custom getter
 
 
 Cleaning output files: