Fix issues related to indirect imports in import cycles

mypy/nodes.py

@@ -325,6 +325,38 @@ def accept(self, visitor: StatementVisitor[T]) -> T:
         return visitor.visit_import_all(self)
 
 
+class ImportedName(SymbolNode):
+    """Indirect reference to a fullname stored in symbol table.
+
+    This node is not present in the original program as such. This is
+    just a temporary artifact in binding imported names. After semantic
+    analysis pass 2, these references should be replaced with direct
+    reference to a real AST node.
+
+    Note that this is neither a Statement nor an Expression so this
+    can't be visited.
+    """
+
+    def __init__(self, target_fullname: str) -> None:
+        self.target_fullname = target_fullname
+
+    def name(self) -> str:
+        return self.target_fullname.split('.')[-1]
+
+    def fullname(self) -> str:
+        return self.target_fullname
+
+    def serialize(self) -> JsonDict:
+        assert False, "ImportedName leaked from semantic analysis"
+
+    @classmethod
+    def deserialize(cls, data: JsonDict) -> 'ImportedName':
+        assert False, "ImportedName should never be serialized"
+
+    def __str__(self) -> str:
+        return 'ImportedName(%s)' % self.target_fullname
+
+
 class FuncBase(Node):
     """Abstract base class for function-like nodes"""
 

mypy/semanal.py

@@ -55,7 +55,7 @@
     YieldFromExpr, NamedTupleExpr, TypedDictExpr, NonlocalDecl, SymbolNode,
     SetComprehension, DictionaryComprehension, TYPE_ALIAS, TypeAliasExpr,
     YieldExpr, ExecStmt, Argument, BackquoteExpr, ImportBase, AwaitExpr,
-    IntExpr, FloatExpr, UnicodeExpr, EllipsisExpr, TempNode, EnumCallExpr,
+    IntExpr, FloatExpr, UnicodeExpr, EllipsisExpr, TempNode, EnumCallExpr, ImportedName,
     COVARIANT, CONTRAVARIANT, INVARIANT, UNBOUND_IMPORTED, LITERAL_YES, ARG_OPT, nongen_builtins,
     collections_type_aliases, get_member_expr_fullname,
 )
@@ -84,7 +84,7 @@
 from mypy.plugin import Plugin, ClassDefContext, SemanticAnalyzerPluginInterface
 from mypy import join
 from mypy.util import get_prefix, correct_relative_import
-from mypy.semanal_shared import PRIORITY_FALLBACKS
+from mypy.semanal_shared import SemanticAnalyzerInterface, PRIORITY_FALLBACKS
 from mypy.scope import Scope
 
 
@@ -174,7 +174,9 @@
 }
 
 
