name conflicts in Python rename (PY-2390)

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

@@ -4,6 +4,7 @@ import com.intellij.lang.ASTNode;
 import com.intellij.psi.PsiElement;
 import com.intellij.psi.PsiNamedElement;
 import com.intellij.psi.StubBasedPsiElement;
+import com.intellij.util.ArrayFactory;
 import com.intellij.util.Processor;
 import com.jetbrains.python.codeInsight.controlflow.ScopeOwner;
 import com.jetbrains.python.psi.stubs.PyClassStub;
@@ -19,6 +20,13 @@ import java.util.List;
 public interface PyClass extends
   PsiNamedElement, PyStatement, NameDefiner, PyDocStringOwner, StubBasedPsiElement<PyClassStub>, ScopeOwner, PyDecoratable
 {
+  ArrayFactory<PyClass> ARRAY_FACTORY = new ArrayFactory<PyClass>() {
+    @Override
+    public PyClass[] create(int count) {
+      return new PyClass[count];
+    }
+  };
+
   @Nullable
   ASTNode getNameNode();
 
@@ -82,6 +90,11 @@ public interface PyClass extends
 
   List<PyTargetExpression> getInstanceAttributes();
 
+  PyClass[] getNestedClasses();
+
+  @Nullable
+  PyClass findNestedClass(String name, boolean inherited);
+
   /**
    * @return true if the class is new-style and descends from 'object'.
    */

python/src/com/jetbrains/python/psi/PyFunction.java

@@ -4,6 +4,7 @@ import com.intellij.lang.ASTNode;
 import com.intellij.psi.PsiNameIdentifierOwner;
 import com.intellij.psi.PsiNamedElement;
 import com.intellij.psi.StubBasedPsiElement;
+import com.intellij.util.ArrayFactory;
 import com.jetbrains.python.codeInsight.controlflow.ScopeOwner;
 import com.jetbrains.python.psi.stubs.PyFunctionStub;
 import com.jetbrains.python.psi.types.PyType;
@@ -18,8 +19,15 @@ public interface PyFunction
 extends
   PsiNamedElement, StubBasedPsiElement<PyFunctionStub>,
   PsiNameIdentifierOwner, PyStatement, Callable, NameDefiner, PyDocStringOwner, ScopeOwner, PyDecoratable {
+
   PyFunction[] EMPTY_ARRAY = new PyFunction[0];
-  
+  ArrayFactory<PyFunction> ARRAY_FACTORY = new ArrayFactory<PyFunction>() {
+    @Override
+    public PyFunction[] create(int count) {
+      return new PyFunction[count];
+    }
+  };
+
   /**
    * Returns the AST node for the function name identifier.
    *

python/src/com/jetbrains/python/psi/impl/PyClassImpl.java

@@ -10,12 +10,10 @@ import com.intellij.psi.ResolveState;
 import com.intellij.psi.StubBasedPsiElement;
 import com.intellij.psi.scope.PsiScopeProcessor;
 import com.intellij.psi.stubs.StubElement;
+import com.intellij.psi.tree.IElementType;
 import com.intellij.psi.util.*;
 import com.intellij.reference.SoftReference;
-import com.intellij.util.Icons;
-import com.intellij.util.IncorrectOperationException;
-import com.intellij.util.Processor;
-import com.intellij.util.SmartList;
+import com.intellij.util.*;
 import com.jetbrains.python.PyElementTypes;
 import com.jetbrains.python.PyNames;
 import com.jetbrains.python.PyTokenTypes;
@@ -303,19 +301,29 @@ public class PyClassImpl extends PyPresentableElementImpl<PyClassStub> implement
 
   @NotNull
   public PyFunction[] getMethods() {
+    return getClassChildren(PyElementTypes.FUNCTION_DECLARATION, PyFunction.ARRAY_FACTORY);
+  }
+
+  @Override
+  public PyClass[] getNestedClasses() {
+    return getClassChildren(PyElementTypes.CLASS_DECLARATION, PyClass.ARRAY_FACTORY);
+  }
+
+  private <T extends PsiElement> T[] getClassChildren(IElementType elementType, ArrayFactory<T> factory) {
     // TODO: gather all top-level functions, maybe within control statements
     final PyClassStub classStub = getStub();
     if (classStub != null) {
-      return classStub.getChildrenByType(PyElementTypes.FUNCTION_DECLARATION, PyFunction.EMPTY_ARRAY);
+      return classStub.getChildrenByType(elementType, factory);
     }
-    List<PyFunction> result = new ArrayList<PyFunction>();
+    List<T> result = new ArrayList<T>();
     final PyStatementList statementList = getStatementList();
     for (PsiElement element : statementList.getChildren()) {
-      if (element instanceof PyFunction) {
-        result.add((PyFunction) element);
+      if (element.getNode().getElementType() == elementType) {
+        //noinspection unchecked
+        result.add((T) element);
       }
     }
-    return result.toArray(new PyFunction[result.size()]);
+    return result.toArray(factory.create(result.size()));
   }
 
   private static class NameFinder<T extends PyElement> implements Processor<T> {
@@ -350,6 +358,15 @@ public class PyClassImpl extends PyPresentableElementImpl<PyClassStub> implement
     return proc.getResult();
   }
 
+  @Nullable
+  @Override
+  public PyClass findNestedClass(String name, boolean inherited) {
+    if (name == null) return null;
+    NameFinder<PyClass> proc = new NameFinder<PyClass>(name);
+    visitNestedClasses(proc, inherited);
+    return proc.getResult();
+  }
+
   @Nullable
   public PyFunction findInitOrNew(boolean inherited) {
     NameFinder<PyFunction> proc;
@@ -616,6 +633,21 @@ public class PyClassImpl extends PyPresentableElementImpl<PyClassStub> implement
     return true;
   }
 
+  public boolean visitNestedClasses(Processor<PyClass> processor, boolean inherited) {
+    PyClass[] nestedClasses = getNestedClasses();
+    for (PyClass nestedClass : nestedClasses) {
+      if (!processor.process(nestedClass)) return false;
+    }
+    if (inherited) {
+      for (PyClass ancestor : iterateAncestors()) {
+        if (!((PyClassImpl) ancestor).visitNestedClasses(processor, false)) {
+          return false;
+        }
+      }
+    }
+    return true;
+  }
+
   public boolean visitClassAttributes(Processor<PyTargetExpression> processor, boolean inherited) {
     List<PyTargetExpression> methods = getClassAttributes();
     for(PyTargetExpression attribute: methods) {

python/src/com/jetbrains/python/refactoring/rename/RenamePyClassProcessor.java

@@ -3,7 +3,6 @@ package com.jetbrains.python.refactoring.rename;
 import com.intellij.psi.PsiElement;
 import com.intellij.psi.PsiReference;
 import com.intellij.psi.search.searches.ReferencesSearch;
-import com.intellij.refactoring.rename.RenamePsiElementProcessor;
 import com.intellij.util.Processor;
 import com.jetbrains.python.PyNames;
 import com.jetbrains.python.codeInsight.PyCodeInsightSettings;
@@ -18,7 +17,7 @@ import java.util.List;
 /**
  * @author yole
  */
-public class RenamePyClassProcessor extends RenamePsiElementProcessor {
+public class RenamePyClassProcessor extends RenamePyElementProcessor {
   @Override
   public boolean canProcessElement(@NotNull PsiElement element) {
     return element instanceof PyClass;

python/src/com/jetbrains/python/refactoring/rename/RenamePyElementProcessor.java

@@ -0,0 +1,48 @@
+package com.jetbrains.python.refactoring.rename;
+
+import com.intellij.psi.PsiElement;
+import com.intellij.psi.util.PsiTreeUtil;
+import com.intellij.refactoring.rename.RenamePsiElementProcessor;
+import com.intellij.util.containers.MultiMap;
+import com.jetbrains.python.codeInsight.controlflow.ScopeOwner;
+import com.jetbrains.python.psi.*;
+
+/**
+ * @author yole
+ */
+public abstract class RenamePyElementProcessor extends RenamePsiElementProcessor {
+  @Override
+  public void findExistingNameConflicts(PsiElement element, String newName, MultiMap<PsiElement, String> conflicts) {
+    PyElement container = PsiTreeUtil.getParentOfType(element, ScopeOwner.class);
+    if (container instanceof PyFile) {
+      PyFile pyFile = (PyFile)container;
+      PyClass conflictingClass = pyFile.findTopLevelClass(newName);
+      if (conflictingClass != null) {
+        conflicts.putValue(conflictingClass, "A class named '" + newName + "' is already defined in " + pyFile.getName());
+      }
+      PyFunction conflictingFunction = pyFile.findTopLevelFunction(newName);
+      if (conflictingFunction != null) {
+        conflicts.putValue(conflictingFunction, "A function named '" + newName + "' is already defined in " + pyFile.getName());
+      }
+      PyTargetExpression conflictingVariable = pyFile.findTopLevelAttribute(newName);
+      if (conflictingVariable != null) {
+        conflicts.putValue(conflictingFunction, "A variable named '" + newName + "' is already defined in " + pyFile.getName());
+      }
+    }
+    else if (container instanceof PyClass) {
+      PyClass pyClass = (PyClass)container;
+      PyClass conflictingClass = pyClass.findNestedClass(newName, true);
+      if (conflictingClass != null) {
+        conflicts.putValue(conflictingClass, "A class named '" + newName + "' is already defined in class '" + pyClass.getName() + "'");
+      }
+      PyFunction conflictingFunction = pyClass.findMethodByName(newName, true);
+      if (conflictingFunction != null) {
+        conflicts.putValue(conflictingFunction, "A function named '" + newName + "' is already defined in class '" + pyClass.getName() + "'");
+      }
+      PyTargetExpression conflictingAttribute = pyClass.findClassAttribute(newName, true);
+      if (conflictingAttribute != null) {
+        conflicts.putValue(conflictingAttribute, "An attribute named '" + newName + "' is already defined in class '" + pyClass.getName() + "'");
+      }
+    }
+  }
+}

python/src/com/jetbrains/python/refactoring/rename/RenamePyFunctionProcessor.java

@@ -20,7 +20,7 @@ import java.util.Map;
 /**
  * @author yole
  */
-public class RenamePyFunctionProcessor extends RenamePsiElementProcessor {
+public class RenamePyFunctionProcessor extends RenamePyElementProcessor {
   @Override
   public boolean canProcessElement(@NotNull PsiElement element) {
     return element instanceof PyFunction;

python/src/com/jetbrains/python/refactoring/rename/RenamePyVariableProcessor.java

@@ -9,10 +9,10 @@ import org.jetbrains.annotations.NotNull;
 /**
  * @author yole
  */
-public class RenamePyVariableProcessor extends RenamePsiElementProcessor {
+public class RenamePyVariableProcessor extends RenamePyElementProcessor {
   @Override
   public boolean canProcessElement(@NotNull PsiElement element) {
-    // extension ordering in python-plugin-common.xml ensures that classes and variables are handled by their own processors
+    // extension ordering in python-plugin-common.xml ensures that classes and functions are handled by their own processors
     return element instanceof PyElement;
   }
 

python/testData/refactoring/rename/classNameConflict.py

@@ -0,0 +1,5 @@
+class Foo:
+    pass
+
+class Ba<caret>r:
+    pass

python/testData/refactoring/rename/classVsFunctionConflict.py

@@ -0,0 +1,5 @@
+def Foo():
+    pass
+
+class Ba<caret>r:
+    pass

python/testData/refactoring/rename/classVsVariableConflict.py

@@ -0,0 +1,4 @@
+Foo = False
+
+class Ba<caret>r:
+    pass

python/testData/refactoring/rename/functionNameConflict.py

@@ -0,0 +1,5 @@
+def foo():
+    pass
+
+def ba<caret>r():
+    pass

python/testData/refactoring/rename/nestedClassNameConflict.py

@@ -0,0 +1,6 @@
+class C:
+    class Foo:
+        pass
+
+    class Ba<caret>r:
+        pass

python/testSrc/com/jetbrains/python/refactoring/PyRenameTest.java

@@ -2,6 +2,7 @@ package com.jetbrains.python.refactoring;
 
 import com.intellij.codeInsight.TargetElementUtilBase;
 import com.intellij.psi.PsiElement;
+import com.intellij.refactoring.BaseRefactoringProcessor;
 import com.jetbrains.python.fixtures.PyLightFixtureTestCase;
 
 /**
@@ -53,6 +54,42 @@ public class PyRenameTest extends PyLightFixtureTestCase {
     doTest("bar");
   }
 
+  public void testClassNameConflict() {  // PY-2390
+    doRenameConflictTest("Foo", "A class named 'Foo' is already defined in classNameConflict.py");
+  }
+
+  public void testClassVsFunctionConflict() {
+    doRenameConflictTest("Foo", "A function named 'Foo' is already defined in classVsFunctionConflict.py");
+  }
+
+  public void testClassVsVariableConflict() {
+    doRenameConflictTest("Foo", "A variable named 'Foo' is already defined in classVsVariableConflict.py");
+  }
+
+  public void testNestedClassNameConflict() {
+    doRenameConflictTest("Foo", "A class named 'Foo' is already defined in class 'C'");
+  }
+
+  public void testFunctionNameConflict() {
+    doRenameConflictTest("foo", "A function named 'foo' is already defined in functionNameConflict.py");
+  }
+
+  public void testVariableNameConflict() {
+    doRenameConflictTest("foo", "A variable named 'foo' is already defined in variableNameConflict.py");
+  }
+
+  private void doRenameConflictTest(String newName, String expectedConflict) {
+    myFixture.configureByFile("refactoring/rename/" + getTestName(true) + ".py");
+    try {
+      myFixture.renameElementAtCaret(newName);
+    }
+    catch (BaseRefactoringProcessor.ConflictsInTestsException ex) {
+      assertEquals(expectedConflict, ex.getMessage());
+      return;
+    }
+    fail("Expected conflict not reported");
+  }
+
   private void doTest(final String newName) {
     myFixture.configureByFile("refactoring/rename/" + getTestName(true) + ".py");
     myFixture.renameElementAtCaret(newName);