From 31573a98e84a678c4e2a371df247e0fa3475fb1d Mon Sep 17 00:00:00 2001 From: Tianyu Geng Date: Fri, 18 Jun 2021 02:12:17 +0300 Subject: [PATCH] FIR checker: report EXPECTED_CONDITION --- .../diagnostics/FirDiagnosticsList.kt | 1 + .../fir/analysis/diagnostics/FirErrors.kt | 2 ++ .../diagnostics/FirDefaultErrorMessages.kt | 2 ++ .../coneDiagnosticToFirDiagnostic.kt | 6 ++-- .../converter/ExpressionsConverter.kt | 36 ++++++++++++++----- .../kotlin/fir/builder/RawFirBuilder.kt | 13 +++++-- .../fir/diagnostics/ConeSimpleDiagnostic.kt | 8 ++++- .../tests/controlStructures/kt657.fir.kt | 4 +-- .../diagnostics/KtFirDataClassConverters.kt | 7 ++++ .../api/fir/diagnostics/KtFirDiagnostics.kt | 5 +++ .../fir/diagnostics/KtFirDiagnosticsImpl.kt | 8 +++++ 11 files changed, 76 insertions(+), 16 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 c1d9f8201ae..e1343173f38 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 @@ -867,6 +867,7 @@ object DIAGNOSTICS_LIST : DiagnosticList("FirErrors") { } val WHEN_EXPRESSIONS by object : DiagnosticGroup("When expressions") { + val EXPECTED_CONDITION by error() val NO_ELSE_IN_WHEN by error(PositioningStrategy.WHEN_EXPRESSION) { parameter>("missingWhenCases") } 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 a5af4a0a1dc..0b2efc3cc5c 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 @@ -63,6 +63,7 @@ import org.jetbrains.kotlin.psi.KtTypeParameter import org.jetbrains.kotlin.psi.KtTypeProjection import org.jetbrains.kotlin.psi.KtTypeReference import org.jetbrains.kotlin.psi.KtValueArgument +import org.jetbrains.kotlin.psi.KtWhenCondition import org.jetbrains.kotlin.psi.KtWhenEntry import org.jetbrains.kotlin.psi.KtWhenExpression import org.jetbrains.kotlin.resolve.ForbiddenNamedArgumentsTarget @@ -475,6 +476,7 @@ object FirErrors { val USELESS_IS_CHECK by warning1() // When expressions + val EXPECTED_CONDITION by error0() val NO_ELSE_IN_WHEN by error1>(SourceElementPositioningStrategies.WHEN_EXPRESSION) val INVALID_IF_AS_EXPRESSION by error0(SourceElementPositioningStrategies.IF_EXPRESSION) val ELSE_MISPLACED_IN_WHEN by error0(SourceElementPositioningStrategies.ELSE_ENTRY) 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 e2e3bb0c283..94ec47ad6fa 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 @@ -119,6 +119,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.EQUALITY_NOT_APPL import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ERROR_FROM_JAVA_RESOLUTION import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ERROR_IN_CONTRACT_DESCRIPTION import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.EXPANDED_TYPE_CANNOT_BE_INHERITED +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.EXPECTED_CONDITION import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.EXPECTED_DECLARATION_WITH_BODY import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.EXPECTED_DELEGATED_PROPERTY import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.EXPECTED_LATEINIT_PROPERTY @@ -1152,6 +1153,7 @@ class FirDefaultErrorMessages { map.put(USELESS_IS_CHECK, "Check for instance is always ''{0}''", TO_STRING) // When expressions + map.put(EXPECTED_CONDITION, "Expected condition of type Boolean") map.put(NO_ELSE_IN_WHEN, "''when'' expression must be exhaustive, add necessary {0}", WHEN_MISSING_CASES) map.put(INVALID_IF_AS_EXPRESSION, "'if' must have both main and 'else' branches if used as an expression") diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/coneDiagnosticToFirDiagnostic.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/coneDiagnosticToFirDiagnostic.kt index 53b1ffe8dbb..456fd41ad53 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/coneDiagnosticToFirDiagnostic.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/coneDiagnosticToFirDiagnostic.kt @@ -75,7 +75,7 @@ private fun ConeDiagnostic.toFirDiagnostic( FirErrors.NO_TYPE_ARGUMENTS_ON_RHS.createOn(qualifiedAccessSource ?: source, this.desiredCount, this.type) is ConeSimpleDiagnostic -> when (source.kind) { is FirFakeSourceElementKind -> null - else -> this.getFactory(source).createOn(qualifiedAccessSource ?: source) + else -> this.getFactory(source)?.createOn(qualifiedAccessSource ?: source) } is ConeInstanceAccessBeforeSuperCall -> FirErrors.INSTANCE_ACCESS_BEFORE_SUPER_CALL.createOn(source, this.target) is ConeStubDiagnostic -> null @@ -322,7 +322,7 @@ private fun ConstraintSystemError.toDiagnostic( private val NewConstraintError.lowerConeType: ConeKotlinType get() = lowerType as ConeKotlinType private val NewConstraintError.upperConeType: ConeKotlinType get() = upperType as ConeKotlinType -private fun ConeSimpleDiagnostic.getFactory(source: FirSourceElement): FirDiagnosticFactory0 { +private fun ConeSimpleDiagnostic.getFactory(source: FirSourceElement): FirDiagnosticFactory0? { @Suppress("UNCHECKED_CAST") return when (kind) { DiagnosticKind.Syntax -> FirErrors.SYNTAX @@ -341,6 +341,7 @@ private fun ConeSimpleDiagnostic.getFactory(source: FirSourceElement): FirDiagno DiagnosticKind.ExpressionExpected -> when (source.elementType) { KtNodeTypes.BINARY_EXPRESSION -> FirErrors.ASSIGNMENT_IN_EXPRESSION_CONTEXT KtNodeTypes.FUN -> FirErrors.ANONYMOUS_FUNCTION_WITH_NAME + KtNodeTypes.WHEN_CONDITION_IN_RANGE, KtNodeTypes.WHEN_CONDITION_IS_PATTERN -> FirErrors.EXPECTED_CONDITION else -> FirErrors.EXPRESSION_EXPECTED } DiagnosticKind.JumpOutsideLoop -> FirErrors.BREAK_OR_CONTINUE_OUTSIDE_A_LOOP @@ -364,6 +365,7 @@ private fun ConeSimpleDiagnostic.getFactory(source: FirSourceElement): FirDiagno DiagnosticKind.UnresolvedSupertype, DiagnosticKind.UnresolvedExpandedType, DiagnosticKind.Other -> FirErrors.OTHER_ERROR + DiagnosticKind.NotRootCause -> null else -> throw IllegalArgumentException("Unsupported diagnostic kind: $kind at $javaClass") } } diff --git a/compiler/fir/raw-fir/light-tree2fir/src/org/jetbrains/kotlin/fir/lightTree/converter/ExpressionsConverter.kt b/compiler/fir/raw-fir/light-tree2fir/src/org/jetbrains/kotlin/fir/lightTree/converter/ExpressionsConverter.kt index 9ce92d585b7..388068addbe 100644 --- a/compiler/fir/raw-fir/light-tree2fir/src/org/jetbrains/kotlin/fir/lightTree/converter/ExpressionsConverter.kt +++ b/compiler/fir/raw-fir/light-tree2fir/src/org/jetbrains/kotlin/fir/lightTree/converter/ExpressionsConverter.kt @@ -704,7 +704,13 @@ class ExpressionsConverter( @OptIn(FirContractViolation::class) val subject = FirExpressionRef() - whenEntryNodes.mapTo(whenEntries) { convertWhenEntry(it, subject.takeIf { hasSubject }) } + whenEntryNodes.mapTo(whenEntries) { + convertWhenEntry( + it, + subject.takeIf { hasSubject }, + hasSubjectVariable = subjectVariable != null + ) + } return buildWhenExpression { source = whenExpression.toFirSourceElement() this.subject = subjectExpression @@ -748,15 +754,19 @@ class ExpressionsConverter( * @see org.jetbrains.kotlin.parsing.KotlinExpressionParsing.parseWhenEntry * @see org.jetbrains.kotlin.parsing.KotlinExpressionParsing.parseWhenEntryNotElse */ - private fun convertWhenEntry(whenEntry: LighterASTNode, whenRefWithSubject: FirExpressionRef?): WhenEntry { + private fun convertWhenEntry( + whenEntry: LighterASTNode, + whenRefWithSubject: FirExpressionRef?, + hasSubjectVariable: Boolean + ): WhenEntry { var isElse = false var firBlock: FirBlock = buildEmptyExpressionBlock() val conditions = mutableListOf() whenEntry.forEachChildren { when (it.tokenType) { WHEN_CONDITION_EXPRESSION -> conditions += convertWhenConditionExpression(it, whenRefWithSubject) - WHEN_CONDITION_IN_RANGE -> conditions += convertWhenConditionInRange(it, whenRefWithSubject) - WHEN_CONDITION_IS_PATTERN -> conditions += convertWhenConditionIsPattern(it, whenRefWithSubject) + WHEN_CONDITION_IN_RANGE -> conditions += convertWhenConditionInRange(it, whenRefWithSubject, hasSubjectVariable) + WHEN_CONDITION_IS_PATTERN -> conditions += convertWhenConditionIsPattern(it, whenRefWithSubject, hasSubjectVariable) ELSE_KEYWORD -> isElse = true BLOCK -> firBlock = declarationsConverter.convertBlock(it) else -> if (it.isExpression()) firBlock = declarationsConverter.convertBlock(it) @@ -799,7 +809,8 @@ class ExpressionsConverter( private fun convertWhenConditionInRange( whenCondition: LighterASTNode, - whenRefWithSubject: FirExpressionRef? + whenRefWithSubject: FirExpressionRef?, + hasSubjectVariable: Boolean ): FirExpression { var isNegate = false var firExpression: FirExpression? = null @@ -825,7 +836,11 @@ class ExpressionsConverter( } else { return buildErrorExpression { source = whenCondition.toFirSourceElement() - diagnostic = ConeSimpleDiagnostic("No expression in condition with expression", DiagnosticKind.Syntax) + diagnostic = ConeSimpleDiagnostic( + "No expression in condition with expression", + // A subject variable without initializer should be highlighted only at the subject expression. + if (hasSubjectVariable) DiagnosticKind.NotRootCause else DiagnosticKind.ExpressionExpected + ) } } @@ -844,7 +859,8 @@ class ExpressionsConverter( private fun convertWhenConditionIsPattern( whenCondition: LighterASTNode, - whenRefWithSubject: FirExpressionRef? + whenRefWithSubject: FirExpressionRef?, + hasSubjectVariable: Boolean ): FirExpression { lateinit var firOperation: FirOperation lateinit var firType: FirTypeRef @@ -863,7 +879,11 @@ class ExpressionsConverter( } else { return buildErrorExpression { source = whenCondition.toFirSourceElement() - diagnostic = ConeSimpleDiagnostic("No expression in condition with expression", DiagnosticKind.Syntax) + diagnostic = ConeSimpleDiagnostic( + "No expression in condition with expression", + // A subject variable without initializer should be highlighted only at the subject expression. + if (hasSubjectVariable) DiagnosticKind.NotRootCause else DiagnosticKind.ExpressionExpected + ) } } diff --git a/compiler/fir/raw-fir/psi2fir/src/org/jetbrains/kotlin/fir/builder/RawFirBuilder.kt b/compiler/fir/raw-fir/psi2fir/src/org/jetbrains/kotlin/fir/builder/RawFirBuilder.kt index 0433f3d537b..5025690e9f3 100644 --- a/compiler/fir/raw-fir/psi2fir/src/org/jetbrains/kotlin/fir/builder/RawFirBuilder.kt +++ b/compiler/fir/raw-fir/psi2fir/src/org/jetbrains/kotlin/fir/builder/RawFirBuilder.kt @@ -195,7 +195,7 @@ open class RawFirBuilder( ) } - private fun KtExpression?.toFirExpression( + private fun KtElement?.toFirExpression( errorReason: String, kind: DiagnosticKind = DiagnosticKind.ExpressionExpected, ): FirExpression { @@ -1747,10 +1747,17 @@ open class RawFirBuilder( result = branchBody } } else { - val ktCondition = entry.conditions.first() as? KtWhenConditionWithExpression + val ktCondition = entry.conditions.first() buildWhenBranch { source = entrySource - condition = ktCondition?.expression.toFirExpression("No expression in condition with expression") + condition = ((ktCondition as? KtWhenConditionWithExpression)?.expression + ?: ktCondition) + .toFirExpression( + "No expression in condition with expression", + // A subject variable without initializer should be highlighted only at the subject expression. + if (ktSubjectExpression is KtVariableDeclaration) DiagnosticKind.NotRootCause + else DiagnosticKind.ExpressionExpected + ) result = branchBody } } diff --git a/compiler/fir/tree/src/org/jetbrains/kotlin/fir/diagnostics/ConeSimpleDiagnostic.kt b/compiler/fir/tree/src/org/jetbrains/kotlin/fir/diagnostics/ConeSimpleDiagnostic.kt index fddf4110465..8b400d19419 100644 --- a/compiler/fir/tree/src/org/jetbrains/kotlin/fir/diagnostics/ConeSimpleDiagnostic.kt +++ b/compiler/fir/tree/src/org/jetbrains/kotlin/fir/diagnostics/ConeSimpleDiagnostic.kt @@ -51,5 +51,11 @@ enum class DiagnosticKind { FloatLiteralOutOfRange, WrongLongSuffix, - Other + Other, + + /** + * Special diagnostic kind that is caused by issues at other places in the code. Such diagnostics should not be reported so that user + * sees less noise. + */ + NotRootCause } diff --git a/compiler/testData/diagnostics/tests/controlStructures/kt657.fir.kt b/compiler/testData/diagnostics/tests/controlStructures/kt657.fir.kt index 779d04e5bbd..3633b010316 100644 --- a/compiler/testData/diagnostics/tests/controlStructures/kt657.fir.kt +++ b/compiler/testData/diagnostics/tests/controlStructures/kt657.fir.kt @@ -9,9 +9,9 @@ fun foo() = cond2() -> 2 4 -> 34 Pair(1, 2) -> 3 - in 1..10 -> 34 + in 1..10 -> 34 4 -> 38 - is Int -> 33 + is Int -> 33 else -> 34 } 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 8a66d4f2592..4d4a71a6d03 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 @@ -48,6 +48,7 @@ import org.jetbrains.kotlin.psi.KtTypeParameter import org.jetbrains.kotlin.psi.KtTypeProjection import org.jetbrains.kotlin.psi.KtTypeReference import org.jetbrains.kotlin.psi.KtValueArgument +import org.jetbrains.kotlin.psi.KtWhenCondition import org.jetbrains.kotlin.psi.KtWhenEntry import org.jetbrains.kotlin.psi.KtWhenExpression @@ -2376,6 +2377,12 @@ internal val KT_DIAGNOSTIC_CONVERTER = KtDiagnosticConverterBuilder.buildConvert token, ) } + add(FirErrors.EXPECTED_CONDITION) { firDiagnostic -> + ExpectedConditionImpl( + firDiagnostic as FirPsiDiagnostic<*>, + token, + ) + } add(FirErrors.NO_ELSE_IN_WHEN) { firDiagnostic -> NoElseInWhenImpl( firDiagnostic.a.map { whenMissingCase -> 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 593a9f8c825..b6b9cab418a 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 @@ -55,6 +55,7 @@ import org.jetbrains.kotlin.psi.KtTypeParameter import org.jetbrains.kotlin.psi.KtTypeProjection import org.jetbrains.kotlin.psi.KtTypeReference import org.jetbrains.kotlin.psi.KtValueArgument +import org.jetbrains.kotlin.psi.KtWhenCondition import org.jetbrains.kotlin.psi.KtWhenEntry import org.jetbrains.kotlin.psi.KtWhenExpression import org.jetbrains.kotlin.resolve.ForbiddenNamedArgumentsTarget @@ -1675,6 +1676,10 @@ sealed class KtFirDiagnostic : KtDiagnosticWithPsi { abstract val compileTimeCheckResult: Boolean } + abstract class ExpectedCondition : KtFirDiagnostic() { + override val diagnosticClass get() = ExpectedCondition::class + } + abstract class NoElseInWhen : KtFirDiagnostic() { override val diagnosticClass get() = NoElseInWhen::class abstract val missingWhenCases: List 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 15ce5dd5136..dff5807e5fe 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 @@ -57,6 +57,7 @@ import org.jetbrains.kotlin.psi.KtTypeParameter import org.jetbrains.kotlin.psi.KtTypeProjection import org.jetbrains.kotlin.psi.KtTypeReference import org.jetbrains.kotlin.psi.KtValueArgument +import org.jetbrains.kotlin.psi.KtWhenCondition import org.jetbrains.kotlin.psi.KtWhenEntry import org.jetbrains.kotlin.psi.KtWhenExpression import org.jetbrains.kotlin.resolve.ForbiddenNamedArgumentsTarget @@ -2708,6 +2709,13 @@ internal class UselessIsCheckImpl( override val firDiagnostic: FirPsiDiagnostic by weakRef(firDiagnostic) } +internal class ExpectedConditionImpl( + firDiagnostic: FirPsiDiagnostic<*>, + override val token: ValidityToken, +) : KtFirDiagnostic.ExpectedCondition(), KtAbstractFirDiagnostic { + override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic) +} + internal class NoElseInWhenImpl( override val missingWhenCases: List, firDiagnostic: FirPsiDiagnostic,