-class SemanticAnalyzerPass2(NodeVisitor[None], SemanticAnalyzerPluginInterface):
+class SemanticAnalyzerPass2(NodeVisitor[None],
+                            SemanticAnalyzerInterface,
+                            SemanticAnalyzerPluginInterface):
     """Semantically analyze parsed mypy files.
 
     The analyzer binds names and does various consistency checks for a
@@ -1488,6 +1490,8 @@ def visit_import_from(self, imp: ImportFrom) -> None:
         module = self.modules.get(import_id)
         for id, as_id in imp.names:
             node = module.names.get(id) if module else None
+            node = self.dereference_module_cross_ref(node)
+
             missing = False
             possible_module_id = import_id + '.' + id
 
@@ -1516,11 +1520,14 @@ def visit_import_from(self, imp: ImportFrom) -> None:
                     ast_node = Var(name, type=typ)
                     symbol = SymbolTableNode(GDEF, ast_node)
                     self.add_symbol(name, symbol, imp)
-                    return
+                    continue
             if node and node.kind != UNBOUND_IMPORTED and not node.module_hidden:
                 node = self.normalize_type_alias(node, imp)
                 if not node:
-                    return
+                    # Normalization failed because target is not defined. Avoid duplicate
+                    # error messages by marking the imported name as unknown.
+                    self.add_unknown_symbol(as_id or id, imp, is_import=True)
+                    continue
                 imported_id = as_id or id
                 existing_symbol = self.globals.get(imported_id)
                 if existing_symbol:
@@ -1550,6 +1557,29 @@ def visit_import_from(self, imp: ImportFrom) -> None:
                 # Missing module.
                 self.add_unknown_symbol(as_id or id, imp, is_import=True)
 
+    def dereference_module_cross_ref(
+            self, node: Optional[SymbolTableNode]) -> Optional[SymbolTableNode]:
+        """Dereference cross references to other modules (if any).
+
+        If the node is not a cross reference, return it unmodified.
+        """
+        seen = set()  # type: Set[str]
+        # Continue until we reach a node that's nota cross reference (or until we find
+        # nothing).
+        while node and isinstance(node.node, ImportedName):
+            fullname = node.node.fullname()
+            if fullname in self.modules:
+                # This is a module reference.
+                return SymbolTableNode(MODULE_REF, self.modules[fullname])
+            if fullname in seen:
+                # Looks like a reference cycle. Just break it.
+                # TODO: Generate a more specific error message.
+                node = None
+                break
+            node = self.lookup_fully_qualified_or_none(fullname)
+            seen.add(fullname)
+        return node
+
     def process_import_over_existing_name(self,
                                           imported_id: str, existing_symbol: SymbolTableNode,
                                           module_symbol: SymbolTableNode,
@@ -1573,6 +1603,14 @@ def process_import_over_existing_name(self,
 
     def normalize_type_alias(self, node: SymbolTableNode,
                              ctx: Context) -> Optional[SymbolTableNode]:
+        """If node refers to a built-in type alias, normalize it.
+
+        An example normalization is 'typing.List' -> '__builtins__.list'.
+
+        By default, if the node doesn't refer to a built-in type alias, return
+        the original node. If normalization fails because the target isn't
+        defined, return None.
+        """
         normalized = False
         fullname = node.fullname
         if fullname in type_aliases:
@@ -1612,7 +1650,10 @@ def visit_import_all(self, i: ImportAll) -> None:
         if i_id in self.modules:
             m = self.modules[i_id]
             self.add_submodules_to_parent_modules(i_id, True)
-            for name, node in m.names.items():
+            for name, orig_node in m.names.items():
+                node = self.dereference_module_cross_ref(orig_node)
+                if node is None:
+                    continue
                 new_node = self.normalize_type_alias(node, i)
                 # if '__all__' exists, all nodes not included have had module_public set to
                 # False, and we can skip checking '_' because it's been explicitly included.
@@ -1670,11 +1711,8 @@ def type_analyzer(self, *,
                       third_pass: bool = False) -> TypeAnalyser:
         if tvar_scope is None:
             tvar_scope = self.tvar_scope
-        tpan = TypeAnalyser(self.lookup_qualified,
-                            self.lookup_fully_qualified,
+        tpan = TypeAnalyser(self,
                             tvar_scope,
-                            self.fail,
-                            self.note,
                             self.plugin,
                             self.options,
                             self.is_typeshed_stub_file,
@@ -1803,11 +1841,8 @@ def analyze_alias(self, rvalue: Expression,
         dynamic = bool(self.function_stack and self.function_stack[-1].is_dynamic())
         global_scope = not self.type and not self.function_stack
         res = analyze_type_alias(rvalue,
-                                 self.lookup_qualified,
-                                 self.lookup_fully_qualified,
+                                 self,
                                  self.tvar_scope,
-                                 self.fail,
-                                 self.note,
                                  self.plugin,
                                  self.options,
                                  self.is_typeshed_stub_file,
@@ -3408,6 +3443,7 @@ def visit_member_expr(self, expr: MemberExpr) -> None:
             # else:
             #     names = file.names
             n = file.names.get(expr.name, None) if file is not None else None
+            n = self.dereference_module_cross_ref(n)
             if n and not n.module_hidden:
                 n = self.normalize_type_alias(n, expr)
                 if not n:
@@ -3813,22 +3849,21 @@ def lookup_fully_qualified(self, name: str) -> SymbolTableNode:
             n = next_sym.node
         return n.names[parts[-1]]
 
-    def lookup_fully_qualified_or_none(self, name: str) -> Optional[SymbolTableNode]:
-        """Lookup a fully qualified name.
+    def lookup_fully_qualified_or_none(self, fullname: str) -> Optional[SymbolTableNode]:
+        """Lookup a fully qualified name that refers to a module-level definition.
 
         Don't assume that the name is defined. This happens in the global namespace --
-        the local module namespace is ignored.
+        the local module namespace is ignored. This does not dereference indirect
+        refs.
+
+        Note that this can't be used for names nested in class namespaces.
         """
-        assert '.' in name
-        parts = name.split('.')
-        n = self.modules[parts[0]]
-        for i in range(1, len(parts) - 1):
-            next_sym = n.names.get(parts[i])
-            if not next_sym:
-                return None
-            assert isinstance(next_sym.node, MypyFile)
-            n = next_sym.node
-        return n.names.get(parts[-1])
+        assert '.' in fullname
+        module, name = fullname.rsplit('.', maxsplit=1)
+        if module not in self.modules:
+            return None
+        filenode = self.modules[module]
+        return filenode.names.get(name)
 
     def qualified_name(self, n: str) -> str:
         if self.type is not None:
@@ -3861,6 +3896,8 @@ def is_module_scope(self) -> bool:
 
     def add_symbol(self, name: str, node: SymbolTableNode,
                    context: Context) -> None:
+        # NOTE: This logic mostly parallels SemanticAnalyzerPass1.add_symbol. If you change
+        #     this, you may have to change the other method as well.
         if self.is_func_scope():
             assert self.locals[-1] is not None
             if name in self.locals[-1]:
