Skip to content

Update imported Vars in the third pass #5635

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 13 commits into from
Sep 21, 2018
43 changes: 43 additions & 0 deletions mypy/semanal_pass3.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,55 @@ def visit_file(self, file_node: MypyFile, fnam: str, options: Options,
self.sem.globals = file_node.names
with experiments.strict_optional_set(options.strict_optional):
self.scope.enter_file(file_node.fullname())
self.update_imported_vars()
self.accept(file_node)
self.analyze_symbol_table(file_node.names)
self.scope.leave()
del self.cur_mod_node
self.patches = []

def update_imported_vars(self) -> None:
"""Update nodes for imported names, if they got updated from Var to TypeInfo or TypeAlias.

This is a simple _band-aid_ fix for "Invalid type" error in import cycles where type
aliases, named tuples, or typed dicts appear. The root cause is that during first pass
definitions like:

A = List[int]

are seen by mypy as variables, because it doesn't know yet that `List` refers to a type.
In the second pass, such `Var` is replaced with a `TypeAlias`. But in import cycle,
import of `A` will still refer to the old `Var` node. Therefore we need to update it.

Note that this is a partial fix that only fixes the "Invalid type" error when a type alias
etc. appears in type context. This doesn't fix errors (e.g. "Cannot determine type of A")
that may appear if the type alias etc. appear in runtime context.

The motivation for partial fix is two-fold:
* The "Invalid type" error often appears in stub files (especially for large
libraries/frameworks) where we have more import cycles, but no runtime
context at all.
* Ideally we should refactor semantic analysis to have deferred nodes, and process
them in smaller passes when there is more info (like we do in type checking phase).
But this is _much_ harder since this requires a large refactoring. Also an alternative
fix of updating node of every `NameExpr` and `MemberExpr` in third pass is costly
from performance point of view, and still nontrivial.
"""
for sym in self.cur_mod_node.names.values():
if sym and isinstance(sym.node, Var):
fullname = sym.node.fullname()
if '.' not in fullname:
continue
mod_name, _, name = fullname.rpartition('.')
if mod_name not in self.sem.modules:
continue
if mod_name != self.sem.cur_mod_id: # imported
new_sym = self.sem.modules[mod_name].names.get(name)
if new_sym and isinstance(new_sym.node, (TypeInfo, TypeAlias)):
# This Var was replaced with a class (like named tuple)
# or alias, update this.
sym.node = new_sym.node

def refresh_partial(self, node: Union[MypyFile, FuncDef, OverloadedFuncDef],
patches: List[Tuple[int, Callable[[], None]]]) -> None:
"""Refresh a stale target in fine-grained incremental mode."""
Expand Down
44 changes: 44 additions & 0 deletions test-data/unit/check-incremental.test
Original file line number Diff line number Diff line change
Expand Up @@ -5123,6 +5123,50 @@ def outer() -> None:
[out]
[out2]

[case testRecursiveAliasImported]
import a
[file a.py]
import lib
x: int
[file a.py.2]
import lib
x: lib.A
reveal_type(x)
[file lib.pyi]
from typing import List
from other import B
A = List[B] # type: ignore
[file other.pyi]
from typing import List
from lib import A
B = List[A]
[builtins fixtures/list.pyi]
[out]
[out2]
tmp/a.py:3: error: Revealed type is 'builtins.list[builtins.list[builtins.list[Any]]]'

[case testRecursiveNamedTupleTypedDict]
import a
[file a.py]
import lib
x: int
[file a.py.2]
import lib
x: lib.A
reveal_type(x.x['x'])
[file lib.pyi]
from typing import NamedTuple
from other import B
A = NamedTuple('A', [('x', B)]) # type: ignore
[file other.pyi]
from mypy_extensions import TypedDict
from lib import A
B = TypedDict('B', {'x': A})
[builtins fixtures/dict.pyi]
[out]
[out2]
tmp/a.py:3: error: Revealed type is 'Tuple[TypedDict('other.B', {'x': Any}), fallback=lib.A]'

[case testFollowImportSkipNotInvalidatedOnPresent]
# flags: --follow-imports=skip
# cmd: mypy -m main
Expand Down
22 changes: 22 additions & 0 deletions test-data/unit/check-namedtuple.test
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,28 @@ my_eval(A([B(1), B(2)])) # OK
[builtins fixtures/isinstancelist.pyi]
[out]

[case testNamedTupleImportCycle]
import b
[file a.py]
class C:
pass

from b import tp
x: tp
reveal_type(x.x) # E: Revealed type is 'builtins.int'
Copy link
Collaborator

Choose a reason for hiding this comment

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

What if you have tp('x') in this file? Or reveal_type(tp)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this will not work. I was thinking about the Invalid type problem. All references in type context will be correctly re-resolved in the third pass. While there is no such thing as forward reference in runtime context.
I however still think this fix may have some value:

  • We have seen many complains about Invalid type, but few complains about Cannot determine type (the error that appears if you try tp('x'))
  • The reason for above fact is that many of the import cycle exist only for annotations, tp('x') will fail at runtime anyway
  • This error quite often appears in stub files for large libraries, where we only care about type context

It is up to you of course.


# Unfortunately runtime part doesn't work yet, see docstring in SemanticAnalyzerPass3.update_imported_vars()
reveal_type(tp) # E: Revealed type is 'Any' \
# E: Cannot determine type of 'tp'
tp('x') # E: Cannot determine type of 'tp'

[file b.py]
from a import C
from typing import NamedTuple

tp = NamedTuple('tp', [('x', int)])
[out]

[case testSubclassOfRecursiveNamedTuple]
from typing import List, NamedTuple

Expand Down
54 changes: 54 additions & 0 deletions test-data/unit/check-type-aliases.test
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,60 @@ reveal_type(D().meth(1)) # E: Revealed type is 'Union[__main__.D*, builtins.int
[builtins fixtures/classmethod.pyi]
[out]

[case testAliasInImportCycle]
# cmd: mypy -m t t2
[file t.py]
MYPY = False
if MYPY:
from t2 import A
x: A
[file t2.py]
import t
from typing import Callable
A = Callable[[], None]
[builtins fixtures/bool.pyi]
[out]

[case testAliasInImportCycle2]
import a
[file a.pyi]
from b import Parameter

class _ParamType:
p: Parameter

_ConvertibleType = _ParamType

def convert_type(ty: _ConvertibleType):
...

[file b.pyi]
from a import _ConvertibleType

class Parameter:
type: _ConvertibleType
[out]

[case testAliasInImportCycle3]
# cmd: mypy -m t t2
[file t.py]
MYPY = False
if MYPY:
from t2 import A
x: A
reveal_type(x) # E: Revealed type is 't2.D'

# Unfortunately runtime part doesn't work yet, see docstring in SemanticAnalyzerPass3.update_imported_vars()
reveal_type(A) # E: Revealed type is 'Any' \
# E: Cannot determine type of 'A'
A() # E: Cannot determine type of 'A'
[file t2.py]
import t
class D: pass
A = D
[builtins fixtures/bool.pyi]
[out]

[case testFlexibleAlias1]
from typing import TypeVar, List, Tuple
from mypy_extensions import FlexibleAlias
Expand Down
23 changes: 23 additions & 0 deletions test-data/unit/check-typeddict.test
Original file line number Diff line number Diff line change
Expand Up @@ -1390,3 +1390,26 @@ def f(x: a.N) -> None:
[out]
tmp/b.py:4: error: Revealed type is 'TypedDict('a.N', {'a': builtins.str})'
tmp/b.py:5: error: Revealed type is 'builtins.str'

[case testTypedDictImportCycle]
import b
[file a.py]
class C:
pass

from b import tp
x: tp
reveal_type(x['x']) # E: Revealed type is 'builtins.int'

# Unfortunately runtime part doesn't work yet, see docstring in SemanticAnalyzerPass3.update_imported_vars()
reveal_type(tp) # E: Revealed type is 'Any' \
# E: Cannot determine type of 'tp'
tp('x') # E: Cannot determine type of 'tp'

[file b.py]
from a import C
from mypy_extensions import TypedDict

tp = TypedDict('tp', {'x': int})
[builtins fixtures/dict.pyi]
[out]