From d0b9b6a3b4dbb423abe393942fedad06f2ff7f6b Mon Sep 17 00:00:00 2001 From: Alexey Sedunov Date: Thu, 10 Dec 2015 19:35:37 +0300 Subject: [PATCH] Introduce Parameter: Support conversion of local variables #KT-9170 Fixed --- .../refactoring/KotlinRefactoringUtil.java | 1 + .../AbstractKotlinInplaceIntroducer.kt | 23 ++++--- .../KotlinInplaceParameterIntroducer.kt | 17 ++++- .../KotlinIntroduceParameterHandler.kt | 69 +++++++++++++------ .../refactoring/introduce/introduceUtil.kt | 6 +- .../introduceParameter/localVar.kt | 5 -- .../introduceParameter/localVar.kt.conflicts | 1 - .../variableConversion/caretAtIdentifier.kt | 4 ++ .../caretAtIdentifier.kt.after | 3 + .../variableConversion/fullSelection.kt | 4 ++ .../variableConversion/fullSelection.kt.after | 3 + .../introduce/ExtractionTestGenerated.java | 27 ++++++-- 12 files changed, 117 insertions(+), 46 deletions(-) delete mode 100644 idea/testData/refactoring/introduceParameter/localVar.kt delete mode 100644 idea/testData/refactoring/introduceParameter/localVar.kt.conflicts create mode 100644 idea/testData/refactoring/introduceParameter/variableConversion/caretAtIdentifier.kt create mode 100644 idea/testData/refactoring/introduceParameter/variableConversion/caretAtIdentifier.kt.after create mode 100644 idea/testData/refactoring/introduceParameter/variableConversion/fullSelection.kt create mode 100644 idea/testData/refactoring/introduceParameter/variableConversion/fullSelection.kt.after diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/KotlinRefactoringUtil.java b/idea/src/org/jetbrains/kotlin/idea/refactoring/KotlinRefactoringUtil.java index b13fe38bd00..eac75f1122f 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/KotlinRefactoringUtil.java +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/KotlinRefactoringUtil.java @@ -425,6 +425,7 @@ public class KotlinRefactoringUtil { if (expressions.size() == 0) { if (failOnEmptySuggestion) throw new IntroduceRefactoringException( KotlinRefactoringBundle.message("cannot.refactor.not.expression")); + callback.run(null); return; } diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/AbstractKotlinInplaceIntroducer.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/AbstractKotlinInplaceIntroducer.kt index 270523e8be3..efd4be75395 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/AbstractKotlinInplaceIntroducer.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/AbstractKotlinInplaceIntroducer.kt @@ -19,21 +19,16 @@ package org.jetbrains.kotlin.idea.refactoring.introduce import com.intellij.openapi.editor.Editor import com.intellij.openapi.editor.RangeMarker import com.intellij.openapi.project.Project -import com.intellij.psi.PsiDocumentManager import com.intellij.psi.PsiFile import com.intellij.refactoring.introduce.inplace.AbstractInplaceIntroducer import com.intellij.refactoring.rename.inplace.InplaceRefactoring -import com.intellij.ui.NonFocusableCheckBox import com.intellij.util.ui.FormBuilder import org.jetbrains.kotlin.idea.KotlinFileType import org.jetbrains.kotlin.idea.core.replaced -import org.jetbrains.kotlin.idea.util.IdeDescriptorRenderers import org.jetbrains.kotlin.idea.util.application.executeWriteCommand -import org.jetbrains.kotlin.psi.KtExpression -import org.jetbrains.kotlin.psi.KtNamedDeclaration -import org.jetbrains.kotlin.psi.KtPsiFactory -import org.jetbrains.kotlin.psi.KtSimpleNameExpression +import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.getNonStrictParentOfType +import org.jetbrains.kotlin.psi.psiUtil.getParentOfTypeAndBranch import java.awt.BorderLayout public abstract class AbstractKotlinInplaceIntroducer( @@ -77,10 +72,16 @@ public abstract class AbstractKotlinInplaceIntroducer( ): KtExpression? { if (exprText == null || !declaration.isValid()) return null - return containingFile - .findElementAt(marker.getStartOffset()) - ?.getNonStrictParentOfType() - ?.replaced(KtPsiFactory(myProject).createExpression(exprText)) + val leaf = containingFile.findElementAt(marker.startOffset) ?: return null + + leaf.getParentOfTypeAndBranch { nameIdentifier }?.let { + return it.replaced(KtPsiFactory(myProject).createDeclaration(exprText)) + } + + val occurrenceExprText = (myExpr as? KtProperty)?.name ?: exprText + return leaf + .getNonStrictParentOfType() + ?.replaced(KtPsiFactory(myProject).createExpression(occurrenceExprText)) } override fun updateTitle(declaration: D?) = updateTitle(declaration, null) diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceParameter/KotlinInplaceParameterIntroducer.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceParameter/KotlinInplaceParameterIntroducer.kt index ffbd871b4d5..57889188221 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceParameter/KotlinInplaceParameterIntroducer.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceParameter/KotlinInplaceParameterIntroducer.kt @@ -19,6 +19,7 @@ package org.jetbrains.kotlin.idea.refactoring.introduce.introduceParameter import com.intellij.codeInsight.template.impl.TemplateManagerImpl import com.intellij.openapi.diagnostic.Logger import com.intellij.openapi.editor.Editor +import com.intellij.openapi.editor.RangeMarker import com.intellij.openapi.editor.impl.DocumentMarkupModel import com.intellij.openapi.editor.markup.EffectType import com.intellij.openapi.editor.markup.HighlighterTargetArea @@ -26,6 +27,7 @@ import com.intellij.openapi.editor.markup.MarkupModel import com.intellij.openapi.editor.markup.TextAttributes import com.intellij.openapi.project.Project import com.intellij.openapi.util.TextRange +import com.intellij.psi.PsiElement import com.intellij.ui.JBColor import com.intellij.ui.NonFocusableCheckBox import org.jetbrains.kotlin.idea.core.refactoring.createPrimaryConstructorParameterListIfAbsent @@ -38,6 +40,7 @@ import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.getElementTextWithContext import org.jetbrains.kotlin.psi.psiUtil.getValueParameterList import org.jetbrains.kotlin.psi.psiUtil.getValueParameters +import org.jetbrains.kotlin.psi.psiUtil.startOffset import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.types.typeUtil.supertypes import org.jetbrains.kotlin.utils.addToStdlib.singletonList @@ -135,7 +138,9 @@ public class KotlinInplaceParameterIntroducer( val parameterType = currentType ?: parameter.getTypeReference()!!.getText() descriptor = descriptor.copy(newParameterName = parameterName!!, newParameterTypeText = parameterType) val modifier = if (valVar != KotlinValVar.None) "${valVar.keywordName} " else "" - val defaultValue = if (withDefaultValue) " = ${newArgumentValue.getText()}" else "" + val defaultValue = if (withDefaultValue) { + " = ${if (newArgumentValue is KtProperty) newArgumentValue.name else newArgumentValue.text}" + } else "" "$modifier$parameterName: $parameterType$defaultValue" } @@ -239,6 +244,16 @@ public class KotlinInplaceParameterIntroducer( revalidate() } + override fun getRangeToRename(element: PsiElement): TextRange { + if (element is KtProperty) return element.nameIdentifier!!.textRange.shiftRight(-element.startOffset) + return super.getRangeToRename(element) + } + + override fun createMarker(element: PsiElement): RangeMarker { + if (element is KtProperty) return super.createMarker(element.nameIdentifier) + return super.createMarker(element) + } + override fun performIntroduce() { getDescriptorToRefactor(isReplaceAllOccurrences()).performRefactoring() } diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceParameter/KotlinIntroduceParameterHandler.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceParameter/KotlinIntroduceParameterHandler.kt index 6caf1ba944b..4e76569212e 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceParameter/KotlinIntroduceParameterHandler.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceParameter/KotlinIntroduceParameterHandler.kt @@ -27,6 +27,7 @@ import com.intellij.psi.search.searches.ReferencesSearch import com.intellij.psi.util.PsiTreeUtil import com.intellij.refactoring.introduce.inplace.AbstractInplaceIntroducer import com.intellij.refactoring.listeners.RefactoringEventListener +import com.intellij.util.SmartList import com.intellij.util.containers.MultiMap import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.DeclarationDescriptor @@ -36,7 +37,6 @@ import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptor import org.jetbrains.kotlin.idea.core.KotlinNameSuggester import org.jetbrains.kotlin.idea.core.NewDeclarationNameValidator -import org.jetbrains.kotlin.idea.util.getResolutionScope import org.jetbrains.kotlin.idea.core.moveInsideParenthesesAndReplaceWith import org.jetbrains.kotlin.idea.core.refactoring.removeTemplateEntryBracesIfPossible import org.jetbrains.kotlin.idea.core.refactoring.runRefactoringWithPostprocessing @@ -50,6 +50,7 @@ import org.jetbrains.kotlin.idea.util.IdeDescriptorRenderers import org.jetbrains.kotlin.idea.util.application.executeCommand import org.jetbrains.kotlin.idea.util.application.runWriteAction import org.jetbrains.kotlin.idea.util.approximateWithResolvableType +import org.jetbrains.kotlin.idea.util.getResolutionScope import org.jetbrains.kotlin.idea.util.psi.patternMatching.KotlinPsiRange import org.jetbrains.kotlin.idea.util.psi.patternMatching.KotlinPsiUnifier import org.jetbrains.kotlin.idea.util.psi.patternMatching.toRange @@ -61,6 +62,7 @@ import org.jetbrains.kotlin.resolve.scopes.receivers.ImplicitReceiver import org.jetbrains.kotlin.types.typeUtil.isNothing import org.jetbrains.kotlin.types.typeUtil.isUnit import org.jetbrains.kotlin.types.typeUtil.supertypes +import org.jetbrains.kotlin.utils.addIfNotNull import java.util.* import kotlin.test.fail @@ -147,10 +149,11 @@ fun IntroduceParameterDescriptor.performRefactoring() { .forEach { methodDescriptor.removeParameter(it) } } + val defaultValue = if (newArgumentValue is KtProperty) (newArgumentValue as KtProperty).initializer else newArgumentValue val parameterInfo = KotlinParameterInfo(callableDescriptor = callableDescriptor, name = newParameterName, - defaultValueForCall = if (withDefaultValue) null else newArgumentValue, - defaultValueForParameter = if (withDefaultValue) newArgumentValue else null, + defaultValueForCall = if (withDefaultValue) null else defaultValue, + defaultValueForParameter = if (withDefaultValue) defaultValue else null, valOrVar = valVar) parameterInfo.currentTypeText = newParameterTypeText methodDescriptor.addParameter(parameterInfo) @@ -214,9 +217,20 @@ public open class KotlinIntroduceParameterHandler( ): KotlinIntroduceHandlerBase() { open fun invoke(project: Project, editor: Editor, expression: KtExpression, targetParent: KtNamedDeclaration) { val physicalExpression = expression.substringContextOrThis + if (physicalExpression is KtProperty && physicalExpression.isLocal && physicalExpression.nameIdentifier == null) { + showErrorHintByKey(project, editor, "cannot.refactor.no.expression", INTRODUCE_PARAMETER) + return + } + val context = physicalExpression.analyze() - val expressionType = expression.extractableSubstringInfo?.type ?: context.getType(physicalExpression) + val expressionType = if (physicalExpression is KtProperty && physicalExpression.isLocal) { + context[BindingContext.VARIABLE, physicalExpression]?.type + } + else { + expression.extractableSubstringInfo?.type ?: context.getType(physicalExpression) + } + if (expressionType == null) { showErrorHint(project, editor, "Expression has no type", INTRODUCE_PARAMETER) return @@ -241,7 +255,13 @@ public open class KotlinIntroduceParameterHandler( else -> null } ?: throw AssertionError("Body element is not found: ${targetParent.getElementTextWithContext()}") val nameValidator = NewDeclarationNameValidator(body, sequenceOf(body), NewDeclarationNameValidator.Target.VARIABLES) - val suggestedNames = KotlinNameSuggester.suggestNamesByType(replacementType, nameValidator, "p") + + val suggestedNames = SmartList().apply { + if (physicalExpression is KtProperty && !ApplicationManager.getApplication().isUnitTestMode) { + addIfNotNull(physicalExpression.name) + } + addAll(KotlinNameSuggester.suggestNamesByType(replacementType, nameValidator, "p")) + } val parametersUsages = findInternalUsagesOfParametersAndReceiver(targetParent, functionDescriptor) @@ -252,21 +272,27 @@ public open class KotlinIntroduceParameterHandler( else { Collections.emptyList() } - val occurrencesToReplace = expression.toRange() - .match(body, KotlinPsiUnifier.DEFAULT) - .filterNot { - val textRange = it.range.getPhysicalTextRange() - forbiddenRanges.any { it.intersects(textRange) } - } - .mapNotNull { - val matchedElement = it.range.elements.singleOrNull() - when (matchedElement) { - is KtExpression -> matchedElement - is KtStringTemplateEntryWithExpression -> matchedElement.getExpression() - else -> null - } as? KtExpression - } - .map { it.toRange() } + + val occurrencesToReplace = if (expression is KtProperty) { + ReferencesSearch.search(expression).mapNotNullTo(SmartList(expression.toRange())) { it.element?.toRange() } + } + else { + expression.toRange() + .match(body, KotlinPsiUnifier.DEFAULT) + .filterNot { + val textRange = it.range.getPhysicalTextRange() + forbiddenRanges.any { it.intersects(textRange) } + } + .mapNotNull { + val matchedElement = it.range.elements.singleOrNull() + val matchedExpr = when (matchedElement) { + is KtExpression -> matchedElement + is KtStringTemplateEntryWithExpression -> matchedElement.expression + else -> null + } as? KtExpression + matchedExpr?.toRange() + } + } project.executeCommand( INTRODUCE_PARAMETER, @@ -296,11 +322,12 @@ public open class KotlinIntroduceParameterHandler( withDefaultValue = false, parametersUsages = parametersUsages, occurrencesToReplace = occurrencesToReplace, - occurrenceReplacer = { + occurrenceReplacer = replacer@ { val expressionToReplace = it.elements.single() as KtExpression val replacingExpression = psiFactory.createExpression(newParameterName) val substringInfo = expressionToReplace.extractableSubstringInfo val result = when { + expressionToReplace is KtProperty -> return@replacer expressionToReplace.delete() expressionToReplace.isLambdaOutsideParentheses() -> { expressionToReplace .getStrictParentOfType()!! diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceUtil.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceUtil.kt index aad21670f0d..c8829b8c600 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceUtil.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceUtil.kt @@ -21,7 +21,6 @@ import com.intellij.openapi.editor.ScrollType import com.intellij.openapi.project.Project import com.intellij.openapi.util.Key import com.intellij.openapi.util.TextRange -import com.intellij.psi.PsiComment import com.intellij.psi.PsiElement import com.intellij.psi.PsiWhiteSpace import com.intellij.psi.util.PsiTreeUtil @@ -122,6 +121,11 @@ fun selectElementsWithTargetParent( } else { if (!editor.getSelectionModel().hasSelection()) { + val elementAtCaret = file.findElementAt(editor.caretModel.offset) + elementAtCaret?.getParentOfTypeAndBranch { nameIdentifier }?.let { + return@selectExpression selectTargetContainer(listOf(it)) + } + editor.getSelectionModel().selectLineAtCaret() } selectMultipleExpressions() diff --git a/idea/testData/refactoring/introduceParameter/localVar.kt b/idea/testData/refactoring/introduceParameter/localVar.kt deleted file mode 100644 index bd833f4e00b..00000000000 --- a/idea/testData/refactoring/introduceParameter/localVar.kt +++ /dev/null @@ -1,5 +0,0 @@ -fun foo(): Int { - val x = 1 - - return 1 + x -} \ No newline at end of file diff --git a/idea/testData/refactoring/introduceParameter/localVar.kt.conflicts b/idea/testData/refactoring/introduceParameter/localVar.kt.conflicts deleted file mode 100644 index 58668cdaac7..00000000000 --- a/idea/testData/refactoring/introduceParameter/localVar.kt.conflicts +++ /dev/null @@ -1 +0,0 @@ -Cannot introduce parameter of type 'Unit' \ No newline at end of file diff --git a/idea/testData/refactoring/introduceParameter/variableConversion/caretAtIdentifier.kt b/idea/testData/refactoring/introduceParameter/variableConversion/caretAtIdentifier.kt new file mode 100644 index 00000000000..db1ca380267 --- /dev/null +++ b/idea/testData/refactoring/introduceParameter/variableConversion/caretAtIdentifier.kt @@ -0,0 +1,4 @@ +fun foo(i: Int) { + val t = 1 + val u = t + i +} \ No newline at end of file diff --git a/idea/testData/refactoring/introduceParameter/variableConversion/caretAtIdentifier.kt.after b/idea/testData/refactoring/introduceParameter/variableConversion/caretAtIdentifier.kt.after new file mode 100644 index 00000000000..32dc87eb8b2 --- /dev/null +++ b/idea/testData/refactoring/introduceParameter/variableConversion/caretAtIdentifier.kt.after @@ -0,0 +1,3 @@ +fun foo(i: Int, i1: Int = 1) { + val u = i1 + i +} \ No newline at end of file diff --git a/idea/testData/refactoring/introduceParameter/variableConversion/fullSelection.kt b/idea/testData/refactoring/introduceParameter/variableConversion/fullSelection.kt new file mode 100644 index 00000000000..db88f221bff --- /dev/null +++ b/idea/testData/refactoring/introduceParameter/variableConversion/fullSelection.kt @@ -0,0 +1,4 @@ +fun foo(i: Int) { + val t = 1 + val u = t + i +} \ No newline at end of file diff --git a/idea/testData/refactoring/introduceParameter/variableConversion/fullSelection.kt.after b/idea/testData/refactoring/introduceParameter/variableConversion/fullSelection.kt.after new file mode 100644 index 00000000000..32dc87eb8b2 --- /dev/null +++ b/idea/testData/refactoring/introduceParameter/variableConversion/fullSelection.kt.after @@ -0,0 +1,3 @@ +fun foo(i: Int, i1: Int = 1) { + val u = i1 + i +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/refactoring/introduce/ExtractionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/refactoring/introduce/ExtractionTestGenerated.java index bc0b91cb296..6aff4d945a7 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/introduce/ExtractionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/refactoring/introduce/ExtractionTestGenerated.java @@ -3370,12 +3370,6 @@ public class ExtractionTestGenerated extends AbstractExtractionTest { doIntroduceSimpleParameterTest(fileName); } - @TestMetadata("localVar.kt") - public void testLocalVar() throws Exception { - String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/introduceParameter/localVar.kt"); - doIntroduceSimpleParameterTest(fileName); - } - @TestMetadata("partialSubstitution.kt") public void testPartialSubstitution() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/introduceParameter/partialSubstitution.kt"); @@ -3666,6 +3660,27 @@ public class ExtractionTestGenerated extends AbstractExtractionTest { doIntroduceSimpleParameterTest(fileName); } } + + @TestMetadata("idea/testData/refactoring/introduceParameter/variableConversion") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class VariableConversion extends AbstractExtractionTest { + public void testAllFilesPresentInVariableConversion() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/refactoring/introduceParameter/variableConversion"), Pattern.compile("^(.+)\\.(kt|kts)$"), true); + } + + @TestMetadata("caretAtIdentifier.kt") + public void testCaretAtIdentifier() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/introduceParameter/variableConversion/caretAtIdentifier.kt"); + doIntroduceSimpleParameterTest(fileName); + } + + @TestMetadata("fullSelection.kt") + public void testFullSelection() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/introduceParameter/variableConversion/fullSelection.kt"); + doIntroduceSimpleParameterTest(fileName); + } + } } @TestMetadata("idea/testData/refactoring/introduceLambdaParameter")