diff --git a/python/psi-api/src/com/jetbrains/python/psi/PyArgumentList.java b/python/psi-api/src/com/jetbrains/python/psi/PyArgumentList.java index 5ba21bf90556..f89f25fdf2d2 100644 --- a/python/psi-api/src/com/jetbrains/python/psi/PyArgumentList.java +++ b/python/psi-api/src/com/jetbrains/python/psi/PyArgumentList.java @@ -20,6 +20,8 @@ import com.jetbrains.python.psi.resolve.PyResolveContext; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.Collection; + /** * Represents an argument list of a function call. * @@ -27,6 +29,12 @@ import org.jetbrains.annotations.Nullable; */ public interface PyArgumentList extends PyElement { + /** + * @return all argument list param expressions (keyword argument or nameless) + */ + @NotNull + Collection getArgumentExpressions(); + @NotNull PyExpression[] getArguments(); diff --git a/python/src/com/jetbrains/python/psi/impl/PyArgumentListImpl.java b/python/src/com/jetbrains/python/psi/impl/PyArgumentListImpl.java index dfe12ca6dc30..2e6120a29303 100644 --- a/python/src/com/jetbrains/python/psi/impl/PyArgumentListImpl.java +++ b/python/src/com/jetbrains/python/psi/impl/PyArgumentListImpl.java @@ -48,6 +48,23 @@ public class PyArgumentListImpl extends PyElementImpl implements PyArgumentList pyVisitor.visitPyArgumentList(this); } + @Override + @NotNull + public Collection getArgumentExpressions() { + final PyExpression[] arguments = getArguments(); + final Collection result = new ArrayList(arguments.length); + for (final PyExpression expression : arguments) { + if (expression instanceof PyKeywordArgument) { + final PyExpression valueExpression = ((PyKeywordArgument)expression).getValueExpression(); + result.add(valueExpression); + } + if (expression instanceof PyReferenceExpression) { + result.add(expression); + } + } + return result; + } + @NotNull public PyExpression[] getArguments() { return childrenToPsi(PythonDialectsTokenSetProvider.INSTANCE.getExpressionTokens(), PyExpression.EMPTY_ARRAY); diff --git a/python/src/com/jetbrains/python/refactoring/classes/DependencyVisitor.java b/python/src/com/jetbrains/python/refactoring/classes/DependencyVisitor.java index b22449d9abb0..6cb4504bb960 100644 --- a/python/src/com/jetbrains/python/refactoring/classes/DependencyVisitor.java +++ b/python/src/com/jetbrains/python/refactoring/classes/DependencyVisitor.java @@ -38,9 +38,24 @@ class DependencyVisitor extends PyRecursiveElementVisitor { } final String calleeName = callee.getName(); - if ((calleeName != null) && calleeName.equals(myElementToFind.getName())) { // Check by name also + final String name = myElementToFind.getName(); + if ((calleeName != null) && calleeName.equals(name)) { // Check by name also myDependencyFound = true; } + + // Member could be used as method param + final PyArgumentList list = node.getArgumentList(); + if (list != null) { + for (final PyExpression expression : node.getArgumentList().getArgumentExpressions()) { + final PsiReference reference = expression.getReference(); + if ((reference != null) && reference.isReferenceTo(myElementToFind)) { + myDependencyFound = true; + } + if ((name != null) && name.equals(expression.getName())) { + myDependencyFound = true; + } + } + } } } diff --git a/python/src/com/jetbrains/python/refactoring/classes/PyClassRefactoringUtil.java b/python/src/com/jetbrains/python/refactoring/classes/PyClassRefactoringUtil.java index 3dfc71bc87f7..25b8a23703cf 100644 --- a/python/src/com/jetbrains/python/refactoring/classes/PyClassRefactoringUtil.java +++ b/python/src/com/jetbrains/python/refactoring/classes/PyClassRefactoringUtil.java @@ -125,7 +125,7 @@ public final class PyClassRefactoringUtil { } @NotNull - public static List copyMethods(Collection methods, PyClass superClass) { + public static List copyMethods(Collection methods, PyClass superClass, boolean skipIfExist ) { if (methods.isEmpty()) { return Collections.emptyList(); } @@ -133,7 +133,7 @@ public final class PyClassRefactoringUtil { rememberNamedReferences(e); } final PyFunction[] elements = methods.toArray(new PyFunction[methods.size()]); - return addMethods(superClass, true, elements); + return addMethods(superClass, skipIfExist, elements); } /** 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 3d6909005f12..281ad586bdd7 100644 --- a/python/src/com/jetbrains/python/refactoring/classes/membersManager/ClassFieldsManager.java +++ b/python/src/com/jetbrains/python/refactoring/classes/membersManager/ClassFieldsManager.java @@ -31,6 +31,19 @@ class ClassFieldsManager extends FieldsManager { protected Collection moveAssignments(@NotNull final PyClass from, @NotNull final Collection statements, @NotNull final PyClass... to) { + return moveAssignmentsImpl(from, statements, to); + } + + /** + * Moves assignments from one class to anothers + * @param from source + * @param statements assignments + * @param to destination + * @return newly created assignments + */ + static Collection moveAssignmentsImpl(@NotNull final PyClass from, + @NotNull final Collection statements, + @NotNull final PyClass... to) { //TODO: Copy/paste with InstanceFieldsManager. Move to parent? final Collection result = new ArrayList(); for (final PyClass destClass : to) { 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 31180c434e3a..b2d8dac2c4a8 100644 --- a/python/src/com/jetbrains/python/refactoring/classes/membersManager/MembersManager.java +++ b/python/src/com/jetbrains/python/refactoring/classes/membersManager/MembersManager.java @@ -20,7 +20,6 @@ import com.google.common.base.Preconditions; import com.google.common.base.Predicate; import com.google.common.collect.Collections2; import com.intellij.psi.PsiElement; -import com.intellij.psi.PsiNamedElement; import com.intellij.util.ArrayUtil; import com.intellij.util.containers.MultiMap; import com.jetbrains.NotNullPredicate; @@ -45,7 +44,11 @@ public abstract class MembersManager implements Function> MANAGERS = - Arrays.asList(new MethodsManager(), new SuperClassesManager(), new ClassFieldsManager(), new InstanceFieldsManager()); + Arrays.asList(new MethodsManager(), + new SuperClassesManager(), + new ClassFieldsManager(), + new InstanceFieldsManager(), + new PropertiesManager()); @NotNull private final Class myExpectedClass; @@ -175,7 +178,8 @@ public abstract class MembersManager implements Function implements Function { final Collection methodsToAbstract = fetchElements(Collections2.filter(members, new AbstractFilter(true))); makeMethodsAbstract(methodsToAbstract, to); - return moveMethods(from, methodsToMove, to); + return moveMethods(from, methodsToMove, true, to); } /** @@ -158,9 +158,10 @@ class MethodsManager extends MembersManager { * @param from source * @param methodsToMove what to move * @param to where + * @param skipIfExist skip (do not add) if method already exists * @return newly added methods */ - private static List moveMethods(final PyClass from, final Collection methodsToMove, final PyClass... to) { + static List moveMethods(final PyClass from, final Collection methodsToMove, final boolean skipIfExist, final PyClass... to) { final List result = new ArrayList(); for (final PyClass destClass : to) { //We move copies here because there may be several destinations @@ -170,7 +171,7 @@ class MethodsManager extends MembersManager { copies.add(newMethod); } - result.addAll(PyClassRefactoringUtil.copyMethods(copies, destClass)); + result.addAll(PyClassRefactoringUtil.copyMethods(copies, destClass, skipIfExist)); } deleteElements(methodsToMove); diff --git a/python/src/com/jetbrains/python/refactoring/classes/membersManager/PropertiesManager.java b/python/src/com/jetbrains/python/refactoring/classes/membersManager/PropertiesManager.java new file mode 100644 index 000000000000..6cf7c0b0cd1b --- /dev/null +++ b/python/src/com/jetbrains/python/refactoring/classes/membersManager/PropertiesManager.java @@ -0,0 +1,162 @@ +package com.jetbrains.python.refactoring.classes.membersManager; + +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.containers.MultiMap; +import com.jetbrains.python.psi.*; +import org.jetbrains.annotations.NotNull; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; + +/** + * Plugin that moves class properties. + * It represents property (whatever old or new) as one of its methods. + * + * @author Ilya.Kazakevich + */ +class PropertiesManager extends MembersManager { + + PropertiesManager() { + super(PyElement.class); + } + + + @NotNull + @Override + protected List getMembersCouldBeMoved(@NotNull final PyClass pyClass) { + final List elements = new ArrayList(pyClass.getProperties().size()); + for (final Property property : pyClass.getProperties().values()) { + elements.add(getElement(property)); + } + return elements; + } + + @NotNull + private static PyElement getElement(@NotNull final Property property) { + final Callable getter = property.getGetter().valueOrNull(); + final Callable setter = property.getSetter().valueOrNull(); + final Callable deleter = property.getDeleter().valueOrNull(); + + if (getter != null) { + return getter; + } + else if (setter != null) { + return setter; + } + else if (deleter != null) { + return deleter; + } + else { + final PyTargetExpression site = property.getDefinitionSite(); + assert site != null : "Property has no methods nor declaration. That is not property"; + return site; + } + } + + @NotNull + private static Property getProperty(@NotNull final PyClass pyClass, @NotNull final PyElement element) { + final Collection properties = pyClass.getProperties().values(); + if (element instanceof PyTargetExpression) { + return getPropertyByTargetExpression(properties, (PyTargetExpression)element); + } + if (element instanceof PyFunction) { + return getPropertyByFunction(properties, (PyFunction)element); + } + throw new IllegalArgumentException("Not function nor target"); + } + + @NotNull + private static Property getPropertyByFunction(@NotNull final Collection properties, + @NotNull final PyFunction functionToSearch) { + for (final Property property : properties) { + for (final PyFunction function : getAllFunctions(property)) { + if (function.equals(functionToSearch)) { + return property; + } + } + } + throw new IllegalArgumentException("No property found"); + } + + @NotNull + private static Property getPropertyByTargetExpression(@NotNull final Iterable properties, + @NotNull final PyTargetExpression element) { + for (final Property property : properties) { + if (element.equals(property.getDefinitionSite())) { + return property; + } + } + throw new IllegalArgumentException("No property found"); + } + + @NotNull + private static Collection getAllFunctions(@NotNull final Property property) { + final Collection result = new ArrayList(3); + final Callable getter = property.getGetter().valueOrNull(); + final Callable setter = property.getSetter().valueOrNull(); + final Callable deleter = property.getDeleter().valueOrNull(); + + if (getter instanceof PyFunction) { + result.add((PyFunction)getter); + } + if (setter instanceof PyFunction) { + result.add((PyFunction)setter); + } + if (deleter instanceof PyFunction) { + result.add((PyFunction)deleter); + } + return result; + } + + @Override + protected Collection moveMembers(@NotNull final PyClass from, + @NotNull final Collection> members, + @NotNull final PyClass... to) { + final Collection result = new ArrayList(); + + final Collection elements = fetchElements(members); + for (final PyElement element : elements) { + final Property property = getProperty(from, element); + final Collection functions = getAllFunctions(property); + MethodsManager.moveMethods(from, functions, false, to); + final PyTargetExpression definitionSite = property.getDefinitionSite(); + if (definitionSite != null) { + final PyAssignmentStatement assignmentStatement = PsiTreeUtil.getParentOfType(definitionSite, PyAssignmentStatement.class); + ClassFieldsManager.moveAssignmentsImpl(from, Collections.singleton(assignmentStatement), to); + } + } + return result; + } + + @NotNull + @Override + public PyMemberInfo apply(@NotNull final PyElement input) { + return new PyMemberInfo(input, false, getName(input), false, this, false); + } + + private static String getName(@NotNull final PyElement input) { + final PyClass clazz = PsiTreeUtil.getParentOfType(input, PyClass.class); + assert clazz != null : "Element not declared in class"; + final Property property = getProperty(clazz, input); + return property.getName(); + } + + @Override + public boolean hasConflict(@NotNull final PyElement member, @NotNull final PyClass aClass) { + return false; + } + + @NotNull + @Override + protected MultiMap getDependencies(@NotNull final PyElement member) { + return new MultiMap(); + } + + @NotNull + @Override + protected Collection getDependencies(@NotNull final MultiMap usedElements) { + return Collections.emptyList(); + } +} diff --git a/python/testData/refactoring/extractsuperclass/properties.after.py b/python/testData/refactoring/extractsuperclass/properties.after.py new file mode 100644 index 000000000000..e512d94e920d --- /dev/null +++ b/python/testData/refactoring/extractsuperclass/properties.after.py @@ -0,0 +1,27 @@ +class ToClass(object): + C = 12 + + def __init__(self): + self.a = 1 + + def _get(self): + return 1 + + def _set(self, value): + pass + + def _delete(self): + pass + + old_property = property(_get, _set, _delete) + + def foo(self): + pass + + +class FromClass(ToClass): + def __init__(self): pass + + + def lala(self): + pass \ No newline at end of file diff --git a/python/testData/refactoring/extractsuperclass/properties.before.py b/python/testData/refactoring/extractsuperclass/properties.before.py new file mode 100644 index 000000000000..3df31fe9c31f --- /dev/null +++ b/python/testData/refactoring/extractsuperclass/properties.before.py @@ -0,0 +1,22 @@ +class FromClass(object): + C = 12 + + def __init__(self): + self.a = 1 + + def _get(self): + return 1 + + def _set(self, value): + pass + + def _delete(self): + pass + + old_property = property(_get, _set, _delete) + + def foo(self): + pass + + def lala(self): + pass \ No newline at end of file diff --git a/python/testData/refactoring/pullup/properties/Class.after.py b/python/testData/refactoring/pullup/properties/Class.after.py new file mode 100644 index 000000000000..1291238ed082 --- /dev/null +++ b/python/testData/refactoring/pullup/properties/Class.after.py @@ -0,0 +1,19 @@ +from SuperClass import SuperClass + + +class AnyClass(SuperClass): + C = 1 + + def __init__(self): + super(AnyClass, self).__init__() + + + + + + + + + def foo(self): + pass + diff --git a/python/testData/refactoring/pullup/properties/Class.py b/python/testData/refactoring/pullup/properties/Class.py new file mode 100644 index 000000000000..5a091a9bd934 --- /dev/null +++ b/python/testData/refactoring/pullup/properties/Class.py @@ -0,0 +1,25 @@ +from SuperClass import SuperClass + + +class AnyClass(SuperClass): + C = 1 + + def __init__(self): + super(AnyClass, self).__init__() + + + @property + def new_property(self): + return 1 + + @new_property.setter + def new_property(self, value): + pass + + @new_property.deleter + def new_property(self): + pass + + def foo(self): + pass + diff --git a/python/testData/refactoring/pullup/properties/SuperClass.after.py b/python/testData/refactoring/pullup/properties/SuperClass.after.py new file mode 100644 index 000000000000..1b2be8de00a9 --- /dev/null +++ b/python/testData/refactoring/pullup/properties/SuperClass.after.py @@ -0,0 +1,15 @@ +class SuperClass(object): + def __init__(self): + pass + + @property + def new_property(self): + return 1 + + @new_property.setter + def new_property(self, value): + pass + + @new_property.deleter + def new_property(self): + pass diff --git a/python/testData/refactoring/pullup/properties/SuperClass.py b/python/testData/refactoring/pullup/properties/SuperClass.py new file mode 100644 index 000000000000..a41b8fdc7db1 --- /dev/null +++ b/python/testData/refactoring/pullup/properties/SuperClass.py @@ -0,0 +1,3 @@ +class SuperClass(object): + def __init__(self): + pass diff --git a/python/testSrc/com/jetbrains/python/refactoring/classes/extractSuperclass/PyExtractSuperclassTest.java b/python/testSrc/com/jetbrains/python/refactoring/classes/extractSuperclass/PyExtractSuperclassTest.java index 13d5b4d4f253..c96bdc2397e1 100644 --- a/python/testSrc/com/jetbrains/python/refactoring/classes/extractSuperclass/PyExtractSuperclassTest.java +++ b/python/testSrc/com/jetbrains/python/refactoring/classes/extractSuperclass/PyExtractSuperclassTest.java @@ -125,6 +125,11 @@ public class PyExtractSuperclassTest extends PyClassRefactoringTest { doSimpleTest("FromClass", "ToClass", null, true, "#instance_field", "#CLASS_FIELD"); } + + public void testProperties() throws Exception { + doSimpleTest("FromClass", "ToClass", null, true, "#C", "#a", "._get", ".foo"); + } + private void doSimpleTest(final String className, final String superclassName, final String expectedError, diff --git a/python/testSrc/com/jetbrains/python/refactoring/classes/pullUp/PyPullUpPresenterTest.java b/python/testSrc/com/jetbrains/python/refactoring/classes/pullUp/PyPullUpPresenterTest.java index ee5371cf24b4..1ee20d27f62a 100644 --- a/python/testSrc/com/jetbrains/python/refactoring/classes/pullUp/PyPullUpPresenterTest.java +++ b/python/testSrc/com/jetbrains/python/refactoring/classes/pullUp/PyPullUpPresenterTest.java @@ -144,7 +144,9 @@ public class PyPullUpPresenterTest extends PyRefactoringPresenterTestCase