From f653c143e47e9f61172881bf9dab82ac1a5f25af Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Tue, 18 Sep 2018 16:25:03 +0100 Subject: [PATCH 01/10] Add tests --- test-data/unit/check-type-aliases.test | 50 ++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/test-data/unit/check-type-aliases.test b/test-data/unit/check-type-aliases.test index 44377d584cd8..51d2da1673c4 100644 --- a/test-data/unit/check-type-aliases.test +++ b/test-data/unit/check-type-aliases.test @@ -427,6 +427,56 @@ 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 testNamedTupleImportCycle] +import b +[file a.py] +class C: + pass + +from b import tp +x: tp + +[file b.py] +from a import C +from typing import NamedTuple + +tp = NamedTuple('tp', [('x', int)]) +[out] + [case testFlexibleAlias1] from typing import TypeVar, List, Tuple from mypy_extensions import FlexibleAlias From fc0e34472e4d0c3a4a04beebab4677aba2750cfb Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Tue, 18 Sep 2018 17:28:43 +0100 Subject: [PATCH 02/10] Update imported variables that turned into types or aliases --- mypy/semanal_pass3.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/mypy/semanal_pass3.py b/mypy/semanal_pass3.py index c00da4d45f99..7b805c064bfa 100644 --- a/mypy/semanal_pass3.py +++ b/mypy/semanal_pass3.py @@ -67,12 +67,25 @@ 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: + for sym in self.cur_mod_node.names.values(): + if sym and isinstance(sym.node, Var): + var = sym.node + mod_name, _, name = var.fullname().rpartition('.') + if mod_name != self.sem.cur_mod_id: # imported + new_sym = self.sem.modules[mod_name].names.get(name) + if 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, FuncItem, OverloadedFuncDef], patches: List[Tuple[int, Callable[[], None]]]) -> None: """Refresh a stale target in fine-grained incremental mode.""" From 0b85239cb14859209352c463cf85c7e1acba9656 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Tue, 18 Sep 2018 17:42:05 +0100 Subject: [PATCH 03/10] Skip missing modules --- mypy/semanal_pass3.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/mypy/semanal_pass3.py b/mypy/semanal_pass3.py index 7b805c064bfa..e366dcd258aa 100644 --- a/mypy/semanal_pass3.py +++ b/mypy/semanal_pass3.py @@ -77,8 +77,12 @@ def visit_file(self, file_node: MypyFile, fnam: str, options: Options, def update_imported_vars(self) -> None: for sym in self.cur_mod_node.names.values(): if sym and isinstance(sym.node, Var): - var = sym.node - mod_name, _, name = var.fullname().rpartition('.') + 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 isinstance(new_sym.node, (TypeInfo, TypeAlias)): From e1558f93df1ba460000603e395ee2673773be453 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Tue, 18 Sep 2018 18:02:41 +0100 Subject: [PATCH 04/10] More tests --- test-data/unit/check-incremental.test | 44 ++++++++++++++++++++++++++ test-data/unit/check-namedtuple.test | 17 ++++++++++ test-data/unit/check-type-aliases.test | 16 ---------- test-data/unit/check-typeddict.test | 18 +++++++++++ 4 files changed, 79 insertions(+), 16 deletions(-) diff --git a/test-data/unit/check-incremental.test b/test-data/unit/check-incremental.test index a902a25d5ed3..5792e39adcf7 100644 --- a/test-data/unit/check-incremental.test +++ b/test-data/unit/check-incremental.test @@ -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 diff --git a/test-data/unit/check-namedtuple.test b/test-data/unit/check-namedtuple.test index 40cd1d10993b..e64d56a5ec26 100644 --- a/test-data/unit/check-namedtuple.test +++ b/test-data/unit/check-namedtuple.test @@ -691,6 +691,23 @@ 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' + +[file b.py] +from a import C +from typing import NamedTuple + +tp = NamedTuple('tp', [('x', int)]) +[out] + [case testUnsafeOverlappingNamedTuple] from typing import NamedTuple diff --git a/test-data/unit/check-type-aliases.test b/test-data/unit/check-type-aliases.test index 51d2da1673c4..8d4eaedcef1e 100644 --- a/test-data/unit/check-type-aliases.test +++ b/test-data/unit/check-type-aliases.test @@ -461,22 +461,6 @@ class Parameter: type: _ConvertibleType [out] -[case testNamedTupleImportCycle] -import b -[file a.py] -class C: - pass - -from b import tp -x: tp - -[file b.py] -from a import C -from typing import NamedTuple - -tp = NamedTuple('tp', [('x', int)]) -[out] - [case testFlexibleAlias1] from typing import TypeVar, List, Tuple from mypy_extensions import FlexibleAlias diff --git a/test-data/unit/check-typeddict.test b/test-data/unit/check-typeddict.test index e989a67965a6..5295b19148e7 100644 --- a/test-data/unit/check-typeddict.test +++ b/test-data/unit/check-typeddict.test @@ -1375,3 +1375,21 @@ 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' + +[file b.py] +from a import C +from mypy_extensions import TypedDict + +tp = TypedDict('tp', {'x': int}) +[builtins fixtures/dict.pyi] +[out] From 8588d4f79acdb6314f61598ec3fb625cfc2ee710 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Tue, 18 Sep 2018 18:06:33 +0100 Subject: [PATCH 05/10] Fix self-check --- mypy/semanal_pass3.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/semanal_pass3.py b/mypy/semanal_pass3.py index e366dcd258aa..008145d4fd05 100644 --- a/mypy/semanal_pass3.py +++ b/mypy/semanal_pass3.py @@ -85,7 +85,7 @@ def update_imported_vars(self) -> None: continue if mod_name != self.sem.cur_mod_id: # imported new_sym = self.sem.modules[mod_name].names.get(name) - if isinstance(new_sym.node, (TypeInfo, TypeAlias)): + 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 From 6f35bef0c2f24dbde17c12a75cf97b791b6e1757 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Thu, 20 Sep 2018 20:55:34 +0100 Subject: [PATCH 06/10] Add detailed docstring --- mypy/semanal_pass3.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/mypy/semanal_pass3.py b/mypy/semanal_pass3.py index 008145d4fd05..3957be3bbbbf 100644 --- a/mypy/semanal_pass3.py +++ b/mypy/semanal_pass3.py @@ -75,6 +75,32 @@ def visit_file(self, file_node: MypyFile, fnam: str, options: Options, 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 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. + """ for sym in self.cur_mod_node.names.values(): if sym and isinstance(sym.node, Var): fullname = sym.node.fullname() From 5fbe0c6af2a4c6b0a6e71d2e60c3f6698692acb8 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Thu, 20 Sep 2018 21:35:18 +0100 Subject: [PATCH 07/10] Add test for runtime behavior --- mypy/semanal_pass3.py | 4 ++-- test-data/unit/check-namedtuple.test | 5 +++++ test-data/unit/check-type-aliases.test | 20 ++++++++++++++++++++ test-data/unit/check-typeddict.test | 5 +++++ 4 files changed, 32 insertions(+), 2 deletions(-) diff --git a/mypy/semanal_pass3.py b/mypy/semanal_pass3.py index 3957be3bbbbf..ae92239eba46 100644 --- a/mypy/semanal_pass3.py +++ b/mypy/semanal_pass3.py @@ -77,7 +77,7 @@ def visit_file(self, file_node: MypyFile, fnam: str, options: Options, 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 + 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: @@ -99,7 +99,7 @@ def update_imported_vars(self) -> None: 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. + from performance point of view, and still nontrivial. """ for sym in self.cur_mod_node.names.values(): if sym and isinstance(sym.node, Var): diff --git a/test-data/unit/check-namedtuple.test b/test-data/unit/check-namedtuple.test index e64d56a5ec26..0db9005e7d70 100644 --- a/test-data/unit/check-namedtuple.test +++ b/test-data/unit/check-namedtuple.test @@ -701,6 +701,11 @@ from b import tp x: tp reveal_type(x.x) # E: Revealed type is 'builtins.int' +# Unfortunately runtime part doesn't work yet, see comment 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 diff --git a/test-data/unit/check-type-aliases.test b/test-data/unit/check-type-aliases.test index 8d4eaedcef1e..b7593a16b3d5 100644 --- a/test-data/unit/check-type-aliases.test +++ b/test-data/unit/check-type-aliases.test @@ -461,6 +461,26 @@ 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 comment 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 diff --git a/test-data/unit/check-typeddict.test b/test-data/unit/check-typeddict.test index 5295b19148e7..3172f0bf3edc 100644 --- a/test-data/unit/check-typeddict.test +++ b/test-data/unit/check-typeddict.test @@ -1386,6 +1386,11 @@ from b import tp x: tp reveal_type(x['x']) # E: Revealed type is 'builtins.int' +# Unfortunately runtime part doesn't work yet, see comment 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 From 34083536418f32b72daa060c9007edfd14ee3b9c Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Thu, 20 Sep 2018 21:37:22 +0100 Subject: [PATCH 08/10] Typo --- test-data/unit/check-namedtuple.test | 2 +- test-data/unit/check-type-aliases.test | 2 +- test-data/unit/check-typeddict.test | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test-data/unit/check-namedtuple.test b/test-data/unit/check-namedtuple.test index 0db9005e7d70..4061078a5038 100644 --- a/test-data/unit/check-namedtuple.test +++ b/test-data/unit/check-namedtuple.test @@ -701,7 +701,7 @@ from b import tp x: tp reveal_type(x.x) # E: Revealed type is 'builtins.int' -# Unfortunately runtime part doesn't work yet, see comment in SemanticAnalyzerPass3.update_imported_vars() +# 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' diff --git a/test-data/unit/check-type-aliases.test b/test-data/unit/check-type-aliases.test index b7593a16b3d5..731763edc004 100644 --- a/test-data/unit/check-type-aliases.test +++ b/test-data/unit/check-type-aliases.test @@ -470,7 +470,7 @@ if MYPY: x: A reveal_type(x) # E: Revealed type is 't2.D' -# Unfortunately runtime part doesn't work yet, see comment in SemanticAnalyzerPass3.update_imported_vars() +# 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' diff --git a/test-data/unit/check-typeddict.test b/test-data/unit/check-typeddict.test index 3172f0bf3edc..41eb700c41c4 100644 --- a/test-data/unit/check-typeddict.test +++ b/test-data/unit/check-typeddict.test @@ -1386,7 +1386,7 @@ from b import tp x: tp reveal_type(x['x']) # E: Revealed type is 'builtins.int' -# Unfortunately runtime part doesn't work yet, see comment in SemanticAnalyzerPass3.update_imported_vars() +# 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' From dcbab2c76f610d5d250e6402f0ff172880aadd6c Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Thu, 20 Sep 2018 21:51:48 +0100 Subject: [PATCH 09/10] Fix merge error --- test-data/unit/check-namedtuple.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-data/unit/check-namedtuple.test b/test-data/unit/check-namedtuple.test index 4d7bdfef4d1c..cf3a348dacce 100644 --- a/test-data/unit/check-namedtuple.test +++ b/test-data/unit/check-namedtuple.test @@ -700,7 +700,6 @@ class C: from b import tp x: tp reveal_type(x.x) # E: Revealed type is 'builtins.int' -[out] # Unfortunately runtime part doesn't work yet, see docstring in SemanticAnalyzerPass3.update_imported_vars() reveal_type(tp) # E: Revealed type is 'Any' \ @@ -712,6 +711,7 @@ from a import C from typing import NamedTuple tp = NamedTuple('tp', [('x', int)]) +[out] [case testSubclassOfRecursiveNamedTuple] from typing import List, NamedTuple From 2bec99996d9d4b989c1437a79e8365d6d3031fb9 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Fri, 21 Sep 2018 16:43:13 +0100 Subject: [PATCH 10/10] Fix typos --- mypy/semanal_pass3.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mypy/semanal_pass3.py b/mypy/semanal_pass3.py index bc769a8faac2..5086eebeba5e 100644 --- a/mypy/semanal_pass3.py +++ b/mypy/semanal_pass3.py @@ -83,11 +83,11 @@ def update_imported_vars(self) -> None: A = List[int] - Are seen by mypy as variables, because it doesn't know yet that `List` refers to a type. + 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 a partial fix that only fixes the "Invalid type" error when a type alias + 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.