From f35680f0a479ea7e33e4e734d5d1a82f1bf3fbc6 Mon Sep 17 00:00:00 2001 From: Tianyu Geng Date: Tue, 7 Sep 2021 15:14:58 -0700 Subject: [PATCH] FIR checker: resolve property with protected getter and public setter Assuming Java class `Super` has a protected `getName` method and a public `setName` method. Then, in a subclass of `Super`, FE1.0 allows calls to `setName` via the property syntax if the property is protected and invisible due to dispatch receiver is not `this` or `super`. --- ...CompilerTestFE10TestdataTestGenerated.java | 6 ++++ ...irOldFrontendDiagnosticsTestGenerated.java | 6 ++++ ...DiagnosticsWithLightTreeTestGenerated.java | 6 ++++ .../fir/java/FirJavaVisibilityChecker.kt | 25 +++++++++++----- .../kotlin/fir/FirVisibilityChecker.kt | 27 ++++++++++++----- .../protectedGetterWithPublicSetter.fir.kt | 30 +++++++++++++++++++ .../protectedGetterWithPublicSetter.kt | 30 +++++++++++++++++++ .../protectedGetterWithPublicSetter.txt | 27 +++++++++++++++++ .../SetterHasHigherAccess.fir.kt | 4 +-- .../test/runners/DiagnosticTestGenerated.java | 6 ++++ 10 files changed, 151 insertions(+), 16 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/properties/protectedGetterWithPublicSetter.fir.kt create mode 100644 compiler/testData/diagnostics/tests/properties/protectedGetterWithPublicSetter.kt create mode 100644 compiler/testData/diagnostics/tests/properties/protectedGetterWithPublicSetter.txt diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java index 96fa8d5d1c2..843faee44ac 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java @@ -22527,6 +22527,12 @@ public class DiagnosisCompilerTestFE10TestdataTestGenerated extends AbstractDiag runTest("compiler/testData/diagnostics/tests/properties/lateinitOnTopLevel.kt"); } + @Test + @TestMetadata("protectedGetterWithPublicSetter.kt") + public void testProtectedGetterWithPublicSetter() throws Exception { + runTest("compiler/testData/diagnostics/tests/properties/protectedGetterWithPublicSetter.kt"); + } + @Nested @TestMetadata("compiler/testData/diagnostics/tests/properties/backingField") @TestDataPath("$PROJECT_ROOT") diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java index 6cce1ea9733..a355249c3cf 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java @@ -22527,6 +22527,12 @@ public class FirOldFrontendDiagnosticsTestGenerated extends AbstractFirDiagnosti runTest("compiler/testData/diagnostics/tests/properties/lateinitOnTopLevel.kt"); } + @Test + @TestMetadata("protectedGetterWithPublicSetter.kt") + public void testProtectedGetterWithPublicSetter() throws Exception { + runTest("compiler/testData/diagnostics/tests/properties/protectedGetterWithPublicSetter.kt"); + } + @Nested @TestMetadata("compiler/testData/diagnostics/tests/properties/backingField") @TestDataPath("$PROJECT_ROOT") diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java index cabe63e43f4..2c0a9854cd1 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java @@ -22527,6 +22527,12 @@ public class FirOldFrontendDiagnosticsWithLightTreeTestGenerated extends Abstrac runTest("compiler/testData/diagnostics/tests/properties/lateinitOnTopLevel.kt"); } + @Test + @TestMetadata("protectedGetterWithPublicSetter.kt") + public void testProtectedGetterWithPublicSetter() throws Exception { + runTest("compiler/testData/diagnostics/tests/properties/protectedGetterWithPublicSetter.kt"); + } + @Nested @TestMetadata("compiler/testData/diagnostics/tests/properties/backingField") @TestDataPath("$PROJECT_ROOT") diff --git a/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/FirJavaVisibilityChecker.kt b/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/FirJavaVisibilityChecker.kt index cd6515a3db8..83ca1d5fe6b 100644 --- a/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/FirJavaVisibilityChecker.kt +++ b/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/FirJavaVisibilityChecker.kt @@ -5,14 +5,18 @@ package org.jetbrains.kotlin.fir.java +import org.jetbrains.kotlin.descriptors.Visibilities import org.jetbrains.kotlin.descriptors.Visibility import org.jetbrains.kotlin.descriptors.java.JavaVisibilities import org.jetbrains.kotlin.fir.* import org.jetbrains.kotlin.fir.declarations.FirDeclaration import org.jetbrains.kotlin.fir.declarations.FirFile +import org.jetbrains.kotlin.fir.declarations.utils.visibility +import org.jetbrains.kotlin.fir.resolve.calls.FirSyntheticPropertySymbol import org.jetbrains.kotlin.fir.resolve.calls.ReceiverValue import org.jetbrains.kotlin.fir.symbols.FirBasedSymbol -import org.jetbrains.kotlin.fir.symbols.impl.* +import org.jetbrains.kotlin.fir.symbols.impl.FirNamedFunctionSymbol +import org.jetbrains.kotlin.fir.symbols.impl.FirVariableSymbol @NoMutableState object FirJavaVisibilityChecker : FirVisibilityChecker() { @@ -22,18 +26,25 @@ object FirJavaVisibilityChecker : FirVisibilityChecker() { useSiteFile: FirFile, containingDeclarations: List, dispatchReceiver: ReceiverValue?, - session: FirSession + session: FirSession, + isCallToPropertySetter: Boolean, ): Boolean { return when (declarationVisibility) { JavaVisibilities.ProtectedAndPackage, JavaVisibilities.ProtectedStaticVisibility -> { if (symbol.packageFqName() == useSiteFile.packageFqName) { true } else { - val ownerLookupTag = symbol.getOwnerLookupTag() - ownerLookupTag != null && canSeeProtectedMemberOf( - containingDeclarations, dispatchReceiver, ownerLookupTag, session, - isVariableOrNamedFunction = symbol is FirVariableSymbol || symbol is FirNamedFunctionSymbol - ) + val ownerLookupTag = symbol.getOwnerLookupTag() ?: return false + if (canSeeProtectedMemberOf( + containingDeclarations, dispatchReceiver, ownerLookupTag, session, + isVariableOrNamedFunction = symbol is FirVariableSymbol || symbol is FirNamedFunctionSymbol + ) + ) return true + + // FE1.0 allows calling public setters with property assignment syntax if the getter is protected. + if (!isCallToPropertySetter || symbol !is FirSyntheticPropertySymbol) return false + val setterVisibility = symbol.setterSymbol?.visibility + setterVisibility != null && setterVisibility == Visibilities.Public } } diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/FirVisibilityChecker.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/FirVisibilityChecker.kt index 6a2acb54710..bc8d6042b26 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/FirVisibilityChecker.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/FirVisibilityChecker.kt @@ -10,6 +10,7 @@ import org.jetbrains.kotlin.descriptors.Visibility import org.jetbrains.kotlin.fir.declarations.* import org.jetbrains.kotlin.fir.declarations.utils.* import org.jetbrains.kotlin.fir.expressions.FirPropertyAccessExpression +import org.jetbrains.kotlin.fir.expressions.FirVariableAssignment import org.jetbrains.kotlin.fir.references.FirSuperReference import org.jetbrains.kotlin.fir.resolve.* import org.jetbrains.kotlin.fir.resolve.calls.Candidate @@ -18,9 +19,9 @@ import org.jetbrains.kotlin.fir.resolve.calls.FirSyntheticFunctionSymbol import org.jetbrains.kotlin.fir.resolve.calls.ReceiverValue import org.jetbrains.kotlin.fir.symbols.ConeClassLikeLookupTag import org.jetbrains.kotlin.fir.symbols.FirBasedSymbol -import org.jetbrains.kotlin.fir.symbols.impl.FirBackingFieldSymbol import org.jetbrains.kotlin.fir.symbols.impl.* -import org.jetbrains.kotlin.fir.types.* +import org.jetbrains.kotlin.fir.types.ConeClassLikeType +import org.jetbrains.kotlin.fir.types.coneType import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.types.AbstractTypeChecker @@ -47,7 +48,8 @@ abstract class FirVisibilityChecker : FirSessionComponent { useSiteFile: FirFile, containingDeclarations: List, dispatchReceiver: ReceiverValue?, - session: FirSession + session: FirSession, + isCallToPropertySetter: Boolean, ): Boolean { return true } @@ -90,7 +92,14 @@ abstract class FirVisibilityChecker : FirSessionComponent { return true } - val visible = isVisible(declaration, session, useSiteFile, containingDeclarations, candidate.dispatchReceiverValue) + val visible = isVisible( + declaration, + session, + useSiteFile, + containingDeclarations, + candidate.dispatchReceiverValue, + candidate.callInfo.callSite is FirVariableAssignment + ) val backingField = declaration.getBackingFieldIfApplicable() if (visible && backingField != null) { @@ -100,6 +109,7 @@ abstract class FirVisibilityChecker : FirSessionComponent { useSiteFile, containingDeclarations, candidate.dispatchReceiverValue, + candidate.callInfo.callSite is FirVariableAssignment, ) } @@ -111,7 +121,8 @@ abstract class FirVisibilityChecker : FirSessionComponent { session: FirSession, useSiteFile: FirFile, containingDeclarations: List, - dispatchReceiver: ReceiverValue? + dispatchReceiver: ReceiverValue?, + isCallToPropertySetter: Boolean = false, ): Boolean { require(declaration is FirDeclaration) val provider = session.firProvider @@ -166,7 +177,8 @@ abstract class FirVisibilityChecker : FirSessionComponent { useSiteFile, containingDeclarations, dispatchReceiver, - session + session, + isCallToPropertySetter, ) } } @@ -177,7 +189,8 @@ abstract class FirVisibilityChecker : FirSessionComponent { useSiteFile: FirFile, containingDeclarations: List, dispatchReceiver: ReceiverValue?, - session: FirSession + session: FirSession, + isCallToPropertySetter: Boolean, ): Boolean private fun canSeePrivateMemberOf( diff --git a/compiler/testData/diagnostics/tests/properties/protectedGetterWithPublicSetter.fir.kt b/compiler/testData/diagnostics/tests/properties/protectedGetterWithPublicSetter.fir.kt new file mode 100644 index 00000000000..96fb4d9a38b --- /dev/null +++ b/compiler/testData/diagnostics/tests/properties/protectedGetterWithPublicSetter.fir.kt @@ -0,0 +1,30 @@ +// FILE: j/Super.java +package j + +public abstract class Super { + protected abstract String getName(); + public abstract void setName(String s); +} + +// FILE: k/test.kt + +package k +import j.Super + +abstract class Sub: Super() { + fun test(s: Super) { + s.name + s.getName() + s.name = "" + s.name = s.name + s.setName("") + } +} + +fun test(s: Super) { + s.name + s.getName() + s.name = "" + s.name = s.name + s.setName("") +} diff --git a/compiler/testData/diagnostics/tests/properties/protectedGetterWithPublicSetter.kt b/compiler/testData/diagnostics/tests/properties/protectedGetterWithPublicSetter.kt new file mode 100644 index 00000000000..44e1c1e1e0e --- /dev/null +++ b/compiler/testData/diagnostics/tests/properties/protectedGetterWithPublicSetter.kt @@ -0,0 +1,30 @@ +// FILE: j/Super.java +package j + +public abstract class Super { + protected abstract String getName(); + public abstract void setName(String s); +} + +// FILE: k/test.kt + +package k +import j.Super + +abstract class Sub: Super() { + fun test(s: Super) { + s.name + s.getName() + s.name = "" + s.name = s.name + s.setName("") + } +} + +fun test(s: Super) { + s.name + s.getName() + s.name = "" + s.name = s.name + s.setName("") +} diff --git a/compiler/testData/diagnostics/tests/properties/protectedGetterWithPublicSetter.txt b/compiler/testData/diagnostics/tests/properties/protectedGetterWithPublicSetter.txt new file mode 100644 index 00000000000..b8aeb94b676 --- /dev/null +++ b/compiler/testData/diagnostics/tests/properties/protectedGetterWithPublicSetter.txt @@ -0,0 +1,27 @@ +package + +package j { + + public abstract class Super { + public constructor Super() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + protected/*protected and package*/ abstract fun getName(): kotlin.String! + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public abstract fun setName(/*0*/ s: kotlin.String!): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } +} + +package k { + public fun test(/*0*/ s: j.Super): kotlin.Unit + + public abstract class Sub : j.Super { + public constructor Sub() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + protected/*protected and package*/ abstract override /*1*/ /*fake_override*/ fun getName(): kotlin.String! + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public abstract override /*1*/ /*fake_override*/ fun setName(/*0*/ s: kotlin.String!): kotlin.Unit + public final fun test(/*0*/ s: j.Super): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } +} diff --git a/compiler/testData/diagnostics/tests/syntheticExtensions/javaProperties/SetterHasHigherAccess.fir.kt b/compiler/testData/diagnostics/tests/syntheticExtensions/javaProperties/SetterHasHigherAccess.fir.kt index aa794ecb83e..7283894b885 100644 --- a/compiler/testData/diagnostics/tests/syntheticExtensions/javaProperties/SetterHasHigherAccess.fir.kt +++ b/compiler/testData/diagnostics/tests/syntheticExtensions/javaProperties/SetterHasHigherAccess.fir.kt @@ -5,8 +5,8 @@ import JavaClass fun foo(javaClass: JavaClass) { val v = javaClass.something - javaClass.something = 1 - javaClass.something++ + javaClass.something = 1 + javaClass.something++ } // FILE: JavaClass.java 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 19ccd31442d..dde9e34c310 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 @@ -22533,6 +22533,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest { runTest("compiler/testData/diagnostics/tests/properties/lateinitOnTopLevel.kt"); } + @Test + @TestMetadata("protectedGetterWithPublicSetter.kt") + public void testProtectedGetterWithPublicSetter() throws Exception { + runTest("compiler/testData/diagnostics/tests/properties/protectedGetterWithPublicSetter.kt"); + } + @Nested @TestMetadata("compiler/testData/diagnostics/tests/properties/backingField") @TestDataPath("$PROJECT_ROOT")