From 221e642d97d9b70e9564df4c8296b88aa0cf8e70 Mon Sep 17 00:00:00 2001 From: Andrey Zinovyev Date: Wed, 30 Jun 2021 13:40:03 +0300 Subject: [PATCH] [FIR] Fix inheritance diagnostics for new FirDeclaration hierarchy Plus several small fixes in reporting itself --- .../diagnostics/FirDiagnosticsList.kt | 34 +++---- .../fir/analysis/diagnostics/FirErrors.kt | 18 ++-- .../FirImplementationMismatchChecker.kt | 89 ++++++++++--------- .../declaration/FirOverrideChecker.kt | 7 +- ...JvmOverridesBackwardCompatibilityHelper.kt | 2 + 5 files changed, 79 insertions(+), 71 deletions(-) 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 110807ab55d..072c20ed823 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 @@ -572,13 +572,13 @@ object DIAGNOSTICS_LIST : DiagnosticList("FirErrors") { } val CANNOT_OVERRIDE_INVISIBLE_MEMBER by error(PositioningStrategy.OVERRIDE_MODIFIER) { - parameter>("overridingMember") - parameter>("baseMember") + parameter("overridingMember") + parameter("baseMember") } val DATA_CLASS_OVERRIDE_CONFLICT by error(PositioningStrategy.DATA_MODIFIER) { - parameter>("overridingMember") - parameter>("baseMember") + parameter("overridingMember") + parameter("baseMember") } val CANNOT_WEAKEN_ACCESS_PRIVILEGE by error(PositioningStrategy.VISIBILITY_MODIFIER) { @@ -598,37 +598,37 @@ object DIAGNOSTICS_LIST : DiagnosticList("FirErrors") { } val RETURN_TYPE_MISMATCH_ON_INHERITANCE by error(PositioningStrategy.DECLARATION_NAME) { - parameter>("conflictingDeclaration1") - parameter>("conflictingDeclaration2") + parameter("conflictingDeclaration1") + parameter("conflictingDeclaration2") } val PROPERTY_TYPE_MISMATCH_ON_INHERITANCE by error(PositioningStrategy.DECLARATION_NAME) { - parameter>("conflictingDeclaration1") - parameter>("conflictingDeclaration2") + parameter("conflictingDeclaration1") + parameter("conflictingDeclaration2") } val VAR_TYPE_MISMATCH_ON_INHERITANCE by error(PositioningStrategy.DECLARATION_NAME) { - parameter>("conflictingDeclaration1") - parameter>("conflictingDeclaration2") + parameter("conflictingDeclaration1") + parameter("conflictingDeclaration2") } val RETURN_TYPE_MISMATCH_BY_DELEGATION by error(PositioningStrategy.DECLARATION_NAME) { - parameter>("delegateDeclaration") - parameter>("baseDeclaration") + parameter("delegateDeclaration") + parameter("baseDeclaration") } val PROPERTY_TYPE_MISMATCH_BY_DELEGATION by error(PositioningStrategy.DECLARATION_NAME) { - parameter>("delegateDeclaration") - parameter>("baseDeclaration") + parameter("delegateDeclaration") + parameter("baseDeclaration") } val VAR_OVERRIDDEN_BY_VAL_BY_DELEGATION by error(PositioningStrategy.DECLARATION_NAME) { - parameter>("delegateDeclaration") - parameter>("baseDeclaration") + parameter("delegateDeclaration") + parameter("baseDeclaration") } val CONFLICTING_INHERITED_MEMBERS by error(PositioningStrategy.DECLARATION_NAME) { - parameter>>("conflictingDeclarations") + parameter>("conflictingDeclarations") } val ABSTRACT_MEMBER_NOT_IMPLEMENTED by error(PositioningStrategy.DECLARATION_NAME) { 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 8583e7c66ac..9255419cf8e 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,18 +344,18 @@ object FirErrors { // overrides val NOTHING_TO_OVERRIDE by error1(SourceElementPositioningStrategies.OVERRIDE_MODIFIER) - val CANNOT_OVERRIDE_INVISIBLE_MEMBER by error2, FirCallableDeclaration<*>>(SourceElementPositioningStrategies.OVERRIDE_MODIFIER) - val DATA_CLASS_OVERRIDE_CONFLICT by error2, FirCallableDeclaration<*>>(SourceElementPositioningStrategies.DATA_MODIFIER) + val CANNOT_OVERRIDE_INVISIBLE_MEMBER by error2(SourceElementPositioningStrategies.OVERRIDE_MODIFIER) + val DATA_CLASS_OVERRIDE_CONFLICT by error2(SourceElementPositioningStrategies.DATA_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) - val RETURN_TYPE_MISMATCH_ON_INHERITANCE by error2, FirCallableDeclaration<*>>(SourceElementPositioningStrategies.DECLARATION_NAME) - val PROPERTY_TYPE_MISMATCH_ON_INHERITANCE by error2, FirCallableDeclaration<*>>(SourceElementPositioningStrategies.DECLARATION_NAME) - val VAR_TYPE_MISMATCH_ON_INHERITANCE by error2, FirCallableDeclaration<*>>(SourceElementPositioningStrategies.DECLARATION_NAME) - val RETURN_TYPE_MISMATCH_BY_DELEGATION by error2, FirCallableDeclaration<*>>(SourceElementPositioningStrategies.DECLARATION_NAME) - val PROPERTY_TYPE_MISMATCH_BY_DELEGATION by error2, FirCallableDeclaration<*>>(SourceElementPositioningStrategies.DECLARATION_NAME) - val VAR_OVERRIDDEN_BY_VAL_BY_DELEGATION by error2, FirCallableDeclaration<*>>(SourceElementPositioningStrategies.DECLARATION_NAME) - val CONFLICTING_INHERITED_MEMBERS by error1>>(SourceElementPositioningStrategies.DECLARATION_NAME) + val RETURN_TYPE_MISMATCH_ON_INHERITANCE by error2(SourceElementPositioningStrategies.DECLARATION_NAME) + val PROPERTY_TYPE_MISMATCH_ON_INHERITANCE by error2(SourceElementPositioningStrategies.DECLARATION_NAME) + val VAR_TYPE_MISMATCH_ON_INHERITANCE by error2(SourceElementPositioningStrategies.DECLARATION_NAME) + val RETURN_TYPE_MISMATCH_BY_DELEGATION by error2(SourceElementPositioningStrategies.DECLARATION_NAME) + val PROPERTY_TYPE_MISMATCH_BY_DELEGATION by error2(SourceElementPositioningStrategies.DECLARATION_NAME) + val VAR_OVERRIDDEN_BY_VAL_BY_DELEGATION by error2(SourceElementPositioningStrategies.DECLARATION_NAME) + val CONFLICTING_INHERITED_MEMBERS by error1>(SourceElementPositioningStrategies.DECLARATION_NAME) val ABSTRACT_MEMBER_NOT_IMPLEMENTED by error2(SourceElementPositioningStrategies.DECLARATION_NAME) val ABSTRACT_CLASS_MEMBER_NOT_IMPLEMENTED by error2(SourceElementPositioningStrategies.DECLARATION_NAME) val INVISIBLE_ABSTRACT_MEMBER_FROM_SUPER by error2(SourceElementPositioningStrategies.DECLARATION_NAME) diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirImplementationMismatchChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirImplementationMismatchChecker.kt index 1338c11250e..93ca78e1fa0 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirImplementationMismatchChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirImplementationMismatchChecker.kt @@ -14,6 +14,10 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors import org.jetbrains.kotlin.fir.analysis.diagnostics.impl.deduplicating import org.jetbrains.kotlin.fir.analysis.diagnostics.reportOn import org.jetbrains.kotlin.fir.declarations.* +import org.jetbrains.kotlin.fir.declarations.utils.classId +import org.jetbrains.kotlin.fir.declarations.utils.isAbstract +import org.jetbrains.kotlin.fir.declarations.utils.isExpect +import org.jetbrains.kotlin.fir.declarations.utils.isSuspend import org.jetbrains.kotlin.fir.resolve.substitution.substitutorByMap import org.jetbrains.kotlin.fir.scopes.FirTypeScope import org.jetbrains.kotlin.fir.scopes.impl.delegatedWrapperData @@ -29,7 +33,7 @@ import org.jetbrains.kotlin.types.AbstractTypeChecker object FirImplementationMismatchChecker : FirClassChecker() { - override fun check(declaration: FirClass<*>, context: CheckerContext, reporter: DiagnosticReporter) { + override fun check(declaration: FirClass, context: CheckerContext, reporter: DiagnosticReporter) { val source = declaration.source ?: return val sourceKind = source.kind if (sourceKind is FirFakeSourceElementKind && sourceKind != FirFakeSourceElementKind.EnumInitializer) return @@ -48,38 +52,41 @@ object FirImplementationMismatchChecker : FirClassChecker() { classScope.processFunctionsByName(name) { checkInheritanceClash(declaration, context, dedupReporter, typeCheckerContext, it) } classScope.processPropertiesByName(name) { checkInheritanceClash(declaration, context, dedupReporter, typeCheckerContext, it) - checkValOverrideVar(declaration, context, dedupReporter, it) + checkValOverridesVar(declaration, context, dedupReporter, it) } checkConflictingMembers(declaration, context, dedupReporter, classScope, name) } } private fun checkInheritanceClash( - containingClass: FirClass<*>, + containingClass: FirClass, context: CheckerContext, reporter: DiagnosticReporter, typeCheckerContext: ConeTypeCheckerContext, symbol: FirCallableSymbol<*> ) { - fun reportTypeMismatch(member1: FirCallableDeclaration<*>, member2: FirCallableDeclaration<*>, isDelegation: Boolean) { - val error = if (member1 is FirProperty && member2 is FirProperty) { - if (member1.isVar || member2.isVar) { - FirErrors.VAR_TYPE_MISMATCH_ON_INHERITANCE - } else { - if (isDelegation) FirErrors.PROPERTY_TYPE_MISMATCH_BY_DELEGATION - else FirErrors.PROPERTY_TYPE_MISMATCH_ON_INHERITANCE + fun reportTypeMismatch(member1: FirCallableDeclaration, member2: FirCallableDeclaration, isDelegation: Boolean) { + val error = when { + member1 is FirProperty && member2 is FirProperty -> { + if (member1.isVar || member2.isVar) { + FirErrors.VAR_TYPE_MISMATCH_ON_INHERITANCE + } else { + if (isDelegation) FirErrors.PROPERTY_TYPE_MISMATCH_BY_DELEGATION + else FirErrors.PROPERTY_TYPE_MISMATCH_ON_INHERITANCE + } + } + else -> { + if (isDelegation) FirErrors.RETURN_TYPE_MISMATCH_BY_DELEGATION + else FirErrors.RETURN_TYPE_MISMATCH_ON_INHERITANCE } - } else { - if (isDelegation) FirErrors.RETURN_TYPE_MISMATCH_BY_DELEGATION - else FirErrors.RETURN_TYPE_MISMATCH_ON_INHERITANCE } reporter.reportOn(containingClass.source, error, member1, member2, context) } fun canOverride( - inheritedMember: FirCallableDeclaration<*>, + inheritedMember: FirCallableDeclaration, inheritedType: ConeKotlinType, - baseMember: FirCallableDeclaration<*>, + baseMember: FirCallableDeclaration, baseType: ConeKotlinType ): Boolean { val inheritedTypeSubstituted = inheritedType.substituteTypeParameters(inheritedMember, baseMember, context) @@ -89,8 +96,6 @@ object FirImplementationMismatchChecker : FirClassChecker() { AbstractTypeChecker.isSubtypeOf(typeCheckerContext, inheritedTypeSubstituted, baseType) } - - if (symbol.callableId.classId != containingClass.classId) return if (symbol !is FirIntersectionCallableSymbol) return val withTypes = symbol.intersections.map { @@ -99,8 +104,8 @@ object FirImplementationMismatchChecker : FirClassChecker() { if (withTypes.any { it.second is ConeKotlinErrorType }) return - var delegation: FirCallableDeclaration<*>? = null - val implementations = mutableListOf>() + var delegation: FirCallableDeclaration? = null + val implementations = mutableListOf() for (intSymbol in symbol.intersections) { val fir = intSymbol.fir @@ -108,26 +113,24 @@ object FirImplementationMismatchChecker : FirClassChecker() { delegation = fir break } - if (!(fir as FirCallableMemberDeclaration<*>).isAbstract) { + if (!(fir as FirCallableMemberDeclaration).isAbstract) { implementations.add(fir) } } - run { - var clash: Pair, FirCallableDeclaration<*>>? = null - val compatible = withTypes.any { (m1, type1) -> - withTypes.all { (m2, type2) -> - val result = canOverride(m1, type1, m2, type2) - if (!result && clash == null && !canOverride(m2, type2, m1, type1)) { - clash = m1 to m2 - } - result + var someClash: Pair? = null + val compatible = withTypes.any { (m1, type1) -> + withTypes.all { (m2, type2) -> + val result = canOverride(m1, type1, m2, type2) + if (!result && someClash == null && !canOverride(m2, type2, m1, type1)) { + someClash = m1 to m2 } + result } - clash?.takeIf { !compatible }?.let { (m1, m2) -> - reportTypeMismatch(m1, m2, false) - return@checkInheritanceClash - } + } + someClash?.takeIf { !compatible }?.let { (m1, m2) -> + reportTypeMismatch(m1, m2, false) + return@checkInheritanceClash } if (delegation != null || implementations.isNotEmpty()) { @@ -142,8 +145,8 @@ object FirImplementationMismatchChecker : FirClassChecker() { } } - private fun checkValOverrideVar( - containingClass: FirClass<*>, + private fun checkValOverridesVar( + containingClass: FirClass, context: CheckerContext, reporter: DiagnosticReporter, symbol: FirVariableSymbol<*> @@ -156,7 +159,7 @@ object FirImplementationMismatchChecker : FirClassChecker() { fir.isVal && fir.delegatedWrapperData?.containingClass?.classId == containingClass.classId } - val delegatedVal = delegates.singleOrNull() ?: return + val delegatedVal = delegates.firstOrNull() ?: return val baseVar = others.find { it is FirPropertySymbol && it.fir.isVar } @@ -167,7 +170,7 @@ object FirImplementationMismatchChecker : FirClassChecker() { } private fun checkConflictingMembers( - containingClass: FirClass<*>, + containingClass: FirClass, context: CheckerContext, reporter: DiagnosticReporter, scope: FirTypeScope, @@ -187,18 +190,20 @@ object FirImplementationMismatchChecker : FirClassChecker() { function.valueParameters.map { it.returnTypeRef.coneType } }.values - val clash = sameArgumentGroups.mapNotNull { fs -> + val clashes = sameArgumentGroups.mapNotNull { fs -> fs.zipWithNext().find { (m1, m2) -> m1.isSuspend != m2.isSuspend || m1.typeParameters.size != m2.typeParameters.size } - }.firstOrNull() ?: return + } - reporter.reportOn(containingClass.source, FirErrors.CONFLICTING_INHERITED_MEMBERS, clash.toList(), context) + clashes.forEach { + reporter.reportOn(containingClass.source, FirErrors.CONFLICTING_INHERITED_MEMBERS, it.toList(), context) + } } private fun ConeKotlinType.substituteTypeParameters( - fromDeclaration: FirCallableDeclaration<*>, - toDeclaration: FirCallableDeclaration<*>, + fromDeclaration: FirCallableDeclaration, + toDeclaration: FirCallableDeclaration, context: CheckerContext ): ConeKotlinType { val fromParams = (fromDeclaration as? FirTypeParametersOwner)?.typeParameters ?: return this 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 aa98fe42e7e..858fc4fa5a1 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 @@ -16,6 +16,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors import org.jetbrains.kotlin.fir.analysis.diagnostics.reportOn import org.jetbrains.kotlin.fir.analysis.overridesBackwardCompatibilityHelper import org.jetbrains.kotlin.fir.declarations.* +import org.jetbrains.kotlin.fir.declarations.utils.isFinal import org.jetbrains.kotlin.fir.declarations.utils.isOverride import org.jetbrains.kotlin.fir.declarations.utils.visibility import org.jetbrains.kotlin.fir.resolve.substitution.substitutorByMap @@ -111,7 +112,7 @@ object FirOverrideChecker : FirClassChecker() { } private fun FirCallableMemberDeclaration.checkVisibility( - containingClass: FirClass<*>, + containingClass: FirClass, reporter: DiagnosticReporter, overriddenSymbols: List>, context: CheckerContext @@ -139,7 +140,7 @@ object FirOverrideChecker : FirClassChecker() { val containingDeclarations = context.containingDeclarations + containingClass val visibilityChecker = context.session.visibilityChecker val hasVisibleBase = overriddenSymbols.any { - val fir = it.fir as? FirCallableMemberDeclaration<*> ?: return@any true + val fir = it.fir as? FirCallableMemberDeclaration ?: return@any true visibilityChecker.isVisible(fir, context.session, file, containingDeclarations, null) } if (!hasVisibleBase) { @@ -204,7 +205,7 @@ object FirOverrideChecker : FirClassChecker() { // Only report if the current member has real source or it's a member property declared inside the primary constructor. if (kind is FirFakeSourceElementKind.DataClassGeneratedMembers) { - overriddenMemberSymbols.find { (it.fir as? FirCallableMemberDeclaration<*>)?.isFinal == true }?.let { base -> + overriddenMemberSymbols.find { (it.fir as? FirCallableMemberDeclaration)?.isFinal == true }?.let { base -> reporter.reportOn( containingClass.source, FirErrors.DATA_CLASS_OVERRIDE_CONFLICT, diff --git a/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/session/FirJvmOverridesBackwardCompatibilityHelper.kt b/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/session/FirJvmOverridesBackwardCompatibilityHelper.kt index 8f5eca3fe0e..cc6254d107c 100644 --- a/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/session/FirJvmOverridesBackwardCompatibilityHelper.kt +++ b/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/session/FirJvmOverridesBackwardCompatibilityHelper.kt @@ -15,7 +15,9 @@ import org.jetbrains.kotlin.fir.declarations.FirCallableMemberDeclaration import org.jetbrains.kotlin.fir.declarations.FirDeclarationOrigin import org.jetbrains.kotlin.fir.declarations.FirProperty import org.jetbrains.kotlin.fir.declarations.FirSimpleFunction +import org.jetbrains.kotlin.fir.declarations.utils.isAbstract import org.jetbrains.kotlin.fir.declarations.utils.isFinal +import org.jetbrains.kotlin.fir.declarations.utils.isInterface import org.jetbrains.kotlin.fir.dispatchReceiverTypeOrNull import org.jetbrains.kotlin.fir.originalOrSelf import org.jetbrains.kotlin.fir.resolve.toFirRegularClass