PY-6637 Make sure that names of new parameters don't clash with other names in functi...
authorMikhail Golubev <mikhail.golubev@jetbrains.com>
Mon, 5 Oct 2015 15:54:02 +0000 (18:54 +0300)
committerMikhail Golubev <mikhail.golubev@jetbrains.com>
Mon, 5 Oct 2015 15:54:02 +0000 (18:54 +0300)
python/src/com/jetbrains/python/refactoring/makeFunctionTopLevel/PyBaseMakeFunctionTopLevelProcessor.java
python/src/com/jetbrains/python/refactoring/makeFunctionTopLevel/PyMakeLocalFunctionTopLevelProcessor.java
python/src/com/jetbrains/python/refactoring/makeFunctionTopLevel/PyMakeMethodTopLevelProcessor.java
python/testData/refactoring/convertTopLevel/methodUniqueParamNames.after.py [new file with mode: 0644]
python/testData/refactoring/convertTopLevel/methodUniqueParamNames.py [new file with mode: 0644]
python/testSrc/com/jetbrains/python/refactoring/PyMakeFunctionTopLevelTest.java

index e0eb710c8144f0ba1c39440661c70f41b15cd97b..078a2c3b235dd67631e0c971761e9410cdcb1cec 100644 (file)
@@ -42,7 +42,6 @@ import org.jetbrains.annotations.NotNull;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
-import java.util.Set;
 
 /**
  * @author Mikhail Golubev
@@ -92,7 +91,7 @@ public abstract class PyBaseMakeFunctionTopLevelProcessor extends BaseRefactorin
 
   @Override
   protected final void performRefactoring(@NotNull UsageInfo[] usages) {
-    final Set<String> newParameters = collectNewParameterNames();
+    final List<String> newParameters = collectNewParameterNames();
 
     assert ApplicationManager.getApplication().isWriteAccessAllowed();
 
@@ -131,7 +130,7 @@ public abstract class PyBaseMakeFunctionTopLevelProcessor extends BaseRefactorin
   }
 
   @NotNull
-  protected abstract Set<String> collectNewParameterNames();
+  protected abstract List<String> collectNewParameterNames();
 
   @NotNull
   protected PyFunction insertNewFunction(@NotNull PyFunction newFunction) {
index ec5d6319c60ecb5102b3af42f3c5d752ae3659a1..7dd384baaff7115fb476d7da8c1890e529cd4b8e 100644 (file)
@@ -15,6 +15,7 @@
  */
 package com.jetbrains.python.refactoring.makeFunctionTopLevel;
 
+import com.google.common.collect.Lists;
 import com.intellij.openapi.editor.Editor;
 import com.intellij.psi.PsiElement;
 import com.intellij.psi.util.PsiTreeUtil;
@@ -30,6 +31,7 @@ import org.jetbrains.annotations.NotNull;
 
 import java.util.Collection;
 import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Set;
 
 import static com.jetbrains.python.psi.PyUtil.as;
@@ -73,7 +75,7 @@ public class PyMakeLocalFunctionTopLevelProcessor extends PyBaseMakeFunctionTopL
 
   @Override
   @NotNull
-  protected Set<String> collectNewParameterNames() {
+  protected List<String> collectNewParameterNames() {
     final Set<String> enclosingScopeReads = new LinkedHashSet<String>();
     for (ScopeOwner owner : PsiTreeUtil.collectElementsOfType(myFunction, ScopeOwner.class)) {
       final AnalysisResult result = analyseScope(owner);
@@ -89,6 +91,6 @@ public class PyMakeLocalFunctionTopLevelProcessor extends PyBaseMakeFunctionTopL
         }
       }
     }
-    return enclosingScopeReads;
+    return Lists.newArrayList(enclosingScopeReads);
   }
 }
index 821781e255327f8907f4de497087133b789fb4b7..00e90d9f3d8740049a6549a8b35ee34d32cab413 100644 (file)
@@ -16,6 +16,7 @@
 package com.jetbrains.python.refactoring.makeFunctionTopLevel;
 
 import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
 import com.intellij.openapi.application.ApplicationManager;
 import com.intellij.openapi.editor.Editor;
 import com.intellij.psi.PsiElement;
@@ -25,6 +26,7 @@ import com.intellij.usageView.UsageInfo;
 import com.intellij.util.Function;
 import com.intellij.util.IncorrectOperationException;
 import com.intellij.util.containers.ContainerUtil;
+import com.intellij.util.containers.MultiMap;
 import com.jetbrains.python.PyBundle;
 import com.jetbrains.python.PyNames;
 import com.jetbrains.python.codeInsight.controlflow.ScopeOwner;
@@ -45,7 +47,7 @@ import static com.jetbrains.python.psi.PyUtil.as;
  */
 public class PyMakeMethodTopLevelProcessor extends PyBaseMakeFunctionTopLevelProcessor {
 
-  private final List<PyReferenceExpression> myReferencesToSelf = new ArrayList<PyReferenceExpression>();
+  private final MultiMap<String, PyReferenceExpression> myAttrReferences = MultiMap.create();
 
   public PyMakeMethodTopLevelProcessor(@NotNull PyFunction targetFunction, @NotNull Editor editor) {
     super(targetFunction, editor);
@@ -133,7 +135,7 @@ public class PyMakeMethodTopLevelProcessor extends PyBaseMakeFunctionTopLevelPro
   }
 
   @NotNull
-  private String selectUniqueName(@NotNull PsiElement anchor) {
+  private String selectUniqueName(@NotNull PsiElement scopeAnchor) {
     final PyClass pyClass = myFunction.getContainingClass();
     assert pyClass != null;
     final Collection<String> suggestions;
@@ -144,22 +146,31 @@ public class PyMakeMethodTopLevelProcessor extends PyBaseMakeFunctionTopLevelPro
       suggestions = Collections.singleton("inst");
     }
     for (String name : suggestions) {
-      if (!(IntroduceValidator.isDefinedInScope(name, anchor) || PyNames.isReserved(name))) {
+      if (isValidName(name, scopeAnchor)) {
         return name;
       }
     }
-    final String shortestName = Iterables.getLast(suggestions);
+
+    //noinspection ConstantConditions
+    return appendNumberUntilValid(Iterables.getLast(suggestions), scopeAnchor);
+  }
+
+  private static boolean isValidName(@NotNull String name, @NotNull PsiElement scopeAnchor) {
+    return !(IntroduceValidator.isDefinedInScope(name, scopeAnchor) || PyNames.isReserved(name));
+  }
+
+  @NotNull
+  private static String appendNumberUntilValid(@NotNull String name, @NotNull PsiElement scopeAnchor) {
     int counter = 1;
-    String name = shortestName;
-    while (IntroduceValidator.isDefinedInScope(name, anchor) || PyNames.isReserved(name)) {
-      name = shortestName + counter;
+    String candidate = name;
+    while (!isValidName(candidate, scopeAnchor)) {
+      candidate = name + counter;
       counter++;
     }
-    //noinspection ConstantConditions
-    return name;
+    return candidate;
   }
 
-  private boolean resolvesToClass(PyExpression qualifier) {
+  private boolean resolvesToClass(@NotNull PyExpression qualifier) {
     for (PsiElement element : PyUtil.multiResolveTopPriority(qualifier, myResolveContext)) {
       if (element == myFunction.getContainingClass()) {
         return true;
@@ -188,21 +199,37 @@ public class PyMakeMethodTopLevelProcessor extends PyBaseMakeFunctionTopLevelPro
   @NotNull
   @Override
   protected PyFunction createNewFunction(@NotNull Collection<String> newParams) {
-    for (PyReferenceExpression expr : myReferencesToSelf) {
-      PyUtil.removeQualifier(expr);
+    final List<String> updatedParamNames = new ArrayList<String>();
+    for (String name : newParams) {
+      final Collection<PyReferenceExpression> reads = myAttrReferences.get(name);
+      final PsiElement anchor = ContainerUtil.getFirstItem(reads);
+      //noinspection ConstantConditions
+      if (!isValidName(name, anchor)) {
+        final String indexedName = appendNumberUntilValid(name, anchor);
+        updatedParamNames.add(indexedName);
+        for (PyReferenceExpression read : reads) {
+          read.replace(myGenerator.createExpressionFromText(LanguageLevel.forElement(read), indexedName));
+        }
+      }
+      else {
+        updatedParamNames.add(name);
+        for (PyReferenceExpression read : reads) {
+          removeQualifier(read);
+        }
+      }
     }
     final PyFunction copied = (PyFunction)myFunction.copy();
     final PyParameter[] params = copied.getParameterList().getParameters();
     if (params.length > 0) {
       params[0].delete();
     }
-    addParameters(copied.getParameterList(), newParams);
+    addParameters(copied.getParameterList(), updatedParamNames);
     return copied;
   }
 
   @NotNull
   @Override
-  protected Set<String> collectNewParameterNames() {
+  protected List<String> collectNewParameterNames() {
     final Set<String> paramNames = new LinkedHashSet<String>();
     for (ScopeOwner owner : PsiTreeUtil.collectElementsOfType(myFunction, ScopeOwner.class)) {
       final AnalysisResult result = analyseScope(owner);
@@ -232,13 +259,13 @@ public class PyMakeMethodTopLevelProcessor extends PyBaseMakeFunctionTopLevelPro
             throw new IncorrectOperationException(PyBundle.message("refactoring.make.function.top.level.error.method.calls"));
           }
           paramNames.add(attrName);
-          myReferencesToSelf.add(parentReference);
+          myAttrReferences.putValue(attrName, parentReference);
         }
         else {
           throw new IncorrectOperationException(PyBundle.message("refactoring.make.function.top.level.error.special.usage.of.self"));
         }
       }
     }
-    return paramNames;
+    return Lists.newArrayList(paramNames);
   }
 }
diff --git a/python/testData/refactoring/convertTopLevel/methodUniqueParamNames.after.py b/python/testData/refactoring/convertTopLevel/methodUniqueParamNames.after.py
new file mode 100644 (file)
index 0000000..6565bb1
--- /dev/null
@@ -0,0 +1,10 @@
+class C:
+    pass
+
+
+def method(foo2, bar1, foo, foo1, bar):
+    print(foo2, bar1)
+
+
+c = C()
+method(c.foo, c.bar, 1, 2, bar=3)
\ No newline at end of file
diff --git a/python/testData/refactoring/convertTopLevel/methodUniqueParamNames.py b/python/testData/refactoring/convertTopLevel/methodUniqueParamNames.py
new file mode 100644 (file)
index 0000000..7e1165a
--- /dev/null
@@ -0,0 +1,6 @@
+class C:
+    def me<caret>thod(self, foo, foo1, bar):
+        print(self.foo, self.bar)
+        
+
+C().method(1, 2, bar=3)
\ No newline at end of file
index 835d7f29c63352ffc9eb3dd3d81a2e3cd1fb03ce..ad43af51311500c4f4c273462cd5d1174b6a7b10 100644 (file)
@@ -194,6 +194,10 @@ public class PyMakeFunctionTopLevelTest extends PyTestCase {
     doTestSuccess();
   }
 
+  public void testMethodUniqueParamNames() {
+    doTestSuccess();
+  }
+
   @Override
   protected String getTestDataPath() {
     return super.getTestDataPath() + "/refactoring/convertTopLevel/";