From 5909b191a59892633afb12e20c04fbb6a2112d1a Mon Sep 17 00:00:00 2001 From: Marco Pennekamp Date: Mon, 6 Feb 2023 19:12:53 +0100 Subject: [PATCH] [FIR] KT-54507 Report `REDUNDANT_MODALITY_MODIFIER` for `open interface` - `REDUNDANT_MODIFIER_FOR_TARGET` is already reported for `open` interfaces, but for consistency and IDE support, the compiler now reports `REDUNDANT_MODALITY_MODIFIER` as well via the extended checker. - `REDUNDANT_MODIFIER_FOR_TARGET` cannot be disabled for this case because it's reported via a basic checker while `REDUNDANT_MODALITY_MODIFIER` is reported via an extended checker. - Rename `implicitModality` to `redundantModalities` and return a set of modalities. The idea of a *single* implicit/redundant modality doesn't stand up to scrutiny. For example, for interfaces with `ABSTRACT` implicit modality, `OPEN` is also a redundant modifier. But this is not necessarily the case for all kinds of declarations. Hypothetically, if it was possible to declare a class that is abstract by default, adding an `open` modifier to that class would not be redundant, as it would make the class instantiable. --- .../RedundantModalityModifierChecker.fir.txt | 2 ++ .../RedundantModalityModifierChecker.kt | 3 ++- .../fir/analysis/checkers/FirHelpers.kt | 19 ++++++++++--------- .../RedundantModalityModifierSyntaxChecker.kt | 12 ++++++------ 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantModalityModifierChecker.fir.txt b/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantModalityModifierChecker.fir.txt index eb9b6be8d6f..cf1b8745a84 100644 --- a/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantModalityModifierChecker.fir.txt +++ b/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantModalityModifierChecker.fir.txt @@ -18,6 +18,8 @@ FILE: RedundantModalityModifierChecker.kt private final fun bar(): R|kotlin/Unit| + public abstract fun loo(): R|kotlin/Unit| + public open fun goo(): R|kotlin/Unit| { } diff --git a/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantModalityModifierChecker.kt b/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantModalityModifierChecker.kt index 14e11b07b99..08b739cc5db 100644 --- a/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantModalityModifierChecker.kt +++ b/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantModalityModifierChecker.kt @@ -12,6 +12,7 @@ interface Interface { // error private final fun bar() + open fun loo() open fun goo() {} abstract fun tar() @@ -84,7 +85,7 @@ abstract class AbstractDerived2 : Interface { // Redundant final object final object FinalObject // Open interface -open interface OpenInterface +open interface OpenInterface class FinalDerived2(override final val gav: Int) : Base() { override fun bar() {} diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirHelpers.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirHelpers.kt index 07b7f2e9365..563231b1f94 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirHelpers.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirHelpers.kt @@ -221,25 +221,26 @@ fun FirClass.modality(): Modality? { } /** - * returns implicit modality by FirMemberDeclaration<*> + * Returns a set of [Modality] modifiers which are redundant for the given [FirMemberDeclaration]. If a modality modifier is redundant, the + * declaration's modality won't be changed by the modifier. */ -fun FirMemberDeclaration.implicitModality(context: CheckerContext): Modality { +fun FirMemberDeclaration.redundantModalities(context: CheckerContext): Set { if (this is FirRegularClass) { return when (classKind) { - ClassKind.INTERFACE -> Modality.ABSTRACT - else -> Modality.FINAL + ClassKind.INTERFACE -> setOf(Modality.ABSTRACT, Modality.OPEN) + else -> setOf(Modality.FINAL) } } - val containingClass = context.findClosestClassOrObject() ?: return Modality.FINAL + val containingClass = context.findClosestClassOrObject() ?: return setOf(Modality.FINAL) return when { - isOverride && !containingClass.isFinal -> Modality.OPEN + isOverride && !containingClass.isFinal -> setOf(Modality.OPEN) containingClass.isInterface -> when { - hasBody() -> Modality.OPEN - else -> Modality.ABSTRACT + hasBody() -> setOf(Modality.OPEN) + else -> setOf(Modality.ABSTRACT, Modality.OPEN) } - else -> Modality.FINAL + else -> setOf(Modality.FINAL) } } diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantModalityModifierSyntaxChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantModalityModifierSyntaxChecker.kt index 052fed98b87..cbecfc375ea 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantModalityModifierSyntaxChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantModalityModifierSyntaxChecker.kt @@ -10,13 +10,13 @@ import org.jetbrains.kotlin.KtRealSourceElementKind import org.jetbrains.kotlin.KtSourceElement import org.jetbrains.kotlin.descriptors.ClassKind import org.jetbrains.kotlin.descriptors.Modality -import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext -import org.jetbrains.kotlin.fir.analysis.checkers.implicitModality -import org.jetbrains.kotlin.fir.analysis.checkers.syntax.FirDeclarationSyntaxChecker import org.jetbrains.kotlin.diagnostics.DiagnosticReporter -import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.REDUNDANT_MODALITY_MODIFIER import org.jetbrains.kotlin.diagnostics.modalityModifier import org.jetbrains.kotlin.diagnostics.reportOn +import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext +import org.jetbrains.kotlin.fir.analysis.checkers.redundantModalities +import org.jetbrains.kotlin.fir.analysis.checkers.syntax.FirDeclarationSyntaxChecker +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.REDUNDANT_MODALITY_MODIFIER import org.jetbrains.kotlin.fir.declarations.FirClass import org.jetbrains.kotlin.fir.declarations.FirDeclaration import org.jetbrains.kotlin.fir.declarations.FirMemberDeclaration @@ -46,8 +46,8 @@ object RedundantModalityModifierSyntaxChecker : FirDeclarationSyntaxChecker