From c718c77c43c9ab88da37d1f608ba17b86a5f022e Mon Sep 17 00:00:00 2001 From: Roman Efremov Date: Thu, 6 Apr 2023 11:54:32 +0200 Subject: [PATCH] [FE] Check visibilities of expected and actual property setters are compatible ^KT-30905 Fixed --- ...DiagnosticsWithLightTreeTestGenerated.java | 6 +++ ...endMPPDiagnosticsWithPsiTestGenerated.java | 6 +++ .../mpp/FirExpectActualResolver.kt | 13 +++++++ .../ClassicPositioningStrategies.kt | 3 ++ .../multiplatform/ExpectedActualResolver.kt | 10 +++++ .../multiplatform/varSetterVisibility.fir.kt | 39 +++++++++++++++++++ .../multiplatform/varSetterVisibility.kt | 39 +++++++++++++++++++ .../test/runners/DiagnosticTestGenerated.java | 6 +++ .../ExpectActualCompatibility.kt | 1 + 9 files changed, 123 insertions(+) create mode 100644 compiler/testData/diagnostics/tests/multiplatform/varSetterVisibility.fir.kt create mode 100644 compiler/testData/diagnostics/tests/multiplatform/varSetterVisibility.kt diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated.java index 623debfbcca..595b9d53c67 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated.java @@ -127,6 +127,12 @@ public class FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated extends Abst runTest("compiler/testData/diagnostics/tests/multiplatform/smartcastOnMemberPropertyFromCommonClass.kt"); } + @Test + @TestMetadata("varSetterVisibility.kt") + public void testVarSetterVisibility() throws Exception { + runTest("compiler/testData/diagnostics/tests/multiplatform/varSetterVisibility.kt"); + } + @Nested @TestMetadata("compiler/testData/diagnostics/tests/multiplatform/defaultArguments") @TestDataPath("$PROJECT_ROOT") diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithPsiTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithPsiTestGenerated.java index 765e69ef2b7..131564738d8 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithPsiTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithPsiTestGenerated.java @@ -127,6 +127,12 @@ public class FirOldFrontendMPPDiagnosticsWithPsiTestGenerated extends AbstractFi runTest("compiler/testData/diagnostics/tests/multiplatform/smartcastOnMemberPropertyFromCommonClass.kt"); } + @Test + @TestMetadata("varSetterVisibility.kt") + public void testVarSetterVisibility() throws Exception { + runTest("compiler/testData/diagnostics/tests/multiplatform/varSetterVisibility.kt"); + } + @Nested @TestMetadata("compiler/testData/diagnostics/tests/multiplatform/defaultArguments") @TestDataPath("$PROJECT_ROOT") diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/mpp/FirExpectActualResolver.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/mpp/FirExpectActualResolver.kt index f10c052cda7..5f9b8c79000 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/mpp/FirExpectActualResolver.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/mpp/FirExpectActualResolver.kt @@ -523,10 +523,23 @@ object FirExpectActualResolver { !equalBy(expected, actual) { p -> p.isVar } -> ExpectActualCompatibility.Incompatible.PropertyKind !equalBy(expected, actual) { p -> p.isLateInit } -> ExpectActualCompatibility.Incompatible.PropertyLateinitModifier expected.isConst && !actual.isConst -> ExpectActualCompatibility.Incompatible.PropertyConstModifier + !arePropertySettersWithCompatibleVisibilities(expected, actual) -> ExpectActualCompatibility.Incompatible.PropertySetterVisibility else -> ExpectActualCompatibility.Compatible } } + private fun arePropertySettersWithCompatibleVisibilities( + expected: FirPropertySymbol, + actual: FirPropertySymbol, + ): Boolean { + val expectedSetterStatus = expected.setterSymbol?.resolvedStatus + val actualSetterStatus = actual.setterSymbol?.resolvedStatus + if (expectedSetterStatus == null || actualSetterStatus == null) { + return true + } + return areDeclarationsWithCompatibleVisibilities(expectedSetterStatus, actualSetterStatus) + } + // ---------------------------------------- Utils ---------------------------------------- private fun List.toTypeList(substitutor: ConeSubstitutor): List { diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/ClassicPositioningStrategies.kt b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/ClassicPositioningStrategies.kt index cdf9abac561..54b00894f8f 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/ClassicPositioningStrategies.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/ClassicPositioningStrategies.kt @@ -106,6 +106,9 @@ object ClassicPositioningStrategies { ExpectActualCompatibility.Incompatible.Visibility -> { element.visibilityModifier() } + ExpectActualCompatibility.Incompatible.PropertySetterVisibility -> { + (element as? KtProperty)?.setter?.modifierList + } }?.let { markElement(it) } ?: ACTUAL_DECLARATION_NAME.mark(element) } } diff --git a/compiler/resolution/src/org/jetbrains/kotlin/resolve/multiplatform/ExpectedActualResolver.kt b/compiler/resolution/src/org/jetbrains/kotlin/resolve/multiplatform/ExpectedActualResolver.kt index b1df5dcd167..1bfa9cbc0f8 100644 --- a/compiler/resolution/src/org/jetbrains/kotlin/resolve/multiplatform/ExpectedActualResolver.kt +++ b/compiler/resolution/src/org/jetbrains/kotlin/resolve/multiplatform/ExpectedActualResolver.kt @@ -370,10 +370,20 @@ object ExpectedActualResolver { !equalBy(expected, actual) { p -> p.isVar } -> Incompatible.PropertyKind !equalBy(expected, actual) { p -> p.isLateInit } -> Incompatible.PropertyLateinitModifier expected.isConst && !actual.isConst -> Incompatible.PropertyConstModifier + !arePropertySettersWithCompatibleVisibilities(expected, actual) -> Incompatible.PropertySetterVisibility else -> Compatible } } + private fun arePropertySettersWithCompatibleVisibilities(expected: PropertyDescriptor, actual: PropertyDescriptor): Boolean { + val expectedSetter = expected.setter + val actualSetter = actual.setter + if (expectedSetter == null || actualSetter == null) { + return true + } + return areDeclarationsWithCompatibleVisibilities(expectedSetter, actualSetter) + } + private fun areCompatibleClassifiers(a: ClassDescriptor, other: ClassifierDescriptor): ExpectActualCompatibility { // Can't check FQ names here because nested expected class may be implemented via actual typealias's expansion with the other FQ name assert(a.name == other.name) { "This function should be invoked only for declarations with the same name: $a, $other" } diff --git a/compiler/testData/diagnostics/tests/multiplatform/varSetterVisibility.fir.kt b/compiler/testData/diagnostics/tests/multiplatform/varSetterVisibility.fir.kt new file mode 100644 index 00000000000..a574d3eeebd --- /dev/null +++ b/compiler/testData/diagnostics/tests/multiplatform/varSetterVisibility.fir.kt @@ -0,0 +1,39 @@ +// MODULE: m1-common +// FILE: common.kt +expect var v1: Boolean + +expect var v2: Boolean + internal set + +expect var v3: Boolean + internal set + +expect open class C { + var foo: Boolean +} + +expect open class C2 { + var foo: Boolean +} + +// MODULE: m1-jvm()()(m1-common) +// FILE: jvm.kt +actual var v1: Boolean = false + private set + +actual var v2: Boolean = false + +actual var v3: Boolean = false + private set + +actual open class C { + actual var foo: Boolean = false + protected set +} + +open class C2Typealias { + var foo: Boolean = false + protected set +} + +actual typealias C2 = C2Typealias diff --git a/compiler/testData/diagnostics/tests/multiplatform/varSetterVisibility.kt b/compiler/testData/diagnostics/tests/multiplatform/varSetterVisibility.kt new file mode 100644 index 00000000000..574a44e008d --- /dev/null +++ b/compiler/testData/diagnostics/tests/multiplatform/varSetterVisibility.kt @@ -0,0 +1,39 @@ +// MODULE: m1-common +// FILE: common.kt +expect var v1: Boolean + +expect var v2: Boolean + internal set + +expect var v3: Boolean + internal set + +expect open class C { + var foo: Boolean +} + +expect open class C2 { + var foo: Boolean +} + +// MODULE: m1-jvm()()(m1-common) +// FILE: jvm.kt +actual var v1: Boolean = false + private set + +actual var v2: Boolean = false + +actual var v3: Boolean = false + private set + +actual open class C { + actual var foo: Boolean = false + protected set +} + +open class C2Typealias { + var foo: Boolean = false + protected set +} + +actual typealias C2 = C2Typealias 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 e8c310e0fbe..e2565b3b182 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 @@ -21912,6 +21912,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest { runTest("compiler/testData/diagnostics/tests/multiplatform/smartcastOnMemberPropertyFromCommonClass.kt"); } + @Test + @TestMetadata("varSetterVisibility.kt") + public void testVarSetterVisibility() throws Exception { + runTest("compiler/testData/diagnostics/tests/multiplatform/varSetterVisibility.kt"); + } + @Nested @TestMetadata("compiler/testData/diagnostics/tests/multiplatform/defaultArguments") @TestDataPath("$PROJECT_ROOT") diff --git a/core/compiler.common/src/org/jetbrains/kotlin/resolve/multiplatform/ExpectActualCompatibility.kt b/core/compiler.common/src/org/jetbrains/kotlin/resolve/multiplatform/ExpectActualCompatibility.kt index 948d2b00cfc..9a4f7fb6524 100644 --- a/core/compiler.common/src/org/jetbrains/kotlin/resolve/multiplatform/ExpectActualCompatibility.kt +++ b/core/compiler.common/src/org/jetbrains/kotlin/resolve/multiplatform/ExpectActualCompatibility.kt @@ -58,6 +58,7 @@ sealed class ExpectActualCompatibility { object PropertyKind : Incompatible("property kinds are different (val vs var)") object PropertyLateinitModifier : Incompatible("modifiers are different (lateinit)") object PropertyConstModifier : Incompatible("modifiers are different (const)") + object PropertySetterVisibility : Incompatible("setter visibility is different") // Classifiers