From fe59dc27b3a7cc928d82b2d4eee3a053b796fc55 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Thu, 16 Oct 2014 21:38:13 +0400 Subject: [PATCH] Make 'when' on nullable enums exhaustive if 'null' entry is present #KT-2902 Fixed --- .../jetbrains/jet/lang/cfg/WhenChecker.java | 41 ++++++++--- .../whenEnumOptimization/nullableEnum.kt | 14 ++++ .../tests/when/ElseOnNullableEnum.kt | 37 ++++++++++ .../tests/when/ElseOnNullableEnum.txt | 71 +++++++++++++++++++ .../when/ElseOnNullableEnumWithSmartCast.kt | 11 +++ .../when/ElseOnNullableEnumWithSmartCast.txt | 57 +++++++++++++++ .../checkers/JetDiagnosticsTestGenerated.java | 12 ++++ ...lackBoxWithStdlibCodegenTestGenerated.java | 6 ++ .../jet/repl/AbstractReplInterpreterTest.kt | 2 +- 9 files changed, 241 insertions(+), 10 deletions(-) create mode 100644 compiler/testData/codegen/boxWithStdlib/whenEnumOptimization/nullableEnum.kt create mode 100644 compiler/testData/diagnostics/tests/when/ElseOnNullableEnum.kt create mode 100644 compiler/testData/diagnostics/tests/when/ElseOnNullableEnum.txt create mode 100644 compiler/testData/diagnostics/tests/when/ElseOnNullableEnumWithSmartCast.kt create mode 100644 compiler/testData/diagnostics/tests/when/ElseOnNullableEnumWithSmartCast.txt diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/cfg/WhenChecker.java b/compiler/frontend/src/org/jetbrains/jet/lang/cfg/WhenChecker.java index 9654ad36b4b..157080d43f5 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/cfg/WhenChecker.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/cfg/WhenChecker.java @@ -26,8 +26,10 @@ import org.jetbrains.jet.lang.resolve.BindingContext; import org.jetbrains.jet.lang.resolve.BindingTrace; import org.jetbrains.jet.lang.resolve.bindingContextUtil.BindingContextUtilPackage; import org.jetbrains.jet.lang.types.JetType; +import org.jetbrains.jet.lang.types.TypeUtils; import org.jetbrains.jet.lang.types.lang.KotlinBuiltIns; +import static org.jetbrains.jet.lang.resolve.BindingContext.EXPRESSION_TYPE; import static org.jetbrains.jet.lang.resolve.DescriptorUtils.isEnumEntry; public final class WhenChecker { @@ -44,13 +46,11 @@ public final class WhenChecker { } public static boolean isWhenByEnum(@NotNull JetWhenExpression expression, @NotNull BindingContext context) { - return getSubjectClassDescriptorIfEnum(expression, context) != null; + return getClassDescriptorOfTypeIfEnum(whenSubjectType(expression, context)) != null; } - private static ClassDescriptor getSubjectClassDescriptorIfEnum(@NotNull JetWhenExpression expression, @NotNull BindingContext context) { - JetExpression subjectExpression = expression.getSubjectExpression(); - if (subjectExpression == null) return null; - JetType type = context.get(BindingContext.EXPRESSION_TYPE, subjectExpression); + @Nullable + private static ClassDescriptor getClassDescriptorOfTypeIfEnum(@Nullable JetType type) { if (type == null) return null; DeclarationDescriptor declarationDescriptor = type.getConstructor().getDeclarationDescriptor(); if (!(declarationDescriptor instanceof ClassDescriptor)) return null; @@ -60,10 +60,17 @@ public final class WhenChecker { return classDescriptor; } - private static boolean isWhenExhaustive(@NotNull JetWhenExpression expression, @NotNull BindingTrace trace) { - ClassDescriptor classDescriptor = getSubjectClassDescriptorIfEnum(expression, trace.getBindingContext()); + @Nullable + private static JetType whenSubjectType(@NotNull JetWhenExpression expression, @NotNull BindingContext context) { + JetExpression subjectExpression = expression.getSubjectExpression(); + return subjectExpression == null ? null : context.get(EXPRESSION_TYPE, subjectExpression); + } - if (classDescriptor == null) return false; + private static boolean isWhenExhaustive(@NotNull JetWhenExpression expression, @NotNull BindingTrace trace) { + JetType type = whenSubjectType(expression, trace.getBindingContext()); + ClassDescriptor classDescriptor = getClassDescriptorOfTypeIfEnum(type); + + if (type == null || classDescriptor == null) return false; boolean isExhaust = true; boolean notEmpty = false; @@ -75,7 +82,7 @@ public final class WhenChecker { } } } - boolean exhaustive = isExhaust && notEmpty; + boolean exhaustive = isExhaust && notEmpty && (!TypeUtils.isNullableType(type) || containsNullCase(expression, trace)); if (exhaustive) { trace.record(BindingContext.EXHAUSTIVE_WHEN, expression); } @@ -101,6 +108,22 @@ public final class WhenChecker { return false; } + private static boolean containsNullCase(@NotNull JetWhenExpression expression, @NotNull BindingTrace trace) { + for (JetWhenEntry entry : expression.getEntries()) { + for (JetWhenCondition condition : entry.getConditions()) { + if (condition instanceof JetWhenConditionWithExpression) { + JetType type = trace.getBindingContext().get( + EXPRESSION_TYPE, ((JetWhenConditionWithExpression) condition).getExpression() + ); + if (type != null && KotlinBuiltIns.getInstance().isNothingOrNullableNothing(type)) { + return true; + } + } + } + } + return false; + } + private static boolean isCheckForEnumEntry( @NotNull JetWhenConditionWithExpression whenExpression, @NotNull ClassDescriptor enumEntry, diff --git a/compiler/testData/codegen/boxWithStdlib/whenEnumOptimization/nullableEnum.kt b/compiler/testData/codegen/boxWithStdlib/whenEnumOptimization/nullableEnum.kt new file mode 100644 index 00000000000..4bd2252796b --- /dev/null +++ b/compiler/testData/codegen/boxWithStdlib/whenEnumOptimization/nullableEnum.kt @@ -0,0 +1,14 @@ +enum class E { + A + B +} + +fun test(e: E?) = when (e) { + E.A -> "Fail A" + null -> "OK" + E.B -> "Fail B" +} + +fun box(): String { + return test(null) +} diff --git a/compiler/testData/diagnostics/tests/when/ElseOnNullableEnum.kt b/compiler/testData/diagnostics/tests/when/ElseOnNullableEnum.kt new file mode 100644 index 00000000000..662b0fa0e4d --- /dev/null +++ b/compiler/testData/diagnostics/tests/when/ElseOnNullableEnum.kt @@ -0,0 +1,37 @@ +// KT-2902 Check for null should be required when match nullable enum element + +// FILE: 1.kt + +enum class E { A B } + +fun test(e: E?) = when (e) { + E.A -> 1 + E.B -> 2 +} + +fun withNull(e: E?) = when (e) { + E.A -> 3 + E.B -> 4 + null -> null +} + +fun nullableNothing(): Nothing? = null +fun withNullableNothing(e: E?) = when (e) { + E.A -> 5 + E.B -> 6 + nullableNothing() -> null +} + +fun platformType() = when (J.foo()) { + E.A -> 7 + E.B -> 8 +} + + +// FILE: J.java + +class J { + static E foo() { + return E.A; + } +} diff --git a/compiler/testData/diagnostics/tests/when/ElseOnNullableEnum.txt b/compiler/testData/diagnostics/tests/when/ElseOnNullableEnum.txt new file mode 100644 index 00000000000..1af2b251fdd --- /dev/null +++ b/compiler/testData/diagnostics/tests/when/ElseOnNullableEnum.txt @@ -0,0 +1,71 @@ +package + +internal fun nullableNothing(): kotlin.Nothing? +internal fun platformType(): kotlin.Int +internal fun test(/*0*/ e: E?): kotlin.Int +internal fun withNull(/*0*/ e: E?): kotlin.Int? +internal fun withNullableNothing(/*0*/ e: E?): kotlin.Int? + +internal final enum class E : kotlin.Enum { + private constructor E() + public final override /*1*/ /*fake_override*/ fun compareTo(/*0*/ other: E): kotlin.Int + public final override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final override /*1*/ /*fake_override*/ fun name(): kotlin.String + public final override /*1*/ /*fake_override*/ fun ordinal(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + public enum entry A : E { + private constructor A() + public final override /*1*/ /*fake_override*/ fun compareTo(/*0*/ other: E): kotlin.Int + public final override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final override /*1*/ /*fake_override*/ fun name(): kotlin.String + public final override /*1*/ /*fake_override*/ fun ordinal(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + public class object : E.A { + private constructor () + public final override /*1*/ /*fake_override*/ fun compareTo(/*0*/ other: E): kotlin.Int + public final override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final override /*1*/ /*fake_override*/ fun name(): kotlin.String + public final override /*1*/ /*fake_override*/ fun ordinal(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } + } + + public enum entry B : E { + private constructor B() + public final override /*1*/ /*fake_override*/ fun compareTo(/*0*/ other: E): kotlin.Int + public final override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final override /*1*/ /*fake_override*/ fun name(): kotlin.String + public final override /*1*/ /*fake_override*/ fun ordinal(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + public class object : E.B { + private constructor () + public final override /*1*/ /*fake_override*/ fun compareTo(/*0*/ other: E): kotlin.Int + public final override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final override /*1*/ /*fake_override*/ fun name(): kotlin.String + public final override /*1*/ /*fake_override*/ fun ordinal(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } + } + + // Static members + public final /*synthesized*/ fun valueOf(/*0*/ value: kotlin.String): E + public final /*synthesized*/ fun values(): kotlin.Array +} + +public/*package*/ open class J { + public/*package*/ constructor J() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + // Static members + public/*package*/ open fun foo(): E! +} diff --git a/compiler/testData/diagnostics/tests/when/ElseOnNullableEnumWithSmartCast.kt b/compiler/testData/diagnostics/tests/when/ElseOnNullableEnumWithSmartCast.kt new file mode 100644 index 00000000000..3111316709d --- /dev/null +++ b/compiler/testData/diagnostics/tests/when/ElseOnNullableEnumWithSmartCast.kt @@ -0,0 +1,11 @@ +enum class E { A B } + +fun foo(e: E, something: Any?): Int { + if (something != null) return 0 + + return when (e) { + E.A -> 1 + E.B -> 2 + something -> 3 + } +} diff --git a/compiler/testData/diagnostics/tests/when/ElseOnNullableEnumWithSmartCast.txt b/compiler/testData/diagnostics/tests/when/ElseOnNullableEnumWithSmartCast.txt new file mode 100644 index 00000000000..5d74db9314d --- /dev/null +++ b/compiler/testData/diagnostics/tests/when/ElseOnNullableEnumWithSmartCast.txt @@ -0,0 +1,57 @@ +package + +internal fun foo(/*0*/ e: E, /*1*/ something: kotlin.Any?): kotlin.Int + +internal final enum class E : kotlin.Enum { + private constructor E() + public final override /*1*/ /*fake_override*/ fun compareTo(/*0*/ other: E): kotlin.Int + public final override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final override /*1*/ /*fake_override*/ fun name(): kotlin.String + public final override /*1*/ /*fake_override*/ fun ordinal(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + public enum entry A : E { + private constructor A() + public final override /*1*/ /*fake_override*/ fun compareTo(/*0*/ other: E): kotlin.Int + public final override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final override /*1*/ /*fake_override*/ fun name(): kotlin.String + public final override /*1*/ /*fake_override*/ fun ordinal(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + public class object : E.A { + private constructor () + public final override /*1*/ /*fake_override*/ fun compareTo(/*0*/ other: E): kotlin.Int + public final override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final override /*1*/ /*fake_override*/ fun name(): kotlin.String + public final override /*1*/ /*fake_override*/ fun ordinal(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } + } + + public enum entry B : E { + private constructor B() + public final override /*1*/ /*fake_override*/ fun compareTo(/*0*/ other: E): kotlin.Int + public final override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final override /*1*/ /*fake_override*/ fun name(): kotlin.String + public final override /*1*/ /*fake_override*/ fun ordinal(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + public class object : E.B { + private constructor () + public final override /*1*/ /*fake_override*/ fun compareTo(/*0*/ other: E): kotlin.Int + public final override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final override /*1*/ /*fake_override*/ fun name(): kotlin.String + public final override /*1*/ /*fake_override*/ fun ordinal(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } + } + + // Static members + public final /*synthesized*/ fun valueOf(/*0*/ value: kotlin.String): E + public final /*synthesized*/ fun values(): kotlin.Array +} \ No newline at end of file diff --git a/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java index 87b27c4aaa9..59f521fa499 100644 --- a/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java @@ -10123,6 +10123,18 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest { doTest(fileName); } + @TestMetadata("ElseOnNullableEnum.kt") + public void testElseOnNullableEnum() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/when/ElseOnNullableEnum.kt"); + doTest(fileName); + } + + @TestMetadata("ElseOnNullableEnumWithSmartCast.kt") + public void testElseOnNullableEnumWithSmartCast() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/when/ElseOnNullableEnumWithSmartCast.kt"); + doTest(fileName); + } + @TestMetadata("NoElseExpectedUnit.kt") public void testNoElseExpectedUnit() throws Exception { String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/when/NoElseExpectedUnit.kt"); diff --git a/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxWithStdlibCodegenTestGenerated.java b/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxWithStdlibCodegenTestGenerated.java index b5b3c61997b..f6917ceeb6b 100644 --- a/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxWithStdlibCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxWithStdlibCodegenTestGenerated.java @@ -2609,6 +2609,12 @@ public class BlackBoxWithStdlibCodegenTestGenerated extends AbstractBlackBoxCode doTestWithStdlib(fileName); } + @TestMetadata("nullableEnum.kt") + public void testNullableEnum() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/boxWithStdlib/whenEnumOptimization/nullableEnum.kt"); + doTestWithStdlib(fileName); + } + @TestMetadata("subjectAny.kt") public void testSubjectAny() throws Exception { String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/boxWithStdlib/whenEnumOptimization/subjectAny.kt"); diff --git a/compiler/tests/org/jetbrains/jet/repl/AbstractReplInterpreterTest.kt b/compiler/tests/org/jetbrains/jet/repl/AbstractReplInterpreterTest.kt index d55f7c219f6..42ef14946a0 100644 --- a/compiler/tests/org/jetbrains/jet/repl/AbstractReplInterpreterTest.kt +++ b/compiler/tests/org/jetbrains/jet/repl/AbstractReplInterpreterTest.kt @@ -81,7 +81,7 @@ public abstract class AbstractReplInterpreterTest : UsefulTestCase() { for ((code, expected) in loadLines(File(path))) { val lineResult = repl.eval(code) - val actual = when (lineResult.getType()) { + val actual = when (lineResult.getType()!!) { ReplInterpreter.LineResultType.SUCCESS -> lineResult.getValue()?.toString() ?: "" ReplInterpreter.LineResultType.ERROR -> lineResult.getErrorText() ReplInterpreter.LineResultType.INCOMPLETE -> INCOMPLETE_LINE_MESSAGE