Typechecking attrs-generated classes

[lvh]

attrs removes object boilerplate:

>>> import attr
>>> @attr.s
... class C(object):
...     x = attr.ib(default=42)

This has many advantages, like eq/hash behavior working, reprs working, easy to add runtime validators, &c. However, it's not obvious to me how I can typecheck creating instances of this, because there's no __init__. (I'm using Python 2.7, so comment syntax, if it matters :))

[gvanrossum]

Hi Laurens, this looks like something that should be doable with the (as yet hypothetical) plugin architecture: #1240

[wsanchez]

A bit more detail, as the problem here isn't just that you can't validate attrs class initializer usage using mypy (and, in fact, you can validate types at runtime using attrs), but also that mypy is emitting false positives, which makes mypy harder to use if you use attrs.

Here's a script thing.py:

from pathlib import Path

from attr import attrib, attrs
from attr.validators import instance_of


@attrs()
class ThingWithPath(object):
    _path = attrib(validator=instance_of(Path))

    def exists(self):
        return self._path.exists()


thing1 = ThingWithPath(Path("."))
print("thing1 exists:", thing1.exists())

thing2 = ThingWithPath(path=Path("."))
print("thing2 exists:", thing2.exists())

Which runs thusly (note the runtime type validation, if that's a thing you like):

wsanchez$ python thing.py
thing1 exists: True
thing2 exists: True
Traceback (most recent call last):
  File "thing.py", line 21, in <module>
    thing3 = ThingWithPath(".")
  File "<attrs generated init 72ca13b0f10a00b12fb26accfd84d196fc2af0c6>", line 5, in __init__
  File "/Users/wsanchez/Dropbox/Developer/PIE/wsanchez/ConfigService/.tox/py35-coverage/lib/python3.5/site-packages/attr/validators.py", line 24, in __call__
    attr, self.type, value,
TypeError: ("'_path' must be <class 'pathlib.Path'> (got '.' that is a <class 'str'>).", Attribute(name='_path', default=NOTHING, validator=<instance_of validator for type <class 'pathlib.Path'>>, repr=True, cmp=True, hash=True, init=True, convert=None, metadata=mappingproxy({})), <class 'pathlib.Path'>, '.')

Running mypy on this script emits:

thing.py:15: error: Too many arguments for "ThingWithPath"
thing.py:18: error: Unexpected keyword argument "path" for "ThingWithPath"
thing.py:21: error: Too many arguments for "ThingWithPath"

And mypy of course exists with a non-zero status.

[gvanrossum]

It's unfortunate, but not unique to attrs -- this is a general problem when you have a metaclass or a class decorator that affects the API of a class. I suppose we could teach mypy to special-case attrs, but that just moves the problem to every other library engaging in such tricks.

A more principled approach would be to add some new magic primitive to PEP 484 (and support in mypy of course) that would allow one to write a stub for attrs that explains to mypy what the effect of the class decorator is. This might be easier now than when this issue was created given that we already have some support for metaclasses now (#2475), so some of the necessary scaffolding in mypy already exists.

You could also imagine that mypy could just parse the full source code for the attrs package and understand what it does without needing any help. But I think that's unlikely to be successful, due to the difficulty for mypy to understand the dynamic tricks such an implementation undoubtedly uses. (I haven't looked.)

In general, supporting code that does runtime manipulation of interfaces is not high on the list of things that mypy and PEP 484 are likely to support soon. It's generally a bit antithetic to the idea of static types, after all. :-)

[wsanchez]

Yup, I was just trying to be more specific about the issue here.

I'm not sure I agree that attrs (or at least its functionality) is "antithetic to the idea of static types". I understand why it creates challenges here, but the degree to which it does dynamic things is limited to one time when the class is created, not some random manipulations as the program is running.

That said, I'm fairly new to mypy and the typing system.

I agree that being able to write a stub would be useful absent more robust but considerably harder to implement solutions.

[ryanhiebert]

I'm not sure I agree that attrs (or at least its functionality) is "antithetic to the idea of static types".

I agree with this assessment. attrs itself is not antithetical, though it's current implementation obviously is. It's really about supporting better use of types, by making them simpler to declare and be useful. I'm sure that if there were a way to support type systems at the same time, that would definitely fall in line with the goals of attrs.

Whether the real solution is pluggability for the type system to allow for this kind of compile-time transformations, or whether the only real solution is to have something that fulfills the goals of attrs built into the language, I'm not sure, but they are two approaches that are both trying to encourage the proper use of types. The dynamic implementation is just that, an implementation detail.

[gvanrossum]

Well, mypy doesn't execute the program. It just reads the source code. So in order to validate the ThingWithPath() calls it would have to know the signature of the __init__() method that is added by the @attrs decorator, which in turn depends on going over the class attributes looking for expressions of a whatever type is returned by attrib(). Figuring that out presumably requires understanding the attrs package thoroughly, which is not possible given the state of the art and mypy's limited understanding of source code.

[fxfitz]

If it helps anyone, we solved this problem recently (specifically with attrs) where we define the init ourselves for attrs. Maybe it could help someone else too! Reusing the example from above:

@attrs(init=False)
class ThingWithPath(object):
    _path = attrib(validator=instance_of(Path))

    def __init__(self, _path):
        self._path = _path

    def exists(self):
        return self._path.exists()

[ambv]

There's discussion on an API in attrs (here: python-attrs/attrs#151) that would help bridging this. I'm following along since I like both typing and attrs for removed (and always correct!) boilerplate.

[wsanchez]

@fxfitz: As @ambv notes, the problem there is that your solution removes one of the big benefits of attrs. And presumably that prevents attrs from doing the validation it would have done in the generated __init__, no?

I suppose that if attrs nuked and replaced your __init__, that may work, and then you'd instead write:

@attrs()
class ThingWithPath(object):
    _path = attrib(validator=instance_of(Path))

    def __init__(self, _path: Path):  # Note added type annotation here
        pass  # This implementation isn't used, so pass

    def exists(self):
        return self._path.exists()

It would be confusing for someone who may put code into __init__ not knowing it's ignored… so it's not 100% great, but maybe better.

[fxfitz]

@wsanchez Ah right, I forget that if you init=False you would lose your validation; my bad! We used a similar approach to what I suggested earlier, but our goals were to (1) get mypy working and (2) still use attrs for the working equality/hash checks.

[Tinche]

Hello, I'm one of the attrs maintainers.

I think it's fair to say mypy will have a really hard time ever working out of the box with attrs since we do some really filthy things under the hood.

I also think making attrs work with mypy would be a very worthwhile thing to try to do, since the point of attrs is to make people use structured data more and this is very complimentary to the idea of static typing checks.

I'd be interested in maybe being involved with the effort to develop a plugin API, if that's the direction we're going in, and ultimately developing a plugin for attrs.

[gvanrossum]

Great plan! I wonder if you could approach this from the other direction: first write a mypy patch that supports attrs, hard-coding recognition of the special constructs it supports; and then, learning from that experience, figure out how a plugin API should be designed.

[Tinche]

I'll give it a shot. No promises on the timeline :)

[JukkaL]

Mypy now has the beginnings of a plugin architecture. It can't quite support attrs yet, but it shouldn't be too hard to extend it with a few extra hooks to allow attrs support.

[ghost]

Is there any update on this?

[JukkaL]

No updates yet.

[Tinche]

I spent a week in the summer going through mypy's source. I came to the conclusion we would need another plugin type, something that runs in between semantic analysis second pass and type checking and modifies attrs classes (IIRC).

[JukkaL]

That sounds reasonable. The new plugin type could be triggered by having a specific class decorator, base class or metaclass. For attrs specifically a class decorator would be the only needed trigger, right?

[ilevkivskyi]

@JukkaL

That sounds reasonable. The new plugin type could be triggered by having a specific class decorator, base class or metaclass

We just need to be sure that this will work correctly with forward references :-)

[Tinche]

@JukkaL It'd be great if it could be triggered on a class decorator that resolved to a specific name (in this case exactly "attr.s" or "attr.attrs"). I say "resolved" to handle people wrapping attr.s.

This could be reused for dataclasses (PEP 557) if it gets passed.

It would also be awesome if this kind of plugin could be pip installed and enabled somehow. Then we could version the plugin on PyPI and do synced releases with attrs.

[hynek]

It would be super cool if we could get some movement into this! 🎉 attrs has just merged type support based on both PEP 526 and as an argument which should lay some groundwork I believe?

Would it be easier to add support only if PEP 526 is supported for now? That would be totally good enough for me. :)

