FIR IDE: introduce HLRedundantVisibilityModifierInspection by extended checker

compiler/psi/src/org/jetbrains/kotlin/psi/KtModifierList.java

@@ -19,6 +19,7 @@ package org.jetbrains.kotlin.psi;
 import com.intellij.lang.ASTNode;
 import com.intellij.psi.PsiElement;
 import com.intellij.psi.stubs.IStubElementType;
+import com.intellij.psi.tree.TokenSet;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.kotlin.lexer.KtModifierKeywordToken;
@@ -66,6 +67,12 @@ public abstract class KtModifierList extends KtElementImplStub<KotlinModifierLis
         return findChildByType(tokenType);
     }
 
+    @Nullable
+    public PsiElement getModifier(@NotNull TokenSet tokenTypes) {
+        return findChildByType(tokenTypes);
+    }
+
+
     public PsiElement getOwner() {
         return getParentByStub();
     }

generators/tests/org/jetbrains/kotlin/generators/tests/GenerateTests.kt

@@ -101,6 +101,7 @@ import org.jetbrains.kotlin.idea.imports.AbstractJsOptimizeImportsTest
 import org.jetbrains.kotlin.idea.imports.AbstractJvmOptimizeImportsTest
 import org.jetbrains.kotlin.idea.index.AbstractKotlinTypeAliasByExpansionShortNameIndexTest
 import org.jetbrains.kotlin.idea.inspections.AbstractHLInspectionTest
+import org.jetbrains.kotlin.idea.inspections.AbstractHLLocalInspectionTest
 import org.jetbrains.kotlin.idea.inspections.AbstractLocalInspectionTest
 import org.jetbrains.kotlin.idea.inspections.AbstractMultiFileLocalInspectionTest
 import org.jetbrains.kotlin.idea.intentions.*
@@ -1115,6 +1116,7 @@ fun main(args: Array<String>) {
                 model("inspections/redundantUnitReturnType", pattern = pattern, singleClass = true)
             }
 
+
             testClass<AbstractHLIntentionTest> {
                 val pattern = "^([\\w\\-_]+)\\.(kt|kts)$"
                 model("intentions/specifyTypeExplicitly", pattern = pattern)
@@ -1125,6 +1127,14 @@ fun main(args: Array<String>) {
             }
         }
 
