Redundant visibility modifier: do not report for public modifier that overrides protected setter visibility

#KT-33580 Fixed

idea/src/org/jetbrains/kotlin/idea/inspections/RedundantVisibilityModifierInspection.kt

@@ -7,19 +7,27 @@ package org.jetbrains.kotlin.idea.inspections
 
 import com.intellij.codeInspection.*
 import com.intellij.psi.PsiElementVisitor
+import org.jetbrains.kotlin.descriptors.CallableMemberDescriptor
+import org.jetbrains.kotlin.descriptors.PropertyDescriptor
+import org.jetbrains.kotlin.descriptors.Visibilities
+import org.jetbrains.kotlin.descriptors.Visibility
 import org.jetbrains.kotlin.idea.core.implicitVisibility
 import org.jetbrains.kotlin.idea.quickfix.RemoveModifierFix
+import org.jetbrains.kotlin.idea.search.usagesSearch.descriptor
 import org.jetbrains.kotlin.lexer.KtTokens
+import org.jetbrains.kotlin.psi.KtDeclaration
+import org.jetbrains.kotlin.psi.KtProperty
 import org.jetbrains.kotlin.psi.KtPropertyAccessor
 import org.jetbrains.kotlin.psi.declarationVisitor
 import org.jetbrains.kotlin.psi.psiUtil.containingClassOrObject
 import org.jetbrains.kotlin.psi.psiUtil.visibilityModifier
+import org.jetbrains.kotlin.utils.addToStdlib.firstNotNullResult
 
 class RedundantVisibilityModifierInspection : AbstractKotlinInspection(), CleanupLocalInspectionTool {
     override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession): PsiElementVisitor {
-        return declarationVisitor { declaration ->
+        return declarationVisitor(fun(declaration: KtDeclaration) {
-            if (declaration is KtPropertyAccessor && declaration.isGetter) return@declarationVisitor // There is a quick fix for REDUNDANT_MODIFIER_IN_GETTER
+            if (declaration is KtPropertyAccessor && declaration.isGetter) return // There is a quick fix for REDUNDANT_MODIFIER_IN_GETTER
-            val visibilityModifier = declaration.visibilityModifier() ?: return@declarationVisitor
+            val visibilityModifier = declaration.visibilityModifier() ?: return
             val implicitVisibility = declaration.implicitVisibility()
             val redundantVisibility = when {
                 visibilityModifier.node.elementType == implicitVisibility ->
@@ -28,18 +36,36 @@ class RedundantVisibilityModifierInspection : AbstractKotlinInspection(), Cleanu
                     KtTokens.INTERNAL_KEYWORD
                 else ->
                     null
-            }
+            } ?: return
-            if (redundantVisibility != null) {
+
-                holder.registerProblem(
+            if (redundantVisibility == KtTokens.PUBLIC_KEYWORD
-                    visibilityModifier,
+                && declaration is KtProperty
-                    "Redundant visibility modifier",
+                && declaration.hasModifier(KtTokens.OVERRIDE_KEYWORD)
-                    ProblemHighlightType.LIKE_UNUSED_SYMBOL,
+                && declaration.isVar
-                    IntentionWrapper(
+                && declaration.setterVisibility().let { it != null && it != Visibilities.PUBLIC }
-                        RemoveModifierFix(declaration, redundantVisibility, isRedundant = true),
+            ) return
-                        declaration.containingFile
+
-                    )
+            holder.registerProblem(
+                visibilityModifier,
+                "Redundant visibility modifier",
+                ProblemHighlightType.LIKE_UNUSED_SYMBOL,
+                IntentionWrapper(
+                    RemoveModifierFix(declaration, redundantVisibility, isRedundant = true),
+                    declaration.containingFile
                 )
-            }
+            )
+        })
+    }
+
+    private fun KtProperty.setterVisibility(): Visibility? {
+        val descriptor = descriptor as? PropertyDescriptor ?: return null
+        if (setter?.visibilityModifier() != null) {
+            val visibility = descriptor.setter?.visibility
+            if (visibility != null) return visibility
         }
+        return (descriptor as? CallableMemberDescriptor)
+            ?.overriddenDescriptors
+            ?.firstNotNullResult { (it as? PropertyDescriptor)?.setter }
+            ?.visibility
     }
 }

idea/testData/inspectionsLocal/redundantVisibilityModifier/publicOverrideProtectedSetter.kt

@@ -0,0 +1,15 @@
+// PROBLEM: none
+
+abstract class A {
+    open var attribute = "a"
+        protected set
+}
+
+class C : A() {
+    <caret>public override var attribute = super.attribute
+}
+
+fun main() {
+    val c = C()
+    c.attribute = "test"
+}

idea/testData/inspectionsLocal/redundantVisibilityModifier/publicOverrideProtectedSetter2.kt

