[FIR] Implement CANNOT_OVERRIDE_INVISIBLE_MEMBER diagnostic

This commit is contained in:
Andrey Zinovyev
2021-06-15 16:20:17 +03:00
committed by teamcityserver
parent f4c8108ee0
commit 9452b788bf
11 changed files with 67 additions and 58 deletions
@@ -570,6 +570,11 @@ object DIAGNOSTICS_LIST : DiagnosticList("FirErrors") {
val NOTHING_TO_OVERRIDE by error<KtModifierListOwner>(PositioningStrategy.OVERRIDE_MODIFIER) {
parameter<FirMemberDeclaration>("declaration")
}
val CANNOT_OVERRIDE_INVISIBLE_MEMBER by error<KtNamedDeclaration>(PositioningStrategy.OVERRIDE_MODIFIER) {
parameter<FirCallableDeclaration<*>>("overridingMember")
parameter<FirCallableDeclaration<*>>("baseMember")
}
val CANNOT_WEAKEN_ACCESS_PRIVILEGE by error<KtModifierListOwner>(PositioningStrategy.VISIBILITY_MODIFIER) {
parameter<Visibility>("overridingVisibility")
@@ -344,6 +344,7 @@ object FirErrors {
// overrides
val NOTHING_TO_OVERRIDE by error1<KtModifierListOwner, FirMemberDeclaration>(SourceElementPositioningStrategies.OVERRIDE_MODIFIER)
val CANNOT_OVERRIDE_INVISIBLE_MEMBER by error2<KtNamedDeclaration, FirCallableDeclaration<*>, FirCallableDeclaration<*>>(SourceElementPositioningStrategies.OVERRIDE_MODIFIER)
val CANNOT_WEAKEN_ACCESS_PRIVILEGE by error3<KtModifierListOwner, Visibility, FirCallableDeclaration, Name>(SourceElementPositioningStrategies.VISIBILITY_MODIFIER)
val CANNOT_CHANGE_ACCESS_PRIVILEGE by error3<KtModifierListOwner, Visibility, FirCallableDeclaration, Name>(SourceElementPositioningStrategies.VISIBILITY_MODIFIER)
val OVERRIDING_FINAL_MEMBER by error2<KtNamedDeclaration, FirCallableDeclaration, Name>(SourceElementPositioningStrategies.OVERRIDE_MODIFIER)
@@ -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<FirCallableSymbol<*>>,
context: CheckerContext
@@ -132,6 +134,20 @@ object FirOverrideChecker : FirClassChecker() {
return
}
}
val file = context.findClosest<FirFile>() ?: 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,
@@ -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(
@@ -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.*
<!INVISIBLE_ABSTRACT_MEMBER_FROM_SUPER!>class ImplDirectFromBase<!> : Base()
<!INVISIBLE_ABSTRACT_MEMBER_FROM_SUPER!>object ImplObjDirectFromBase<!> : Base()
class ImplDirectFromBaseWithOverride : BaseWithOverride()
class ImplDirectFromBaseWithOverrid : Base() {
override fun internalFoo(): String = ""
}
<!INVISIBLE_ABSTRACT_MEMBER_FROM_SUPER!>class ImplViaIntermediate<!> : Intermediate()
fun foo() {
ImplDirectFromBase().foo()
ImplObjDirectFromBase.foo()
ImplDirectFromBaseWithOverride().foo()
ImplViaIntermediate().foo()
}
@@ -1,3 +1,4 @@
// FIR_IDENTICAL
// !LANGUAGE: +ProhibitInvisibleAbstractMethodsInSuperclasses
// MODULE: base
// FILE: Base.kt
@@ -27,7 +27,7 @@ import base.*
// TESTCASE NUMBER: 1
class Case1 : BaseJava() {
override fun foo(b: Boolean?) {}
<!CANNOT_OVERRIDE_INVISIBLE_MEMBER!>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?) {}
<!CANNOT_OVERRIDE_INVISIBLE_MEMBER!>override<!> fun foo(b: Boolean?) {}
}
fun case2() {
@@ -27,7 +27,7 @@ import base.*
// TESTCASE NUMBER: 1
class Case1 : BaseKotlin() {
override fun foo(b: Boolean?) {}
<!CANNOT_OVERRIDE_INVISIBLE_MEMBER!>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?) {}
<!CANNOT_OVERRIDE_INVISIBLE_MEMBER!>override<!> fun foo(b: Boolean?) {}
}
}
@@ -45,7 +45,7 @@ fun case1() {
abstract class AbstractClassCase2 : BaseKotlin() {}
class Case2: AbstractClassCase2() {
override fun foo(b: Boolean?) {}
<!CANNOT_OVERRIDE_INVISIBLE_MEMBER!>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?) {}
<!CANNOT_OVERRIDE_INVISIBLE_MEMBER!>override<!> fun foo(b: Boolean?) {}
}
}
@@ -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,
@@ -1151,6 +1151,12 @@ sealed class KtFirDiagnostic<PSI: PsiElement> : KtDiagnosticWithPsi<PSI> {
abstract val declaration: KtSymbol
}
abstract class CannotOverrideInvisibleMember : KtFirDiagnostic<KtNamedDeclaration>() {
override val diagnosticClass get() = CannotOverrideInvisibleMember::class
abstract val overridingMember: KtCallableSymbol
abstract val baseMember: KtCallableSymbol
}
abstract class CannotWeakenAccessPrivilege : KtFirDiagnostic<KtModifierListOwner>() {
override val diagnosticClass get() = CannotWeakenAccessPrivilege::class
abstract val overridingVisibility: Visibility
@@ -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<KtNamedDeclaration> {
override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic)
}
internal class CannotWeakenAccessPrivilegeImpl(
override val overridingVisibility: Visibility,
override val overridden: KtCallableSymbol,