Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

attrs plugin should support attrs.evolve #14525

Closed
ikonst opened this issue Jan 25, 2023 · 6 comments · Fixed by #14526
Closed

attrs plugin should support attrs.evolve #14525

ikonst opened this issue Jan 25, 2023 · 6 comments · Fixed by #14526

Comments

@ikonst
Copy link
Contributor

ikonst commented Jan 25, 2023

Feature

In the attrs plugin, we should add special treatment for attrs.evolve.

Pitch

We should validate attr names and types match.

@tmke8
Copy link
Contributor

tmke8 commented Jan 27, 2023

Similar treatment for dataclasses.replace would also be nice. (There was previously PR #14263 but that one only checked that the first argument to replace is a dataclass – which can also be done by just modifying the type stubs.)

@ikonst
Copy link
Contributor Author

ikonst commented Jan 27, 2023

By modifying the type annotations? I don't think dataclasses have any common base, do they? Or maybe through a protocol?

@tmke8
Copy link
Contributor

tmke8 commented Jan 27, 2023

By modifying the type annotations? I don't think dataclasses have any common base, do they? Or maybe through a protocol?

Yes, @AlexWaygood suggested doing it with a protocol: python/typeshed#9362

@AlexWaygood
Copy link
Member

By modifying the type annotations? I don't think dataclasses have any common base, do they? Or maybe through a protocol?

Yes, @AlexWaygood suggested doing it with a protocol: python/typeshed#9362

Applying the same technique to attrs would be difficult unless all type checkers could be counted on to synthesise __attrs_attrs__ attributes for attrs classes in the same way that they can be counted on to synthesise __dataclass_fields__ attributes for classes decorated with @dataclass. Mypy has a custom plugin for attrs, meaning it can be counted on to synthesise this attribute. On the other hand, pyright has (reasonably) resisted adding any special-casing for any third-party libraries.

@ikonst
Copy link
Contributor Author

ikonst commented Jan 27, 2023

If attrs ships its own type annotations, then they could decide that e.g. you could only used them with mypy and a certain plugin being installed, but I agree we cannot make this decision for typeshed's annotations.

And for this case, the plugin can easily make this check, and I would have to in #14526 anyway.

@AlexWaygood
Copy link
Member

To be clear, attrs does ship its own type annotations, and typeshed does not have stubs for attrs. If somebody did want to try a solution for attrs along the lines of my dataclasses PR to typeshed, they'd have to implement it in the annotations shipped by attrs, not in typeshed.

JelleZijlstra pushed a commit that referenced this issue Mar 6, 2023
Validate `attr.evolve` calls to specify correct arguments and types.

The implementation makes it so that at every point where `attr.evolve`
is called, the signature is modified to expect the attrs class'
initializer's arguments (but so that they're all kw-only and optional).

Notes:
- Added `class dict: pass` to some fixtures files since our attrs type
stubs now have **kwargs and that triggers a `builtin.dict` lookup in
dozens of attrs tests.
- Looking up the type of the 1st argument with
`ctx.api.expr_checker.accept(inst_arg)` which is a hack since it's not
part of the plugin API. This is a compromise for due to #10216.

Fixes #14525.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants