PY-3450 Refactoring: pull up class variables/attributes: Fixed, but refactoring is required.

python/src/com/jetbrains/python/refactoring/classes/PyClassRefactoringUtil.java

@@ -139,6 +139,22 @@ public class PyClassRefactoringUtil {
     return false;
   }
 
+  /**
+   * Moves class field to another class
+   * @param expressions list of class fields
+   * @param superClass where to move them
+   */
+  public static void moveFields(@NotNull final Collection<PyTargetExpression> expressions, @NotNull final PyClass superClass) {
+    for (final PyTargetExpression expression : expressions) {
+      final PyAssignmentStatement expAssignmentStatement = PsiTreeUtil.getParentOfType(expression, PyAssignmentStatement.class);
+      assert expAssignmentStatement != null: "Target expression has no assignment statement";
+      final PyStatementList superClassStatement = superClass.getStatementList();
+      PyUtil.addElementToStatementList(expAssignmentStatement.copy(), superClassStatement, true);
+      expAssignmentStatement.delete();
+      PyPsiUtils.removeRedundantPass(superClassStatement);
+    }
+
+  }
   public static void moveMethods(Collection<PyFunction> methods, PyClass superClass) {
     if (methods.size() == 0) return;
     for (PsiElement e : methods) {

python/src/com/jetbrains/python/refactoring/classes/membersManager/ClassFieldsManager.java

@@ -0,0 +1,53 @@
+package com.jetbrains.python.refactoring.classes.membersManager;
+
+import com.google.common.base.Predicate;
+import com.google.common.collect.Collections2;
+import com.intellij.psi.PsiElement;
+import com.jetbrains.python.psi.PyAssignmentStatement;
+import com.jetbrains.python.psi.PyClass;
+import com.jetbrains.python.psi.PyElement;
+import com.jetbrains.python.psi.PyTargetExpression;
+import com.jetbrains.python.refactoring.classes.PyClassRefactoringUtil;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+/**
+ * Moves class attributes up
+ * @author Ilya.Kazakevich
+ */
+class ClassFieldsManager extends MembersManager {
+  @NotNull
+  @Override
+  protected List<PyElement> getMembersCouldBeMoved(@NotNull final PyClass pyClass) {
+    return new ArrayList<PyElement>(Collections2.filter(pyClass.getClassAttributes(), new SimpleAssignmentsOnly()));
+  }
+
+  @Override
+  protected void moveMembers(@NotNull final PyClass from, @NotNull final PyClass to, @NotNull final Collection<PyElement> members) {
+    //TODO: Use generics to prevent casting
+    PyClassRefactoringUtil.moveFields((Collection)members, to);
+  }
+
+  @NotNull
+  @Override
+  public PyMemberInfo apply(@NotNull final PyElement input) {
+    return new PyMemberInfo(input, true, input.getText(), false, this); //TODO: Check overrides
+  }
+
+  private static class SimpleAssignmentsOnly implements Predicate<PyTargetExpression> {
+    //Support only simplest cases like CLASS_VAR = 42.
+    //Tuples (CLASS_VAR_1, CLASS_VAR_2) = "spam", "eggs" are not supported by now
+    @Override
+    public boolean apply(@Nullable final PyTargetExpression input) {
+      if (input == null) {
+        return false; //Filter out empties (which probably would never be here)
+      }
+      final PsiElement parent = input.getParent();
+      return (parent != null) && PyAssignmentStatement.class.isAssignableFrom(parent.getClass());
+    }
+  }
+}

python/src/com/jetbrains/python/refactoring/classes/membersManager/MembersManager.java

@@ -26,7 +26,10 @@ public abstract class MembersManager implements Function<PyElement, PyMemberInfo
   /**
    * List of managers. Class delegates all logic to them.
    */
-  private static final Collection<MembersManager> MANAGERS = Arrays.asList(new MethodsManager(), new SuperClassesManager());
+  private static final Collection<MembersManager> MANAGERS = Arrays.asList(
+    new MethodsManager(),
+    new SuperClassesManager(),
+    new ClassFieldsManager());
   private static final PyMemberExtractor PY_MEMBER_EXTRACTOR = new PyMemberExtractor();
 
   protected MembersManager() {

python/testData/refactoring/pullup/moveClassAttributesNoPass.after.py

@@ -0,0 +1,9 @@
+class Parent2:
+    CLASS_VAR = 42
+
+    def doo(self):
+        pass
+
+
+class Child2(Parent2):
+    pass

python/testData/refactoring/pullup/moveClassAttributesNoPass.py

@@ -0,0 +1,7 @@
+class Parent2:
+    def doo(self):
+        pass
+
+
+class Child2(Parent2):
+    CLASS_VAR = 42

python/testData/refactoring/pullup/moveClassAttributesSimple.after.py

@@ -0,0 +1,6 @@
+class Parent:
+    CLASS_VAR = 42
+
+
+class Child(Parent):
+    pass

python/testData/refactoring/pullup/moveClassAttributesSimple.py

@@ -0,0 +1,6 @@
+class Parent:
+    pass
+
+
+class Child(Parent):
+    CLASS_VAR = 42

python/testData/refactoring/pullup/presenter/file.py

@@ -30,6 +30,8 @@ class BadMro(MainParent, object, SubParent1, SubParent2):
     pass
 
 class SomeMembersDisabled(SubParent1, date): #SubParent1 is disabled
+    CLASS_FIELD = 42
+    (CLASS_FIELD_A,CLASS_FIELD_B) = (42,100500) We do not support tuples in class assignments for now (see ClassFieldsManager)
     def foo(self): #should be disabled
         pass
     def bar(self):

python/testSrc/com/jetbrains/python/refactoring/classes/PyClassRefactoringTest.java

@@ -34,17 +34,31 @@ public abstract class PyClassRefactoringTest extends PyTestCase {
   /**
    * @param className  class where member should be found
    * @param memberName member that starts with dot (<code>.</code>) is treated as method.
+   *                   member that starts with dash (<code>#</code>) is treated as attribute.
    *                   It is treated parent class otherwise
    * @return member or null if not found
    */
   @NotNull
   protected PyElement findMember(@NotNull String className, @NotNull String memberName) {
-    boolean findMethod = memberName.contains(".");
-    PyElement result = (findMethod ? findMethod(className, memberName.substring(1)) : findClass(memberName));
+    final PyElement result;
+    //TODO: Get rid of this chain of copy pastes
+    if (memberName.contains(".")) {
+      result = findMethod(className, memberName.substring(1));
+    }
+    else if (memberName.contains("#")) {
+      result = findField(className, memberName.substring(1));
+    }
+    else {
+      result = findClass(memberName);
+    }
     Assert.assertNotNull(String.format("No member %s found in class %s", memberName, className), result);
     return result;
   }
 
+  private PyElement findField(final String className, final String memberName) {
+    return findClass(className).findClassAttribute(memberName, false);
+  }
+
   private PyFunction findMethod(final String className, final String name) {
     final PyClass clazz = findClass(className);
     return clazz.findMethodByName(name, false);

python/testSrc/com/jetbrains/python/refactoring/classes/PyPullUpTest.java

@@ -50,6 +50,13 @@ public class PyPullUpTest extends PyClassRefactoringTest {
     doHelperTest("Child", "Spam", "Parent_1");
   }
 
+  public void testMoveClassAttributesSimple() {
+    doHelperTest("Child", "#CLASS_VAR", "Parent");
+  }
+  public void testMoveClassAttributesNoPass() {
+    doHelperTest("Child2", "#CLASS_VAR", "Parent2");
+  }
+
   public void testMultiFile() {   // PY-2810
     doMultiFileTest();
   }

python/testSrc/com/jetbrains/python/refactoring/classes/pullUp/PyPullUpPresenterTest.java

@@ -102,6 +102,7 @@ public class PyPullUpPresenterTest extends PyTestCase {
     @SuppressWarnings("unchecked") Matcher<Iterable<? extends Pair<String, Boolean>>> matcher = Matchers
       .containsInAnyOrder(
         Pair.create("date", true),
+        Pair.create("CLASS_FIELD", true),
         Pair.create("SubParent1", false),
         Pair.create("foo", false),
         Pair.create("bar", true));