From 997e062de9be792e26c8003eed459b1026f376bb Mon Sep 17 00:00:00 2001 From: Brian Norman Date: Mon, 24 Jul 2023 15:34:31 -0500 Subject: [PATCH] [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 --- .../diagnostics/declarationErrors.kt | 5 +- ...CompilerTestFE10TestdataTestGenerated.java | 6 ++ ...sticCompilerFE10TestDataTestGenerated.java | 6 ++ ...eeOldFrontendDiagnosticsTestGenerated.java | 6 ++ ...siOldFrontendDiagnosticsTestGenerated.java | 6 ++ .../cfa/FirPropertyInitializationAnalyzer.kt | 3 +- .../checkers/CommonDeclarationCheckers.kt | 2 +- .../fir/analysis/checkers/FirHelpers.kt | 9 +++ .../declaration/FirMemberPropertiesChecker.kt | 46 ++++++------ .../FirTopLevelPropertiesChecker.kt | 75 ++++++++++++------- ...irReassignmentAndInvisibleSetterChecker.kt | 24 +++++- .../TopLevelPropertyInitialization.fir.kt | 31 ++++++++ .../TopLevelPropertyInitialization.kt | 31 ++++++++ .../test/runners/DiagnosticTestGenerated.java | 6 ++ 14 files changed, 202 insertions(+), 54 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/backingField/TopLevelPropertyInitialization.fir.kt create mode 100644 compiler/testData/diagnostics/tests/backingField/TopLevelPropertyInitialization.kt diff --git a/analysis/analysis-api/testData/components/diagnosticsProvider/diagnostics/declarationErrors.kt b/analysis/analysis-api/testData/components/diagnosticsProvider/diagnostics/declarationErrors.kt index eb1c7e4e1b2..a622ad7bc19 100644 --- a/analysis/analysis-api/testData/components/diagnosticsProvider/diagnostics/declarationErrors.kt +++ b/analysis/analysis-api/testData/components/diagnosticsProvider/diagnostics/declarationErrors.kt @@ -3,4 +3,7 @@ fun foo(): String class A { abstract fun foo() } -class B: A \ No newline at end of file +class B: A + +// IGNORE_FIR +// Does not pass because of KT-61296 diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java index dbf9dfdfdde..5b946bae0c0 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java @@ -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 { diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java index 2e1d70c37a5..d35067d2eea 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java @@ -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 { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java index ee6d7660ecc..204f91ab88e 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java @@ -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 { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java index c2d5b764ea2..c4c2bfb2df8 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java @@ -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 { diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/FirPropertyInitializationAnalyzer.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/FirPropertyInitializationAnalyzer.kt index 9bb18ccb93f..6b7d7958cee 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/FirPropertyInitializationAnalyzer.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/FirPropertyInitializationAnalyzer.kt @@ -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) 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 0e70ce53b2b..60516cc6fcb 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 @@ -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 diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirHelpers.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirHelpers.kt index 5a829ed82bf..84e2e49053b 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirHelpers.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirHelpers.kt @@ -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<*>? { diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertiesChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertiesChecker.kt index eb46fe51562..934182e16da 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertiesChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertiesChecker.kt @@ -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 && diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertiesChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertiesChecker.kt index 9c72dcfb72c..63fc2d74259 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertiesChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertiesChecker.kt @@ -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() + else -> declarations.filterIsInstance() + } + + 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 diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirReassignmentAndInvisibleSetterChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirReassignmentAndInvisibleSetterChecker.kt index 81dfb0ea944..463293dd1e7 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirReassignmentAndInvisibleSetterChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirReassignmentAndInvisibleSetterChecker.kt @@ -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 diff --git a/compiler/testData/diagnostics/tests/backingField/TopLevelPropertyInitialization.fir.kt b/compiler/testData/diagnostics/tests/backingField/TopLevelPropertyInitialization.fir.kt new file mode 100644 index 00000000000..a874a823de5 --- /dev/null +++ b/compiler/testData/diagnostics/tests/backingField/TopLevelPropertyInitialization.fir.kt @@ -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 } +val c: Int +val d by lazy { c = 2; 1 } +val e: Int + get() { + c = 3 + return c + } + +class Class { + val i: Int + val j = 1.also { i = 2 } + val k: Int + val l by lazy { k = 2; 1 } + val n: Int + get() { + k = 3 + return k + } +} + +fun main() { + val x: Int + val y = 1.also { x = 2 } + val z: Int + val w by lazy { z = 2; 1 } +} diff --git a/compiler/testData/diagnostics/tests/backingField/TopLevelPropertyInitialization.kt b/compiler/testData/diagnostics/tests/backingField/TopLevelPropertyInitialization.kt new file mode 100644 index 00000000000..31107ca04f9 --- /dev/null +++ b/compiler/testData/diagnostics/tests/backingField/TopLevelPropertyInitialization.kt @@ -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 } +val c: Int +val d by lazy { c = 2; 1 } +val e: Int + get() { + c = 3 + return c + } + +class Class { + val i: Int + val j = 1.also { i = 2 } + val k: Int + val l by lazy { k = 2; 1 } + val n: Int + get() { + k = 3 + return k + } +} + +fun main() { + val x: Int + val y = 1.also { x = 2 } + val z: Int + val w by lazy { z = 2; 1 } +} diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java index 22cc56453ff..239e7e966ec 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java @@ -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 {