From 80d5a1a1dbef9ab007fd1b25bc24bcf179f38155 Mon Sep 17 00:00:00 2001 From: Jinseong Jeon Date: Thu, 28 Jan 2021 16:26:00 -0800 Subject: [PATCH] FIR checker: add diagnostic OVERRIDING_FINAL_MEMBER --- .../declaration/FirOverrideChecker.kt | 42 +++++++++++++++++++ .../diagnostics/FirDefaultErrorMessages.kt | 6 +++ .../diagnostics/FirDiagnosticRenderers.kt | 8 ++++ .../fir/analysis/diagnostics/FirErrors.kt | 4 ++ .../tests/TraitOverrideObjectMethods.fir.kt | 2 +- .../tests/enum/kt8972_cloneNotAllowed.fir.kt | 2 +- .../enum/overrideFinalEnumMethods.fir.kt | 6 +-- .../override/OverridingFinalMember.fir.kt | 7 ---- .../tests/override/OverridingFinalMember.kt | 1 + .../overrideMemberFromFinalClass.fir.kt | 2 +- 10 files changed, 67 insertions(+), 13 deletions(-) delete mode 100644 compiler/testData/diagnostics/tests/override/OverridingFinalMember.fir.kt 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 ce54fb8955f..55f3ba6031c 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 @@ -5,9 +5,11 @@ package org.jetbrains.kotlin.fir.analysis.checkers.declaration +import org.jetbrains.kotlin.descriptors.Modality import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors +import org.jetbrains.kotlin.fir.containingClass import org.jetbrains.kotlin.fir.declarations.* import org.jetbrains.kotlin.fir.resolve.substitution.substitutorByMap import org.jetbrains.kotlin.fir.scopes.FirTypeScope @@ -80,6 +82,20 @@ object FirOverrideChecker : FirRegularClassChecker() { return substitutorByMap(map).substituteOrSelf(this) } + private fun checkModality( + overriddenSymbols: List>, + ): FirCallableDeclaration<*>? { + for (overridden in overriddenSymbols) { + if (overridden.fir !is FirMemberDeclaration) continue + val modality = (overridden.fir as FirMemberDeclaration).status.modality + val isEffectivelyFinal = modality == null || modality == Modality.FINAL + if (isEffectivelyFinal) { + return overridden.fir + } + } + return null + } + private fun FirCallableMemberDeclaration<*>.checkReturnType( overriddenSymbols: List>, typeCheckerContext: AbstractTypeCheckerContext, @@ -121,9 +137,14 @@ object FirOverrideChecker : FirRegularClassChecker() { val overriddenFunctionSymbols = firTypeScope.retrieveDirectOverriddenOf(function) if (overriddenFunctionSymbols.isEmpty()) { + reporter.reportNothingToOverride(function) return } + checkModality(overriddenFunctionSymbols)?.let { + reporter.reportOverridingFinalMember(function, it) + } + val restriction = function.checkReturnType( overriddenSymbols = overriddenFunctionSymbols, typeCheckerContext = typeCheckerContext, @@ -149,9 +170,14 @@ object FirOverrideChecker : FirRegularClassChecker() { val overriddenPropertySymbols = firTypeScope.retrieveDirectOverriddenOf(property) if (overriddenPropertySymbols.isEmpty()) { + reporter.reportNothingToOverride(property) return } + checkModality(overriddenPropertySymbols)?.let { + reporter.reportOverridingFinalMember(property, it) + } + val restriction = property.checkReturnType( overriddenSymbols = overriddenPropertySymbols, typeCheckerContext = typeCheckerContext, @@ -167,6 +193,22 @@ object FirOverrideChecker : FirRegularClassChecker() { } } + private fun DiagnosticReporter.reportNothingToOverride(declaration: FirMemberDeclaration) { + // TODO: not ready yet, e.g., Collections + // declaration.source?.let { report(FirErrors.NOTHING_TO_OVERRIDE.on(it, declaration)) } + } + + private fun DiagnosticReporter.reportOverridingFinalMember( + overriding: FirMemberDeclaration, + overridden: FirCallableDeclaration<*>, + ) { + overriding.source?.let { source -> + overridden.containingClass()?.let { containingClass -> + report(FirErrors.OVERRIDING_FINAL_MEMBER.on(source, 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 e9de9cc691b..b02d8fdf4d9 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 @@ -10,6 +10,7 @@ import org.jetbrains.kotlin.diagnostics.rendering.DiagnosticFactoryToRendererMap import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.AMBIGUOUS_CALLS import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.DECLARATION_NAME import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.FQ_NAMES_IN_TYPES +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.NAME import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.NULLABLE_STRING import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.PROPERTY_NAME import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnosticRenderers.RENDER_TYPE @@ -90,6 +91,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NON_ABSTRACT_FUNC import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NON_MEMBER_FUNCTION_NO_BODY import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NON_PRIVATE_CONSTRUCTOR_IN_ENUM import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NON_PRIVATE_CONSTRUCTOR_IN_SEALED +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NOTHING_TO_OVERRIDE import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NOT_AN_ANNOTATION_CLASS import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NOT_A_LOOP_LABEL import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NOT_A_SUPERTYPE @@ -97,6 +99,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NO_THIS import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NO_TYPE_FOR_TYPE_PARAMETER import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NULLABLE_TYPE_OF_ANNOTATION_MEMBER import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.OTHER_ERROR +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.OVERRIDING_FINAL_MEMBER import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.PRIMARY_CONSTRUCTOR_DELEGATION_CALL_EXPECTED import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.PRIMARY_CONSTRUCTOR_REQUIRED_FOR_DATA_CLASS import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.PRIVATE_FUNCTION_WITH_NO_BODY @@ -328,6 +331,9 @@ class FirDefaultErrorMessages : DefaultErrorMessages.Extension { map.put(TYPE_PARAMETER_IN_CATCH_CLAUSE, "Type parameter is forbidden for catch parameter") // Overrides + 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( 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 40a12118675..71c60e4a34e 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 @@ -47,6 +47,14 @@ object FirDiagnosticRenderers { element.render() } + val NAME = Renderer { element: FirElement -> + when (element) { + is FirMemberDeclaration -> DECLARATION_NAME.render(element) + is FirCallableDeclaration<*> -> element.symbol.callableId.callableName.asString() + else -> "???" + } + } + 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 129114cca3c..11bb6018b57 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 @@ -10,6 +10,7 @@ import com.intellij.psi.PsiTypeElement import org.jetbrains.kotlin.contracts.description.EventOccurrencesRange import org.jetbrains.kotlin.fir.FirEffectiveVisibility import org.jetbrains.kotlin.fir.FirSourceElement +import org.jetbrains.kotlin.fir.declarations.FirCallableDeclaration import org.jetbrains.kotlin.fir.declarations.FirClass import org.jetbrains.kotlin.fir.declarations.FirMemberDeclaration import org.jetbrains.kotlin.fir.symbols.AbstractFirBasedSymbol @@ -137,6 +138,9 @@ object FirErrors { val TYPE_PARAMETER_IN_CATCH_CLAUSE by error0() // Overrides + val NOTHING_TO_OVERRIDE by error1(SourceElementPositioningStrategies.OVERRIDE_MODIFIER) + val OVERRIDING_FINAL_MEMBER by error2, Name>(SourceElementPositioningStrategies.OVERRIDE_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/TraitOverrideObjectMethods.fir.kt b/compiler/testData/diagnostics/tests/TraitOverrideObjectMethods.fir.kt index a7c50ea9086..bdac51dc5c4 100644 --- a/compiler/testData/diagnostics/tests/TraitOverrideObjectMethods.fir.kt +++ b/compiler/testData/diagnostics/tests/TraitOverrideObjectMethods.fir.kt @@ -2,5 +2,5 @@ interface MyTrait: Object { override fun toString(): String public override fun finalize() - public override fun wait() + public override fun wait() } diff --git a/compiler/testData/diagnostics/tests/enum/kt8972_cloneNotAllowed.fir.kt b/compiler/testData/diagnostics/tests/enum/kt8972_cloneNotAllowed.fir.kt index b37092ccccf..0a93180d2f3 100644 --- a/compiler/testData/diagnostics/tests/enum/kt8972_cloneNotAllowed.fir.kt +++ b/compiler/testData/diagnostics/tests/enum/kt8972_cloneNotAllowed.fir.kt @@ -1,7 +1,7 @@ // !WITH_NEW_INFERENCE enum class E : Cloneable { A; - override fun clone(): Any { + override fun clone(): Any { return super.clone() } } diff --git a/compiler/testData/diagnostics/tests/enum/overrideFinalEnumMethods.fir.kt b/compiler/testData/diagnostics/tests/enum/overrideFinalEnumMethods.fir.kt index 69b11a657d8..56a79f9ed22 100644 --- a/compiler/testData/diagnostics/tests/enum/overrideFinalEnumMethods.fir.kt +++ b/compiler/testData/diagnostics/tests/enum/overrideFinalEnumMethods.fir.kt @@ -4,8 +4,8 @@ enum class E { override val name: String = "lol" override val ordinal: Int = 0 - override fun compareTo(other: E) = -1 + override fun compareTo(other: E) = -1 - override fun equals(other: Any?) = true - override fun hashCode() = -1 + override fun equals(other: Any?) = true + override fun hashCode() = -1 } diff --git a/compiler/testData/diagnostics/tests/override/OverridingFinalMember.fir.kt b/compiler/testData/diagnostics/tests/override/OverridingFinalMember.fir.kt deleted file mode 100644 index 95f20afd171..00000000000 --- a/compiler/testData/diagnostics/tests/override/OverridingFinalMember.fir.kt +++ /dev/null @@ -1,7 +0,0 @@ -open class A { - final fun foo() {} -} - -class B : A() { - override fun foo() {} -} diff --git a/compiler/testData/diagnostics/tests/override/OverridingFinalMember.kt b/compiler/testData/diagnostics/tests/override/OverridingFinalMember.kt index 1d90924753d..6dbf09585d8 100644 --- a/compiler/testData/diagnostics/tests/override/OverridingFinalMember.kt +++ b/compiler/testData/diagnostics/tests/override/OverridingFinalMember.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL open class A { final fun foo() {} } diff --git a/compiler/testData/diagnostics/tests/override/overrideMemberFromFinalClass.fir.kt b/compiler/testData/diagnostics/tests/override/overrideMemberFromFinalClass.fir.kt index c0430ca06a2..6585b36718a 100644 --- a/compiler/testData/diagnostics/tests/override/overrideMemberFromFinalClass.fir.kt +++ b/compiler/testData/diagnostics/tests/override/overrideMemberFromFinalClass.fir.kt @@ -5,7 +5,7 @@ class Foo { class Bar : Foo() { override fun openFoo() {} - override fun finalFoo() {} + override fun finalFoo() {} }