@@ -0,0 +1,16 @@
+// PROBLEM: none
+
+abstract class A {
+    open var attribute = "a"
+        protected set
+}
+
+class C : A() {
+    <caret>public override var attribute = super.attribute
+        set
+}
+
+fun main() {
+    val c = C()
+    c.attribute = "test"
+}

idea/testData/inspectionsLocal/redundantVisibilityModifier/publicOverrideProtectedSetter3.kt

@@ -0,0 +1,14 @@
+abstract class A {
+    open var attribute = "a"
+        protected set
+}
+
+class C : A() {
+    <caret>public override var attribute = super.attribute
+        public set
+}
+
+fun main() {
+    val c = C()
+    c.attribute = "test"
+}

idea/testData/inspectionsLocal/redundantVisibilityModifier/publicOverrideProtectedSetter3.kt.after

@@ -0,0 +1,14 @@
+abstract class A {
+    open var attribute = "a"
+        protected set
+}
+
+class C : A() {
+    override var attribute = super.attribute
+        public set
+}
+
+fun main() {
+    val c = C()
+    c.attribute = "test"
+}

idea/testData/inspectionsLocal/redundantVisibilityModifier/publicOverrideProtectedSetter4.kt

@@ -0,0 +1,19 @@
+// PROBLEM: none
+
+abstract class A {
+    open var attribute = "a"
+        protected set
+}
+
+abstract class B : A() {
+    override var attribute = "b"
+}
+
+class C : B() {
+    <caret>public override var attribute = super.attribute
+}
+
+fun main() {
+    val c = C()
+    c.attribute = "test"
+}

idea/testData/inspectionsLocal/redundantVisibilityModifier/publicOverrideProtectedSetter5.kt

@@ -0,0 +1,20 @@
+// PROBLEM: none
+
+abstract class A {
+    open var attribute = "a"
+        protected set
+}
+
+abstract class B : A() {
+    override var attribute = "b"
+        set
+}
+
+class C : B() {
+    <caret>public override var attribute = super.attribute
+}
+
+fun main() {
+    val c = C()
+    c.attribute = "test"
+}

idea/testData/inspectionsLocal/redundantVisibilityModifier/publicOverrideProtectedSetter6.kt

@@ -0,0 +1,18 @@
+abstract class A {
+    open var attribute = "a"
+        protected set
+}
+
+abstract class B : A() {
+    override var attribute = "b"
+        public set
+}
+
+class C : B() {
+    <caret>public override var attribute = super.attribute
+}
+
+fun main() {
+    val c = C()
+    c.attribute = "test"
+}

idea/testData/inspectionsLocal/redundantVisibilityModifier/publicOverrideProtectedSetter6.kt.after

@@ -0,0 +1,18 @@
+abstract class A {
+    open var attribute = "a"
+        protected set
+}
+
+abstract class B : A() {
+    override var attribute = "b"
+        public set
+}
+
+class C : B() {
+    override var attribute = super.attribute
+}
+
+fun main() {
+    val c = C()
+    c.attribute = "test"
+}

idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java

@@ -7863,6 +7863,36 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
         public void testOverridePropertySetter() throws Exception {
             runTest("idea/testData/inspectionsLocal/redundantVisibilityModifier/overridePropertySetter.kt");
         }
+
+        @TestMetadata("publicOverrideProtectedSetter.kt")
+        public void testPublicOverrideProtectedSetter() throws Exception {
+            runTest("idea/testData/inspectionsLocal/redundantVisibilityModifier/publicOverrideProtectedSetter.kt");
+        }
+
+        @TestMetadata("publicOverrideProtectedSetter2.kt")
+        public void testPublicOverrideProtectedSetter2() throws Exception {
+            runTest("idea/testData/inspectionsLocal/redundantVisibilityModifier/publicOverrideProtectedSetter2.kt");
+        }
+
+        @TestMetadata("publicOverrideProtectedSetter3.kt")
+        public void testPublicOverrideProtectedSetter3() throws Exception {
+            runTest("idea/testData/inspectionsLocal/redundantVisibilityModifier/publicOverrideProtectedSetter3.kt");
+        }
+
+        @TestMetadata("publicOverrideProtectedSetter4.kt")
+        public void testPublicOverrideProtectedSetter4() throws Exception {
+            runTest("idea/testData/inspectionsLocal/redundantVisibilityModifier/publicOverrideProtectedSetter4.kt");
+        }
+
+        @TestMetadata("publicOverrideProtectedSetter5.kt")
+        public void testPublicOverrideProtectedSetter5() throws Exception {
+            runTest("idea/testData/inspectionsLocal/redundantVisibilityModifier/publicOverrideProtectedSetter5.kt");
+        }
+
+        @TestMetadata("publicOverrideProtectedSetter6.kt")
+        public void testPublicOverrideProtectedSetter6() throws Exception {
+            runTest("idea/testData/inspectionsLocal/redundantVisibilityModifier/publicOverrideProtectedSetter6.kt");
+        }
     }
 
     @TestMetadata("idea/testData/inspectionsLocal/redundantWith")