From 4915d8dda33140b7f4ee7a7cc8b9d7d9cb9e4a33 Mon Sep 17 00:00:00 2001 From: Tianyu Geng Date: Wed, 23 Jun 2021 13:31:40 -0700 Subject: [PATCH] FIR checker: support DUPLICATE_LABEL_IN_WHEN Changes from FE1.0: 1. As discussed previously, no expression evaluation happens during this check. 2. FE1.0 doesn't check redundant object comparisons. --- .../diagnostics/FirDiagnosticsList.kt | 1 + .../fir/analysis/diagnostics/FirErrors.kt | 1 + .../expression/FirWhenConditionChecker.kt | 46 ++++++++++++++++++- .../diagnostics/FirDefaultErrorMessages.kt | 2 + .../tests/when/DuplicatedLabels.fir.kt | 25 ++++++---- .../tests/when/DuplicatedLabels.kt | 9 ++++ .../tests/when/DuplicatedLabels.txt | 9 ++++ .../when-expression/p-4/neg/1.1.fir.kt | 8 ++-- .../when-expression/p-4/pos/1.1.fir.kt | 8 ++-- .../diagnostics/KtFirDataClassConverters.kt | 6 +++ .../api/fir/diagnostics/KtFirDiagnostics.kt | 4 ++ .../fir/diagnostics/KtFirDiagnosticsImpl.kt | 7 +++ 12 files changed, 109 insertions(+), 17 deletions(-) diff --git a/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt b/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt index 2defbd77a5e..b9ac3edd237 100644 --- a/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt +++ b/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt @@ -1063,6 +1063,7 @@ object DIAGNOSTICS_LIST : DiagnosticList("FirErrors") { parameter("illegalReason") } val COMMA_IN_WHEN_CONDITION_WITHOUT_ARGUMENT by error(PositioningStrategy.COMMAS) + val DUPLICATE_LABEL_IN_WHEN by warning() } val CONTEXT_TRACKING by object : DiagnosticGroup("Context tracking") { diff --git a/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt b/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt index 66fe7052947..7573c37cdfc 100644 --- a/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt +++ b/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt @@ -563,6 +563,7 @@ object FirErrors { val ELSE_MISPLACED_IN_WHEN by error0(SourceElementPositioningStrategies.ELSE_ENTRY) val ILLEGAL_DECLARATION_IN_WHEN_SUBJECT by error1() val COMMA_IN_WHEN_CONDITION_WITHOUT_ARGUMENT by error0(SourceElementPositioningStrategies.COMMAS) + val DUPLICATE_LABEL_IN_WHEN by warning0() // Context tracking val TYPE_PARAMETER_IS_NOT_AN_EXPRESSION by error1() diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirWhenConditionChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirWhenConditionChecker.kt index 0bf30b8bb65..f8f7147078a 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirWhenConditionChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirWhenConditionChecker.kt @@ -5,12 +5,19 @@ package org.jetbrains.kotlin.fir.analysis.checkers.expression +import org.jetbrains.kotlin.descriptors.ClassKind import org.jetbrains.kotlin.fir.analysis.checkers.checkCondition +import org.jetbrains.kotlin.fir.analysis.checkers.classKind import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors +import org.jetbrains.kotlin.fir.analysis.diagnostics.reportOn import org.jetbrains.kotlin.fir.analysis.diagnostics.withSuppressedDiagnostics -import org.jetbrains.kotlin.fir.expressions.FirWhenExpression +import org.jetbrains.kotlin.fir.expressions.* import org.jetbrains.kotlin.fir.expressions.impl.FirElseIfTrueCondition +import org.jetbrains.kotlin.fir.symbols.impl.FirEnumEntrySymbol +import org.jetbrains.kotlin.fir.types.ConeKotlinType +import org.jetbrains.kotlin.fir.types.coneType object FirWhenConditionChecker : FirWhenExpressionChecker() { override fun check(expression: FirWhenExpression, context: CheckerContext, reporter: DiagnosticReporter) { @@ -21,5 +28,42 @@ object FirWhenConditionChecker : FirWhenExpressionChecker() { checkCondition(condition, context, reporter) } } + if (expression.subject != null) { + checkDuplicatedLabels(expression, context, reporter) + } + } + + private fun checkDuplicatedLabels(expression: FirWhenExpression, context: CheckerContext, reporter: DiagnosticReporter) { + // The second part of each pair indicates whether the `is` check is positive or negated. + val checkedTypes = hashSetOf>() + val checkedConstants = hashSetOf() + for (branch in expression.branches) { + when (val condition = branch.condition) { + is FirEqualityOperatorCall -> { + val arguments = condition.arguments + if (arguments.size == 2 && arguments[0] is FirWhenSubjectExpression) { + val value = when (val targetExpression = arguments[1]) { + is FirConstExpression<*> -> targetExpression.value + is FirQualifiedAccessExpression -> targetExpression.calleeReference.toResolvedCallableSymbol() as? FirEnumEntrySymbol + ?: continue + is FirResolvedQualifier -> { + val classSymbol = targetExpression.symbol ?: continue + if (classSymbol.classKind != ClassKind.OBJECT) continue + classSymbol.classId + } + else -> continue + } + if (!checkedConstants.add(value)) { + reporter.reportOn(condition.source, FirErrors.DUPLICATE_LABEL_IN_WHEN, context) + } + } + } + is FirTypeOperatorCall -> { + if (!checkedTypes.add(condition.conversionTypeRef.coneType to condition.operation)) { + reporter.reportOn(condition.conversionTypeRef.source, FirErrors.DUPLICATE_LABEL_IN_WHEN, context) + } + } + } + } } } \ No newline at end of file diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirDefaultErrorMessages.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirDefaultErrorMessages.kt index 5736a742b0f..933e24b33b1 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirDefaultErrorMessages.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirDefaultErrorMessages.kt @@ -143,6 +143,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.DEPRECATED_TYPE_P import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.DEPRECATION import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.DEPRECATION_ERROR import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.DESERIALIZATION_ERROR +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.DUPLICATE_LABEL_IN_WHEN import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.DYNAMIC_UPPER_BOUND import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.EMPTY_RANGE import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ENUM_ENTRY_AS_TYPE @@ -1417,6 +1418,7 @@ class FirDefaultErrorMessages { WHEN_MISSING_CASES ) map.put(COMMA_IN_WHEN_CONDITION_WITHOUT_ARGUMENT, "Deprecated syntax. Use '||' instead of commas in when-condition for 'when' without argument") + map.put(DUPLICATE_LABEL_IN_WHEN, "Duplicate label in when") // Context tracking map.put(TYPE_PARAMETER_IS_NOT_AN_EXPRESSION, "Type parameter ''{0}'' is not an expression", SYMBOL) diff --git a/compiler/testData/diagnostics/tests/when/DuplicatedLabels.fir.kt b/compiler/testData/diagnostics/tests/when/DuplicatedLabels.fir.kt index 4b05362114d..f459dd47917 100644 --- a/compiler/testData/diagnostics/tests/when/DuplicatedLabels.fir.kt +++ b/compiler/testData/diagnostics/tests/when/DuplicatedLabels.fir.kt @@ -15,10 +15,10 @@ const val four = 4 fun first(arg: Int) = when (arg) { 1 -> 2 2 -> 3 - 1 -> 4 + 1 -> 4 4 -> 5 - 1 -> 6 - 2 -> 7 + 1 -> 6 + 2 -> 7 // Error should be here: see KT-11971 four -> 8 else -> 0 @@ -28,8 +28,8 @@ fun second(arg: String): Int { when (arg) { "ABC" -> return 0 "DEF" -> return 1 - "ABC" -> return -1 - "DEF" -> return -2 + "ABC" -> return -1 + "DEF" -> return -2 } return 42 } @@ -39,8 +39,9 @@ fun third(arg: Any?): Int { null -> return -1 is String -> return 0 is Double -> return 1 - is Double -> return 2 - null -> return 3 + is Double -> return 2 + null -> return 3 + !is String -> return 4 else -> return 5 } } @@ -50,7 +51,7 @@ enum class Color { RED, GREEN, BLUE } fun fourth(arg: Color) = when (arg) { Color.RED -> "RED" Color.GREEN -> "GREEN" - Color.RED -> "BLUE" + Color.RED -> "BLUE" Color.BLUE -> "BLUE" } @@ -59,3 +60,11 @@ fun fifth(arg: Any?) = when (arg) { else -> "" else -> null } + +object Foo + +fun sixth(arg: Any?) = when (arg) { + Foo -> "" + Foo -> "" + else -> null +} diff --git a/compiler/testData/diagnostics/tests/when/DuplicatedLabels.kt b/compiler/testData/diagnostics/tests/when/DuplicatedLabels.kt index ffddb756eb7..ba23b384a53 100644 --- a/compiler/testData/diagnostics/tests/when/DuplicatedLabels.kt +++ b/compiler/testData/diagnostics/tests/when/DuplicatedLabels.kt @@ -41,6 +41,7 @@ fun third(arg: Any?): Int { is Double -> return 1 is Double -> return 2 null -> return 3 + !is String -> return 4 else -> return 5 } } @@ -59,3 +60,11 @@ fun fifth(arg: Any?) = when (arg) { else -> "" else -> null } + +object Foo + +fun sixth(arg: Any?) = when (arg) { + Foo -> "" + Foo -> "" + else -> null +} diff --git a/compiler/testData/diagnostics/tests/when/DuplicatedLabels.txt b/compiler/testData/diagnostics/tests/when/DuplicatedLabels.txt index 142ee7156a9..12ecc70d9c1 100644 --- a/compiler/testData/diagnostics/tests/when/DuplicatedLabels.txt +++ b/compiler/testData/diagnostics/tests/when/DuplicatedLabels.txt @@ -6,6 +6,7 @@ package test { public fun first(/*0*/ arg: kotlin.Int): kotlin.Int public fun fourth(/*0*/ arg: test.Color): kotlin.String public fun second(/*0*/ arg: kotlin.String): kotlin.Int + public fun sixth(/*0*/ arg: kotlin.Any?): kotlin.String? public fun third(/*0*/ arg: kotlin.Any?): kotlin.Int public final enum class Color : kotlin.Enum { @@ -30,4 +31,12 @@ package test { public final /*synthesized*/ fun valueOf(/*0*/ value: kotlin.String): test.Color public final /*synthesized*/ fun values(): kotlin.Array } + + public object Foo { + private constructor Foo() + 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-spec/testData/diagnostics/linked/expressions/when-expression/p-4/neg/1.1.fir.kt b/compiler/tests-spec/testData/diagnostics/linked/expressions/when-expression/p-4/neg/1.1.fir.kt index 7b0a61a6302..2f59e90a64b 100644 --- a/compiler/tests-spec/testData/diagnostics/linked/expressions/when-expression/p-4/neg/1.1.fir.kt +++ b/compiler/tests-spec/testData/diagnostics/linked/expressions/when-expression/p-4/neg/1.1.fir.kt @@ -26,7 +26,7 @@ fun case1() { val z = JavaEnum.Val_1 val when2 = when (z) { JavaEnum.Val_1 -> { } - JavaEnum.Val_1 -> { } + JavaEnum.Val_1 -> { } } } @@ -37,7 +37,7 @@ fun case2() { val b = false val when2: Any = when (b) { false -> { } - false -> { } + false -> { } } } @@ -47,7 +47,7 @@ fun case3() { val a = false val when2: Any = when (a) { true -> { } - true -> { } + true -> { } } } @@ -58,7 +58,7 @@ fun case4() { val when2 = when (x){ is SClass.A ->{ } is SClass.B ->{ } - is SClass.B ->{ } + is SClass.B ->{ } } } diff --git a/compiler/tests-spec/testData/diagnostics/linked/expressions/when-expression/p-4/pos/1.1.fir.kt b/compiler/tests-spec/testData/diagnostics/linked/expressions/when-expression/p-4/pos/1.1.fir.kt index db667a1f77b..4a43164c7af 100644 --- a/compiler/tests-spec/testData/diagnostics/linked/expressions/when-expression/p-4/pos/1.1.fir.kt +++ b/compiler/tests-spec/testData/diagnostics/linked/expressions/when-expression/p-4/pos/1.1.fir.kt @@ -37,7 +37,7 @@ fun case1() { val when3 = when (z) { JavaEnum.Val_1 -> { } JavaEnum.Val_2 -> { } - JavaEnum.Val_2 -> { } + JavaEnum.Val_2 -> { } } } @@ -57,7 +57,7 @@ fun case2() { } val when3: Any = when (b) { false -> { } - false -> { } + false -> { } !false -> { } } } @@ -78,7 +78,7 @@ fun case3() { val when3: Any = when (a) { true -> { } false -> { } - false -> { } + false -> { } } } @@ -102,7 +102,7 @@ fun case4() { val when3 = when (x){ is SClass.A ->{ } is SClass.B ->{ } - is SClass.B ->{ } + is SClass.B ->{ } is SClass.C ->{ } } } diff --git a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt index 3746e3cce93..3cc4bfb97fe 100644 --- a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt +++ b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt @@ -2933,6 +2933,12 @@ internal val KT_DIAGNOSTIC_CONVERTER = KtDiagnosticConverterBuilder.buildConvert token, ) } + add(FirErrors.DUPLICATE_LABEL_IN_WHEN) { firDiagnostic -> + DuplicateLabelInWhenImpl( + firDiagnostic as FirPsiDiagnostic, + token, + ) + } add(FirErrors.TYPE_PARAMETER_IS_NOT_AN_EXPRESSION) { firDiagnostic -> TypeParameterIsNotAnExpressionImpl( firSymbolBuilder.classifierBuilder.buildTypeParameterSymbol(firDiagnostic.a.fir), diff --git a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt index daa658da5fa..d6d573203dc 100644 --- a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt +++ b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt @@ -2050,6 +2050,10 @@ sealed class KtFirDiagnostic : KtDiagnosticWithPsi { override val diagnosticClass get() = CommaInWhenConditionWithoutArgument::class } + abstract class DuplicateLabelInWhen : KtFirDiagnostic() { + override val diagnosticClass get() = DuplicateLabelInWhen::class + } + abstract class TypeParameterIsNotAnExpression : KtFirDiagnostic() { override val diagnosticClass get() = TypeParameterIsNotAnExpression::class abstract val typeParameter: KtTypeParameterSymbol diff --git a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt index 967ce2f58cc..f0ffc5bc926 100644 --- a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt +++ b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt @@ -3305,6 +3305,13 @@ internal class CommaInWhenConditionWithoutArgumentImpl( override val firDiagnostic: FirPsiDiagnostic by weakRef(firDiagnostic) } +internal class DuplicateLabelInWhenImpl( + firDiagnostic: FirPsiDiagnostic, + override val token: ValidityToken, +) : KtFirDiagnostic.DuplicateLabelInWhen(), KtAbstractFirDiagnostic { + override val firDiagnostic: FirPsiDiagnostic by weakRef(firDiagnostic) +} + internal class TypeParameterIsNotAnExpressionImpl( override val typeParameter: KtTypeParameterSymbol, firDiagnostic: FirPsiDiagnostic,