diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirOverrideChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirOverrideChecker.kt index a3692071c3b..bd020bdebd7 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirOverrideChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirOverrideChecker.kt @@ -103,6 +103,31 @@ object FirOverrideChecker : FirRegularClassChecker() { return overriddenSymbols.find { (it.fir as? FirProperty)?.isVar == true }?.fir?.safeAs() } + private fun FirCallableMemberDeclaration<*>.checkVisibility( + reporter: DiagnosticReporter, + overriddenSymbols: List>, + ) { + val visibilities = overriddenSymbols.mapNotNull { + if (it.fir !is FirMemberDeclaration) return@mapNotNull null + it to (it.fir as FirMemberDeclaration).visibility + }.sortedBy { pair -> + // Regard `null` compare as Int.MIN so that we can report CANNOT_CHANGE_... first deterministically + visibility.compareTo(pair.second) ?: Int.MIN_VALUE + } + + for ((overridden, overriddenVisibility) in visibilities) { + val compare = visibility.compareTo(overriddenVisibility) + if (compare == null) { + // TODO: not ready yet (even after determinism massage), e.g., a Kotlin class that extends a Java class + // reporter.reportCannotChangeAccessPrivilege(this, overridden.fir) + return + } else if (compare < 0) { + reporter.reportCannotWeakenAccessPrivilege(this, overridden.fir) + return + } + } + } + private fun FirCallableMemberDeclaration<*>.checkReturnType( overriddenSymbols: List>, typeCheckerContext: AbstractTypeCheckerContext, @@ -152,6 +177,8 @@ object FirOverrideChecker : FirRegularClassChecker() { reporter.reportOverridingFinalMember(function, it) } + function.checkVisibility(reporter, overriddenFunctionSymbols) + val restriction = function.checkReturnType( overriddenSymbols = overriddenFunctionSymbols, typeCheckerContext = typeCheckerContext, @@ -189,6 +216,8 @@ object FirOverrideChecker : FirRegularClassChecker() { reporter.reportVarOverriddenByVal(property, it) } + property.checkVisibility(reporter, overriddenPropertySymbols) + val restriction = property.checkReturnType( overriddenSymbols = overriddenPropertySymbols, typeCheckerContext = typeCheckerContext, @@ -227,6 +256,28 @@ object FirOverrideChecker : FirRegularClassChecker() { overriding.source?.let { report(FirErrors.VAR_OVERRIDDEN_BY_VAL.on(it, overriding, overridden)) } } + private fun DiagnosticReporter.reportCannotWeakenAccessPrivilege( + overriding: FirMemberDeclaration, + overridden: FirCallableDeclaration<*>, + ) { + overriding.source?.let { source -> + overridden.containingClass()?.let { containingClass -> + report(FirErrors.CANNOT_WEAKEN_ACCESS_PRIVILEGE.on(source, overriding.visibility, overridden, containingClass.name)) + } + } + } + + private fun DiagnosticReporter.reportCannotChangeAccessPrivilege( + overriding: FirMemberDeclaration, + overridden: FirCallableDeclaration<*>, + ) { + overriding.source?.let { source -> + overridden.containingClass()?.let { containingClass -> + report(FirErrors.CANNOT_CHANGE_ACCESS_PRIVILEGE.on(source, overriding.visibility, overridden, containingClass.name)) + } + } + } + private fun DiagnosticReporter.reportReturnTypeMismatchOnFunction( overriding: FirMemberDeclaration, overridden: FirMemberDeclaration diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirDefaultErrorMessages.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirDefaultErrorMessages.kt index 1811b4b922a..f17fdc08ea8 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirDefaultErrorMessages.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirDefaultErrorMessages.kt @@ -17,6 +17,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.REND import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.SYMBOL import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.SYMBOLS import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.TO_STRING +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.VISIBILITY import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ABSTRACT_DELEGATED_PROPERTY import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ABSTRACT_FUNCTION_IN_NON_ABSTRACT_CLASS import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ABSTRACT_FUNCTION_WITH_BODY @@ -34,6 +35,8 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ARRAY_EQUALITY_OP import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ASSIGNED_VALUE_IS_NEVER_READ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ASSIGN_OPERATOR_AMBIGUITY import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.BREAK_OR_CONTINUE_OUTSIDE_A_LOOP +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.CANNOT_CHANGE_ACCESS_PRIVILEGE +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.CANNOT_WEAKEN_ACCESS_PRIVILEGE import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.CAN_BE_REPLACED_WITH_OPERATOR_ASSIGNMENT import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.CAN_BE_VAL import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.CATCH_PARAMETER_WITH_DEFAULT_VALUE @@ -335,6 +338,21 @@ class FirDefaultErrorMessages : DefaultErrorMessages.Extension { map.put(NOTHING_TO_OVERRIDE, "''{0}'' overrides nothing", DECLARATION_NAME) map.put(OVERRIDING_FINAL_MEMBER, "''{0}'' in ''{1}'' is final and cannot be overridden", NAME, TO_STRING) + map.put( + CANNOT_WEAKEN_ACCESS_PRIVILEGE, + "Cannot weaken access privilege ''{0}'' for ''{1}'' in ''{2}''", + VISIBILITY, + NAME, + TO_STRING + ) + map.put( + CANNOT_CHANGE_ACCESS_PRIVILEGE, + "Cannot change access privilege ''{0}'' for ''{1}'' in ''{2}''", + VISIBILITY, + NAME, + TO_STRING + ) + map.put( RETURN_TYPE_MISMATCH_ON_OVERRIDE, "Return type of ''{0}'' is not a subtype of the return type of the overridden member ''{1}''", diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirDiagnosticRenderers.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirDiagnosticRenderers.kt index 71c60e4a34e..3e35205c1a9 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirDiagnosticRenderers.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirDiagnosticRenderers.kt @@ -5,6 +5,7 @@ package org.jetbrains.kotlin.fir.analysis.diagnostics +import org.jetbrains.kotlin.descriptors.Visibility import org.jetbrains.kotlin.diagnostics.rendering.Renderer import org.jetbrains.kotlin.fir.FirElement import org.jetbrains.kotlin.fir.FirRenderer @@ -55,6 +56,10 @@ object FirDiagnosticRenderers { } } + val VISIBILITY = Renderer { visibility: Visibility -> + visibility.externalDisplayName + } + val DECLARATION_NAME = Renderer { declaration: FirMemberDeclaration -> val name = when (declaration) { is FirProperty -> declaration.name diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt index f3684ebea2d..6b848a85fbb 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt @@ -8,6 +8,7 @@ package org.jetbrains.kotlin.fir.analysis.diagnostics import com.intellij.psi.PsiElement import com.intellij.psi.PsiTypeElement import org.jetbrains.kotlin.contracts.description.EventOccurrencesRange +import org.jetbrains.kotlin.descriptors.Visibility import org.jetbrains.kotlin.fir.FirEffectiveVisibility import org.jetbrains.kotlin.fir.FirSourceElement import org.jetbrains.kotlin.fir.declarations.FirCallableDeclaration @@ -141,6 +142,9 @@ object FirErrors { val NOTHING_TO_OVERRIDE by error1(SourceElementPositioningStrategies.OVERRIDE_MODIFIER) val OVERRIDING_FINAL_MEMBER by error2, Name>(SourceElementPositioningStrategies.OVERRIDE_MODIFIER) + val CANNOT_WEAKEN_ACCESS_PRIVILEGE by error3, Name>(SourceElementPositioningStrategies.VISIBILITY_MODIFIER) + val CANNOT_CHANGE_ACCESS_PRIVILEGE by error3, Name>(SourceElementPositioningStrategies.VISIBILITY_MODIFIER) + val RETURN_TYPE_MISMATCH_ON_OVERRIDE by error2(SourceElementPositioningStrategies.DECLARATION_RETURN_TYPE) val PROPERTY_TYPE_MISMATCH_ON_OVERRIDE by error2(SourceElementPositioningStrategies.DECLARATION_RETURN_TYPE) val VAR_TYPE_MISMATCH_ON_OVERRIDE by error2(SourceElementPositioningStrategies.DECLARATION_RETURN_TYPE) diff --git a/compiler/testData/diagnostics/tests/scopes/VisibilityInheritModifier.fir.kt b/compiler/testData/diagnostics/tests/scopes/VisibilityInheritModifier.fir.kt index bde07b70546..fe0830c44b9 100644 --- a/compiler/testData/diagnostics/tests/scopes/VisibilityInheritModifier.fir.kt +++ b/compiler/testData/diagnostics/tests/scopes/VisibilityInheritModifier.fir.kt @@ -65,7 +65,7 @@ open class L : T { } class M : L() { - internal override fun foo() {} + internal override fun foo() {} } //--------------- interface R { @@ -81,5 +81,5 @@ interface Q : R { } class S : P, Q { - internal override fun foo() {} + internal override fun foo() {} } diff --git a/compiler/testData/diagnostics/tests/scopes/kt1248.fir.kt b/compiler/testData/diagnostics/tests/scopes/kt1248.fir.kt deleted file mode 100644 index a083534a2b3..00000000000 --- a/compiler/testData/diagnostics/tests/scopes/kt1248.fir.kt +++ /dev/null @@ -1,12 +0,0 @@ -//KT-1248 Control visibility of overrides needed -package kt1248 - -interface ParseResult { - public val success : Boolean - public val value : T -} - -class Success(internal override val value : T) : ParseResult { - internal override val success : Boolean = true -} - diff --git a/compiler/testData/diagnostics/tests/scopes/kt1248.kt b/compiler/testData/diagnostics/tests/scopes/kt1248.kt index d7d37268638..3f800bfb141 100644 --- a/compiler/testData/diagnostics/tests/scopes/kt1248.kt +++ b/compiler/testData/diagnostics/tests/scopes/kt1248.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL //KT-1248 Control visibility of overrides needed package kt1248 diff --git a/compiler/testData/diagnostics/tests/scopes/kt151.fir.kt b/compiler/testData/diagnostics/tests/scopes/kt151.fir.kt index 3cf914f2bb4..6b2b2ef4735 100644 --- a/compiler/testData/diagnostics/tests/scopes/kt151.fir.kt +++ b/compiler/testData/diagnostics/tests/scopes/kt151.fir.kt @@ -28,11 +28,11 @@ class D : C(), T { } class E : C(), T { - internal override fun foo() {} + internal override fun foo() {} } class F : C(), T { - private override fun foo() {} + private override fun foo() {} } class G : C(), T {