[gvanrossum]

I support having a plugin for this; I think the plugin will have to be contributed (Tin, Hynek, @ambv?). Jukka should be able to help you getting you started (the plugin API is not yet well-documented), but you'll have to do most of the work yourself, as this is not on the roadmap (and neither is PEP 557, dataclasses BTW).

[hynek]

I seem to remember that @chadrik mentioned that he'd like to get involved too. I won't be able to work on the plugin but will happily help with work that's necessary on the attrs side.

[ilevkivskyi]

but you'll have to do most of the work yourself, as this is not on the roadmap (and neither is PEP 557, dataclasses BTW).

I will probably have some time second half of October to take care about dataclasses.

Concerning the plugins, I already mentioned this, I would prefer not to allow plugins to replace TypeInfos (because of problems with forward references), but only allow to modify TypeInfos in-place (for example update names symbol table, or similar).

[JukkaL]

Concerning the plugins, I already mentioned this, I would prefer not to allow plugins to replace TypeInfos (because of problems with forward references), but only allow to modify TypeInfos in-place (for example update names symbol table, or similar).

Yeah, just directly modifying TypeInfos should be enough. This shouldn't be hard to support.

We might eventually need some additional features to support specific libraries such as Django models. For example, I believe that Django supports declaring an attribute type using a string literal without explicitly importing the target class, which could make things hard for mypy. The extensions can wait until we have basic support in, however.

