From f861b107988a9f228cb7dac5edf40f0e048724a2 Mon Sep 17 00:00:00 2001 From: Toshiaki Kameyama Date: Tue, 19 Mar 2019 17:18:20 +0900 Subject: [PATCH] Add "Add getter/setter" quick fix for uninitialized property #KT-30078 Fixed --- .../org/jetbrains/kotlin/psi/KtPsiFactory.kt | 5 +- .../idea/intentions/AddAccessorIntentions.kt | 52 +++++++++++++++---- .../kotlin/idea/quickfix/QuickFixRegistrar.kt | 4 ++ .../addPropertyAccessors/setter/noType.kt | 1 + .../setter/noType.kt.after | 6 --- .../quickfix/addPropertyAccessors/val.kt | 5 ++ .../addPropertyAccessors/val.kt.after | 8 +++ .../quickfix/addPropertyAccessors/var.kt | 5 ++ .../addPropertyAccessors/var.kt.after | 9 ++++ .../addPropertyAccessors/varHasGetter.kt | 7 +++ .../varHasGetter.kt.after | 8 +++ .../addPropertyAccessors/varHasSetter.kt | 6 +++ .../varHasSetter.kt.after | 9 ++++ .../modifiers/noLateinitOnNullable.kt | 3 ++ .../modifiers/noLateinitOnPrimitive.kt | 3 ++ .../QuickFixMultiFileTestGenerated.java | 13 +++++ .../idea/quickfix/QuickFixTestGenerated.java | 33 ++++++++++++ 17 files changed, 161 insertions(+), 16 deletions(-) delete mode 100644 idea/testData/intentions/addPropertyAccessors/setter/noType.kt.after create mode 100644 idea/testData/quickfix/addPropertyAccessors/val.kt create mode 100644 idea/testData/quickfix/addPropertyAccessors/val.kt.after create mode 100644 idea/testData/quickfix/addPropertyAccessors/var.kt create mode 100644 idea/testData/quickfix/addPropertyAccessors/var.kt.after create mode 100644 idea/testData/quickfix/addPropertyAccessors/varHasGetter.kt create mode 100644 idea/testData/quickfix/addPropertyAccessors/varHasGetter.kt.after create mode 100644 idea/testData/quickfix/addPropertyAccessors/varHasSetter.kt create mode 100644 idea/testData/quickfix/addPropertyAccessors/varHasSetter.kt.after diff --git a/compiler/psi/src/org/jetbrains/kotlin/psi/KtPsiFactory.kt b/compiler/psi/src/org/jetbrains/kotlin/psi/KtPsiFactory.kt index b0f0465c7e8..40b8b2da7ff 100644 --- a/compiler/psi/src/org/jetbrains/kotlin/psi/KtPsiFactory.kt +++ b/compiler/psi/src/org/jetbrains/kotlin/psi/KtPsiFactory.kt @@ -255,7 +255,10 @@ class KtPsiFactory @JvmOverloads constructor(private val project: Project, val m } fun createPropertyGetter(expression: KtExpression): KtPropertyAccessor { - val property = createProperty("val x get() = 1") + val property = if (expression is KtBlockExpression) + createProperty("val x get() {\nreturn 1\n}") + else + createProperty("val x get() = 1") val getter = property.getter!! val bodyExpression = getter.bodyExpression!! diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/AddAccessorIntentions.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/AddAccessorIntentions.kt index b80641753b8..4bd13848dca 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/AddAccessorIntentions.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/AddAccessorIntentions.kt @@ -16,18 +16,21 @@ package org.jetbrains.kotlin.idea.intentions +import com.intellij.codeInsight.intention.IntentionAction import com.intellij.codeInsight.intention.LowPriorityAction import com.intellij.openapi.editor.Editor import com.intellij.openapi.util.TextRange import org.jetbrains.kotlin.descriptors.CallableMemberDescriptor +import org.jetbrains.kotlin.diagnostics.Diagnostic import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptorIfAny +import org.jetbrains.kotlin.idea.quickfix.KotlinSingleIntentionActionFactory import org.jetbrains.kotlin.idea.refactoring.isAbstract -import org.jetbrains.kotlin.idea.util.findAnnotation import org.jetbrains.kotlin.idea.util.hasJvmFieldAnnotation import org.jetbrains.kotlin.lexer.KtTokens -import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.psi.KtProperty +import org.jetbrains.kotlin.psi.KtPropertyAccessor import org.jetbrains.kotlin.psi.KtPsiFactory +import org.jetbrains.kotlin.psi.psiUtil.startOffset abstract class AbstractAddAccessorsIntention( private val addGetter: Boolean, @@ -45,25 +48,56 @@ abstract class AbstractAddAccessorsIntention( val descriptor = element.resolveToDescriptorIfAny() as? CallableMemberDescriptor ?: return null if (descriptor.isExpect) return null + val hasInitializer = element.hasInitializer() + if (element.typeReference == null && !hasInitializer) return null if (addSetter && (!element.isVar || element.setter != null)) return null - if (addGetter && ((element.typeReference == null && element.initializer == null) || element.getter != null)) return null - return element.nameIdentifier?.textRange + if (addGetter && element.getter != null) return null + return if (hasInitializer) element.nameIdentifier?.textRange else element.textRange } override fun applyTo(element: KtProperty, editor: Editor?) { + val hasInitializer = element.hasInitializer() val psiFactory = KtPsiFactory(element) if (addGetter) { - val expression = psiFactory.createExpression("field") + val expression = if (hasInitializer) psiFactory.createExpression("field") else psiFactory.createBlock("TODO()") val getter = psiFactory.createPropertyGetter(expression) - if (element.setter != null) + val added = if (element.setter != null) { element.addBefore(getter, element.setter) - else + } else { element.add(getter) + } + if (!hasInitializer) { + (added as? KtPropertyAccessor)?.bodyBlockExpression?.statements?.firstOrNull()?.let { + editor?.caretModel?.moveToOffset(it.startOffset) + } + } } if (addSetter) { - val expression = psiFactory.createBlock("field = value") + val expression = if (hasInitializer) psiFactory.createBlock("field = value") else psiFactory.createEmptyBody() val setter = psiFactory.createPropertySetter(expression) - element.add(setter) + val added = element.add(setter) + if (!hasInitializer && !addGetter) { + (added as? KtPropertyAccessor)?.bodyBlockExpression?.lBrace?.let { + editor?.caretModel?.moveToOffset(it.startOffset + 1) + } + } + } + } + + companion object : KotlinSingleIntentionActionFactory() { + override fun createAction(diagnostic: Diagnostic): IntentionAction? { + val property = diagnostic.psiElement as? KtProperty ?: return null + return if (property.isVar) { + val getter = property.getter + val setter = property.setter + when { + getter == null && setter == null -> AddPropertyAccessorsIntention() + getter == null -> AddPropertyGetterIntention() + else -> AddPropertySetterIntention() + } + } else { + AddPropertyGetterIntention() + } } } } diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt index 6856307758d..aebbf7871e2 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt @@ -22,6 +22,7 @@ import org.jetbrains.kotlin.diagnostics.Errors.* import org.jetbrains.kotlin.idea.core.overrideImplement.ImplementAsConstructorParameter import org.jetbrains.kotlin.idea.core.overrideImplement.ImplementMembersHandler import org.jetbrains.kotlin.idea.inspections.* +import org.jetbrains.kotlin.idea.intentions.AbstractAddAccessorsIntention import org.jetbrains.kotlin.idea.intentions.AddValVarToConstructorParameterAction import org.jetbrains.kotlin.idea.intentions.ConvertPropertyInitializerToGetterIntention import org.jetbrains.kotlin.idea.intentions.MoveMemberToCompanionObjectIntention @@ -603,5 +604,8 @@ class QuickFixRegistrar : QuickFixContributor { CONSTANT_EXPECTED_TYPE_MISMATCH.registerFactory(SurroundWithLambdaFix) NO_SET_METHOD.registerFactory(ChangeToMutableCollectionFix) + + MUST_BE_INITIALIZED_OR_BE_ABSTRACT.registerFactory(AbstractAddAccessorsIntention) + MUST_BE_INITIALIZED.registerFactory(AbstractAddAccessorsIntention) } } diff --git a/idea/testData/intentions/addPropertyAccessors/setter/noType.kt b/idea/testData/intentions/addPropertyAccessors/setter/noType.kt index 6e1b77d20e1..b37c403588a 100644 --- a/idea/testData/intentions/addPropertyAccessors/setter/noType.kt +++ b/idea/testData/intentions/addPropertyAccessors/setter/noType.kt @@ -1,3 +1,4 @@ +// IS_APPLICABLE: false // SKIP_ERRORS_BEFORE // SKIP_ERRORS_AFTER var x \ No newline at end of file diff --git a/idea/testData/intentions/addPropertyAccessors/setter/noType.kt.after b/idea/testData/intentions/addPropertyAccessors/setter/noType.kt.after deleted file mode 100644 index 29fd5586793..00000000000 --- a/idea/testData/intentions/addPropertyAccessors/setter/noType.kt.after +++ /dev/null @@ -1,6 +0,0 @@ -// SKIP_ERRORS_BEFORE -// SKIP_ERRORS_AFTER -var x - set(value) { - field = value - } diff --git a/idea/testData/quickfix/addPropertyAccessors/val.kt b/idea/testData/quickfix/addPropertyAccessors/val.kt new file mode 100644 index 00000000000..55298cf2540 --- /dev/null +++ b/idea/testData/quickfix/addPropertyAccessors/val.kt @@ -0,0 +1,5 @@ +// "Add getter" "true" +// WITH_RUNTIME +class Test { + val x: Int +} \ No newline at end of file diff --git a/idea/testData/quickfix/addPropertyAccessors/val.kt.after b/idea/testData/quickfix/addPropertyAccessors/val.kt.after new file mode 100644 index 00000000000..aafd77339d9 --- /dev/null +++ b/idea/testData/quickfix/addPropertyAccessors/val.kt.after @@ -0,0 +1,8 @@ +// "Add getter" "true" +// WITH_RUNTIME +class Test { + val x: Int + get() { + TODO() + } +} \ No newline at end of file diff --git a/idea/testData/quickfix/addPropertyAccessors/var.kt b/idea/testData/quickfix/addPropertyAccessors/var.kt new file mode 100644 index 00000000000..c01e53b1150 --- /dev/null +++ b/idea/testData/quickfix/addPropertyAccessors/var.kt @@ -0,0 +1,5 @@ +// "Add getter and setter" "true" +// WITH_RUNTIME +class Test { + var x: Int +} \ No newline at end of file diff --git a/idea/testData/quickfix/addPropertyAccessors/var.kt.after b/idea/testData/quickfix/addPropertyAccessors/var.kt.after new file mode 100644 index 00000000000..0cf7def505b --- /dev/null +++ b/idea/testData/quickfix/addPropertyAccessors/var.kt.after @@ -0,0 +1,9 @@ +// "Add getter and setter" "true" +// WITH_RUNTIME +class Test { + var x: Int + get() { + TODO() + } + set(value) {} +} \ No newline at end of file diff --git a/idea/testData/quickfix/addPropertyAccessors/varHasGetter.kt b/idea/testData/quickfix/addPropertyAccessors/varHasGetter.kt new file mode 100644 index 00000000000..ead42a039e3 --- /dev/null +++ b/idea/testData/quickfix/addPropertyAccessors/varHasGetter.kt @@ -0,0 +1,7 @@ +// "Add setter" "true" +class Test { + var x: Int + get() { + return 1 + } +} \ No newline at end of file diff --git a/idea/testData/quickfix/addPropertyAccessors/varHasGetter.kt.after b/idea/testData/quickfix/addPropertyAccessors/varHasGetter.kt.after new file mode 100644 index 00000000000..8153f5e1411 --- /dev/null +++ b/idea/testData/quickfix/addPropertyAccessors/varHasGetter.kt.after @@ -0,0 +1,8 @@ +// "Add setter" "true" +class Test { + var x: Int + get() { + return 1 + } + set(value) {} +} \ No newline at end of file diff --git a/idea/testData/quickfix/addPropertyAccessors/varHasSetter.kt b/idea/testData/quickfix/addPropertyAccessors/varHasSetter.kt new file mode 100644 index 00000000000..1b4835daa07 --- /dev/null +++ b/idea/testData/quickfix/addPropertyAccessors/varHasSetter.kt @@ -0,0 +1,6 @@ +// "Add getter" "true" +// WITH_RUNTIME +class Test { + var x: Int + set(value) {} +} \ No newline at end of file diff --git a/idea/testData/quickfix/addPropertyAccessors/varHasSetter.kt.after b/idea/testData/quickfix/addPropertyAccessors/varHasSetter.kt.after new file mode 100644 index 00000000000..bfc6afd8468 --- /dev/null +++ b/idea/testData/quickfix/addPropertyAccessors/varHasSetter.kt.after @@ -0,0 +1,9 @@ +// "Add getter" "true" +// WITH_RUNTIME +class Test { + var x: Int + get() { + TODO() + } + set(value) {} +} \ No newline at end of file diff --git a/idea/testData/quickfix/modifiers/noLateinitOnNullable.kt b/idea/testData/quickfix/modifiers/noLateinitOnNullable.kt index 2f7e5a0398b..0edae1a40bb 100644 --- a/idea/testData/quickfix/modifiers/noLateinitOnNullable.kt +++ b/idea/testData/quickfix/modifiers/noLateinitOnNullable.kt @@ -3,6 +3,9 @@ // ACTION: Make 'a' abstract // ACTION: Move to constructor parameters // ACTION: Move to constructor +// ACTION: Add getter +// ACTION: Add getter and setter +// ACTION: Add setter // ERROR: Property must be initialized or be abstract class A { diff --git a/idea/testData/quickfix/modifiers/noLateinitOnPrimitive.kt b/idea/testData/quickfix/modifiers/noLateinitOnPrimitive.kt index 9ee40768a60..5bc5a247f63 100644 --- a/idea/testData/quickfix/modifiers/noLateinitOnPrimitive.kt +++ b/idea/testData/quickfix/modifiers/noLateinitOnPrimitive.kt @@ -4,6 +4,9 @@ // ACTION: Make 'a' abstract // ACTION: Move to constructor parameters // ACTION: Move to constructor +// ACTION: Add getter +// ACTION: Add getter and setter +// ACTION: Add setter // ERROR: Property must be initialized or be abstract class A { diff --git a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixMultiFileTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixMultiFileTestGenerated.java index 581df73140d..e710e33462b 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixMultiFileTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixMultiFileTestGenerated.java @@ -221,6 +221,19 @@ public class QuickFixMultiFileTestGenerated extends AbstractQuickFixMultiFileTes } } + @TestMetadata("idea/testData/quickfix/addPropertyAccessors") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class AddPropertyAccessors extends AbstractQuickFixMultiFileTest { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTestWithExtraFile, TargetBackend.ANY, testDataFilePath); + } + + public void testAllFilesPresentInAddPropertyAccessors() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/quickfix/addPropertyAccessors"), Pattern.compile("^(\\w+)\\.((before\\.Main\\.\\w+)|(test))$"), TargetBackend.ANY, true); + } + } + @TestMetadata("idea/testData/quickfix/addPropertyToSupertype") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) diff --git a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java index f56876d4746..0f34dccf2fb 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java @@ -846,6 +846,39 @@ public class QuickFixTestGenerated extends AbstractQuickFixTest { } } + @TestMetadata("idea/testData/quickfix/addPropertyAccessors") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class AddPropertyAccessors extends AbstractQuickFixTest { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath); + } + + public void testAllFilesPresentInAddPropertyAccessors() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/quickfix/addPropertyAccessors"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), TargetBackend.ANY, true); + } + + @TestMetadata("val.kt") + public void testVal() throws Exception { + runTest("idea/testData/quickfix/addPropertyAccessors/val.kt"); + } + + @TestMetadata("var.kt") + public void testVar() throws Exception { + runTest("idea/testData/quickfix/addPropertyAccessors/var.kt"); + } + + @TestMetadata("varHasGetter.kt") + public void testVarHasGetter() throws Exception { + runTest("idea/testData/quickfix/addPropertyAccessors/varHasGetter.kt"); + } + + @TestMetadata("varHasSetter.kt") + public void testVarHasSetter() throws Exception { + runTest("idea/testData/quickfix/addPropertyAccessors/varHasSetter.kt"); + } + } + @TestMetadata("idea/testData/quickfix/addPropertyToSupertype") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)