Add inspection: Refactor sealed sub-class to object #KT-20305 Fixed

2018-02-12 20:50:36 +03:00
parent 7cf6c70fbf
commit df7968678a
31 changed files with 514 additions and 0 deletions
@@ -0,0 +1,5 @@
+<html>
+<body>
+This inspection reports direct inheritors of <b>sealed class</b> that have default constructors only and thus can be objects.
+</body>
+</html>
@@ -2765,6 +2765,15 @@
                     language="kotlin"
    />

+    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.CanSealedSubClassBeObjectInspection"
+                     displayName="Refactor Sealed Sub-class to Object"
+                     groupPath="Kotlin"
+                     groupName="Style issues"
+                     enabledByDefault="true"
+                     level="INFO"
+                     language="kotlin"
+    />
+
    <referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>

    <fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
@@ -0,0 +1,88 @@
+/*
+ * Copyright 2010-2017 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jetbrains.kotlin.idea.inspections
+
+import com.intellij.codeInspection.LocalInspectionToolSession
+import com.intellij.codeInspection.ProblemHighlightType
+import com.intellij.codeInspection.ProblemsHolder
+import com.intellij.psi.PsiElementVisitor
+import org.jetbrains.kotlin.asJava.classes.KtLightClassImpl
+import org.jetbrains.kotlin.idea.core.getModalityFromDescriptor
+import org.jetbrains.kotlin.idea.quickfix.sealedSubClassToObject.ConvertSealedSubClassToObjectFix
+import org.jetbrains.kotlin.idea.search.declarationsSearch.HierarchySearchRequest
+import org.jetbrains.kotlin.idea.search.declarationsSearch.searchInheritors
+import org.jetbrains.kotlin.lexer.KtTokens
+import org.jetbrains.kotlin.psi.KtClass
+import org.jetbrains.kotlin.psi.KtVisitorVoid
+
+class CanSealedSubClassBeObjectInspection : AbstractKotlinInspection() {
+    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession): PsiElementVisitor {
+        return object : KtVisitorVoid() {
+            override fun visitClass(klass: KtClass) {
+                if (!klass.hasModifier(KtTokens.SEALED_KEYWORD)) return
+                if (klass.getModalityFromDescriptor() != KtTokens.SEALED_KEYWORD) return
+
+                klass.getSubclasses()
+                    .withEmptyConstructors()
+                    .thatAreFinal()
+                    .thatHasNoInnerClasses()
+                    .thatHasNoCompanionObjects()
+                    .forEach { reportPossibleObject(it) }
+            }
+
+            private fun reportPossibleObject(klass: KtClass) {
+                val keyword = klass.getClassOrInterfaceKeyword() ?: return
+                holder.registerProblem(
+                    keyword,
+                    "Sealed Sub-class should be changed To Object",
+                    ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
+                    ConvertSealedSubClassToObjectFix()
+                )
+            }
+        }
+    }
+
+    private fun KtClass.getSubclasses(): List<KtLightClassImpl> {
+        return HierarchySearchRequest(this, this.useScope, false)
+            .searchInheritors().filterIsInstance<KtLightClassImpl>()
+    }
+
+    private fun List<KtLightClassImpl>.withEmptyConstructors(): List<KtClass> {
+        return map { it.kotlinOrigin }.filterIsInstance<KtClass>()
+            .filter { it.primaryConstructorParameters.isEmpty() }
+            .filter { klass -> klass.secondaryConstructors.all { cons -> cons.valueParameters.isEmpty() } }
+    }
+
+    private fun List<KtClass>.thatHasNoCompanionObjects(): List<KtClass> {
+        return filter { klass -> klass.companionObjects.isEmpty() }
+    }
+
+    private fun List<KtClass>.thatAreFinal(): List<KtClass> {
+        return filter { klass -> klass.getModalityFromDescriptor() == KtTokens.FINAL_KEYWORD }
+    }
+
+    private fun List<KtClass>.thatHasNoInnerClasses(): List<KtClass> {
+        return filter { klass -> klass.hasNoInnerClass() }
+    }
+
+    private fun KtClass.hasNoInnerClass(): Boolean {
+        val internalClasses = getBody()
+            ?.declarations
+            ?.filterIsInstance<KtClass>() ?: return true
+
+        return internalClasses.none { klass -> klass.isInner() }
+    }
+}
@@ -0,0 +1,105 @@
+/*
+ * Copyright 2000-2018 JetBrains s.r.o. 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.quickfix.sealedSubClassToObject
+
+import com.intellij.codeInspection.LocalQuickFix
+import com.intellij.codeInspection.ProblemDescriptor
+import com.intellij.lang.Language
+import com.intellij.openapi.project.Project
+import com.intellij.psi.JavaPsiFacade
+import com.intellij.psi.PsiElement
+import com.intellij.psi.impl.source.tree.JavaElementType
+import com.intellij.psi.search.searches.ReferencesSearch
+import org.jetbrains.kotlin.KtNodeTypes
+import org.jetbrains.kotlin.lexer.KtTokens
+import org.jetbrains.kotlin.psi.KtClass
+import org.jetbrains.kotlin.psi.KtPsiFactory
+import org.jetbrains.kotlin.psi.buildExpression
+import org.jetbrains.kotlin.psi.psiUtil.getParentOfType
+
+class ConvertSealedSubClassToObjectFix : LocalQuickFix {
+
+    override fun getFamilyName() = "Convert Sealed Sub-class to Object"
+
+    companion object {
+        val JAVA_LANG = Language.findLanguageByID("JAVA")
+        val KOTLIN_LANG = Language.findLanguageByID("kotlin")
+    }
+
+    override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
+        val klass = descriptor.psiElement.getParentOfType<KtClass>(false) ?: return
+
+        changeInstances(klass)
+        changeDeclaration(klass)
+    }
+
+    /**
+     * Changes declaration of class to object.
+     */
+    private fun changeDeclaration(element: KtClass) {
+        val factory = KtPsiFactory(element)
+
+        element.changeToObject(factory)
+        element.transformToObject(factory)
+    }
+
+    private fun KtClass.changeToObject(factory: KtPsiFactory) {
+        getClassOrInterfaceKeyword()?.replace(factory.createExpression(KtTokens.OBJECT_KEYWORD.value))
+        secondaryConstructors.forEach { delete() }
+        primaryConstructor?.delete()
+    }
+
+    private fun KtClass.transformToObject(factory: KtPsiFactory) {
+        replace(factory.createObject(text))
+    }
+
+    /**
+     * Replace instantiations of the class with links to the singleton instance of the object.
+     */
+    private fun changeInstances(klass: KtClass) {
+        mapReferencesByLanguage(klass)
+            .apply {
+                replaceKotlin(klass)
+                replaceJava(klass)
+            }
+    }
+
+    /**
+     * Map references to this class by language
+     */
+    private fun mapReferencesByLanguage(klass: KtClass) = ReferencesSearch.search(klass)
+        .groupBy({ it.element.language }, { it.element.parent })
+
+    /**
+     * Replace Kotlin instantiations to a straightforward call to the singleton.
+     */
+    private fun Map<Language, List<PsiElement>>.replaceKotlin(klass: KtClass) {
+        val list = this[KOTLIN_LANG] ?: return
+        val singletonCall = KtPsiFactory(klass).buildExpression { appendName(klass.nameAsSafeName) }
+
+        list.filter { it.node.elementType == KtNodeTypes.CALL_EXPRESSION }
+            .forEach { it.replace(singletonCall) }
+    }
+
+    /**
+     * Replace Java instantiations to an instance of the object, unless it is the only thing
+     * done in the statement, in which IDEA will consider wrong, so I delete the line.
+     */
+    private fun Map<Language, List<PsiElement>>.replaceJava(klass: KtClass) {
+        val list = this[JAVA_LANG] ?: return
+        val first = list.firstOrNull() ?: return
+        val elementFactory = JavaPsiFacade.getElementFactory(klass.project)
+        val javaSingletonCall = elementFactory.createExpressionFromText("${klass.name}.INSTANCE", first)
+
+        list.filter { it.node.elementType == JavaElementType.NEW_EXPRESSION }
+            .forEach {
+                when (it.parent.node.elementType) {
+                    JavaElementType.EXPRESSION_STATEMENT -> it.delete()
+                    else -> it.replace(javaSingletonCall)
+                }
+            }
+    }
+}
@@ -0,0 +1,50 @@
+<problems>
+    <problem>
+        <file>sealed.kt</file>
+        <line>8</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="temp:///src/sealed.kt" />
+        <problem_class severity="INFORMATION" attribute_key="INFO_ATTRIBUTES">Sealed Sub-class should be changed To Object</problem_class>
+        <description>Sealed Sub-class should be changed To Object</description>
+    </problem>
+    <problem>
+        <file>sealed.kt</file>
+        <line>10</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="temp:///src/sealed.kt" />
+        <problem_class severity="INFORMATION" attribute_key="INFO_ATTRIBUTES">Sealed Sub-class should be changed To Object</problem_class>
+        <description>Sealed Sub-class should be changed To Object</description>
+    </problem>
+    <problem>
+        <file>sealed.kt</file>
+        <line>13</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="temp:///src/sealed.kt" />
+        <problem_class severity="INFORMATION" attribute_key="INFO_ATTRIBUTES">Sealed Sub-class should be changed To Object</problem_class>
+        <description>Sealed Sub-class should be changed To Object</description>
+    </problem>
+    <problem>
+        <file>sealed.kt</file>
+        <line>15</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="temp:///src/sealed.kt" />
+        <problem_class severity="INFORMATION" attribute_key="INFO_ATTRIBUTES">Sealed Sub-class should be changed To Object</problem_class>
+        <description>Sealed Sub-class should be changed To Object</description>
+    </problem>
+    <problem>
+        <file>sealed.kt</file>
+        <line>41</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="temp:///src/sealed.kt" />
+        <problem_class severity="INFORMATION" attribute_key="INFO_ATTRIBUTES">Sealed Sub-class should be changed To Object</problem_class>
+        <description>Sealed Sub-class should be changed To Object</description>
+    </problem>
+    <problem>
+        <file>sealed.kt</file>
+        <line>45</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="temp:///src/sealed.kt" />
+        <problem_class severity="INFORMATION" attribute_key="INFO_ATTRIBUTES">Sealed Sub-class should be changed To Object</problem_class>
+        <description>Sealed Sub-class should be changed To Object</description>
+    </problem>
+</problems>
@@ -0,0 +1 @@
+// INSPECTION_CLASS: org.jetbrains.kotlin.idea.inspections.CanSealedSubClassBeObjectInspection
@@ -0,0 +1,47 @@
+sealed class Sealed {
+    open class OpenSubSealedNestedNoChild() : Sealed()
+
+    open class SubSealedNestedWithChild  : Sealed()
+
+    class SubSealedNestedChild() : SubSealedNestedWithChild()
+
+    final class FinalSubSealedNested() : Sealed()
+
+    class SubSealedNested() : Sealed()
+}
+
+class SubSealedParentheses() : Sealed()
+
+class SubSealedNoParentheses : Sealed()
+
+open class SubSealedWithChild : Sealed()
+
+class SubSealedChild : SubSealedWithChild()
+
+class SubSealedParameters(val value : String) : Sealed()
+
+object SubSealedObject() : Sealed()
+
+class SubSealedObjectWithPrimaryConstructor(parameter: String) : Sealed()
+
+class SubSealedObjectWithSecondaryConstructor() : Sealed() {
+    constructor(parameter: String): this()
+}
+
+object AlreadyObject: Sealed()
+
+class SubSealedWithCompanion: Sealed() {
+    companion object
+}
+
+class SubSealedWithInner: Sealed() {
+    inner class Inner
+}
+
+class SubSealedWithNested: Sealed() {
+    class Nested
+}
+
+class SubSealedWithFunction: Sealed() {
+    fun internalFunction() { }
+}
@@ -0,0 +1 @@
+org.jetbrains.kotlin.idea.inspections.CanSealedSubClassBeObjectInspection
@@ -0,0 +1,3 @@
+sealed class Sealed
+
+private <caret>class SubSealed : Sealed()
@@ -0,0 +1,3 @@
+sealed class Sealed
+
+private object SubSealed : Sealed()
@@ -0,0 +1,3 @@
+sealed class Sealed
+
+<caret>class SubSealed() : Sealed()
@@ -0,0 +1,3 @@
+sealed class Sealed
+
+object SubSealed : Sealed()
@@ -0,0 +1,3 @@
+sealed class Sealed
+
+<caret>class SubSealed : Sealed()
@@ -0,0 +1,3 @@
+sealed class Sealed
+
+object SubSealed : Sealed()
@@ -0,0 +1,11 @@
+import seal.*
+
+class JavaSealedTest {
+    public void testNesting() {
+        new SubSealed.Nested();
+        Supplier<SubSealed.Nested> nestedSupplier = SubSealed.Nested::new;
+
+        SubSealed.INSTANCE.internalFunction();
+        Runnable noArgFunction = SubSealed.INSTANCE::internalFunction;
+    }
+}
@@ -0,0 +1,16 @@
+import seal.*
+
+class KotlinSealedTest {
+    fun testSeal() {
+        val internalFunction = SubSealed::internalFunction
+
+        val nestedClass = SubSealed.Nested()
+
+        SubSealed.Nested()
+        SubSealed::Nested
+
+        SubSealed.internalFunction()
+        SubSealed::internalFunction
+        SubSealed::internalFunction
+    }
+}
@@ -0,0 +1,9 @@
+package seal
+
+sealed class Sealed
+
+object SubSealed : Sealed() {
+    class Nested
+
+    fun internalFunction() {}
+}
@@ -0,0 +1,11 @@
+import seal.*
+
+class JavaSealedTest {
+    public void testNesting() {
+        new SubSealed.Nested();
+        Supplier<SubSealed.Nested> nestedSupplier = SubSealed.Nested::new;
+
+        new SubSealed().internalFunction();
+        Runnable noArgFunction = new SubSealed()::internalFunction;
+    }
+}
@@ -0,0 +1,16 @@
+import seal.*
+
+class KotlinSealedTest {
+    fun testSeal() {
+        val internalFunction = SubSealed()::internalFunction
+
+        val nestedClass = SubSealed.Nested()
+
+        SubSealed.Nested()
+        SubSealed::Nested
+
+        SubSealed().internalFunction()
+        SubSealed()::internalFunction
+        SubSealed::internalFunction
+    }
+}
@@ -0,0 +1,9 @@
+package seal
+
+sealed class Sealed
+
+<caret>class SubSealed : Sealed() {
+    class Nested
+
+    fun internalFunction() {}
+}
@@ -0,0 +1,5 @@
+{
+  "mainFile": "Seal.kt",
+  "inspectionClass": "org.jetbrains.kotlin.idea.inspections.CanSealedSubClassBeObjectInspection",
+  "fix": "Convert Sealed Sub-class to Object"
+}
@@ -0,0 +1,13 @@
+import seal.*
+
+class JavaSealedTest {
+    Sealed sealedInsideClass = SubSealed.INSTANCE;
+
+    public void testSeal() {
+        Sealed sealedInsideMethod = SubSealed.INSTANCE;
+
+        SubSealed.INSTANCE.toString();
+
+        // Will be deleted because Java doesn't allow a expression to be used as a statement
+    }
+}
@@ -0,0 +1,13 @@
+import seal.*
+
+val sealedOutsideClass = SubSealed
+
+class KotlinSealedTest {
+    val sealedInsideClass = SubSealed
+
+    fun testSeal() {
+        val sealedInsideMethod = SubSealed
+
+        SubSealed
+    }
+}
@@ -0,0 +1,5 @@
+package seal
+
+sealed class Sealed
+
+object SubSealed : Sealed()
@@ -0,0 +1,14 @@
+import seal.*
+
+class JavaSealedTest {
+    Sealed sealedInsideClass = new SubSealed();
+
+    public void testSeal() {
+        Sealed sealedInsideMethod = new SubSealed();
+
+        new SubSealed().toString();
+
+        // Will be deleted because Java doesn't allow a expression to be used as a statement
+        new SubSealed();
+    }
+}
@@ -0,0 +1,13 @@
+import seal.*
+
+val sealedOutsideClass = SubSealed()
+
+class KotlinSealedTest {
+    val sealedInsideClass = SubSealed()
+
+    fun testSeal() {
+        val sealedInsideMethod = SubSealed()
+
+        SubSealed()
+    }
+}
@@ -0,0 +1,5 @@
+package seal
+
+sealed class Sealed
+
+<caret>class SubSealed : Sealed()
@@ -0,0 +1,5 @@
+{
+  "mainFile": "Seal.kt",
+  "inspectionClass": "org.jetbrains.kotlin.idea.inspections.CanSealedSubClassBeObjectInspection",
+  "fix": "Convert Sealed Sub-class to Object"
+}
@@ -372,6 +372,12 @@ public class InspectionTestGenerated extends AbstractInspectionTest {
            doTest(fileName);
        }

