From 2d4ef8d1e699befabd40d0237643a91502086882 Mon Sep 17 00:00:00 2001 From: Toshiaki Kameyama Date: Sat, 24 Mar 2018 08:30:39 +0300 Subject: [PATCH] Allow "add constructor invocation" for sealed top-level inheritors So #KT-23320 Fixed --- .../idea/quickfix/SuperClassNotInitialized.kt | 54 ++++++++++--------- .../addParenthesisForInvalidSealedClass.kt | 8 +++ .../addParenthesisForInvalidSealedClass2.kt | 9 ++++ .../addParenthesisForSealedClass.kt | 3 ++ .../addParenthesisForSealedClass.kt.after | 3 ++ .../idea/quickfix/QuickFixTestGenerated.java | 18 +++++++ 6 files changed, 71 insertions(+), 24 deletions(-) create mode 100644 idea/testData/quickfix/supertypeInitialization/addParenthesisForInvalidSealedClass.kt create mode 100644 idea/testData/quickfix/supertypeInitialization/addParenthesisForInvalidSealedClass2.kt create mode 100644 idea/testData/quickfix/supertypeInitialization/addParenthesisForSealedClass.kt create mode 100644 idea/testData/quickfix/supertypeInitialization/addParenthesisForSealedClass.kt.after diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/SuperClassNotInitialized.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/SuperClassNotInitialized.kt index 04c739a41a3..8bedffeff03 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/SuperClassNotInitialized.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/SuperClassNotInitialized.kt @@ -22,9 +22,7 @@ import com.intellij.codeInsight.intention.IntentionAction import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.editor.Editor import com.intellij.openapi.project.Project -import org.jetbrains.kotlin.descriptors.ClassDescriptor -import org.jetbrains.kotlin.descriptors.ConstructorDescriptor -import org.jetbrains.kotlin.descriptors.VariableDescriptor +import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.diagnostics.Diagnostic import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptorIfAny @@ -49,7 +47,7 @@ import org.jetbrains.kotlin.utils.addIfNotNull import java.util.* object SuperClassNotInitialized : KotlinIntentionActionsFactory() { - private val DISPLAY_MAX_PARAMS = 5 + private const val DISPLAY_MAX_PARAMS = 5 override fun doCreateActions(diagnostic: Diagnostic): List { val delegator = diagnostic.psiElement as KtSuperTypeEntry @@ -61,12 +59,21 @@ object SuperClassNotInitialized : KotlinIntentionActionsFactory() { val superClass = (type.constructor.declarationDescriptor as? ClassDescriptor) ?: return emptyList() val classDescriptor = classOrObjectDeclaration.resolveToDescriptorIfAny(BodyResolveMode.FULL) ?: return emptyList() - val constructors = superClass.constructors.filter { it.isVisible(classDescriptor) } + val containingPackage = superClass.containingDeclaration as? PackageFragmentDescriptor + val inSameFile = containingPackage == classDescriptor.containingDeclaration + val constructors = superClass.constructors.filter { + it.isVisible(classDescriptor) || (superClass.modality == Modality.SEALED && inSameFile) + } if (constructors.isEmpty()) return emptyList() // no accessible constructor val fixes = ArrayList() - fixes.add(AddParenthesisFix(delegator, putCaretIntoParenthesis = constructors.singleOrNull()?.valueParameters?.isNotEmpty() ?: true)) + fixes.add( + AddParenthesisFix( + delegator, + putCaretIntoParenthesis = constructors.singleOrNull()?.valueParameters?.isNotEmpty() ?: true + ) + ) if (classOrObjectDeclaration is KtClass) { val superType = classDescriptor.typeConstructor.supertypes.firstOrNull { it.constructor.declarationDescriptor == superClass } @@ -74,8 +81,8 @@ object SuperClassNotInitialized : KotlinIntentionActionsFactory() { val substitutor = TypeConstructorSubstitution.create(superClass.typeConstructor, superType.arguments).buildSubstitutor() val substitutedConstructors = constructors - .filter { it.valueParameters.isNotEmpty() } - .mapNotNull { it.substitute(substitutor) } + .filter { it.valueParameters.isNotEmpty() } + .mapNotNull { it.substitute(substitutor) } if (substitutedConstructors.isNotEmpty()) { val parameterTypes: List> = substitutedConstructors.map { @@ -87,9 +94,8 @@ object SuperClassNotInitialized : KotlinIntentionActionsFactory() { val maxParams = parameterTypes.map { it.size }.max()!! val maxParamsToDisplay = if (maxParams <= DISPLAY_MAX_PARAMS) { maxParams - } - else { - (DISPLAY_MAX_PARAMS..maxParams-1).firstOrNull(::canRenderOnlyFirstParameters) ?: maxParams + } else { + (DISPLAY_MAX_PARAMS until maxParams).firstOrNull(::canRenderOnlyFirstParameters) ?: maxParams } for ((constructor, types) in substitutedConstructors.zip(parameterTypes)) { @@ -106,8 +112,8 @@ object SuperClassNotInitialized : KotlinIntentionActionsFactory() { } private class AddParenthesisFix( - element: KtSuperTypeEntry, - val putCaretIntoParenthesis: Boolean + element: KtSuperTypeEntry, + val putCaretIntoParenthesis: Boolean ) : KotlinQuickFixAction(element), HighPriorityAction { override fun getFamilyName() = "Change to constructor invocation" //TODO? @@ -131,21 +137,21 @@ object SuperClassNotInitialized : KotlinIntentionActionsFactory() { } private class AddParametersFix( - element: KtSuperTypeEntry, - classDeclaration: KtClass, - parametersToAdd: Collection, - private val argumentText: String, - private val text: String + element: KtSuperTypeEntry, + classDeclaration: KtClass, + parametersToAdd: Collection, + private val argumentText: String, + private val text: String ) : KotlinQuickFixAction(element) { private val classDeclarationPointer = classDeclaration.createSmartPointer() private val parametersToAddPointers = parametersToAdd.map { it.createSmartPointer() } companion object { fun create( - element: KtSuperTypeEntry, - classDeclaration: KtClass, - superConstructor: ConstructorDescriptor, - text: String + element: KtSuperTypeEntry, + classDeclaration: KtClass, + superConstructor: ConstructorDescriptor, + text: String ): AddParametersFix? { val superParameters = superConstructor.valueParameters assert(superParameters.isNotEmpty()) @@ -159,7 +165,7 @@ object SuperClassNotInitialized : KotlinIntentionActionsFactory() { val nameRendered = parameter.name.render() val varargElementType = parameter.varargElementType - if (argumentText.length > 0) { + if (argumentText.isNotEmpty()) { argumentText.append(", ") } argumentText.append(if (varargElementType != null) "*$nameRendered" else nameRendered) @@ -168,7 +174,7 @@ object SuperClassNotInitialized : KotlinIntentionActionsFactory() { val existingParameter = oldParameters.firstOrNull { it.name == nameString } if (existingParameter != null) { val type = (existingParameter.resolveToParameterDescriptorIfAny() as? VariableDescriptor)?.type - ?: return null + ?: return null if (type.isSubtypeOf(parameter.type)) continue // use existing parameter } diff --git a/idea/testData/quickfix/supertypeInitialization/addParenthesisForInvalidSealedClass.kt b/idea/testData/quickfix/supertypeInitialization/addParenthesisForInvalidSealedClass.kt new file mode 100644 index 00000000000..d9b4d23b05a --- /dev/null +++ b/idea/testData/quickfix/supertypeInitialization/addParenthesisForInvalidSealedClass.kt @@ -0,0 +1,8 @@ +// "Change to constructor invocation" "false" +// ERROR: This type has a constructor, and thus must be initialized here +// ERROR: This type is sealed, so it can be inherited by only its own nested classes or objects +sealed class A + +fun test() { + class B : A +} \ No newline at end of file diff --git a/idea/testData/quickfix/supertypeInitialization/addParenthesisForInvalidSealedClass2.kt b/idea/testData/quickfix/supertypeInitialization/addParenthesisForInvalidSealedClass2.kt new file mode 100644 index 00000000000..c626322e349 --- /dev/null +++ b/idea/testData/quickfix/supertypeInitialization/addParenthesisForInvalidSealedClass2.kt @@ -0,0 +1,9 @@ +// "Change to constructor invocation" "false" +// ERROR: This type has a constructor, and thus must be initialized here +// ERROR: This type is sealed, so it can be inherited by only its own nested classes or objects + +class My { + sealed class A + + class B : A +} \ No newline at end of file diff --git a/idea/testData/quickfix/supertypeInitialization/addParenthesisForSealedClass.kt b/idea/testData/quickfix/supertypeInitialization/addParenthesisForSealedClass.kt new file mode 100644 index 00000000000..ce32493418a --- /dev/null +++ b/idea/testData/quickfix/supertypeInitialization/addParenthesisForSealedClass.kt @@ -0,0 +1,3 @@ +// "Change to constructor invocation" "true" +sealed class A +class B : A \ No newline at end of file diff --git a/idea/testData/quickfix/supertypeInitialization/addParenthesisForSealedClass.kt.after b/idea/testData/quickfix/supertypeInitialization/addParenthesisForSealedClass.kt.after new file mode 100644 index 00000000000..eda0b471468 --- /dev/null +++ b/idea/testData/quickfix/supertypeInitialization/addParenthesisForSealedClass.kt.after @@ -0,0 +1,3 @@ +// "Change to constructor invocation" "true" +sealed class A +class B : A() \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java index 0430dd24dd4..b06b3312dc0 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java @@ -10319,6 +10319,18 @@ public class QuickFixTestGenerated extends AbstractQuickFixTest { doTest(fileName); } + @TestMetadata("addParenthesisForInvalidSealedClass.kt") + public void testAddParenthesisForInvalidSealedClass() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/supertypeInitialization/addParenthesisForInvalidSealedClass.kt"); + doTest(fileName); + } + + @TestMetadata("addParenthesisForInvalidSealedClass2.kt") + public void testAddParenthesisForInvalidSealedClass2() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/supertypeInitialization/addParenthesisForInvalidSealedClass2.kt"); + doTest(fileName); + } + @TestMetadata("addParenthesisForLocalClass.kt") public void testAddParenthesisForLocalClass() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/supertypeInitialization/addParenthesisForLocalClass.kt"); @@ -10331,6 +10343,12 @@ public class QuickFixTestGenerated extends AbstractQuickFixTest { doTest(fileName); } + @TestMetadata("addParenthesisForSealedClass.kt") + public void testAddParenthesisForSealedClass() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/supertypeInitialization/addParenthesisForSealedClass.kt"); + doTest(fileName); + } + public void testAllFilesPresentInSupertypeInitialization() throws Exception { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/quickfix/supertypeInitialization"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), TargetBackend.ANY, true); }