Skip to content

Don't error on re-exported imports in cycles. Fixes #4049. #4495

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

Closed
wants to merge 17 commits into from
Closed
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
102 changes: 56 additions & 46 deletions mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -1489,6 +1489,7 @@ def visit_import_from(self, imp: ImportFrom) -> None:
for id, as_id in imp.names:
node = module.names.get(id) if module else None
missing = False
is_same_module = import_id == self.cur_mod_id
possible_module_id = import_id + '.' + id

# If the module does not contain a symbol with the name 'id',
Expand All @@ -1497,6 +1498,7 @@ def visit_import_from(self, imp: ImportFrom) -> None:
mod = self.modules.get(possible_module_id)
if mod is not None:
node = SymbolTableNode(MODULE_REF, mod)
is_same_module = False
self.add_submodules_to_parent_modules(possible_module_id, True)
elif possible_module_id in self.missing_modules:
missing = True
Expand All @@ -1517,7 +1519,12 @@ def visit_import_from(self, imp: ImportFrom) -> None:
symbol = SymbolTableNode(GDEF, ast_node)
self.add_symbol(name, symbol, imp)
return
if node and node.kind != UNBOUND_IMPORTED and not node.module_hidden:

# If we found the node being imported, and it's not in a hidden
# module, nor a recursive self-import (which should be an import
# error), go ahead and update our local symbol table.
if node and not node.module_hidden and not (
node.kind == UNBOUND_IMPORTED and is_same_module):
node = self.normalize_type_alias(node, imp)
if not node:
return
Expand Down Expand Up @@ -3398,51 +3405,7 @@ def visit_member_expr(self, expr: MemberExpr) -> None:
base.accept(self)
# Bind references to module attributes.
if isinstance(base, RefExpr) and base.kind == MODULE_REF:
# This branch handles the case foo.bar where foo is a module.
# In this case base.node is the module's MypyFile and we look up
# bar in its namespace. This must be done for all types of bar.
file = cast(Optional[MypyFile], base.node) # can't use isinstance due to issue #2999
# TODO: Should we actually use this? Not sure if this makes a difference.
# if file.fullname() == self.cur_mod_id:
# names = self.globals
# else:
# names = file.names
n = file.names.get(expr.name, None) if file is not None else None
if n and not n.module_hidden:
n = self.normalize_type_alias(n, expr)
if not n:
return
n = self.rebind_symbol_table_node(n)
if n:
# TODO: What if None?
expr.kind = n.kind
expr.fullname = n.fullname
expr.node = n.node
elif file is not None and file.is_stub and '__getattr__' in file.names:
# If there is a module-level __getattr__, then any attribute on the module is valid
# per PEP 484.
getattr_defn = file.names['__getattr__']
if isinstance(getattr_defn.node, FuncDef):
if isinstance(getattr_defn.node.type, CallableType):
typ = getattr_defn.node.type.ret_type
else:
typ = AnyType(TypeOfAny.special_form)
expr.kind = MDEF
expr.fullname = '{}.{}'.format(file.fullname(), expr.name)
expr.node = Var(expr.name, type=typ)
else:
# We only catch some errors here; the rest will be
# caught during type checking.
#
# This way we can report a larger number of errors in
# one type checker run. If we reported errors here,
# the build would terminate after semantic analysis
# and we wouldn't be able to report any type errors.
full_name = '%s.%s' % (file.fullname() if file is not None else None, expr.name)
mod_name = " '%s'" % file.fullname() if file is not None else ''
if full_name in obsolete_name_mapping:
self.fail("Module%s has no attribute %r (it's now called %r)" % (
mod_name, expr.name, obsolete_name_mapping[full_name]), expr)
self.bind_module_attribute_reference(base, expr)
elif isinstance(base, RefExpr):
# This branch handles the case C.bar (or cls.bar or self.bar inside
# a classmethod/method), where C is a class and bar is a type
Expand Down Expand Up @@ -3472,6 +3435,53 @@ def visit_member_expr(self, expr: MemberExpr) -> None:
expr.fullname = n.fullname
expr.node = n.node

def bind_module_attribute_reference(self, base: RefExpr, expr: MemberExpr) -> None:
# This method handles the case foo.bar where foo is a module.
# In this case base.node is the module's MypyFile and we look up
# bar in its namespace. This must be done for all types of bar.
file = cast(Optional[MypyFile], base.node) # can't use isinstance due to issue #2999
# TODO: Should we actually use this? Not sure if this makes a difference.
# if file.fullname() == self.cur_mod_id:
# names = self.globals
# else:
# names = file.names
n = file.names.get(expr.name, None) if file is not None else None
if n and not n.module_hidden:
n = self.normalize_type_alias(n, expr)
if not n:
return
n = self.rebind_symbol_table_node(n)
if n:
# TODO: What if None?
expr.kind = n.kind
expr.fullname = n.fullname
expr.node = n.node
elif file is not None and file.is_stub and '__getattr__' in file.names:
# If there is a module-level __getattr__, then any attribute on the module is valid
# per PEP 484.
getattr_defn = file.names['__getattr__']
if isinstance(getattr_defn.node, FuncDef):
if isinstance(getattr_defn.node.type, CallableType):
typ = getattr_defn.node.type.ret_type
else:
typ = AnyType(TypeOfAny.special_form)
expr.kind = MDEF
expr.fullname = '{}.{}'.format(file.fullname(), expr.name)
expr.node = Var(expr.name, type=typ)
else:
# We only catch some errors here; the rest will be
# caught during type checking.
#
# This way we can report a larger number of errors in
# one type checker run. If we reported errors here,
# the build would terminate after semantic analysis
# and we wouldn't be able to report any type errors.
full_name = '%s.%s' % (file.fullname() if file is not None else None, expr.name)
mod_name = " '%s'" % file.fullname() if file is not None else ''
if full_name in obsolete_name_mapping:
self.fail("Module%s has no attribute %r (it's now called %r)" % (
mod_name, expr.name, obsolete_name_mapping[full_name]), expr)

