More precise diagnostics is added for smart cast impossible #KT-8810 Fixed

compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java

@@ -654,7 +654,7 @@ public interface Errors {
 
     DiagnosticFactory1<KtExpression, KotlinType> IMPLICIT_CAST_TO_UNIT_OR_ANY = DiagnosticFactory1.create(WARNING);
 
-    DiagnosticFactory2<KtExpression, KotlinType, String> SMARTCAST_IMPOSSIBLE = DiagnosticFactory2.create(ERROR);
+    DiagnosticFactory3<KtExpression, KotlinType, String, String> SMARTCAST_IMPOSSIBLE = DiagnosticFactory3.create(ERROR);
 
     DiagnosticFactory0<KtNullableType> USELESS_NULLABLE_CHECK = DiagnosticFactory0.create(WARNING, NULLABLE_TYPE);
 

compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java

@@ -477,7 +477,7 @@ public class DefaultErrorMessages {
             }
         }, DECLARATION_NAME);
         MAP.put(SMARTCAST_IMPOSSIBLE,
-                "Smart cast to ''{0}'' is impossible, because ''{1}'' could have changed since the is-check", RENDER_TYPE, STRING);
+                "Smart cast to ''{0}'' is impossible, because ''{1}'' is a {2}", RENDER_TYPE, STRING, STRING);
 
         MAP.put(MISSING_CONSTRUCTOR_KEYWORD, "Use 'constructor' keyword after modifiers of primary constructor");
 

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

