Repository navigation
Typing the ast.AST subclass constructors #8378
Description
We currently have static types for the fields of most (all?) ast classes, but none of these have typed constructors, e.g.:
Lines 79 to 86 in 62cde01
| class ClassDef(stmt): | |
| if sys.version_info >= (3, 10): | |
| __match_args__ = ("name", "bases", "keywords", "body", "decorator_list") | |
| name: _Identifier | |
| bases: list[expr] | |
| keywords: list[keyword] | |
| body: list[stmt] | |
| decorator_list: list[expr] |
I think technically this might be because none of these subclasses actually have a unique constructor, but that shouldn't stop us from typing each of the constructors, since in practise the constructor arguments must correspond to the class fields.
Before I do this, though, I'm firstly wondering if an easy solution here would be to apply the dataclass_transform decorator (note: not the same as the dataclass decorator). This would simply tell the type checker that all the class fields can and should be provided in the constructor, which is broadly correct. However I don't have a deep understanding of how the ast.AST constructor works, so this might not be the correct behaviour. For ClassDef this might look like:
from typing_extensions import dataclass_transform
@dataclass_transform
class ClassDef(stmt):
if sys.version_info >= (3, 10):
__match_args__ = ("name", "bases", "keywords", "body", "decorator_list")
name: _Identifier
bases: list[expr]
keywords: list[keyword]
body: list[stmt]
decorator_list: list[expr] If this isn't sufficient, I propose that we simply add an __init__() stub to each subclass. For instance, for the ClassDef above, this might look like:
class ClassDef(stmt):
if sys.version_info >= (3, 10):
__match_args__ = ("name", "bases", "keywords", "body", "decorator_list")
name: _Identifier
bases: list[expr]
keywords: list[keyword]
body: list[stmt]
decorator_list: list[expr]
def __init__(
name: _Identifier
bases: list[expr]
keywords: list[keyword]
body: list[stmt]
decorator_list: list[expr]
):
...
Activity
hauntsaninja commentedon Jul 24, 2022
I'd be in favour of adding
__init__stubs to each subclass. PR welcome!multimeric commentedon Jul 24, 2022
Thoughts on the
dataclass_transformidea?hauntsaninja commentedon Jul 24, 2022
I would prefer explicit
__init__s, since typically typeshed prefers to mirror closely what's happening at the runtime, rather than getting fancy and lying. Also not sure how well supported PEP 681 currently is by type checkers — last I checked mypy doesn't yet support it.(Note dataclass_transform would have to be applied to a base class to work, not to the class itself. You'd also want to be careful to get all the params right, e.g. we shouldn't synthesise comparison methods and stuff)
Akuli commentedon Jul 24, 2022
Invoking the constructors seems fragile to me. For example, if you instantiate
ast.Module, it's easy to forget passing in a second argument and end up with aModulethat doesn't have atype_ignoresattribute:You can even make an empty module that doesn't have any attributes:
So code that instantiates AST nodes will break every time a new attribute is added. Maybe it means that you shouldn't do this and the type checker should error if you do this, or maybe it just means that type checking is unusually important here.
multimeric commentedon Jul 24, 2022
@Akuli I interpret it as type checking being particularly important for this module, which is why I raised this issue. imo ideally the actual constructor would validate the arguments too, but it doesn't seem to do that, and I don't want to touch the C code that provides the constructor, so I'm instead aiming at a type annotated constructor.