From c085eb650f1ee12317192df46240dc28d6e062fd Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Mon, 14 Dec 2015 19:46:03 +0300 Subject: [PATCH] Better diagnostics for non-exhaustive whens, relevant test #KT-10295 Fixed --- .../JavaNullabilityWarningsChecker.kt | 2 +- .../cfg/ControlFlowInformationProvider.java | 15 ++++-- .../org/jetbrains/kotlin/cfg/WhenChecker.kt | 20 ++++---- .../jetbrains/kotlin/diagnostics/Errors.java | 5 +- .../rendering/DefaultErrorMessages.java | 4 +- .../kotlin/diagnostics/rendering/Renderers.kt | 14 ++++++ idea/testData/checker/WhenNonExhaustive.kt | 47 +++++++++++++++++++ .../checkers/PsiCheckerTestGenerated.java | 6 +++ 8 files changed, 95 insertions(+), 18 deletions(-) create mode 100644 idea/testData/checker/WhenNonExhaustive.kt diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JavaNullabilityWarningsChecker.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JavaNullabilityWarningsChecker.kt index 857b11b8ee8..2fd2a39e97a 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JavaNullabilityWarningsChecker.kt +++ b/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())) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.java b/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.java index 69b4fd48619..e796ea64cd4 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.java +++ b/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)) { - trace.report(NO_ELSE_IN_WHEN.on(whenExpression)); + List necessaryCases = WhenChecker.getNecessaryCases(whenExpression, trace); + 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 - && !WhenChecker.isWhenOnEnumExhaustive(whenExpression, trace, enumClassDescriptor)) { - trace.report(NON_EXHAUSTIVE_WHEN.on(whenExpression)); + if (enumClassDescriptor != null) { + List missingCases = WhenChecker.getEnumMissingCases( + whenExpression, trace, enumClassDescriptor + ); + if (!missingCases.isEmpty()) { + trace.report(NON_EXHAUSTIVE_WHEN.on(whenExpression, missingCases)); + } } } } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/cfg/WhenChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/cfg/WhenChecker.kt index c7550487012..bfa903c4de5 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/cfg/WhenChecker.kt +++ b/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.hasUnknown: Boolean + get() = firstOrNull() == UnknownMissingCase + private interface WhenExhaustivenessChecker { fun getMissingCases( expression: KtWhenExpression, @@ -135,7 +138,7 @@ private abstract class WhenOnClassExhaustivenessChecker : WhenExhaustivenessChec ): List { // when on empty enum / sealed is considered non-exhaustive, see test whenOnEmptySealed if (memberDescriptors.isEmpty()) return listOf(UnknownMissingCase) - val checkedDescriptors = HashSet() + val checkedDescriptors = LinkedHashSet() 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() + val memberClassDescriptors = LinkedHashSet() 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) = - expression.isUsedAsExpression(trace.bindingContext) && !isWhenExhaustive(expression, trace) + fun getNecessaryCases(expression: KtWhenExpression, trace: BindingTrace) = + 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. diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index d09ce59ba07..98de95a19b6 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/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 EXPECTED_CONDITION = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 ELSE_MISPLACED_IN_WHEN = DiagnosticFactory0.create(ERROR, ELSE_ENTRY); - DiagnosticFactory0 NO_ELSE_IN_WHEN = DiagnosticFactory0.create(ERROR, WHEN_EXPRESSION); - DiagnosticFactory0 NON_EXHAUSTIVE_WHEN = DiagnosticFactory0.create(WARNING, WHEN_EXPRESSION); + DiagnosticFactory1> NO_ELSE_IN_WHEN = DiagnosticFactory1.create(ERROR, WHEN_EXPRESSION); + DiagnosticFactory1> NON_EXHAUSTIVE_WHEN = DiagnosticFactory1.create(WARNING, WHEN_EXPRESSION); DiagnosticFactory0 COMMA_IN_WHEN_CONDITION_WITHOUT_ARGUMENT = DiagnosticFactory0.create(ERROR); // Type mismatch diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java index 570f82ce5f2..909698ee907 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/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(NON_EXHAUSTIVE_WHEN, "'when' expression contains only some variants and no '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 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"); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/Renderers.kt b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/Renderers.kt index 856f5d9a7d0..86fbdc85a1b 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/Renderers.kt +++ b/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> = 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" + } + } } diff --git a/idea/testData/checker/WhenNonExhaustive.kt b/idea/testData/checker/WhenNonExhaustive.kt new file mode 100644 index 00000000000..4ccc6c7e4c3 --- /dev/null +++ b/idea/testData/checker/WhenNonExhaustive.kt @@ -0,0 +1,47 @@ +fun nonExhaustiveInt(x: Int) = when(x) { + 0 -> false +} + +fun nonExhaustiveBoolean(b: Boolean) = when(b) { + false -> 0 +} + +enum class Color { + RED, + GREEN, + BLUE +} + +fun nonExhaustiveEnum(c: Color) = when(c) { + Color.GREEN -> 0xff00 +} + +fun nonExhaustiveNullable(c: Color?) = when(c) { + Color.RED -> 0xff + Color.BLUE -> 0xff0000 +} + +fun whenOnEnum(c: Color) { + when(c) { + Color.BLUE -> {} + Color.GREEN -> {} + } +} + +sealed class Variant { + object Singleton : Variant() + + class Something : Variant() + + object Another : Variant() +} + +fun nonExhaustiveSealed(v: Variant) = when(v) { + Variant.Singleton -> false +} + +sealed class Empty + +fun nonExhaustiveEmpty(e: Empty) = when(e) {} + +fun nonExhaustiveNullableEmpty(e: Empty?) = when(e) {} diff --git a/idea/tests/org/jetbrains/kotlin/checkers/PsiCheckerTestGenerated.java b/idea/tests/org/jetbrains/kotlin/checkers/PsiCheckerTestGenerated.java index b0a016440b1..8efed23ab0f 100644 --- a/idea/tests/org/jetbrains/kotlin/checkers/PsiCheckerTestGenerated.java +++ b/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")