diff --git a/idea/idea.iml b/idea/idea.iml index 2bed2eef6a6..f00aeb631f4 100644 --- a/idea/idea.iml +++ b/idea/idea.iml @@ -56,5 +56,6 @@ + \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantModalityModifierInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantModalityModifierInspection.kt index 30a3cf6a122..54513ae959e 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantModalityModifierInspection.kt +++ b/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 + } } diff --git a/idea/testData/inspections/allOpenSimple/inspectionData/expected.xml b/idea/testData/inspections/allOpenSimple/inspectionData/expected.xml new file mode 100644 index 00000000000..a43466e1377 --- /dev/null +++ b/idea/testData/inspections/allOpenSimple/inspectionData/expected.xml @@ -0,0 +1,19 @@ + + + test.kt + 22 + light_idea_test_case + + Redundant modality modifier + Redundant modality modifier + + + + test.kt + 23 + light_idea_test_case + + Redundant modality modifier + Redundant modality modifier + + diff --git a/idea/testData/inspections/allOpenSimple/inspectionData/inspections.test b/idea/testData/inspections/allOpenSimple/inspectionData/inspections.test new file mode 100644 index 00000000000..be84d245e96 --- /dev/null +++ b/idea/testData/inspections/allOpenSimple/inspectionData/inspections.test @@ -0,0 +1 @@ +// INSPECTION_CLASS: org.jetbrains.kotlin.idea.inspections.RedundantModalityModifierInspection diff --git a/idea/testData/inspections/allOpenSimple/test.kt b/idea/testData/inspections/allOpenSimple/test.kt new file mode 100644 index 00000000000..1c76eae3c72 --- /dev/null +++ b/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() {} +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java index e4f293f7466..e1b3c291bf1 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java +++ b/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"); diff --git a/plugins/allopen/allopen-cli/src/AllOpenDeclarationAttributeAltererExtension.kt b/plugins/allopen/allopen-cli/src/AllOpenDeclarationAttributeAltererExtension.kt index 12d7d2e3de3..0c6d92e5073 100644 --- a/plugins/allopen/allopen-cli/src/AllOpenDeclarationAttributeAltererExtension.kt +++ b/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 } } \ No newline at end of file diff --git a/plugins/allopen/allopen-ide/src/IdeAllOpenDeclarationAttributeAltererExtension.kt b/plugins/allopen/allopen-ide/src/IdeAllOpenDeclarationAttributeAltererExtension.kt index 96671bc65c0..298defbca3a 100644 --- a/plugins/allopen/allopen-ide/src/IdeAllOpenDeclarationAttributeAltererExtension.kt +++ b/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 { + if (ApplicationManager.getApplication().isUnitTestMode) { + return ANNOTATIONS_FOR_TESTS + } + val module = ModuleUtilCore.findModuleForPsiElement(modifierListOwner) ?: return emptyList() return cache.value.getOrPut(module) { diff --git a/plugins/plugins-tests/tests/org/jetbrains/kotlin/allopen/AbstractBytecodeListingTestForAllOpen.kt b/plugins/plugins-tests/tests/org/jetbrains/kotlin/allopen/AbstractBytecodeListingTestForAllOpen.kt index bb338b66d21..ead367c4916 100644 --- a/plugins/plugins-tests/tests/org/jetbrains/kotlin/allopen/AbstractBytecodeListingTestForAllOpen.kt +++ b/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)) } } \ No newline at end of file