PY-10829 Extracted function is inserted *after* the function where it is used

I updated test data accordingly and removed now obsolete tests about skipping preceding comments.
2026-09-18 09:34:34 +07:00 · 2015-04-02 20:08:32 +03:00
parent 77b11662fa
commit 45cc683033
41 changed files with 191 additions and 224 deletions
@@ -467,17 +467,18 @@ public class PyExtractMethodUtil {
    }
    final PsiNamedElement parent = PsiTreeUtil.getParentOfType(anchor, PyFile.class, PyClass.class, PyFunction.class);

-    final PsiElement insertionAnchor;
-    if (parent instanceof PyFile || parent instanceof PyClass) {
-      final PsiElement target = parent instanceof PyClass ? ((PyClass)parent).getStatementList() : parent;
-      insertionAnchor = PyPsiUtils.getParentRightBefore(anchor, target);
+    final PsiElement result;
+    // The only safe case to insert extracted function *after* original scope owner is function.
+    if (parent instanceof PyFunction) {
+      result = parent.getParent().addAfter(generatedMethod, parent);
    }
    else {
-      insertionAnchor = parent;
+      final PsiElement target = parent instanceof PyClass ? ((PyClass)parent).getStatementList() : parent;
+      final PsiElement insertionAnchor = PyPsiUtils.getParentRightBefore(anchor, target);
+      assert insertionAnchor != null;
+      final Couple<PsiComment> comments = PyPsiUtils.getPrecedingComments(insertionAnchor);
+      result = insertionAnchor.getParent().addBefore(generatedMethod, comments != null ? comments.getFirst() : insertionAnchor);
    }
-    assert insertionAnchor != null;
-    final Couple<PsiComment> comments = PyPsiUtils.getPrecedingComments(insertionAnchor);
-    final PsiElement result = insertionAnchor.getParent().addBefore(generatedMethod, comments != null ? comments.getFirst() : insertionAnchor);
    // to ensure correct reformatting, mark the entire method as generated
    result.accept(new PsiRecursiveElementVisitor() {
      @Override
@@ -1,6 +1,6 @@
-def bar(r_new):
-    return PI * r_new ** 2
-
-
 def cylinder_volume(r, h):
    h * bar(r)
+
+
+def bar(r_new):
+    return PI * r_new ** 2
@@ -1,8 +1,8 @@
 class C:
-    @classmethod
-    def baz(cls):
-        print('foo', cls)
-
    @classmethod
    def foo(cls):
        cls.baz()
+
+    @classmethod
+    def baz(cls):
+        print('foo', cls)
@@ -1,11 +1,11 @@
+def foo():
+    c = bar()
+    return c
+
+
 def bar():
    class C(object):
        pass

    c = C()
-    return c
-
-
-def foo():
-    c = bar()
    return c
@@ -1,7 +0,0 @@
-def bar():
-    print("Hello")
-
-
-#Comment to method
-def foo():
-    bar()
@@ -1,3 +0,0 @@
-#Comment to method
-def foo():
-    <selection>print("Hello")</selection>
@@ -1,10 +1,10 @@
-def bar(c_new):
-    if c_new:
-        print(1)
-
-
 def foo(c):
    x = 1
    bar(c)
    # Comment
-    return x
+    return x
+
+
+def bar(c_new):
+    if c_new:
+        print(1)
@@ -1,9 +1,9 @@
-def bar():
-    if 11 > 4:
-        print('ok')
-
-
 def main(indices):
    foo = True
    bar()
    return foo
+
+
+def bar():
+    if 11 > 4:
+        print('ok')
@@ -1,11 +1,11 @@
-def foo(a_new):
-    b1 = do_smth_with(a_new)
-    return b1
-
-
 def f():
    a = do_smth()
    b1 = foo(a)
    a = do_smth()
    b = foo(a + 1)
    do_smth_with(b1, b)
+
+
+def foo(a_new):
+    b1 = do_smth_with(a_new)
+    return b1
@@ -1,11 +1,11 @@

 class A:
-    def foo(self):
-        print 1
-
    def baz(self):
        self.foo()
        self.foo()

+    def foo(self):
+        print 1
+
    def bar(self):
        self.foo()
@@ -1,8 +1,8 @@
-def foo():
-    a = 1
-    print a
-
-
 def bar():
    foo()
    foo()
+
+
+def foo():
+    a = 1
+    print a
@@ -1,10 +1,10 @@
-def foo():
-    a = 1
-    return a
-
-
 def bar():
    a = foo()
    print a
    a = foo()
    print a
+
+
+def foo():
+    a = 1
+    return a
@@ -1,10 +1,10 @@
-def foo(a_new):
-    print a_new
-
-
 def bar():
    a = 1
    b = 2

    foo(a)
    foo(b)
+
+
+def foo(a_new):
+    print a_new
@@ -1,8 +1,3 @@
-def baz(f_new):
-    length = len(f_new.readlines())  # <---extract something from here
-    print("hi from else")
-
-
 def foo():
    for arg in sys.argv[1:]:
        try:
@@ -11,4 +6,9 @@ def foo():
            print('cannot open', arg)
        else:
            baz(f)
-            #anything else you need
+            #anything else you need
+
+
+def baz(f_new):
+    length = len(f_new.readlines())  # <---extract something from here
+    print("hi from else")
@@ -1,8 +1,8 @@
-def plus(a_new, b_new):
-    return a_new + b_new * 123
-
-
 def f():
    a = 1
    b = 1
-    c = plus(a, b)
+    c = plus(a, b)
+
+
+def plus(a_new, b_new):
+    return a_new + b_new * 123
@@ -1,10 +1,10 @@
+def __init__(self):
+    for base in self__class__.__bases__:
+        bar(base, self)
+
+
 def bar(base_new, self_new):
    try:
        base_new.__init__(self_new)
    except AttributeError:
        pass
-
-
-def __init__(self):
-    for base in self__class__.__bases__:
-        bar(base, self)
@@ -1,11 +1,10 @@
 x = 0

+def foo():
+    global x
+    bar()
+

 def bar():
    global x
-    x = 1
-
-
-def foo():
-    global x
-    bar()
+    x = 1
@@ -1,9 +1,9 @@
-def bar(f_new):
-    return f_new(1)
-
-
 def foo():
    def f(x):
        return x

-    return bar(f)
+    return bar(f)
+
+
+def bar(f_new):
+    return f_new(1)
@@ -1,15 +1,15 @@
-def bar(foo_new, i_new):
-    need_break = False
-    if i_new > 2:
-        foo_new = False
-        need_break = True
-    return foo_new, need_break
-
-
 def main(indices):
    foo = True
    for i in indices:
        foo, need_break = bar(foo, i)
        if need_break:
            break
-    return foo
+    return foo
+
+
+def bar(foo_new, i_new):
+    need_break = False
+    if i_new > 2:
+        foo_new = False
+        need_break = True
+    return foo_new, need_break
@@ -1,7 +1,7 @@
 class C:
+  def foo(self):
+      self.bar()
+
  def bar(self):
      for x in [1, 2]:
-          print x
-
-  def foo(self):
-      self.bar()
+          print x
@@ -1,10 +1,10 @@
 class C:
-    def baz(self, arg_new):
-        self.bar(arg_new)
-
    def foo(self, option, arg):
        if option:
            self.baz(arg)

+    def baz(self, arg_new):
+        self.bar(arg_new)
+
    def bar(self, arg):
        pass
@@ -1,10 +1,10 @@
 class Foo(X, Y, Z):
+    def __init__(self):
+        for base in self__class__.__bases__:
+            self.bar(base)
+
    def bar(self, base_new):
        try:
            base_new.__init__(self)
        except AttributeError:
            pass
-
-    def __init__(self):
-        for base in self__class__.__bases__:
-            self.bar(base)
@@ -1,9 +1,9 @@
 class C:
+    def foo(self):
+        return self.bar()
+
    def bar(self):
        if (self.cond):
            return 1
        else:
-            return 2
-
-    def foo(self):
-        return self.bar()
+            return 2
@@ -1,13 +1,13 @@
 def foo():
    x = 1
+    def bar():
+        nonlocal x
+        baz()
+        print(x)

    def baz():
        nonlocal x
        x = 2

-    def bar():
-        nonlocal x
-        baz()
-        print(x)
    bar()
 foo()
@@ -1,12 +1,12 @@
-def sum_squares(a_new, result_new):
-    while a_new < 10:
-        result_new += a_new * a_new
-        a_new += 1
-    return result_new
-
-
 def f():
    a = 10
    result = 0
    result = sum_squares(a, result)
    print("Sum of squares: " + result)
+
+
+def sum_squares(a_new, result_new):
+    while a_new < 10:
+        result_new += a_new * a_new
+        a_new += 1
+    return result_new
@@ -1,12 +1,12 @@
+def f():
+    a = 1
+    a, result = sum_squares(a)
+    print("Sum of squares: " + a + " = " + result)
+
+
 def sum_squares(a_new):
    result = 0
    while a_new < 10:
        result += a_new * a_new
        a_new += 1
    return a_new, result
-
-
-def f():
-    a = 1
-    a, result = sum_squares(a)
-    print("Sum of squares: " + a + " = " + result)
@@ -1,6 +1,6 @@
-def bar(a_new):
-    print(a_new)
-
-
 def foo(a):
    bar(a)
+
+
+def bar(a_new):
+    print(a_new)
@@ -2,14 +2,6 @@ class Foo:
        def __init__(self):
            self.tmp = False

-        def bar(self, condition3_new, condition4_new):
-            self.tmp2 = True
-            if condition3_new:
-                print(condition3_new)
-            if condition4_new:
-                print(condition4_new)
-            print("misterious extract method test")
-
        def extract_method(self, condition1, condition2, condition3, condition4):
            list = (1, 2, 3)
            a = 6
@@ -21,5 +13,15 @@ class Foo:
                    print(b)
            else:
                self.bar(condition3, condition4)
+
+        def bar(self, condition3_new, condition4_new):
+            self.tmp2 = True
+            if condition3_new:
+                print(condition3_new)
+            if condition4_new:
+                print(condition4_new)
+            print("misterious extract method test")
+
+
 f = Foo()
 f.extract_method(True, True, True, True)
@@ -1,6 +1,6 @@
-def bar(p_name_new, params_new):
-    return p_name_new + '(' + ', '.join(params_new) + ')'
-
-
 def x(p_name, params):
-    return bar(p_name, params), None
+    return bar(p_name, params), None
+
+
+def bar(p_name_new, params_new):
+    return p_name_new + '(' + ', '.join(params_new) + ')'
@@ -1,9 +0,0 @@
-# Unrelated comment
-def bar():
-    print("Hello")
-
-
-# Comment to method
-# continuation
-def foo():
-    bar()
@@ -1,6 +0,0 @@
-# Unrelated comment
-
-# Comment to method
-# continuation
-def foo():
-    <selection>print("Hello")</selection>
@@ -1,10 +1,10 @@
+def foo(x):
+    bar()
+    print(1)
+
+
 def bar():
    try:
        pass
    except Exception:
-        raise
-
-
-def foo(x):
-    bar()
-    print(1)
+        raise
@@ -1,8 +1,8 @@
-def foo(a_new, b_new):
-    print(a_new + b_new * 123)
-
-
 def f():
    a = 1
    b = 1
-    foo(a, b)
+    foo(a, b)
+
+
+def foo(a_new, b_new):
+    print(a_new + b_new * 123)
@@ -1,10 +1,10 @@
-def foo(a_new, b_new):
-    puts(a_new + b_new * 123)
-    print("Hello from extract method")
-
-
 def f():
    a = 1
    b = 1
    foo(a, b)

+
+def foo(a_new, b_new):
+    puts(a_new + b_new * 123)
+    print("Hello from extract method")
+
@@ -1,8 +1,8 @@
 class C:
    @staticmethod
-    def baz():
-        print "hello world"
+    def foo():
+        C.baz()

    @staticmethod
-    def foo():
-        C.baz()
+    def baz():
+        print "hello world"
@@ -1,12 +1,12 @@
+def foo(f):
+    x = 1
+    x = bar(f, x)
+    return x
+
+
 def bar(f_new, x_new):
    try:
        x_new = f_new()
    except Exception:
        pass
-    return x_new
-
-
-def foo(f):
-    x = 1
-    x = bar(f, x)
-    return x
+    return x_new
@@ -1,10 +1,10 @@
+def foo(x):
+    bar()
+    return x
+
+
 def bar():
    try:
        print(1)
    finally:
        pass
-
-
-def foo(x):
-    bar()
-    return x
@@ -1,11 +1,11 @@
-def bar(a_new):
-    do_smth
-    a_new += 1
-    return a_new
-
-
 def f():
    a = 1
    while a < 10:
        a = bar(a)
    print(a)
+
+
+def bar(a_new):
+    do_smth
+    a_new += 1
+    return a_new
@@ -1,11 +1,11 @@
+def f(xs):
+    found = False
+    found = yield from bar(found, xs)
+    print(found)
+
+
 def bar(found_new, xs_new):
    for x in xs_new:
        yield x
        found_new = True
-    return found_new
-
-
-def f(xs):
-    found = False
-    found = yield from bar(found, xs)
-    print(found)
+    return found_new
@@ -3,10 +3,10 @@ def f(x, y):
    return x, y


-def bar(in1_new, in2_new):
-    return (yield from f(in1_new, in2_new))
-
-
 def g(in1, in2):
    out1, out2 = yield from bar(in1, in2)
-    print(out1, out2)
+    print(out1, out2)
+
+
+def bar(in1_new, in2_new):
+    return (yield from f(in1_new, in2_new))
@@ -121,16 +121,6 @@ public class PyExtractMethodTest extends LightMarkedTestCase {
    doTest("sum_squares");
  }

-  // PY-2903
-  public void testComment() {
-    doTest("bar");
-  }
-
-  // PY-2903
-  public void testSeveralCommentsAbove() {
-    doTest("bar");
-  }
-
  public void testFile() {
    doTest("bar");
  }