From 9625c325277ae4865b7f414e4634fc3c7bd111b8 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Wed, 18 Jan 2017 18:40:42 +0300 Subject: [PATCH] Exhaustive when on sealed trees implemented #KT-13130 Fixed Also #KT-13227 Fixed --- .../org/jetbrains/kotlin/cfg/WhenChecker.kt | 85 +++++++++++-------- .../tests/sealed/ExhaustiveOnRoot.kt | 23 +++++ .../tests/sealed/ExhaustiveOnRoot.txt | 33 +++++++ .../tests/sealed/ExhaustiveOnTree.kt | 34 ++++++++ .../tests/sealed/ExhaustiveOnTree.txt | 78 +++++++++++++++++ .../tests/sealed/ExhaustiveOnTriangleStar.kt | 24 ++++++ .../tests/sealed/ExhaustiveOnTriangleStar.txt | 33 +++++++ .../checkers/DiagnosticsTestGenerated.java | 18 ++++ 8 files changed, 292 insertions(+), 36 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/sealed/ExhaustiveOnRoot.kt create mode 100644 compiler/testData/diagnostics/tests/sealed/ExhaustiveOnRoot.txt create mode 100644 compiler/testData/diagnostics/tests/sealed/ExhaustiveOnTree.kt create mode 100644 compiler/testData/diagnostics/tests/sealed/ExhaustiveOnTree.txt create mode 100644 compiler/testData/diagnostics/tests/sealed/ExhaustiveOnTriangleStar.kt create mode 100644 compiler/testData/diagnostics/tests/sealed/ExhaustiveOnTriangleStar.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/cfg/WhenChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/cfg/WhenChecker.kt index 8b32265550b..9a97912f956 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/cfg/WhenChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/cfg/WhenChecker.kt @@ -20,6 +20,7 @@ import com.intellij.psi.PsiElement import org.jetbrains.kotlin.builtins.KotlinBuiltIns import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.ClassKind +import org.jetbrains.kotlin.descriptors.Modality import org.jetbrains.kotlin.diagnostics.Errors import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.* @@ -34,7 +35,6 @@ import org.jetbrains.kotlin.resolve.DescriptorUtils.isEnumEntry import org.jetbrains.kotlin.resolve.bindingContextUtil.isUsedAsExpression import org.jetbrains.kotlin.resolve.constants.CompileTimeConstant import org.jetbrains.kotlin.resolve.constants.evaluate.ConstantExpressionEvaluator -import org.jetbrains.kotlin.resolve.descriptorUtil.computeSealedSubclasses import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.types.TypeUtils import java.util.* @@ -144,6 +144,36 @@ internal abstract class WhenOnClassExhaustivenessChecker : WhenExhaustivenessChe else -> null } + protected val ClassDescriptor.deepSealedSubclasses: List + get() = this.sealedSubclasses.flatMap { + if (it.modality == Modality.SEALED) it.deepSealedSubclasses + else setOf(it) + } + + private val KtWhenCondition.negated + get() = (this as? KtWhenConditionIsPattern)?.isNegated ?: false + + private fun KtWhenCondition.isRelevant(checkedDescriptor: ClassDescriptor) = + this !is KtWhenConditionWithExpression || + DescriptorUtils.isObject(checkedDescriptor) || + DescriptorUtils.isEnumEntry(checkedDescriptor) + + private fun KtWhenCondition.getCheckedDescriptor(context: BindingContext): ClassDescriptor? { + return when (this) { + is KtWhenConditionIsPattern -> { + val checkedType = context.get(BindingContext.TYPE, typeReference) ?: return null + TypeUtils.getClassDescriptor(checkedType) + } + is KtWhenConditionWithExpression -> { + val reference = expression?.let { getReference(it) } ?: return null + context.get(BindingContext.REFERENCE_TARGET, reference) as? ClassDescriptor + } + else -> { + null + } + } + } + protected fun getMissingClassCases( whenExpression: KtWhenExpression, subclasses: Set, @@ -152,50 +182,32 @@ internal abstract class WhenOnClassExhaustivenessChecker : WhenExhaustivenessChe // when on empty enum / sealed is considered non-exhaustive, see test whenOnEmptySealed if (subclasses.isEmpty()) return listOf(UnknownMissingCase) - val checkedDescriptors = LinkedHashSet() + val checkedDescriptors = linkedSetOf() for (whenEntry in whenExpression.entries) { for (condition in whenEntry.conditions) { - var negated = false - var checkedDescriptor: ClassDescriptor? = null - if (condition is KtWhenConditionIsPattern) { - val checkedType = context.get(BindingContext.TYPE, condition.typeReference) - if (checkedType != null) { - checkedDescriptor = TypeUtils.getClassDescriptor(checkedType) - } - negated = condition.isNegated - } - else if (condition is KtWhenConditionWithExpression) { - if (condition.expression != null) { - val reference = getReference(condition.expression) - if (reference != null) { - val target = context.get(BindingContext.REFERENCE_TARGET, reference) - if (target is ClassDescriptor) { - checkedDescriptor = target - } - } - } - } + val negated = condition.negated + val checkedDescriptor = condition.getCheckedDescriptor(context) ?: continue + val checkedDescriptorSubclasses = + if (checkedDescriptor.modality == Modality.SEALED) checkedDescriptor.deepSealedSubclasses + else listOf(checkedDescriptor) // Checks are important only for nested subclasses of the sealed class // In additional, check without "is" is important only for objects - if (checkedDescriptor == null || - !subclasses.contains(checkedDescriptor) || - (condition is KtWhenConditionWithExpression && - !DescriptorUtils.isObject(checkedDescriptor) && - !DescriptorUtils.isEnumEntry(checkedDescriptor))) { + if (checkedDescriptorSubclasses.none { subclasses.contains(it) } || + !condition.isRelevant(checkedDescriptor)) { continue } if (negated) { - if (checkedDescriptors.contains(checkedDescriptor)) return listOf() // all members are already there + if (checkedDescriptors.containsAll(checkedDescriptorSubclasses)) return listOf() checkedDescriptors.addAll(subclasses) - checkedDescriptors.remove(checkedDescriptor) + checkedDescriptors.removeAll(checkedDescriptorSubclasses) } else { - checkedDescriptors.add(checkedDescriptor) + checkedDescriptors.addAll(checkedDescriptorSubclasses) } } } - return (subclasses - checkedDescriptors).toList().map { ClassMissingCase(it) } + return (subclasses - checkedDescriptors).map(::ClassMissingCase) } } @@ -209,7 +221,7 @@ private object WhenOnEnumExhaustivenessChecker : WhenOnClassExhaustivenessChecke assert(isEnumClass(subjectDescriptor)) { "isWhenOnEnumExhaustive should be called with an enum class descriptor" } val entryDescriptors = DescriptorUtils.getAllDescriptors(subjectDescriptor!!.unsubstitutedInnerClassesScope) - .filter { isEnumEntry(it) } + .filter(::isEnumEntry) .filterIsInstance() .toSet() return getMissingClassCases(expression, entryDescriptors, context) + @@ -222,6 +234,7 @@ private object WhenOnEnumExhaustivenessChecker : WhenOnClassExhaustivenessChecke } internal object WhenOnSealedExhaustivenessChecker : WhenOnClassExhaustivenessChecker() { + override fun getMissingCases( expression: KtWhenExpression, context: BindingContext, @@ -231,9 +244,9 @@ internal object WhenOnSealedExhaustivenessChecker : WhenOnClassExhaustivenessChe assert(DescriptorUtils.isSealedClass(subjectDescriptor)) { "isWhenOnSealedClassExhaustive should be called with a sealed class descriptor: $subjectDescriptor" } - val subclasses = subjectDescriptor!!.sealedSubclasses - // When on a sealed class without derived members is considered non-exhaustive (see test WhenOnEmptySealed) - return getMissingClassCases(expression, subclasses.toSet(), context) + + + val allSubclasses = subjectDescriptor!!.deepSealedSubclasses + return getMissingClassCases(expression, allSubclasses.toSet(), context) + WhenOnNullableExhaustivenessChecker.getMissingCases(expression, context, nullable) } @@ -268,7 +281,7 @@ object WhenChecker { } private fun whenSubjectType(expression: KtWhenExpression, context: BindingContext) = - expression.subjectExpression?.let { context.get(SMARTCAST, it)?.defaultType ?: context.getType(it) } ?: null + expression.subjectExpression?.let { context.get(SMARTCAST, it)?.defaultType ?: context.getType(it) } @JvmStatic fun getEnumMissingCases( diff --git a/compiler/testData/diagnostics/tests/sealed/ExhaustiveOnRoot.kt b/compiler/testData/diagnostics/tests/sealed/ExhaustiveOnRoot.kt new file mode 100644 index 00000000000..cdb5b081b38 --- /dev/null +++ b/compiler/testData/diagnostics/tests/sealed/ExhaustiveOnRoot.kt @@ -0,0 +1,23 @@ +sealed class Stmt + +class ForStmt : Stmt() + +sealed class Expr : Stmt() { + object BinExpr : Expr() +} + +fun test(x: Stmt): String = + when (x) { + is Expr -> "expr" + is Stmt -> "stmt" + } + +fun test2(x: Stmt): String = + when (x) { + is Expr -> "expr" + } + +fun test3(x: Expr): String = + when (x) { + is Stmt -> "stmt" + } \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/sealed/ExhaustiveOnRoot.txt b/compiler/testData/diagnostics/tests/sealed/ExhaustiveOnRoot.txt new file mode 100644 index 00000000000..48902800437 --- /dev/null +++ b/compiler/testData/diagnostics/tests/sealed/ExhaustiveOnRoot.txt @@ -0,0 +1,33 @@ +package + +public fun test(/*0*/ x: Stmt): kotlin.String +public fun test2(/*0*/ x: Stmt): kotlin.String +public fun test3(/*0*/ x: Expr): kotlin.String + +public sealed class Expr : Stmt { + private constructor Expr() + 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 + + public object BinExpr : Expr { + private constructor BinExpr() + 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 + } +} + +public final class ForStmt : Stmt { + public constructor ForStmt() + 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 +} + +public sealed class Stmt { + private constructor Stmt() + 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 +} diff --git a/compiler/testData/diagnostics/tests/sealed/ExhaustiveOnTree.kt b/compiler/testData/diagnostics/tests/sealed/ExhaustiveOnTree.kt new file mode 100644 index 00000000000..292767ade22 --- /dev/null +++ b/compiler/testData/diagnostics/tests/sealed/ExhaustiveOnTree.kt @@ -0,0 +1,34 @@ +sealed class Base { + sealed class A : Base() { + object A1 : A() + sealed class A2 : A() + } + sealed class B : Base() { + sealed class B1 : B() + object B2 : B() + } + + fun foo() = when (this) { + is A -> 1 + is B.B1 -> 2 + B.B2 -> 3 + // No else required + } + + fun bar() = when (this) { + is A -> 1 + is B.B1 -> 2 + } + + fun baz() = when (this) { + is A -> 1 + B.B2 -> 3 + // No else required (no possible B1 instances) + } + + fun negated() = when (this) { + !is A -> 1 + A.A1 -> 2 + is A.A2 -> 3 + } +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/sealed/ExhaustiveOnTree.txt b/compiler/testData/diagnostics/tests/sealed/ExhaustiveOnTree.txt new file mode 100644 index 00000000000..afa4a4d3207 --- /dev/null +++ b/compiler/testData/diagnostics/tests/sealed/ExhaustiveOnTree.txt @@ -0,0 +1,78 @@ +package + +public sealed class Base { + private constructor Base() + public final fun bar(): kotlin.Int + public final fun baz(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final fun foo(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final fun negated(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + public sealed class A : Base { + private constructor A() + public final override /*1*/ /*fake_override*/ fun bar(): kotlin.Int + public final override /*1*/ /*fake_override*/ fun baz(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final override /*1*/ /*fake_override*/ fun foo(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final override /*1*/ /*fake_override*/ fun negated(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + public object A1 : Base.A { + private constructor A1() + public final override /*1*/ /*fake_override*/ fun bar(): kotlin.Int + public final override /*1*/ /*fake_override*/ fun baz(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final override /*1*/ /*fake_override*/ fun foo(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final override /*1*/ /*fake_override*/ fun negated(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } + + public sealed class A2 : Base.A { + private constructor A2() + public final override /*1*/ /*fake_override*/ fun bar(): kotlin.Int + public final override /*1*/ /*fake_override*/ fun baz(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final override /*1*/ /*fake_override*/ fun foo(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final override /*1*/ /*fake_override*/ fun negated(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } + } + + public sealed class B : Base { + private constructor B() + public final override /*1*/ /*fake_override*/ fun bar(): kotlin.Int + public final override /*1*/ /*fake_override*/ fun baz(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final override /*1*/ /*fake_override*/ fun foo(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final override /*1*/ /*fake_override*/ fun negated(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + public sealed class B1 : Base.B { + private constructor B1() + public final override /*1*/ /*fake_override*/ fun bar(): kotlin.Int + public final override /*1*/ /*fake_override*/ fun baz(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final override /*1*/ /*fake_override*/ fun foo(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final override /*1*/ /*fake_override*/ fun negated(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } + + public object B2 : Base.B { + private constructor B2() + public final override /*1*/ /*fake_override*/ fun bar(): kotlin.Int + public final override /*1*/ /*fake_override*/ fun baz(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final override /*1*/ /*fake_override*/ fun foo(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final override /*1*/ /*fake_override*/ fun negated(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } + } +} diff --git a/compiler/testData/diagnostics/tests/sealed/ExhaustiveOnTriangleStar.kt b/compiler/testData/diagnostics/tests/sealed/ExhaustiveOnTriangleStar.kt new file mode 100644 index 00000000000..5991129f143 --- /dev/null +++ b/compiler/testData/diagnostics/tests/sealed/ExhaustiveOnTriangleStar.kt @@ -0,0 +1,24 @@ +sealed class A +sealed class B : A() + +class C : B() +class D : B() + +fun test(a: A): Any { + return when (a) { + is C -> "" + is D -> "" + } +} + +fun test2(a: A): Any { + return when (a) { + is B -> "" + } +} + +fun test3(a: A): Any { + return when (a) { + is D -> "" + } +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/sealed/ExhaustiveOnTriangleStar.txt b/compiler/testData/diagnostics/tests/sealed/ExhaustiveOnTriangleStar.txt new file mode 100644 index 00000000000..d093f4f9b27 --- /dev/null +++ b/compiler/testData/diagnostics/tests/sealed/ExhaustiveOnTriangleStar.txt @@ -0,0 +1,33 @@ +package + +public fun test(/*0*/ a: A): kotlin.Any +public fun test2(/*0*/ a: A): kotlin.Any +public fun test3(/*0*/ a: A): kotlin.Any + +public sealed class A { + private constructor A() + 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 +} + +public sealed class B : A { + private constructor B() + 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 +} + +public final class C : B { + public constructor C() + 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 +} + +public final class D : B { + public constructor D() + 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 +} diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index c111a8a3d93..dca35bac279 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -18334,6 +18334,24 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { doTest(fileName); } + @TestMetadata("ExhaustiveOnRoot.kt") + public void testExhaustiveOnRoot() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/sealed/ExhaustiveOnRoot.kt"); + doTest(fileName); + } + + @TestMetadata("ExhaustiveOnTree.kt") + public void testExhaustiveOnTree() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/sealed/ExhaustiveOnTree.kt"); + doTest(fileName); + } + + @TestMetadata("ExhaustiveOnTriangleStar.kt") + public void testExhaustiveOnTriangleStar() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/sealed/ExhaustiveOnTriangleStar.kt"); + doTest(fileName); + } + @TestMetadata("ExhaustiveWhen.kt") public void testExhaustiveWhen() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/sealed/ExhaustiveWhen.kt");