+        @TestMetadata("sealedSubClassCanBeObject/inspectionData/inspections.test")
+        public void testSealedSubClassCanBeObject_inspectionData_Inspections_test() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/sealedSubClassCanBeObject/inspectionData/inspections.test");
+            doTest(fileName);
+        }
+
        @TestMetadata("sortModifiers/inspectionData/inspections.test")
        public void testSortModifiers_inspectionData_Inspections_test() throws Exception {
            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/sortModifiers/inspectionData/inspections.test");
@@ -1234,6 +1234,33 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
        }
    }

+    @TestMetadata("idea/testData/inspectionsLocal/convertSealedSubClassToObject")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class ConvertSealedSubClassToObject extends AbstractLocalInspectionTest {
+        public void testAllFilesPresentInConvertSealedSubClassToObject() throws Exception {
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/convertSealedSubClassToObject"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), TargetBackend.ANY, true);
+        }
+
+        @TestMetadata("convertSubClassWithModifiers.kt")
+        public void testConvertSubClassWithModifiers() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/convertSealedSubClassToObject/convertSubClassWithModifiers.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("convertSubClassWithParentheses.kt")
+        public void testConvertSubClassWithParentheses() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/convertSealedSubClassToObject/convertSubClassWithParentheses.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("convertSubClassWithoutParentheses.kt")
+        public void testConvertSubClassWithoutParentheses() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/convertSealedSubClassToObject/convertSubClassWithoutParentheses.kt");
+            doTest(fileName);
+        }
+    }
+
    @TestMetadata("idea/testData/inspectionsLocal/copyWithoutNamedArguments")
    @TestDataPath("$PROJECT_ROOT")
    @RunWith(JUnit3RunnerWithInners.class)
