PY-6637 Handle simple cases without nonlocal references and generators
authorMikhail Golubev <mikhail.golubev@jetbrains.com>
Mon, 28 Sep 2015 17:09:56 +0000 (20:09 +0300)
committerMikhail Golubev <mikhail.golubev@jetbrains.com>
Mon, 5 Oct 2015 10:08:59 +0000 (13:08 +0300)
python/resources/intentionDescriptions/PyConvertLocalFunctionToTopLevelFunction/after.py.template [new file with mode: 0644]
python/resources/intentionDescriptions/PyConvertLocalFunctionToTopLevelFunction/before.py.template [new file with mode: 0644]
python/resources/intentionDescriptions/PyConvertLocalFunctionToTopLevelFunction/description.html [new file with mode: 0644]
python/src/META-INF/python-core.xml
python/src/com/jetbrains/python/codeInsight/intentions/PyConvertLocalFunctionToTopLevelFunction.java
python/src/com/jetbrains/python/refactoring/extractmethod/PyExtractMethodUtil.java
python/testData/intentions/afterMakeLocalFunctionTopLevelSimple.py [new file with mode: 0644]
python/testData/intentions/beforeMakeLocalFunctionTopLevelSimple.py [new file with mode: 0644]
python/testSrc/com/jetbrains/python/intentions/PyIntentionTest.java

diff --git a/python/resources/intentionDescriptions/PyConvertLocalFunctionToTopLevelFunction/after.py.template b/python/resources/intentionDescriptions/PyConvertLocalFunctionToTopLevelFunction/after.py.template
new file mode 100644 (file)
index 0000000..d3d72aa
--- /dev/null
@@ -0,0 +1,6 @@
+def nested(y):
+    return x + y
+       
+
+def func(x):
+    return nested(x, 42)
\ No newline at end of file
diff --git a/python/resources/intentionDescriptions/PyConvertLocalFunctionToTopLevelFunction/before.py.template b/python/resources/intentionDescriptions/PyConvertLocalFunctionToTopLevelFunction/before.py.template
new file mode 100644 (file)
index 0000000..a7ba141
--- /dev/null
@@ -0,0 +1,4 @@
+def func(x):
+    def nested(y):
+        return x + y
+    return nested(42)
\ No newline at end of file
diff --git a/python/resources/intentionDescriptions/PyConvertLocalFunctionToTopLevelFunction/description.html b/python/resources/intentionDescriptions/PyConvertLocalFunctionToTopLevelFunction/description.html
new file mode 100644 (file)
index 0000000..83c8819
--- /dev/null
@@ -0,0 +1,7 @@
+<html>
+<body>
+<span style="font-family: verdana,sans-serif;">
+  This intention converts local function to top-level function.
+</span>
+</body>
+</html>
\ No newline at end of file
index 410b3ff9b4ccfcfbb5ff4785a137d238ff95122b..8e6db9ea9549b029d24ae1f5930326a8286360e7 100644 (file)
       <className>com.jetbrains.python.codeInsight.intentions.PyConvertStaticMethodToFunctionIntention</className>
       <category>Python</category>
     </intentionAction>
+    
+    <intentionAction>
+      <className>com.jetbrains.python.codeInsight.intentions.PyConvertLocalFunctionToTopLevelFunction</className>
+      <category>Python</category>
+    </intentionAction>
+
 
     <intentionAction>
       <className>com.jetbrains.python.codeInsight.intentions.SpecifyTypeInDocstringIntention</className>
index a55f9eb53ebcebcd81bb89a181db6d024f47cf11..84f9454566d19013f6ed4ee757005be48ea106cf 100644 (file)
 package com.jetbrains.python.codeInsight.intentions;
 
 import com.intellij.codeInsight.controlflow.ControlFlow;
+import com.intellij.codeInsight.controlflow.Instruction;
 import com.intellij.codeInsight.intention.impl.BaseIntentionAction;
 import com.intellij.openapi.editor.Editor;
 import com.intellij.openapi.project.Project;
+import com.intellij.openapi.roots.ProjectRootManager;
+import com.intellij.openapi.util.text.StringUtil;
+import com.intellij.openapi.vfs.VirtualFile;
 import com.intellij.psi.PsiElement;
 import com.intellij.psi.PsiFile;
 import com.intellij.psi.PsiReference;
 import com.intellij.psi.util.PsiTreeUtil;
+import com.intellij.usageView.UsageInfo;
 import com.intellij.util.IncorrectOperationException;
