From 5186ba80e260f77eff73569e3b84fc92f8650bc0 Mon Sep 17 00:00:00 2001 From: Kirill Rakhman Date: Mon, 18 Dec 2023 11:42:29 +0100 Subject: [PATCH] [FIR] Check for setter visibility in resolution stage This is required for the following commit where candidates from the original scope in presence of smart cast will not be ignored if they're the same symbol. --- ...yModifierCheckerForInnerComponents.fir.txt | 2 +- ...irReassignmentAndInvisibleSetterChecker.kt | 56 ------------------- .../coneDiagnosticToFirDiagnostic.kt | 6 ++ .../kotlin/fir/resolve/ResolveUtils.kt | 11 +++- .../fir/resolve/calls/ResolutionStages.kt | 13 +++-- .../fir/resolve/calls/ResolutionDiagnostic.kt | 2 + .../resolve/diagnostics/ConeDiagnostics.kt | 6 ++ .../tests/privateInFile/visibility.fir.kt | 2 +- ...ectedWithGenericsInDifferentPackage.fir.kt | 2 +- .../diagnostics/tests/scopes/kt1244.fir.kt | 2 +- .../diagnostics/tests/scopes/kt1738.fir.kt | 2 +- .../protectedVisibility/withSmartcast.fir.kt | 2 +- .../tests/scopes/visibility.fir.kt | 2 +- .../tests/scopes/visibility3.fir.kt | 2 +- .../variance/privateToThis/SetVar.fir.kt | 10 ++-- 15 files changed, 44 insertions(+), 76 deletions(-) diff --git a/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierCheckerForInnerComponents.fir.txt b/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierCheckerForInnerComponents.fir.txt index 3fd976aa244..ce0aa4d0cde 100644 --- a/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierCheckerForInnerComponents.fir.txt +++ b/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierCheckerForInnerComponents.fir.txt @@ -149,5 +149,5 @@ FILE: RedundantVisibilityModifierCheckerForInnerComponents.kt } public final fun test5(): R|kotlin/Unit| { - R|/B5.B5|().R|/B5.attribute| = String(c) + R|/B5.B5|().R|/B5.attribute#| = String(c) } 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 5f02915e399..0b48b634ef0 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 @@ -5,7 +5,6 @@ package org.jetbrains.kotlin.fir.analysis.checkers.expression -import org.jetbrains.kotlin.descriptors.Visibilities import org.jetbrains.kotlin.diagnostics.DiagnosticReporter import org.jetbrains.kotlin.diagnostics.reportOn import org.jetbrains.kotlin.fir.analysis.cfa.evaluatedInPlace @@ -15,78 +14,23 @@ import org.jetbrains.kotlin.fir.analysis.checkers.context.findClosest import org.jetbrains.kotlin.fir.analysis.checkers.getContainingSymbol import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors import org.jetbrains.kotlin.fir.declarations.* -import org.jetbrains.kotlin.fir.declarations.utils.visibility import org.jetbrains.kotlin.fir.diagnostics.ConeSimpleDiagnostic import org.jetbrains.kotlin.fir.diagnostics.DiagnosticKind import org.jetbrains.kotlin.fir.expressions.* -import org.jetbrains.kotlin.fir.originalForSubstitutionOverride import org.jetbrains.kotlin.fir.references.* import org.jetbrains.kotlin.fir.resolve.dfa.controlFlowGraph import org.jetbrains.kotlin.fir.resolve.diagnostics.ConeDiagnosticWithCandidates import org.jetbrains.kotlin.fir.resolve.diagnostics.ConeUnresolvedNameError -import org.jetbrains.kotlin.fir.symbols.SymbolInternals import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol -import org.jetbrains.kotlin.fir.types.resolvedType -import org.jetbrains.kotlin.fir.types.toRegularClassSymbol -import org.jetbrains.kotlin.fir.visibilityChecker object FirReassignmentAndInvisibleSetterChecker : FirVariableAssignmentChecker() { override fun check(expression: FirVariableAssignment, context: CheckerContext, reporter: DiagnosticReporter) { - checkInvisibleSetter(expression, context, reporter) checkValReassignmentViaBackingField(expression, context, reporter) checkValReassignmentOnValueParameter(expression, context, reporter) checkVariableExpected(expression, context, reporter) checkValReassignment(expression, context, reporter) } - private fun checkInvisibleSetter( - expression: FirVariableAssignment, - context: CheckerContext, - reporter: DiagnosticReporter - ) { - fun shouldInvisibleSetterBeReported(symbol: FirPropertySymbol): Boolean { - @OptIn(SymbolInternals::class) - val setterFir = symbol.setterSymbol?.fir ?: symbol.originalForSubstitutionOverride?.setterSymbol?.fir - if (setterFir != null) { - return !context.session.visibilityChecker.isVisible( - setterFir, - context.session, - context.findClosest()!!, - context.containingDeclarations, - expression.dispatchReceiver, - ) - } - - return false - } - - val callableSymbol = expression.calleeReference?.toResolvedCallableSymbol() - if (callableSymbol is FirPropertySymbol && shouldInvisibleSetterBeReported(callableSymbol)) { - val explicitReceiver = expression.explicitReceiver - // Try to get type from smartcast - if (explicitReceiver is FirSmartCastExpression) { - val symbol = explicitReceiver.originalExpression.resolvedType.toRegularClassSymbol(context.session) - if (symbol != null) { - for (declarationSymbol in symbol.declarationSymbols) { - if (declarationSymbol is FirPropertySymbol && declarationSymbol.name == callableSymbol.name) { - if (!shouldInvisibleSetterBeReported(declarationSymbol)) { - return - } - } - } - } - } - reporter.reportOn( - expression.lValue.source, - FirErrors.INVISIBLE_SETTER, - callableSymbol, - callableSymbol.setterSymbol?.visibility ?: Visibilities.Private, - callableSymbol.callableId, - context - ) - } - } - private fun checkValReassignmentViaBackingField( expression: FirVariableAssignment, context: CheckerContext, diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/coneDiagnosticToFirDiagnostic.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/coneDiagnosticToFirDiagnostic.kt index 0748e0e9305..69fa4dd7fb4 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/coneDiagnosticToFirDiagnostic.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/coneDiagnosticToFirDiagnostic.kt @@ -96,6 +96,12 @@ private fun ConeDiagnostic.toKtDiagnostic( } is ConeTypeVisibilityError -> symbol.toInvisibleReferenceDiagnostic(smallestUnresolvablePrefix.last().source) + is ConeSetterVisibilityError -> FirErrors.INVISIBLE_SETTER.createOn( + source, + symbol, + symbol.setterSymbol?.visibility ?: symbol.visibility, + symbol.callableId + ) is ConeVisibilityError -> symbol.toInvisibleReferenceDiagnostic(source) is ConeInapplicableWrongReceiver -> when (val diagnostic = primaryDiagnostic) { is DynamicReceiverExpectedButWasNonDynamic -> diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/ResolveUtils.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/ResolveUtils.kt index e9a421034e6..6b643f5cdc3 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/ResolveUtils.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/ResolveUtils.kt @@ -578,11 +578,12 @@ fun createConeDiagnosticForCandidateWithError( applicability: CandidateApplicability, candidate: Candidate, ): ConeDiagnostic { + val symbol = candidate.symbol return when (applicability) { CandidateApplicability.HIDDEN -> ConeHiddenCandidateError(candidate) CandidateApplicability.K2_VISIBILITY_ERROR -> { val session = candidate.callInfo.session - val declaration = candidate.symbol.fir + val declaration = symbol.fir if (declaration is FirMemberDeclaration && session.visibilityChecker.isVisible(declaration, candidate, skipCheckForContainingClassVisibility = true) ) { @@ -604,13 +605,17 @@ fun createConeDiagnosticForCandidateWithError( return ConeVisibilityError(it.symbol) } } - ConeVisibilityError(candidate.symbol) + if (symbol is FirPropertySymbol && SetterVisibilityError in candidate.diagnostics) { + ConeSetterVisibilityError(symbol) + } else { + ConeVisibilityError(symbol) + } } CandidateApplicability.INAPPLICABLE_WRONG_RECEIVER -> ConeInapplicableWrongReceiver(listOf(candidate)) CandidateApplicability.K2_NO_COMPANION_OBJECT -> ConeNoCompanionObject(candidate) else -> { if (TypeParameterAsExpression in candidate.diagnostics) { - ConeTypeParameterInQualifiedAccess(candidate.symbol as FirTypeParameterSymbol) + ConeTypeParameterInQualifiedAccess(symbol as FirTypeParameterSymbol) } else { ConeInapplicableCandidateError(applicability, candidate) } diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/ResolutionStages.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/ResolutionStages.kt index 14034b35076..fcb98768f67 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/ResolutionStages.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/ResolutionStages.kt @@ -7,12 +7,10 @@ package org.jetbrains.kotlin.fir.resolve.calls import org.jetbrains.kotlin.config.LanguageFeature import org.jetbrains.kotlin.descriptors.Modality -import org.jetbrains.kotlin.fir.FirElement +import org.jetbrains.kotlin.fir.* import org.jetbrains.kotlin.fir.declarations.* import org.jetbrains.kotlin.fir.declarations.utils.* import org.jetbrains.kotlin.fir.expressions.* -import org.jetbrains.kotlin.fir.languageVersionSettings -import org.jetbrains.kotlin.fir.matchingParameterFunctionType import org.jetbrains.kotlin.fir.references.FirSuperReference import org.jetbrains.kotlin.fir.references.FirThisReference import org.jetbrains.kotlin.fir.resolve.BodyResolveComponents @@ -33,7 +31,6 @@ import org.jetbrains.kotlin.fir.symbols.SyntheticSymbol import org.jetbrains.kotlin.fir.symbols.impl.* import org.jetbrains.kotlin.fir.symbols.lazyResolveToPhase import org.jetbrains.kotlin.fir.types.* -import org.jetbrains.kotlin.fir.visibilityChecker import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.resolve.calls.inference.isSubtypeConstraintCompatible import org.jetbrains.kotlin.resolve.calls.inference.model.ConstraintKind @@ -611,6 +608,14 @@ internal object CheckVisibility : CheckerStage() { sink.yieldDiagnostic(VisibilityError) return } + + if (symbol is FirPropertySymbol && callInfo.callSite is FirVariableAssignment) { + val setterFir = symbol.setterSymbol?.fir ?: symbol.originalForSubstitutionOverride?.setterSymbol?.fir + if (setterFir != null && !visibilityChecker.isVisible(setterFir, candidate)) { + sink.yieldDiagnostic(SetterVisibilityError) + return + } + } } if (declaration is FirConstructor) { diff --git a/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/calls/ResolutionDiagnostic.kt b/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/calls/ResolutionDiagnostic.kt index 7d5830d45cc..e1854f38bc4 100644 --- a/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/calls/ResolutionDiagnostic.kt +++ b/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/calls/ResolutionDiagnostic.kt @@ -86,6 +86,8 @@ object HiddenCandidate : ResolutionDiagnostic(HIDDEN) object VisibilityError : ResolutionDiagnostic(K2_VISIBILITY_ERROR) +object SetterVisibilityError : ResolutionDiagnostic(K2_VISIBILITY_ERROR) + object ResolvedWithLowPriority : ResolutionDiagnostic(RESOLVED_LOW_PRIORITY) object ResolvedWithSynthetic : ResolutionDiagnostic(K2_SYNTHETIC_RESOLVED) diff --git a/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/diagnostics/ConeDiagnostics.kt b/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/diagnostics/ConeDiagnostics.kt index 94cc501a127..d97ed331fc9 100644 --- a/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/diagnostics/ConeDiagnostics.kt +++ b/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/diagnostics/ConeDiagnostics.kt @@ -111,6 +111,12 @@ open class ConeVisibilityError( override val reason: String get() = "HIDDEN: ${describeSymbol(symbol)} is invisible" } +open class ConeSetterVisibilityError( + override val symbol: FirPropertySymbol +) : ConeDiagnosticWithSymbol> { + override val reason: String get() = "HIDDEN SETTER: ${describeSymbol(symbol)} is invisible" +} + class ConeTypeVisibilityError( symbol: FirBasedSymbol<*>, val smallestUnresolvablePrefix: List, diff --git a/compiler/testData/diagnostics/tests/privateInFile/visibility.fir.kt b/compiler/testData/diagnostics/tests/privateInFile/visibility.fir.kt index c295655fc0d..9fb008ec5e4 100644 --- a/compiler/testData/diagnostics/tests/privateInFile/visibility.fir.kt +++ b/compiler/testData/diagnostics/tests/privateInFile/visibility.fir.kt @@ -35,7 +35,7 @@ fun test() { val u : A = A() val z = x - x = 30 + x = 30 val po = PO diff --git a/compiler/testData/diagnostics/tests/protectedWithGenericsInDifferentPackage.fir.kt b/compiler/testData/diagnostics/tests/protectedWithGenericsInDifferentPackage.fir.kt index d945df12f2c..9a701133163 100644 --- a/compiler/testData/diagnostics/tests/protectedWithGenericsInDifferentPackage.fir.kt +++ b/compiler/testData/diagnostics/tests/protectedWithGenericsInDifferentPackage.fir.kt @@ -26,7 +26,7 @@ package foo fun test(s: bar.Sub) { s.name - s.name = "" + s.name = "" s.name2 s.name2 = "" s.doSomething() diff --git a/compiler/testData/diagnostics/tests/scopes/kt1244.fir.kt b/compiler/testData/diagnostics/tests/scopes/kt1244.fir.kt index ea017d6e151..39b2d25e557 100644 --- a/compiler/testData/diagnostics/tests/scopes/kt1244.fir.kt +++ b/compiler/testData/diagnostics/tests/scopes/kt1244.fir.kt @@ -8,6 +8,6 @@ class A { class B() { init { - A().a = "Hello" + A().a = "Hello" } } diff --git a/compiler/testData/diagnostics/tests/scopes/kt1738.fir.kt b/compiler/testData/diagnostics/tests/scopes/kt1738.fir.kt index bea22005df0..5d610889436 100644 --- a/compiler/testData/diagnostics/tests/scopes/kt1738.fir.kt +++ b/compiler/testData/diagnostics/tests/scopes/kt1738.fir.kt @@ -6,6 +6,6 @@ class A(private var i: Int, var j: Int) { } fun test(a: A) { - a.i++ + a.i++ a.j++ } diff --git a/compiler/testData/diagnostics/tests/scopes/protectedVisibility/withSmartcast.fir.kt b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/withSmartcast.fir.kt index 0b9cea0fc54..3faa2c70851 100644 --- a/compiler/testData/diagnostics/tests/scopes/protectedVisibility/withSmartcast.fir.kt +++ b/compiler/testData/diagnostics/tests/scopes/protectedVisibility/withSmartcast.fir.kt @@ -14,7 +14,7 @@ class Derived : Base() { x.foo() x.bar() - x.x = x.x + 1 + x.x = x.x + 1 x.y = x.y + 1 if (x is Derived) { diff --git a/compiler/testData/diagnostics/tests/scopes/visibility.fir.kt b/compiler/testData/diagnostics/tests/scopes/visibility.fir.kt index 896ae1e4879..f60ce0df05b 100644 --- a/compiler/testData/diagnostics/tests/scopes/visibility.fir.kt +++ b/compiler/testData/diagnostics/tests/scopes/visibility.fir.kt @@ -54,7 +54,7 @@ open class C : T { } fun test4(c: C) { - c.i++ + c.i++ } class D : C() { diff --git a/compiler/testData/diagnostics/tests/scopes/visibility3.fir.kt b/compiler/testData/diagnostics/tests/scopes/visibility3.fir.kt index 2511df96b50..0b0c86b8c7c 100644 --- a/compiler/testData/diagnostics/tests/scopes/visibility3.fir.kt +++ b/compiler/testData/diagnostics/tests/scopes/visibility3.fir.kt @@ -31,7 +31,7 @@ fun test() { val u : A = A() val z = x - x = 30 + x = 30 val po = PO } diff --git a/compiler/testData/diagnostics/tests/variance/privateToThis/SetVar.fir.kt b/compiler/testData/diagnostics/tests/variance/privateToThis/SetVar.fir.kt index f2176354448..1f9d8ef2c99 100644 --- a/compiler/testData/diagnostics/tests/variance/privateToThis/SetVar.fir.kt +++ b/compiler/testData/diagnostics/tests/variance/privateToThis/SetVar.fir.kt @@ -15,23 +15,23 @@ class Test { this.i = getT() with(Test()) { i = getT() // resolved to this@Test.i - this.i = getT() - this@with.i = getT() + this.i = getT() + this@with.i = getT() this@Test.i = getT() } } fun test(t: Test) { - t.i = getT() + t.i = getT() } companion object { fun test(t: Test) { - t.i = getT() + t.i = getT() } } } fun test(t: Test) { - t.i = getT() + t.i = getT() }