FIR: move checkPropertyAccesses to FirPropertyInitializationAnalyzer

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/AbstractFirPropertyInitializationChecker.kt

@@ -13,11 +13,5 @@ import org.jetbrains.kotlin.fir.resolve.dfa.cfg.ControlFlowGraph
 import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol
 
 abstract class AbstractFirPropertyInitializationChecker {
-    abstract fun analyze(
+    abstract fun analyze(data: PropertyInitializationInfoData, reporter: DiagnosticReporter, context: CheckerContext)
-        graph: ControlFlowGraph,
-        reporter: DiagnosticReporter,
-        data: PropertyInitializationInfoData,
-        properties: Set<FirPropertySymbol>,
-        context: CheckerContext
-    )
 }

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/FirPropertyInitializationAnalyzer.kt

@@ -5,20 +5,116 @@
 
 package org.jetbrains.kotlin.fir.analysis.cfa
 
+import org.jetbrains.kotlin.contracts.description.canBeRevisited
+import org.jetbrains.kotlin.contracts.description.isDefinitelyVisited
+import org.jetbrains.kotlin.contracts.description.isInPlace
+import org.jetbrains.kotlin.descriptors.ClassKind
 import org.jetbrains.kotlin.diagnostics.DiagnosticReporter
+import org.jetbrains.kotlin.diagnostics.reportOn
 import org.jetbrains.kotlin.fir.analysis.cfa.util.PropertyInitializationInfoData
 import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
-import org.jetbrains.kotlin.fir.analysis.checkers.declaration.checkPropertyAccesses
+import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors
-import org.jetbrains.kotlin.fir.analysis.checkers.declaration.requiresInitialization
+import org.jetbrains.kotlin.fir.declarations.*
-import org.jetbrains.kotlin.fir.resolve.dfa.cfg.ControlFlowGraph
+import org.jetbrains.kotlin.fir.declarations.utils.hasBackingField
+import org.jetbrains.kotlin.fir.declarations.utils.hasExplicitBackingField
+import org.jetbrains.kotlin.fir.declarations.utils.isLateInit
+import org.jetbrains.kotlin.fir.expressions.FirQualifiedAccess
+import org.jetbrains.kotlin.fir.expressions.FirThisReceiverExpression
+import org.jetbrains.kotlin.fir.isCatchParameter
+import org.jetbrains.kotlin.fir.references.toResolvedPropertySymbol
+import org.jetbrains.kotlin.fir.resolve.dfa.cfg.*
+import org.jetbrains.kotlin.fir.symbols.SymbolInternals
 import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol
+import org.jetbrains.kotlin.fir.symbols.impl.FirSyntheticPropertySymbol
 
 object FirPropertyInitializationAnalyzer : AbstractFirPropertyInitializationChecker() {
-    override fun analyze(
+    override fun analyze(data: PropertyInitializationInfoData, reporter: DiagnosticReporter, context: CheckerContext) {
-        graph: ControlFlowGraph,
+        data.checkPropertyAccesses(context, reporter)
-        reporter: DiagnosticReporter,
+    }
-        data: PropertyInitializationInfoData,
+}
-        properties: Set<FirPropertySymbol>,
+
-        context: CheckerContext
+val FirDeclaration.evaluatedInPlace: Boolean
-    ) = graph.checkPropertyAccesses(properties.filterTo(mutableSetOf()) { it.requiresInitialization }, null, context, reporter, data)
+    get() = when (this) {
+        is FirAnonymousFunction -> invocationKind.isInPlace
+        is FirAnonymousObject -> classKind != ClassKind.ENUM_ENTRY
+        is FirConstructor -> true // child of class initialization graph
+        is FirFunction, is FirClass -> false
+        else -> true // property initializer, etc.
+    }
+
+@OptIn(SymbolInternals::class)
+val FirPropertySymbol.requiresInitialization: Boolean
+    get() = this !is FirSyntheticPropertySymbol && !hasInitializer && !hasExplicitBackingField &&
+            hasBackingField && fir.isCatchParameter != true
+
+fun PropertyInitializationInfoData.checkPropertyAccesses(context: CheckerContext, reporter: DiagnosticReporter) {
+    // If a property has an initializer (or does not need one), then any reads are OK while any writes are OK
+    // if it's a `var` and bad if it's a `val`. `FirReassignmentAndInvisibleSetterChecker` does this without a CFG.
+    val filtered = properties.filterTo(mutableSetOf()) { it.requiresInitialization }
+    if (filtered.isEmpty()) return
+
+    checkPropertyAccesses(graph, filtered, context, reporter, null, mutableMapOf())
+}
+
+@OptIn(SymbolInternals::class)
+private fun PropertyInitializationInfoData.checkPropertyAccesses(
+    graph: ControlFlowGraph,
+    properties: Set<FirPropertySymbol>,
+    context: CheckerContext,
+    reporter: DiagnosticReporter,
+    scope: FirDeclaration?,
+    scopes: MutableMap<FirPropertySymbol, FirDeclaration?>,
+) {
+    fun FirQualifiedAccess.hasCorrectReceiver() =
+        (dispatchReceiver as? FirThisReceiverExpression)?.calleeReference?.boundSymbol == receiver
+
+    for (node in graph.nodes) {
+        when {
+            // TODO: `node.isUnion` - f({ x = 1 }, { x = 2 }) - which to report?
+            //  Also this is currently indistinguishable from x = 1; f({}, {}).
+
+            node is VariableDeclarationNode -> {
+                val symbol = node.fir.symbol
+                if (scope != null && receiver == null && node.fir.isVal && symbol in properties) {
+                    // It's OK to initialize this variable from a nested called-in-place function, but not from
+                    // a non-called-in-place function or a non-anonymous-object class initializer.
+                    scopes[symbol] = scope
+                }
+            }
+
+            node is VariableAssignmentNode -> {
+                val symbol = node.fir.calleeReference.toResolvedPropertySymbol() ?: continue
+                if (!symbol.fir.isVal || !node.fir.hasCorrectReceiver() || symbol !in properties) continue
+
+                if (scope != scopes[symbol]) {
+                    val error = if (receiver != null)
+                        FirErrors.CAPTURED_MEMBER_VAL_INITIALIZATION
+                    else
+                        FirErrors.CAPTURED_VAL_INITIALIZATION
+                    reporter.reportOn(node.fir.lValue.source, error, symbol, context)
+                } else if (getValue(node).values.any { it[symbol]?.canBeRevisited() == true }) {
+                    reporter.reportOn(node.fir.lValue.source, FirErrors.VAL_REASSIGNMENT, symbol, context)
+                }
+            }
+
+            node is QualifiedAccessNode -> {
+                val symbol = node.fir.calleeReference.toResolvedPropertySymbol() ?: continue
+                if (!symbol.isLateInit && node.fir.hasCorrectReceiver() && symbol in properties &&
+                    getValue(node).values.any { it[symbol]?.isDefinitelyVisited() != true }
+                ) {
+                    reporter.reportOn(node.fir.source, FirErrors.UNINITIALIZED_VARIABLE, symbol, context)
+                }
+            }
+
+            // In the class case, subgraphs of the exit node are member functions, which are considered to not
+            // be part of initialization, so any val is considered to be initialized there and the CFG is not
+            // needed. The errors on reassignments will be emitted by `FirReassignmentAndInvisibleSetterChecker`.
+            node is CFGNodeWithSubgraphs<*> && (receiver == null || node !== graph.exitNode) -> {
+                for (subGraph in node.subGraphs) {
+                    val newScope = subGraph.declaration?.takeIf { !it.evaluatedInPlace } ?: scope
+                    checkPropertyAccesses(subGraph, properties, context, reporter, newScope, scopes)
+                }
+            }
+        }
+    }
 }

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/util/PropertyInitializationInfoCollector.kt

@@ -13,7 +13,11 @@ import org.jetbrains.kotlin.fir.resolve.dfa.cfg.*
 import org.jetbrains.kotlin.fir.symbols.FirBasedSymbol
 import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol
 
