Fixed several bugs in new-style MRO algorithm

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

@@ -14,7 +14,7 @@ import com.intellij.psi.stubs.StubElement;
 import com.intellij.psi.tree.TokenSet;
 import com.intellij.psi.util.*;
 import com.intellij.util.*;
-import com.intellij.util.containers.*;
+import com.intellij.util.containers.ContainerUtil;
 import com.jetbrains.python.*;
 import com.jetbrains.python.codeInsight.controlflow.ControlFlowCache;
 import com.jetbrains.python.codeInsight.controlflow.ScopeOwner;
@@ -34,8 +34,6 @@ import org.jetbrains.annotations.Nullable;
 
 import javax.swing.*;
 import java.util.*;
-import java.util.HashMap;
-import java.util.HashSet;
 
 /**
  * @author yole
@@ -295,6 +293,7 @@ public class PyClassImpl extends PyPresentableElementImpl<PyClassStub> implement
       }
       if (nonBlankSequences.isEmpty()) return result;
       // find a clean head
+      boolean found = false;
       PyClassLikeType head = null; // to keep compiler happy; really head is assigned in the loop at least once.
       for (List<PyClassLikeType> seq : nonBlankSequences) {
         head = seq.get(0);
@@ -306,39 +305,48 @@ public class PyClassImpl extends PyPresentableElementImpl<PyClassStub> implement
           }
         }
         if (!head_in_tails) {
+          found = true;
           break;
         }
         else {
           head = null; // as a signal
         }
       }
+      if (!found) {
+        // Inconsistent hierarchy results in TypeError
+        throw new IllegalStateException("Inconsistent class hierarchy");
+      }
       // our head is clean;
       result.add(head);
       // remove it from heads of other sequences
       for (List<PyClassLikeType> seq : nonBlankSequences) {
-        if (Comparing.equal(seq.get(0), head)) seq.remove(0);
+        if (Comparing.equal(seq.get(0), head)) {
+          seq.remove(0);
+        }
       }
     } // we either return inside the loop or die by assertion
   }
 
   @NotNull
-  private static List<PyClassLikeType> mroLinearize(@NotNull PyClassLikeType type, @NotNull List<PyClassLikeType> seen, boolean addThisType,
+  private static List<PyClassLikeType> mroLinearize(@NotNull PyClassLikeType type, @NotNull Set<PyClassLikeType> seen, boolean addThisType,
                                                     @NotNull TypeEvalContext context) {
-    assert (seen.indexOf(type) < 0) : "Circular import structure on " + PyUtil.nvl(type);
+    if (seen.contains(type)) {
+      throw new IllegalStateException("Circular class inheritance");
+    }
     final List<PyClassLikeType> bases = type.getSuperClassTypes(context);
-    List<List<PyClassLikeType>> lins = new ArrayList<List<PyClassLikeType>>(bases.size() * 2);
-    ArrayList<PyClassLikeType> new_seen = new ArrayList<PyClassLikeType>(seen.size() + 1);
-    new_seen.add(type);
+    List<List<PyClassLikeType>> lines = new ArrayList<List<PyClassLikeType>>();
     for (PyClassLikeType base : bases) {
       if (base != null) {
-        List<PyClassLikeType> lin = mroLinearize(base, new_seen, true, context);
-        if (!lin.isEmpty()) lins.add(lin);
+        final Set<PyClassLikeType> newSeen = new HashSet<PyClassLikeType>(seen);
+        newSeen.add(type);
+        List<PyClassLikeType> lin = mroLinearize(base, newSeen, true, context);
+        if (!lin.isEmpty()) lines.add(lin);
       }
     }
-    for (PyClassLikeType base : bases) {
-      lins.add(new SmartList<PyClassLikeType>(base));
+    if (!bases.isEmpty()) {
+      lines.add(bases);
     }
-    List<PyClassLikeType> result = mroMerge(lins);
+    List<PyClassLikeType> result = mroMerge(lines);
     if (addThisType) {
       result.add(0, type);
     }
@@ -1062,7 +1070,11 @@ public class PyClassImpl extends PyPresentableElementImpl<PyClassStub> implement
   private List<PyClassLikeType> getMROAncestorTypes(@NotNull TypeEvalContext context) {
     final PyType thisType = context.getType(this);
     if (thisType instanceof PyClassLikeType) {
-      return mroLinearize((PyClassLikeType)thisType, Collections.<PyClassLikeType>emptyList(), false, context);
+      try {
+        return mroLinearize((PyClassLikeType)thisType, new HashSet<PyClassLikeType>(), false, context);
+      }
+      catch (IllegalStateException ignored) {
+      }
     }
     return Collections.emptyList();
   }

python/testData/codeInsight/classMRO/CircularInheritance.py

@@ -0,0 +1,5 @@
+from CircularInheritance2 import B
+
+
+class Foo(B):
+    pass

python/testData/codeInsight/classMRO/CircularInheritance2.py

@@ -0,0 +1,9 @@
+from CircularInheritance import Foo
+
+
+class X(object):
+    pass
+
+
+class B(X, Foo):
+    pass

python/testData/codeInsight/classMRO/ExampleFromDoc1.py

@@ -0,0 +1,22 @@
+class D(object):
+    pass
+
+
+class E(object):
+    pass
+
+
+class F(object):
+    pass
+
+
+class B(D, E):
+    pass
+
+
+class C(D, F):
+    pass
+
+
+class A(B, C):
+    pass

python/testData/codeInsight/classMRO/ExampleFromDoc2.py

@@ -0,0 +1,22 @@
+class D(object):
+    pass
+
+
+class E(object):
+    pass
+
+
+class F(object):
+    pass
+
+
+class B(E, D):
+    pass
+
+
+class C(D, F):
+    pass
+
+
+class A(B, C):
+    pass

python/testData/codeInsight/classMRO/ExampleFromDoc3.py

@@ -0,0 +1,10 @@
+class F(object):
+    pass
+
+
+class E(F):
+    pass
+
+
+class G(F, E):
+    pass

python/testData/codeInsight/classMRO/ExampleFromDoc4.py

@@ -0,0 +1,10 @@
+class F(object):
+    pass
+
+
+class E(F):
+    pass
+
+
+class G(E, F):
+    pass

python/testData/codeInsight/classMRO/MROConflict.py

@@ -0,0 +1,18 @@
+class X(object):
+    pass
+
+
+class Y(object):
+    pass
+
+
+class A(X, Y):
+    pass
+
+
+class B(Y, X):
+    pass
+
+
+class C(A, B):
+    pass

python/testData/codeInsight/classMRO/SimpleDiamond.py

@@ -0,0 +1,10 @@
+class B1(object):
+    pass
+
+
+class B2(object):
+    pass
+
+
+class C(B1, B2):
+    pass

python/testSrc/com/jetbrains/python/PythonAllTestsSuite.java

@@ -3,6 +3,7 @@ package com.jetbrains.python;
 import com.jetbrains.cython.*;
 import com.jetbrains.django.lang.template.DjangoTemplateParserTest;
 import com.jetbrains.jinja2.Jinja2ParserTest;
+import com.jetbrains.python.codeInsight.PyClassMROTest;
 import com.jetbrains.python.codeInsight.PyCompletionPatternsTest;
 import com.jetbrains.python.inspections.*;
 import com.jetbrains.python.refactoring.*;
@@ -36,6 +37,7 @@ public class PythonAllTestsSuite {
     PyResolveTest.class,
     Py3ResolveTest.class,
     PyMultiFileResolveTest.class,
+    PyClassMROTest.class,
     PyResolveCalleeTest.class,
     CythonResolveTest.class,
     PyAssignmentMappingTest.class,

python/testSrc/com/jetbrains/python/codeInsight/PyClassMROTest.java

@@ -0,0 +1,77 @@
+package com.jetbrains.python.codeInsight;
+
+import com.jetbrains.python.fixtures.PyTestCase;
+import com.jetbrains.python.psi.PyClass;
+import com.jetbrains.python.psi.types.PyClassLikeType;
+import com.jetbrains.python.psi.types.TypeEvalContext;
+import org.jetbrains.annotations.NotNull;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+/**
+ * @author vlan
+ */
+public class PyClassMROTest extends PyTestCase {
+  public void testSimpleDiamond() {
+    assertMRO(getClass("C"), "B1", "B2", "object");
+  }
+
+  // TypeError in Python
+  public void testMROConflict() {
+    assertMRO(getClass("C"));
+  }
+
+  public void testCircularInheritance() {
+    final String testName = getTestName(false);
+    myFixture.configureByFiles(getPath(testName), getPath(testName + "2"));
+    final PyClass cls = myFixture.findElementByText("Foo", PyClass.class);
+    assertNotNull(cls);
+    assertMRO(cls);
+  }
+
+  public void testExampleFromDoc1() {
+    assertMRO(getClass("A"), "B", "C", "D", "E", "F", "object");
+  }
+
+  public void testExampleFromDoc2() {
+    assertMRO(getClass("A"), "B", "E", "C", "D", "F", "object");
+  }
+
+  public void testExampleFromDoc3() {
+    assertMRO(getClass("G"));
+  }
+
+  public void testExampleFromDoc4() {
+    assertMRO(getClass("G"), "E", "F", "object");
+  }
+
+  public void assertMRO(@NotNull PyClass cls, @NotNull String... mro) {
+    final List<PyClassLikeType> types = cls.getAncestorTypes(TypeEvalContext.fastStubOnly(null));
+    final List<String> classNames = new ArrayList<String>();
+    for (PyClassLikeType type : types) {
+      if (type != null) {
+        final String name = type.getName();
+        if (name != null) {
+          classNames.add(name);
+          continue;
+        }
+      }
+      classNames.add("unknown");
+    }
+    assertOrderedEquals(classNames, Arrays.asList(mro));
+  }
+
+  @NotNull
+  public PyClass getClass(@NotNull String name) {
+    myFixture.configureByFile(getPath(getTestName(false)));
+    final PyClass cls = myFixture.findElementByText(name, PyClass.class);
+    assertNotNull(cls);
+    return cls;
+  }
+
+  private static String getPath(String name) {
+    return "codeInsight/classMRO/" + name + ".py";
+  }
+}