+        testGroup("idea/idea-fir/tests", "idea") {
+            testClass<AbstractHLLocalInspectionTest> {
+                val pattern = "^([\\w\\-_]+)\\.(kt|kts)$"
+                model("testData/inspectionsLocal/redundantVisibilityModifier", pattern = pattern)
+                model("idea-fir/testData/inspectionsLocal", pattern = pattern)
+            }
+        }
+
         testGroup("idea/idea-fir/tests", "idea/idea-completion/testData") {
             testClass<AbstractHighLevelJvmBasicCompletionTest> {
                 model("basic/common")

idea/idea-fir/src/org/jetbrains/kotlin/idea/fir/applicators/ApplicabilityRanges.kt

@@ -6,8 +6,12 @@
 package org.jetbrains.kotlin.idea.fir.applicators
 
 import com.intellij.psi.PsiElement
+import com.intellij.psi.tree.TokenSet
 import org.jetbrains.kotlin.idea.fir.api.applicator.applicabilityTarget
+import org.jetbrains.kotlin.idea.refactoring.safeDelete.removeOverrideModifier
+import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.psi.KtCallableDeclaration
+import org.jetbrains.kotlin.psi.KtModifierListOwner
 
 object ApplicabilityRanges {
     val SELF = applicabilityTarget<PsiElement> { it }
@@ -15,4 +19,10 @@ object ApplicabilityRanges {
     val CALLABLE_RETURN_TYPE = applicabilityTarget<KtCallableDeclaration> { decalration ->
         decalration.typeReference?.typeElement
     }
+
+    val VISIBILITY_MODIFIER = modifier(KtTokens.VISIBILITY_MODIFIERS)
+
+    fun modifier(tokens: TokenSet) = applicabilityTarget<KtModifierListOwner> { declaration ->
+        declaration.modifierList?.getModifier(tokens)
+    }
 }

idea/idea-fir/src/org/jetbrains/kotlin/idea/fir/applicators/ModifierApplicators.kt

@@ -0,0 +1,42 @@
+/*
+ * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.idea.fir.applicators
+
+import com.intellij.psi.PsiElement
+import com.intellij.psi.tree.TokenSet
+import org.jetbrains.kotlin.idea.KotlinBundle
+import org.jetbrains.kotlin.idea.fir.api.applicator.HLApplicatorInput
+import org.jetbrains.kotlin.idea.fir.api.applicator.applicator
+import org.jetbrains.kotlin.idea.util.application.runWriteAction
+import org.jetbrains.kotlin.lexer.KtModifierKeywordToken
+
+import org.jetbrains.kotlin.psi.KtModifierListOwner
+
+object ModifierApplicators {
+    fun removeModifierApplicator(modifier: TokenSet, familyName: () -> String) = applicator<KtModifierListOwner, Modifier> {
+        familyName(familyName)
+        actionName { _, (modifier) -> KotlinBundle.message("remove.0.modifier", modifier.value) }
+
+        isApplicableByPsi { modifierOwner ->
+            modifierOwner.modifierList?.getModifier(modifier) != null
+        }
+
+        applyTo { modifierOwner, (modifier) ->
+            runWriteAction {
+                modifierOwner.removeModifier(modifier)
+            }
+        }
+    }
+
+    class Modifier(val modifier: KtModifierKeywordToken) : HLApplicatorInput {
+        override fun isValidFor(psi: PsiElement): Boolean {
+            if (psi !is KtModifierListOwner) return false
+            return psi.hasModifier(modifier)
+        }
+
+        operator fun component1() = modifier
+    }
+}

idea/idea-fir/src/org/jetbrains/kotlin/idea/fir/inspections/diagnosticBased/HLRedundantVisibilityModifierInspection.kt

@@ -0,0 +1,46 @@
+/*
+ * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.idea.fir.inspections.diagnosticBased
+
+import com.intellij.codeInspection.ProblemHighlightType
+import org.jetbrains.kotlin.idea.KotlinBundle
+import org.jetbrains.kotlin.idea.KotlinBundleIndependent
+import org.jetbrains.kotlin.idea.fir.api.AbstractHLDiagnosticBasedInspection
+import org.jetbrains.kotlin.idea.fir.api.applicator.*
+import org.jetbrains.kotlin.idea.fir.api.inputByDiagnosticProvider
+import org.jetbrains.kotlin.idea.fir.applicators.ApplicabilityRanges
+import org.jetbrains.kotlin.idea.fir.applicators.ModifierApplicators
+import org.jetbrains.kotlin.idea.frontend.api.fir.diagnostics.KtFirDiagnostic
+import org.jetbrains.kotlin.lexer.KtTokens
+import org.jetbrains.kotlin.psi.KtModifierListOwner
+import org.jetbrains.kotlin.psi.psiUtil.visibilityModifierType
+
+class HLRedundantVisibilityModifierInspection :
+    AbstractHLDiagnosticBasedInspection<KtModifierListOwner, KtFirDiagnostic.RedundantVisibilityModifier, ModifierApplicators.Modifier>(
+        elementType = KtModifierListOwner::class,
+        diagnosticType = KtFirDiagnostic.RedundantVisibilityModifier::class
+    ) {
+
+    override val inputByDiagnosticProvider =
+        inputByDiagnosticProvider<KtModifierListOwner, KtFirDiagnostic.RedundantVisibilityModifier, ModifierApplicators.Modifier> { diagnostic ->
+            val modifier = diagnostic.psi.visibilityModifierType() ?: return@inputByDiagnosticProvider null
+            ModifierApplicators.Modifier(modifier)
+        }
+
+    override val presentation: HLPresentation<KtModifierListOwner> = presentation {
+        highlightType(ProblemHighlightType.LIKE_UNUSED_SYMBOL)
+    }
+
+    override val applicabilityRange: HLApplicabilityRange<KtModifierListOwner> = ApplicabilityRanges.VISIBILITY_MODIFIER
+
+    override val applicator: HLApplicator<KtModifierListOwner, ModifierApplicators.Modifier> =
+        ModifierApplicators.removeModifierApplicator(
+            KtTokens.VISIBILITY_MODIFIERS,
+            KotlinBundle.lazyMessage("redundant.visibility.modifier")
+        ).with {
+            actionName { _, (modifier) -> KotlinBundleIndependent.message("remove.redundant.0.modifier", modifier.value) }
+        }
+}

idea/idea-fir/testData/inspectionsLocal/redundantVisibilityModifierFir/.firInspection

@@ -0,0 +1 @@
+org.jetbrains.kotlin.idea.fir.inspections.diagnosticBased.HLRedundantVisibilityModifierInspection

idea/idea-fir/testData/inspectionsLocal/redundantVisibilityModifierFir/publicFunInPublicClass.kt

@@ -0,0 +1,3 @@
+public class C {
+    <caret>public fun bar() {}
+}

idea/idea-fir/testData/inspectionsLocal/redundantVisibilityModifierFir/publicFunInPublicClass.kt.after

@@ -0,0 +1,3 @@
+public class C {
+    fun bar() {}
+}

idea/idea-fir/testData/inspectionsLocal/redundantVisibilityModifierFir/publicValInPublicClass.kt

@@ -0,0 +1,3 @@
+public class C {
+    <caret>public val foo: Int = 0
+}

idea/idea-fir/testData/inspectionsLocal/redundantVisibilityModifierFir/publicValInPublicClass.kt.after

@@ -0,0 +1,3 @@
+public class C {
+    val foo: Int = 0
+}

idea/idea-fir/tests/org/jetbrains/kotlin/idea/inspections/HLLocalInspectionTestGenerated.java

@@ -0,0 +1,110 @@
+/*
+ * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.idea.inspections;
+
+import com.intellij.testFramework.TestDataPath;
+import org.jetbrains.kotlin.test.JUnit3RunnerWithInners;
+import org.jetbrains.kotlin.test.KotlinTestUtils;
+import org.jetbrains.kotlin.test.util.KtTestUtil;
+import org.jetbrains.kotlin.test.TestMetadata;
+import org.junit.runner.RunWith;
+
+import java.io.File;
+import java.util.regex.Pattern;
+
+/** This class is generated by {@link org.jetbrains.kotlin.generators.tests.TestsPackage}. DO NOT MODIFY MANUALLY */
+@SuppressWarnings("all")
+@RunWith(JUnit3RunnerWithInners.class)
+public class HLLocalInspectionTestGenerated extends AbstractHLLocalInspectionTest {
+    @TestMetadata("idea/testData/inspectionsLocal/redundantVisibilityModifier")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class RedundantVisibilityModifier extends AbstractHLLocalInspectionTest {
+        private void runTest(String testDataFilePath) throws Exception {
+            KotlinTestUtils.runTest(this::doTest, this, testDataFilePath);
+        }
+
+        public void testAllFilesPresentInRedundantVisibilityModifier() throws Exception {
+            KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("idea/testData/inspectionsLocal/redundantVisibilityModifier"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), null, true);
+        }
+
+        @TestMetadata("internalInPrivateClass.kt")
+        public void testInternalInPrivateClass() throws Exception {
+            runTest("idea/testData/inspectionsLocal/redundantVisibilityModifier/internalInPrivateClass.kt");
+        }
+
+        @TestMetadata("overridePropertySetter.kt")
+        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/idea-fir/testData/inspectionsLocal")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class InspectionsLocal extends AbstractHLLocalInspectionTest {
+        private void runTest(String testDataFilePath) throws Exception {
+            KotlinTestUtils.runTest(this::doTest, this, testDataFilePath);
+        }
+
+        public void testAllFilesPresentInInspectionsLocal() throws Exception {
+            KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("idea/idea-fir/testData/inspectionsLocal"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), null, true);
+        }
+
+        @TestMetadata("idea/idea-fir/testData/inspectionsLocal/redundantVisibilityModifierFir")
+        @TestDataPath("$PROJECT_ROOT")
+        @RunWith(JUnit3RunnerWithInners.class)
+        public static class RedundantVisibilityModifierFir extends AbstractHLLocalInspectionTest {
+            private void runTest(String testDataFilePath) throws Exception {
+                KotlinTestUtils.runTest(this::doTest, this, testDataFilePath);
+            }
+
+            public void testAllFilesPresentInRedundantVisibilityModifierFir() throws Exception {
+                KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("idea/idea-fir/testData/inspectionsLocal/redundantVisibilityModifierFir"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), null, true);
+            }
+
+            @TestMetadata("publicFunInPublicClass.kt")
+            public void testPublicFunInPublicClass() throws Exception {
+                runTest("idea/idea-fir/testData/inspectionsLocal/redundantVisibilityModifierFir/publicFunInPublicClass.kt");
+            }
+
+            @TestMetadata("publicValInPublicClass.kt")
+            public void testPublicValInPublicClass() throws Exception {
+                runTest("idea/idea-fir/testData/inspectionsLocal/redundantVisibilityModifierFir/publicValInPublicClass.kt");
+            }
+        }
+    }
+}

idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/components/KtFirDiagnosticProvider.kt

@@ -5,16 +5,18 @@
 
 package org.jetbrains.kotlin.idea.frontend.api.fir.components
 
+import com.intellij.psi.PsiElement
 import org.jetbrains.kotlin.fir.FirSourceElement
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDiagnostic
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirPsiDiagnostic
 import org.jetbrains.kotlin.fir.analysis.diagnostics.toFirDiagnostic
 import org.jetbrains.kotlin.fir.diagnostics.ConeDiagnostic
+import org.jetbrains.kotlin.idea.fir.low.level.api.api.DiagnosticCheckerFilter
 import org.jetbrains.kotlin.idea.fir.low.level.api.api.collectDiagnosticsForFile
 import org.jetbrains.kotlin.idea.fir.low.level.api.api.getDiagnostics
 import org.jetbrains.kotlin.idea.frontend.api.ValidityToken
+import org.jetbrains.kotlin.idea.frontend.api.components.KtDiagnosticCheckerFilter
 import org.jetbrains.kotlin.idea.frontend.api.components.KtDiagnosticProvider
-import org.jetbrains.kotlin.idea.frontend.api.diagnostics.KtDiagnostic
 import org.jetbrains.kotlin.idea.frontend.api.diagnostics.KtDiagnosticWithPsi
 import org.jetbrains.kotlin.idea.frontend.api.fir.KtFirAnalysisSession
 import org.jetbrains.kotlin.idea.frontend.api.fir.diagnostics.KT_DIAGNOSTIC_CONVERTER
