Running dataclass transform in a later pass to fix crashes #12762
Conversation
This comment has been minimized.
This comment has been minimized.
mypy/plugin.py
Outdated
| @@ -692,9 +692,32 @@ 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*. | |||
There was a problem hiding this comment.
Maybe briefly mention that placeholders are caused by forward refs (plugin devs read this file as docs).
| @@ -390,6 +397,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. | |||
There was a problem hiding this comment.
Maybe also add the meaning of returning None to the docstring?
mypy/semanal_main.py
Outdated
| # 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" |
There was a problem hiding this comment.
This looks a bit arbitrary. I would say we need a large number, so this will not happen in legitimate cases.
| 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): |
There was a problem hiding this comment.
Not sure why these changes are needed here.
There was a problem hiding this comment.
Some errors were reported incorrectly in test cases (in the wrong file etc.) without these changes. I think that that this previously implicitly assumed that the context state was set up via some earlier pass, and the new pass invalidated some of the assumptions. Now we explicitly set the state to what we require.
This comment has been minimized.
This comment has been minimized.
1 similar comment
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
This is similar to #12762, but for attrs classes.
The dataclass plugin could crash if it encountered a placeholder. Fix the issue by
running the plugin after the main semantic analysis pass, when all placeholders have
been resolved.
Also add a new hook called
get_class_decorator_hook_2that is used by thedataclass plugin.
We may want to do a similar change to the attrs plugin, but let's change one thing
at a time.
Fix #12685.