PY-3450 Refactoring: pull up class variables/attributes (almost done: need to check dependencies, resolve conflicts and use same functionality for "extract superclass" and "pushdown")

platform/lang-impl/src/com/intellij/codeInsight/template/TemplateBuilderImpl.java

@@ -279,7 +279,7 @@ public class TemplateBuilderImpl implements TemplateBuilder {
   public void run() {
     final Project project = myFile.getProject();
     VirtualFile file = myFile.getVirtualFile();
-    assert file != null;
+    assert file != null: "Virtual file is null for " + file;
     OpenFileDescriptor descriptor = new OpenFileDescriptor(project, file);
     final Editor editor = FileEditorManager.getInstance(project).openTextEditor(descriptor, true);
 

python/psi-api/src/com/jetbrains/python/psi/PyClass.java

@@ -155,6 +155,7 @@ public interface PyClass extends PsiNameIdentifierOwner, PyStatement, NameDefine
 
   List<PyTargetExpression> getClassAttributes();
 
+  @Nullable
   PyTargetExpression findClassAttribute(@NotNull String name, boolean inherited);
 
   List<PyTargetExpression> getInstanceAttributes();

python/src/com/jetbrains/python/inspections/quickfix/AddMethodQuickFix.java

@@ -38,6 +38,7 @@ import org.jetbrains.annotations.NotNull;
 import static com.jetbrains.python.psi.PyUtil.sure;
 
 /**
+ * TODO: Refactor and move to {@link com.jetbrains.python.refactoring.classes.PyClassRefactoringUtil#createMethod(String, com.jetbrains.python.psi.PyClass, com.jetbrains.python.psi.PyFunction.Modifier, String...)}
  * Adds a method foo to class X if X.foo() is unresolved.
  * User: dcheryasov
  * Date: Apr 5, 2009 6:51:26 PM

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

@@ -32,6 +32,7 @@ import com.jetbrains.python.codeInsight.PyCodeInsightSettings;
 import com.jetbrains.python.codeInsight.imports.AddImportHelper;
 import com.jetbrains.python.psi.*;
 import com.jetbrains.python.psi.impl.PyBuiltinCache;
+import com.jetbrains.python.psi.impl.PyFunctionBuilder;
 import com.jetbrains.python.psi.impl.PyImportedModule;
 import com.jetbrains.python.psi.impl.PyPsiUtils;
 import com.jetbrains.python.psi.resolve.QualifiedNameFinder;
@@ -40,6 +41,9 @@ import org.jetbrains.annotations.Nullable;
 
 import java.util.*;
 
+import static com.jetbrains.python.psi.PyFunction.Modifier.CLASSMETHOD;
+import static com.jetbrains.python.psi.PyFunction.Modifier.STATICMETHOD;
+
 /**
  * @author Dennis.Ushakov
  */
@@ -437,4 +441,41 @@ public class PyClassRefactoringUtil {
     }
     return null;
   }
+
+  /**
+   * Creates class method
+   * @param methodName name if new method (be sure to check {@link com.jetbrains.python.PyNames} for special methods)
+   * @param pyClass class to add method
+   * @param modifier if method static or class or simple instance method (null)>
+   * @param parameterNames method parameters
+   * @return newly created method
+   */
+  @NotNull
+  public static PyFunction createMethod(@NotNull final String methodName,
+                                        @NotNull final PyClass pyClass,
+                                        @Nullable final PyFunction.Modifier modifier,
+                                        @NotNull final String... parameterNames) {
+    final PyFunctionBuilder builder = new PyFunctionBuilder(methodName);
+
+
+    //TODO: Take names from codestyle?
+    if (modifier == null) {
+      builder.parameter(PyNames.CANONICAL_SELF);
+    }
+    else if (modifier == CLASSMETHOD) {
+      builder.parameter(PyNames.CANONICAL_CLS);
+      builder.decorate(PyNames.CLASSMETHOD);
+    }
+    else if (modifier == STATICMETHOD) {
+      builder.decorate(PyNames.STATICMETHOD);
+    }
+
+    for (final String parameterName : parameterNames) {
+      builder.parameter(parameterName);
+    }
+
+    final PyFunction function = builder.addFunction(pyClass.getStatementList(), LanguageLevel.getDefault());
+    addMethods(pyClass, new PyElement[]{function}, true);
+    return function;
+  }
 }

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