[elazarg]

@Tinche, I believe that the right point for a plugin is as a replacement for metaclass[1]. I.e. mypy will read the class, summarize it and send the result to the plugin. The latter, in turn, will return a call to some build_class(fields, bases, ...) function, which will be exposed as part of the plugin API. This way will mimic the actual semantics[2], and will not require fiddling with TypeInfo - it will build the TypeInfo, instead of replacing it or changing it, without being tied to the actual implementation of TypeInfo.

It should be possible to port existing "plugins" such as support for namedtuple, enum and typeddict.

I would like to work on this API, but I think it should wait until after (significant part of) #4083.

[1] I am speaking about metaclass but this is applicable for decorators too.
[2] Essentially specializing an abstract interpreter for the metaclass.

[euresti]

I tried playing around with this. I hacked up mypy to call get_method_signature_hook on all methods (in visit_call_expr) then I added the following code to a plugin:

class MyPlugin(Plugin):

    def get_method_signature_hook(self, fullname):
        if fullname in ('foo.A', 'foo.B'):
            return attr_init_signature_callback
        return None

def attr_init_signature_callback(ctx: MethodSigContext) -> CallableType:
    signature = ctx.default_signature
    names = ctx.type.ret_type.type.names

    arg_types = []
    arg_kinds = []
    arg_names = []
    has_default = set()

    # One way to check for default values is to load the code.
    # There may be a better way to do this.
    cls = namedAny(str(ctx.type.ret_type))
    for field in attr.fields(cls):
        print(field)
        if field.default is not NOTHING:
            has_default.add(field.name)

    for name, table in names.items():
        if table.type:
            arg_names.append(name.lstrip("_"))
            arg_types.append(table.type)
            arg_kinds.append(ARG_OPT if name in has_default else ARG_POS)

    return signature.copy_modified(
        arg_types=arg_types,
        arg_kinds=arg_kinds,
        arg_names=arg_names,
    )

Anyway I believe that if there was a plugin for a class decorator then this could be easily done. I tried looking into where one would add such a thing but I must confess that I got scared. Any pointers?

I have a medium sized code base that uses attrs everywhere and so I can't bring in mypy so I'm motivated to work on this in my spare time.

[gvanrossum]

I don't think there are any docs for plugins, and that's semi-intended -- we don't want people to go develop their own plugins yet and then have them break when the interface changes. So your only source of information is reading the code of the existing plugins. That said, I think it's important that we have this particular plugin in the mypy codebase, so we will take any PRs you send our way seriously!

[ilevkivskyi]

@euresti

Anyway I believe that if there was a plugin for a class decorator then this could be easily done. I tried looking into where one would add such a thing but I must confess that I got scared. Any pointers?

It is a good idea to add a processing step in semanal.py where a TypeInfo generated by semantically analyzing a class definition will be updated by a plugin if there is a decorator with a given full name.

