From e30b9758f4ec917a8ed3b634abffa61e6cb76b53 Mon Sep 17 00:00:00 2001 From: fitermay Date: Wed, 10 May 2017 22:56:06 -0400 Subject: [PATCH] Introduce action to add missing when branches on sealed class Made via diagnostics NON_EXHAUSTIVE_WHEN_FOR_SEALED_CLASS with INFO severity and quick-fix So #KT-17580 Fixed --- .../cfg/ControlFlowInformationProvider.kt | 7 ++++ .../org/jetbrains/kotlin/cfg/WhenChecker.kt | 13 +++++++- .../jetbrains/kotlin/diagnostics/Errors.java | 4 +++ .../rendering/DefaultErrorMessages.java | 1 + .../NonExhaustiveWarningForSealedClass.kt | 16 +++++++++ .../NonExhaustiveWarningForSealedClass.txt | 33 +++++++++++++++++++ .../checkers/DiagnosticsTestGenerated.java | 6 ++++ .../idea/highlighter/KotlinPsiChecker.kt | 4 +-- .../kotlin/idea/quickfix/QuickFixRegistrar.kt | 2 ++ .../addRemainingBranchesSealedStatement.kt | 16 +++++++++ ...dRemainingBranchesSealedStatement.kt.after | 19 +++++++++++ .../idea/quickfix/QuickFixTestGenerated.java | 6 ++++ 12 files changed, 124 insertions(+), 3 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/when/NonExhaustiveWarningForSealedClass.kt create mode 100644 compiler/testData/diagnostics/tests/when/NonExhaustiveWarningForSealedClass.txt create mode 100644 idea/testData/quickfix/when/addRemainingBranchesSealedStatement.kt create mode 100644 idea/testData/quickfix/when/addRemainingBranchesSealedStatement.kt.after diff --git a/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.kt b/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.kt index 81f23a8e1f9..6f9a33f81b9 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.kt @@ -808,6 +808,13 @@ class ControlFlowInformationProvider private constructor( trace.report(NON_EXHAUSTIVE_WHEN.on(element, enumMissingCases)) } } + val sealedClassDescriptor = WhenChecker.getClassDescriptorOfTypeIfSealed(subjectType) + if (sealedClassDescriptor != null) { + val sealedMissingCases = WhenChecker.getSealedMissingCases(element, context, sealedClassDescriptor) + if (!sealedMissingCases.isEmpty()) { + trace.report(NON_EXHAUSTIVE_WHEN_ON_SEALED_CLASS.on(element, sealedMissingCases)) + } + } } } } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/cfg/WhenChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/cfg/WhenChecker.kt index 602083366ec..1880bbfc491 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/cfg/WhenChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/cfg/WhenChecker.kt @@ -32,7 +32,6 @@ import org.jetbrains.kotlin.resolve.CompileTimeConstantUtils import org.jetbrains.kotlin.resolve.DescriptorUtils import org.jetbrains.kotlin.resolve.DescriptorUtils.isEnumClass 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.types.KotlinType @@ -275,6 +274,11 @@ object WhenChecker { return classDescriptor } + @JvmStatic + fun getClassDescriptorOfTypeIfSealed(type: KotlinType?): ClassDescriptor? + = type?.let { TypeUtils.getClassDescriptor(it) }?.takeIf { DescriptorUtils.isSealedClass(it) } + + @JvmStatic fun whenSubjectType(expression: KtWhenExpression, context: BindingContext) = expression.subjectExpression?.let { context.get(SMARTCAST, it)?.defaultType ?: context.getType(it) } @@ -286,6 +290,13 @@ object WhenChecker { enumClassDescriptor: ClassDescriptor ) = WhenOnEnumExhaustivenessChecker.getMissingCases(expression, context, enumClassDescriptor, false) + @JvmStatic + fun getSealedMissingCases( + expression: KtWhenExpression, + context: BindingContext, + sealedClassDescriptor: ClassDescriptor + ) = WhenOnSealedExhaustivenessChecker.getMissingCases(expression, context, sealedClassDescriptor, false) + fun getMissingCases(expression: KtWhenExpression, context: BindingContext): List { val type = whenSubjectType(expression, context) ?: return listOf(UnknownMissingCase) val nullable = type.isMarkedNullable diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index 6392ded875d..2f705a9bad5 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -50,6 +50,7 @@ import java.util.Map; import static org.jetbrains.kotlin.diagnostics.PositioningStrategies.*; import static org.jetbrains.kotlin.diagnostics.Severity.ERROR; +import static org.jetbrains.kotlin.diagnostics.Severity.INFO; import static org.jetbrains.kotlin.diagnostics.Severity.WARNING; /** @@ -849,6 +850,9 @@ public interface Errors { DiagnosticFactory0 REDUNDANT_ELSE_IN_WHEN = DiagnosticFactory0.create(WARNING, ELSE_ENTRY); DiagnosticFactory1> NO_ELSE_IN_WHEN = DiagnosticFactory1.create(ERROR, WHEN_EXPRESSION); DiagnosticFactory1> NON_EXHAUSTIVE_WHEN = DiagnosticFactory1.create(WARNING, WHEN_EXPRESSION); + DiagnosticFactory1> + NON_EXHAUSTIVE_WHEN_ON_SEALED_CLASS = DiagnosticFactory1.create(INFO, WHEN_EXPRESSION); + DiagnosticFactory0 COMMA_IN_WHEN_CONDITION_WITHOUT_ARGUMENT = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 DUPLICATE_LABEL_IN_WHEN = DiagnosticFactory0.create(WARNING); 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 9ba12409967..3a7249758c2 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -520,6 +520,7 @@ public class DefaultErrorMessages { 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(NON_EXHAUSTIVE_WHEN_ON_SEALED_CLASS, "''when'' expression on sealed classes 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/testData/diagnostics/tests/when/NonExhaustiveWarningForSealedClass.kt b/compiler/testData/diagnostics/tests/when/NonExhaustiveWarningForSealedClass.kt new file mode 100644 index 00000000000..66d9dcea181 --- /dev/null +++ b/compiler/testData/diagnostics/tests/when/NonExhaustiveWarningForSealedClass.kt @@ -0,0 +1,16 @@ +sealed class S + +object First : S() + +class Derived(val s: String) : S() + +object Last : S() + +fun use(s: String) = s + +fun foo(s: S) { + when (s) { + First -> {} + is Derived -> use(s.s) + } +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/when/NonExhaustiveWarningForSealedClass.txt b/compiler/testData/diagnostics/tests/when/NonExhaustiveWarningForSealedClass.txt new file mode 100644 index 00000000000..41884e0fcb4 --- /dev/null +++ b/compiler/testData/diagnostics/tests/when/NonExhaustiveWarningForSealedClass.txt @@ -0,0 +1,33 @@ +package + +public fun foo(/*0*/ s: S): kotlin.Unit +public fun use(/*0*/ s: kotlin.String): kotlin.String + +public final class Derived : S { + public constructor Derived(/*0*/ s: kotlin.String) + public final val s: kotlin.String + 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 First : S { + private constructor First() + 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 Last : S { + private constructor Last() + 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 S { + private constructor S() + 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 21f28fd4b2c..c643ff26961 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -23458,6 +23458,12 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { doTest(fileName); } + @TestMetadata("NonExhaustiveWarningForSealedClass.kt") + public void testNonExhaustiveWarningForSealedClass() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/when/NonExhaustiveWarningForSealedClass.kt"); + doTest(fileName); + } + @TestMetadata("NonExhaustiveWarningNull.kt") public void testNonExhaustiveWarningNull() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/when/NonExhaustiveWarningNull.kt"); diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/highlighter/KotlinPsiChecker.kt b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/highlighter/KotlinPsiChecker.kt index 7e6b011347f..8a0ab303614 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/highlighter/KotlinPsiChecker.kt +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/highlighter/KotlinPsiChecker.kt @@ -250,7 +250,7 @@ private class ElementAnnotator(private val element: PsiElement, } ) } - Severity.INFO -> return // Do nothing + Severity.INFO -> AnnotationPresentationInfo(ranges, highlightType = ProblemHighlightType.INFORMATION) } setUpAnnotations(diagnostics, presentationInfo) @@ -298,7 +298,7 @@ private class AnnotationPresentationInfo( holder.createWarningAnnotation(range, defaultMessage) } } - else -> throw IllegalArgumentException("Only ERROR and WARNING diagnostics are supported") + Severity.INFO -> holder.createInfoAnnotation(range, defaultMessage) } annotation.tooltip = getMessage(diagnostic) diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt index 100ef3e93d6..f41f96ff6b5 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt @@ -241,6 +241,8 @@ class QuickFixRegistrar : QuickFixContributor { REDUNDANT_ELSE_IN_WHEN.registerFactory(RemoveWhenElseBranchFix) NON_EXHAUSTIVE_WHEN.registerFactory(AddWhenElseBranchFix) NON_EXHAUSTIVE_WHEN.registerFactory(AddWhenRemainingBranchesFix) + NON_EXHAUSTIVE_WHEN_ON_SEALED_CLASS.registerFactory(AddWhenElseBranchFix) + NON_EXHAUSTIVE_WHEN_ON_SEALED_CLASS.registerFactory(AddWhenRemainingBranchesFix) BREAK_OR_CONTINUE_IN_WHEN.registerFactory(AddLoopLabelFix) NO_TYPE_ARGUMENTS_ON_RHS.registerFactory(AddStarProjectionsFix.IsExpressionFactory) diff --git a/idea/testData/quickfix/when/addRemainingBranchesSealedStatement.kt b/idea/testData/quickfix/when/addRemainingBranchesSealedStatement.kt new file mode 100644 index 00000000000..c2160d2ec5d --- /dev/null +++ b/idea/testData/quickfix/when/addRemainingBranchesSealedStatement.kt @@ -0,0 +1,16 @@ +// "Add remaining branches" "true" +// ERROR: Unresolved reference: TODO +// ERROR: Unresolved reference: TODO +// ERROR: Unresolved reference: TODO +sealed class Variant { + object Singleton : Variant() + + class Something(val x: Int) : Variant() + + object Another : Variant() +} +fun test(v: Variant?) { + when(v) { + Variant.Singleton -> "s" + } +} diff --git a/idea/testData/quickfix/when/addRemainingBranchesSealedStatement.kt.after b/idea/testData/quickfix/when/addRemainingBranchesSealedStatement.kt.after new file mode 100644 index 00000000000..81475ff6a45 --- /dev/null +++ b/idea/testData/quickfix/when/addRemainingBranchesSealedStatement.kt.after @@ -0,0 +1,19 @@ +// "Add remaining branches" "true" +// ERROR: Unresolved reference: TODO +// ERROR: Unresolved reference: TODO +// ERROR: Unresolved reference: TODO +sealed class Variant { + object Singleton : Variant() + + class Something(val x: Int) : Variant() + + object Another : Variant() +} +fun test(v: Variant?) { + when(v) { + Variant.Singleton -> "s" + is Variant.Something -> TODO() + Variant.Another -> TODO() + null -> TODO() + } +} diff --git a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java index 64a749b95b7..4917d6d07fa 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java @@ -10957,6 +10957,12 @@ public class QuickFixTestGenerated extends AbstractQuickFixTest { doTest(fileName); } + @TestMetadata("addRemainingBranchesSealedStatement.kt") + public void testAddRemainingBranchesSealedStatement() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/when/addRemainingBranchesSealedStatement.kt"); + doTest(fileName); + } + public void testAllFilesPresentInWhen() throws Exception { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/quickfix/when"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), TargetBackend.ANY, true); }