[FIR] Check top-level property initialization via CFG

Now that files have a CFG, use it to validate properties are initialized
correctly. Update FirTopLevelPropertiesChecker to collect initialization
info for the property being checked - similar to
FirMemberPropertiesChecker - and validate proper initialization.

#KT-56683 Fixed
#KT-58531 Fixed
This commit is contained in:
Brian Norman
2023-07-24 15:34:31 -05:00
committed by Space Team
parent b55fda0c55
commit 997e062de9
14 changed files with 202 additions and 54 deletions
@@ -3,4 +3,7 @@ fun foo(): String
class A {
abstract fun foo()
}
class B: A
class B: A
// IGNORE_FIR
// Does not pass because of KT-61296
@@ -3242,6 +3242,12 @@ public class DiagnosticCompilerTestFE10TestdataTestGenerated extends AbstractDia
runTest("compiler/testData/diagnostics/tests/backingField/SetterWithExplicitType.kt");
}
@Test
@TestMetadata("TopLevelPropertyInitialization.kt")
public void testTopLevelPropertyInitialization() throws Exception {
runTest("compiler/testData/diagnostics/tests/backingField/TopLevelPropertyInitialization.kt");
}
@Test
@TestMetadata("ValDeferredInitInFinalClass.kt")
public void testValDeferredInitInFinalClass() throws Exception {
@@ -3242,6 +3242,12 @@ public class LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated
runTest("compiler/testData/diagnostics/tests/backingField/SetterWithExplicitType.kt");
}
@Test
@TestMetadata("TopLevelPropertyInitialization.kt")
public void testTopLevelPropertyInitialization() throws Exception {
runTest("compiler/testData/diagnostics/tests/backingField/TopLevelPropertyInitialization.kt");
}
@Test
@TestMetadata("ValDeferredInitInFinalClass.kt")
public void testValDeferredInitInFinalClass() throws Exception {
@@ -3242,6 +3242,12 @@ public class FirLightTreeOldFrontendDiagnosticsTestGenerated extends AbstractFir
runTest("compiler/testData/diagnostics/tests/backingField/SetterWithExplicitType.kt");
}
@Test
@TestMetadata("TopLevelPropertyInitialization.kt")
public void testTopLevelPropertyInitialization() throws Exception {
runTest("compiler/testData/diagnostics/tests/backingField/TopLevelPropertyInitialization.kt");
}
@Test
@TestMetadata("ValDeferredInitInFinalClass.kt")
public void testValDeferredInitInFinalClass() throws Exception {
@@ -3248,6 +3248,12 @@ public class FirPsiOldFrontendDiagnosticsTestGenerated extends AbstractFirPsiDia
runTest("compiler/testData/diagnostics/tests/backingField/SetterWithExplicitType.kt");
}
@Test
@TestMetadata("TopLevelPropertyInitialization.kt")
public void testTopLevelPropertyInitialization() throws Exception {
runTest("compiler/testData/diagnostics/tests/backingField/TopLevelPropertyInitialization.kt");
}
@Test
@TestMetadata("ValDeferredInitInFinalClass.kt")
public void testValDeferredInitInFinalClass() throws Exception {
@@ -17,6 +17,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors
import org.jetbrains.kotlin.fir.declarations.*
import org.jetbrains.kotlin.fir.declarations.utils.hasBackingField
import org.jetbrains.kotlin.fir.declarations.utils.hasExplicitBackingField
import org.jetbrains.kotlin.fir.declarations.utils.isExternal
import org.jetbrains.kotlin.fir.declarations.utils.isLateInit
import org.jetbrains.kotlin.fir.expressions.*
import org.jetbrains.kotlin.fir.isCatchParameter
@@ -126,7 +127,7 @@ private fun PropertyInitializationInfoData.checkPropertyAccesses(
node is QualifiedAccessNode -> {
if (doNotReportUninitializedVariable) continue
val symbol = node.fir.calleeReference.toResolvedPropertySymbol() ?: continue
if (!symbol.isLateInit && node.fir.hasCorrectReceiver() && symbol in properties &&
if (!symbol.isLateInit && !symbol.isExternal && node.fir.hasCorrectReceiver() && symbol in properties &&
getValue(node).values.any { it[symbol]?.isDefinitelyVisited() != true }
) {
reporter.reportOn(node.fir.source, FirErrors.UNINITIALIZED_VARIABLE, symbol, context)
@@ -79,7 +79,6 @@ object CommonDeclarationCheckers : DeclarationCheckers() {
FirPropertyFromParameterChecker,
FirLocalVariableTypeParametersSyntaxChecker,
FirDelegateUsesExtensionPropertyTypeParameterChecker,
FirTopLevelPropertiesChecker,
FirLocalExtensionPropertyChecker,
ContractSyntaxV2PropertyChecker,
FirVolatileAnnotationChecker,
@@ -144,6 +143,7 @@ object CommonDeclarationCheckers : DeclarationCheckers() {
get() = setOf(
FirImportsChecker,
FirUnresolvedInMiddleOfImportChecker,
FirTopLevelPropertiesChecker,
)
override val controlFlowAnalyserCheckers: Set<FirControlFlowChecker>
@@ -27,6 +27,7 @@ import org.jetbrains.kotlin.fir.expressions.impl.FirEmptyExpressionBlock
import org.jetbrains.kotlin.fir.references.FirSuperReference
import org.jetbrains.kotlin.fir.references.toResolvedCallableSymbol
import org.jetbrains.kotlin.fir.resolve.*
import org.jetbrains.kotlin.fir.resolve.providers.firProvider
import org.jetbrains.kotlin.fir.resolve.providers.symbolProvider
import org.jetbrains.kotlin.fir.scopes.*
import org.jetbrains.kotlin.fir.scopes.impl.declaredMemberScope
@@ -151,6 +152,14 @@ fun FirBasedSymbol<*>.getContainingClassSymbol(session: FirSession): FirClassLik
else -> null
}
/**
* Returns the containing class or file if the callable is top-level.
*/
fun FirCallableSymbol<*>.getContainingSymbol(session: FirSession): FirBasedSymbol<*>? {
return getContainingClassSymbol(session)
?: session.firProvider.getFirCallableContainerFile(this)?.symbol
}
fun FirDeclaration.getContainingClassSymbol(session: FirSession) = symbol.getContainingClassSymbol(session)
fun FirClassLikeSymbol<*>.outerClassSymbol(context: CheckerContext): FirClassLikeSymbol<*>? {
@@ -55,31 +55,33 @@ object FirMemberPropertiesChecker : FirClassChecker() {
data.checkPropertyAccesses(isForClassInitialization = true, context, reporter)
return data.getValue(graph.exitNode)[NormalPath]
}
}
private fun checkProperty(
containingDeclaration: FirClass,
property: FirProperty,
isDefinitelyAssignedInConstructor: Boolean,
context: CheckerContext,
reporter: DiagnosticReporter,
reachable: Boolean
) {
val source = property.source ?: return
if (source.kind is KtFakeSourceElementKind) return
// If multiple (potentially conflicting) modality modifiers are specified, not all modifiers are recorded at `status`.
// So, our source of truth should be the full modifier list retrieved from the source.
val modifierList = property.source.getModifierList()
internal fun checkProperty(
containingDeclaration: FirClass?,
property: FirProperty,
isDefinitelyAssigned: Boolean,
context: CheckerContext,
reporter: DiagnosticReporter,
reachable: Boolean,
) {
val source = property.source ?: return
if (source.kind is KtFakeSourceElementKind) return
// If multiple (potentially conflicting) modality modifiers are specified, not all modifiers are recorded at `status`.
// So, our source of truth should be the full modifier list retrieved from the source.
val modifierList = property.source.getModifierList()
checkPropertyInitializer(
containingDeclaration,
property,
modifierList,
isDefinitelyAssignedInConstructor,
reporter,
context,
reachable
)
checkPropertyInitializer(
containingDeclaration,
property,
modifierList,
isDefinitelyAssigned,
reporter,
context,
reachable
)
if (containingDeclaration != null) {
val hasAbstractModifier = KtTokens.ABSTRACT_KEYWORD in modifierList
val isAbstract = property.isAbstract || hasAbstractModifier
if (containingDeclaration.isInterface &&
@@ -5,46 +5,67 @@
package org.jetbrains.kotlin.fir.analysis.checkers.declaration
import org.jetbrains.kotlin.KtFakeSourceElementKind
import org.jetbrains.kotlin.KtSourceElement
import org.jetbrains.kotlin.config.LanguageFeature
import org.jetbrains.kotlin.config.LanguageVersionSettings
import org.jetbrains.kotlin.contracts.description.isDefinitelyVisited
import org.jetbrains.kotlin.descriptors.Modality
import org.jetbrains.kotlin.diagnostics.DiagnosticReporter
import org.jetbrains.kotlin.diagnostics.KtDiagnosticFactory0
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.PropertyInitializationInfo
import org.jetbrains.kotlin.fir.analysis.cfa.util.PropertyInitializationInfoData
import org.jetbrains.kotlin.fir.analysis.checkers.FirModifierList
import org.jetbrains.kotlin.fir.analysis.checkers.contains
import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
import org.jetbrains.kotlin.fir.analysis.checkers.getModifierList
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors
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.resolve.dfa.cfg.NormalPath
import org.jetbrains.kotlin.fir.resolve.dfa.controlFlowGraph
import org.jetbrains.kotlin.fir.resolve.diagnostics.ConeLocalVariableNoTypeOrInitializer
import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol
import org.jetbrains.kotlin.fir.types.FirErrorTypeRef
import org.jetbrains.kotlin.lexer.KtTokens
// See old FE's [DeclarationsChecker]
object FirTopLevelPropertiesChecker : FirPropertyChecker() {
override fun check(declaration: FirProperty, context: CheckerContext, reporter: DiagnosticReporter) {
// Only report on top level callable declarations
if (context.containingDeclarations.size > 1) return
object FirTopLevelPropertiesChecker : FirFileChecker() {
override fun check(declaration: FirFile, context: CheckerContext, reporter: DiagnosticReporter) {
val info = declaration.collectionInitializationInfo(context, reporter)
for (innerDeclaration in declaration.declarations) {
if (innerDeclaration is FirProperty) {
val symbol = innerDeclaration.symbol
val isDefinitelyAssigned = info?.get(symbol)?.isDefinitelyVisited() == true
checkProperty(containingDeclaration = null, innerDeclaration, isDefinitelyAssigned, context, reporter, reachable = true)
}
}
}
val source = declaration.source ?: return
if (source.kind is KtFakeSourceElementKind) return
// If multiple (potentially conflicting) modality modifiers are specified, not all modifiers are recorded at `status`.
// So, our source of truth should be the full modifier list retrieved from the source.
val modifierList = source.getModifierList()
private fun FirFile.collectionInitializationInfo(
context: CheckerContext,
reporter: DiagnosticReporter,
): PropertyInitializationInfo? {
val graph = (this as? FirControlFlowGraphOwner)?.controlFlowGraphReference?.controlFlowGraph ?: return null
checkPropertyInitializer(
containingClass = null,
declaration,
modifierList,
isDefinitelyAssignedInConstructor = false, // Only member properties can be assigned in constructors
reporter,
context
)
// Scripts are nested as a single declaration under FirFiles and contain their own statements. To properly check all "top-level"
// properties, script statements need to be unwrapped.
val topLevelProperties = when (val script = declarations.singleOrNull()) {
is FirScript -> script.statements.filterIsInstance<FirProperty>()
else -> declarations.filterIsInstance<FirProperty>()
}
val propertySymbols = topLevelProperties.mapNotNullTo(mutableSetOf()) { declaration ->
(declaration.symbol as? FirPropertySymbol)?.takeIf { it.requiresInitialization(false) }
}
if (propertySymbols.isEmpty()) return null
// TODO, KT-59803: merge with `FirPropertyInitializationAnalyzer` for fewer passes.
val data = PropertyInitializationInfoData(propertySymbols, receiver = null, graph)
data.checkPropertyAccesses(isForClassInitialization = false, context, reporter)
return data.getValue(graph.exitNode)[NormalPath]
}
}
@@ -53,10 +74,10 @@ internal fun checkPropertyInitializer(
containingClass: FirClass?,
property: FirProperty,
modifierList: FirModifierList?,
isDefinitelyAssignedInConstructor: Boolean,
isDefinitelyAssigned: Boolean,
reporter: DiagnosticReporter,
context: CheckerContext,
reachable: Boolean = true
reachable: Boolean = true,
) {
val inInterface = containingClass?.isInterface == true
val hasAbstractModifier = KtTokens.ABSTRACT_KEYWORD in modifierList
@@ -118,7 +139,7 @@ internal fun checkPropertyInitializer(
val propertySource = property.source ?: return
val isExternal = property.isEffectivelyExternal(containingClass, context)
val isCorrectlyInitialized =
property.initializer != null || isDefinitelyAssignedInConstructor && !property.hasSetterAccessorImplementation &&
property.initializer != null || isDefinitelyAssigned && !property.hasSetterAccessorImplementation &&
property.getEffectiveModality(containingClass, context.languageVersionSettings) != Modality.OPEN
if (
backingFieldRequired &&
@@ -135,14 +156,14 @@ internal fun checkPropertyInitializer(
val isOpenValDeferredInitDeprecationWarning =
!context.languageVersionSettings.supportsFeature(LanguageFeature.ProhibitOpenValDeferredInitialization) &&
property.getEffectiveModality(containingClass, context.languageVersionSettings) == Modality.OPEN && property.isVal &&
isDefinitelyAssignedInConstructor
isDefinitelyAssigned
// KT-61228
val isFalsePositiveDeferredInitDeprecationWarning = isOpenValDeferredInitDeprecationWarning &&
property.getEffectiveModality(containingClass) == Modality.FINAL
if (!isFalsePositiveDeferredInitDeprecationWarning) {
reportMustBeInitialized(
property,
isDefinitelyAssignedInConstructor,
isDefinitelyAssigned,
containingClass,
propertySource,
isOpenValDeferredInitDeprecationWarning,
@@ -169,7 +190,7 @@ internal fun checkPropertyInitializer(
private fun reportMustBeInitialized(
property: FirProperty,
isDefinitelyAssignedInConstructor: Boolean,
isDefinitelyAssigned: Boolean,
containingClass: FirClass?,
propertySource: KtSourceElement,
isOpenValDeferredInitDeprecationWarning: Boolean,
@@ -180,7 +201,7 @@ private fun reportMustBeInitialized(
val suggestMakingItFinal = containingClass != null &&
!property.hasSetterAccessorImplementation &&
property.getEffectiveModality(containingClass, context.languageVersionSettings) != Modality.FINAL &&
isDefinitelyAssignedInConstructor
isDefinitelyAssigned
val suggestMakingItAbstract = containingClass != null && !property.hasAnyAccessorImplementation
if (isOpenValDeferredInitDeprecationWarning && !suggestMakingItFinal && suggestMakingItAbstract) {
error("Not reachable case. Every \"open val + deferred init\" case that could be made `abstract`, also could be made `final`")
@@ -189,7 +210,7 @@ private fun reportMustBeInitialized(
!context.languageVersionSettings.supportsFeature(LanguageFeature.ProhibitMissedMustBeInitializedWhenThereIsNoPrimaryConstructor) &&
containingClass != null &&
containingClass.primaryConstructorIfAny(context.session) == null &&
isDefinitelyAssignedInConstructor
isDefinitelyAssigned
val factory = when {
suggestMakingItFinal && suggestMakingItAbstract -> FirErrors.MUST_BE_INITIALIZED_OR_FINAL_OR_ABSTRACT
suggestMakingItFinal -> FirErrors.MUST_BE_INITIALIZED_OR_BE_FINAL
@@ -12,6 +12,7 @@ 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.getContainingSymbol
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors
import org.jetbrains.kotlin.fir.declarations.*
import org.jetbrains.kotlin.fir.declarations.utils.visibility
@@ -20,6 +21,7 @@ import org.jetbrains.kotlin.fir.diagnostics.DiagnosticKind
import org.jetbrains.kotlin.fir.expressions.*
import org.jetbrains.kotlin.fir.originalForSubstitutionOverride
import org.jetbrains.kotlin.fir.references.*
import org.jetbrains.kotlin.fir.resolve.dfa.controlFlowGraph
import org.jetbrains.kotlin.fir.resolve.diagnostics.ConeDiagnosticWithCandidates
import org.jetbrains.kotlin.fir.resolve.diagnostics.ConeUnresolvedNameError
import org.jetbrains.kotlin.fir.symbols.SymbolInternals
@@ -137,8 +139,12 @@ object FirReassignmentAndInvisibleSetterChecker : FirVariableAssignmentChecker()
val property = expression.calleeReference?.toResolvedPropertySymbol() ?: return
if (property.isVar) return
// Assignments of uninitialized `val`s must be checked via CFG, since the first one is OK.
// See `FirPropertyInitializationAnalyzer` for locals and `FirMemberPropertiesChecker` for backing fields in initializers.
if (property.isLocal && property.requiresInitialization(isForClassInitialization = false)) return
// See `FirPropertyInitializationAnalyzer` for locals, `FirMemberPropertiesChecker` for backing fields in initializers,
// and `FirTopLevelPropertiesChecker` for top-level properties.
if (
(property.isLocal || isInFileGraph(property, context))
&& property.requiresInitialization(isForClassInitialization = false)
) return
if (
isInOwnersInitializer(expression.dispatchReceiver.unwrapSmartcastExpression(), context)
&& property.requiresInitialization(isForClassInitialization = true)
@@ -147,6 +153,20 @@ object FirReassignmentAndInvisibleSetterChecker : FirVariableAssignmentChecker()
reporter.reportOn(expression.lValue.source, FirErrors.VAL_REASSIGNMENT, property, context)
}
private fun isInFileGraph(property: FirPropertySymbol, context: CheckerContext): Boolean {
val declarations = context.containingDeclarations.dropWhile { it !is FirFile }
val file = declarations.firstOrNull() as? FirFile ?: return false
if (file.symbol != property.getContainingSymbol(context.session)) return false
// Starting with the CFG for the containing FirFile, check if all following declarations are contained as sub-CFGs.
// If there is a break in the chain, then the variable assignment is not part of the file CFG, and VAL_REASSIGNMENT should be
// reported by this checker.
val containingGraph = declarations
.map { (it as? FirControlFlowGraphOwner)?.controlFlowGraphReference?.controlFlowGraph }
.reduceOrNull { acc, graph -> graph?.takeIf { acc != null && it in acc.subGraphs } }
return containingGraph != null
}
private fun isInOwnersInitializer(receiver: FirExpression, context: CheckerContext): Boolean {
val uninitializedThisSymbol = (receiver as? FirThisReceiverExpression)?.calleeReference?.boundSymbol ?: return false
val containingDeclarations = context.containingDeclarations
@@ -0,0 +1,31 @@
// !DIAGNOSTICS: -UNUSED_PARAMETER -UNUSED_VARIABLE -ASSIGNED_BUT_NEVER_ACCESSED_VARIABLE
// WITH_STDLIB
val a: Int
val b = 1.also { a = 2 }
<!MUST_BE_INITIALIZED!>val c: Int<!>
val d by lazy { <!CAPTURED_VAL_INITIALIZATION!>c<!> = 2; 1 }
val e: Int
get() {
<!VAL_REASSIGNMENT!>c<!> = 3
return c
}
class Class {
val i: Int
val j = 1.also { i = 2 }
<!MUST_BE_INITIALIZED_OR_BE_ABSTRACT!>val k: Int<!>
val l by lazy { <!CAPTURED_MEMBER_VAL_INITIALIZATION!>k<!> = 2; 1 }
val n: Int
get() {
<!VAL_REASSIGNMENT!>k<!> = 3
return k
}
}
fun main() {
val x: Int
val y = 1.also { x = 2 }
val z: Int
val w by lazy { <!CAPTURED_VAL_INITIALIZATION!>z<!> = 2; 1 }
}
@@ -0,0 +1,31 @@
// !DIAGNOSTICS: -UNUSED_PARAMETER -UNUSED_VARIABLE -ASSIGNED_BUT_NEVER_ACCESSED_VARIABLE
// WITH_STDLIB
val a: Int
val b = 1.also { a = 2 }
<!MUST_BE_INITIALIZED!>val c: Int<!>
val d by lazy { c = 2; 1 }
val e: Int
get() {
<!VAL_REASSIGNMENT!>c<!> = 3
return c
}
class Class {
val i: Int
val j = 1.also { i = 2 }
<!MUST_BE_INITIALIZED_OR_BE_ABSTRACT!>val k: Int<!>
val l by lazy { <!CAPTURED_MEMBER_VAL_INITIALIZATION!>k<!> = 2; 1 }
val n: Int
get() {
<!VAL_REASSIGNMENT!>k<!> = 3
return k
}
}
fun main() {
val x: Int
val y = 1.also { x = 2 }
val z: Int
val w by lazy { <!CAPTURED_VAL_INITIALIZATION!>z<!> = 2; 1 }
}
@@ -3248,6 +3248,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest {
runTest("compiler/testData/diagnostics/tests/backingField/SetterWithExplicitType.kt");
}
@Test
@TestMetadata("TopLevelPropertyInitialization.kt")
public void testTopLevelPropertyInitialization() throws Exception {
runTest("compiler/testData/diagnostics/tests/backingField/TopLevelPropertyInitialization.kt");
}
@Test
@TestMetadata("ValDeferredInitInFinalClass.kt")
public void testValDeferredInitInFinalClass() throws Exception {