From 7db551c452d25871950a75f7f2ec492af97da73e Mon Sep 17 00:00:00 2001 From: Brian Norman Date: Wed, 16 Aug 2023 07:15:29 -0500 Subject: [PATCH] [FIR] Property accessors are not part of class initialization When checking for class val property reassignment diagnostic, property initializers should be treated as part of the class initialization. However, property accessors should not. Previously, only the property itself was checked for both of these situations and resulted in not reporting diagnostic within property accessors. #KT-59744 Fixed --- ...ocalProperty_initializedProperties.fir.txt | 27 ++++++++++++++++ ...fNonLocalProperty_initializedProperties.kt | 29 ++++++++++++++++- ...irReassignmentAndInvisibleSetterChecker.kt | 31 +++++++++++++------ 3 files changed, 76 insertions(+), 11 deletions(-) diff --git a/compiler/fir/analysis-tests/testData/resolve/cfa/reassignOfNonLocalProperty_initializedProperties.fir.txt b/compiler/fir/analysis-tests/testData/resolve/cfa/reassignOfNonLocalProperty_initializedProperties.fir.txt index d8a54a3508f..04e75db70a6 100644 --- a/compiler/fir/analysis-tests/testData/resolve/cfa/reassignOfNonLocalProperty_initializedProperties.fir.txt +++ b/compiler/fir/analysis-tests/testData/resolve/cfa/reassignOfNonLocalProperty_initializedProperties.fir.txt @@ -28,6 +28,33 @@ FILE: reassignOfNonLocalProperty_initializedProperties.kt ) public get(): R|kotlin/String| + public final var b: R|kotlin/String| = String(hello) + public get(): R|kotlin/String| { + this@R|/Some|.R|/Some.x| = String(error) + this@R|/Some|.R|/y| = String(error) + R|/z| = String(error) + ^ this@R|/Some|.F|/Some.b| + } + public set(value: R|kotlin/String|): R|kotlin/Unit| { + this@R|/Some|.R|/Some.x| = R|/value| + this@R|/Some|.R|/y| = R|/value| + R|/z| = R|/value| + this@R|/Some|.F|/Some.b| = R|/value| + } + + public final var c: R|kotlin/String| + public get(): R|kotlin/String| { + this@R|/Some|.R|/Some.x| = String(error) + this@R|/Some|.R|/y| = String(error) + R|/z| = String(error) + ^ String(hello) + } + public set(value: R|kotlin/String|): R|kotlin/Unit| { + this@R|/Some|.R|/Some.x| = R|/value| + this@R|/Some|.R|/y| = R|/value| + R|/z| = R|/value| + } + public final fun test_1(): R|kotlin/Unit| { this@R|/Some|.R|/Some.x| = String(error) this@R|/Some|.R|/y| = String(error) diff --git a/compiler/fir/analysis-tests/testData/resolve/cfa/reassignOfNonLocalProperty_initializedProperties.kt b/compiler/fir/analysis-tests/testData/resolve/cfa/reassignOfNonLocalProperty_initializedProperties.kt index 5d5a2c458d1..dbe084b22bc 100644 --- a/compiler/fir/analysis-tests/testData/resolve/cfa/reassignOfNonLocalProperty_initializedProperties.kt +++ b/compiler/fir/analysis-tests/testData/resolve/cfa/reassignOfNonLocalProperty_initializedProperties.kt @@ -1,4 +1,4 @@ -// ISSUE: KT-55493 +// ISSUE: KT-55493, KT-59744 // WITH_STDLIB val z: String = "ok" @@ -22,6 +22,33 @@ class Some { "hello" } + var b: String = "hello" + get() { + x = "error" + y = "error" + z = "error" + return field + } + set(value) { + x = value + y = value + z = value + field = value + } + + var c: String + get() { + x = "error" + y = "error" + z = "error" + return "hello" + } + set(value) { + x = value + y = value + z = value + } + fun test_1() { x = "error" y = "error" 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 5fdfd7b939f..1b0472efcd9 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 @@ -148,18 +148,29 @@ object FirReassignmentAndInvisibleSetterChecker : FirVariableAssignmentChecker() private fun isInOwnersInitializer(receiver: FirExpression, context: CheckerContext): Boolean { val uninitializedThisSymbol = (receiver as? FirThisReceiverExpression)?.calleeReference?.boundSymbol ?: return false - var foundInitializer = false - for ((i, declaration) in context.containingDeclarations.withIndex()) { - if (declaration is FirClass) { - foundInitializer = if (context.containingDeclarations.getOrNull(i + 1)?.evaluatedInPlace == false) { - // In member function of a class, assume all outer classes are already initialized - // by the time this function is called. - false - } else { - foundInitializer || declaration.symbol == uninitializedThisSymbol + val containingDeclarations = context.containingDeclarations + + val index = containingDeclarations.indexOfFirst { it is FirClass && it.symbol == uninitializedThisSymbol } + if (index == -1) return false + + for (i in index until containingDeclarations.size) { + if (containingDeclarations[i] is FirClass) { + // Properties need special consideration as some parts are evaluated in-place (initializers) and others are not (accessors). + // So it is not enough to just check the FirProperty - which is treated as in-place - but the following declaration needs to + // be checked if and only if it is a property accessor. + val container = when (val next = containingDeclarations.getOrNull(i + 1)) { + is FirProperty -> containingDeclarations.getOrNull(i + 2)?.takeIf { it is FirPropertyAccessor } ?: next + else -> next + } + + // In member function of a class, assume all outer classes are already initialized + // by the time this function is called. + if (container?.evaluatedInPlace == false) { + return false } } } - return foundInitializer + + return true } }