K1: report a warning for invisible setter accessed from a derived class

The issue is that during binding fake overrides, the compiler doesn't
 differ setters from its properties, so the compiler uses the same
 visibility for setter and entire property.

 Changing logic at the binding stage can cause some unpredictable consequences so
 the fix is to do this differentiation right at the reporting stage

 ^KT-56662 Fixed

Merge-request: KT-MR-9565
Merged-by: Michail Zarečenskij <Mikhail.Zarechenskiy@jetbrains.com>
This commit is contained in:
Mikhail Zarechenskiy
2023-04-17 11:08:16 +00:00
committed by Space Team
parent d8f253d07b
commit fc37885d6d
14 changed files with 332 additions and 9 deletions
@@ -27554,6 +27554,18 @@ public class DiagnosticCompilerTestFE10TestdataTestGenerated extends AbstractDia
runTest("compiler/testData/diagnostics/tests/scopes/initializerScopeOfExtensionProperty.kt");
}
@Test
@TestMetadata("invisibleInternalSetterAccessFromDeriviedClass.kt")
public void testInvisibleInternalSetterAccessFromDeriviedClass() throws Exception {
runTest("compiler/testData/diagnostics/tests/scopes/invisibleInternalSetterAccessFromDeriviedClass.kt");
}
@Test
@TestMetadata("invisibleInternalSetterAccessFromDeriviedClassOn.kt")
public void testInvisibleInternalSetterAccessFromDeriviedClassOn() throws Exception {
runTest("compiler/testData/diagnostics/tests/scopes/invisibleInternalSetterAccessFromDeriviedClassOn.kt");
}
@Test
@TestMetadata("invisibleSetter.kt")
public void testInvisibleSetter() throws Exception {
@@ -27554,6 +27554,18 @@ public class LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated
runTest("compiler/testData/diagnostics/tests/scopes/initializerScopeOfExtensionProperty.kt");
}
@Test
@TestMetadata("invisibleInternalSetterAccessFromDeriviedClass.kt")
public void testInvisibleInternalSetterAccessFromDeriviedClass() throws Exception {
runTest("compiler/testData/diagnostics/tests/scopes/invisibleInternalSetterAccessFromDeriviedClass.kt");
}
@Test
@TestMetadata("invisibleInternalSetterAccessFromDeriviedClassOn.kt")
public void testInvisibleInternalSetterAccessFromDeriviedClassOn() throws Exception {
runTest("compiler/testData/diagnostics/tests/scopes/invisibleInternalSetterAccessFromDeriviedClassOn.kt");
}
@Test
@TestMetadata("invisibleSetter.kt")
public void testInvisibleSetter() throws Exception {
@@ -27554,6 +27554,18 @@ public class FirLightTreeOldFrontendDiagnosticsTestGenerated extends AbstractFir
runTest("compiler/testData/diagnostics/tests/scopes/initializerScopeOfExtensionProperty.kt");
}
@Test
@TestMetadata("invisibleInternalSetterAccessFromDeriviedClass.kt")
public void testInvisibleInternalSetterAccessFromDeriviedClass() throws Exception {
runTest("compiler/testData/diagnostics/tests/scopes/invisibleInternalSetterAccessFromDeriviedClass.kt");
}
@Test
@TestMetadata("invisibleInternalSetterAccessFromDeriviedClassOn.kt")
public void testInvisibleInternalSetterAccessFromDeriviedClassOn() throws Exception {
runTest("compiler/testData/diagnostics/tests/scopes/invisibleInternalSetterAccessFromDeriviedClassOn.kt");
}
@Test
@TestMetadata("invisibleSetter.kt")
public void testInvisibleSetter() throws Exception {
@@ -27566,6 +27566,18 @@ public class FirPsiOldFrontendDiagnosticsTestGenerated extends AbstractFirPsiDia
runTest("compiler/testData/diagnostics/tests/scopes/initializerScopeOfExtensionProperty.kt");
}
@Test
@TestMetadata("invisibleInternalSetterAccessFromDeriviedClass.kt")
public void testInvisibleInternalSetterAccessFromDeriviedClass() throws Exception {
runTest("compiler/testData/diagnostics/tests/scopes/invisibleInternalSetterAccessFromDeriviedClass.kt");
}
@Test
@TestMetadata("invisibleInternalSetterAccessFromDeriviedClassOn.kt")
public void testInvisibleInternalSetterAccessFromDeriviedClassOn() throws Exception {
runTest("compiler/testData/diagnostics/tests/scopes/invisibleInternalSetterAccessFromDeriviedClassOn.kt");
}
@Test
@TestMetadata("invisibleSetter.kt")
public void testInvisibleSetter() throws Exception {
@@ -47,6 +47,7 @@ import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall
import org.jetbrains.kotlin.resolve.calls.model.VariableAsFunctionResolvedCall
import org.jetbrains.kotlin.resolve.calls.util.*
import org.jetbrains.kotlin.resolve.checkers.PlatformDiagnosticSuppressor
import org.jetbrains.kotlin.resolve.descriptorUtil.firstOverridden
import org.jetbrains.kotlin.resolve.descriptorUtil.isEffectivelyExternal
import org.jetbrains.kotlin.resolve.descriptorUtil.module
import org.jetbrains.kotlin.resolve.scopes.receivers.ExtensionReceiver
@@ -494,15 +495,19 @@ class ControlFlowInformationProviderImpl private constructor(
if (DescriptorVisibilityUtils.isVisible(receiverValue, variableDescriptor, descriptor, languageVersionSettings)
&& setterDescriptor != null
&& !DescriptorVisibilityUtils.isVisible(receiverValue, setterDescriptor, descriptor, languageVersionSettings)
) {
report(
Errors.INVISIBLE_SETTER.on(
expression, variableDescriptor, setterDescriptor.visibility,
setterDescriptor
), ctxt
)
return true
if (!DescriptorVisibilityUtils.isVisible(receiverValue, setterDescriptor, descriptor, languageVersionSettings)) {
report(
INVISIBLE_SETTER.on(
expression, variableDescriptor, setterDescriptor.visibility,
setterDescriptor
), ctxt
)
return true
} else {
// don't return anything as only warning is reported (not error), so further diagnostics are also important
reportVisibilityWarningForInternalFakeSetterOverride(setterDescriptor, expression, variableDescriptor, ctxt)
}
}
}
val isThisOrNoDispatchReceiver = PseudocodeUtil.isThisOrNoDispatchReceiver(writeValueInstruction, trace.bindingContext)
@@ -560,6 +565,35 @@ class ControlFlowInformationProviderImpl private constructor(
return false
}
private fun reportVisibilityWarningForInternalFakeSetterOverride(
setterDescriptor: PropertySetterDescriptor,
expression: KtExpression,
variableDescriptor: PropertyDescriptor,
ctxt: VariableInitContext
) {
if (setterDescriptor.kind.isReal) return
if (setterDescriptor.visibility.isPublicAPI) return
val containingClass = setterDescriptor.containingDeclaration as? ClassDescriptor ?: return
val firstRealOverridden = setterDescriptor.firstOverridden { it.kind.isReal } ?: return
val visibleOverrides = OverridingUtil.filterVisibleFakeOverrides(containingClass, listOf(firstRealOverridden))
if (visibleOverrides.isEmpty()) {
val diagnostic =
when (languageVersionSettings.supportsFeature(LanguageFeature.ProhibitAccessToInvisibleSetterFromDerivedClass)) {
true -> INVISIBLE_SETTER
else -> INVISIBLE_SETTER_FROM_DERIVED
}
report(
diagnostic.on(
expression, variableDescriptor, setterDescriptor.visibility,
setterDescriptor
), ctxt
)
}
}
private fun reportValReassigned(expression: KtExpression, variableDescriptor: VariableDescriptor, ctxt: VariableInitContext) {
report(VAL_REASSIGNMENT_VIA_BACKING_FIELD.on(languageVersionSettings, expression, variableDescriptor), ctxt)
}
@@ -1204,6 +1204,8 @@ public interface Errors {
DiagnosticFactory3<PsiElement, DeclarationDescriptor, DescriptorVisibility, DeclarationDescriptor> INVISIBLE_SETTER =
DiagnosticFactory3.create(ERROR);
DiagnosticFactory3<PsiElement, DeclarationDescriptor, DescriptorVisibility, DeclarationDescriptor> INVISIBLE_SETTER_FROM_DERIVED =
DiagnosticFactory3.create(WARNING);
DiagnosticFactory1<PsiElement, KtKeywordToken> VAL_OR_VAR_ON_LOOP_PARAMETER = DiagnosticFactory1.create(ERROR);
DiagnosticFactory1<PsiElement, KtKeywordToken> VAL_OR_VAR_ON_FUN_PARAMETER = DiagnosticFactory1.create(ERROR);
@@ -414,6 +414,8 @@ public class DefaultErrorMessages {
MAP.put(SETTER_PROJECTED_OUT, "Setter for ''{0}'' is removed by type projection", NAME);
MAP.put(INVISIBLE_SETTER, "Cannot assign to ''{0}'': the setter is {1} in {2}", NAME, VISIBILITY,
NAME_OF_CONTAINING_DECLARATION_OR_FILE);
MAP.put(INVISIBLE_SETTER_FROM_DERIVED, "Cannot assign to ''{0}'': the setter is {1} in {2}. This warning will be an error soon. See https://youtrack.jetbrains.com/issue/KT-56662 for details", NAME, VISIBILITY,
NAME_OF_CONTAINING_DECLARATION_OR_FILE);
MAP.put(INITIALIZATION_BEFORE_DECLARATION, "Variable cannot be initialized before declaration", NAME);
MAP.put(VARIABLE_EXPECTED, "Variable expected");
@@ -0,0 +1,56 @@
// !LANGUAGE:-ProhibitAccessToInvisibleSetterFromDerivedClass
// IGNORE_REVERSED_RESOLVE
// MODULE: m1
// FILE: base.kt
open class Base {
var foo: String = ""
internal set
}
// MODULE: m2(m1)
// FILE: derived1.kt
fun testBase(b: Base) {
b.<!INVISIBLE_SETTER!>foo<!> = "other"
}
open class Derived1(foo: String) : Base() {
init {
this.<!INVISIBLE_SETTER!>foo<!> = foo
}
fun bar(param: String) {
<!INVISIBLE_SETTER!>foo<!> = param
this.<!INVISIBLE_SETTER!>foo<!> = param
}
}
open class Derived2 : Derived1("")
fun testFunction(d: Derived1) {
d.<!INVISIBLE_SETTER!>foo<!> = "other"
}
// MODULE: m3(m2, m1)
// FILE: derived2.kt
fun testDerivied1(d: Derived1) {
d.<!INVISIBLE_SETTER!>foo<!> = "other"
}
fun testDerivied2(d: Derived2) {
d.<!INVISIBLE_SETTER!>foo<!> = "other"
}
class Derivied3(foo: String) : Derived2() {
init {
this.<!INVISIBLE_SETTER!>foo<!> = foo
}
fun bar1(param: String) {
<!INVISIBLE_SETTER!>foo<!> = param
this.<!INVISIBLE_SETTER!>foo<!> = param
}
}
@@ -0,0 +1,56 @@
// !LANGUAGE:-ProhibitAccessToInvisibleSetterFromDerivedClass
// IGNORE_REVERSED_RESOLVE
// MODULE: m1
// FILE: base.kt
open class Base {
var foo: String = ""
internal set
}
// MODULE: m2(m1)
// FILE: derived1.kt
fun testBase(b: Base) {
<!INVISIBLE_SETTER!>b.foo<!> = "other"
}
open class Derived1(foo: String) : Base() {
init {
<!INVISIBLE_SETTER_FROM_DERIVED!>this.<!DEBUG_INFO_LEAKING_THIS!>foo<!><!> = foo
}
fun bar(param: String) {
<!INVISIBLE_SETTER_FROM_DERIVED!>foo<!> = param
<!INVISIBLE_SETTER_FROM_DERIVED!>this.foo<!> = param
}
}
open class Derived2 : Derived1("")
fun testFunction(d: Derived1) {
<!INVISIBLE_SETTER_FROM_DERIVED!>d.foo<!> = "other"
}
// MODULE: m3(m2, m1)
// FILE: derived2.kt
fun testDerivied1(d: Derived1) {
<!INVISIBLE_SETTER!>d.foo<!> = "other"
}
fun testDerivied2(d: Derived2) {
<!INVISIBLE_SETTER!>d.foo<!> = "other"
}
class Derivied3(foo: String) : Derived2() {
init {
<!INVISIBLE_SETTER_FROM_DERIVED!>this.foo<!> = foo
}
fun bar1(param: String) {
<!INVISIBLE_SETTER_FROM_DERIVED!>foo<!> = param
<!INVISIBLE_SETTER_FROM_DERIVED!>this.foo<!> = param
}
}
@@ -0,0 +1,56 @@
// !LANGUAGE:+ProhibitAccessToInvisibleSetterFromDerivedClass
// IGNORE_REVERSED_RESOLVE
// MODULE: m1
// FILE: base.kt
open class Base {
var foo: String = ""
internal set
}
// MODULE: m2(m1)
// FILE: derived1.kt
fun testBase(b: Base) {
b.<!INVISIBLE_SETTER!>foo<!> = "other"
}
open class Derived1(foo: String) : Base() {
init {
this.<!INVISIBLE_SETTER!>foo<!> = foo
}
fun bar(param: String) {
<!INVISIBLE_SETTER!>foo<!> = param
this.<!INVISIBLE_SETTER!>foo<!> = param
}
}
open class Derived2 : Derived1("")
fun testFunction(d: Derived1) {
d.<!INVISIBLE_SETTER!>foo<!> = "other"
}
// MODULE: m3(m2, m1)
// FILE: derived2.kt
fun testDerivied1(d: Derived1) {
d.<!INVISIBLE_SETTER!>foo<!> = "other"
}
fun testDerivied2(d: Derived2) {
d.<!INVISIBLE_SETTER!>foo<!> = "other"
}
class Derivied3(foo: String) : Derived2() {
init {
this.<!INVISIBLE_SETTER!>foo<!> = foo
}
fun bar1(param: String) {
<!INVISIBLE_SETTER!>foo<!> = param
this.<!INVISIBLE_SETTER!>foo<!> = param
}
}
@@ -0,0 +1,56 @@
// !LANGUAGE:+ProhibitAccessToInvisibleSetterFromDerivedClass
// IGNORE_REVERSED_RESOLVE
// MODULE: m1
// FILE: base.kt
open class Base {
var foo: String = ""
internal set
}
// MODULE: m2(m1)
// FILE: derived1.kt
fun testBase(b: Base) {
<!INVISIBLE_SETTER!>b.foo<!> = "other"
}
open class Derived1(foo: String) : Base() {
init {
<!INVISIBLE_SETTER!>this.<!DEBUG_INFO_LEAKING_THIS!>foo<!><!> = foo
}
fun bar(param: String) {
<!INVISIBLE_SETTER!>foo<!> = param
<!INVISIBLE_SETTER!>this.foo<!> = param
}
}
open class Derived2 : Derived1("")
fun testFunction(d: Derived1) {
<!INVISIBLE_SETTER!>d.foo<!> = "other"
}
// MODULE: m3(m2, m1)
// FILE: derived2.kt
fun testDerivied1(d: Derived1) {
<!INVISIBLE_SETTER!>d.foo<!> = "other"
}
fun testDerivied2(d: Derived2) {
<!INVISIBLE_SETTER!>d.foo<!> = "other"
}
class Derivied3(foo: String) : Derived2() {
init {
<!INVISIBLE_SETTER!>this.foo<!> = foo
}
fun bar1(param: String) {
<!INVISIBLE_SETTER!>foo<!> = param
<!INVISIBLE_SETTER!>this.foo<!> = param
}
}
@@ -28332,6 +28332,18 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest {
runTest("compiler/testData/diagnostics/tests/scopes/initializerScopeOfExtensionProperty.kt");
}
@Test
@TestMetadata("invisibleInternalSetterAccessFromDeriviedClass.kt")
public void testInvisibleInternalSetterAccessFromDeriviedClass() throws Exception {
runTest("compiler/testData/diagnostics/tests/scopes/invisibleInternalSetterAccessFromDeriviedClass.kt");
}
@Test
@TestMetadata("invisibleInternalSetterAccessFromDeriviedClassOn.kt")
public void testInvisibleInternalSetterAccessFromDeriviedClassOn() throws Exception {
runTest("compiler/testData/diagnostics/tests/scopes/invisibleInternalSetterAccessFromDeriviedClassOn.kt");
}
@Test
@TestMetadata("invisibleSetter.kt")
public void testInvisibleSetter() throws Exception {
@@ -287,6 +287,7 @@ enum class LanguageFeature(
ForbidInferringPostponedTypeVariableIntoDeclaredUpperBound(KOTLIN_2_0, kind = BUG_FIX), // KT-47986
ProhibitUseSiteGetTargetAnnotations(KOTLIN_2_0, kind = BUG_FIX), // KT-15470
KeepNullabilityWhenApproximatingLocalType(KOTLIN_2_0, kind = BUG_FIX), // KT-53982
ProhibitAccessToInvisibleSetterFromDerivedClass(KOTLIN_2_0, kind = BUG_FIX), // KT-56662
// 2.1
@@ -805,7 +805,7 @@ public class OverridingUtil {
}
@NotNull
private static Collection<CallableMemberDescriptor> filterVisibleFakeOverrides(
public static Collection<CallableMemberDescriptor> filterVisibleFakeOverrides(
@NotNull final ClassDescriptor current,
@NotNull Collection<CallableMemberDescriptor> toFilter
) {