@@ -3873,8 +3910,10 @@ def add_symbol(self, name: str, node: SymbolTableNode,
             self.type.names[name] = node
         else:
             existing = self.globals.get(name)
-            if existing and (not isinstance(node.node, MypyFile) or
-                             existing.node != node.node) and existing.kind != UNBOUND_IMPORTED:
+            if (existing
+                    and (not isinstance(node.node, MypyFile) or existing.node != node.node)
+                    and existing.kind != UNBOUND_IMPORTED
+                    and not isinstance(existing.node, ImportedName)):
                 # Modules can be imported multiple times to support import
                 # of multiple submodules of a package (e.g. a.x and a.y).
                 ok = False

mypy/semanal_pass1.py

@@ -23,14 +23,16 @@
 from mypy.nodes import (
     MypyFile, SymbolTable, SymbolTableNode, Var, Block, AssignmentStmt, FuncDef, Decorator,
     ClassDef, TypeInfo, ImportFrom, Import, ImportAll, IfStmt, WhileStmt, ForStmt, WithStmt,
-    TryStmt, OverloadedFuncDef, Lvalue, Context, LDEF, GDEF, MDEF, UNBOUND_IMPORTED, MODULE_REF,
-    implicit_module_attrs
+    TryStmt, OverloadedFuncDef, Lvalue, Context, ImportedName, LDEF, GDEF, MDEF, UNBOUND_IMPORTED,
+    MODULE_REF, implicit_module_attrs
 )
 from mypy.types import Type, UnboundType, UnionType, AnyType, TypeOfAny, NoneTyp
 from mypy.semanal import SemanticAnalyzerPass2, infer_reachability_of_if_statement
+from mypy.semanal_shared import create_indirect_imported_name
 from mypy.options import Options
 from mypy.sametypes import is_same_type
 from mypy.visitor import NodeVisitor
+from mypy.util import correct_relative_import
 
 
 class SemanticAnalyzerPass1(NodeVisitor[None]):
@@ -62,6 +64,7 @@ def visit_file(self, file: MypyFile, fnam: str, mod_id: str, options: Options) -
         self.pyversion = options.python_version
         self.platform = options.platform
         sem.cur_mod_id = mod_id
+        sem.cur_mod_node = file
         sem.errors.set_file(fnam, mod_id)
         sem.globals = SymbolTable()
         sem.global_decls = [set()]
@@ -148,7 +151,8 @@ def visit_func_def(self, func: FuncDef) -> None:
         if at_module and func.name() in sem.globals:
             # Already defined in this module.
             original_sym = sem.globals[func.name()]
-            if original_sym.kind == UNBOUND_IMPORTED:
+            if (original_sym.kind == UNBOUND_IMPORTED or
+                    isinstance(original_sym.node, ImportedName)):
                 # Ah this is an imported name. We can't resolve them now, so we'll postpone
                 # this until the main phase of semantic analysis.
                 return
@@ -241,7 +245,12 @@ def visit_import_from(self, node: ImportFrom) -> None:
         for name, as_name in node.names:
             imported_name = as_name or name
             if imported_name not in self.sem.globals:
-                self.add_symbol(imported_name, SymbolTableNode(UNBOUND_IMPORTED, None), node)
+                sym = create_indirect_imported_name(self.sem.cur_mod_node,
+                                                    node.id,
+                                                    node.relative,
+                                                    name)
+                if sym:
+                    self.add_symbol(imported_name, sym, context=node)
 
     def visit_import(self, node: Import) -> None:
         node.is_top_level = self.sem.is_module_scope()
@@ -312,8 +321,9 @@ def kind_by_scope(self) -> int:
 
     def add_symbol(self, name: str, node: SymbolTableNode,
                    context: Context) -> None:
-        # This is related to SemanticAnalyzerPass2.add_symbol. Since both methods will
-        # be called on top-level definitions, they need to co-operate.
+        # NOTE: This is closely related to SemanticAnalyzerPass2.add_symbol. Since both methods
+        #     will be called on top-level definitions, they need to co-operate. If you change
+        #     this, you may have to change the other method as well.
         if self.sem.is_func_scope():
             assert self.sem.locals[-1] is not None
             if name in self.sem.locals[-1]:
@@ -325,8 +335,10 @@ def add_symbol(self, name: str, node: SymbolTableNode,
         else:
             assert self.sem.type is None  # Pass 1 doesn't look inside classes
             existing = self.sem.globals.get(name)
-            if existing and (not isinstance(node.node, MypyFile) or
-                             existing.node != node.node) and existing.kind != UNBOUND_IMPORTED:
+            if (existing
+                    and (not isinstance(node.node, MypyFile) or existing.node != node.node)
+                    and existing.kind != UNBOUND_IMPORTED
+                    and not isinstance(existing.node, ImportedName)):
                 # Modules can be imported multiple times to support import
                 # of multiple submodules of a package (e.g. a.x and a.y).
                 ok = False