From d96772444eb5d3537e4209f0765ec8b8a51494b6 Mon Sep 17 00:00:00 2001 From: "vladislavf7@gmail.com" Date: Mon, 13 Jul 2020 16:05:43 +0300 Subject: [PATCH] [FIR] Fix redundant visibility checker --- .../RedundantVisibilityModifierChecker.kt | 20 +++++++++++ .../RedundantVisibilityModifierChecker.txt | 34 +++++++++++++++++++ .../RedundantVisibilityModifierChecker.kt | 34 +++++++++++++------ 3 files changed, 77 insertions(+), 11 deletions(-) diff --git a/compiler/fir/analysis-tests/testData/extendedCheckers/RedundantVisibilityModifierChecker.kt b/compiler/fir/analysis-tests/testData/extendedCheckers/RedundantVisibilityModifierChecker.kt index 571bf5853af..4f8fd4293a1 100644 --- a/compiler/fir/analysis-tests/testData/extendedCheckers/RedundantVisibilityModifierChecker.kt +++ b/compiler/fir/analysis-tests/testData/extendedCheckers/RedundantVisibilityModifierChecker.kt @@ -50,3 +50,23 @@ fun f() { } public var baz = 0 + +open class J { + protected val baz = 0 + protected get() = field * 2 + var baf = 0 + public get() = 1 + public set(value) { + field = value + } + + var buf = 0 + private get() = 42 + protected set(value) { + field = value + } + + var bar = 0 + get() = 3.1415926535 + set(value) {} +} diff --git a/compiler/fir/analysis-tests/testData/extendedCheckers/RedundantVisibilityModifierChecker.txt b/compiler/fir/analysis-tests/testData/extendedCheckers/RedundantVisibilityModifierChecker.txt index a256b84f68e..068457ed6c4 100644 --- a/compiler/fir/analysis-tests/testData/extendedCheckers/RedundantVisibilityModifierChecker.txt +++ b/compiler/fir/analysis-tests/testData/extendedCheckers/RedundantVisibilityModifierChecker.txt @@ -114,3 +114,37 @@ FILE: RedundantVisibilityModifierChecker.kt public final var baz: R|kotlin/Int| = Int(0) public get(): R|kotlin/Int| public set(value: R|kotlin/Int|): R|kotlin/Unit| + public open class J : R|kotlin/Any| { + public constructor(): R|J| { + super() + } + + protected final val baz: R|kotlin/Int| = Int(0) + protected get(): R|kotlin/Int| { + ^ this@R|/J|.F|/J.baz|.R|kotlin/Int.times|(Int(2)) + } + + public final var baf: R|kotlin/Int| = Int(0) + public get(): R|kotlin/Int| { + ^ Int(1) + } + public set(value: R|kotlin/Int|): R|kotlin/Unit| { + this@R|/J|.F|/J.baf| = R|/value| + } + + public final var buf: R|kotlin/Int| = Int(0) + private get(): R|kotlin/Int| { + ^ Int(42) + } + protected set(value: R|kotlin/Int|): R|kotlin/Unit| { + this@R|/J|.F|/J.buf| = R|/value| + } + + public final var bar: R|kotlin/Int| = Int(0) + public get(): R|kotlin/Int| { + ^ Double(3.1415926535) + } + public set(value: R|kotlin/Int|): R|kotlin/Unit| { + } + + } diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantVisibilityModifierChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantVisibilityModifierChecker.kt index 11f2117b197..5171e2bdb98 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantVisibilityModifierChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/RedundantVisibilityModifierChecker.kt @@ -23,27 +23,30 @@ object RedundantVisibilityModifierChecker : FirBasicDeclarationChecker() { override val isExtended = true override fun check(declaration: FirDeclaration, context: CheckerContext, reporter: DiagnosticReporter) { - if (declaration is FirPropertyAccessor && declaration.isGetter) return - if (declaration !is FirMemberDeclaration) return if (declaration is FirConstructor && declaration.source is FirFakeSourceElement<*>) return + if (declaration.source is FirFakeSourceElement<*>) return + if ( + declaration !is FirMemberDeclaration + && !(declaration is FirPropertyAccessor + && declaration.visibility == (context.containingDeclarations.last() as FirProperty).visibility) + ) return val modifierList = (declaration.source.getModifierList() as? FirPsiModifierList)?.modifierList ?: return - val visibilityModifier = modifierList.getVisibility() + val visibilityModifier = modifierList.getVisibility() ?: return val implicitVisibility = declaration.implicitVisibility(context) - val containingMemberDeclaration = context.containingDeclarations.getOrNull(1) as? FirMemberDeclaration + val containingMemberDeclaration = context.findClosest() val redundantVisibility = when { visibilityModifier == implicitVisibility -> implicitVisibility visibilityModifier == Visibilities.INTERNAL && containingMemberDeclaration?.visibility == Visibilities.PRIVATE - || declaration.visibility == Visibilities.INTERNAL -> Visibilities.INTERNAL + || declaration.visibilityForDeclarationAndPropertyAccessor == Visibilities.INTERNAL -> Visibilities.INTERNAL else -> null } ?: return if (redundantVisibility == Visibilities.PUBLIC - && declaration is FirProperty - && declaration.isVar - && declaration.setter?.visibility.let { it != null && it != Visibilities.PUBLIC } + && declaration is FirPropertyAccessor + && declaration.source is FirFakeSourceElement<*> ) return val source = modifierList.visibilityModifier()?.toFirPsiSourceElement() @@ -52,8 +55,10 @@ object RedundantVisibilityModifierChecker : FirBasicDeclarationChecker() { private fun FirDeclaration.implicitVisibility(context: CheckerContext): Visibility { return when { - this is FirPropertyAccessor && isSetter && status.isOverride -> { - this.visibility + this is FirPropertyAccessor && isSetter && status.isOverride -> this.visibility + + this is FirPropertyAccessor -> { + context.findClosest()?.visibility ?: Visibilities.DEFAULT_VISIBILITY } this is FirConstructor -> { @@ -69,7 +74,7 @@ object RedundantVisibilityModifierChecker : FirBasicDeclarationChecker() { } this is FirSimpleFunction - && context.containingDeclarations.getOrNull(1) is FirClass<*> + && context.containingDeclarations.last() is FirClass<*> && this.isOverride -> findFunctionVisibility(this, context) else -> Visibilities.DEFAULT_VISIBILITY @@ -102,4 +107,11 @@ object RedundantVisibilityModifierChecker : FirBasicDeclarationChecker() { } return null } + + private val FirDeclaration.visibilityForDeclarationAndPropertyAccessor + get() = when (this) { + is FirMemberDeclaration -> status.visibility + is FirPropertyAccessor -> status.visibility + else -> null + } }