diff --git a/idea/resources/inspectionDescriptions/CanSealedSubClassBeObject.html b/idea/resources/inspectionDescriptions/CanSealedSubClassBeObject.html index 4a8662c3ede..af276fbf04a 100644 --- a/idea/resources/inspectionDescriptions/CanSealedSubClassBeObject.html +++ b/idea/resources/inspectionDescriptions/CanSealedSubClassBeObject.html @@ -1,5 +1,5 @@ -This inspection reports direct inheritors of sealed class that have default constructors only and thus can be objects. +This inspection reports direct inheritors of sealed class that have no state and thus can be objects. \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/CanSealedSubClassBeObjectInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/CanSealedSubClassBeObjectInspection.kt index 2ea70008a35..c951da6f93a 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/CanSealedSubClassBeObjectInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/CanSealedSubClassBeObjectInspection.kt @@ -20,13 +20,18 @@ 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.caches.resolve.resolveToDescriptorIfAny import org.jetbrains.kotlin.idea.core.getModalityFromDescriptor import org.jetbrains.kotlin.idea.quickfix.sealedSubClassToObject.ConvertSealedSubClassToObjectFix +import org.jetbrains.kotlin.idea.refactoring.isAbstract 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.KtProperty import org.jetbrains.kotlin.psi.KtVisitorVoid +import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils +import org.jetbrains.kotlin.resolve.descriptorUtil.getSuperClassNotAny class CanSealedSubClassBeObjectInspection : AbstractKotlinInspection() { override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession): PsiElementVisitor { @@ -35,13 +40,16 @@ class CanSealedSubClassBeObjectInspection : AbstractKotlinInspection() { if (!klass.hasModifier(KtTokens.SEALED_KEYWORD)) return if (klass.getModalityFromDescriptor() != KtTokens.SEALED_KEYWORD) return - klass.getSubclasses() + val candidates = klass.getSubclasses() .withEmptyConstructors() .thatAreFinal() .thatHasNoTypeParameters() .thatHasNoInnerClasses() .thatHasNoCompanionObjects() - .forEach { reportPossibleObject(it) } + .thatHasNoState() + if (candidates.isEmpty() || !klass.hasNoState() || !klass.baseClassHasNoState()) return + + candidates.forEach { reportPossibleObject(it) } } private fun reportPossibleObject(klass: KtClass) { @@ -56,6 +64,14 @@ class CanSealedSubClassBeObjectInspection : AbstractKotlinInspection() { } } + private tailrec fun KtClass.baseClassHasNoState(): Boolean { + val descriptor = resolveToDescriptorIfAny() ?: return false + val superDescriptor = descriptor.getSuperClassNotAny() ?: return true // No super class -- no state + val superClass = DescriptorToSourceUtils.descriptorToDeclaration(superDescriptor) as? KtClass ?: return false + if (!superClass.hasNoState()) return false + return superClass.baseClassHasNoState() + } + private fun KtClass.getSubclasses(): List { return HierarchySearchRequest(this, this.useScope, false) .searchInheritors().filterIsInstance() @@ -83,6 +99,28 @@ class CanSealedSubClassBeObjectInspection : AbstractKotlinInspection() { return filter { klass -> klass.hasNoInnerClass() } } + private fun List.thatHasNoState(): List { + return filter { it.hasNoState() } + } + + private fun KtClass.hasNoState(): Boolean { + if (primaryConstructor?.valueParameters?.isNotEmpty() == true) return false + val body = getBody() + return body == null || run { + val properties = body.declarations.filterIsInstance() + properties.none { property -> + // Simplified "backing field required" + when { + property.isAbstract() -> false + property.initializer != null -> true + property.delegate != null -> false + !property.isVar -> property.getter == null + else -> property.getter == null || property.setter == null + } + } + } + } + private fun KtClass.hasNoInnerClass(): Boolean { val internalClasses = getBody() ?.declarations diff --git a/idea/testData/inspectionsLocal/convertSealedSubClassToObject/baseState.kt b/idea/testData/inspectionsLocal/convertSealedSubClassToObject/baseState.kt new file mode 100644 index 00000000000..e1d6be376cb --- /dev/null +++ b/idea/testData/inspectionsLocal/convertSealedSubClassToObject/baseState.kt @@ -0,0 +1,7 @@ +// PROBLEM: none + +abstract class Base(var x: String) + +sealed class Sealed(s: String) : Base(s) + +class Derived : Sealed("123") \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/convertSealedSubClassToObject/fakeState.kt b/idea/testData/inspectionsLocal/convertSealedSubClassToObject/fakeState.kt new file mode 100644 index 00000000000..9b4feeddb1f --- /dev/null +++ b/idea/testData/inspectionsLocal/convertSealedSubClassToObject/fakeState.kt @@ -0,0 +1,18 @@ +// WITH_RUNTIME + +abstract class Base { + var s: String + get() = "Hello" + set(value) {} +} + +sealed class Sealed : Base() { + open val x: List + get() = emptyList() +} + +class Derived : Sealed() { + var length: Int + get() = s.length + set(value) {} +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/convertSealedSubClassToObject/fakeState.kt.after b/idea/testData/inspectionsLocal/convertSealedSubClassToObject/fakeState.kt.after new file mode 100644 index 00000000000..4472ccbd688 --- /dev/null +++ b/idea/testData/inspectionsLocal/convertSealedSubClassToObject/fakeState.kt.after @@ -0,0 +1,18 @@ +// WITH_RUNTIME + +abstract class Base { + var s: String + get() = "Hello" + set(value) {} +} + +sealed class Sealed : Base() { + open val x: List + get() = emptyList() +} + +object Derived : Sealed() { + var length: Int + get() = s.length + set(value) {} +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/convertSealedSubClassToObject/ownState.kt b/idea/testData/inspectionsLocal/convertSealedSubClassToObject/ownState.kt new file mode 100644 index 00000000000..2ba5f9e3555 --- /dev/null +++ b/idea/testData/inspectionsLocal/convertSealedSubClassToObject/ownState.kt @@ -0,0 +1,8 @@ +// PROBLEM: none +// WITH_RUNTIME + +sealed class SC { + class U : SC() { + val a = mutableListOf() + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/convertSealedSubClassToObject/sealedState.kt b/idea/testData/inspectionsLocal/convertSealedSubClassToObject/sealedState.kt new file mode 100644 index 00000000000..fa95b431c5a --- /dev/null +++ b/idea/testData/inspectionsLocal/convertSealedSubClassToObject/sealedState.kt @@ -0,0 +1,6 @@ +// PROBLEM: none + +sealed class SC { + var u = 0 + class C : SC() {} +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java index cdae52a8182..5c980efeda2 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -1225,6 +1225,11 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/convertSealedSubClassToObject"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), TargetBackend.ANY, true); } + @TestMetadata("baseState.kt") + public void testBaseState() throws Exception { + runTest("idea/testData/inspectionsLocal/convertSealedSubClassToObject/baseState.kt"); + } + @TestMetadata("convertSubClassWithModifiers.kt") public void testConvertSubClassWithModifiers() throws Exception { runTest("idea/testData/inspectionsLocal/convertSealedSubClassToObject/convertSubClassWithModifiers.kt"); @@ -1240,6 +1245,11 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { runTest("idea/testData/inspectionsLocal/convertSealedSubClassToObject/convertSubClassWithoutParentheses.kt"); } + @TestMetadata("fakeState.kt") + public void testFakeState() throws Exception { + runTest("idea/testData/inspectionsLocal/convertSealedSubClassToObject/fakeState.kt"); + } + @TestMetadata("generic.kt") public void testGeneric() throws Exception { runTest("idea/testData/inspectionsLocal/convertSealedSubClassToObject/generic.kt"); @@ -1255,6 +1265,16 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { runTest("idea/testData/inspectionsLocal/convertSealedSubClassToObject/openSubclass.kt"); } + @TestMetadata("ownState.kt") + public void testOwnState() throws Exception { + runTest("idea/testData/inspectionsLocal/convertSealedSubClassToObject/ownState.kt"); + } + + @TestMetadata("sealedState.kt") + public void testSealedState() throws Exception { + runTest("idea/testData/inspectionsLocal/convertSealedSubClassToObject/sealedState.kt"); + } + @TestMetadata("withCompanion.kt") public void testWithCompanion() throws Exception { runTest("idea/testData/inspectionsLocal/convertSealedSubClassToObject/withCompanion.kt");