From 6001a9641d686cc7e6975ef905fe312e8f966870 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 10 May 2022 13:18:37 +0100 Subject: [PATCH 01/16] WIP fix dataclass transform --- mypy/plugin.py | 4 +++ mypy/plugins/default.py | 14 ++++++-- mypy/semanal_main.py | 52 +++++++++++++++++++++++++++ test-data/unit/check-dataclasses.test | 18 ++++++++++ 4 files changed, 85 insertions(+), 3 deletions(-) diff --git a/mypy/plugin.py b/mypy/plugin.py index 72d28c39436a..c7e6f858e71f 100644 --- a/mypy/plugin.py +++ b/mypy/plugin.py @@ -698,6 +698,10 @@ def get_class_decorator_hook(self, fullname: str """ return None + def get_class_decorator_hook_2(self, fullname: str + ) -> Optional[Callable[[ClassDefContext], None]]: + return None + def get_metaclass_hook(self, fullname: str ) -> Optional[Callable[[ClassDefContext], None]]: """Update class definition for given declared metaclasses. diff --git a/mypy/plugins/default.py b/mypy/plugins/default.py index a7fa2cfaa868..ff44a320d3fc 100644 --- a/mypy/plugins/default.py +++ b/mypy/plugins/default.py @@ -95,7 +95,6 @@ def get_attribute_hook(self, fullname: str def get_class_decorator_hook(self, fullname: str ) -> Optional[Callable[[ClassDefContext], None]]: from mypy.plugins import attrs - from mypy.plugins import dataclasses from mypy.plugins import functools if fullname in attrs.attr_class_makers: @@ -116,13 +115,22 @@ def get_class_decorator_hook(self, fullname: str attrs.attr_class_maker_callback, auto_attribs_default=None, ) - elif fullname in dataclasses.dataclass_makers: - return dataclasses.dataclass_class_maker_callback + #elif fullname in dataclasses.dataclass_makers: + # return dataclasses.dataclass_class_maker_callback elif fullname in functools.functools_total_ordering_makers: return functools.functools_total_ordering_maker_callback return None + def get_class_decorator_hook_2(self, fullname: str + ) -> Optional[Callable[[ClassDefContext], None]]: + from mypy.plugins import dataclasses + + if fullname in dataclasses.dataclass_makers: + return dataclasses.dataclass_class_maker_callback + + return None + def contextmanager_callback(ctx: FunctionContext) -> Type: """Infer a better return type for 'contextlib.contextmanager'.""" diff --git a/mypy/semanal_main.py b/mypy/semanal_main.py index 7a82032b46b7..18b0145d9aab 100644 --- a/mypy/semanal_main.py +++ b/mypy/semanal_main.py @@ -82,6 +82,8 @@ def semantic_analysis_for_scc(graph: 'Graph', scc: List[str], errors: Errors) -> apply_semantic_analyzer_patches(patches) # This pass might need fallbacks calculated above. check_type_arguments(graph, scc, errors) + # Run class decorator hooks (they requite complete MROs). + apply_class_decorator_hooks(graph, scc, errors) calculate_class_properties(graph, scc, errors) check_blockers(graph, scc) # Clean-up builtins, so that TypeVar etc. are not accessible without importing. @@ -382,6 +384,56 @@ def check_type_arguments_in_targets(targets: List[FineGrainedDeferredNode], stat target.node.accept(analyzer) +from mypy.nodes import Expression, CallExpr, IndexExpr, RefExpr +from mypy.plugin import ClassDefContext + + +def apply_class_decorator_hooks(graph: 'Graph', scc: List[str], errors: Errors) -> None: + incomplete = True + while incomplete: + incomplete = False + for module in scc: + state = graph[module] + tree = state.tree + assert tree + for _, node, _ in tree.local_definitions(): + if isinstance(node.node, TypeInfo): + if not apply_hooks_to_class(state.manager.semantic_analyzer, + module, node.node, errors): + incomplete = True + + +def apply_hooks_to_class(self: SemanticAnalyzer, module: str, info: TypeInfo, + errors: Errors) -> bool: + + def get_fullname(expr: Expression) -> Optional[str]: + if isinstance(expr, CallExpr): + return get_fullname(expr.callee) + elif isinstance(expr, IndexExpr): + return get_fullname(expr.base) + elif isinstance(expr, RefExpr): + if expr.fullname: + return expr.fullname + # If we don't have a fullname look it up. This happens because base classes are + # analyzed in a different manner (see exprtotype.py) and therefore those AST + # nodes will not have full names. + sym = self.lookup_type_node(expr) + if sym: + return sym.fullname + return None + + saved = (module, info, None) # module, class, function + with errors.scope.saved_scope(saved) if errors.scope else nullcontext(): + defn = info.defn + for decorator in defn.decorators: + decorator_name = get_fullname(decorator) + if decorator_name: + hook = self.plugin.get_class_decorator_hook_2(decorator_name) + if hook: + hook(ClassDefContext(defn, decorator, self)) + return True + + def calculate_class_properties(graph: 'Graph', scc: List[str], errors: Errors) -> None: for module in scc: tree = graph[module].tree diff --git a/test-data/unit/check-dataclasses.test b/test-data/unit/check-dataclasses.test index bce1ee24a31a..52e2a00bf5eb 100644 --- a/test-data/unit/check-dataclasses.test +++ b/test-data/unit/check-dataclasses.test @@ -1619,3 +1619,21 @@ Child(x='', y='') # E: Argument "y" to "Child" has incompatible type "str"; exp Child(x='', y=1) Child(x=None, y=None) [builtins fixtures/dataclasses.pyi] + +[case testDataclassGenericInheritanceSpecialCase] +# flags: --python-version 3.7 +from dataclasses import dataclass +from typing import Generic, TypeVar + +T = TypeVar("T") + +@dataclass +class Parent(Generic[T]): + key: str + +@dataclass +class Child1(Parent["Child2"]): ... + +@dataclass +class Child2(Parent["Child1"]): ... +[builtins fixtures/dataclasses.pyi] From 27488f1be83d4c318f430fca976ff7873e9fec87 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 10 May 2022 13:36:18 +0100 Subject: [PATCH 02/16] Fix --- mypy/plugin.py | 4 ++++ mypy/semanal_main.py | 23 ++++++++++++++--------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/mypy/plugin.py b/mypy/plugin.py index c7e6f858e71f..c82f2106c8e1 100644 --- a/mypy/plugin.py +++ b/mypy/plugin.py @@ -819,6 +819,10 @@ def get_class_decorator_hook(self, fullname: str ) -> Optional[Callable[[ClassDefContext], None]]: return self._find_hook(lambda plugin: plugin.get_class_decorator_hook(fullname)) + def get_class_decorator_hook_2(self, fullname: str + ) -> Optional[Callable[[ClassDefContext], None]]: + return self._find_hook(lambda plugin: plugin.get_class_decorator_hook_2(fullname)) + def get_metaclass_hook(self, fullname: str ) -> Optional[Callable[[ClassDefContext], None]]: return self._find_hook(lambda plugin: plugin.get_metaclass_hook(fullname)) diff --git a/mypy/semanal_main.py b/mypy/semanal_main.py index 18b0145d9aab..ccd2139cc000 100644 --- a/mypy/semanal_main.py +++ b/mypy/semanal_main.py @@ -45,6 +45,7 @@ from mypy.checker import FineGrainedDeferredNode from mypy.server.aststrip import SavedAttributes from mypy.util import is_typeshed_file +from mypy.options import Options import mypy.build if TYPE_CHECKING: @@ -399,11 +400,15 @@ def apply_class_decorator_hooks(graph: 'Graph', scc: List[str], errors: Errors) for _, node, _ in tree.local_definitions(): if isinstance(node.node, TypeInfo): if not apply_hooks_to_class(state.manager.semantic_analyzer, - module, node.node, errors): + module, node.node, state.options, tree, errors): incomplete = True -def apply_hooks_to_class(self: SemanticAnalyzer, module: str, info: TypeInfo, +def apply_hooks_to_class(self: SemanticAnalyzer, + module: str, + info: TypeInfo, + options: Options, + file_node: MypyFile, errors: Errors) -> bool: def get_fullname(expr: Expression) -> Optional[str]: @@ -423,13 +428,13 @@ def get_fullname(expr: Expression) -> Optional[str]: return None saved = (module, info, None) # module, class, function - with errors.scope.saved_scope(saved) if errors.scope else nullcontext(): - defn = info.defn - for decorator in defn.decorators: - decorator_name = get_fullname(decorator) - if decorator_name: - hook = self.plugin.get_class_decorator_hook_2(decorator_name) - if hook: + defn = info.defn + for decorator in defn.decorators: + decorator_name = get_fullname(decorator) + if decorator_name: + hook = self.plugin.get_class_decorator_hook_2(decorator_name) + if hook: + with self.file_context(file_node, options, info): hook(ClassDefContext(defn, decorator, self)) return True From 5e39a1766754e826b7647c99d6b4cf54afa84201 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 10 May 2022 13:46:39 +0100 Subject: [PATCH 03/16] Fix daemon --- mypy/semanal_main.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mypy/semanal_main.py b/mypy/semanal_main.py index ccd2139cc000..11f148da386d 100644 --- a/mypy/semanal_main.py +++ b/mypy/semanal_main.py @@ -135,6 +135,7 @@ def semantic_analysis_for_targets( check_type_arguments_in_targets(nodes, state, state.manager.errors) calculate_class_properties(graph, [state.id], state.manager.errors) + apply_class_decorator_hooks(graph, [state.id], state.manager.errors) def restore_saved_attrs(saved_attrs: SavedAttributes) -> None: From f2a89d6b4184586fd368ab3ce1a0a642399b427f Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 10 May 2022 13:48:41 +0100 Subject: [PATCH 04/16] Fix invalid decorator --- mypy/semanal_main.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/mypy/semanal_main.py b/mypy/semanal_main.py index 11f148da386d..c202665963f6 100644 --- a/mypy/semanal_main.py +++ b/mypy/semanal_main.py @@ -431,12 +431,12 @@ def get_fullname(expr: Expression) -> Optional[str]: saved = (module, info, None) # module, class, function defn = info.defn for decorator in defn.decorators: - decorator_name = get_fullname(decorator) - if decorator_name: - hook = self.plugin.get_class_decorator_hook_2(decorator_name) - if hook: - with self.file_context(file_node, options, info): - hook(ClassDefContext(defn, decorator, self)) + with self.file_context(file_node, options, info): + decorator_name = get_fullname(decorator) + if decorator_name: + hook = self.plugin.get_class_decorator_hook_2(decorator_name) + if hook: + hook(ClassDefContext(defn, decorator, self)) return True From 67cda31c085cb2811b8f92837ef2e3de6987dd76 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 10 May 2022 13:50:26 +0100 Subject: [PATCH 05/16] Update test case --- test-data/unit/check-dataclasses.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-data/unit/check-dataclasses.test b/test-data/unit/check-dataclasses.test index 52e2a00bf5eb..5db0ea66afab 100644 --- a/test-data/unit/check-dataclasses.test +++ b/test-data/unit/check-dataclasses.test @@ -1207,7 +1207,7 @@ class A2: from dataclasses import dataclass @dataclass -class A: # E: Name "x" already defined (possibly by an import) +class A: x: int = 0 x: int = 0 # E: Name "x" already defined on line 7 From be1d92df96856049a85cc988573d76becc97424c Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 10 May 2022 13:58:55 +0100 Subject: [PATCH 06/16] Fix indent --- mypy/semanal_main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/semanal_main.py b/mypy/semanal_main.py index c202665963f6..72b99d043280 100644 --- a/mypy/semanal_main.py +++ b/mypy/semanal_main.py @@ -436,7 +436,7 @@ def get_fullname(expr: Expression) -> Optional[str]: if decorator_name: hook = self.plugin.get_class_decorator_hook_2(decorator_name) if hook: - hook(ClassDefContext(defn, decorator, self)) + hook(ClassDefContext(defn, decorator, self)) return True From f428de7f7991aecb0701e088f62a8d59863e30a4 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 10 May 2022 14:22:49 +0100 Subject: [PATCH 07/16] Fix class property error reporting --- mypy/semanal_main.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mypy/semanal_main.py b/mypy/semanal_main.py index 72b99d043280..b418a64a8aa7 100644 --- a/mypy/semanal_main.py +++ b/mypy/semanal_main.py @@ -442,12 +442,12 @@ def get_fullname(expr: Expression) -> Optional[str]: def calculate_class_properties(graph: 'Graph', scc: List[str], errors: Errors) -> None: for module in scc: - tree = graph[module].tree + state = graph[module] + tree = state.tree assert tree for _, node, _ in tree.local_definitions(): if isinstance(node.node, TypeInfo): - saved = (module, node.node, None) # module, class, function - with errors.scope.saved_scope(saved) if errors.scope else nullcontext(): + with state.manager.semantic_analyzer.file_context(tree, state.options, node.node): calculate_class_abstract_status(node.node, tree.is_stub, errors) check_protocol_status(node.node, errors) calculate_class_vars(node.node) From 86d0d6a43d1eee3edd3f0f4de54c3ae0cede8394 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 10 May 2022 14:26:30 +0100 Subject: [PATCH 08/16] Fix lint --- mypy/plugin.py | 2 +- mypy/plugins/default.py | 2 -- mypy/semanal_main.py | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/mypy/plugin.py b/mypy/plugin.py index c82f2106c8e1..47070294d636 100644 --- a/mypy/plugin.py +++ b/mypy/plugin.py @@ -820,7 +820,7 @@ def get_class_decorator_hook(self, fullname: str return self._find_hook(lambda plugin: plugin.get_class_decorator_hook(fullname)) def get_class_decorator_hook_2(self, fullname: str - ) -> Optional[Callable[[ClassDefContext], None]]: + ) -> Optional[Callable[[ClassDefContext], None]]: return self._find_hook(lambda plugin: plugin.get_class_decorator_hook_2(fullname)) def get_metaclass_hook(self, fullname: str diff --git a/mypy/plugins/default.py b/mypy/plugins/default.py index ff44a320d3fc..02340f5af7a0 100644 --- a/mypy/plugins/default.py +++ b/mypy/plugins/default.py @@ -115,8 +115,6 @@ def get_class_decorator_hook(self, fullname: str attrs.attr_class_maker_callback, auto_attribs_default=None, ) - #elif fullname in dataclasses.dataclass_makers: - # return dataclasses.dataclass_class_maker_callback elif fullname in functools.functools_total_ordering_makers: return functools.functools_total_ordering_maker_callback diff --git a/mypy/semanal_main.py b/mypy/semanal_main.py index b418a64a8aa7..0cc7918edfc8 100644 --- a/mypy/semanal_main.py +++ b/mypy/semanal_main.py @@ -428,7 +428,6 @@ def get_fullname(expr: Expression) -> Optional[str]: return sym.fullname return None - saved = (module, info, None) # module, class, function defn = info.defn for decorator in defn.decorators: with self.file_context(file_node, options, info): From 9c06485e9b0234b23b1391e10422a2ff93ddff8a Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 10 May 2022 15:09:35 +0100 Subject: [PATCH 09/16] Refactor --- mypy/semanal.py | 37 +++++++++++++++++++------------------ mypy/semanal_main.py | 19 +------------------ 2 files changed, 20 insertions(+), 36 deletions(-) diff --git a/mypy/semanal.py b/mypy/semanal.py index 555cb749074e..d68928ef21ad 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -1234,43 +1234,44 @@ def analyze_namedtuple_classdef(self, defn: ClassDef) -> bool: def apply_class_plugin_hooks(self, defn: ClassDef) -> None: """Apply a plugin hook that may infer a more precise definition for a class.""" - def get_fullname(expr: Expression) -> Optional[str]: - if isinstance(expr, CallExpr): - return get_fullname(expr.callee) - elif isinstance(expr, IndexExpr): - return get_fullname(expr.base) - elif isinstance(expr, RefExpr): - if expr.fullname: - return expr.fullname - # If we don't have a fullname look it up. This happens because base classes are - # analyzed in a different manner (see exprtotype.py) and therefore those AST - # nodes will not have full names. - sym = self.lookup_type_node(expr) - if sym: - return sym.fullname - return None for decorator in defn.decorators: - decorator_name = get_fullname(decorator) + decorator_name = self.get_fullname_for_hook(decorator) if decorator_name: hook = self.plugin.get_class_decorator_hook(decorator_name) if hook: hook(ClassDefContext(defn, decorator, self)) if defn.metaclass: - metaclass_name = get_fullname(defn.metaclass) + metaclass_name = self.get_fullname_for_hook(defn.metaclass) if metaclass_name: hook = self.plugin.get_metaclass_hook(metaclass_name) if hook: hook(ClassDefContext(defn, defn.metaclass, self)) for base_expr in defn.base_type_exprs: - base_name = get_fullname(base_expr) + base_name = self.get_fullname_for_hook(base_expr) if base_name: hook = self.plugin.get_base_class_hook(base_name) if hook: hook(ClassDefContext(defn, base_expr, self)) + def get_fullname_for_hook(self, expr: Expression) -> Optional[str]: + if isinstance(expr, CallExpr): + return self.get_fullname_for_hook(expr.callee) + elif isinstance(expr, IndexExpr): + return self.get_fullname_for_hook(expr.base) + elif isinstance(expr, RefExpr): + if expr.fullname: + return expr.fullname + # If we don't have a fullname look it up. This happens because base classes are + # analyzed in a different manner (see exprtotype.py) and therefore those AST + # nodes will not have full names. + sym = self.lookup_type_node(expr) + if sym: + return sym.fullname + return None + def analyze_class_keywords(self, defn: ClassDef) -> None: for value in defn.keywords.values(): value.accept(self) diff --git a/mypy/semanal_main.py b/mypy/semanal_main.py index 0cc7918edfc8..8d369971bd00 100644 --- a/mypy/semanal_main.py +++ b/mypy/semanal_main.py @@ -411,27 +411,10 @@ def apply_hooks_to_class(self: SemanticAnalyzer, options: Options, file_node: MypyFile, errors: Errors) -> bool: - - def get_fullname(expr: Expression) -> Optional[str]: - if isinstance(expr, CallExpr): - return get_fullname(expr.callee) - elif isinstance(expr, IndexExpr): - return get_fullname(expr.base) - elif isinstance(expr, RefExpr): - if expr.fullname: - return expr.fullname - # If we don't have a fullname look it up. This happens because base classes are - # analyzed in a different manner (see exprtotype.py) and therefore those AST - # nodes will not have full names. - sym = self.lookup_type_node(expr) - if sym: - return sym.fullname - return None - defn = info.defn for decorator in defn.decorators: with self.file_context(file_node, options, info): - decorator_name = get_fullname(decorator) + decorator_name = self.get_fullname_for_hook(decorator) if decorator_name: hook = self.plugin.get_class_decorator_hook_2(decorator_name) if hook: From dca48b5b2c8fabd62d3ac5691c8d1d2facd50342 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 10 May 2022 15:57:16 +0100 Subject: [PATCH 10/16] Clean up --- mypy/semanal_main.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/mypy/semanal_main.py b/mypy/semanal_main.py index 8d369971bd00..21e4c3af599d 100644 --- a/mypy/semanal_main.py +++ b/mypy/semanal_main.py @@ -83,8 +83,8 @@ def semantic_analysis_for_scc(graph: 'Graph', scc: List[str], errors: Errors) -> apply_semantic_analyzer_patches(patches) # This pass might need fallbacks calculated above. check_type_arguments(graph, scc, errors) - # Run class decorator hooks (they requite complete MROs). - apply_class_decorator_hooks(graph, scc, errors) + # Run class decorator hooks (they requite complete MROs and no placeholders). + apply_class_plugin_hooks(graph, scc, errors) calculate_class_properties(graph, scc, errors) check_blockers(graph, scc) # Clean-up builtins, so that TypeVar etc. are not accessible without importing. @@ -135,7 +135,7 @@ def semantic_analysis_for_targets( check_type_arguments_in_targets(nodes, state, state.manager.errors) calculate_class_properties(graph, [state.id], state.manager.errors) - apply_class_decorator_hooks(graph, [state.id], state.manager.errors) + apply_class_plugin_hooks(graph, [state.id], state.manager.errors) def restore_saved_attrs(saved_attrs: SavedAttributes) -> None: @@ -390,7 +390,16 @@ def check_type_arguments_in_targets(targets: List[FineGrainedDeferredNode], stat from mypy.plugin import ClassDefContext -def apply_class_decorator_hooks(graph: 'Graph', scc: List[str], errors: Errors) -> None: +def apply_class_plugin_hooks(graph: 'Graph', scc: List[str], errors: Errors) -> None: + """Apply class plugin hooks within a SCC. + + We run these after to the main semantic analysis so that the hooks + don't need to deal with incomplete definitions such as placeholder + types. + + Note that some hooks incorrectly run during the main semantic + analysis pass, for historical reasons. + """ incomplete = True while incomplete: incomplete = False @@ -411,6 +420,7 @@ def apply_hooks_to_class(self: SemanticAnalyzer, options: Options, file_node: MypyFile, errors: Errors) -> bool: + # TODO: Move more class-related hooks here? defn = info.defn for decorator in defn.decorators: with self.file_context(file_node, options, info): From 9fbeb92ad24b86a818cedc64dc30f09b28e3ab4f Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 10 May 2022 15:58:30 +0100 Subject: [PATCH 11/16] More cleanup --- mypy/semanal_main.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/mypy/semanal_main.py b/mypy/semanal_main.py index 21e4c3af599d..7ef7f3cd4fb3 100644 --- a/mypy/semanal_main.py +++ b/mypy/semanal_main.py @@ -46,6 +46,7 @@ from mypy.server.aststrip import SavedAttributes from mypy.util import is_typeshed_file from mypy.options import Options +from mypy.plugin import ClassDefContext import mypy.build if TYPE_CHECKING: @@ -386,10 +387,6 @@ def check_type_arguments_in_targets(targets: List[FineGrainedDeferredNode], stat target.node.accept(analyzer) -from mypy.nodes import Expression, CallExpr, IndexExpr, RefExpr -from mypy.plugin import ClassDefContext - - def apply_class_plugin_hooks(graph: 'Graph', scc: List[str], errors: Errors) -> None: """Apply class plugin hooks within a SCC. From 15d57f052f70368f6532150c86a2da1ca7bee617 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 10 May 2022 16:23:12 +0100 Subject: [PATCH 12/16] Support multiple passes --- mypy/plugin.py | 4 +-- mypy/plugins/dataclasses.py | 35 +++++++++++++++--------- mypy/plugins/default.py | 5 +++- mypy/semanal_main.py | 10 +++++-- test-data/unit/check-dataclasses.test | 38 ++++++++++++++++++++++++--- 5 files changed, 72 insertions(+), 20 deletions(-) diff --git a/mypy/plugin.py b/mypy/plugin.py index 47070294d636..cffc290bea41 100644 --- a/mypy/plugin.py +++ b/mypy/plugin.py @@ -699,7 +699,7 @@ def get_class_decorator_hook(self, fullname: str return None def get_class_decorator_hook_2(self, fullname: str - ) -> Optional[Callable[[ClassDefContext], None]]: + ) -> Optional[Callable[[ClassDefContext], bool]]: return None def get_metaclass_hook(self, fullname: str @@ -820,7 +820,7 @@ def get_class_decorator_hook(self, fullname: str return self._find_hook(lambda plugin: plugin.get_class_decorator_hook(fullname)) def get_class_decorator_hook_2(self, fullname: str - ) -> Optional[Callable[[ClassDefContext], None]]: + ) -> Optional[Callable[[ClassDefContext], bool]]: return self._find_hook(lambda plugin: plugin.get_class_decorator_hook_2(fullname)) def get_metaclass_hook(self, fullname: str diff --git a/mypy/plugins/dataclasses.py b/mypy/plugins/dataclasses.py index 62b4c89bd674..126b90b1b673 100644 --- a/mypy/plugins/dataclasses.py +++ b/mypy/plugins/dataclasses.py @@ -110,7 +110,7 @@ class DataclassTransformer: def __init__(self, ctx: ClassDefContext) -> None: self._ctx = ctx - def transform(self) -> None: + def transform(self) -> bool: """Apply all the necessary transformations to the underlying dataclass so as to ensure it is fully type checked according to the rules in PEP 557. @@ -119,12 +119,11 @@ def transform(self) -> None: info = self._ctx.cls.info attributes = self.collect_attributes() if attributes is None: - # Some definitions are not ready, defer() should be already called. - return + # Some definitions are not ready. we need another pass. + return False for attr in attributes: if attr.type is None: - ctx.api.defer() - return + return False decorator_arguments = { 'init': _get_decorator_bool_argument(self._ctx, 'init', True), 'eq': _get_decorator_bool_argument(self._ctx, 'eq', True), @@ -236,6 +235,8 @@ def transform(self) -> None: 'frozen': decorator_arguments['frozen'], } + return True + def add_slots(self, info: TypeInfo, attributes: List[DataclassAttribute], @@ -315,14 +316,11 @@ def collect_attributes(self) -> Optional[List[DataclassAttribute]]: sym = cls.info.names.get(lhs.name) if sym is None: - # This name is likely blocked by a star import. We don't need to defer because - # defer() is already called by mark_incomplete(). + # There was probably a semantic analysis error. continue node = sym.node - if isinstance(node, PlaceholderNode): - # This node is not ready yet. - return None + assert not isinstance(node, PlaceholderNode) assert isinstance(node, Var) # x: ClassVar[int] is ignored by dataclasses. @@ -390,6 +388,9 @@ def collect_attributes(self) -> Optional[List[DataclassAttribute]]: # we'll have unmodified attrs laying around. all_attrs = attrs.copy() for info in cls.info.mro[1:-1]: + if 'dataclass_tag' in info.metadata and 'dataclass' not in info.metadata: + # We haven't processed the base class yet. Need another pass. + return None if 'dataclass' not in info.metadata: continue @@ -517,11 +518,21 @@ def _add_dataclass_fields_magic_attribute(self) -> None: ) -def dataclass_class_maker_callback(ctx: ClassDefContext) -> None: +def dataclass_tag_callback(ctx: ClassDefContext) -> None: + """Record that we have a dataclass in the main semantic analysis pass. + + The later pass implemented by DataclassTransformer will use this + to detect dataclasses in base classes. + """ + # The value is ignored, only the existence matters. + ctx.cls.info.metadata['dataclass_tag'] = {} + + +def dataclass_class_maker_callback(ctx: ClassDefContext) -> bool: """Hooks into the class typechecking process to add support for dataclasses. """ transformer = DataclassTransformer(ctx) - transformer.transform() + return transformer.transform() def _collect_field_args(expr: Expression, diff --git a/mypy/plugins/default.py b/mypy/plugins/default.py index 02340f5af7a0..50e0e8cb4315 100644 --- a/mypy/plugins/default.py +++ b/mypy/plugins/default.py @@ -95,6 +95,7 @@ def get_attribute_hook(self, fullname: str def get_class_decorator_hook(self, fullname: str ) -> Optional[Callable[[ClassDefContext], None]]: from mypy.plugins import attrs + from mypy.plugins import dataclasses from mypy.plugins import functools if fullname in attrs.attr_class_makers: @@ -115,13 +116,15 @@ def get_class_decorator_hook(self, fullname: str attrs.attr_class_maker_callback, auto_attribs_default=None, ) + elif fullname in dataclasses.dataclass_makers: + return dataclasses.dataclass_tag_callback elif fullname in functools.functools_total_ordering_makers: return functools.functools_total_ordering_maker_callback return None def get_class_decorator_hook_2(self, fullname: str - ) -> Optional[Callable[[ClassDefContext], None]]: + ) -> Optional[Callable[[ClassDefContext], bool]]: from mypy.plugins import dataclasses if fullname in dataclasses.dataclass_makers: diff --git a/mypy/semanal_main.py b/mypy/semanal_main.py index 7ef7f3cd4fb3..4bbdd00bc7a8 100644 --- a/mypy/semanal_main.py +++ b/mypy/semanal_main.py @@ -397,8 +397,13 @@ def apply_class_plugin_hooks(graph: 'Graph', scc: List[str], errors: Errors) -> Note that some hooks incorrectly run during the main semantic analysis pass, for historical reasons. """ + num_passes = 0 incomplete = True + # If we encounter a base class that has not been processed, we'll run another + # pass. This should eventually reach a fixed point. while incomplete: + assert num_passes < 5, "Internal error: too many class plugin hook passes" + num_passes += 1 incomplete = False for module in scc: state = graph[module] @@ -419,14 +424,15 @@ def apply_hooks_to_class(self: SemanticAnalyzer, errors: Errors) -> bool: # TODO: Move more class-related hooks here? defn = info.defn + ok = True for decorator in defn.decorators: with self.file_context(file_node, options, info): decorator_name = self.get_fullname_for_hook(decorator) if decorator_name: hook = self.plugin.get_class_decorator_hook_2(decorator_name) if hook: - hook(ClassDefContext(defn, decorator, self)) - return True + ok = ok and hook(ClassDefContext(defn, decorator, self)) + return ok def calculate_class_properties(graph: 'Graph', scc: List[str], errors: Errors) -> None: diff --git a/test-data/unit/check-dataclasses.test b/test-data/unit/check-dataclasses.test index 5db0ea66afab..872438a234cf 100644 --- a/test-data/unit/check-dataclasses.test +++ b/test-data/unit/check-dataclasses.test @@ -1620,20 +1620,52 @@ Child(x='', y=1) Child(x=None, y=None) [builtins fixtures/dataclasses.pyi] -[case testDataclassGenericInheritanceSpecialCase] +[case testDataclassGenericInheritanceSpecialCase1] # flags: --python-version 3.7 from dataclasses import dataclass -from typing import Generic, TypeVar +from typing import Generic, TypeVar, List T = TypeVar("T") @dataclass class Parent(Generic[T]): - key: str + x: List[T] @dataclass class Child1(Parent["Child2"]): ... @dataclass class Child2(Parent["Child1"]): ... + +def f(c: Child2) -> None: + reveal_type(Child1([c]).x) # N: Revealed type is "builtins.list[__main__.Child2]" + +def g(c: Child1) -> None: + reveal_type(Child2([c]).x) # N: Revealed type is "builtins.list[__main__.Child1]" +[builtins fixtures/dataclasses.pyi] + +[case testDataclassGenericInheritanceSpecialCase2] +# flags: --python-version 3.7 +from dataclasses import dataclass +from typing import Generic, TypeVar + +T = TypeVar("T") + +# A subclass might be analyzed before base in import cycles. They are +# defined here in reversed order to simulate this. + +@dataclass +class Child1(Parent["Child2"]): + x: int + +@dataclass +class Child2(Parent["Child1"]): + y: int + +@dataclass +class Parent(Generic[T]): + key: str + +Child1(x=1, key='') +Child2(y=1, key='') [builtins fixtures/dataclasses.pyi] From a167af68de8b54a3bad7392ea9aa98e6b805df83 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 10 May 2022 16:27:33 +0100 Subject: [PATCH 13/16] Add docstring --- mypy/plugins/dataclasses.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/mypy/plugins/dataclasses.py b/mypy/plugins/dataclasses.py index 126b90b1b673..4d9236992e3b 100644 --- a/mypy/plugins/dataclasses.py +++ b/mypy/plugins/dataclasses.py @@ -107,6 +107,15 @@ def expand_typevar_from_subtype(self, sub_type: TypeInfo) -> None: class DataclassTransformer: + """Implement the behavior of @dataclass. + + Note that this may be executed multiple times on the same class, so + everything here must be idempotent. + + This runs after the main semantic analysis pass, so you can assume that + there are no placeholders. + """ + def __init__(self, ctx: ClassDefContext) -> None: self._ctx = ctx @@ -119,7 +128,7 @@ def transform(self) -> bool: info = self._ctx.cls.info attributes = self.collect_attributes() if attributes is None: - # Some definitions are not ready. we need another pass. + # Some definitions are not ready. We need another pass. return False for attr in attributes: if attr.type is None: From 543d95501400ce882fc1997b4bc57af3fd036e96 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 10 May 2022 16:34:01 +0100 Subject: [PATCH 14/16] Improve docstrings --- mypy/plugin.py | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/mypy/plugin.py b/mypy/plugin.py index cffc290bea41..65885015df05 100644 --- a/mypy/plugin.py +++ b/mypy/plugin.py @@ -692,14 +692,33 @@ def get_class_decorator_hook(self, fullname: str The plugin can modify a TypeInfo _in place_ (for example add some generated methods to the symbol table). This hook is called after the class body was - semantically analyzed. + semantically analyzed, but *there may still be placeholders*. - The hook is called with full names of all class decorators, for example + NOTE: Usually get_class_decorator_hook_2 is the better option, since it + guarantees that there are no placeholders. + + The hook is called with full names of all class decorators. + + The hook can be called multiple times per class, so it must be + idempotent. """ return None def get_class_decorator_hook_2(self, fullname: str ) -> Optional[Callable[[ClassDefContext], bool]]: + """Update class definition for given class decorators. + + Similar to get_class_decorator_hook, but this runs in a later pass when + placeholders have been resolved. + + The hook can return False if some base class hasn't been + processed yet using class hooks. It causes all class hooks + (that are run in this same pass) to be invoked another time for + the file(s) currently being processed. + + The hook can be called multiple times per class, so it must be + idempotent. + """ return None def get_metaclass_hook(self, fullname: str From 51a3f0625a119a639fbd89195537eb43f44551a3 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Wed, 11 May 2022 12:27:30 +0100 Subject: [PATCH 15/16] Address feedback --- mypy/plugin.py | 3 ++- mypy/plugins/dataclasses.py | 3 +++ mypy/semanal_main.py | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/mypy/plugin.py b/mypy/plugin.py index 65885015df05..2f571d7eecc6 100644 --- a/mypy/plugin.py +++ b/mypy/plugin.py @@ -692,7 +692,8 @@ def get_class_decorator_hook(self, fullname: str The plugin can modify a TypeInfo _in place_ (for example add some generated methods to the symbol table). This hook is called after the class body was - semantically analyzed, but *there may still be placeholders*. + semantically analyzed, but *there may still be placeholders* (typically + caused by forward references). NOTE: Usually get_class_decorator_hook_2 is the better option, since it guarantees that there are no placeholders. diff --git a/mypy/plugins/dataclasses.py b/mypy/plugins/dataclasses.py index 4d9236992e3b..24077bb4a549 100644 --- a/mypy/plugins/dataclasses.py +++ b/mypy/plugins/dataclasses.py @@ -304,6 +304,9 @@ def collect_attributes(self) -> Optional[List[DataclassAttribute]]: b: SomeOtherType = ... are collected. + + Return None if some dataclass base class hasn't been processed + yet and thus we'll need to ask for another pass. """ # First, collect attributes belonging to the current class. ctx = self._ctx diff --git a/mypy/semanal_main.py b/mypy/semanal_main.py index 4bbdd00bc7a8..bb0af8edc46f 100644 --- a/mypy/semanal_main.py +++ b/mypy/semanal_main.py @@ -402,7 +402,7 @@ def apply_class_plugin_hooks(graph: 'Graph', scc: List[str], errors: Errors) -> # If we encounter a base class that has not been processed, we'll run another # pass. This should eventually reach a fixed point. while incomplete: - assert num_passes < 5, "Internal error: too many class plugin hook passes" + assert num_passes < 10, "Internal error: too many class plugin hook passes" num_passes += 1 incomplete = False for module in scc: From 379b8aaabaa4454b1d4da68eae8df92a4183a39a Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Wed, 11 May 2022 12:40:11 +0100 Subject: [PATCH 16/16] Add another test case from #10140 --- test-data/unit/check-dataclasses.test | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test-data/unit/check-dataclasses.test b/test-data/unit/check-dataclasses.test index 872438a234cf..4cddc59b0153 100644 --- a/test-data/unit/check-dataclasses.test +++ b/test-data/unit/check-dataclasses.test @@ -1669,3 +1669,19 @@ class Parent(Generic[T]): Child1(x=1, key='') Child2(y=1, key='') [builtins fixtures/dataclasses.pyi] + +[case testDataclassGenericWithBound] +# flags: --python-version 3.7 +from dataclasses import dataclass +from typing import Generic, TypeVar + +T = TypeVar("T", bound="C") + +@dataclass +class C(Generic[T]): + x: int + +c: C[C] +d: C[str] # E: Type argument "str" of "C" must be a subtype of "C[Any]" +C(x=2) +[builtins fixtures/dataclasses.pyi]