From d84cc4333c901972cb85aefaf6564b6b703797a5 Mon Sep 17 00:00:00 2001 From: Andrey Zinovyev Date: Tue, 27 Jul 2021 11:45:50 +0300 Subject: [PATCH] [FIR] Add INITIALIZATION_BEFORE_DECLARATION diagnostic --- .../diagnostics/FirDiagnosticsList.kt | 4 + .../fir/analysis/diagnostics/FirErrors.kt | 1 + .../kotlin/fir/analysis/cfa/CfgTraverser.kt | 16 ++-- .../checkers/CommonDeclarationCheckers.kt | 3 +- .../declaration/FirDeclarationCheckerUtils.kt | 4 +- .../FirPropertyInitializationChecker.kt | 81 +++++++++++++++++++ .../diagnostics/FirDefaultErrorMessages.kt | 3 + .../assignmentInLocalsInConstructor.fir.kt | 2 +- .../tests/controlFlowAnalysis/kt897.fir.kt | 20 ----- .../tests/controlFlowAnalysis/kt897.kt | 1 + .../diagnostics/KtFirDataClassConverters.kt | 7 ++ .../api/fir/diagnostics/KtFirDiagnostics.kt | 5 ++ .../fir/diagnostics/KtFirDiagnosticsImpl.kt | 8 ++ 13 files changed, 123 insertions(+), 32 deletions(-) create mode 100644 compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirPropertyInitializationChecker.kt delete mode 100644 compiler/testData/diagnostics/tests/controlFlowAnalysis/kt897.fir.kt 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 25500a4de10..94d727fedbe 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 @@ -939,6 +939,10 @@ object DIAGNOSTICS_LIST : DiagnosticList("FirErrors") { } val WRONG_IMPLIES_CONDITION by warning() val VARIABLE_WITH_NO_TYPE_NO_INITIALIZER by error(PositioningStrategy.DECLARATION_NAME) + + val INITIALIZATION_BEFORE_DECLARATION by error() { + parameter("property") + } } val NULLABILITY by object : DiagnosticGroup("Nullability") { 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 f0f6cca1f7d..2ecce319a56 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 @@ -498,6 +498,7 @@ object FirErrors { val LEAKED_IN_PLACE_LAMBDA by error1>() val WRONG_IMPLIES_CONDITION by warning0() val VARIABLE_WITH_NO_TYPE_NO_INITIALIZER by error0(SourceElementPositioningStrategies.DECLARATION_NAME) + val INITIALIZATION_BEFORE_DECLARATION by error1>() // Nullability val UNSAFE_CALL by error2(SourceElementPositioningStrategies.DOT_BY_QUALIFIED) diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/CfgTraverser.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/CfgTraverser.kt index 96ec0904e74..f6bf02f67f0 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/CfgTraverser.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/CfgTraverser.kt @@ -33,10 +33,11 @@ fun ControlFlowGraph.traverse( // ---------------------- Path-sensitive data collection ----------------------- -fun , K : Any, V : Any> ControlFlowGraph.collectDataForNode( +fun ControlFlowGraph.collectDataForNode( direction: TraverseDirection, initialInfo: I, - visitor: ControlFlowGraphVisitor>> + visitor: ControlFlowGraphVisitor>>, + visitSubGraphs: Boolean = true ): Map, I> { val nodeMap = LinkedHashMap, I>() val startNode = getEnterNode(direction) @@ -44,22 +45,23 @@ fun , K : Any, V : Any> ControlFlowGraph.collectDat val changed = mutableMapOf, Boolean>() do { - collectDataForNodeInternal(direction, initialInfo, visitor, nodeMap, changed) + collectDataForNodeInternal(direction, initialInfo, visitor, nodeMap, changed, visitSubGraphs) } while (changed.any { it.value }) return nodeMap } -private fun , K : Any, V : Any> ControlFlowGraph.collectDataForNodeInternal( +private fun ControlFlowGraph.collectDataForNodeInternal( direction: TraverseDirection, initialInfo: I, visitor: ControlFlowGraphVisitor>>, nodeMap: MutableMap, I>, - changed: MutableMap, Boolean> + changed: MutableMap, Boolean>, + visitSubGraphs: Boolean = true ) { val nodes = getNodesInOrder(direction) for (node in nodes) { - if (direction == TraverseDirection.Backward && node is CFGNodeWithCfgOwner<*>) { + if (visitSubGraphs && direction == TraverseDirection.Backward && node is CFGNodeWithCfgOwner<*>) { node.subGraphs.forEach { it.collectDataForNodeInternal(direction, initialInfo, visitor, nodeMap, changed) } } val previousNodes = when (direction) { @@ -83,7 +85,7 @@ private fun , K : Any, V : Any> ControlFlowGraph.co if (hasChanged) { nodeMap[node] = newData } - if (direction == TraverseDirection.Forward && node is CFGNodeWithCfgOwner<*>) { + if (visitSubGraphs && direction == TraverseDirection.Forward && node is CFGNodeWithCfgOwner<*>) { node.subGraphs.forEach { it.collectDataForNodeInternal(direction, initialInfo, visitor, nodeMap, changed) } } } diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/CommonDeclarationCheckers.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/CommonDeclarationCheckers.kt index 771262a72a9..8047c4f520b 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/CommonDeclarationCheckers.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/CommonDeclarationCheckers.kt @@ -90,7 +90,8 @@ object CommonDeclarationCheckers : DeclarationCheckers() { FirFunInterfaceDeclarationChecker, FirNestedClassChecker, FirInlineClassDeclarationChecker, - FirOuterClassArgumentsRequiredChecker + FirOuterClassArgumentsRequiredChecker, + FirPropertyInitializationChecker, ) override val constructorCheckers: Set diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirDeclarationCheckerUtils.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirDeclarationCheckerUtils.kt index 7e130495752..77aa89bea4a 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirDeclarationCheckerUtils.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirDeclarationCheckerUtils.kt @@ -25,7 +25,6 @@ import org.jetbrains.kotlin.fir.resolve.diagnostics.ConeLocalVariableNoTypeOrIni import org.jetbrains.kotlin.fir.symbols.FirBasedSymbol import org.jetbrains.kotlin.fir.symbols.impl.* import org.jetbrains.kotlin.fir.types.FirErrorTypeRef -import org.jetbrains.kotlin.fir.types.FirImplicitTypeRef import org.jetbrains.kotlin.lexer.KtTokens internal fun isInsideExpectClass(containingClass: FirClass, context: CheckerContext): Boolean { @@ -111,8 +110,7 @@ internal fun checkPropertyInitializer( val returnTypeRef = property.returnTypeRef if (property.initializer == null && property.delegate == null && - (returnTypeRef is FirImplicitTypeRef || - (returnTypeRef is FirErrorTypeRef && returnTypeRef.diagnostic is ConeLocalVariableNoTypeOrInitializer)) + returnTypeRef is FirErrorTypeRef && returnTypeRef.diagnostic is ConeLocalVariableNoTypeOrInitializer ) { property.source?.let { reporter.reportOn(it, FirErrors.PROPERTY_WITH_NO_TYPE_NO_INITIALIZER, context) diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirPropertyInitializationChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirPropertyInitializationChecker.kt new file mode 100644 index 00000000000..4780a86aa36 --- /dev/null +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirPropertyInitializationChecker.kt @@ -0,0 +1,81 @@ +/* + * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.fir.analysis.checkers.declaration + +import org.jetbrains.kotlin.fir.analysis.cfa.TraverseDirection +import org.jetbrains.kotlin.fir.analysis.cfa.collectDataForNode +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.declarations.FirAnonymousInitializer +import org.jetbrains.kotlin.fir.declarations.FirProperty +import org.jetbrains.kotlin.fir.declarations.FirRegularClass +import org.jetbrains.kotlin.fir.expressions.FirVariableAssignment +import org.jetbrains.kotlin.fir.expressions.toResolvedCallableSymbol +import org.jetbrains.kotlin.fir.resolve.dfa.cfg.CFGNode +import org.jetbrains.kotlin.fir.resolve.dfa.cfg.ControlFlowGraphVisitor +import org.jetbrains.kotlin.fir.resolve.dfa.cfg.EdgeLabel +import org.jetbrains.kotlin.fir.resolve.dfa.cfg.VariableAssignmentNode +import org.jetbrains.kotlin.fir.resolve.dfa.controlFlowGraph +import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol +import org.jetbrains.kotlin.utils.addToStdlib.safeAs + +object FirPropertyInitializationChecker : FirRegularClassChecker() { + + override fun check(declaration: FirRegularClass, context: CheckerContext, reporter: DiagnosticReporter) { + val properties = mutableSetOf() + val toReport = mutableSetOf() + for (decl in declaration.declarations.asReversed()) { + when (decl) { + is FirProperty -> properties.add(decl.symbol) + is FirAnonymousInitializer -> + if (properties.isNotEmpty()) { + collectInitAssignments(decl, properties, toReport) + } + else -> { + } + } + } + toReport.forEach { exp -> + val propertySymbol = exp.lValue.toResolvedCallableSymbol() ?: return@forEach + reporter.reportOn(exp.lValue.source, FirErrors.INITIALIZATION_BEFORE_DECLARATION, propertySymbol, context) + } + } + + private fun collectInitAssignments( + init: FirAnonymousInitializer, + properties: Properties, + assignments: MutableSet + ) { + val graph = init.controlFlowGraphReference?.controlFlowGraph ?: return + val visitor = AssignmentVisitor(properties) { assignments.add(it) } + graph.collectDataForNode(TraverseDirection.Forward, emptySet(), visitor, visitSubGraphs = false) + } + + private class AssignmentVisitor(private val interestingProperties: Properties, private val acceptor: (FirVariableAssignment) -> Unit) : + ControlFlowGraphVisitor>>() { + + override fun visitNode(node: CFGNode<*>, data: Collection>): Properties { + if (data.isEmpty()) return emptySet() + return data.map { it.second }.reduce { l, r -> l.intersect(r) } + } + + override fun visitVariableAssignmentNode(node: VariableAssignmentNode, data: Collection>): Properties { + val input = visitNode(node, data) + val propertySymbol = node.fir.lValue.toResolvedCallableSymbol().safeAs() ?: return input + return if (propertySymbol in interestingProperties && propertySymbol !in input) { + acceptor(node.fir) + input + propertySymbol + } else { + input + } + } + } + +} + +private typealias Properties = Set \ 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 9d8fd9996b6..36aa6c048eb 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 @@ -208,6 +208,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.INCONSISTENT_TYPE import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.INCONSISTENT_TYPE_PARAMETER_VALUES import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.INFERENCE_ERROR import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.INFIX_MODIFIER_REQUIRED +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.INITIALIZATION_BEFORE_DECLARATION import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.INITIALIZER_REQUIRED_FOR_DESTRUCTURING_DECLARATION import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.INITIALIZER_TYPE_MISMATCH import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.INLINE_CLASS_CANNOT_BE_RECURSIVE @@ -1131,6 +1132,8 @@ class FirDefaultErrorMessages { map.put(VARIABLE_WITH_NO_TYPE_NO_INITIALIZER, "This variable must either have a type annotation or be initialized") + map.put(INITIALIZATION_BEFORE_DECLARATION, "Variable cannot be initialized before declaration", SYMBOL) + map.put(MUST_BE_INITIALIZED, "Property must be initialized") map.put(MUST_BE_INITIALIZED_OR_BE_ABSTRACT, "Property must be initialized or be abstract") map.put(EXTENSION_PROPERTY_MUST_HAVE_ACCESSORS_OR_BE_ABSTRACT, "Extension property must have accessors or be abstract") diff --git a/compiler/testData/diagnostics/tests/controlFlowAnalysis/assignmentInLocalsInConstructor.fir.kt b/compiler/testData/diagnostics/tests/controlFlowAnalysis/assignmentInLocalsInConstructor.fir.kt index c2888dde4c0..f20a3b350a6 100644 --- a/compiler/testData/diagnostics/tests/controlFlowAnalysis/assignmentInLocalsInConstructor.fir.kt +++ b/compiler/testData/diagnostics/tests/controlFlowAnalysis/assignmentInLocalsInConstructor.fir.kt @@ -24,7 +24,7 @@ class Test2 { } } - a = "2" + a = "2" t.some() } diff --git a/compiler/testData/diagnostics/tests/controlFlowAnalysis/kt897.fir.kt b/compiler/testData/diagnostics/tests/controlFlowAnalysis/kt897.fir.kt deleted file mode 100644 index 2f74c87b7ee..00000000000 --- a/compiler/testData/diagnostics/tests/controlFlowAnalysis/kt897.fir.kt +++ /dev/null @@ -1,20 +0,0 @@ -//KT-897 Don't allow assignment to a property before it is defined - -package kt897 - -class A() { - init { - i = 11 - } - val i : Int? = null // must be an error - - init { - j = 1 - } - var j : Int = 2 - - init { - k = 3 - } - val k : Int -} diff --git a/compiler/testData/diagnostics/tests/controlFlowAnalysis/kt897.kt b/compiler/testData/diagnostics/tests/controlFlowAnalysis/kt897.kt index eb279e36d50..0f56423a5a9 100644 --- a/compiler/testData/diagnostics/tests/controlFlowAnalysis/kt897.kt +++ b/compiler/testData/diagnostics/tests/controlFlowAnalysis/kt897.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL //KT-897 Don't allow assignment to a property before it is defined package kt897 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 924ca3204b1..b1e4a1a3b0d 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 @@ -2554,6 +2554,13 @@ internal val KT_DIAGNOSTIC_CONVERTER = KtDiagnosticConverterBuilder.buildConvert token, ) } + add(FirErrors.INITIALIZATION_BEFORE_DECLARATION) { firDiagnostic -> + InitializationBeforeDeclarationImpl( + firSymbolBuilder.buildSymbol(firDiagnostic.a.fir), + firDiagnostic as FirPsiDiagnostic, + token, + ) + } add(FirErrors.UNSAFE_CALL) { firDiagnostic -> UnsafeCallImpl( firSymbolBuilder.typeBuilder.buildKtType(firDiagnostic.a), 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 f06434b5320..40f62aae7a1 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 @@ -1790,6 +1790,11 @@ sealed class KtFirDiagnostic : KtDiagnosticWithPsi { override val diagnosticClass get() = VariableWithNoTypeNoInitializer::class } + abstract class InitializationBeforeDeclaration : KtFirDiagnostic() { + override val diagnosticClass get() = InitializationBeforeDeclaration::class + abstract val property: KtSymbol + } + abstract class UnsafeCall : KtFirDiagnostic() { override val diagnosticClass get() = UnsafeCall::class abstract val receiverType: KtType 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 6386a68d78a..eda09529934 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 @@ -2883,6 +2883,14 @@ internal class VariableWithNoTypeNoInitializerImpl( override val firDiagnostic: FirPsiDiagnostic by weakRef(firDiagnostic) } +internal class InitializationBeforeDeclarationImpl( + override val property: KtSymbol, + firDiagnostic: FirPsiDiagnostic, + override val token: ValidityToken, +) : KtFirDiagnostic.InitializationBeforeDeclaration(), KtAbstractFirDiagnostic { + override val firDiagnostic: FirPsiDiagnostic by weakRef(firDiagnostic) +} + internal class UnsafeCallImpl( override val receiverType: KtType, override val receiverExpression: KtExpression?,