diff --git a/compiler/fir/analysis-tests/testData/extendedCheckers/RedundantVisibilityModifierChecker.kt b/compiler/fir/analysis-tests/testData/extendedCheckers/RedundantVisibilityModifierChecker.kt index 4f8fd4293a1..add1dfeb269 100644 --- a/compiler/fir/analysis-tests/testData/extendedCheckers/RedundantVisibilityModifierChecker.kt +++ b/compiler/fir/analysis-tests/testData/extendedCheckers/RedundantVisibilityModifierChecker.kt @@ -1,3 +1,32 @@ +fun f() { + + class LocalClass { + internal var foo = 0 + } + LocalClass().foo = 1 + public var baz = 0 +} + +internal inline fun internal() { + f() +} + +class C { + internal val z = object { + fun foo() = 13 + } +} + +class Foo2< + T1, + T2: T1, + > { + fun foo2() {} + + internal inner class B +} + public class C { public val foo: Int = 0 @@ -35,20 +64,6 @@ interface I { fun bar() } -fun f() { - val i = object : I { - internal var foo = 0 - override fun bar() {} - } - i.foo = 1 - - class LocalClass { - internal var foo = 0 - } - LocalClass().foo = 1 - public var baz = 0 -} - public var baz = 0 open class J { diff --git a/compiler/fir/analysis-tests/testData/extendedCheckers/RedundantVisibilityModifierChecker.txt b/compiler/fir/analysis-tests/testData/extendedCheckers/RedundantVisibilityModifierChecker.txt index 068457ed6c4..a5c6e619865 100644 --- a/compiler/fir/analysis-tests/testData/extendedCheckers/RedundantVisibilityModifierChecker.txt +++ b/compiler/fir/analysis-tests/testData/extendedCheckers/RedundantVisibilityModifierChecker.txt @@ -1,4 +1,57 @@ FILE: RedundantVisibilityModifierChecker.kt + public final fun f(): R|kotlin/Unit| { + local final class LocalClass : R|kotlin/Any| { + public[local] constructor(): R|LocalClass| { + super() + } + + internal[local] final var foo: R|kotlin/Int| = Int(0) + internal get(): R|kotlin/Int| + internal set(value: R|kotlin/Int|): R|kotlin/Unit| + + } + + R|/LocalClass.LocalClass|().R|/LocalClass.foo| = Int(1) + lvar baz: R|kotlin/Int| = Int(0) + } + internal final inline fun internal(): R|kotlin/Unit| { + R|/f|() + } + public final class C : R|kotlin/Any| { + public constructor(): R|C| { + super() + } + + internal final val z: R|| = object : R|kotlin/Any| { + private[local] constructor(): R|| { + super() + } + + public[local] final fun foo(): R|kotlin/Int| { + ^foo Int(13) + } + + } + + internal get(): R|| + + } + public final class Foo2 : R|kotlin/Any| { + public constructor(): R|Foo2| { + super() + } + + public final fun foo2(): R|kotlin/Unit| { + } + + internal final inner class B : R|kotlin/Any| { + public[internal] constructor(): R|Foo2.B| { + super() + } + + } + + } public final class C : R|kotlin/Any| { public constructor(): R|C| { super() @@ -81,36 +134,6 @@ FILE: RedundantVisibilityModifierChecker.kt public abstract fun bar(): R|kotlin/Unit| } - public final fun f(): R|kotlin/Unit| { - lval i: R|| = object : R|I| { - private[local] constructor(): R|| { - super() - } - - internal[local] final var foo: R|kotlin/Int| = Int(0) - internal get(): R|kotlin/Int| - internal set(value: R|kotlin/Int|): R|kotlin/Unit| - - public[local] final override fun bar(): R|kotlin/Unit| { - } - - } - - R|/i|.R|/.foo| = Int(1) - local final class LocalClass : R|kotlin/Any| { - public[local] constructor(): R|LocalClass| { - super() - } - - internal[local] final var foo: R|kotlin/Int| = Int(0) - internal get(): R|kotlin/Int| - internal set(value: R|kotlin/Int|): R|kotlin/Unit| - - } - - R|/LocalClass.LocalClass|().R|/LocalClass.foo| = Int(1) - lvar baz: R|kotlin/Int| = Int(0) - } public final var baz: R|kotlin/Int| = Int(0) public get(): R|kotlin/Int| 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 e8ff99735c7..da8a807bc2a 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 @@ -17,6 +17,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors import org.jetbrains.kotlin.fir.declarations.* import org.jetbrains.kotlin.fir.toFirPsiSourceElement +import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.psiUtil.visibilityModifier object RedundantVisibilityModifierChecker : FirBasicDeclarationChecker() { @@ -25,8 +26,7 @@ object RedundantVisibilityModifierChecker : FirBasicDeclarationChecker() { if (declaration.source is FirFakeSourceElement<*>) return if ( declaration !is FirMemberDeclaration - && !(declaration is FirPropertyAccessor - && declaration.visibility == (context.containingDeclarations.last() as FirProperty).visibility) + && !(declaration is FirPropertyAccessor && declaration.visibility == context.lastInContainingDeclarationsVisibility) ) return val modifierList = (declaration.source.getModifierList() as? FirPsiModifierList)?.modifierList ?: return @@ -36,15 +36,21 @@ object RedundantVisibilityModifierChecker : FirBasicDeclarationChecker() { val redundantVisibility = when { visibilityModifier == implicitVisibility -> implicitVisibility - visibilityModifier == Visibilities.INTERNAL - && containingMemberDeclaration?.visibility == Visibilities.PRIVATE - || declaration.visibilityForDeclarationAndPropertyAccessor == Visibilities.INTERNAL -> Visibilities.INTERNAL + + modifierList.hasModifier(KtTokens.INTERNAL_KEYWORD) && + containingMemberDeclaration.let { + it != null && (it.isLocalMember || modifierList.hasModifier(KtTokens.PRIVATE_KEYWORD)) + } -> Visibilities.INTERNAL + else -> null } ?: return - if (redundantVisibility == Visibilities.PUBLIC - && declaration is FirPropertyAccessor - && declaration.source is FirFakeSourceElement<*> + if ( + redundantVisibility == Visibilities.PUBLIC + && declaration is FirProperty + && modifierList.hasModifier(KtTokens.OVERRIDE_KEYWORD) + && declaration.isVar + && declaration.setter?.visibility == Visibilities.PUBLIC ) return val source = modifierList.visibilityModifier()?.toFirPsiSourceElement() @@ -100,16 +106,22 @@ object RedundantVisibilityModifierChecker : FirBasicDeclarationChecker() { (symbol.fir as? FirMemberDeclaration)?.visibility?.let { return it } + (symbol.fir as? FirPropertyAccessor)?.visibility?.let { return it } + return null } - private val FirDeclaration.visibilityForDeclarationAndPropertyAccessor + private val FirMemberDeclaration.isLocalMember: Boolean get() = when (this) { - is FirMemberDeclaration -> status.visibility - is FirPropertyAccessor -> status.visibility - else -> null + is FirProperty -> this.isLocal + is FirRegularClass -> this.isLocal + is FirSimpleFunction -> this.isLocal + else -> false } + + private val CheckerContext.lastInContainingDeclarationsVisibility + get() = (this.containingDeclarations.last() as FirProperty).visibility }