diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerFirTestdataTestGenerated.java b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerFirTestdataTestGenerated.java index eb7ffe46f11..e8e4b8d7fdd 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerFirTestdataTestGenerated.java +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerFirTestdataTestGenerated.java @@ -2273,6 +2273,12 @@ public class DiagnosisCompilerFirTestdataTestGenerated extends AbstractDiagnosis runTest("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantSingleExpressionStringTemplateChecker.kt"); } + @Test + @TestMetadata("RedundantVisibilityDueToOverride.kt") + public void testRedundantVisibilityDueToOverride() throws Exception { + runTest("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityDueToOverride.kt"); + } + @Test @TestMetadata("RedundantVisibilityModifierChecker.kt") public void testRedundantVisibilityModifierChecker() throws Exception { diff --git a/compiler/fir/analysis-tests/legacy-fir-tests/tests-gen/org/jetbrains/kotlin/fir/LazyBodyIsNotTouchedTilContractsPhaseTestGenerated.java b/compiler/fir/analysis-tests/legacy-fir-tests/tests-gen/org/jetbrains/kotlin/fir/LazyBodyIsNotTouchedTilContractsPhaseTestGenerated.java index a8c596b504b..25759941174 100644 --- a/compiler/fir/analysis-tests/legacy-fir-tests/tests-gen/org/jetbrains/kotlin/fir/LazyBodyIsNotTouchedTilContractsPhaseTestGenerated.java +++ b/compiler/fir/analysis-tests/legacy-fir-tests/tests-gen/org/jetbrains/kotlin/fir/LazyBodyIsNotTouchedTilContractsPhaseTestGenerated.java @@ -1990,6 +1990,11 @@ public class LazyBodyIsNotTouchedTilContractsPhaseTestGenerated extends Abstract runTest("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantSingleExpressionStringTemplateChecker.kt"); } + @TestMetadata("RedundantVisibilityDueToOverride.kt") + public void testRedundantVisibilityDueToOverride() throws Exception { + runTest("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityDueToOverride.kt"); + } + @TestMetadata("RedundantVisibilityModifierChecker.kt") public void testRedundantVisibilityModifierChecker() throws Exception { runTest("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityModifierChecker.kt"); diff --git a/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityDueToOverride.fir.txt b/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityDueToOverride.fir.txt new file mode 100644 index 00000000000..bf520002e53 --- /dev/null +++ b/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityDueToOverride.fir.txt @@ -0,0 +1,41 @@ +FILE: RedundantVisibilityDueToOverride.kt + public abstract class A : R|kotlin/Any| { + public constructor(): R|A| { + super() + } + + public open var attribute: R|kotlin/String| = String(a) + public get(): R|kotlin/String| + protected set(value: R|kotlin/String|): R|kotlin/Unit| + + } + public final class C1 : R|A| { + public constructor(): R|C1| { + super() + } + + public final override var attribute: R|kotlin/String| = this@R|/C1|.super.R|/A.attribute| + public get(): R|kotlin/String| + public set(value: R|kotlin/String|): R|kotlin/Unit| + + } + public abstract class B2 : R|A| { + public constructor(): R|B2| { + super() + } + + public open override var attribute: R|kotlin/String| = String(b) + public get(): R|kotlin/String| + public set(value: R|kotlin/String|): R|kotlin/Unit| + + } + public final class C2 : R|B2| { + public constructor(): R|C2| { + super() + } + + public final override var attribute: R|kotlin/String| = this@R|/C2|.super.R|/B2.attribute| + public get(): R|kotlin/String| + public set(value: R|kotlin/String|): R|kotlin/Unit| + + } diff --git a/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityDueToOverride.kt b/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityDueToOverride.kt new file mode 100644 index 00000000000..aee7c8bf409 --- /dev/null +++ b/compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityDueToOverride.kt @@ -0,0 +1,18 @@ +abstract class A { + open var attribute = "a" + protected set +} + +class C1 : A() { + public override var attribute = super.attribute + public set +} + +abstract class B2 : A() { + override var attribute = "b" + public set +} + +class C2 : B2() { + public override var attribute = super.attribute +} diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticTestGenerated.java index fe2833f1f84..d3fb87f4cd3 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticTestGenerated.java @@ -2273,6 +2273,12 @@ public class FirDiagnosticTestGenerated extends AbstractFirDiagnosticTest { runTest("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantSingleExpressionStringTemplateChecker.kt"); } + @Test + @TestMetadata("RedundantVisibilityDueToOverride.kt") + public void testRedundantVisibilityDueToOverride() throws Exception { + runTest("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityDueToOverride.kt"); + } + @Test @TestMetadata("RedundantVisibilityModifierChecker.kt") public void testRedundantVisibilityModifierChecker() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticsWithLightTreeTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticsWithLightTreeTestGenerated.java index 88b32c58651..0a80fd75bff 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticsWithLightTreeTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticsWithLightTreeTestGenerated.java @@ -2273,6 +2273,12 @@ public class FirDiagnosticsWithLightTreeTestGenerated extends AbstractFirDiagnos runTest("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantSingleExpressionStringTemplateChecker.kt"); } + @Test + @TestMetadata("RedundantVisibilityDueToOverride.kt") + public void testRedundantVisibilityDueToOverride() throws Exception { + runTest("compiler/fir/analysis-tests/testData/resolve/extendedCheckers/RedundantVisibilityDueToOverride.kt"); + } + @Test @TestMetadata("RedundantVisibilityModifierChecker.kt") public void testRedundantVisibilityModifierChecker() throws Exception { diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantVisibilityModifierSyntaxChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantVisibilityModifierSyntaxChecker.kt index db4bf29fa9f..593288a3431 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantVisibilityModifierSyntaxChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantVisibilityModifierSyntaxChecker.kt @@ -21,6 +21,14 @@ import org.jetbrains.kotlin.fir.analysis.checkers.toVisibilityOrNull import org.jetbrains.kotlin.fir.analysis.diagnostics.* import org.jetbrains.kotlin.fir.declarations.* import org.jetbrains.kotlin.fir.declarations.utils.* +import org.jetbrains.kotlin.fir.declarations.utils.isEnumClass +import org.jetbrains.kotlin.fir.declarations.utils.isOverride +import org.jetbrains.kotlin.fir.declarations.utils.isSealed +import org.jetbrains.kotlin.fir.declarations.utils.visibility +import org.jetbrains.kotlin.fir.resolve.ScopeSession +import org.jetbrains.kotlin.fir.scopes.getDirectOverriddenFunctions +import org.jetbrains.kotlin.fir.scopes.getDirectOverriddenProperties +import org.jetbrains.kotlin.fir.scopes.unsubstitutedScope import org.jetbrains.kotlin.fir.symbols.impl.FirRegularClassSymbol import org.jetbrains.kotlin.lexer.KtModifierKeywordToken import org.jetbrains.kotlin.psi.KtDeclaration @@ -45,24 +53,39 @@ object RedundantVisibilityModifierSyntaxChecker : FirDeclarationSyntaxChecker() require(containingMemberDeclaration is FirDeclaration?) + + val isHidden = explicitVisibility.isEffectivelyHiddenBy(containingMemberDeclaration) - val redundantVisibility = when { - explicitVisibility == implicitVisibility -> implicitVisibility - explicitVisibility.isEffectivelyHiddenBy(containingMemberDeclaration) -> explicitVisibility - else -> return + if (explicitVisibility != implicitVisibility && !isHidden) { + return } - if ( - redundantVisibility == Visibilities.Public - && element is FirProperty - && source.treeStructure.overrideModifier(source.lighterASTNode) != null - && element.isVar - && element.setter?.visibility == Visibilities.Public - ) return + if (element.isPublicOverriddenWithNonPublicBase(containingMemberDeclaration, context)) { + return + } reporter.reportOn(source, FirErrors.REDUNDANT_VISIBILITY_MODIFIER, context) } + private fun FirDeclaration.isPublicOverriddenWithNonPublicBase( + container: FirMemberDeclaration?, + context: CheckerContext, + ): Boolean { + if (container !is FirClass) { + return false + } + + val scope = container.unsubstitutedScope(context.session, ScopeSession(), false) + + val overridden = when (this) { + is FirProperty -> scope.getDirectOverriddenProperties(symbol) + is FirSimpleFunction -> scope.getDirectOverriddenFunctions(symbol) + else -> return false + } + + return overridden.any { it.visibility != Visibilities.Public } + } + private fun Visibility?.isEffectivelyHiddenBy(declaration: FirMemberDeclaration?): Boolean { val containerVisibility = declaration?.effectiveVisibility?.toVisibility() ?: return false