Change Signature: Introduce variable for expression which can't be safely copied in the course of argument substitution

2015-04-27 15:39:55 +03:00
parent 14f63cdce5
commit 7831e75955
17 changed files with 267 additions and 42 deletions
@@ -120,7 +120,11 @@ fun JetExpression.convertToIfStatement(condition: JetExpression, thenClause: Jet
 fun JetIfExpression.introduceValueForCondition(occurrenceInThenClause: JetExpression, editor: Editor) {
    val project = this.getProject()
    val occurrenceInConditional = (this.getCondition() as JetBinaryExpression).getLeft()!!
-    KotlinIntroduceVariableHandler.doRefactoring(project, editor, occurrenceInConditional, listOf(occurrenceInConditional, occurrenceInThenClause))
+    KotlinIntroduceVariableHandler.doRefactoring(project,
+                                                 editor,
+                                                 occurrenceInConditional,
+                                                 listOf(occurrenceInConditional, occurrenceInThenClause),
+                                                 null)
 }

 fun JetElement.replace(expressionAsString: String): PsiElement =
@@ -16,6 +16,7 @@

 package org.jetbrains.kotlin.idea.refactoring.changeSignature.usages;

+import com.intellij.openapi.util.Ref;
 import com.intellij.psi.PsiElement;
 import com.intellij.psi.PsiReference;
 import com.intellij.psi.util.PsiTreeUtil;
@@ -24,6 +25,8 @@ import gnu.trove.TIntArrayList;
 import gnu.trove.TIntProcedure;
 import kotlin.Function1;
 import kotlin.KotlinPackage;
+import kotlin.Pair;
+import kotlin.Unit;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.kotlin.descriptors.*;
@@ -31,9 +34,11 @@ import org.jetbrains.kotlin.idea.caches.resolve.ResolvePackage;
 import org.jetbrains.kotlin.idea.refactoring.changeSignature.JetChangeInfo;
 import org.jetbrains.kotlin.idea.refactoring.changeSignature.JetParameterInfo;
 import org.jetbrains.kotlin.idea.refactoring.introduce.extractionEngine.ExtractionEnginePackage;
+import org.jetbrains.kotlin.idea.refactoring.introduce.introduceVariable.KotlinIntroduceVariableHandler;
 import org.jetbrains.kotlin.idea.util.ShortenReferences;
 import org.jetbrains.kotlin.idea.util.psiModificationUtil.PsiModificationUtilPackage;
 import org.jetbrains.kotlin.psi.*;
+import org.jetbrains.kotlin.resolve.BindingContext;
 import org.jetbrains.kotlin.resolve.calls.callUtil.CallUtilPackage;
 import org.jetbrains.kotlin.resolve.calls.model.ExpressionValueArgument;
 import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall;
@@ -45,21 +50,22 @@ import org.jetbrains.kotlin.resolve.scopes.receivers.ReceiverValue;
 import org.jetbrains.kotlin.resolve.scopes.receivers.ThisReceiver;
 import org.jetbrains.kotlin.types.JetType;
 import org.jetbrains.kotlin.types.checker.JetTypeChecker;
+import org.jetbrains.kotlin.types.expressions.OperatorConventions;

 import java.util.*;

 import static org.jetbrains.kotlin.psi.PsiPackage.JetPsiFactory;

 public class JetFunctionCallUsage extends JetUsageInfo<JetCallElement> {
-    private static final Comparator<Map.Entry<PsiReference, DeclarationDescriptor>>
-            REVERSE_TEXT_OFFSET_COMPARATOR = new Comparator<Map.Entry<PsiReference, DeclarationDescriptor>>() {
+    private static final Comparator<Pair<JetElement, JetElement>>
+            REVERSED_TEXT_OFFSET_COMPARATOR = new Comparator<Pair<JetElement, JetElement>>() {
        @Override
        public int compare(
-                @NotNull Map.Entry<PsiReference, DeclarationDescriptor> o1,
-                @NotNull Map.Entry<PsiReference, DeclarationDescriptor> o2
+                @NotNull Pair<JetElement, JetElement> p1,
+                @NotNull Pair<JetElement, JetElement> p2
        ) {
-            int offset1 = o1.getKey().getElement().getTextRange().getStartOffset();
-            int offset2 = o2.getKey().getElement().getTextRange().getStartOffset();
+            int offset1 = p1.getFirst().getTextRange().getStartOffset();
+            int offset2 = p2.getFirst().getTextRange().getStartOffset();
            return offset1 < offset2 ? 1
                                     : offset1 > offset2 ? -1
                                                         : 0;
@@ -75,12 +81,14 @@ public class JetFunctionCallUsage extends JetUsageInfo<JetCallElement> {
    };

    private final JetFunctionDefinitionUsage<?> callee;
+    private final BindingContext context;
    private final ResolvedCall<? extends CallableDescriptor> resolvedCall;

    public JetFunctionCallUsage(@NotNull JetCallElement element, JetFunctionDefinitionUsage callee) {
        super(element);
        this.callee = callee;
-        this.resolvedCall = CallUtilPackage.getResolvedCall(element, ResolvePackage.analyze(element, BodyResolveMode.FULL));
+        this.context = ResolvePackage.analyze(element, BodyResolveMode.FULL);
+        this.resolvedCall = CallUtilPackage.getResolvedCall(element, context);
    }

    @Override
@@ -147,11 +155,36 @@ public class JetFunctionCallUsage extends JetUsageInfo<JetCallElement> {
        return getReceiverExpression(receiverValue, psiFactory);
    }

+    private boolean needSeparateVariable(@NotNull PsiElement element) {
+        if (element instanceof JetConstantExpression) return false;
+        if (element instanceof JetThisExpression) return false;
+        if (element instanceof JetSimpleNameExpression) return false;
+
+        //noinspection SuspiciousMethodCalls
+        if (element instanceof JetBinaryExpression
+            && OperatorConventions.ASSIGNMENT_OPERATIONS.containsKey(((JetBinaryExpression) element).getOperationToken())) return true;
+
+        //noinspection SuspiciousMethodCalls
+        if (element instanceof JetUnaryExpression
+            && OperatorConventions.INCREMENT_OPERATIONS.contains(((JetUnaryExpression) element).getOperationToken())) return true;
+
+        if (element instanceof JetCallExpression) {
+            ResolvedCall<? extends CallableDescriptor> resolvedCall =
+                    CallUtilPackage.getResolvedCall((JetCallExpression) element, context);
+            return resolvedCall != null && resolvedCall.getResultingDescriptor() instanceof ConstructorDescriptor;
+        }
+
+        for (PsiElement child : element.getChildren()) {
+            if (needSeparateVariable(child)) return true;
+        }
+        return false;
+    }
+
    @NotNull
    private JetExpression substituteReferences(
            @NotNull JetExpression expression,
            @NotNull Map<PsiReference, DeclarationDescriptor> referenceMap,
-            @NotNull JetPsiFactory psiFactory
+            @NotNull final JetPsiFactory psiFactory
    ) {
        if (referenceMap.isEmpty() || resolvedCall == null) return expression;

@@ -161,10 +194,9 @@ public class JetFunctionCallUsage extends JetUsageInfo<JetCallElement> {
                ExtractionEnginePackage.createNameCounterpartMap(expression, newExpression);

        Map<ValueParameterDescriptor, ResolvedValueArgument> valueArguments = resolvedCall.getValueArguments();
-        // Sort by descending offset so that call arguments are replaced before call itself
-        List<Map.Entry<PsiReference, DeclarationDescriptor>> sortedEntries =
-                ContainerUtil.sorted(referenceMap.entrySet(), REVERSE_TEXT_OFFSET_COMPARATOR);
-        for (Map.Entry<PsiReference, DeclarationDescriptor> e : sortedEntries) {
+
+        List<Pair<JetElement, JetElement>> replacements = new ArrayList<Pair<JetElement, JetElement>>();
+        for (Map.Entry<PsiReference, DeclarationDescriptor> e : referenceMap.entrySet()) {
            DeclarationDescriptor descriptor = e.getValue();

            JetExpression argumentExpression;
@@ -191,6 +223,31 @@ public class JetFunctionCallUsage extends JetUsageInfo<JetCallElement> {
            }
            if (argumentExpression == null) continue;

+            //noinspection unchecked
+            if (needSeparateVariable(argumentExpression) &&
+                PsiTreeUtil.getNonStrictParentOfType(getElement(),
+                                                     JetConstructorDelegationCall.class,
+                                                     JetDelegationSpecifier.class,
+                                                     JetParameter.class) == null) {
+
+                final Ref<JetExpression> newExpressionRef = new Ref<JetExpression>();
+                KotlinIntroduceVariableHandler.doRefactoring(
+                        getProject(),
+                        null,
+                        argumentExpression,
+                        Collections.singletonList(argumentExpression),
+                        new Function1<JetProperty, Unit>() {
+                            @Override
+                            public Unit invoke(JetProperty property) {
+                                //noinspection ConstantConditions
+                                newExpressionRef.set(psiFactory.createExpression(property.getName()));
+                                return null;
+                            }
+                        }
+                );
+                argumentExpression = newExpressionRef.get();
+            }
+
            //noinspection SuspiciousMethodCalls
            JetExpression expressionToReplace = nameCounterpartMap.get(e.getKey().getElement());
            if (expressionToReplace == null) continue;
@@ -210,13 +267,24 @@ public class JetFunctionCallUsage extends JetUsageInfo<JetCallElement> {
                        continue;
                    }
                }
-                expressionToReplace.replace(psiFactory.createExpression(argumentExpression.getText() + "." + expressionToReplace.getText()));
+                replacements.add(
+                        new Pair<JetElement, JetElement>(
+                                expressionToReplace,
+                                psiFactory.createExpression(argumentExpression.getText() + "." + expressionToReplace.getText())
+                        )
+                );
            }
            else {
-                expressionToReplace.replace(argumentExpression);
+                replacements.add(new Pair<JetElement, JetElement>(expressionToReplace, argumentExpression));
            }
        }

+        // Sort by descending offset so that call arguments are replaced before call itself
+        ContainerUtil.sort(replacements, REVERSED_TEXT_OFFSET_COMPARATOR);
+        for (Pair<JetElement, JetElement> replacement : replacements) {
+            replacement.getFirst().replace(replacement.getSecond());
+        }
+
        return newExpression;
    }

@@ -32,6 +32,7 @@ import com.intellij.refactoring.HelpID;
 import com.intellij.refactoring.introduce.inplace.OccurrencesChooser;
 import kotlin.Function1;
 import kotlin.KotlinPackage;
+import kotlin.Unit;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.kotlin.analyzer.AnalysisResult;
@@ -81,7 +82,7 @@ public class KotlinIntroduceVariableHandler extends KotlinIntroduceHandlerBase {
        JetRefactoringUtil.SelectExpressionCallback callback = new JetRefactoringUtil.SelectExpressionCallback() {
            @Override
            public void run(@Nullable JetExpression expression) {
-                doRefactoring(project, editor, expression, null);
+                doRefactoring(project, editor, expression, null, null);
            }
        };
        try {
@@ -94,8 +95,11 @@ public class KotlinIntroduceVariableHandler extends KotlinIntroduceHandlerBase {


    public static void doRefactoring(
-            @NotNull final Project project, final Editor editor, @Nullable JetExpression _expression,
-            @Nullable List<JetExpression> occurrencesToReplace
+            @NotNull final Project project,
+            @Nullable final Editor editor,
+            @Nullable JetExpression _expression,
+            @Nullable List<JetExpression> occurrencesToReplace,
+            @Nullable final Function1<JetProperty, Unit> onNonInteractiveFinish
    ) {
        if (_expression == null) {
            showErrorHint(project, editor, JetRefactoringBundle.message("cannot.refactor.no.expression"));
@@ -162,6 +166,7 @@ public class KotlinIntroduceVariableHandler extends KotlinIntroduceHandlerBase {
            return;
        }
        final boolean isInplaceAvailableOnDataContext =
+            editor != null &&
            editor.getSettings().isVariableInplaceRenameEnabled() &&
            !ApplicationManager.getApplication().isUnitTestMode();

@@ -212,19 +217,24 @@ public class KotlinIntroduceVariableHandler extends KotlinIntroduceHandlerBase {
                        ApplicationManager.getApplication().runWriteAction(introduceRunnable);
                        JetProperty property = propertyRef.get();
                        if (property != null) {
-                            editor.getCaretModel().moveToOffset(property.getTextOffset());
-                            editor.getSelectionModel().removeSelection();
-                            if (isInplaceAvailableOnDataContext) {
-                                PsiDocumentManager.getInstance(project).commitDocument(editor.getDocument());
-                                PsiDocumentManager.getInstance(project).
-                                    doPostponedOperationsAndUnblockDocument(editor.getDocument());
-                                KotlinInplaceVariableIntroducer<JetProperty> variableIntroducer =
-                                    new KotlinInplaceVariableIntroducer<JetProperty>(property, editor, project, INTRODUCE_VARIABLE,
-                                                                     references.toArray(new JetExpression[references.size()]),
-                                                                     reference.get(), finalReplaceOccurrence,
-                                                                     property, /*todo*/false, /*todo*/false,
-                                                                     expressionType, finalNoTypeInference);
-                                variableIntroducer.performInplaceRefactoring(suggestedNamesSet);
+                            if (editor != null) {
+                                editor.getCaretModel().moveToOffset(property.getTextOffset());
+                                editor.getSelectionModel().removeSelection();
+                                if (isInplaceAvailableOnDataContext) {
+                                    PsiDocumentManager.getInstance(project).commitDocument(editor.getDocument());
+                                    PsiDocumentManager.getInstance(project).
+                                            doPostponedOperationsAndUnblockDocument(editor.getDocument());
+                                    KotlinInplaceVariableIntroducer<JetProperty> variableIntroducer =
+                                            new KotlinInplaceVariableIntroducer<JetProperty>(property, editor, project, INTRODUCE_VARIABLE,
+                                                                                             references.toArray(new JetExpression[references.size()]),
+                                                                                             reference.get(), finalReplaceOccurrence,
+                                                                                             property, /*todo*/false, /*todo*/false,
+                                                                                             expressionType, finalNoTypeInference);
+                                    variableIntroducer.performInplaceRefactoring(suggestedNamesSet);
+                                }
+                            }
+                            else if (onNonInteractiveFinish != null) {
+                                onNonInteractiveFinish.invoke(property);
                            }
                        }
                    }
@@ -0,0 +1,12 @@
+// WITH_DEFAULT_VALUE: false
+class T(val t: Int)
+
+fun foo(a: Int): Int {
+    return <selection>T(a + 1)</selection>.t / 2
+}
+
+fun bar(x: Int = foo(T(2).t))
+
+fun test() {
+    foo(T(2).t)
+}
@@ -0,0 +1,13 @@
+// WITH_DEFAULT_VALUE: false
+class T(val t: Int)
+
+fun foo(t: T): Int {
+    return t.t / 2
+}
+
+fun bar(x: Int = foo(T(T(2).t + 1)))
+
+fun test() {
+    val i = T(2).t
+    foo(T(i + 1))
+}
@@ -0,0 +1,9 @@
+// WITH_DEFAULT_VALUE: false
+
+fun foo(a: Int, b: Int): Int {
+    return <selection>a * b</selection> / 2
+}
+
+fun test() {
+    foo(1.plus(2), 3.minus(4))
+}
@@ -0,0 +1,9 @@
+// WITH_DEFAULT_VALUE: false
+
+fun foo(i: Int): Int {
+    return i / 2
+}
+
+fun test() {
+    foo(1.plus(2) * 3.minus(4))
+}
@@ -0,0 +1,22 @@
+// WITH_DEFAULT_VALUE: false
+class T(val t: Int)
+
+open class A {
+    constructor(): this(1)
+
+    constructor(a: Int) {
+        val x = <selection>T(a + 1)</selection>.t / 2
+    }
+}
+
+class B: A {
+    constructor(n: Int): super(n + 1)
+}
+
+class C: A(1) {
+
+}
+
+fun test() {
+    A(2)
+}
@@ -0,0 +1,22 @@
+// WITH_DEFAULT_VALUE: false
+class T(val t: Int)
+
+open class A {
+    constructor(): this(T(1 + 1))
+
+    constructor(t: T) {
+        val x = t.t / 2
+    }
+}
+
+class B: A {
+    constructor(n: Int): super(T(n + 1 + 1))
+}
+
+class C: A(T(1 + 1)) {
+
+}
+
+fun test() {
+    A(T(2 + 1))
+}
@@ -7,12 +7,14 @@ class A(val a: Int) {
    }

    fun test() {
-        A(1).foo(a + A(1).a)
+        val a1 = A(1)
+        a1.foo(a + a1.a)
    }
 }

 fun test() {
    val t = with(A(1)) {
-        A(2).foo(a + A(2).a)
+        val a = A(2)
+        a.foo(this.a + a.a)
    }
 }
@@ -8,9 +8,13 @@ class A(val a: Int) {
 }

 fun test() {
-    A(1).foo(A(1).a + A(2).a)
+    val a = A(1)
+    val a1 = A(2)
+    a.foo(a.a + a1.a)
    with(A(1)) {
-        foo(a + A(2).a)
-        this.foo(a + A(2).a)
+        val a1 = A(2)
+        foo(this.a + a1.a)
+        val a = A(2)
+        this.foo(this.a + a.a)
    }
 }
@@ -8,9 +8,13 @@ class A(val a: Int) {
 }

 fun test() {
-    A(1).foo(A(1).a + A(2).a)
+    val a = A(1)
+    val a1 = A(2)
+    a.foo(a.a + a1.a)
    with(A(1)) {
-        foo(a + A(2).a)
-        this.foo(a + A(2).a)
+        val a1 = A(2)
+        foo(this.a + a1.a)
+        val a = A(2)
+        this.foo(this.a + a.a)
    }
 }
@@ -0,0 +1,10 @@
+// WITH_DEFAULT_VALUE: false
+
+fun foo(a: Int, b: Int): Int {
+    return <selection>a * b</selection> / 2
+}
+
+fun test() {
+    var x = 1
+    foo(++x, x++)
+}
@@ -0,0 +1,12 @@
+// WITH_DEFAULT_VALUE: false
+
+fun foo(i: Int): Int {
+    return i / 2
+}
+
+fun test() {
+    var x = 1
+    val i = ++x
+    val i1 = x++
+    foo(i * i1)
+}
@@ -2284,6 +2284,12 @@ public class JetExtractionTestGenerated extends AbstractJetExtractionTest {
            doIntroduceSimpleParameterTest(fileName);
        }

+        @TestMetadata("defaultValueNoTempVar.kt")
+        public void testDefaultValueNoTempVar() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/introduceParameter/defaultValueNoTempVar.kt");
+            doIntroduceSimpleParameterTest(fileName);
+        }
+
        @TestMetadata("forbiddenUsages.kt")
        public void testForbiddenUsages() throws Exception {
            String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/introduceParameter/forbiddenUsages.kt");
@@ -2368,9 +2374,21 @@ public class JetExtractionTestGenerated extends AbstractJetExtractionTest {
            doIntroduceSimpleParameterTest(fileName);
        }

-        @TestMetadata("substituteComplexExpressions.kt")
-        public void testSubstituteComplexExpressions() throws Exception {
-            String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/introduceParameter/substituteComplexExpressions.kt");
+        @TestMetadata("substituteBinaryExpressions.kt")
+        public void testSubstituteBinaryExpressions() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/introduceParameter/substituteBinaryExpressions.kt");
+            doIntroduceSimpleParameterTest(fileName);
+        }
+
+        @TestMetadata("substituteCallExpressions.kt")
+        public void testSubstituteCallExpressions() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/introduceParameter/substituteCallExpressions.kt");
+            doIntroduceSimpleParameterTest(fileName);
+        }
+
+        @TestMetadata("substituteDelegationCallsNoTempVar.kt")
+        public void testSubstituteDelegationCallsNoTempVar() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/introduceParameter/substituteDelegationCallsNoTempVar.kt");
            doIntroduceSimpleParameterTest(fileName);
        }

@@ -2404,6 +2422,12 @@ public class JetExtractionTestGenerated extends AbstractJetExtractionTest {
            doIntroduceSimpleParameterTest(fileName);
        }

+        @TestMetadata("substituteIncrement.kt")
+        public void testSubstituteIncrement() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/introduceParameter/substituteIncrement.kt");
+            doIntroduceSimpleParameterTest(fileName);
+        }
+
        @TestMetadata("substituteInfixCall.kt")
        public void testSubstituteInfixCall() throws Exception {
            String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/introduceParameter/substituteInfixCall.kt");