-class PropertyInitializationInfoData(properties: Set<FirPropertySymbol>, receiver: FirBasedSymbol<*>?, graph: ControlFlowGraph) {
+class PropertyInitializationInfoData(
+    val properties: Set<FirPropertySymbol>,
+    val receiver: FirBasedSymbol<*>?,
+    val graph: ControlFlowGraph,
+) {
     private val data by lazy(LazyThreadSafetyMode.NONE) {
         graph.collectDataForNode(TraverseDirection.Forward, PropertyInitializationInfoCollector(properties, receiver))
     }

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertiesChecker.kt

@@ -6,10 +6,7 @@
 package org.jetbrains.kotlin.fir.analysis.checkers.declaration
 
 import org.jetbrains.kotlin.KtFakeSourceElementKind
-import org.jetbrains.kotlin.contracts.description.canBeRevisited
 import org.jetbrains.kotlin.contracts.description.isDefinitelyVisited
-import org.jetbrains.kotlin.contracts.description.isInPlace
-import org.jetbrains.kotlin.descriptors.ClassKind
 import org.jetbrains.kotlin.descriptors.Visibilities
 import org.jetbrains.kotlin.fir.analysis.cfa.util.PropertyInitializationInfo
 import org.jetbrains.kotlin.fir.analysis.checkers.contains
@@ -18,20 +15,15 @@ import org.jetbrains.kotlin.fir.analysis.checkers.getModifierList
 import org.jetbrains.kotlin.diagnostics.DiagnosticReporter
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors
 import org.jetbrains.kotlin.diagnostics.reportOn
+import org.jetbrains.kotlin.fir.analysis.cfa.checkPropertyAccesses
+import org.jetbrains.kotlin.fir.analysis.cfa.requiresInitialization
 import org.jetbrains.kotlin.fir.analysis.cfa.util.PropertyInitializationInfoData
 import org.jetbrains.kotlin.fir.declarations.*
 import org.jetbrains.kotlin.fir.declarations.impl.FirDefaultPropertyAccessor
 import org.jetbrains.kotlin.fir.declarations.utils.*
-import org.jetbrains.kotlin.fir.expressions.FirQualifiedAccess
+import org.jetbrains.kotlin.fir.resolve.dfa.cfg.NormalPath
-import org.jetbrains.kotlin.fir.expressions.FirThisReceiverExpression
-import org.jetbrains.kotlin.fir.isCatchParameter
-import org.jetbrains.kotlin.fir.references.toResolvedPropertySymbol
-import org.jetbrains.kotlin.fir.resolve.dfa.cfg.*
 import org.jetbrains.kotlin.fir.resolve.dfa.controlFlowGraph
-import org.jetbrains.kotlin.fir.symbols.FirBasedSymbol
-import org.jetbrains.kotlin.fir.symbols.SymbolInternals
 import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol
-import org.jetbrains.kotlin.fir.symbols.impl.FirSyntheticPropertySymbol
 import org.jetbrains.kotlin.lexer.KtTokens
 
 // See old FE's [DeclarationsChecker]
@@ -61,7 +53,7 @@ object FirMemberPropertiesChecker : FirClassChecker() {
         // TODO: this also visits non-constructor member functions...
         // TODO: merge with `FirPropertyInitializationAnalyzer` for fewer passes.
         val data = PropertyInitializationInfoData(memberPropertySymbols, symbol, graph)
-        graph.checkPropertyAccesses(memberPropertySymbols, symbol, context, reporter, data)
+        data.checkPropertyAccesses(context, reporter)
         return data.getValue(graph.exitNode)[NormalPath]
     }
 
@@ -136,96 +128,3 @@ object FirMemberPropertiesChecker : FirClassChecker() {
         }
     }
 }