@@ -26,17 +28,26 @@ internal class KtFirDiagnosticProvider(
     override val analysisSession: KtFirAnalysisSession,
     override val token: ValidityToken,
 ) : KtDiagnosticProvider(), KtFirAnalysisSessionComponent {
-    override fun getDiagnosticsForElement(element: KtElement): Collection<KtDiagnosticWithPsi<*>> = withValidityAssertion {
-        element.getDiagnostics(firResolveState).map { it.asKtDiagnostic() }
+    override fun getDiagnosticsForElement(
+        element: KtElement,
+        filter: KtDiagnosticCheckerFilter
+    ): Collection<KtDiagnosticWithPsi<*>> = withValidityAssertion {
+        element.getDiagnostics(firResolveState, filter.asLLFilter()).map { it.asKtDiagnostic() }
     }
 
-    override fun collectDiagnosticsForFile(ktFile: KtFile): Collection<KtDiagnosticWithPsi<*>> =
-        ktFile.collectDiagnosticsForFile(firResolveState).map { it.asKtDiagnostic() }
+    override fun collectDiagnosticsForFile(ktFile: KtFile, filter: KtDiagnosticCheckerFilter): Collection<KtDiagnosticWithPsi<*>> =
+        ktFile.collectDiagnosticsForFile(firResolveState, filter.asLLFilter()).map { it.asKtDiagnostic() }
 
     fun firDiagnosticAsKtDiagnostic(diagnostic: FirPsiDiagnostic<*>): KtDiagnosticWithPsi<*> {
         return KT_DIAGNOSTIC_CONVERTER.convert(analysisSession, diagnostic as FirDiagnostic<*>)
     }
 
+    private fun KtDiagnosticCheckerFilter.asLLFilter() = when (this) {
+        KtDiagnosticCheckerFilter.ONLY_COMMON_CHECKERS -> DiagnosticCheckerFilter.ONLY_COMMON_CHECKERS
+        KtDiagnosticCheckerFilter.ONLY_EXTENDED_CHECKERS -> DiagnosticCheckerFilter.ONLY_EXTENDED_CHECKERS
+        KtDiagnosticCheckerFilter.EXTENDED_AND_COMMON_CHECKERS -> DiagnosticCheckerFilter.EXTENDED_AND_COMMON_CHECKERS
+    }
+
     fun coneDiagnosticAsKtDiagnostic(coneDiagnostic: ConeDiagnostic, source: FirSourceElement): KtDiagnosticWithPsi<*>? {
         val firDiagnostic = coneDiagnostic.toFirDiagnostic(source) ?: return null
         check(firDiagnostic is FirPsiDiagnostic<*>)

idea/resources-en/inspectionDescriptions/HLRedundantVisibilityModifier.html

@@ -0,0 +1,6 @@
+<html>
+<body>
+This inspection reports visibility modifiers which match the default visibility of an element
+(<b>public</b> for most elements, <b>protected</b> for members that override a protected member).
+</body>
+</html>

idea/resources-fir/META-INF/firInspections.xml

@@ -8,5 +8,14 @@
                      level="WARNING"
                      language="kotlin"
                      key="inspection.redundant.unit.return.type.display.name" bundle="messages.KotlinBundle"/>
+
+    <localInspection implementationClass="org.jetbrains.kotlin.idea.fir.inspections.diagnosticBased.HLRedundantVisibilityModifierInspection"
+                     groupPath="Kotlin"
+                     groupName="Redundant constructs"
+                     enabledByDefault="true"
+                     cleanupTool="true"
+                     level="WARNING"
+                     language="kotlin"
+                     key="inspection.redundant.visibility.modifier.display.name" bundle="messages.KotlinBundle"/>
   </extensions>
 </idea-plugin>

idea/testData/inspectionsLocal/redundantVisibilityModifier/.firInspection

@@ -0,0 +1 @@
+org.jetbrains.kotlin.idea.fir.inspections.diagnosticBased.HLRedundantVisibilityModifierInspection

idea/testData/inspectionsLocal/redundantVisibilityModifier/internalInPrivateClass.kt

@@ -1,3 +1,5 @@
 private class Foo {
     <caret>internal fun bar() {}
-}
+}
+
+// IGNORE_FIR KT-44939

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

@@ -1,3 +1,5 @@
 private class Foo {
     fun bar() {}
-}
+}
+
+// IGNORE_FIR KT-44939

idea/testData/inspectionsLocal/redundantVisibilityModifier/publicOverrideProtectedSetter3.kt

@@ -12,3 +12,5 @@ fun main() {
     val c = C()
     c.attribute = "test"
 }
+
+// IGNORE_FIR KT-44939

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

@@ -12,3 +12,5 @@ fun main() {
     val c = C()
     c.attribute = "test"
 }
+
+// IGNORE_FIR KT-44939

idea/testData/inspectionsLocal/redundantVisibilityModifier/publicOverrideProtectedSetter6.kt

@@ -15,4 +15,6 @@ class C : B() {
 fun main() {
     val c = C()
     c.attribute = "test"
-}
+}
+
+// IGNORE_FIR KT-44939

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

@@ -15,4 +15,6 @@ class C : B() {
 fun main() {
     val c = C()
     c.attribute = "test"
-}
+}
+
+// IGNORE_FIR KT-44939