FIR checker: add diagnostic CANNOT_WEAKEN_ACCESS_PRIVILEGE

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<FirCallableSymbol<*>>,
+    ) {
+        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<FirCallableSymbol<*>>,
         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

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}''",

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

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<FirSourceElement, KtModifierListOwner, FirMemberDeclaration>(SourceElementPositioningStrategies.OVERRIDE_MODIFIER)
     val OVERRIDING_FINAL_MEMBER by error2<FirSourceElement, KtNamedDeclaration, FirCallableDeclaration<*>, Name>(SourceElementPositioningStrategies.OVERRIDE_MODIFIER)
 
+    val CANNOT_WEAKEN_ACCESS_PRIVILEGE by error3<FirSourceElement, KtModifierListOwner, Visibility, FirCallableDeclaration<*>, Name>(SourceElementPositioningStrategies.VISIBILITY_MODIFIER)
+    val CANNOT_CHANGE_ACCESS_PRIVILEGE by error3<FirSourceElement, KtModifierListOwner, Visibility, FirCallableDeclaration<*>, Name>(SourceElementPositioningStrategies.VISIBILITY_MODIFIER)
+
     val RETURN_TYPE_MISMATCH_ON_OVERRIDE by error2<FirSourceElement, KtNamedDeclaration, FirMemberDeclaration, FirMemberDeclaration>(SourceElementPositioningStrategies.DECLARATION_RETURN_TYPE)
     val PROPERTY_TYPE_MISMATCH_ON_OVERRIDE by error2<FirSourceElement, KtNamedDeclaration, FirMemberDeclaration, FirMemberDeclaration>(SourceElementPositioningStrategies.DECLARATION_RETURN_TYPE)
     val VAR_TYPE_MISMATCH_ON_OVERRIDE by error2<FirSourceElement, KtNamedDeclaration, FirMemberDeclaration, FirMemberDeclaration>(SourceElementPositioningStrategies.DECLARATION_RETURN_TYPE)

compiler/testData/diagnostics/tests/scopes/VisibilityInheritModifier.fir.kt

@@ -65,7 +65,7 @@ open class L : T {
 }
 
 class M : L() {
-    internal override fun foo() {}
+    <!CANNOT_WEAKEN_ACCESS_PRIVILEGE!>internal<!> override fun foo() {}
 }
 //---------------
 interface R {
@@ -81,5 +81,5 @@ interface Q : R {
 }
 
 class S : P, Q {
-    internal override fun foo() {}
+    <!CANNOT_WEAKEN_ACCESS_PRIVILEGE!>internal<!> override fun foo() {}
 }

compiler/testData/diagnostics/tests/scopes/kt1248.fir.kt

@@ -1,12 +0,0 @@
-//KT-1248 Control visibility of overrides needed
-package kt1248
-
-interface ParseResult<out T> {
-    public val success : Boolean
-    public val value : T
-}
-
-class Success<T>(internal override val value : T) : ParseResult<T> {
-    internal override val success : Boolean = true
-}
-

compiler/testData/diagnostics/tests/scopes/kt1248.kt

@@ -1,3 +1,4 @@
+// FIR_IDENTICAL
 //KT-1248 Control visibility of overrides needed
 package kt1248
 

compiler/testData/diagnostics/tests/scopes/kt151.fir.kt

@@ -28,11 +28,11 @@ class D : C(), T {
 }
 
 class E : C(), T {
-    internal override fun foo() {}
+    <!CANNOT_WEAKEN_ACCESS_PRIVILEGE!>internal<!> override fun foo() {}
 }
 
 class F : C(), T {
-    <!INCOMPATIBLE_MODIFIERS!>private<!> <!INCOMPATIBLE_MODIFIERS!>override<!> fun foo() {}
+    <!CANNOT_WEAKEN_ACCESS_PRIVILEGE, INCOMPATIBLE_MODIFIERS!>private<!> <!INCOMPATIBLE_MODIFIERS!>override<!> fun foo() {}
 }
 
 class G : C(), T {