-
-val FirDeclaration.evaluatedInPlace: Boolean
-    get() = when (this) {
-        is FirAnonymousFunction -> invocationKind.isInPlace
-        is FirAnonymousObject -> classKind != ClassKind.ENUM_ENTRY
-        is FirConstructor -> true // child of class initialization graph
-        is FirFunction, is FirClass -> false
-        else -> true // property initializer, etc.
-    }
-
-@OptIn(SymbolInternals::class)
-val FirPropertySymbol.requiresInitialization: Boolean
-    get() = this !is FirSyntheticPropertySymbol && !hasInitializer && !hasExplicitBackingField &&
-            hasBackingField && fir.isCatchParameter != true
-
-fun ControlFlowGraph.checkPropertyAccesses(
-    properties: Set<FirPropertySymbol>,
-    receiver: FirBasedSymbol<*>?,
-    context: CheckerContext,
-    reporter: DiagnosticReporter,
-    data: PropertyInitializationInfoData
-) {
-    // NOTE: assert(properties.all { it.requiresInitialization })
-    // If a property has an initializer (or does not need one), then any reads are OK while any writes are OK
-    // if it's a `var` and bad if it's a `val`. `FirReassignmentAndInvisibleSetterChecker` does this without a CFG.
-    if (properties.isEmpty()) return
-
-    checkPropertyAccesses(properties, receiver, context, reporter, data, null, mutableMapOf())
-}
-
-@OptIn(SymbolInternals::class)
-private fun ControlFlowGraph.checkPropertyAccesses(
-    properties: Set<FirPropertySymbol>,
-    receiver: FirBasedSymbol<*>?,
-    context: CheckerContext,
-    reporter: DiagnosticReporter,
-    data: PropertyInitializationInfoData,
-    scope: FirDeclaration?,
-    scopes: MutableMap<FirPropertySymbol, FirDeclaration?>,
-) {
-    fun FirQualifiedAccess.hasCorrectReceiver() =
-        (dispatchReceiver as? FirThisReceiverExpression)?.calleeReference?.boundSymbol == receiver
-
-    for (node in nodes) {
-        when {
-            // TODO: `node.isUnion` - f({ x = 1 }, { x = 2 }) - which to report?
-            //  Also this is currently indistinguishable from x = 1; f({}, {}).
-
-            node is VariableDeclarationNode -> {
-                val symbol = node.fir.symbol
-                if (scope != null && receiver == null && node.fir.isVal && symbol in properties) {
-                    // It's OK to initialize this variable from a nested called-in-place function, but not from
-                    // a non-called-in-place function or a non-anonymous-object class initializer.
-                    scopes[symbol] = scope
-                }
-            }
-
-            node is VariableAssignmentNode -> {
-                val symbol = node.fir.calleeReference.toResolvedPropertySymbol() ?: continue
-                if (!symbol.fir.isVal || !node.fir.hasCorrectReceiver() || symbol !in properties) continue
-
-                if (scope != scopes[symbol]) {
-                    val error = if (receiver != null)
-                        FirErrors.CAPTURED_MEMBER_VAL_INITIALIZATION
-                    else
-                        FirErrors.CAPTURED_VAL_INITIALIZATION
-                    reporter.reportOn(node.fir.lValue.source, error, symbol, context)
-                } else if (data.getValue(node).values.any { it[symbol]?.canBeRevisited() == true }) {
-                    reporter.reportOn(node.fir.lValue.source, FirErrors.VAL_REASSIGNMENT, symbol, context)
-                }
-            }
-
-            node is QualifiedAccessNode -> {
-                val symbol = node.fir.calleeReference.toResolvedPropertySymbol() ?: continue
-                if (!symbol.isLateInit && node.fir.hasCorrectReceiver() && symbol in properties &&
-                    data.getValue(node).values.any { it[symbol]?.isDefinitelyVisited() != true }
-                ) {
-                    reporter.reportOn(node.fir.source, FirErrors.UNINITIALIZED_VARIABLE, symbol, context)
-                }
-            }
-
-            // In the class case, subgraphs of the exit node are member functions, which are considered to not
-            // be part of initialization, so any val is considered to be initialized there and the CFG is not
-            // needed. The errors on reassignments will be emitted by `FirReassignmentAndInvisibleSetterChecker`.
-            node is CFGNodeWithSubgraphs<*> && (receiver == null || node !== exitNode) -> {
-                for (subGraph in node.subGraphs) {
-                    val newScope = subGraph.declaration?.takeIf { !it.evaluatedInPlace } ?: scope
-                    subGraph.checkPropertyAccesses(properties, receiver, context, reporter, data, newScope, scopes)
-                }
-            }
-        }
-    }
-}

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirReassignmentAndInvisibleSetterChecker.kt

@@ -8,10 +8,10 @@ package org.jetbrains.kotlin.fir.analysis.checkers.expression
 import org.jetbrains.kotlin.descriptors.Visibilities
 import org.jetbrains.kotlin.diagnostics.DiagnosticReporter
 import org.jetbrains.kotlin.diagnostics.reportOn
+import org.jetbrains.kotlin.fir.analysis.cfa.evaluatedInPlace
+import org.jetbrains.kotlin.fir.analysis.cfa.requiresInitialization
 import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
 import org.jetbrains.kotlin.fir.analysis.checkers.context.findClosest
