From 8d647fa01680e3f47b684282e0872b42c3e42229 Mon Sep 17 00:00:00 2001 From: Nikolay Lunyak Date: Tue, 19 Oct 2021 18:57:22 +0300 Subject: [PATCH] [FIR] KT-49265: Fix implicit visibility checks --- ...nosisCompilerFirTestdataTestGenerated.java | 6 + ...TouchedTilContractsPhaseTestGenerated.java | 5 + .../RedundantVisibilityDueToOverride.kt | 4 +- ...yModifierCheckerForInnerComponents.fir.txt | 153 ++++++++++++ ...bilityModifierCheckerForInnerComponents.kt | 77 ++++++ .../runners/FirDiagnosticTestGenerated.java | 6 + ...DiagnosticsWithLightTreeTestGenerated.java | 6 + ...edundantVisibilityModifierSyntaxChecker.kt | 235 ++++++++++++++---- .../FirStatusResolveTransformer.kt | 28 ++- .../resolve/transformers/FirStatusResolver.kt | 111 +++++---- 10 files changed, 535 insertions(+), 96 deletions(-) create mode 100644 compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierCheckerForInnerComponents.fir.txt create mode 100644 compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierCheckerForInnerComponents.kt diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerFirTestdataTestGenerated.java b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerFirTestdataTestGenerated.java index f446231187f..1f26fa6a3f0 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerFirTestdataTestGenerated.java +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerFirTestdataTestGenerated.java @@ -2285,6 +2285,12 @@ public class DiagnosisCompilerFirTestdataTestGenerated extends AbstractDiagnosis runTest("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierChecker.kt"); } + @Test + @TestMetadata("RedundantVisibilityModifierCheckerForInnerComponents.kt") + public void testRedundantVisibilityModifierCheckerForInnerComponents() throws Exception { + runTest("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierCheckerForInnerComponents.kt"); + } + @Nested @TestMetadata("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/canBeReplacedWithOperatorAssignment") @TestDataPath("$PROJECT_ROOT") diff --git a/compiler/fir/analysis-tests/legacy-fir-tests/tests-gen/org/jetbrains/kotlin/fir/LazyBodyIsNotTouchedTilContractsPhaseTestGenerated.java b/compiler/fir/analysis-tests/legacy-fir-tests/tests-gen/org/jetbrains/kotlin/fir/LazyBodyIsNotTouchedTilContractsPhaseTestGenerated.java index fedfa35fc62..7fe64522d07 100644 --- a/compiler/fir/analysis-tests/legacy-fir-tests/tests-gen/org/jetbrains/kotlin/fir/LazyBodyIsNotTouchedTilContractsPhaseTestGenerated.java +++ b/compiler/fir/analysis-tests/legacy-fir-tests/tests-gen/org/jetbrains/kotlin/fir/LazyBodyIsNotTouchedTilContractsPhaseTestGenerated.java @@ -2000,6 +2000,11 @@ public class LazyBodyIsNotTouchedTilContractsPhaseTestGenerated extends Abstract runTest("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierChecker.kt"); } + @TestMetadata("RedundantVisibilityModifierCheckerForInnerComponents.kt") + public void testRedundantVisibilityModifierCheckerForInnerComponents() throws Exception { + runTest("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierCheckerForInnerComponents.kt"); + } + @TestMetadata("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/canBeReplacedWithOperatorAssignment") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) diff --git a/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityDueToOverride.kt b/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityDueToOverride.kt index 06e0c081efd..aee7c8bf409 100644 --- a/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityDueToOverride.kt +++ b/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityDueToOverride.kt @@ -5,12 +5,12 @@ abstract class A { class C1 : A() { public override var attribute = super.attribute - public set + public set } abstract class B2 : A() { override var attribute = "b" - public set + public set } class C2 : B2() { diff --git a/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierCheckerForInnerComponents.fir.txt b/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierCheckerForInnerComponents.fir.txt new file mode 100644 index 00000000000..a3f116dc2d3 --- /dev/null +++ b/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierCheckerForInnerComponents.fir.txt @@ -0,0 +1,153 @@ +FILE: RedundantVisibilityModifierCheckerForInnerComponents.kt + public abstract class Foo : R|kotlin/Any| { + public constructor(): R|Foo| { + super() + } + + public abstract var id: R|kotlin/Int| + public get(): R|kotlin/Int| + protected set(value: R|kotlin/Int|): R|kotlin/Unit| + + protected open val a: R|kotlin/String| = String(test) + protected get(): R|kotlin/String| + + } + public final class Bar : R|Foo| { + public constructor(): R|Bar| { + super() + } + + public final override var id: R|kotlin/Int| = Int(1) + public get(): R|kotlin/Int| + public set(value: R|kotlin/Int|): R|kotlin/Unit| + + public final override val a: R|kotlin/String| = String(rest) + public get(): R|kotlin/String| + + } + public abstract class A1 : R|kotlin/Any| { + public constructor(): R|A1| { + super() + } + + public open var attribute: R|kotlin/String| = String(a) + public get(): R|kotlin/String| + protected set(value: R|kotlin/String|): R|kotlin/Unit| + + } + public final class C1 : R|A1| { + public constructor(): R|C1| { + super() + } + + public final override var attribute: R|kotlin/String| = this@R|/C1|.super.R|/A1.attribute| + public get(): R|kotlin/String| + public set(value: R|kotlin/String|): R|kotlin/Unit| + + } + public final fun test1(): R|kotlin/Unit| { + R|/C1.C1|().R|/C1.attribute| = String(c) + } + public abstract class A2 : R|kotlin/Any| { + public constructor(): R|A2| { + super() + } + + public open var attribute: R|kotlin/String| = String(a) + public get(): R|kotlin/String| + protected set(value: R|kotlin/String|): R|kotlin/Unit| + + } + public final class C2 : R|A2| { + public constructor(): R|C2| { + super() + } + + public final override var attribute: R|kotlin/String| = this@R|/C2|.super.R|/A2.attribute| + public get(): R|kotlin/String| + public set(value: R|kotlin/String|): R|kotlin/Unit| + + } + public abstract class A3 : R|kotlin/Any| { + public constructor(): R|A3| { + super() + } + + public open var attribute: R|kotlin/String| = String(a) + public get(): R|kotlin/String| + protected set(value: R|kotlin/String|): R|kotlin/Unit| + + } + public abstract class B3 : R|A3| { + public constructor(): R|B3| { + super() + } + + public open override var attribute: R|kotlin/String| = String(b) + public get(): R|kotlin/String| + protected set(value: R|kotlin/String|): R|kotlin/Unit| + + } + public final class C3 : R|B3| { + public constructor(): R|C3| { + super() + } + + public final override var attribute: R|kotlin/String| = this@R|/C3|.super.R|/B3.attribute| + public get(): R|kotlin/String| + public set(value: R|kotlin/String|): R|kotlin/Unit| + + } + public abstract class A4 : R|kotlin/Any| { + public constructor(): R|A4| { + super() + } + + public open var attribute: R|kotlin/String| = String(a) + public get(): R|kotlin/String| + protected set(value: R|kotlin/String|): R|kotlin/Unit| + + } + public abstract class B4 : R|A4| { + public constructor(): R|B4| { + super() + } + + public open override var attribute: R|kotlin/String| = String(b) + public get(): R|kotlin/String| + protected set(value: R|kotlin/String|): R|kotlin/Unit| + + } + public final class C4 : R|B4| { + public constructor(): R|C4| { + super() + } + + public final override var attribute: R|kotlin/String| = this@R|/C4|.super.R|/B4.attribute| + public get(): R|kotlin/String| + public set(value: R|kotlin/String|): R|kotlin/Unit| + + } + public abstract class A5 : R|kotlin/Any| { + public constructor(): R|A5| { + super() + } + + public open var attribute: R|kotlin/String| = String(a) + public get(): R|kotlin/String| + protected set(value: R|kotlin/String|): R|kotlin/Unit| + + } + public final class B5 : R|A5| { + public constructor(): R|B5| { + super() + } + + public final override var attribute: R|kotlin/String| = String(b) + public get(): R|kotlin/String| + protected set(value: R|kotlin/String|): R|kotlin/Unit| + + } + public final fun test5(): R|kotlin/Unit| { + R|/B5.B5|().R|/B5.attribute| = String(c) + } diff --git a/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierCheckerForInnerComponents.kt b/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierCheckerForInnerComponents.kt new file mode 100644 index 00000000000..e93414a8368 --- /dev/null +++ b/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierCheckerForInnerComponents.kt @@ -0,0 +1,77 @@ +abstract class Foo { + abstract var id: Int + protected set + + protected open val a = "test" +} + +class Bar : Foo() { + override var id: Int = 1 + public set + + public override val a = "rest" +} + +abstract class A1 { + open var attribute = "a" + protected set +} + +class C1 : A1() { + public override var attribute = super.attribute +} + +fun test1() { + C1().attribute = "c" +} + +abstract class A2 { + open var attribute = "a" + protected set +} + +class C2 : A2() { + public override var attribute = super.attribute + set +} + +abstract class A3 { + open var attribute = "a" + protected set +} + +abstract class B3 : A3() { + override var attribute = "b" +} + +class C3 : B3() { + public override var attribute = super.attribute +} + +abstract class A4 { + open var attribute = "a" + protected set +} + +abstract class B4 : A4() { + override var attribute = "b" + set +} + +class C4 : B4() { + public override var attribute = super.attribute +} + +abstract class A5 { + open var attribute = "a" + protected set +} + +class B5 : A5() { + override var attribute = "b" + set +} + +fun test5() { + B5().attribute = "c" +} diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticTestGenerated.java index 1b3d221b651..6cade97e2d7 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticTestGenerated.java @@ -2285,6 +2285,12 @@ public class FirDiagnosticTestGenerated extends AbstractFirDiagnosticTest { runTest("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierChecker.kt"); } + @Test + @TestMetadata("RedundantVisibilityModifierCheckerForInnerComponents.kt") + public void testRedundantVisibilityModifierCheckerForInnerComponents() throws Exception { + runTest("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierCheckerForInnerComponents.kt"); + } + @Nested @TestMetadata("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/canBeReplacedWithOperatorAssignment") @TestDataPath("$PROJECT_ROOT") diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticsWithLightTreeTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticsWithLightTreeTestGenerated.java index fc20616fc73..225cfa0e674 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticsWithLightTreeTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticsWithLightTreeTestGenerated.java @@ -2285,6 +2285,12 @@ public class FirDiagnosticsWithLightTreeTestGenerated extends AbstractFirDiagnos runTest("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierChecker.kt"); } + @Test + @TestMetadata("RedundantVisibilityModifierCheckerForInnerComponents.kt") + public void testRedundantVisibilityModifierCheckerForInnerComponents() throws Exception { + runTest("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierCheckerForInnerComponents.kt"); + } + @Nested @TestMetadata("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/canBeReplacedWithOperatorAssignment") @TestDataPath("$PROJECT_ROOT") diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantVisibilityModifierSyntaxChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantVisibilityModifierSyntaxChecker.kt index 777e00402e3..3551e92ac6b 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantVisibilityModifierSyntaxChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantVisibilityModifierSyntaxChecker.kt @@ -5,22 +5,17 @@ package org.jetbrains.kotlin.fir.analysis.checkers.extended -import org.jetbrains.kotlin.KtFakeSourceElement import org.jetbrains.kotlin.KtFakeSourceElementKind import org.jetbrains.kotlin.KtSourceElement import org.jetbrains.kotlin.descriptors.Visibilities import org.jetbrains.kotlin.descriptors.Visibility +import org.jetbrains.kotlin.fir.analysis.checkers.* import org.jetbrains.kotlin.diagnostics.DiagnosticReporter -import org.jetbrains.kotlin.diagnostics.overrideModifier import org.jetbrains.kotlin.diagnostics.reportOn import org.jetbrains.kotlin.diagnostics.visibilityModifier 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.findClosestClassOrObject -import org.jetbrains.kotlin.fir.analysis.checkers.getContainingClassSymbol -import org.jetbrains.kotlin.fir.analysis.checkers.overriddenFunctions import org.jetbrains.kotlin.fir.analysis.checkers.syntax.FirDeclarationSyntaxChecker -import org.jetbrains.kotlin.fir.analysis.checkers.toVisibilityOrNull import org.jetbrains.kotlin.fir.analysis.diagnostics.* import org.jetbrains.kotlin.fir.declarations.* import org.jetbrains.kotlin.fir.declarations.utils.* @@ -28,10 +23,8 @@ import org.jetbrains.kotlin.fir.declarations.utils.isEnumClass import org.jetbrains.kotlin.fir.declarations.utils.isOverride import org.jetbrains.kotlin.fir.declarations.utils.isSealed import org.jetbrains.kotlin.fir.declarations.utils.visibility -import org.jetbrains.kotlin.fir.resolve.ScopeSession -import org.jetbrains.kotlin.fir.scopes.getDirectOverriddenFunctions -import org.jetbrains.kotlin.fir.scopes.getDirectOverriddenProperties -import org.jetbrains.kotlin.fir.scopes.unsubstitutedScope +import org.jetbrains.kotlin.fir.scopes.* +import org.jetbrains.kotlin.fir.symbols.impl.FirCallableSymbol import org.jetbrains.kotlin.fir.symbols.impl.FirRegularClassSymbol import org.jetbrains.kotlin.lexer.KtModifierKeywordToken import org.jetbrains.kotlin.psi.KtDeclaration @@ -44,50 +37,136 @@ object RedundantVisibilityModifierSyntaxChecker : FirDeclarationSyntaxChecker() + if (element is FirConstructor && element.source?.kind is KtFakeSourceElementKind) { + return + } + when (element) { + is FirProperty -> checkPropertyAndReport(element, context, reporter) + else -> checkElementAndReport(element, context, reporter) + } + } + + private fun checkPropertyAndReport( + property: FirProperty, + context: CheckerContext, + reporter: DiagnosticReporter + ) { + var setterImplicitVisibility: Visibility? = null + + context.withDeclaration(property) { + property.setter?.let { setter -> + val visibility = setter.implicitVisibility(it) + setterImplicitVisibility = visibility + checkElementAndReport(setter, visibility, property, it, reporter) + } + + property.getter?.let { getter -> + checkElementAndReport(getter, property, it, reporter) + } + + property.backingField?.let { field -> + checkElementAndReport(field, property, it, reporter) + } + } + + if (property.canMakeSetterMoreAccessible(setterImplicitVisibility)) { + return + } + + checkElementAndReport(property, context, reporter) + } + + private fun checkElementAndReport( + element: FirDeclaration, + context: CheckerContext, + reporter: DiagnosticReporter + ) = checkElementAndReport( + element, + context.findClosest(), + context, + reporter + ) + + private fun checkElementAndReport( + element: FirDeclaration, + containingMemberDeclaration: FirMemberDeclaration?, + context: CheckerContext, + reporter: DiagnosticReporter + ) = checkElementAndReport( + element, + element.implicitVisibility(context), + containingMemberDeclaration, + context, + reporter + ) + + private fun checkElementAndReport( + element: FirDeclaration, + implicitVisibility: Visibility, + containingMemberDeclaration: FirMemberDeclaration?, + context: CheckerContext, + reporter: DiagnosticReporter + ) { + if (element.source?.kind is KtFakeSourceElementKind) { + return + } + + val isAccessorWithSameVisibility = element is FirPropertyAccessor + && element.visibility == context.containingPropertyVisibility + + if (element !is FirMemberDeclaration && !isAccessorWithSameVisibility) { + return + } + + val explicitVisibility = element.source?.explicitVisibility val isHidden = explicitVisibility.isEffectivelyHiddenBy(containingMemberDeclaration) if (explicitVisibility != implicitVisibility && !isHidden) { return } - if (element.isPublicOverriddenWithNonPublicBase(containingMemberDeclaration, context)) { - return - } - - reporter.reportOn(source, FirErrors.REDUNDANT_VISIBILITY_MODIFIER, context) + reporter.reportOn(element.source, FirErrors.REDUNDANT_VISIBILITY_MODIFIER, context) } - private fun FirDeclaration.isPublicOverriddenWithNonPublicBase( - container: FirMemberDeclaration?, - context: CheckerContext, - ): Boolean { - if (container !is FirClass) { + private fun FirProperty.canMakeSetterMoreAccessible(setterImplicitVisibility: Visibility?): Boolean { + if (!isOverride) { return false } - val scope = container.unsubstitutedScope(context.session, ScopeSession(), false) - - val overridden = when (this) { - is FirProperty -> scope.getDirectOverriddenProperties(symbol) - is FirSimpleFunction -> scope.getDirectOverriddenFunctions(symbol) - else -> return false + if (!hasSetterWithImplicitVisibility) { + return false } - return overridden.any { it.visibility != Visibilities.Public } + if (setterImplicitVisibility == null) { + return false + } + + return setterImplicitVisibility != visibility } + private val FirProperty.hasSetterWithImplicitVisibility: Boolean + get() { + val theSetter = setter ?: return false + + if (source?.lighterASTNode == theSetter.source?.lighterASTNode) { + return true + } + + val theSource = theSetter.source ?: return true + return theSource.explicitVisibility == null + } + + private val KtSourceElement.explicitVisibility: Visibility? + get() { + val visibilityModifier = treeStructure.visibilityModifier(lighterASTNode) + return (visibilityModifier?.tokenType as? KtModifierKeywordToken)?.toVisibilityOrNull() + } + private fun Visibility?.isEffectivelyHiddenBy(declaration: FirMemberDeclaration?): Boolean { val containerVisibility = declaration?.effectiveVisibility?.toVisibility() ?: return false @@ -101,7 +180,11 @@ object RedundantVisibilityModifierSyntaxChecker : FirDeclarationSyntaxChecker this.visibility + this is FirPropertyAccessor + && isSetter + && context.containingDeclarations.size >= 2 + && context.containingDeclarations.asReversed()[1] is FirClass + && propertySymbol?.isOverride == true -> findPropertyAccessorVisibility(this, context) this is FirPropertyAccessor -> { context.findClosest()?.visibility ?: Visibilities.DEFAULT_VISIBILITY @@ -123,23 +206,79 @@ object RedundantVisibilityModifierSyntaxChecker : FirDeclarationSyntaxChecker findFunctionVisibility(this, context) + this is FirProperty + && context.containingDeclarations.last() is FirClass + && this.isOverride -> findPropertyVisibility(this, context) + else -> Visibilities.DEFAULT_VISIBILITY } } + private fun findBiggestVisibility( + processSymbols: ((FirCallableSymbol<*>) -> ProcessorAction) -> Unit + ): Visibility { + var current: Visibility = Visibilities.Private + + processSymbols { + val difference = Visibilities.compare(current, it.visibility) + + if (difference != null && difference < 0) { + current = it.visibility + } + + ProcessorAction.NEXT + } + + return current + } + + private fun findPropertyAccessorVisibility(accessor: FirPropertyAccessor, context: CheckerContext): Visibility { + val containingClass = context.findClosestClassOrObject()?.symbol ?: return Visibilities.Public + val propertySymbol = accessor.propertySymbol ?: return Visibilities.Public + + val scope = containingClass.unsubstitutedScope( + context.sessionHolder.session, + context.sessionHolder.scopeSession, + withForcedTypeCalculator = false + ) + + return findBiggestVisibility { checkVisibility -> + scope.processPropertiesByName(propertySymbol.name) {} + scope.processOverriddenProperties(propertySymbol) { property -> + val setter = property.setterSymbol ?: return@processOverriddenProperties ProcessorAction.NEXT + checkVisibility(setter) + } + } + } + + private fun findPropertyVisibility(property: FirProperty, context: CheckerContext): Visibility { + val containingClass = context.findClosestClassOrObject()?.symbol ?: return Visibilities.Public + + val scope = containingClass.unsubstitutedScope( + context.sessionHolder.session, + context.sessionHolder.scopeSession, + withForcedTypeCalculator = false + ) + + return findBiggestVisibility { + scope.processPropertiesByName(property.symbol.name) {} + scope.processOverriddenProperties(property.symbol, it) + } + } + private fun findFunctionVisibility(function: FirSimpleFunction, context: CheckerContext): Visibility { val currentClassSymbol = context.findClosestClassOrObject()?.symbol ?: return Visibilities.Unknown - val overriddenFunctions = function.overriddenFunctions(currentClassSymbol, context) - var visibility: Visibility = Visibilities.Private - for (func in overriddenFunctions) { - val currentVisibility = func.visibility - val compareResult = Visibilities.compare(currentVisibility, visibility) - if (compareResult != null && compareResult > 0) { - visibility = currentVisibility - } - } - return visibility + val scope = currentClassSymbol.unsubstitutedScope( + context.sessionHolder.session, + context.sessionHolder.scopeSession, + withForcedTypeCalculator = false + ) + + return findBiggestVisibility { + scope.processFunctionsByName(function.symbol.name) {} + scope.processOverriddenFunctions(function.symbol, it) + } } private val CheckerContext.containingPropertyVisibility diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirStatusResolveTransformer.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirStatusResolveTransformer.kt index 14be577d08c..b74a077394e 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirStatusResolveTransformer.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirStatusResolveTransformer.kt @@ -442,10 +442,17 @@ abstract class AbstractFirStatusResolveTransformer( private fun transformPropertyAccessor( propertyAccessor: FirPropertyAccessor, containingProperty: FirProperty, + overriddenStatuses: List = emptyList(), ) { propertyAccessor.transformStatus( this, - statusResolver.resolveStatus(propertyAccessor, containingClass, containingProperty, isLocal = false) + statusResolver.resolveStatus( + propertyAccessor, + containingClass, + containingProperty, + isLocal = false, + overriddenStatuses, + ) ) propertyAccessor.transformValueParameters(this, null) @@ -474,10 +481,25 @@ abstract class AbstractFirStatusResolveTransformer( property: FirProperty, data: FirResolvedDeclarationStatus? ): FirStatement { - property.transformStatus(this, statusResolver.resolveStatus(property, containingClass, isLocal = false)) + val overridden = statusResolver.getOverriddenProperties(property, containingClass) + + val overriddenProperties = overridden.map { + it.ensureResolved(FirResolvePhase.STATUS) + it.status as FirResolvedDeclarationStatus + } + + val overriddenSetters = overridden.mapNotNull { + it.setter?.ensureResolved(FirResolvePhase.STATUS) + it.setter?.status as? FirResolvedDeclarationStatus + } + + property.transformStatus( + this, + statusResolver.resolveStatus(property, containingClass, false, overriddenProperties) + ) property.getter?.let { transformPropertyAccessor(it, property) } - property.setter?.let { transformPropertyAccessor(it, property) } + property.setter?.let { transformPropertyAccessor(it, property, overriddenSetters) } property.backingField?.let { it.transformStatus( diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirStatusResolver.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirStatusResolver.kt index 09e0c077c18..93851c6f036 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirStatusResolver.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirStatusResolver.kt @@ -6,7 +6,7 @@ package org.jetbrains.kotlin.fir.resolve.transformers import org.jetbrains.kotlin.descriptors.* -import org.jetbrains.kotlin.fir.FirSession +import org.jetbrains.kotlin.fir.* import org.jetbrains.kotlin.fir.declarations.* import org.jetbrains.kotlin.fir.declarations.impl.FirDeclarationStatusImpl import org.jetbrains.kotlin.fir.declarations.impl.FirResolvedDeclarationStatusImpl @@ -14,9 +14,7 @@ import org.jetbrains.kotlin.fir.declarations.utils.effectiveVisibility import org.jetbrains.kotlin.fir.declarations.utils.isExpect import org.jetbrains.kotlin.fir.declarations.utils.isOverride import org.jetbrains.kotlin.fir.declarations.utils.visibility -import org.jetbrains.kotlin.fir.extensions.FirStatusTransformerExtension -import org.jetbrains.kotlin.fir.extensions.extensionService -import org.jetbrains.kotlin.fir.extensions.statusTransformerExtensions +import org.jetbrains.kotlin.fir.extensions.* import org.jetbrains.kotlin.fir.render import org.jetbrains.kotlin.fir.resolve.ScopeSession import org.jetbrains.kotlin.fir.scopes.ProcessorAction @@ -83,43 +81,69 @@ class FirStatusResolver( } @OptIn(ExperimentalStdlibApi::class) - fun resolveStatus(property: FirProperty, containingClass: FirClass?, isLocal: Boolean): FirResolvedDeclarationStatus { - val status = property.applyExtensionTransformers { transformStatus(it, property, containingClass, isLocal) } - return resolveStatus(property, status, containingClass, null, isLocal) l@{ - if (containingClass == null) return@l emptyList() - @Suppress("RemoveExplicitTypeArguments") // Workaround for KT-42175 - buildList { - val scope = containingClass.unsubstitutedScope(session, scopeSession, withForcedTypeCalculator = false) - scope.processPropertiesByName(property.name) {} - scope.processDirectOverriddenPropertiesWithBaseScope(property.symbol) { symbol, _ -> - this += symbol.fir - ProcessorAction.NEXT - } - }.map { + fun getOverriddenProperties( + property: FirProperty, + containingClass: FirClass?, + ): List { + if (containingClass == null) { + return emptyList() + } + + val scope = containingClass.unsubstitutedScope(session, scopeSession, withForcedTypeCalculator = false) + + return buildList { + scope.processPropertiesByName(property.name) {} + scope.processDirectOverriddenPropertiesWithBaseScope(property.symbol) { symbol, _ -> + this += symbol.fir + ProcessorAction.NEXT + } + } + } + + @OptIn(ExperimentalStdlibApi::class) + fun resolveStatus( + property: FirProperty, + containingClass: FirClass?, + isLocal: Boolean, + overriddenStatuses: List? = null, + ): FirResolvedDeclarationStatus { + val statuses = overriddenStatuses ?: getOverriddenProperties(property, containingClass) + .map { it.ensureResolved(FirResolvePhase.STATUS) it.status as FirResolvedDeclarationStatus } + + val status = property.applyExtensionTransformers { transformStatus(it, property, containingClass, isLocal) } + return resolveStatus(property, status, containingClass, null, isLocal, statuses) + } + + private fun getOverriddenStatuses( + function: FirSimpleFunction, + containingClass: FirClass? + ): List { + if (containingClass == null) { + return emptyList() + } + + @Suppress("RemoveExplicitTypeArguments") // Workaround for KT-42175 + return buildList { + val scope = containingClass.unsubstitutedScope(session, scopeSession, withForcedTypeCalculator = false) + val symbol = function.symbol + scope.processFunctionsByName(function.name) {} + scope.processDirectOverriddenFunctionsWithBaseScope(symbol) { overriddenSymbol, _ -> + this += overriddenSymbol.fir + ProcessorAction.NEXT + } + }.mapNotNull { + it.status as? FirResolvedDeclarationStatus } } @OptIn(ExperimentalStdlibApi::class) fun resolveStatus(function: FirSimpleFunction, containingClass: FirClass?, isLocal: Boolean): FirResolvedDeclarationStatus { val status = function.applyExtensionTransformers { transformStatus(it, function, containingClass, isLocal) } - return resolveStatus(function, status, containingClass, null, isLocal) l@{ - if (containingClass == null) return@l emptyList() - @Suppress("RemoveExplicitTypeArguments") // Workaround for KT-42175 - buildList { - val scope = containingClass.unsubstitutedScope(session, scopeSession, withForcedTypeCalculator = false) - val symbol = function.symbol - scope.processFunctionsByName(function.name) {} - scope.processDirectOverriddenFunctionsWithBaseScope(symbol) { overriddenSymbol, _ -> - this += overriddenSymbol.fir - ProcessorAction.NEXT - } - }.mapNotNull { - it.status as? FirResolvedDeclarationStatus - } - } + val overriddenStatuses = getOverriddenStatuses(function, containingClass) + return resolveStatus(function, status, containingClass, null, isLocal, overriddenStatuses) } fun resolveStatus( @@ -128,7 +152,7 @@ class FirStatusResolver( isLocal: Boolean ): FirResolvedDeclarationStatus { val status = regularClass.applyExtensionTransformers { transformStatus(it, regularClass, containingClass, isLocal) } - return resolveStatus(regularClass, status, containingClass, null, isLocal) { emptyList() } + return resolveStatus(regularClass, status, containingClass, null, isLocal, emptyList()) } fun resolveStatus( @@ -137,28 +161,29 @@ class FirStatusResolver( isLocal: Boolean ): FirResolvedDeclarationStatus { val status = typeAlias.applyExtensionTransformers { transformStatus(it, typeAlias, containingClass, isLocal) } - return resolveStatus(typeAlias, status, containingClass, null, isLocal) { emptyList() } + return resolveStatus(typeAlias, status, containingClass, null, isLocal, emptyList()) } fun resolveStatus( propertyAccessor: FirPropertyAccessor, containingClass: FirClass?, containingProperty: FirProperty?, - isLocal: Boolean + isLocal: Boolean, + overriddenStatuses: List = emptyList(), ): FirResolvedDeclarationStatus { val status = propertyAccessor.applyExtensionTransformers { transformStatus(it, propertyAccessor, containingClass, containingProperty, isLocal) } - return resolveStatus(propertyAccessor, status, containingClass, containingProperty, isLocal) { emptyList() } + return resolveStatus(propertyAccessor, status, containingClass, containingProperty, isLocal, overriddenStatuses) } fun resolveStatus(constructor: FirConstructor, containingClass: FirClass?, isLocal: Boolean): FirResolvedDeclarationStatus { val status = constructor.applyExtensionTransformers { transformStatus(it, constructor, containingClass, isLocal) } - return resolveStatus(constructor, status, containingClass, null, isLocal) { emptyList() } + return resolveStatus(constructor, status, containingClass, null, isLocal, emptyList()) } fun resolveStatus(field: FirField, containingClass: FirClass?, isLocal: Boolean): FirResolvedDeclarationStatus { - return resolveStatus(field, field.status, containingClass, null, isLocal) { emptyList() } + return resolveStatus(field, field.status, containingClass, null, isLocal, emptyList()) } fun resolveStatus( @@ -167,27 +192,27 @@ class FirStatusResolver( isLocal: Boolean ): FirResolvedDeclarationStatus { val status = backingField.applyExtensionTransformers { transformStatus(it, backingField, containingClass, isLocal) } - return resolveStatus(backingField, status, containingClass, null, isLocal) { emptyList() } + return resolveStatus(backingField, status, containingClass, null, isLocal, emptyList()) } fun resolveStatus(enumEntry: FirEnumEntry, containingClass: FirClass?, isLocal: Boolean): FirResolvedDeclarationStatus { val status = enumEntry.applyExtensionTransformers { transformStatus(it, enumEntry, containingClass, isLocal) } - return resolveStatus(enumEntry, status, containingClass, null, isLocal) { emptyList() } + return resolveStatus(enumEntry, status, containingClass, null, isLocal, emptyList()) } - private inline fun resolveStatus( + private fun resolveStatus( declaration: FirDeclaration, status: FirDeclarationStatus, containingClass: FirClass?, containingProperty: FirProperty?, isLocal: Boolean, - overriddenExtractor: () -> List + overriddenStatuses: List ): FirResolvedDeclarationStatus { if (status is FirResolvedDeclarationStatus) return status require(status is FirDeclarationStatusImpl) @Suppress("UNCHECKED_CAST") - val overriddenStatuses = overriddenExtractor() as List + overriddenStatuses as List val visibility = when (status.visibility) { Visibilities.Unknown -> when { isLocal -> Visibilities.Local