[FIR] Fix redundant visibility modifier checker

compiler/fir/analysis-tests/testData/extendedCheckers/RedundantVisibilityModifierChecker.kt

@@ -1,3 +1,32 @@
+fun f() {
+
+    class LocalClass {
+        <!REDUNDANT_VISIBILITY_MODIFIER!>internal<!> var foo = 0
+    }
+    LocalClass().foo = 1
+    <!REDUNDANT_VISIBILITY_MODIFIER!>public<!> var baz = 0
+}
+
+internal inline fun internal() {
+    f()
+}
+
+class C {
+    internal val z = object {
+        fun foo() = 13
+    }
+}
+
+class Foo2<
+        T1,
+        T2: T1,
+        > {
+    fun <T1,
+            T2, > foo2() {}
+
+    internal inner class B<T,T2,>
+}
+
 <!REDUNDANT_VISIBILITY_MODIFIER!>public<!> class C {
     <!REDUNDANT_VISIBILITY_MODIFIER!>public<!> val foo: Int = 0
 
@@ -35,20 +64,6 @@ interface I {
     fun bar()
 }
 
-fun f() {
-    val i = object : I {
-        <!REDUNDANT_VISIBILITY_MODIFIER!>internal<!> var foo = 0
-        override fun bar() {}
-    }
-    i.foo = 1
-
-    class LocalClass {
-        <!REDUNDANT_VISIBILITY_MODIFIER!>internal<!> var foo = 0
-    }
-    LocalClass().foo = 1
-    <!REDUNDANT_VISIBILITY_MODIFIER!>public<!> var baz = 0
-}
-
 <!REDUNDANT_VISIBILITY_MODIFIER!>public<!> var baz = 0
 
 open class J {

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<R|kotlin/Any|>()
+            }
+
+            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<R|kotlin/Any|>()
+        }
+
+        internal final val z: R|<anonymous>| = object : R|kotlin/Any| {
+            private[local] constructor(): R|<anonymous>| {
+                super<R|kotlin/Any|>()
+            }
+
+            public[local] final fun foo(): R|kotlin/Int| {
+                ^foo Int(13)
+            }
+
+        }
+
+            internal get(): R|<anonymous>|
+
+    }
+    public final class Foo2<T1, T2 : R|T1|> : R|kotlin/Any| {
+        public constructor<T1, T2 : R|T1|>(): R|Foo2<T1, T2>| {
+            super<R|kotlin/Any|>()
+        }
+
+        public final fun <T1, T2> foo2(): R|kotlin/Unit| {
+        }
+
+        internal final inner class B<T, T2, T1, T2 : R|T1|> : R|kotlin/Any| {
+            public[internal] constructor<T, T2>(): R|Foo2.B<T, T2, T1, T2>| {
+                super<R|kotlin/Any|>()
+            }
+
+        }
+
+    }
     public final class C : R|kotlin/Any| {
         public constructor(): R|C| {
             super<R|kotlin/Any|>()
@@ -81,36 +134,6 @@ FILE: RedundantVisibilityModifierChecker.kt
         public abstract fun bar(): R|kotlin/Unit|
 
     }
-    public final fun f(): R|kotlin/Unit| {
-        lval i: R|<anonymous>| = object : R|I| {
-            private[local] constructor(): R|<anonymous>| {
-                super<R|kotlin/Any|>()
-            }
-
-            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|<local>/i|.R|/<anonymous>.foo| = Int(1)
-        local final class LocalClass : R|kotlin/Any| {
-            public[local] constructor(): R|LocalClass| {
-                super<R|kotlin/Any|>()
-            }
-
-            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|

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 is FirPropertyAccessor && declaration.visibility == context.lastInContainingDeclarationsVisibility)
-                    && declaration.visibility == (context.containingDeclarations.last() as FirProperty).visibility)
         ) 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
+            modifierList.hasModifier(KtTokens.INTERNAL_KEYWORD) &&
-                    || declaration.visibilityForDeclarationAndPropertyAccessor == Visibilities.INTERNAL -> Visibilities.INTERNAL
+                    containingMemberDeclaration.let {
+                        it != null && (it.isLocalMember || modifierList.hasModifier(KtTokens.PRIVATE_KEYWORD))
+                    } -> Visibilities.INTERNAL
+
             else -> null
         } ?: return
 
-        if (redundantVisibility == Visibilities.PUBLIC
+        if (
-            && declaration is FirPropertyAccessor
+            redundantVisibility == Visibilities.PUBLIC
-            && declaration.source is FirFakeSourceElement<*>
+            && 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 FirProperty -> this.isLocal
-            is FirPropertyAccessor -> status.visibility
+            is FirRegularClass -> this.isLocal
-            else -> null
+            is FirSimpleFunction -> this.isLocal
+            else -> false
         }
+
+    private val CheckerContext.lastInContainingDeclarationsVisibility
+        get() = (this.containingDeclarations.last() as FirProperty).visibility
 }