PY-21099: Store dependency information and use it for refactoring, obey target ref. dependency

* Dependency info may be lost when new member is created, so we store it on PSI level
* Support target reference dependency
* Method can't be abstract when property depends on it

GitOrigin-RevId: 24b9b64ef4ce60dd19bc4696e1d7aabf6a260067

python/python-psi-impl/src/com/jetbrains/python/refactoring/classes/DependencyVisitor.java

@@ -29,7 +29,7 @@ import org.jetbrains.annotations.NotNull;
  *
  * @author Ilya.Kazakevich
  */
-class DependencyVisitor extends PyRecursiveElementVisitor {
+final class DependencyVisitor extends PyRecursiveElementVisitor {
 
   @NotNull
   private final PyElement myElementToFind;
@@ -42,6 +42,19 @@ class DependencyVisitor extends PyRecursiveElementVisitor {
     myElementToFind = elementToFind;
   }
 
+  @Override
+  public void visitPyTargetExpression(@NotNull PyTargetExpression node) {
+    var value = node.findAssignedValue();
+    if (value != null) {
+      var reference = value.getReference();
+      if (reference != null && reference.isReferenceTo(myElementToFind)) {
+        myDependencyFound = true;
+        return;
+      }
+    }
+    super.visitPyTargetExpression(node);
+  }
+
   @Override
   public void visitPyCallExpression(@NotNull final PyCallExpression node) {
     final PyExpression callee = node.getCallee();

python/python-psi-impl/src/com/jetbrains/python/refactoring/classes/PyDependenciesComparator.java

@@ -15,6 +15,8 @@
  */
 package com.jetbrains.python.refactoring.classes;
 
+import com.intellij.openapi.util.Key;
+import com.intellij.psi.PsiElement;
 import com.jetbrains.python.psi.PyAssignmentStatement;
 import com.jetbrains.python.psi.PyElement;
 import com.jetbrains.python.psi.PyExpressionStatement;
@@ -23,13 +25,18 @@ import org.jetbrains.annotations.NotNull;
 
 import java.io.Serializable;
 import java.util.Comparator;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.UUID;
+import java.util.function.Supplier;
 
 /**
  * Compares elements by their dependencies.
  * If A depends on B, then A < B
  */
 public final class PyDependenciesComparator implements Comparator<PyElement>, Serializable {
-
+  private final static Key<UUID> UUID_KEY = new Key<>("pyUUIDKey");
+  private final static Key<Set<UUID>> DEPENDENCIES_KEY = new Key<>("pyUUIDDependencies");
   /**
    * Singleton comparator instance
    */
@@ -38,6 +45,46 @@ public final class PyDependenciesComparator implements Comparator<PyElement>, Se
   private PyDependenciesComparator() {
   }
 
+  @NotNull
+  private static <T> T getObject(@NotNull Key<T> key, @NotNull PyElement element, @NotNull Supplier<T> create) {
+    var value = element.getCopyableUserData(key);
+    if (value == null) {
+      value = create.get();
+      element.putCopyableUserData(key, value);
+    }
+    return value;
+  }
+
+  /**
+   * Same as {@link #compare(PyElement, PyElement)} but stores dependency info in {@link PsiElement#getCopyableUserData(Key)}
+   * so dependency information is stored until {@link #clearDependencyInfo(Iterable)} is called
+   */
+  public int compareAndStoreDependency(@NotNull final PyElement o1, @NotNull final PyElement o2) {
+    var o1UUID = getObject(UUID_KEY, o1, () -> UUID.randomUUID());
+    var o2UUID = getObject(UUID_KEY, o2, () -> UUID.randomUUID());
+
+    int result = compare(o1, o2);
+    if (result > 0) {
+      //o1 depends on 02
+      getObject(DEPENDENCIES_KEY, o1, () -> new HashSet<>()).add(o2UUID);
+    }
+    else if (result < 0) {
+      // o2 depends on 01
+      getObject(DEPENDENCIES_KEY, o2, () -> new HashSet<>()).add(o1UUID);
+    }
+    return result;
+  }
+
+  /**
+   * Remove dependency info created by {@link #compareAndStoreDependency(PyElement, PyElement)}
+   */
+  public static void clearDependencyInfo(@NotNull Iterable<PyElement> elements) {
+    for (var element : elements) {
+      element.putCopyableUserData(UUID_KEY, null);
+      element.putCopyableUserData(DEPENDENCIES_KEY, null);
+    }
+  }
+
   @Override
   public int compare(@NotNull final PyElement o1, @NotNull final PyElement o2) {
     if (depends(o1, o2)) {
@@ -64,11 +111,27 @@ public final class PyDependenciesComparator implements Comparator<PyElement>, Se
    * @return true if first param depends on second.
    */
   public static boolean depends(@NotNull final PyElement o1, @NotNull final PyElement o2) {
+    var uuid2 = o2.getCopyableUserData(UUID_KEY);
+    if (uuid2 != null) {
+      var dependencies = o1.getCopyableUserData(DEPENDENCIES_KEY);
+      if (dependencies != null) {
+        return dependencies.contains(uuid2);
+      }
+    }
+
     final DependencyVisitor visitor = new DependencyVisitor(o2);
     o1.accept(visitor);
     return visitor.isDependencyFound();
   }
 
+  /**
+   * Copies dependency info from one element to another if {@link PsiElement#copy()} can't be used
+   */
+  public static void copyDependencyInfo(@NotNull PyElement from, @NotNull PyFunction to) {
+    to.putCopyableUserData(UUID_KEY, from.getCopyableUserData(UUID_KEY));
+    to.putCopyableUserData(DEPENDENCIES_KEY, from.getCopyableUserData(DEPENDENCIES_KEY));
+  }
+
   /**
    * Types of class members in order, they should appear
    */

python/src/com/jetbrains/python/refactoring/classes/extractSuperclass/PyExtractSuperclassInfoModel.java

@@ -18,6 +18,7 @@ package com.jetbrains.python.refactoring.classes.extractSuperclass;
 import com.intellij.refactoring.classMembers.AbstractUsesDependencyMemberInfoModel;
 import com.jetbrains.python.psi.PyClass;
 import com.jetbrains.python.psi.PyElement;
+import com.jetbrains.python.psi.PyTargetExpression;
 import com.jetbrains.python.refactoring.classes.membersManager.PyMemberInfo;
 import org.jetbrains.annotations.NotNull;
 
@@ -31,9 +32,20 @@ class PyExtractSuperclassInfoModel extends AbstractUsesDependencyMemberInfoModel
 
   @Override
   public boolean isAbstractEnabled(final PyMemberInfo<PyElement> member) {
+    if (propertyDependsOnThisMethod(member)) {
+      return false;
+    }
     return member.isCouldBeAbstract() && isMemberEnabled(member);
   }
 
+  /**
+   * If property depends on method, this method can't be made abstract
+   */
+  private boolean propertyDependsOnThisMethod(@NotNull PyMemberInfo<PyElement> member) {
+    var dependencies = myMemberDependencyGraph.getDependenciesOf(member.getMember());
+    return (dependencies != null && dependencies.stream().anyMatch(o -> o instanceof PyTargetExpression));
+  }
+
   @Override
   public int checkForProblems(@NotNull final PyMemberInfo<PyElement> member) {
     return member.isChecked() ? OK : super.checkForProblems(member);

python/src/com/jetbrains/python/refactoring/classes/extractSuperclass/PyExtractSuperclassPresenterImpl.java

@@ -117,8 +117,10 @@ class PyExtractSuperclassPresenterImpl extends MembersBasedPresenterNoPreviewImp
 
   @Override
   protected void refactorNoPreview() {
+    var infos = myView.getSelectedMemberInfos();
+    disableItemsCantBeAbstract(infos);
     PyExtractSuperclassHelper
-      .extractSuperclass(myClassUnderRefactoring, myView.getSelectedMemberInfos(), myView.getSuperClassName(), myView.getModuleFile());
+      .extractSuperclass(myClassUnderRefactoring, infos, myView.getSuperClassName(), myView.getModuleFile());
   }
 
   @NotNull
@@ -126,4 +128,15 @@ class PyExtractSuperclassPresenterImpl extends MembersBasedPresenterNoPreviewImp
   protected Iterable<? extends PyClass> getDestClassesToCheckConflicts() {
     return Collections.emptyList(); // No conflict can take place in newly created classes
   }
+
+  /**
+   * Methods may still be marked abstract because user marked them before they were disabled
+   */
+  private void disableItemsCantBeAbstract(@NotNull Collection<PyMemberInfo<PyElement>> infos) {
+    for (var info : infos) {
+      if (info.isToAbstract() && !myModel.isAbstractEnabled(info)) {
+        info.setToAbstract(false);
+      }
+    }
+  }
 }

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

@@ -20,6 +20,7 @@ import com.google.common.collect.Collections2;
 import com.google.common.collect.Lists;
 import com.intellij.psi.PsiElement;
 import com.intellij.psi.util.PsiTreeUtil;
+import com.intellij.util.ObjectUtils;
 import com.intellij.util.containers.MultiMap;
 import com.jetbrains.NotNullPredicate;
 import com.jetbrains.python.psi.*;
@@ -110,7 +111,8 @@ abstract class FieldsManager extends MembersManager<PyTargetExpression> {
   @NotNull
   @Override
   public PyMemberInfo<PyTargetExpression> apply(@NotNull final PyTargetExpression input) {
-    return new PyMemberInfo<>(input, myStatic, input.getText(), isOverrides(input), this, false);
+    var parent = ObjectUtils.tryCast(input.getParent(), PyAssignmentStatement.class);
+    return new PyMemberInfo<>(input, parent != null ? parent : input, myStatic, input.getText(), isOverrides(input), this, false);
   }
 
   @Nullable

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

@@ -7,6 +7,7 @@ import com.google.common.base.Predicate;
 import com.google.common.collect.Collections2;
 import com.intellij.psi.PsiElement;
 import com.intellij.util.ArrayUtil;
+import com.intellij.util.containers.ContainerUtil;
 import com.intellij.util.containers.MultiMap;
 import com.jetbrains.NotNullPredicate;
 import com.jetbrains.python.psi.PyClass;
@@ -92,24 +93,16 @@ public abstract class MembersManager<T extends PyElement> implements Function<T,
     final PyClass @NotNull ... to
   ) {
     List<PyMemberInfo<PyElement>> memberInfosSorted = new ArrayList<>(memberInfos);
-    memberInfosSorted.sort((o1, o2) -> PyDependenciesComparator.INSTANCE.compare(o1.getMember(), o2.getMember()));
+
+
+    memberInfosSorted.sort((o1, o2) -> PyDependenciesComparator.INSTANCE.compareAndStoreDependency(o1.getElementToStoreDependency(),
+                                                                                                   o2.getElementToStoreDependency()));
 
     for (PyMemberInfo<PyElement> info : memberInfosSorted) {
       TypeSafeMovingStrategy.moveCheckingTypesAtRunTime(from, info.getMembersManager(), Collections.singleton(info), to);
     }
 
-
-    /*//Move at once, sort
-    final Multimap<MembersManager<PyElement>, PyMemberInfo<PyElement>> managerToMember = ArrayListMultimap.create();
-    //Collect map (manager)->(list_of_memebers)
-    for (final PyMemberInfo<PyElement> memberInfo : memberInfos) {
-      managerToMember.put(memberInfo.getMembersManager(), memberInfo);
-    }
-    //Move members via manager
-    for (final MembersManager<PyElement> membersManager : managerToMember.keySet()) {
-      final Collection<PyMemberInfo<PyElement>> members = managerToMember.get(membersManager);
-      TypeSafeMovingStrategy.moveCheckingTypesAtRunTime(from, membersManager, members, to);
-    }*/
+    PyDependenciesComparator.clearDependencyInfo(ContainerUtil.map(memberInfos, o -> o.getElementToStoreDependency()));
   }
 
 

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

@@ -19,6 +19,7 @@ import com.jetbrains.python.psi.impl.PyFunctionBuilder;
 import com.jetbrains.python.psi.types.TypeEvalContext;
 import com.jetbrains.python.refactoring.PyPsiRefactoringUtil;
 import com.jetbrains.python.refactoring.classes.PyClassRefactoringUtil;
+import com.jetbrains.python.refactoring.classes.PyDependenciesComparator;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -94,7 +95,9 @@ class MethodsManager extends MembersManager<PyFunction> {
       for (final PyClass destClass : to) {
         final PyFunctionBuilder functionBuilder = PyFunctionBuilder.copySignature(function, DECORATORS_MAY_BE_COPIED_TO_ABSTRACT);
         functionBuilder.decorate(PyNames.ABSTRACTMETHOD);
-        PyClassRefactoringUtil.addMethods(destClass, false, functionBuilder.buildFunction());
+        PyFunction abstractMethod = functionBuilder.buildFunction();
+        PyDependenciesComparator.copyDependencyInfo(function, abstractMethod);
+        PyClassRefactoringUtil.addMethods(destClass, false, abstractMethod);
         classesToAddMetaAbc.add(destClass);
       }
     }

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

@@ -24,19 +24,16 @@ import org.jetbrains.annotations.Nullable;
 /**
  * @author Dennis.Ushakov
  */
-public class PyMemberInfo<T extends PyElement> extends MemberInfoBase<T> {
+public final class PyMemberInfo<T extends PyElement> extends MemberInfoBase<T> {
   @NotNull
   private final MembersManager<T> myMembersManager;
   private final boolean myCouldBeAbstract;
+  @NotNull
+  private final PyElement myElementToStoreDependency;
+
 
   /**
-   * @param couldBeAbstract if element could be marked as abstract (like abstract method)
-   * @param member         element itself
-   * @param isStatic       is it static or not?
-   * @param displayName    element display name
-   * @param overrides      does it overrides something? TRUE if is overriden, FALSE if implemented, null if not implemented or overriden
-   *                       TODO: use primitive instead? "Implemeneted" has nothing to do with python duck-typing
-   * @param membersManager manager that knows how to handle this member
+   * @see #PyMemberInfo(PyElement, PyElement, boolean, String, Boolean, MembersManager, boolean)
    */
   PyMemberInfo(@NotNull final T member,
                final boolean isStatic,
@@ -44,14 +41,40 @@ public class PyMemberInfo<T extends PyElement> extends MemberInfoBase<T> {
                @Nullable final Boolean overrides,
                @NotNull final MembersManager<T> membersManager,
                final boolean couldBeAbstract) {
+    this(member, member, isStatic, displayName, overrides, membersManager, couldBeAbstract);
+  }
+
+  /**
+   * @param couldBeAbstract if element could be marked as abstract (like abstract method)
+   * @param member          element itself
+   * @param elementToStoreDependency if memeber's parent is copied and used to calculate dependency, provide it here
+   * @param isStatic        is it static or not?
+   * @param displayName     element display name
+   * @param overrides       does it overrides something? TRUE if is overriden, FALSE if implemented, null if not implemented or overriden
+   *                                              TODO: use primitive instead? "Implemeneted" has nothing to do with python duck-typing
+   * @param membersManager  manager that knows how to handle this member
+   */
+  PyMemberInfo(@NotNull final T member,
+               @NotNull PyElement elementToStoreDependency,
+               final boolean isStatic,
+               @NotNull final String displayName,
+               @Nullable final Boolean overrides,
+               @NotNull final MembersManager<T> membersManager,
+               final boolean couldBeAbstract) {
     super(member);
     this.isStatic = isStatic;
+    this.myElementToStoreDependency = elementToStoreDependency;
     this.displayName = displayName;
     this.overrides = overrides;
     myMembersManager = membersManager;
     myCouldBeAbstract = couldBeAbstract;
   }
 
+  @NotNull
+  public PyElement getElementToStoreDependency() {
+    return myElementToStoreDependency;
+  }
+
   @NotNull
   MembersManager<T> getMembersManager() {
     return myMembersManager;

python/testData/refactoring/extractsuperclass/classPropertyDependsOnMethod.after.py

@@ -0,0 +1,16 @@
+from abc import ABCMeta, abstractmethod
+
+
+class Spam(metaclass=ABCMeta):
+    @abstractmethod
+    def __add__(self, other):
+        pass
+
+    __radd__ = __add__
+
+
+class C(Spam):
+    def __add__(self, other):
+        pass
+#
+#

python/testData/refactoring/extractsuperclass/classPropertyDependsOnMethod.before.py

@@ -0,0 +1,7 @@
+class C:
+    def __add__(self, other):
+        pass
+
+    __radd__ = __add__
+#
+#

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

@@ -17,4 +17,9 @@ class StaticOnly(object):
 
 class OldClass():
     def foo(self):
-        pass
+        pass
+
+
+class ExtractMe:
+    def __add__(self, other):
+    __radd__ = __add__

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

@@ -5,9 +5,10 @@ import org.jetbrains.annotations.NotNull;
 
 /**
  * Test only {@link com.jetbrains.python.refactoring.classes.membersManager.PyMemberInfo} representation.
+ *
  * @author Ilya.Kazakevich
  */
-public class PyPresenterTestMemberEntry {
+public final class PyPresenterTestMemberEntry {
   @NonNls @NotNull
   private final String myName;
   private final boolean myEnabled;
@@ -37,6 +38,16 @@ public class PyPresenterTestMemberEntry {
            '}';
   }
 
+
+  public boolean mayBeAbstract() {
+    return myMayBeAbstract;
+  }
+
+  @NotNull
+  public String getName() {
+    return myName;
+  }
+
   @Override
   public boolean equals(final Object o) {
     if (this == o) return true;

python/testSrc/com/jetbrains/python/refactoring/classes/extractSuperclass/PyExtractSuperclassPresenterTest.java

@@ -1,6 +1,7 @@
 // Copyright 2000-2017 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
 package com.jetbrains.python.refactoring.classes.extractSuperclass;
 
+import com.intellij.refactoring.classMembers.MemberInfoChange;
 import com.jetbrains.python.psi.LanguageLevel;
 import com.jetbrains.python.psi.PyClass;
 import com.jetbrains.python.psi.PyElement;
@@ -24,7 +25,7 @@ import java.util.List;
  *
  * @author Ilya.Kazakevich
  */
-public class PyExtractSuperclassPresenterTest
+public final class PyExtractSuperclassPresenterTest
   extends PyRefactoringPresenterTestCase<PyExtractSuperclassInitializationInfo, PyExtractSuperclassView> {
 
   public PyExtractSuperclassPresenterTest() {
@@ -124,6 +125,16 @@ public class PyExtractSuperclassPresenterTest
     compareMembers(members, matcher);
   }
 
+  public void testCantAbstractWhenDependentProperty() {
+    var functionName = "__add__";
+    launchAndGetMembers("ExtractMe");
+    var function = getMemberEntryByName(functionName);
+    Assert.assertTrue("Property is not checked, method must be open to be abstract", function.mayBeAbstract());
+    selectMember("__radd__");
+    function = getMemberEntryByName(functionName);
+    Assert.assertFalse("Property is checked, method can't be abstract", function.mayBeAbstract());
+  }
+
   /**
    * Checks that class fields could be moved while "extends object" is not in list
    */
@@ -166,4 +177,20 @@ public class PyExtractSuperclassPresenterTest
     final PyMemberInfoStorage storage = new PyMemberInfoStorage(childClass);
     return new PyExtractSuperclassPresenterImpl(myView, childClass, storage);
   }
+
+  private void selectMember(@NotNull String name) {
+    var info = myViewConfigCapture.getValue();
+    var infos = info.getMemberInfos();
+    for (var i : infos) {
+      if (i.getDisplayName().startsWith(name)) {
+        i.setChecked(true);
+      }
+    }
+    info.getMemberInfoModel().memberInfoChanged(new MemberInfoChange<>(infos));
+  }
+
+  @NotNull
+  private PyPresenterTestMemberEntry getMemberEntryByName(@NotNull String memberName) {
+    return getMembers().stream().filter(o -> o.getName().startsWith(memberName)).findFirst().orElseThrow();
+  }
 }

python/testSrc/com/jetbrains/python/refactoring/classes/extractSuperclass/PyExtractSuperclassTest.java

@@ -24,12 +24,17 @@ import java.util.List;
 /**
  * @author Dennis.Ushakov
  */
-public class PyExtractSuperclassTest extends PyClassRefactoringTest {
+public final class PyExtractSuperclassTest extends PyClassRefactoringTest {
 
   public PyExtractSuperclassTest() {
     super("extractsuperclass");
   }
 
+  // PY-21099
+  public void testClassPropertyDependsOnMethod() {
+    doSimpleTest("C", "Spam", null, true, true, ".__add__", "#__radd__");
+  }
+
   // Checks if class explicitly extends object we shall move it even in Py3K (PY-19137)
   public void testPy3ParentHasObject() {
     doSimpleTest("Child", "Parent", null, true, false, ".spam");