+import com.intellij.util.containers.ContainerUtil;
 import com.jetbrains.python.PyBundle;
 import com.jetbrains.python.codeInsight.controlflow.ControlFlowCache;
+import com.jetbrains.python.codeInsight.controlflow.ReadWriteInstruction;
 import com.jetbrains.python.codeInsight.controlflow.ScopeOwner;
-import com.jetbrains.python.psi.PyFile;
-import com.jetbrains.python.psi.PyFunction;
-import com.jetbrains.python.psi.PyUtil;
+import com.jetbrains.python.codeInsight.dataflow.scope.ScopeUtil;
+import com.jetbrains.python.psi.*;
+import com.jetbrains.python.psi.impl.PyPsiUtils;
+import com.jetbrains.python.refactoring.PyRefactoringUtil;
 import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
+import java.util.*;
+
+import static com.jetbrains.python.psi.PyUtil.as;
+
 /**
  * @author Mikhail Golubev
  */
 public class PyConvertLocalFunctionToTopLevelFunction extends BaseIntentionAction {
+  public PyConvertLocalFunctionToTopLevelFunction() {
+    setText(PyBundle.message("INTN.convert.local.function.to.top.level.function"));
+  }
+
   @Nls
   @NotNull
   @Override
@@ -58,18 +74,27 @@ public class PyConvertLocalFunctionToTopLevelFunction extends BaseIntentionActio
     if (element == null) {
       return null;
     }
+    PyFunction result = null;
     if (isLocalFunction(element.getParent()) && ((PyFunction)element.getParent()).getNameIdentifier() == element) {
-      return (PyFunction)element.getParent();
+      result = (PyFunction)element.getParent();
     }
-    final PsiReference reference = element.getReference();
-    if (reference == null) {
-      return null;
+    else {
+      final PyReferenceExpression refExpr = PsiTreeUtil.getParentOfType(element, PyReferenceExpression.class);
+      if (refExpr == null) {
+        return null;
+      }
+      final PsiElement resolved = refExpr.getReference().resolve();
+      if (isLocalFunction(resolved)) {
+        result = (PyFunction)resolved;
+      }
     }
-    final PsiElement resolved = reference.resolve();
-    if (isLocalFunction(resolved)) {
-      return (PyFunction)resolved;
+    if (result != null) {
+      final VirtualFile virtualFile = result.getContainingFile().getVirtualFile();
+      if (virtualFile != null && ProjectRootManager.getInstance(file.getProject()).getFileIndex().isInLibraryClasses(virtualFile)) {
+        return null;
+      }
     }
-    return null;
+    return result;
   }
 
   private static boolean isLocalFunction(@Nullable PsiElement resolved) {
@@ -83,6 +108,76 @@ public class PyConvertLocalFunctionToTopLevelFunction extends BaseIntentionActio
   public void invoke(@NotNull Project project, Editor editor, PsiFile file) throws IncorrectOperationException {
     final PyFunction function = findNestedFunctionUnderCaret(editor, file);
     assert function != null;
-    final ControlFlow flow = ControlFlowCache.getControlFlow(function);
+    final Set<String> enclosingScopeReads = new LinkedHashSet<String>(); 
+    final Collection<ScopeOwner> scopeOwners = PsiTreeUtil.collectElementsOfType(function, ScopeOwner.class);
+    for (ScopeOwner owner : scopeOwners) {
+      for (PsiElement element : findReadsFromEnclosingScope(owner, function)) {
+        if (element instanceof PyElement) {
+          ContainerUtil.addIfNotNull(enclosingScopeReads, ((PyElement)element).getName());
+        }
+      }
+    }
+    final String commaSeparatedNames = StringUtil.join(enclosingScopeReads, ", ");
+
+    // Update existing usages
+    final PyElementGenerator elementGenerator = PyElementGenerator.getInstance(file.getProject());
+    for (UsageInfo usage : PyRefactoringUtil.findUsages(function, false)) {
+      final PsiElement element = usage.getElement();
+      if (element != null) {
+        final PyCallExpression parentCall = as(element.getParent(), PyCallExpression.class);
+        if (parentCall != null) {
+          final PyArgumentList argList = parentCall.getArgumentList();
+          if (argList != null) {
+            final StringBuilder argListText = new StringBuilder(argList.getText());
+            argListText.insert(1, commaSeparatedNames + (argList.getArguments().length > 0 ? ", " : ""));
+            argList.replace(elementGenerator.createArgumentList(LanguageLevel.forElement(element), argListText.toString()));
+          }
+        }
+      }
+    }
+
+    // Replace function
+    PyFunction copiedFunction = (PyFunction)function.copy();
+    final PyParameterList paramList = copiedFunction.getParameterList();
+    final StringBuilder paramListText = new StringBuilder(paramList.getText());
+    paramListText.insert(1, commaSeparatedNames + (paramList.getParameters().length > 0 ? ", " : ""));
+    paramList.replace(elementGenerator.createParameterList(LanguageLevel.forElement(function), paramListText.toString()));
+
+    // See AddImportHelper.getFileInsertPosition()
+    final PsiElement anchor = PyPsiUtils.getParentRightBefore(function, file);
+
+    copiedFunction = (PyFunction)file.addAfter(copiedFunction, anchor);
+    function.delete();
+    
+    editor.getSelectionModel().removeSelection();
+    editor.getCaretModel().moveToOffset(copiedFunction.getTextOffset());
+  }
+
+  @NotNull
+  private static List<PsiElement> findReadsFromEnclosingScope(@NotNull ScopeOwner owner, @NotNull PyFunction targetFunction) {
+    final ControlFlow controlFlow = ControlFlowCache.getControlFlow(owner);
+    final List<PsiElement> result = new ArrayList<PsiElement>();
+    for (Instruction instruction : controlFlow.getInstructions()) {
+      if (instruction instanceof ReadWriteInstruction) {
+        final ReadWriteInstruction readInstruction = (ReadWriteInstruction)instruction;
+        if (readInstruction.getAccess().isReadAccess()) {
+          final PsiElement element = readInstruction.getElement();
+          if (element != null) {
+            for (PsiReference reference : element.getReferences()) {
+              final PsiElement resolved = reference.resolve();
+              if (resolved != null && isFromEnclosingScope(resolved, targetFunction)) {
+                result.add(element);
+                break;
+              }
+            }
+          }
+        }
+      }
+    }
+    return result; 
+  }
+
+  private static boolean isFromEnclosingScope(@NotNull PsiElement element, @NotNull PyFunction targetFunction) {
+    return !PsiTreeUtil.isAncestor(targetFunction, element, false) && !(ScopeUtil.getScopeOwner(element) instanceof PsiFile);
   }
 }
index b0ffe97eec0bedffad0b568885b4c5a19d8352f4..9a7161c8d057bf3dcc8c6737e0ed03a1961dc1d1 100644 (file)
@@ -489,7 +489,7 @@ public class PyExtractMethodUtil {
     final PsiNamedElement parent = PsiTreeUtil.getParentOfType(anchor, PyFile.class, PyClass.class, PyFunction.class);
 
     final PsiElement result;
-    // The only safe case to insert extracted function *after* original scope owner is function.
+    // The only safe case to insert extracted function *after* the original scope owner is when it's function.
     if (parent instanceof PyFunction) {
       result = parent.getParent().addAfter(generatedMethod, parent);
     }
diff --git a/python/testData/intentions/afterMakeLocalFunctionTopLevelSimple.py b/python/testData/intentions/afterMakeLocalFunctionTopLevelSimple.py
new file mode 100644 (file)
index 0000000..790bd89
--- /dev/null
@@ -0,0 +1,14 @@
+global_var = 'spam'
+
+
+def enclosing(p1, p2):
+    x = 42
+
+    local(p1, x, 'foo')
+
+
+def local(p1, x, p):
+    def nested():
+        print(p, x)
+
+    print(p1, p)
\ No newline at end of file
diff --git a/python/testData/intentions/beforeMakeLocalFunctionTopLevelSimple.py b/python/testData/intentions/beforeMakeLocalFunctionTopLevelSimple.py
new file mode 100644 (file)
index 0000000..262beac
--- /dev/null
@@ -0,0 +1,13 @@
+global_var = 'spam'
+
+
+def enclosing(p1, p2):
+    x = 42
+
+    def lo<caret>cal(p):
+        def nested():
+            print(p, x)
+
+        print(p1, p)
+
+    local('foo')
\ No newline at end of file
index 7d23e79313ffbaea85535404bc198894ec7418b1..ffb997b6e46dea772e9fe795c81c516a6ec5ebfd 100644 (file)
@@ -686,6 +686,11 @@ public class  PyIntentionTest extends PyTestCase {
     doTest(PyBundle.message("INTN.convert.static.method.to.function"));
   }
 
+  // PY-6637
+  public void testMakeLocalFunctionTopLevelSimple() {
+    doTest(PyBundle.message("INTN.convert.local.function.to.top.level.function"));
+  }
+
   private void doDocStubTest(@NotNull DocStringFormat format) {
     runWithDocStringFormat(format, new Runnable() {
       @Override