diff --git a/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt b/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt index bfd4f05cb38..538995880b8 100644 --- a/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt +++ b/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt @@ -570,6 +570,11 @@ object DIAGNOSTICS_LIST : DiagnosticList("FirErrors") { val NOTHING_TO_OVERRIDE by error(PositioningStrategy.OVERRIDE_MODIFIER) { parameter("declaration") } + + val CANNOT_OVERRIDE_INVISIBLE_MEMBER by error(PositioningStrategy.OVERRIDE_MODIFIER) { + parameter>("overridingMember") + parameter>("baseMember") + } val CANNOT_WEAKEN_ACCESS_PRIVILEGE by error(PositioningStrategy.VISIBILITY_MODIFIER) { parameter("overridingVisibility") diff --git a/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt b/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt index 691c8df3e64..672ba081550 100644 --- a/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt +++ b/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt @@ -344,6 +344,7 @@ object FirErrors { // overrides val NOTHING_TO_OVERRIDE by error1(SourceElementPositioningStrategies.OVERRIDE_MODIFIER) + val CANNOT_OVERRIDE_INVISIBLE_MEMBER by error2, FirCallableDeclaration<*>>(SourceElementPositioningStrategies.OVERRIDE_MODIFIER) val CANNOT_WEAKEN_ACCESS_PRIVILEGE by error3(SourceElementPositioningStrategies.VISIBILITY_MODIFIER) val CANNOT_CHANGE_ACCESS_PRIVILEGE by error3(SourceElementPositioningStrategies.VISIBILITY_MODIFIER) val OVERRIDING_FINAL_MEMBER by error2(SourceElementPositioningStrategies.OVERRIDE_MODIFIER) 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 964168457e0..855605b0d6e 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 @@ -9,6 +9,7 @@ import org.jetbrains.kotlin.descriptors.Modality import org.jetbrains.kotlin.descriptors.Visibilities import org.jetbrains.kotlin.fir.* 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.unsubstitutedScope import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors @@ -44,7 +45,7 @@ object FirOverrideChecker : FirClassChecker() { for (it in declaration.declarations) { if (it is FirSimpleFunction || it is FirProperty) { - checkMember(it as FirCallableMemberDeclaration, reporter, typeCheckerContext, firTypeScope, context) + checkMember(it as FirCallableMemberDeclaration, declaration, reporter, typeCheckerContext, firTypeScope, context) } } } @@ -110,6 +111,7 @@ object FirOverrideChecker : FirClassChecker() { } private fun FirCallableMemberDeclaration.checkVisibility( + containingClass: FirClass<*>, reporter: DiagnosticReporter, overriddenSymbols: List>, context: CheckerContext @@ -132,6 +134,20 @@ object FirOverrideChecker : FirClassChecker() { return } } + + val file = context.findClosest() ?: return + val containingDeclarations = context.containingDeclarations + containingClass + val visibilityChecker = context.session.visibilityChecker + val hasVisibleBase = overriddenSymbols.any { + val fir = it.fir as? FirCallableMemberDeclaration<*> ?: return@any true + visibilityChecker.isVisible(fir, context.session, file, containingDeclarations, null) + } + if (!hasVisibleBase) { + //NB: Old FE reports this in an attempt to override private member, + //while the new FE doesn't treat super's private members as overridable, so you won't get them here + //instead you will get NOTHING_TO_OVERRIDE, which seems acceptable + reporter.reportOn(source, FirErrors.CANNOT_OVERRIDE_INVISIBLE_MEMBER, this, overriddenSymbols.first().fir, context) + } } // See [OverrideResolver#isReturnTypeOkForOverride] @@ -170,10 +186,11 @@ object FirOverrideChecker : FirClassChecker() { private fun checkMember( member: FirCallableMemberDeclaration, + containingClass: FirClass, reporter: DiagnosticReporter, typeCheckerContext: AbstractTypeCheckerContext, firTypeScope: FirTypeScope, - context: CheckerContext, + context: CheckerContext ) { val overriddenMemberSymbols = firTypeScope.retrieveDirectOverriddenOf(member) @@ -188,12 +205,12 @@ object FirOverrideChecker : FirClassChecker() { if (kind !is FirRealSourceElementKind && kind !is FirFakeSourceElementKind.PropertyFromParameter) return val overridden = overriddenMemberSymbols.first().originalOrSelf() - val containingClass = overridden.containingClass()?.toFirRegularClass(context.session) ?: return + val overriddenClass = overridden.containingClass()?.toFirRegularClass(context.session) ?: return reporter.reportOn( member.source, FirErrors.VIRTUAL_MEMBER_HIDDEN, member, - containingClass, + overriddenClass, context ) return @@ -214,7 +231,7 @@ object FirOverrideChecker : FirClassChecker() { } } - member.checkVisibility(reporter, overriddenMemberSymbols, context) + member.checkVisibility(containingClass, reporter, overriddenMemberSymbols, context) val restriction = member.checkReturnType( overriddenSymbols = overriddenMemberSymbols, 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 887871d941f..5c9fe055300 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 @@ -62,6 +62,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.CANNOT_ALL_UNDER_ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.CANNOT_BE_IMPORTED import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.CANNOT_CHANGE_ACCESS_PRIVILEGE import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.CANNOT_INFER_PARAMETER_TYPE +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.CANNOT_OVERRIDE_INVISIBLE_MEMBER 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 @@ -860,6 +861,14 @@ class FirDefaultErrorMessages { // Overrides map.put(NOTHING_TO_OVERRIDE, "''{0}'' overrides nothing", DECLARATION_NAME) + + map.put( + CANNOT_OVERRIDE_INVISIBLE_MEMBER, + "''{0}'' has no access to ''{1}'', so it cannot override it", + FQ_NAMES_IN_TYPES, + FQ_NAMES_IN_TYPES + ) + map.put(OVERRIDING_FINAL_MEMBER, "''{0}'' in ''{1}'' is final and cannot be overridden", NAME, TO_STRING) map.put( diff --git a/compiler/testData/diagnostics/tests/visibility/abstractInvisibleMemberFromKotlin.fir.kt b/compiler/testData/diagnostics/tests/visibility/abstractInvisibleMemberFromKotlin.fir.kt deleted file mode 100644 index 2314d5d9cb5..00000000000 --- a/compiler/testData/diagnostics/tests/visibility/abstractInvisibleMemberFromKotlin.fir.kt +++ /dev/null @@ -1,47 +0,0 @@ -// !LANGUAGE: +ProhibitInvisibleAbstractMethodsInSuperclasses -// MODULE: base -// FILE: Base.kt -package base - -abstract class Base { - fun foo(): String { - return internalFoo() - } - internal abstract fun internalFoo(): String -} - -open class BaseWithOverride : Base() { - override fun internalFoo(): String = "" -} - -// MODULE: intermediate(base) -// FILE: Intermediate.kt -package intermediate -import base.* - -abstract class Intermediate : Base() - -// MODULE: impl(base, intermediate) -// FILE: Impl.kt -package impl -import base.* -import intermediate.* - -class ImplDirectFromBase : Base() - -object ImplObjDirectFromBase : Base() - -class ImplDirectFromBaseWithOverride : BaseWithOverride() - -class ImplDirectFromBaseWithOverrid : Base() { - override fun internalFoo(): String = "" -} - -class ImplViaIntermediate : Intermediate() - -fun foo() { - ImplDirectFromBase().foo() - ImplObjDirectFromBase.foo() - ImplDirectFromBaseWithOverride().foo() - ImplViaIntermediate().foo() -} diff --git a/compiler/testData/diagnostics/tests/visibility/abstractInvisibleMemberFromKotlin.kt b/compiler/testData/diagnostics/tests/visibility/abstractInvisibleMemberFromKotlin.kt index 7857f417ac8..3e58a4dc1ce 100644 --- a/compiler/testData/diagnostics/tests/visibility/abstractInvisibleMemberFromKotlin.kt +++ b/compiler/testData/diagnostics/tests/visibility/abstractInvisibleMemberFromKotlin.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL // !LANGUAGE: +ProhibitInvisibleAbstractMethodsInSuperclasses // MODULE: base // FILE: Base.kt diff --git a/compiler/tests-spec/testData/diagnostics/linked/declarations/classifier-declaration/class-declaration/abstract-classes/p-2/neg/1.6.fir.kt b/compiler/tests-spec/testData/diagnostics/linked/declarations/classifier-declaration/class-declaration/abstract-classes/p-2/neg/1.6.fir.kt index 2b711accb1c..843147c0d35 100644 --- a/compiler/tests-spec/testData/diagnostics/linked/declarations/classifier-declaration/class-declaration/abstract-classes/p-2/neg/1.6.fir.kt +++ b/compiler/tests-spec/testData/diagnostics/linked/declarations/classifier-declaration/class-declaration/abstract-classes/p-2/neg/1.6.fir.kt @@ -27,7 +27,7 @@ import base.* // TESTCASE NUMBER: 1 class Case1 : BaseJava() { - override fun foo(b: Boolean?) {} + override fun foo(b: Boolean?) {} } fun case1() { @@ -41,7 +41,7 @@ fun case1() { abstract class AbstractClassCase2 : BaseJava() {} class Case2: AbstractClassCase2() { - override fun foo(b: Boolean?) {} + override fun foo(b: Boolean?) {} } fun case2() { diff --git a/compiler/tests-spec/testData/diagnostics/linked/declarations/classifier-declaration/class-declaration/abstract-classes/p-2/neg/1.7.fir.kt b/compiler/tests-spec/testData/diagnostics/linked/declarations/classifier-declaration/class-declaration/abstract-classes/p-2/neg/1.7.fir.kt index e62896b2f42..1883b65b441 100644 --- a/compiler/tests-spec/testData/diagnostics/linked/declarations/classifier-declaration/class-declaration/abstract-classes/p-2/neg/1.7.fir.kt +++ b/compiler/tests-spec/testData/diagnostics/linked/declarations/classifier-declaration/class-declaration/abstract-classes/p-2/neg/1.7.fir.kt @@ -27,7 +27,7 @@ import base.* // TESTCASE NUMBER: 1 class Case1 : BaseKotlin() { - override fun foo(b: Boolean?) {} + override fun foo(b: Boolean?) {} } fun case1() { @@ -35,7 +35,7 @@ fun case1() { v.boo(true) val o = object : BaseKotlin() { - override fun foo(b: Boolean?) {} + override fun foo(b: Boolean?) {} } } @@ -45,7 +45,7 @@ fun case1() { abstract class AbstractClassCase2 : BaseKotlin() {} class Case2: AbstractClassCase2() { - override fun foo(b: Boolean?) {} + override fun foo(b: Boolean?) {} } fun case2() { @@ -53,7 +53,7 @@ fun case2() { v.boo(true) val o = object : AbstractClassCase2() { - override fun foo(b: Boolean?) {} + override fun foo(b: Boolean?) {} } } diff --git a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt index 77f99651ff0..69c835560a2 100644 --- a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt +++ b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt @@ -1626,6 +1626,14 @@ internal val KT_DIAGNOSTIC_CONVERTER = KtDiagnosticConverterBuilder.buildConvert token, ) } + add(FirErrors.CANNOT_OVERRIDE_INVISIBLE_MEMBER) { firDiagnostic -> + CannotOverrideInvisibleMemberImpl( + firSymbolBuilder.callableBuilder.buildCallableSymbol(firDiagnostic.a as FirCallableDeclaration), + firSymbolBuilder.callableBuilder.buildCallableSymbol(firDiagnostic.b as FirCallableDeclaration), + firDiagnostic as FirPsiDiagnostic<*>, + token, + ) + } add(FirErrors.CANNOT_WEAKEN_ACCESS_PRIVILEGE) { firDiagnostic -> CannotWeakenAccessPrivilegeImpl( firDiagnostic.a, diff --git a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt index c2e62734637..7c39bed8a00 100644 --- a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt +++ b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt @@ -1151,6 +1151,12 @@ sealed class KtFirDiagnostic : KtDiagnosticWithPsi { abstract val declaration: KtSymbol } + abstract class CannotOverrideInvisibleMember : KtFirDiagnostic() { + override val diagnosticClass get() = CannotOverrideInvisibleMember::class + abstract val overridingMember: KtCallableSymbol + abstract val baseMember: KtCallableSymbol + } + abstract class CannotWeakenAccessPrivilege : KtFirDiagnostic() { override val diagnosticClass get() = CannotWeakenAccessPrivilege::class abstract val overridingVisibility: Visibility diff --git a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt index 826a4e933dd..abcfd185c97 100644 --- a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt +++ b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt @@ -1854,6 +1854,15 @@ internal class NothingToOverrideImpl( override val firDiagnostic: FirPsiDiagnostic by weakRef(firDiagnostic) } +internal class CannotOverrideInvisibleMemberImpl( + override val overridingMember: KtCallableSymbol, + override val baseMember: KtCallableSymbol, + firDiagnostic: FirPsiDiagnostic<*>, + override val token: ValidityToken, +) : KtFirDiagnostic.CannotOverrideInvisibleMember(), KtAbstractFirDiagnostic { + override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic) +} + internal class CannotWeakenAccessPrivilegeImpl( override val overridingVisibility: Visibility, override val overridden: KtCallableSymbol,