def visit_op_expr(self, expr: OpExpr) -> None:
expr.left.accept(self)

Expand Down
58 changes: 56 additions & 2 deletions mypy/semanal_pass3.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
Node, Expression, MypyFile, FuncDef, FuncItem, Decorator, RefExpr, Context, TypeInfo, ClassDef,
Block, TypedDictExpr, NamedTupleExpr, AssignmentStmt, IndexExpr, TypeAliasExpr, NameExpr,
CallExpr, NewTypeExpr, ForStmt, WithStmt, CastExpr, TypeVarExpr, TypeApplication, Lvalue,
TupleExpr, RevealTypeExpr, SymbolTableNode, Var, ARG_POS, OverloadedFuncDef
TupleExpr, RevealTypeExpr, SymbolTableNode, Var, ARG_POS, OverloadedFuncDef, ImportFrom,
MemberExpr, UNBOUND_IMPORTED, MODULE_REF
)
from mypy.types import (
Type, Instance, AnyType, TypeOfAny, CallableType, TupleType, TypeVarType, TypedDictType,
Expand Down Expand Up @@ -62,13 +63,14 @@ def visit_file(self, file_node: MypyFile, fnam: str, options: Options,
self.patches = patches
self.is_typeshed_file = self.errors.is_typeshed_file(fnam)
self.sem.cur_mod_id = file_node.fullname()
self.cur_mod_node = file_node
self.sem.cur_mod_node = self.cur_mod_node = file_node
self.sem.globals = file_node.names
with experiments.strict_optional_set(options.strict_optional):
self.scope.enter_file(file_node.fullname())
self.accept(file_node)
self.scope.leave()
del self.cur_mod_node
del self.sem.cur_mod_node
self.patches = []

def refresh_partial(self, node: Union[MypyFile, FuncItem, OverloadedFuncDef],
Expand Down Expand Up @@ -219,6 +221,14 @@ def visit_assignment_stmt(self, s: AssignmentStmt) -> None:
resulted from this assignment (if any). Currently this includes
NewType, TypedDict, NamedTuple, and TypeVar.
"""
if (isinstance(s.type, AnyType) and
s.type.type_of_any == TypeOfAny.from_unbound_import and
len(s.lvalues) == 1 and
isinstance(s.lvalues[0], NameExpr)):
node = self.sem.globals.get(s.lvalues[0].name)
if node and isinstance(node.node, Var) and s.unanalyzed_type:
s.type = self.sem.anal_type(s.unanalyzed_type, third_pass=True)
node.node.type = s.type
self.analyze(s.type, s)
if isinstance(s.rvalue, IndexExpr) and isinstance(s.rvalue.analyzed, TypeAliasExpr):
self.analyze(s.rvalue.analyzed.type, s.rvalue.analyzed, warn=True)
Expand Down Expand Up @@ -276,6 +286,50 @@ def visit_type_application(self, e: TypeApplication) -> None:
self.analyze(type, e)
super().visit_type_application(e)

def visit_import_from(self, imp: ImportFrom) -> None:
import_id = self.sem.correct_relative_import(imp)
module = self.modules.get(import_id)
if not module:
return
for id, as_id in imp.names:
my_node = self.sem.globals.get(as_id or id)
src_node = module.names.get(id)
# Fixup remaining UNBOUND_IMPORTED nodes from import cycles
if my_node and src_node and my_node.kind == UNBOUND_IMPORTED:
my_node.kind = src_node.kind
my_node.node = src_node.node
my_node.type_override = src_node.type_override
my_node.normalized = src_node.normalized
my_node.alias_tvars = src_node.alias_tvars

def visit_name_expr(self, expr: NameExpr) -> None:
# Fixup remaining UNBOUND_IMPORTED nodes from import cycles
if expr.kind == UNBOUND_IMPORTED:
# TODO: this works only for module-level attributes, because not all
# the namespace context is set up correctly in pass 3.
n = self.sem.lookup(expr.name, expr, suppress_errors=True)
if n:
expr.kind = n.kind
expr.node = n.node
expr.fullname = n.fullname

def visit_member_expr(self, expr: MemberExpr) -> None:
# Fixup remaining UNBOUND_IMPORTED from import cycles
base = expr.expr
base.accept(self)
if isinstance(base, RefExpr) and base.kind == MODULE_REF:
if isinstance(base, NameExpr) and expr.kind == UNBOUND_IMPORTED:
module_id = base.name
module = self.modules.get(module_id)
node = module and module.names.get(expr.name)
if node:
expr.kind = node.kind
expr.node = node.node
expr.fullname = node.fullname
elif expr.kind is None:
self.sem.bind_module_attribute_reference(base, expr)
super().visit_member_expr(expr)

# Helpers

def perform_transform(self, node: Union[Node, SymbolTableNode],
Expand Down
2 changes: 1 addition & 1 deletion mypy/typeanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ def visit_unbound_type(self, t: UnboundType) -> Type:
# UNBOUND_IMPORTED can happen if an unknown name was imported.
if sym.kind != UNBOUND_IMPORTED:
self.fail('Internal error (node is None, kind={})'.format(sym.kind), t)
return AnyType(TypeOfAny.special_form)
return AnyType(TypeOfAny.from_unbound_import)
fullname = sym.node.fullname()
hook = self.plugin.get_type_analyze_hook(fullname)
if hook:
Expand Down
2 changes: 2 additions & 0 deletions mypy/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,8 @@ class TypeOfAny(Enum):
special_form = 'special_form'
# Does this Any come from interaction with another Any?
from_another_any = 'from_another_any'
# Does this Any come from an unresolved unbound import (e.g. from import cycle)?
from_unbound_import = 'from_unbound_import'


class AnyType(Type):
Expand Down
23 changes: 23 additions & 0 deletions test-data/unit/check-incremental.test
Original file line number Diff line number Diff line change
Expand Up @@ -4127,3 +4127,26 @@ class Foo:
x = Foo()
[out]
[out2]

[case testImportReExportInCycle]
from m import One
[file m/__init__.py]
from .one import One
from .two import Two
[file m/one.py]
class One:
pass
[file m/two.py]
import m
class Two:
pass
[file m/one.py.2]
class One:
name: str
[file m/two.py.2]
import m
reveal_type(m.One.name)
class Two:
pass
[out2]
tmp/m/two.py:2: error: Revealed type is 'builtins.str'
82 changes: 82 additions & 0 deletions test-data/unit/check-modules.test
Original file line number Diff line number Diff line change
Expand Up @@ -1955,3 +1955,85 @@ import c
tmp/b.py:1: error: Cannot find module named 'c'
main:1: error: Cannot find module named 'c'
main:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help)

[case testImportFromReExportInCycle]
from m import One
reveal_type(One)
[file m/__init__.py]
from .one import One
from .two import Two
reveal_type(One)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Again, it would be nice to test how all these references work with type aliases and named tuples, for example. (In separate tese cases.)

[file m/one.py]
class One:
pass
[file m/two.py]
from m import One
reveal_type(One)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Note that references to One in annotations still don't work correctly:

from m import One
o: One
reveal_type(o)  # Any

Similarly, if we make One a type alias and use it in an annotation, it will be treated as Any.

If we create a subclass of One in m.two, it seems to have an implicit Any base class, which is unexpected.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catches! I think these are probably fixable by adding more third-pass visits to different node types to propagate type info that we missed in second pass due to the cycle, but it does begin to feel a bit like a game of whack-a-mole.

x: One
reveal_type(x)
# TODO also resolve One correctly when used as base class (currently resolves as Any)
class Two:
pass
[out]
tmp/m/two.py:2: error: Revealed type is 'def () -> m.one.One'
tmp/m/two.py:4: error: Revealed type is 'm.one.One'
tmp/m/__init__.py:3: error: Revealed type is 'def () -> m.one.One'
main:2: error: Revealed type is 'def () -> m.one.One'

[case testImportReExportInCycle]
from m import One
reveal_type(One)
[file m/__init__.py]
from .one import One
from .two import Two
reveal_type(One)
[file m/one.py]
class One:
pass
[file m/two.py]
import m
reveal_type(m.One)
x: m.One
reveal_type(x)
class Two:
pass
[out]
tmp/m/two.py:2: error: Revealed type is 'def () -> m.one.One'
tmp/m/two.py:4: error: Revealed type is 'm.one.One'
tmp/m/__init__.py:3: error: Revealed type is 'def () -> m.one.One'
main:2: error: Revealed type is 'def () -> m.one.One'

[case testImportReExportedNamedTupleInCycle]
from m import One
[file m/__init__.py]
from .one import One
from .two import Two
[file m/one.py]
from typing import NamedTuple
class One(NamedTuple):
name: str
[file m/two.py]
import m
x = m.One(name="Foo")
reveal_type(x.name)
class Two:
pass
[out]
tmp/m/two.py:3: error: Revealed type is 'builtins.str'

[case testImportReExportedTypeAliasInCycle]
from m import One
[file m/__init__.py]
from .one import One
from .two import Two
[file m/one.py]
from typing import Union
One = Union[int, str]
[file m/two.py]
import m
x: m.One
reveal_type(x)
class Two:
pass
[out]
tmp/m/two.py:3: error: Revealed type is 'Union[builtins.int, builtins.str]'