@@ -1,57 +1,40 @@
 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<PyTargetExpression> {
+class ClassFieldsManager extends FieldsManager {
 
   ClassFieldsManager() {
-    super(PyTargetExpression.class);
+    super(true);
   }
 
-  @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<PyTargetExpression> members) {
+  protected void moveMembers(@NotNull final PyClass from,
+                             @NotNull final PyClass to,
+                             @NotNull final Collection<PyTargetExpression> members) {
     PyClassRefactoringUtil.moveFieldDeclarationToStatement(members, to.getStatementList());
   }
 
-  @NotNull
   @Override
-  public PyMemberInfo apply(@NotNull final PyElement input) {
-    return new PyMemberInfo(input, true, input.getText(), false, this); //TODO: Check overrides
+  protected boolean classHasField(@NotNull final PyClass pyClass, @NotNull final String fieldName) {
+    return pyClass.findClassAttribute(fieldName, true) != null;
   }
 
-  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());
-    }
+  @NotNull
+  @Override
+  protected List<PyTargetExpression> getFieldsByClass(@NotNull PyClass pyClass) {
+    return pyClass.getClassAttributes();
   }
 }

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

@@ -0,0 +1,96 @@
+package com.jetbrains.python.refactoring.classes.membersManager;
+
+import com.google.common.base.Predicate;
+import com.google.common.collect.Collections2;
+import com.google.common.collect.Lists;
+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.Collection;
+import java.util.List;
+
+/**
+ * Parent of all field-based plugins (like class fields, instance fields and so on)
+ * @author Ilya.Kazakevich
+ */
+abstract class FieldsManager extends MembersManager<PyTargetExpression> {
+  private static final SimpleAssignmentsOnly SIMPLE_ASSIGNMENTS_ONLY = new SimpleAssignmentsOnly();
+  private final boolean myStatic;
+
+  /**
+   * @param isStatic is field static or not?
+   */
+  protected FieldsManager(final boolean isStatic) {
+    super(PyTargetExpression.class);
+    myStatic = isStatic;
+  }
+
+  @NotNull
+  @Override
+  protected List<PyElement> getMembersCouldBeMoved(@NotNull final PyClass pyClass) {
+    return Lists.<PyElement>newArrayList(Collections2.filter(getFieldsByClass(pyClass), SIMPLE_ASSIGNMENTS_ONLY));
+  }
+
+  /**
+   * Checks if class has fields. Only child may know how to obtain field
+   * @param pyClass class to check
+   * @param fieldName field name
+   * @return true if has one
+   */
+  protected abstract boolean classHasField(@NotNull PyClass pyClass, @NotNull String fieldName);
+
+  /**
+   * Returns all fields by class. Only child may know how to obtain fields
+   * @param pyClass class to check
+   * @return list of fields in target expression (declaration) form
+   */
+  @NotNull
+  protected abstract List<PyTargetExpression> getFieldsByClass(@NotNull PyClass pyClass);
+
+
+  @NotNull
+  @Override
+  public PyMemberInfo apply(@NotNull final PyElement input) {
+    return new PyMemberInfo(input, myStatic, input.getText(), isOverrides((PyTargetExpression)input), this);
+  }
+
+  @Nullable
+  private Boolean isOverrides(@NotNull final PyTargetExpression input) {
+    final PyClass aClass = input.getContainingClass();
+    final String name = input.getName();
+    if (name == null) {
+      return null; //Field with out of name can't override something
+    }
+
+    assert aClass != null : "Target expression declared outside of class:" + input;
+
+    return classHasField(aClass, name) ? true : null;
+  }
+
+  @Override
+  protected void moveMembers(@NotNull final PyClass from,
+                             @NotNull final PyClass to,
+                             @NotNull final Collection<PyTargetExpression> members) {
+    PyClassRefactoringUtil.moveFieldDeclarationToStatement(members, to.getStatementList());
+  }
+
+
+  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/InstanceFieldsManager.java

@@ -0,0 +1,46 @@
+package com.jetbrains.python.refactoring.classes.membersManager;
+
+import com.jetbrains.python.PyNames;
+import com.jetbrains.python.psi.*;
+import com.jetbrains.python.refactoring.classes.PyClassRefactoringUtil;
+import org.jetbrains.annotations.NotNull;
+
+import java.util.Collection;
+import java.util.List;
+
+/**
+ * @author Ilya.Kazakevich
+ */
+class InstanceFieldsManager extends FieldsManager {
+  InstanceFieldsManager() {
+    super(false);
+  }
+
+
+  @Override
+  protected void moveMembers(@NotNull final PyClass from,
+                             @NotNull final PyClass to,
+                             @NotNull final Collection<PyTargetExpression> members) {
+    //We need __init__ method, and if there is no any -- we need to create it
+    PyFunction initMethod = to.findMethodByName(PyNames.INIT, false);
+    if (initMethod == null) {
+      initMethod = PyClassRefactoringUtil.createMethod(PyNames.INIT, to, null);
+    }
+    final PyStatementList statementList = initMethod.getStatementList();
+    if (statementList == null) {
+      return; //TODO: Investigate how could it be
+    }
+    PyClassRefactoringUtil.moveFieldDeclarationToStatement(members, statementList);
+  }
+
+  @Override
+  protected boolean classHasField(@NotNull final PyClass pyClass, @NotNull final String fieldName) {
+    return pyClass.findInstanceAttribute(fieldName, true) != null;
+  }
+
+  @NotNull
+  @Override
+  protected List<PyTargetExpression> getFieldsByClass(@NotNull final PyClass pyClass) {
+    return pyClass.getInstanceAttributes();
+  }
+}

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

@@ -31,7 +31,7 @@ public abstract class MembersManager<T extends PyElement> implements Function<Py
    * List of managers. Class delegates all logic to them.
    */
   private static final Collection<? extends MembersManager<?>> MANAGERS =
-    Arrays.asList(new MethodsManager(), new SuperClassesManager(), new ClassFieldsManager());
+    Arrays.asList(new MethodsManager(), new SuperClassesManager(), new ClassFieldsManager(), new InstanceFieldsManager());
   private static final PyMemberExtractor PY_MEMBER_EXTRACTOR = new PyMemberExtractor();
 
   @NotNull
@@ -121,7 +121,8 @@ public abstract class MembersManager<T extends PyElement> implements Function<Py
   /**
    * Filters out named elements (ones that subclasses {@link com.intellij.psi.PsiNamedElement}) and {@link com.jetbrains.python.psi.PyElement})
    * that are null or has null name.
-   * You need it sometimes when code has errors (i.e. bad formatted code with annotation may treat annotation as method with null name)
+   * You need it sometimes when code has errors (i.e. bad formatted code with annotation may treat annotation as method with null name.
+   * note: we should probably throw exceptions in such cases and display "refactoring not available" window in handler)
    *
    * @param elementsToFilter collection of elements to filter
    * @param <T>              element type
@@ -141,7 +142,12 @@ public abstract class MembersManager<T extends PyElement> implements Function<Py
    */
   protected abstract void moveMembers(@NotNull PyClass from, @NotNull PyClass to, @NotNull Collection<T> members);
 
-  //TODO: Doc
+  /**
+   * Creates {@link com.jetbrains.python.refactoring.classes.membersManager.PyMemberInfo} from {@link com.jetbrains.python.psi.PyElement}
+   * This process is plugin-specific and should be implemented in each plugin
+   * @param input element
+   * @return member info
+   */
   @SuppressWarnings("NullableProblems") //IDEA-120100
   @NotNull
   @Override

python/src/com/jetbrains/python/refactoring/classes/pullUp/PyPullUpConflictsUtil.java

@@ -38,6 +38,7 @@ public class PyPullUpConflictsUtil {
     for (PyMemberInfo info : infos) {
       PsiElement member = info.getMember();
       boolean isConflict = false;
+      //TODO: Delegate to MemeberManagers here
       if (member instanceof PyFunction) {
         final String name = ((PyFunction)member).getName();
         if (name == null) continue;

python/src/com/jetbrains/python/refactoring/classes/pullUp/PyPullUpPresenterImpl.java

@@ -31,6 +31,7 @@ import org.jetbrains.annotations.NotNull;
 
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 
 /**
  * Pull-up presenter implementation
@@ -87,12 +88,13 @@ class PyPullUpPresenterImpl extends AbstractUsesDependencyMemberInfoModel<PyElem
   public boolean isMemberEnabled(PyMemberInfo member) {
     PyClass currentSuperClass = myView.getSelectedParent();
     if (member.getMember() instanceof PyClass) {
+      //TODO: Delegate to Memebers Managers
       PyClass memberClass = (PyClass)member.getMember();
       if (memberClass.isSubclass(currentSuperClass) || currentSuperClass.isSubclass(memberClass)) {
         return false; //Class is already parent of superclass
       }
     }
-    if (! PyPullUpConflictsUtil.checkConflicts(Arrays.asList(member), myView.getSelectedParent()).isEmpty()) {
+    if (! PyPullUpConflictsUtil.checkConflicts(Collections.singletonList(member), myView.getSelectedParent()).isEmpty()) {
       return false; //Member has conflict
     }
     return (!myStorage.getDuplicatedMemberInfos(currentSuperClass).contains(member)) && member.getMember() != currentSuperClass;

python/src/com/jetbrains/python/refactoring/extractmethod/PyExtractMethodUtil.java

@@ -59,6 +59,7 @@ import org.jetbrains.annotations.Nullable;
 import java.util.*;
 
 /**
+ * * TODO: Merge with {@link com.jetbrains.python.refactoring.classes.PyClassRefactoringUtil#createMethod(String, com.jetbrains.python.psi.PyClass, com.jetbrains.python.psi.PyFunction.Modifier, String...)}
  * @author oleg
  */
 public class PyExtractMethodUtil {

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

@@ -0,0 +1,9 @@
+class Parent:
+      def __init__(self):
+          self.instance_field = "eggs"
+
+
+class Child(Parent):
+    def __init__(self):
+        Parent2.__init__(self)
+

python/testData/refactoring/pullup/moveInstanceAttributesNoInit.py

@@ -0,0 +1,9 @@
+class Parent:
+      pass
+
+
+class Child(Parent):
+    def __init__(self):
+        Parent2.__init__(self)
+        self.instance_field = "eggs"
+

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

@@ -0,0 +1,9 @@
+class Parent:
+    def __init__(self):
+        self.instance_field = "eggs"
+
+
+class Child(Parent):
+    def __init__(self):
+        Parent2.__init__(self)
+

python/testData/refactoring/pullup/moveInstanceAttributesSimple.py

@@ -0,0 +1,10 @@
+class Parent:
+    def __init__(self):
+        pass
+
+
+class Child(Parent):
+    def __init__(self):
+        Parent2.__init__(self)
+        self.instance_field = "eggs"
+

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

@@ -29,7 +29,11 @@ class NoMembers(object):
 class BadMro(MainParent, object, SubParent1, SubParent2):
     pass
 
-class SomeMembersDisabled(SubParent1, date): #SubParent1 is disabled
+class HugeChild(SubParent1, date): #SubParent1 is disabled
+    def __init__(self):
+        self.instance_field_1 = 42
+        self.instance_field_2 = 100500
+
     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

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

@@ -20,6 +20,7 @@ import com.jetbrains.python.fixtures.PyTestCase;
 import com.jetbrains.python.psi.PyClass;
 import com.jetbrains.python.psi.PyElement;
 import com.jetbrains.python.psi.PyFunction;
+import com.jetbrains.python.psi.PyTargetExpression;
 import com.jetbrains.python.psi.stubs.PyClassNameIndex;
 import org.hamcrest.Matchers;
 import org.jetbrains.annotations.NotNull;
@@ -56,7 +57,12 @@ public abstract class PyClassRefactoringTest extends PyTestCase {
   }
 
   private PyElement findField(final String className, final String memberName) {
-    return findClass(className).findClassAttribute(memberName, false);
+    final PyClass aClass = findClass(className);
+    final PyTargetExpression attribute = aClass.findClassAttribute(memberName, false);
+    if (attribute != null) {
+      return attribute;
+    }
+    return aClass.findInstanceAttribute(memberName, false);
   }
 
   private PyFunction findMethod(final String className, final String name) {

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

@@ -57,6 +57,13 @@ public class PyPullUpTest extends PyClassRefactoringTest {
     doHelperTest("Child2", "#CLASS_VAR", "Parent2");
   }
 
+  public void testMoveInstanceAttributesSimple() {
+    doHelperTest("Child", "#instance_field", "Parent");
+  }
+  public void testMoveInstanceAttributesNoInit() {
+    doHelperTest("Child", "#instance_field", "Parent");
+  }
+
   public void testMultiFile() {   // PY-2810
     doMultiFileTest();
   }

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

@@ -86,10 +86,10 @@ public class PyPullUpPresenterTest extends PyTestCase {
   }
 
   /**
-   * Checks that some members are not allowed
+   * Checks that some members are not allowed, while others are
    */
-  public void testDisabledMembers() throws Exception {
-    PyPullUpPresenterImpl sut = configureByClass("SomeMembersDisabled");
+  public void testMembers() throws Exception {
+    PyPullUpPresenterImpl sut = configureByClass("HugeChild");
     EasyMock.expect(myView.getSelectedParent()).andReturn(getClassByName("SubParent1")).anyTimes();
 
     myMocksControl.replay();
@@ -104,11 +104,14 @@ public class PyPullUpPresenterTest extends PyTestCase {
     final Matcher<Iterable<? extends Entry>> matcher = Matchers
       .containsInAnyOrder(new Entry("extends date", true, false),
                           new Entry("CLASS_FIELD", true, true),
+                          new Entry("__init__(self)", true, false),
                           new Entry("extends SubParent1", false, false),
                           new Entry("foo(self)", false, false),
                           new Entry("bar(self)", true, false),
                           new Entry("static_1(cls)", true, true),
                           new Entry("static_2()", true, true),
+                          new Entry("self.instance_field_1", true, false),
+                          new Entry("self.instance_field_2", true, false),
                           new Entry("bad_method()", true, false)
       );
     Assert.assertThat("Wrong members or their states", memberNamesAndStatus, matcher);