Better diagnostics for non-exhaustive whens, relevant test #KT-10295 Fixed

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

@@ -105,7 +105,7 @@ class JavaNullabilityWarningsChecker : AdditionalTypeChecker {
                         if (type.isFlexible() && TypeUtils.isNullableType(type.flexibility().upperBound) && !type.annotations.isMarkedNotNull()) {
                             val enumClassDescriptor = WhenChecker.getClassDescriptorOfTypeIfEnum(type) ?: return
 
-                            if (WhenChecker.isWhenOnEnumExhaustive(expression, c.trace, enumClassDescriptor)
+                            if (WhenChecker.getEnumMissingCases(expression, c.trace, enumClassDescriptor).isEmpty()
                                 && !WhenChecker.containsNullCase(expression, c.trace)) {
 
                                 c.trace.report(ErrorsJvm.WHEN_ENUM_CAN_BE_NULL_IN_JAVA.on(expression.getSubjectExpression()))

compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.java

@@ -743,15 +743,20 @@ public class ControlFlowInformationProvider {
                             KtWhenExpression whenExpression = (KtWhenExpression) element;
                             if (whenExpression.getElseExpression() != null) continue;
 
-                            if (WhenChecker.mustHaveElse(whenExpression, trace)) {
+                            List<WhenMissingCase> necessaryCases = WhenChecker.getNecessaryCases(whenExpression, trace);
-                                trace.report(NO_ELSE_IN_WHEN.on(whenExpression));
+                            if (!necessaryCases.isEmpty()) {
+                                trace.report(NO_ELSE_IN_WHEN.on(whenExpression, necessaryCases));
                             }
                             else if (whenExpression.getSubjectExpression() != null) {
                                 ClassDescriptor enumClassDescriptor = WhenChecker.getClassDescriptorOfTypeIfEnum(
                                         trace.getType(whenExpression.getSubjectExpression()));
-                                if (enumClassDescriptor != null
+                                if (enumClassDescriptor != null) {
-                                    && !WhenChecker.isWhenOnEnumExhaustive(whenExpression, trace, enumClassDescriptor)) {
+                                    List<WhenMissingCase> missingCases = WhenChecker.getEnumMissingCases(
-                                    trace.report(NON_EXHAUSTIVE_WHEN.on(whenExpression));
+                                            whenExpression, trace, enumClassDescriptor
+                                    );
+                                    if (!missingCases.isEmpty()) {
+                                        trace.report(NON_EXHAUSTIVE_WHEN.on(whenExpression, missingCases));
+                                    }
                                 }
                             }
                         }

compiler/frontend/src/org/jetbrains/kotlin/cfg/WhenChecker.kt

@@ -34,18 +34,21 @@ import org.jetbrains.kotlin.types.*
 import org.jetbrains.kotlin.types.KotlinType
 import org.jetbrains.kotlin.types.TypeUtils
 
-import java.util.HashSet
-
 import org.jetbrains.kotlin.resolve.DescriptorUtils.isEnumClass
 import org.jetbrains.kotlin.resolve.DescriptorUtils.isEnumEntry
 import org.jetbrains.kotlin.resolve.bindingContextUtil.isUsedAsExpression
+import java.util.*
 
 interface WhenMissingCase
 
+// Always must be first in the list
 private object UnknownMissingCase : WhenMissingCase {
     override fun toString() = "unknown"
 }
 
+val List<WhenMissingCase>.hasUnknown: Boolean
+    get() = firstOrNull() == UnknownMissingCase
+
 private interface WhenExhaustivenessChecker {
     fun getMissingCases(
             expression: KtWhenExpression,
@@ -135,7 +138,7 @@ private abstract class WhenOnClassExhaustivenessChecker : WhenExhaustivenessChec
     ): List<WhenMissingCase> {
         // when on empty enum / sealed is considered non-exhaustive, see test whenOnEmptySealed
         if (memberDescriptors.isEmpty()) return listOf(UnknownMissingCase)
-        val checkedDescriptors = HashSet<ClassDescriptor>()
+        val checkedDescriptors = LinkedHashSet<ClassDescriptor>()
         for (whenEntry in whenExpression.entries) {
             for (condition in whenEntry.conditions) {
                 var negated = false
@@ -214,7 +217,7 @@ private object WhenOnSealedExhaustivenessChecker : WhenOnClassExhaustivenessChec
         assert(subjectDescriptor!!.modality === Modality.SEALED) {
             "isWhenOnSealedClassExhaustive should be called with a sealed class descriptor"
         }
-        val memberClassDescriptors = HashSet<ClassDescriptor>()
+        val memberClassDescriptors = LinkedHashSet<ClassDescriptor>()
         collectNestedSubclasses(subjectDescriptor!!, subjectDescriptor, memberClassDescriptors)
         // When on a sealed class without derived members is considered non-exhaustive (see test WhenOnEmptySealed)
         return getMissingClassCases(expression, memberClassDescriptors, trace)
@@ -248,8 +251,9 @@ object WhenChecker {
                                                 WhenOnNullableExhaustivenessChecker)
 
     @JvmStatic
-    fun mustHaveElse(expression: KtWhenExpression, trace: BindingTrace) =
+    fun getNecessaryCases(expression: KtWhenExpression, trace: BindingTrace) =
-            expression.isUsedAsExpression(trace.bindingContext) && !isWhenExhaustive(expression, trace)
+            if (expression.isUsedAsExpression(trace.bindingContext)) getMissingCases(expression, trace)
+            else listOf()
 
     @JvmStatic
     fun isWhenByEnum(expression: KtWhenExpression, context: BindingContext) =
@@ -268,11 +272,11 @@ object WhenChecker {
             expression.subjectExpression?.let { context.getType(it) } ?: null
 
     @JvmStatic
-    fun isWhenOnEnumExhaustive(
+    fun getEnumMissingCases(
             expression: KtWhenExpression,
             trace: BindingTrace,
             enumClassDescriptor: ClassDescriptor
-    ) = WhenOnEnumExhaustivenessChecker.getMissingCases(expression, trace, enumClassDescriptor, false).isEmpty()
+    ) = WhenOnEnumExhaustivenessChecker.getMissingCases(expression, trace, enumClassDescriptor, false)
 
     /**
      * It's assumed that function is called for a final type. In this case the only possible smart cast is to not nullable type.

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

@@ -21,6 +21,7 @@ import com.intellij.openapi.util.TextRange;
 import com.intellij.psi.PsiElement;
 import com.intellij.psi.impl.source.tree.LeafPsiElement;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.kotlin.cfg.WhenMissingCase;
 import org.jetbrains.kotlin.descriptors.*;
 import org.jetbrains.kotlin.lexer.KtKeywordToken;
 import org.jetbrains.kotlin.lexer.KtModifierKeywordToken;
@@ -705,8 +706,8 @@ public interface Errors {
 
     DiagnosticFactory0<KtWhenCondition> EXPECTED_CONDITION = DiagnosticFactory0.create(ERROR);
     DiagnosticFactory0<KtWhenEntry> ELSE_MISPLACED_IN_WHEN = DiagnosticFactory0.create(ERROR, ELSE_ENTRY);
-    DiagnosticFactory0<KtWhenExpression> NO_ELSE_IN_WHEN = DiagnosticFactory0.create(ERROR, WHEN_EXPRESSION);
+    DiagnosticFactory1<KtWhenExpression, List<WhenMissingCase>> NO_ELSE_IN_WHEN = DiagnosticFactory1.create(ERROR, WHEN_EXPRESSION);
-    DiagnosticFactory0<KtWhenExpression> NON_EXHAUSTIVE_WHEN = DiagnosticFactory0.create(WARNING, WHEN_EXPRESSION);
+    DiagnosticFactory1<KtWhenExpression, List<WhenMissingCase>> NON_EXHAUSTIVE_WHEN = DiagnosticFactory1.create(WARNING, WHEN_EXPRESSION);
     DiagnosticFactory0<PsiElement> COMMA_IN_WHEN_CONDITION_WITHOUT_ARGUMENT = DiagnosticFactory0.create(ERROR);
 
     // Type mismatch

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

@@ -419,8 +419,8 @@ public class DefaultErrorMessages {
         MAP.put(ELSE_MISPLACED_IN_WHEN, "'else' entry must be the last one in a when-expression");
         MAP.put(COMMA_IN_WHEN_CONDITION_WITHOUT_ARGUMENT, "Deprecated syntax. Use '||' instead of commas in when-condition for 'when' without argument");
 
-        MAP.put(NO_ELSE_IN_WHEN, "'when' expression must be exhaustive or contain an 'else' branch");
+        MAP.put(NO_ELSE_IN_WHEN, "'when' expression must be exhaustive, add necessary {0}", RENDER_WHEN_MISSING_CASES);
-        MAP.put(NON_EXHAUSTIVE_WHEN, "'when' expression contains only some variants and no 'else' branch");
+        MAP.put(NON_EXHAUSTIVE_WHEN, "'when' expression on enum is recommended to be exhaustive, add {0}", RENDER_WHEN_MISSING_CASES);
 
         MAP.put(TYPE_MISMATCH_IN_RANGE, "Type mismatch: incompatible types of range and element checked in it");
         MAP.put(CYCLIC_INHERITANCE_HIERARCHY, "There's a cycle in the inheritance hierarchy for this type");

compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/Renderers.kt

@@ -16,12 +16,15 @@
 
 package org.jetbrains.kotlin.diagnostics.rendering
 
+import com.google.common.base.Strings
 import com.google.common.collect.Lists
 import com.google.common.collect.Sets
 import com.intellij.openapi.diagnostic.Logger
 import com.intellij.openapi.util.text.StringUtil
 import com.intellij.psi.PsiElement
 import org.jetbrains.kotlin.builtins.KotlinBuiltIns
+import org.jetbrains.kotlin.cfg.WhenMissingCase
+import org.jetbrains.kotlin.cfg.hasUnknown
 import org.jetbrains.kotlin.descriptors.*
 import org.jetbrains.kotlin.diagnostics.rendering.TabledDescriptorRenderer.newTable
 import org.jetbrains.kotlin.diagnostics.rendering.TabledDescriptorRenderer.newText
@@ -407,4 +410,15 @@ object Renderers {
         }
         append("(").append(renderTypes(inferenceErrorData.valueArgumentsTypes)).append(")")
     }
+
+    @JvmField val RENDER_WHEN_MISSING_CASES: Renderer<List<WhenMissingCase>> = Renderer {
+        if (!it.hasUnknown) {
+            val list = it.map { "'$it'" }.joinToString(", ")
+            val branches = if (it.size > 1) "branches" else "branch"
+            "$list $branches or 'else' branch instead"
+        }
+        else {
+            "'else' branch"
+        }
+    }
 }

idea/testData/checker/WhenNonExhaustive.kt

@@ -0,0 +1,47 @@
+fun nonExhaustiveInt(x: Int) = <error descr="[NO_ELSE_IN_WHEN] when expression must be exhaustive, add necessary 'else' branch">when</error>(x) {
+    0 -> false
+}
+
+fun nonExhaustiveBoolean(b: Boolean) = <error descr="[NO_ELSE_IN_WHEN] when expression must be exhaustive, add necessary 'true' branch or 'else' branch instead">when</error>(b) {
+    false -> 0
+}
+
+enum class Color { 
+    RED,
+    GREEN,
+    BLUE
+}
+
+fun nonExhaustiveEnum(c: Color) = <error descr="[NO_ELSE_IN_WHEN] when expression must be exhaustive, add necessary 'RED', 'BLUE' branches or 'else' branch instead">when</error>(c) {
+    Color.GREEN -> 0xff00
+}
+
+fun nonExhaustiveNullable(c: Color?) = <error descr="[NO_ELSE_IN_WHEN] when expression must be exhaustive, add necessary 'GREEN', 'null' branches or 'else' branch instead">when</error>(c) {
+    Color.RED -> 0xff
+    Color.BLUE -> 0xff0000
+}
+
+fun whenOnEnum(c: Color) {
+    <warning descr="[NON_EXHAUSTIVE_WHEN] when expression on enum is recommended to be exhaustive, add 'RED' branch or 'else' branch instead">when</warning>(c) {
+        Color.BLUE -> {}
+        Color.GREEN -> {}
+    }
+}
+
+sealed class Variant {
+    object Singleton : Variant()
+   
+    class Something : Variant()
+
+    object Another : Variant()
+}
+
+fun nonExhaustiveSealed(v: Variant) = <error descr="[NO_ELSE_IN_WHEN] when expression must be exhaustive, add necessary 'is Something', 'Another' branches or 'else' branch instead">when</error>(v) {
+    Variant.Singleton -> false
+}
+
+sealed class Empty
+
+fun nonExhaustiveEmpty(e: Empty) = <error descr="[NO_ELSE_IN_WHEN] when expression must be exhaustive, add necessary 'else' branch">when</error>(<warning>e</warning>) {}
+
+fun nonExhaustiveNullableEmpty(e: Empty?) = <error descr="[NO_ELSE_IN_WHEN] when expression must be exhaustive, add necessary 'else' branch">when</error>(<warning>e</warning>) {}

idea/tests/org/jetbrains/kotlin/checkers/PsiCheckerTestGenerated.java

@@ -366,6 +366,12 @@ public class PsiCheckerTestGenerated extends AbstractPsiCheckerTest {
             String fileName = KotlinTestUtils.navigationMetadata("idea/testData/checker/WhenInEnumInExtensionProperty.kt");
             doTest(fileName);
         }
+
+        @TestMetadata("WhenNonExhaustive.kt")
+        public void testWhenNonExhaustive() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/checker/WhenNonExhaustive.kt");
+            doTest(fileName);
+        }
     }
 
     @TestMetadata("idea/testData/checker/regression")