Also note this older comment

Concerning the plugins, I already mentioned this, I would prefer not to allow plugins to replace TypeInfos (because of problems with forward references), but only allow to modify TypeInfos in-place (for example update names symbol table, or similar).

You can ask me if you need some more help with this.

[euresti]

Cool. Just so I see if I'm going into the correct code,

It looks like def analyze_class_body(self, defn: ClassDef) -> Iterator[bool]: is the code that handles the creation of the class along with the decorator. (i.e. it's not treated like applying a function to a type, which is how functions decorators work?)

In there I can check the decorator and do one of a couple of things.

1. I could add a FuncDef to the ClassDef so that the rest of the code believes there is a Function in there called __init__

or

2. I can create a new SymbolTableNode (somehow) and add it to defn.info.names['__init__']

Based on your comment I assume 2 is what we would want.

Now what's the best way to create this SymbolTableNode? I tried SymbolTableNode.deserialize but that ended up giving me a De-serialization failure: TypeInfo not fixed error.

Hmm. Maybe I should join a chat channel somewhere instead of commenting here. Let me know.

[ilevkivskyi]

@euresti

It looks like def analyze_class_body(self, defn: ClassDef) -> Iterator[bool]: is the code that handles the creation of the class along with the decorator.

Yes, you can add a plugin hook somewhre after yield True near the very end of this function.

Based on your comment I assume 2 is what we would want.

Yes, but I think for some technical reasons you will need to create a "fake" FuncDef for __init__, but this is not hard, you can check how build_namedtuple_typeinfo works. Namely the code around

        add_method('__init__', ret=NoneTyp(), name=info.name(),
                   args=[make_init_arg(var) for var in vars])

Something like this should be in your plugin that will be called by the hook.

Now what's the best way to create this SymbolTableNode?

Like any other class, by just calling it. Again, check this in build_namedtuple_typeinfo around this code:

        info.names[var.name()] = SymbolTableNode(MDEF, var)

Hmm. Maybe I should join a chat channel somewhere instead of commenting here. Let me know.

I think it is OK to discuss here. Or instead you can make a WIP PR and we can continue there. You can mention @Tinche there, he also seems to be interested in this.

Just to reiterate, there are two separate steps:

• Create plugin infrastructure for updating analyzed classes if there are some special decorators and/or base classes and/or metaclass.
• Write an actual attrs plugin.

[euresti]

Ok cool. I've played with add_method and got it to work for one attr.s case, which means this is actually possible. But I have a question about the plugin itself.

You mentioned that we should only be modifying TypeInfo but none of the plugins take TypeInfos they all take Type or Expression. I can still extract the TypeInfo from

I'm thinking something like this could work.

ClassDecoratorContext = NamedTuple(
    'ClassDecoratorContext', [
        ('decorator', Expression),
        ('cls', Type),
        ('context', Context),
        ('api', CheckerPluginInterface)])

Also which part of the code should do the info.names[funcname] = node Should the plugin do that or should the caller of the plugin do it? I ask because all of the current plugins return something rather then mutate something passed in.

Ok here's a proposal and you can tell me if I'm way off.

The plugin takes the ClassDecoratorContext above and returns a dictionary of name -> SymbolTableNode Then in analyze_class_body after the yield True. We call the plugin and iterate over the return dictionary adding the SymbolTableNodes to the info.name[name]

This should make it possible to allow the plugin to add other methods if it wants. Let me know!

[euresti]

Oh wait. Just re-read the thread:

Concerning the plugins, I already mentioned this, I would prefer not to allow plugins to replace TypeInfos (because of problems with forward references), but only allow to modify TypeInfos in-place (for example update names symbol table, or similar).

ClassDecoratorContext = NamedTuple(
    'ClassDecoratorContext', [
        ('decorator', Expression),
        ('cls', TypeInfo),
        ('api', SemanticAnalysisPluginInterface)])

And the function will return None I guess.

[ilevkivskyi]

@euresti

You mentioned that we should only be modifying TypeInfo but none of the plugins take TypeInfos they all take Type or Expression.

Yes. You will need to add a new plugin kind. If you have time, I think it makes sense (to be future proof) to ensure that plugins can be triggered not only by a specific decorator full name, but also by a specific metaclass or base class full name.

Oh wait. Just re-read the thread:

Yes, this looks like a reasonable API for the plugin. The plugin should modify TypeInfo in place and return None. I think however it is better to pass the whole ClassDef AST node (again, to be future proof) then the decorator expressions can be easily extracted by the plugin when necessary.

[euresti]

@ilevkivskyi Just added an RFC PR. (Sorry for pinging, I don't know if github notifies when PRs are added)

[euresti]

@Tinche @hynek Now that the plugin scaffolding is nearly finished I started working on the actual plugin. You can see the code here: https://github.com/euresti/mypy/blob/attrs_plugin/mypy/plugin.py#L421

It actually works!

import attr
import typing


@attr.s(auto_attribs=True)
class Auto:
    normal: int
    _private: int
    def_arg: int = 18
    _def_kwarg: int = attr.ib(validator=None, default=18)

    FOO: typing.ClassVar[int] = 18

reveal_type(Auto)
Auto(1, 2, 3)


@attr.s
class Typed:
    normal: int = attr.ib()
    _private: int = attr.ib()
    def_arg: int = attr.ib(18)
    _def_kwarg: int = attr.ib(validator=None, default=18)

    FOO: int = 18

reveal_type(Typed)   
Typed(1, 2, 3)


@attr.s
class UnTyped:
    normal = attr.ib()
    _private = attr.ib()
    def_arg = attr.ib(18)
    _def_kwarg = attr.ib(validator=None, default=18)

    FOO = 18

reveal_type(UnTyped)
UnTyped(1, 2, 3)

Which now gives the following output from mypy:

foo.py:14: error: Revealed type is 'def (normal: builtins.int, private: builtins.int, def_arg: builtins.int =, def_kwarg: builtins.int =) -> foo.Auto'
foo.py:27: error: Revealed type is 'def (normal: builtins.int, private: builtins.int, def_arg: builtins.int =, def_kwarg: builtins.int =) -> foo.Typed'
foo.py:33: error: Need type annotation for variable
foo.py:34: error: Need type annotation for variable
foo.py:35: error: Need type annotation for variable
foo.py:36: error: Need type annotation for variable
foo.py:40: error: Revealed type is 'def (normal: Any, private: Any, def_arg: Any =, def_kwarg: Any =) -> foo.UnTyped'

But I'm not sure if this code belongs in mypy or in attrs or maybe in a completely different project altogether? Also the pyi file has to go somewhere but I'm not sure if it goes in the typeshed or somewhere else. Please advise.

Also I imagine dataclasses will look pretty similar to the auto_attrib case.

[Tinche]

Cool! I can take a deeper look when I have time.

Now might be a good time to consider where this code might actually live. Does mypy support this plugin living outside and somehow picking it up after it being pip installed? We will want to have this plugin versioned according to which attrs version it can support.

[hynek]

Great work David, I'm super excited that someone found the time to pull this thru!

I think part of the question where to put it also kind of depends on how the plugin APIs will evolve? It would be unfortunate if certain versions of attrs only worked with certain versions of mypy.

So I think it might make sense to to put it either into mypy or start a new package in the python-attrs orga? attrs proper seems like the worst possible place at this point.

[ilevkivskyi]

@euresti Stub files should go to typeshed. After PEP 561 is accepted and implemented, you can host the stub wherever you want and distribute them via PyPI/pip install.

As for the plugin it is less clear. Currently, mypy doesn't support plugins installed via pip install, but I think that the long term goal is to support this. Therefore, there are three options:

1. First add the attrs plugin for mypy as a PR to mypy, then implement the installable plugin infrastructure.
2. First implement the full infrastructure, then publish the attrs plugin on PyPI.
3. Work on two projects in parallel.

I think option 1 is the fastest way to get the attrs support. I am leaning towards this option, but I don't have a strong preference.

[hynek]

I think option 1 is the best for both projects until your plugin API stabilises. In the end you’ll get failing tests if you break something. :)

Once the plugin API is solid, we can talk about extracting it into a separate project.

attrs proper seems like a terrible option in every way.

[JukkaL]

I agree that including the plugin with mypy is the right course of action. It may be a while before mypy supports installing plugins through pip.

[ilevkivskyi]

@euresti OK, it looks like everyone likes the first option, so go ahead and make a PR to mypy.