From 3c39bd052ab1b4a814b6ab46645d24ce4d4e799b Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 16 Jun 2017 15:28:35 +0100 Subject: [PATCH 1/3] Fix crash with forward reference in TypedDict Move join to happen after semantic analysis to avoid crash due to incomplete MRO in TypeInfo. The implementation uses a new semantic analysis 'fixup' phase. Fixes #3319. --- mypy/build.py | 12 ++++++++++-- mypy/semanal.py | 22 +++++++++++++++++++--- mypy/server/update.py | 1 + test-data/unit/check-typeddict.test | 26 ++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index e4b202c4e72b..cc5425ca5054 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -21,7 +21,7 @@ from os.path import dirname, basename from typing import (AbstractSet, Dict, Iterable, Iterator, List, - NamedTuple, Optional, Set, Tuple, Union) + NamedTuple, Optional, Set, Tuple, Union, Callable) # Can't use TYPE_CHECKING because it's not in the Python 3.5.1 stdlib MYPY = False if MYPY: @@ -1601,8 +1601,10 @@ def parse_file(self) -> None: self.check_blockers() def semantic_analysis(self) -> None: + fixups = [] # type: List[Callable[[], None]] with self.wrap_context(): - self.manager.semantic_analyzer.visit_file(self.tree, self.xpath, self.options) + self.manager.semantic_analyzer.visit_file(self.tree, self.xpath, self.options, fixups) + self.fixups = fixups def semantic_analysis_pass_three(self) -> None: with self.wrap_context(): @@ -1610,6 +1612,10 @@ def semantic_analysis_pass_three(self) -> None: if self.options.dump_type_stats: dump_type_stats(self.tree, self.xpath) + def semantic_analysis_fixups(self) -> None: + for fixup in self.fixups: + fixup() + def type_check_first_pass(self) -> None: manager = self.manager if self.options.semantic_analysis_only: @@ -2043,6 +2049,8 @@ def process_stale_scc(graph: Graph, scc: List[str], manager: BuildManager) -> No graph[id].semantic_analysis_pass_three() for id in fresh: graph[id].calculate_mros() + for id in stale: + graph[id].semantic_analysis_fixups() for id in stale: graph[id].type_check_first_pass() more = True diff --git a/mypy/semanal.py b/mypy/semanal.py index 523edc8563e0..aae2366f88e5 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -257,13 +257,20 @@ def __init__(self, self.postponed_functions_stack = [] self.all_exports = set() # type: Set[str] - def visit_file(self, file_node: MypyFile, fnam: str, options: Options) -> None: + def visit_file(self, file_node: MypyFile, fnam: str, options: Options, + fixups: List[Callable[[], None]]) -> None: + """Run semantic analysis phase 2 over a file. + + Add callbacks by mutating the fixups list argument. They will be called + after all semantic analysis phases but before type checking. + """ self.options = options self.errors.set_file(fnam, file_node.fullname()) self.cur_mod_node = file_node self.cur_mod_id = file_node.fullname() self.is_stub_file = fnam.lower().endswith('.pyi') self.globals = file_node.names + self.fixups = fixups if 'builtins' in self.modules: self.globals['__builtins__'] = SymbolTableNode( @@ -290,6 +297,7 @@ def visit_file(self, file_node: MypyFile, fnam: str, options: Options) -> None: g.module_public = False del self.options + del self.fixups def refresh_partial(self, node: Union[MypyFile, FuncItem]) -> None: """Refresh a stale target in fine-grained incremental mode.""" @@ -2366,11 +2374,19 @@ def fail_typeddict_arg(self, message: str, def build_typeddict_typeinfo(self, name: str, items: List[str], types: List[Type]) -> TypeInfo: - mapping_value_type = join.join_type_list(types) fallback = (self.named_type_or_none('typing.Mapping', - [self.str_type(), mapping_value_type]) + [self.str_type(), self.object_type()]) or self.object_type()) + def fixup() -> None: + mapping_value_type = join.join_type_list(types) + fallback.args[1] = mapping_value_type + + # We can't calculate the complete fallback type until after semantic + # analysis, since otherwise MROs might be incomplete. Postpone a fixup + # function that patches the fallback. + self.fixups.append(fixup) + info = self.basic_new_typeinfo(name, fallback) info.typeddict_type = TypedDictType(OrderedDict(zip(items, types)), fallback) diff --git a/mypy/server/update.py b/mypy/server/update.py index f303a99d0efa..b43cb5a66fdc 100644 --- a/mypy/server/update.py +++ b/mypy/server/update.py @@ -147,6 +147,7 @@ def build_incremental_step(manager: BuildManager, # TODO: state.fix_suppressed_dependencies()? state.semantic_analysis() state.semantic_analysis_pass_three() + # TODO: state.semantic_analysis_fixups() state.type_check_first_pass() # TODO: state.type_check_second_pass()? state.finish_passes() diff --git a/test-data/unit/check-typeddict.test b/test-data/unit/check-typeddict.test index c16de82b67e5..ee5217e77a4e 100644 --- a/test-data/unit/check-typeddict.test +++ b/test-data/unit/check-typeddict.test @@ -813,3 +813,29 @@ p = TaggedPoint(type='2d', x=42, y=1337) p.get('x', 1 + 'y') # E: Unsupported operand types for + ("int" and "str") [builtins fixtures/dict.pyi] [typing fixtures/typing-full.pyi] + + +-- Special cases + +[case testForwardReferenceInTypedDict] +from typing import Mapping +from mypy_extensions import TypedDict +X = TypedDict('X', {'b': 'B', 'c': 'C'}) +class B: pass +class C(B): pass +x: X +reveal_type(x) # E: Revealed type is 'TypedDict(b=__main__.B, c=__main__.C, _fallback=__main__.X)' +m1: Mapping[str, B] = x +m2: Mapping[str, C] = x # E: Incompatible types in assignment (expression has type "X", variable has type Mapping[str, C]) +[builtins fixtures/dict.pyi] + +[case testForwardReferenceToTypedDictInTypedDict] +from typing import Mapping +from mypy_extensions import TypedDict +# Forward references don't quite work yet +X = TypedDict('X', {'a': 'A'}) # E: Invalid type "__main__.A" +A = TypedDict('A', {'b': int}) +x: X +reveal_type(x) # E: Revealed type is 'TypedDict(a=TypedDict(b=builtins.int, _fallback=__main__.A), _fallback=__main__.X)' +reveal_type(x['a']['b']) # E: Revealed type is 'builtins.int' +[builtins fixtures/dict.pyi] From 3fb42d2971633d1b8dd6842555d3651b271a865b Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 20 Jun 2017 12:11:28 +0100 Subject: [PATCH 2/3] Add test case based on feedback --- test-data/unit/check-typeddict.test | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test-data/unit/check-typeddict.test b/test-data/unit/check-typeddict.test index ee5217e77a4e..6a16d13bc468 100644 --- a/test-data/unit/check-typeddict.test +++ b/test-data/unit/check-typeddict.test @@ -829,6 +829,20 @@ m1: Mapping[str, B] = x m2: Mapping[str, C] = x # E: Incompatible types in assignment (expression has type "X", variable has type Mapping[str, C]) [builtins fixtures/dict.pyi] +[case testForwardReferenceInClassTypedDict] +from typing import Mapping +from mypy_extensions import TypedDict +class X(TypedDict): + b: 'B' + c: 'C' +class B: pass +class C(B): pass +x: X +reveal_type(x) # E: Revealed type is 'TypedDict(b=__main__.B, c=__main__.C, _fallback=__main__.X)' +m1: Mapping[str, B] = x +m2: Mapping[str, C] = x # E: Incompatible types in assignment (expression has type "X", variable has type Mapping[str, C]) +[builtins fixtures/dict.pyi] + [case testForwardReferenceToTypedDictInTypedDict] from typing import Mapping from mypy_extensions import TypedDict From f6decd93a6183a4b3e1dd72348985ecf50f4e81d Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Wed, 21 Jun 2017 15:03:41 +0100 Subject: [PATCH 3/3] Address review feedback --- mypy/build.py | 14 +++++++------- mypy/semanal.py | 18 +++++++++--------- mypy/server/update.py | 2 +- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index cc5425ca5054..eac1b027667a 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -1601,10 +1601,10 @@ def parse_file(self) -> None: self.check_blockers() def semantic_analysis(self) -> None: - fixups = [] # type: List[Callable[[], None]] + patches = [] # type: List[Callable[[], None]] with self.wrap_context(): - self.manager.semantic_analyzer.visit_file(self.tree, self.xpath, self.options, fixups) - self.fixups = fixups + self.manager.semantic_analyzer.visit_file(self.tree, self.xpath, self.options, patches) + self.patches = patches def semantic_analysis_pass_three(self) -> None: with self.wrap_context(): @@ -1612,9 +1612,9 @@ def semantic_analysis_pass_three(self) -> None: if self.options.dump_type_stats: dump_type_stats(self.tree, self.xpath) - def semantic_analysis_fixups(self) -> None: - for fixup in self.fixups: - fixup() + def semantic_analysis_apply_patches(self) -> None: + for patch_func in self.patches: + patch_func() def type_check_first_pass(self) -> None: manager = self.manager @@ -2050,7 +2050,7 @@ def process_stale_scc(graph: Graph, scc: List[str], manager: BuildManager) -> No for id in fresh: graph[id].calculate_mros() for id in stale: - graph[id].semantic_analysis_fixups() + graph[id].semantic_analysis_apply_patches() for id in stale: graph[id].type_check_first_pass() more = True diff --git a/mypy/semanal.py b/mypy/semanal.py index aae2366f88e5..389efe182be9 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -258,10 +258,10 @@ def __init__(self, self.all_exports = set() # type: Set[str] def visit_file(self, file_node: MypyFile, fnam: str, options: Options, - fixups: List[Callable[[], None]]) -> None: + patches: List[Callable[[], None]]) -> None: """Run semantic analysis phase 2 over a file. - Add callbacks by mutating the fixups list argument. They will be called + Add callbacks by mutating the patches list argument. They will be called after all semantic analysis phases but before type checking. """ self.options = options @@ -270,7 +270,7 @@ def visit_file(self, file_node: MypyFile, fnam: str, options: Options, self.cur_mod_id = file_node.fullname() self.is_stub_file = fnam.lower().endswith('.pyi') self.globals = file_node.names - self.fixups = fixups + self.patches = patches if 'builtins' in self.modules: self.globals['__builtins__'] = SymbolTableNode( @@ -297,7 +297,7 @@ def visit_file(self, file_node: MypyFile, fnam: str, options: Options, g.module_public = False del self.options - del self.fixups + del self.patches def refresh_partial(self, node: Union[MypyFile, FuncItem]) -> None: """Refresh a stale target in fine-grained incremental mode.""" @@ -2378,14 +2378,14 @@ def build_typeddict_typeinfo(self, name: str, items: List[str], [self.str_type(), self.object_type()]) or self.object_type()) - def fixup() -> None: - mapping_value_type = join.join_type_list(types) - fallback.args[1] = mapping_value_type + def patch() -> None: + # Calculate the correct value type for the fallback Mapping. + fallback.args[1] = join.join_type_list(types) # We can't calculate the complete fallback type until after semantic - # analysis, since otherwise MROs might be incomplete. Postpone a fixup + # analysis, since otherwise MROs might be incomplete. Postpone a callback # function that patches the fallback. - self.fixups.append(fixup) + self.patches.append(patch) info = self.basic_new_typeinfo(name, fallback) info.typeddict_type = TypedDictType(OrderedDict(zip(items, types)), fallback) diff --git a/mypy/server/update.py b/mypy/server/update.py index b43cb5a66fdc..839470777914 100644 --- a/mypy/server/update.py +++ b/mypy/server/update.py @@ -147,7 +147,7 @@ def build_incremental_step(manager: BuildManager, # TODO: state.fix_suppressed_dependencies()? state.semantic_analysis() state.semantic_analysis_pass_three() - # TODO: state.semantic_analysis_fixups() + # TODO: state.semantic_analysis_apply_patches() state.type_check_first_pass() # TODO: state.type_check_second_pass()? state.finish_passes()