Skip to content

Recompute class MROs in ThirdPass, to handle import loops #1397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 19, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 30 additions & 15 deletions mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@
it will reject Dict[int]. We don't do this in the second pass,
since we infer the type argument counts of classes during this
pass, and it is possible to refer to classes defined later in a
file, which would not have the type argument count set yet.
file, which would not have the type argument count set yet. This
pass also recomputes the method resolution order of each class, in
case one of its bases belongs to a module involved in an import
loop.

Semantic analysis of types is implemented in module mypy.typeanal.

Expand All @@ -41,7 +44,7 @@
"""

from typing import (
List, Dict, Set, Tuple, cast, Any, overload, TypeVar, Union, Optional
List, Dict, Set, Tuple, cast, Any, overload, TypeVar, Union, Optional, Callable
)

from mypy.nodes import (
Expand Down Expand Up @@ -753,22 +756,17 @@ def analyze_base_classes(self, defn: ClassDef) -> None:
obj = self.object_type()
defn.base_types.insert(0, obj)
defn.info.bases = defn.base_types
# Calculate the MRO. It might be incomplete at this point if
# the bases of defn include classes imported from other
# modules in an import loop. We'll recompute it in ThirdPass.
if not self.verify_base_classes(defn):
defn.info.mro = []
return
try:
defn.info.calculate_mro()
except MroError:
self.fail("Cannot determine consistent method resolution order "
'(MRO) for "%s"' % defn.name, defn)
defn.info.mro = []
else:
# If there are cyclic imports, we may be missing 'object' in
# the MRO. Fix MRO if needed.
if defn.info.mro[-1].fullname() != 'builtins.object':
defn.info.mro.append(self.object_type().type)
# The property of falling back to Any is inherited.
defn.info.fallback_to_any = any(baseinfo.fallback_to_any for baseinfo in defn.info.mro)
calculate_class_mro(defn, self.fail)
# If there are cyclic imports, we may be missing 'object' in
# the MRO. Fix MRO if needed.
if defn.info.mro and defn.info.mro[-1].fullname() != 'builtins.object':
defn.info.mro.append(self.object_type().type)

def expr_to_analyzed_type(self, expr: Node) -> Type:
if isinstance(expr, CallExpr):
Expand Down Expand Up @@ -2437,6 +2435,12 @@ def visit_func_def(self, fdef: FuncDef) -> None:
def visit_class_def(self, tdef: ClassDef) -> None:
for type in tdef.info.bases:
self.analyze(type)
# Recompute MRO now that we have analyzed all modules, to pick
# up superclasses of bases imported from other modules in an
# import loop. (Only do so if we succeeded the first time.)
if tdef.info.mro:
tdef.info.mro = [] # Force recomputation
calculate_class_mro(tdef, self.fail)
super().visit_class_def(tdef)

def visit_decorator(self, dec: Decorator) -> None:
Expand Down Expand Up @@ -2569,6 +2573,17 @@ def refers_to_class_or_function(node: Node) -> bool:
OverloadedFuncDef)))


def calculate_class_mro(defn: ClassDef, fail: Callable[[str, Context], None]) -> None:
try:
defn.info.calculate_mro()
except MroError:
fail("Cannot determine consistent method resolution order "
'(MRO) for "%s"' % defn.name, defn)
defn.info.mro = []
# The property of falling back to Any is inherited.
defn.info.fallback_to_any = any(baseinfo.fallback_to_any for baseinfo in defn.info.mro)


def find_duplicate(list: List[T]) -> T:
"""If the list has duplicates, return one of the duplicates.

Expand Down
30 changes: 30 additions & 0 deletions mypy/test/data/check-modules.test
Original file line number Diff line number Diff line change
Expand Up @@ -739,3 +739,33 @@ y = 42
[builtins fixtures/module.py]
[out]
tmp/main.py:4: error: Unsupported left operand type for + ("int")

[case testSuperclassInImportCycle]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this assume that import loops are processed in a particular order? If yes, as we might change the order in the future, it may be better to have another test case would demonstrate the issue if the modules in the import cycle were processed in the opposite order.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, good point. This test is sensitive to the import order. I added a second test with the imports in the main module reversed.

import a
import d
a.A().f(d.D())
[file a.py]
if 0:
import d
class B: pass
class C(B): pass
class A:
def f(self, x: B) -> None: pass
[file d.py]
import a
class D(a.C): pass

[case testSuperclassInImportCycleReversedImports]
import d
import a
a.A().f(d.D())
[file a.py]
if 0:
import d
class B: pass
class C(B): pass
class A:
def f(self, x: B) -> None: pass
[file d.py]
import a
class D(a.C): pass