Check if modality is refined in "Redundant modifier" inspection

idea/idea.iml

@@ -56,5 +56,6 @@
     <orderEntry type="library" scope="TEST" name="native-platform-uberjar" level="project" />
     <orderEntry type="module" module-name="allopen-ide" scope="TEST" />
     <orderEntry type="module" module-name="noarg-ide" scope="TEST" />
+    <orderEntry type="module" module-name="descriptors" />
   </component>
 </module>

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

@@ -18,23 +18,57 @@ package org.jetbrains.kotlin.idea.inspections
 
 import com.intellij.codeInspection.*
 import com.intellij.psi.PsiElementVisitor
+import com.intellij.psi.tree.IElementType
+import org.jetbrains.kotlin.descriptors.Modality
+import org.jetbrains.kotlin.extensions.DeclarationAttributeAltererExtension
+import org.jetbrains.kotlin.idea.caches.resolve.analyze
 import org.jetbrains.kotlin.idea.core.implicitModality
+import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.psi.KtDeclaration
 import org.jetbrains.kotlin.psi.KtVisitorVoid
 import org.jetbrains.kotlin.psi.psiUtil.modalityModifier
+import org.jetbrains.kotlin.resolve.BindingContext
+import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
 
 class RedundantModalityModifierInspection : AbstractKotlinInspection(), CleanupLocalInspectionTool {
     override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession): PsiElementVisitor {
         return object : KtVisitorVoid() {
             override fun visitDeclaration(declaration: KtDeclaration) {
                 val modalityModifier = declaration.modalityModifier() ?: return
-                if (modalityModifier.node.elementType == declaration.implicitModality()) {
-                    holder.registerProblem(modalityModifier,
-                                           "Redundant modality modifier",
-                                           ProblemHighlightType.LIKE_UNUSED_SYMBOL,
-                                           RemoveModifierFix("Remove redundant modality modifier"))
-                }
+                val modalityModifierType = modalityModifier.node.elementType
+                val implicitModality = declaration.implicitModality()
+
+                if (modalityModifierType != implicitModality) return
+
+                // Descriptor may have the different modality (in case of all-open plugin enabled, for example)
+                val modalityFromPsi = getModality(implicitModality)
+                if (modalityFromPsi != null && hasRefinedModalityInDescriptor(declaration, modalityFromPsi)) return
+
+                holder.registerProblem(modalityModifier,
+                                       "Redundant modality modifier",
+                                       ProblemHighlightType.LIKE_UNUSED_SYMBOL,
+                                       RemoveModifierFix("Remove redundant modality modifier"))
             }
         }
     }
+
+    private fun hasRefinedModalityInDescriptor(
+            declaration: KtDeclaration,
+            implicitModality: Modality
+    ): Boolean {
+        val bindingContext = declaration.analyze(BodyResolveMode.PARTIAL)
+        val descriptor = bindingContext[BindingContext.DECLARATION_TO_DESCRIPTOR, declaration] ?: return false
+        return DeclarationAttributeAltererExtension.getInstances(declaration.project).any {
+            it.refineDeclarationModality(declaration, descriptor, descriptor.containingDeclaration,
+                                         implicitModality, bindingContext) != null
+        }
+    }
+
+    private fun getModality(modalityToken: IElementType): Modality? = when (modalityToken) {
+        KtTokens.FINAL_KEYWORD -> Modality.FINAL
+        KtTokens.SEALED_KEYWORD -> Modality.SEALED
+        KtTokens.OPEN_KEYWORD -> Modality.OPEN
+        KtTokens.ABSTRACT_KEYWORD -> Modality.ABSTRACT
+        else -> null
+    }
 }

idea/testData/inspections/allOpenSimple/inspectionData/expected.xml

@@ -0,0 +1,19 @@
+<problems>
+    <problem>
+        <file>test.kt</file>
+        <line>22</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
+        <problem_class severity="WARNING" attribute_key="NOT_USED_ELEMENT_ATTRIBUTES">Redundant modality modifier</problem_class>
+        <description>Redundant modality modifier</description>
+    </problem>
+
+    <problem>
+        <file>test.kt</file>
+        <line>23</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
+        <problem_class severity="WARNING" attribute_key="NOT_USED_ELEMENT_ATTRIBUTES">Redundant modality modifier</problem_class>
+        <description>Redundant modality modifier</description>
+    </problem>
+</problems>

idea/testData/inspections/allOpenSimple/inspectionData/inspections.test

@@ -0,0 +1 @@
+// INSPECTION_CLASS: org.jetbrains.kotlin.idea.inspections.RedundantModalityModifierInspection

idea/testData/inspections/allOpenSimple/test.kt

@@ -0,0 +1,24 @@
+annotation class AllOpen
+
+@AllOpen
+class TestWithAllOpen {
+    val prop: String = ""
+    fun method() {}
+
+    open val openProp: String = ""
+    open fun openMethod() {}
+
+    final val finalProp: String = ""
+    final fun finalMethod() {}
+}
+
+class TestWithoutAllOpen {
+    val prop: String = ""
+    fun method() {}
+
+    open val openProp: String = ""
+    open fun openMethod() {}
+
+    final val finalProp: String = ""
+    final fun finalMethod() {}
+}

idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java

@@ -119,6 +119,12 @@ public class InspectionTestGenerated extends AbstractInspectionTest {
             KotlinTestUtils.assertAllTestsPresentInSingleGeneratedClass(this.getClass(), new File("idea/testData/inspections"), Pattern.compile("^(inspections\\.test)$"), TargetBackend.ANY);
         }
 
+        @TestMetadata("allOpenSimple/inspectionData/inspections.test")
+        public void testAllOpenSimple_inspectionData_Inspections_test() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/allOpenSimple/inspectionData/inspections.test");
+            doTest(fileName);
+        }
+
         @TestMetadata("androidIllegalIdentifiers/inspectionData/inspections.test")
         public void testAndroidIllegalIdentifiers_inspectionData_Inspections_test() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/androidIllegalIdentifiers/inspectionData/inspections.test");

plugins/allopen/allopen-cli/src/AllOpenDeclarationAttributeAltererExtension.kt

@@ -16,6 +16,8 @@
 
 package org.jetbrains.kotlin.allopen
 
+import org.jetbrains.annotations.TestOnly
+import org.jetbrains.kotlin.descriptors.ClassDescriptor
 import org.jetbrains.kotlin.descriptors.DeclarationDescriptor
 import org.jetbrains.kotlin.descriptors.Modality
 import org.jetbrains.kotlin.extensions.AnnotationBasedExtension
@@ -31,6 +33,11 @@ class CliAllOpenDeclarationAttributeAltererExtension(
 }
 
 abstract class AbstractAllOpenDeclarationAttributeAltererExtension : DeclarationAttributeAltererExtension, AnnotationBasedExtension {
+    companion object {
+        @TestOnly
+        val ANNOTATIONS_FOR_TESTS = listOf("AllOpen", "AllOpen2", "test.AllOpen")
+    }
+
     override fun refineDeclarationModality(
             modifierListOwner: KtModifierListOwner,
             declaration: DeclarationDescriptor?,
@@ -42,14 +49,14 @@ abstract class AbstractAllOpenDeclarationAttributeAltererExtension : Declaration
             return null
         }
 
-        // Explicit final
-        if (modifierListOwner.hasModifier(KtTokens.FINAL_KEYWORD)) {
-            return Modality.FINAL
+        val descriptor = declaration as? ClassDescriptor ?: containingDeclaration ?: return null
+        if (descriptor.hasSpecialAnnotation(modifierListOwner)) {
+            return if (modifierListOwner.hasModifier(KtTokens.FINAL_KEYWORD))
+                Modality.FINAL // Explicit final
+            else
+                Modality.OPEN
         }
 
-        val descriptor = declaration ?: containingDeclaration ?: return null
-        if (descriptor.hasSpecialAnnotation(modifierListOwner)) return Modality.OPEN
-
         return null
     }
 }

plugins/allopen/allopen-ide/src/IdeAllOpenDeclarationAttributeAltererExtension.kt

@@ -16,10 +16,10 @@
 
 package org.jetbrains.kotlin.allopen.ide
 
+import com.intellij.openapi.application.ApplicationManager
 import com.intellij.openapi.module.Module
 import com.intellij.openapi.module.ModuleUtilCore
 import com.intellij.openapi.project.Project
-import com.intellij.openapi.roots.ProjectRootManager
 import com.intellij.openapi.roots.ProjectRootModificationTracker
 import com.intellij.psi.util.CachedValue
 import com.intellij.psi.util.CachedValueProvider
@@ -41,6 +41,10 @@ class IdeAllOpenDeclarationAttributeAltererExtension(val project: Project) : Abs
     }
 
     override fun getAnnotationFqNames(modifierListOwner: KtModifierListOwner): List<String> {
+        if (ApplicationManager.getApplication().isUnitTestMode) {
+            return ANNOTATIONS_FOR_TESTS
+        }
+
         val module = ModuleUtilCore.findModuleForPsiElement(modifierListOwner) ?: return emptyList()
 
         return cache.value.getOrPut(module) {

plugins/plugins-tests/tests/org/jetbrains/kotlin/allopen/AbstractBytecodeListingTestForAllOpen.kt

@@ -21,12 +21,9 @@ import org.jetbrains.kotlin.codegen.AbstractBytecodeListingTest
 import org.jetbrains.kotlin.extensions.DeclarationAttributeAltererExtension
 
 abstract class AbstractBytecodeListingTestForAllOpen : AbstractBytecodeListingTest() {
-    private companion object {
-        val ALLOPEN_ANNOTATIONS = listOf("AllOpen", "AllOpen2", "test.AllOpen")
-    }
-
     override fun setupEnvironment(environment: KotlinCoreEnvironment) {
         DeclarationAttributeAltererExtension.registerExtension(
-                environment.project, CliAllOpenDeclarationAttributeAltererExtension(ALLOPEN_ANNOTATIONS))
+                environment.project, CliAllOpenDeclarationAttributeAltererExtension(
+                AbstractAllOpenDeclarationAttributeAltererExtension.ANNOTATIONS_FOR_TESTS))
     }
 }