@@ -453,7 +453,7 @@ public class CandidateResolver(
             val expression = (receiverArgument as? ExpressionReceiver)?.expression
             val dataFlowValue = DataFlowValueFactory.createDataFlowValue(receiverArgument, this)
 
-            val smartCastResult = smartCastManager.checkAndRecordPossibleCast(
+            val smartCastResult = SmartCastManager.checkAndRecordPossibleCast(
                     dataFlowValue, expectedReceiverParameterType, expression, this, /*recordType =*/ true
             )
 

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

@@ -26,14 +26,29 @@ import org.jetbrains.kotlin.types.KotlinType
  */
 class DataFlowValue(val id: Any?, val type: KotlinType, val kind: DataFlowValue.Kind, val immanentNullability: Nullability) {
 
-    enum class Kind(private val str: String) {
+    enum class Kind(private val str: String, val description: String = str) {
+        // Local value, or parameter, or private / internal member value without open / custom getter,
+        // or protected / public member value from the same module without open / custom getter
         // Smart casts are completely safe
         STABLE_VALUE("stable"),
-        // Smart casts are safe but possible changes in loops / closures ahead must be taken into account
+        // Member value with open / custom getter
-        PREDICTABLE_VARIABLE("predictable"),
         // Smart casts are not safe
-        UNPREDICTABLE_VARIABLE("unpredictable"),
+        MEMBER_VALUE_WITH_GETTER("custom getter", "member value that has open or custom getter"),
-        OTHER("other");
+        // Protected / public member value from another module
+        // Smart casts are not safe
+        ALIEN_PUBLIC_VALUE("alien public", "public API member value declared in different module"),
+        // Local variable not yet captured by a changing closure
+        // Smart casts are safe but possible changes in loops / closures ahead must be taken into account
+        PREDICTABLE_VARIABLE("predictable", "local variable that can be changed since the check in a loop"),
+        // Local variable already captured by a changing closure
+        // Smart casts are not safe
+        UNPREDICTABLE_VARIABLE("unpredictable", "local variable that is captured by a changing closure"),
+        // Member variable regardless of its visibility
+        // Smart casts are not safe
+        MEMBER_VARIABLE("member", "member variable that can be changed from another thread"),
+        // Some complex expression
+        // Smart casts are not safe
+        OTHER("other", "complex expression");
 
         override fun toString() = str
 
@@ -60,12 +75,12 @@ class DataFlowValue(val id: Any?, val type: KotlinType, val kind: DataFlowValue.
     }
 
     override fun toString(): String {
-        return kind.toString() + (id?.toString()) + " " + immanentNullability
+        return kind.toString() + " " + id?.toString() + " " + immanentNullability
     }
 
     override fun hashCode(): Int {
         var result = if (kind.isStable()) 1 else 0
-        result = 31 * result + (type?.hashCode() ?: 0)
+        result = 31 * result + type.hashCode()
         result = 31 * result + (id?.hashCode() ?: 0)
         return result
     }

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

@@ -207,10 +207,7 @@ public class DataFlowValueFactory {
             return selectorInfo;
         }
         return createInfo(Pair.create(receiverInfo.id, selectorInfo.id),
-                          receiverInfo.kind.isStable() && selectorInfo.kind.isStable()
+                          receiverInfo.kind.isStable() ? selectorInfo.kind : OTHER);
-                          ? STABLE_VALUE
-                          // x.y can never be a local variable
-                          : OTHER);
     }
 
     @NotNull
@@ -379,16 +376,31 @@ public class DataFlowValueFactory {
         return true;
     }
 
+    private static Kind propertyKind(@NotNull PropertyDescriptor propertyDescriptor, @Nullable ModuleDescriptor usageModule) {
+        if (propertyDescriptor.isVar()) return MEMBER_VARIABLE;
+        if (!isFinal(propertyDescriptor)) return MEMBER_VALUE_WITH_GETTER;
+        if (!hasDefaultGetter(propertyDescriptor)) return MEMBER_VALUE_WITH_GETTER;
+        if (!invisibleFromOtherModules(propertyDescriptor)) {
+            ModuleDescriptor declarationModule = DescriptorUtils.getContainingModule(propertyDescriptor);
+            if (usageModule == null || !usageModule.equals(declarationModule)) {
+                return ALIEN_PUBLIC_VALUE;
+            }
+        }
+        return STABLE_VALUE;
+    }
+
     private static Kind variableKind(
             @NotNull VariableDescriptor variableDescriptor,
             @Nullable ModuleDescriptor usageModule,
             @NotNull BindingContext bindingContext,
             @NotNull KtElement accessElement
     ) {
-        if (isStableValue(variableDescriptor, usageModule)) return STABLE_VALUE;
+        if (variableDescriptor instanceof PropertyDescriptor) {
-        boolean isLocalVar = variableDescriptor.isVar() && variableDescriptor instanceof LocalVariableDescriptor;
+            return propertyKind((PropertyDescriptor) variableDescriptor, usageModule);
-        if (!isLocalVar) return OTHER;
+        }
-        if (variableDescriptor instanceof SyntheticFieldDescriptor) return OTHER;
+        if (!(variableDescriptor instanceof LocalVariableDescriptor) && !(variableDescriptor instanceof ParameterDescriptor)) return OTHER;
+        if (!variableDescriptor.isVar()) return STABLE_VALUE;
+        if (variableDescriptor instanceof SyntheticFieldDescriptor) return MEMBER_VARIABLE;
 
         // Local variable classification: PREDICTABLE or UNPREDICTABLE
         PreliminaryDeclarationVisitor preliminaryVisitor =
@@ -430,15 +442,7 @@ public class DataFlowValueFactory {
     ) {
         if (variableDescriptor.isVar()) return false;
         if (variableDescriptor instanceof PropertyDescriptor) {
-            PropertyDescriptor propertyDescriptor = (PropertyDescriptor) variableDescriptor;
+            return propertyKind((PropertyDescriptor) variableDescriptor, usageModule) == STABLE_VALUE;
-            if (!isFinal(propertyDescriptor)) return false;
-            if (!hasDefaultGetter(propertyDescriptor)) return false;
-            if (!invisibleFromOtherModules(propertyDescriptor)) {
-                ModuleDescriptor declarationModule = DescriptorUtils.getContainingModule(propertyDescriptor);
-                if (usageModule == null || !usageModule.equals(declarationModule)) {
-                    return false;
-                }
-            }
         }
         return true;
     }

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

@@ -150,10 +150,10 @@ public class SmartCastManager {
             @NotNull KtExpression expression,
             @NotNull KotlinType type,
             @NotNull BindingTrace trace,
-            boolean canBeCast,
+            @NotNull DataFlowValue dataFlowValue,
             boolean recordExpressionType
     ) {
-        if (canBeCast) {
+        if (dataFlowValue.isPredictable()) {
             trace.record(SMARTCAST, expression, type);
             if (recordExpressionType) {
                 //TODO
@@ -162,12 +162,12 @@ public class SmartCastManager {
             }
         }
         else {
-            trace.report(SMARTCAST_IMPOSSIBLE.on(expression, type, expression.getText()));
+            trace.report(SMARTCAST_IMPOSSIBLE.on(expression, type, expression.getText(), dataFlowValue.getKind().getDescription()));
         }
     }
 
     @Nullable
-    public SmartCastResult checkAndRecordPossibleCast(
+    public static SmartCastResult checkAndRecordPossibleCast(
             @NotNull DataFlowValue dataFlowValue,
             @NotNull KotlinType expectedType,
             @Nullable KtExpression expression,
@@ -177,7 +177,7 @@ public class SmartCastManager {
         for (KotlinType possibleType : c.dataFlowInfo.getPossibleTypes(dataFlowValue)) {
             if (ArgumentTypeResolver.isSubtypeOfForArgumentType(possibleType, expectedType)) {
                 if (expression != null) {
-                    recordCastOrError(expression, possibleType, c.trace, dataFlowValue.isPredictable(), recordExpressionType);
+                    recordCastOrError(expression, possibleType, c.trace, dataFlowValue, recordExpressionType);
                 }
                 return new SmartCastResult(possibleType, dataFlowValue.isPredictable());
             }
@@ -203,8 +203,7 @@ public class SmartCastManager {
             if (ArgumentTypeResolver.isSubtypeOfForArgumentType(dataFlowValue.getType(), nullableExpectedType)) {
                 if (!immanentlyNotNull) {
                     if (expression != null) {
-                        recordCastOrError(expression, dataFlowValue.getType(), c.trace, dataFlowValue.isPredictable(),
+                        recordCastOrError(expression, dataFlowValue.getType(), c.trace, dataFlowValue, recordExpressionType);
-                                          recordExpressionType);
                     }
                 }
 

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

@@ -252,7 +252,7 @@ public class DataFlowAnalyzer {
     ) {
         DataFlowValue dataFlowValue = DataFlowValueFactory.createDataFlowValue(expression, expressionType, c);
 
-        SmartCastResult result = smartCastManager.checkAndRecordPossibleCast(dataFlowValue, c.expectedType, expression, c, false);
+        SmartCastResult result = SmartCastManager.checkAndRecordPossibleCast(dataFlowValue, c.expectedType, expression, c, false);
         return result != null ? result.getResultType() : null;
     }
 

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

@@ -0,0 +1,5 @@
+val x: Int? = 0
+    get() {
+        if (field != null) return <!DEBUG_INFO_SMARTCAST!>field<!>.hashCode()
+        return null
+    }

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

@@ -0,0 +1,3 @@
+package
+
+public val x: kotlin.Int? = 0

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

@@ -14433,6 +14433,12 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest {
                 doTest(fileName);
             }
 
+            @TestMetadata("fieldInGetter.kt")
+            public void testFieldInGetter() throws Exception {
+                String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/smartCasts/fieldInGetter.kt");
+                doTest(fileName);
+            }
+
             @TestMetadata("fieldPlus.kt")
             public void testFieldPlus() throws Exception {
                 String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/smartCasts/fieldPlus.kt");

idea/src/org/jetbrains/kotlin/idea/quickfix/CastExpressionFix.java

@@ -25,6 +25,7 @@ import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.kotlin.diagnostics.Diagnostic;
 import org.jetbrains.kotlin.diagnostics.DiagnosticWithParameters2;
+import org.jetbrains.kotlin.diagnostics.DiagnosticWithParameters3;
 import org.jetbrains.kotlin.diagnostics.Errors;
 import org.jetbrains.kotlin.idea.KotlinBundle;
 import org.jetbrains.kotlin.idea.caches.resolve.ResolutionUtils;
@@ -104,7 +105,7 @@ public class CastExpressionFix extends KotlinQuickFixAction<KtExpression> {
             @Nullable
             @Override
             public IntentionAction createAction(@NotNull Diagnostic diagnostic) {
-                DiagnosticWithParameters2<KtExpression, KotlinType, String> diagnosticWithParameters =
+                DiagnosticWithParameters3<KtExpression, KotlinType, String, String> diagnosticWithParameters =
                         Errors.SMARTCAST_IMPOSSIBLE.cast(diagnostic);
                 return new CastExpressionFix(diagnosticWithParameters.getPsiElement(), diagnosticWithParameters.getA());
             }

idea/testData/checker/infos/SmartCasts.kt

@@ -216,11 +216,39 @@ fun f(): String {
             <info>a</info> = 42
             <error descr="[TYPE_MISMATCH] Type mismatch: inferred type is kotlin.Any but kotlin.String was expected">a</error>
         }
-        return <error descr="[SMARTCAST_IMPOSSIBLE] Smart cast to 'kotlin.String' is impossible, because 'a' could have changed since the is-check">a</error>
+        return <error descr="[SMARTCAST_IMPOSSIBLE] Smart cast to 'kotlin.String' is impossible, because 'a' is a local variable that is captured by a changing closure">a</error>
     }
     return ""
 }
 
+class Mutable(var <info descr="This property has a backing field">x</info>: String?) {
+
+    val xx: String?
+        <info descr="null">get</info>() = x
+
+    fun foo(): String {
+        if (x is String) {
+            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>
+        }
+        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>
+        }
+        return ""
+    }
+
+    fun bar(other: Mutable): String {
+        var y = other
+        if (y.x is String) {
+            return <error descr="[SMARTCAST_IMPOSSIBLE] Smart cast to 'kotlin.String' is impossible, because 'y.x' is a complex expression">y.x</error>
+        }
+        return ""
+    }
+}
+
 fun foo(aa: Any): Int {
     var a = aa
     if (a is Int) {