-import org.jetbrains.kotlin.fir.analysis.checkers.declaration.evaluatedInPlace
-import org.jetbrains.kotlin.fir.analysis.checkers.declaration.requiresInitialization
 import org.jetbrains.kotlin.fir.analysis.checkers.toRegularClassSymbol
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors
 import org.jetbrains.kotlin.fir.declarations.*

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/CanBeValChecker.kt

@@ -21,18 +21,12 @@ import org.jetbrains.kotlin.fir.resolve.dfa.cfg.*
 import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol
 
 object CanBeValChecker : AbstractFirPropertyInitializationChecker() {
-    override fun analyze(
+    override fun analyze(data: PropertyInitializationInfoData, reporter: DiagnosticReporter, context: CheckerContext) {
-        graph: ControlFlowGraph,
-        reporter: DiagnosticReporter,
-        data: PropertyInitializationInfoData,
-        properties: Set<FirPropertySymbol>,
-        context: CheckerContext
-    ) {
         val unprocessedProperties = mutableSetOf<FirPropertySymbol>()
         val propertiesCharacteristics = mutableMapOf<FirPropertySymbol, EventOccurrencesRange>()
 
-        val reporterVisitor = UninitializedPropertyReporter(data, properties, unprocessedProperties, propertiesCharacteristics)
+        val reporterVisitor = UninitializedPropertyReporter(data, unprocessedProperties, propertiesCharacteristics)
-        graph.traverse(reporterVisitor)
+        data.graph.traverse(reporterVisitor)
 
         for (property in unprocessedProperties) {
             val source = property.source
@@ -74,7 +68,6 @@ object CanBeValChecker : AbstractFirPropertyInitializationChecker() {
 
     private class UninitializedPropertyReporter(
         val data: PropertyInitializationInfoData,
-        val localProperties: Set<FirPropertySymbol>,
         val unprocessedProperties: MutableSet<FirPropertySymbol>,
         val propertiesCharacteristics: MutableMap<FirPropertySymbol, EventOccurrencesRange>
     ) : ControlFlowGraphVisitorVoid() {
@@ -82,7 +75,7 @@ object CanBeValChecker : AbstractFirPropertyInitializationChecker() {
 
         override fun visitVariableAssignmentNode(node: VariableAssignmentNode) {
             val symbol = node.fir.calleeReference.toResolvedPropertySymbol() ?: return
-            if (symbol !in localProperties) return
+            if (symbol !in data.properties) return
             unprocessedProperties.remove(symbol)
 
             val currentCharacteristic = propertiesCharacteristics.getOrDefault(symbol, EventOccurrencesRange.ZERO)

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/UnusedChecker.kt

@@ -31,15 +31,9 @@ import org.jetbrains.kotlin.fir.types.coneType
 import org.jetbrains.kotlin.fir.types.isFunctionalType
 
 object UnusedChecker : AbstractFirPropertyInitializationChecker() {
-    override fun analyze(
+    override fun analyze(data: PropertyInitializationInfoData, reporter: DiagnosticReporter, context: CheckerContext) {
-        graph: ControlFlowGraph,
+        val ownData = ValueWritesWithoutReading(context.session, data.properties).getData(data.graph)
-        reporter: DiagnosticReporter,
+        data.graph.traverse(CfaVisitor(ownData, reporter, context))
-        data: PropertyInitializationInfoData,
-        properties: Set<FirPropertySymbol>,
-        context: CheckerContext
-    ) {
-        val ownData = ValueWritesWithoutReading(context.session, properties).getData(graph)
-        graph.traverse(CfaVisitor(ownData, reporter, context))
     }
 
     class CfaVisitor(

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/collectors/components/ControlFlowAnalysisDiagnosticComponent.kt

@@ -34,7 +34,7 @@ class ControlFlowAnalysisDiagnosticComponent(
         val properties = mutableSetOf<FirPropertySymbol>().apply { graph.traverse(LocalPropertyCollector(this)) }
         if (properties.isNotEmpty()) {
             val data = PropertyInitializationInfoData(properties, receiver = null, graph)
-            variableAssignmentCheckers.forEach { it.analyze(graph, reporter, data, properties, context) }
+            variableAssignmentCheckers.forEach { it.analyze(data, reporter, context) }
         }
     }