From 955663ad6c6d6ece7c19f30bfc34db354826f4ba Mon Sep 17 00:00:00 2001 From: "Ilya.Kazakevich" Date: Tue, 4 Feb 2014 00:47:26 +0400 Subject: [PATCH] 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") --- .../template/TemplateBuilderImpl.java | 2 +- .../src/com/jetbrains/python/psi/PyClass.java | 1 + .../quickfix/AddMethodQuickFix.java | 1 + .../classes/PyClassRefactoringUtil.java | 41 ++++++++ .../membersManager/ClassFieldsManager.java | 41 +++----- .../classes/membersManager/FieldsManager.java | 96 +++++++++++++++++++ .../membersManager/InstanceFieldsManager.java | 46 +++++++++ .../membersManager/MembersManager.java | 12 ++- .../classes/pullUp/PyPullUpConflictsUtil.java | 1 + .../classes/pullUp/PyPullUpPresenterImpl.java | 4 +- .../extractmethod/PyExtractMethodUtil.java | 1 + .../moveInstanceAttributesNoInit.after.py | 9 ++ .../pullup/moveInstanceAttributesNoInit.py | 9 ++ .../moveInstanceAttributesSimple.after.py | 9 ++ .../pullup/moveInstanceAttributesSimple.py | 10 ++ .../refactoring/pullup/presenter/file.py | 6 +- .../classes/PyClassRefactoringTest.java | 8 +- .../refactoring/classes/PyPullUpTest.java | 7 ++ .../classes/pullUp/PyPullUpPresenterTest.java | 9 +- 19 files changed, 274 insertions(+), 39 deletions(-) create mode 100644 python/src/com/jetbrains/python/refactoring/classes/membersManager/FieldsManager.java create mode 100644 python/src/com/jetbrains/python/refactoring/classes/membersManager/InstanceFieldsManager.java create mode 100644 python/testData/refactoring/pullup/moveInstanceAttributesNoInit.after.py create mode 100644 python/testData/refactoring/pullup/moveInstanceAttributesNoInit.py create mode 100644 python/testData/refactoring/pullup/moveInstanceAttributesSimple.after.py create mode 100644 python/testData/refactoring/pullup/moveInstanceAttributesSimple.py diff --git a/platform/lang-impl/src/com/intellij/codeInsight/template/TemplateBuilderImpl.java b/platform/lang-impl/src/com/intellij/codeInsight/template/TemplateBuilderImpl.java index 26d46527065b..9d6426fa02ee 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/template/TemplateBuilderImpl.java +++ b/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); diff --git a/python/psi-api/src/com/jetbrains/python/psi/PyClass.java b/python/psi-api/src/com/jetbrains/python/psi/PyClass.java index 53dd81ab746c..566fe0a2dbba 100644 --- a/python/psi-api/src/com/jetbrains/python/psi/PyClass.java +++ b/python/psi-api/src/com/jetbrains/python/psi/PyClass.java @@ -155,6 +155,7 @@ public interface PyClass extends PsiNameIdentifierOwner, PyStatement, NameDefine List getClassAttributes(); + @Nullable PyTargetExpression findClassAttribute(@NotNull String name, boolean inherited); List getInstanceAttributes(); diff --git a/python/src/com/jetbrains/python/inspections/quickfix/AddMethodQuickFix.java b/python/src/com/jetbrains/python/inspections/quickfix/AddMethodQuickFix.java index 30d99103b887..6d90ec3be366 100644 --- a/python/src/com/jetbrains/python/inspections/quickfix/AddMethodQuickFix.java +++ b/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 diff --git a/python/src/com/jetbrains/python/refactoring/classes/PyClassRefactoringUtil.java b/python/src/com/jetbrains/python/refactoring/classes/PyClassRefactoringUtil.java index 07b7f8d4518f..360e9f8a10b0 100644 --- a/python/src/com/jetbrains/python/refactoring/classes/PyClassRefactoringUtil.java +++ b/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; + } } diff --git a/python/src/com/jetbrains/python/refactoring/classes/membersManager/ClassFieldsManager.java b/python/src/com/jetbrains/python/refactoring/classes/membersManager/ClassFieldsManager.java index f6333d772599..7d02461501d1 100644 --- a/python/src/com/jetbrains/python/refactoring/classes/membersManager/ClassFieldsManager.java +++ b/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 { +class ClassFieldsManager extends FieldsManager { ClassFieldsManager() { - super(PyTargetExpression.class); + super(true); } - @NotNull - @Override - protected List getMembersCouldBeMoved(@NotNull final PyClass pyClass) { - return new ArrayList(Collections2.filter(pyClass.getClassAttributes(), new SimpleAssignmentsOnly())); - } @Override - protected void moveMembers(@NotNull final PyClass from, @NotNull final PyClass to, @NotNull final Collection members) { + protected void moveMembers(@NotNull final PyClass from, + @NotNull final PyClass to, + @NotNull final Collection 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 { - //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 getFieldsByClass(@NotNull PyClass pyClass) { + return pyClass.getClassAttributes(); } } diff --git a/python/src/com/jetbrains/python/refactoring/classes/membersManager/FieldsManager.java b/python/src/com/jetbrains/python/refactoring/classes/membersManager/FieldsManager.java new file mode 100644 index 000000000000..1721f738cbe4 --- /dev/null +++ b/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 { + 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 getMembersCouldBeMoved(@NotNull final PyClass pyClass) { + return Lists.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 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 members) { + PyClassRefactoringUtil.moveFieldDeclarationToStatement(members, to.getStatementList()); + } + + + private static class SimpleAssignmentsOnly implements Predicate { + //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()); + } + } +} diff --git a/python/src/com/jetbrains/python/refactoring/classes/membersManager/InstanceFieldsManager.java b/python/src/com/jetbrains/python/refactoring/classes/membersManager/InstanceFieldsManager.java new file mode 100644 index 000000000000..27ca59f88c1e --- /dev/null +++ b/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 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 getFieldsByClass(@NotNull final PyClass pyClass) { + return pyClass.getInstanceAttributes(); + } +} diff --git a/python/src/com/jetbrains/python/refactoring/classes/membersManager/MembersManager.java b/python/src/com/jetbrains/python/refactoring/classes/membersManager/MembersManager.java index 21cc2ceaf2f5..7bbefd9d2fa2 100644 --- a/python/src/com/jetbrains/python/refactoring/classes/membersManager/MembersManager.java +++ b/python/src/com/jetbrains/python/refactoring/classes/membersManager/MembersManager.java @@ -31,7 +31,7 @@ public abstract class MembersManager implements Function> 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 implements Function element type @@ -141,7 +142,12 @@ public abstract class MembersManager implements Function 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 diff --git a/python/src/com/jetbrains/python/refactoring/classes/pullUp/PyPullUpConflictsUtil.java b/python/src/com/jetbrains/python/refactoring/classes/pullUp/PyPullUpConflictsUtil.java index c1f5eeaa4240..0cfd25fc1748 100644 --- a/python/src/com/jetbrains/python/refactoring/classes/pullUp/PyPullUpConflictsUtil.java +++ b/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; diff --git a/python/src/com/jetbrains/python/refactoring/classes/pullUp/PyPullUpPresenterImpl.java b/python/src/com/jetbrains/python/refactoring/classes/pullUp/PyPullUpPresenterImpl.java index 99295d9f607e..ad8cdd5bde3f 100644 --- a/python/src/com/jetbrains/python/refactoring/classes/pullUp/PyPullUpPresenterImpl.java +++ b/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> 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);