@@ -25,6 +25,18 @@ public class MultiFileLocalInspectionTestGenerated extends AbstractMultiFileLoca
        KotlinTestUtils.assertAllTestsPresentInSingleGeneratedClass(this.getClass(), new File("idea/testData/multiFileLocalInspections"), Pattern.compile("^(.+)\\.test$"), TargetBackend.ANY);
    }

+    @TestMetadata("convertSealedSubClassToObject/convertCallableReferenceUsages/convertCallableReferenceUsages.test")
+    public void testConvertSealedSubClassToObject_convertCallableReferenceUsages_ConvertCallableReferenceUsages() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/multiFileLocalInspections/convertSealedSubClassToObject/convertCallableReferenceUsages/convertCallableReferenceUsages.test");
+        doTest(fileName);
+    }
+
+    @TestMetadata("convertSealedSubClassToObject/convertInOtherFiles/convertInOtherFiles.test")
+    public void testConvertSealedSubClassToObject_convertInOtherFiles_ConvertInOtherFiles() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/multiFileLocalInspections/convertSealedSubClassToObject/convertInOtherFiles/convertInOtherFiles.test");
+        doTest(fileName);
+    }
+
    @TestMetadata("moveFileToPackageMatchingDirectory/moveToDefaultDirectory/moveToDefaultDirectory.test")
    public void testMoveFileToPackageMatchingDirectory_moveToDefaultDirectory_MoveToDefaultDirectory() throws Exception {
        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/multiFileLocalInspections/moveFileToPackageMatchingDirectory/moveToDefaultDirectory/moveToDefaultDirectory.test");
				`@@ -0,0 +1 @@`
				`// INSPECTION_CLASS: org.jetbrains.kotlin.idea.inspections.CanSealedSubClassBeObjectInspection`
				`@@ -0,0 +1 @@`
				`org.jetbrains.kotlin.idea.inspections.CanSealedSubClassBeObjectInspection`