python / mypy

Optional static typing for Python
https://www.mypy-lang.org/
Other
18.17k stars 2.77k forks source link

Supporting attrs extensions with different default arguments for decorators. #12774

Open t4lz opened 2 years ago

t4lz commented 2 years ago

attrs' decorators can (and sometimes should) be wrapped, to create custom decorators. Using a "fake plugin", the custom decorators can be added to the maker lists and thus receive the special attrs treatment from mypy. However, currently, mypy's attrs plugin has the default values of the arguments of attrs' decorators hardcoded. This means that it does not detect when the custom decorators define different default values, and thus raises (false positive) errors on some cases.

Here is an example for such a case:

import attr
from typing import Any

def my_attr_s(cls, kw_only: bool = True) -> Any:  # Added to attr_class_makers using "fake plugin".
    return attr.s(kw_only=kw_only)(cls)

def my_attr_ib(**kwargs) -> Any:  # Added to attr_attrib_makers using "fake plugin".
    return attr.ib(**kwargs)

@my_attr_s
class A:
    optional: str = my_attr_ib(default="This attrib is not required now.")

@my_attr_s
class B(A):
    required: str = my_attr_ib()

When type-checking this file, mypy gives the following error:

16: error: Non-default attributes not allowed after default attributes.

This error should actually not be emitted, because as opposed to what would happen if we we decorate A and B with attr.s, there is no problem here, because here we have kw_only set to True (by default, and nothing else was passed), so the order of the attributes is actually valid.

If mypy would take the decorator's arguments' default values for arguments that were not passed, it could avoid such false positives. I would like to submit a PR which fixes this.

erictraut commented 2 years ago

PEP 681 (dataclass_transform) provides a potential solution here if and when mypy implements support for it. You could use it to annotate the decorator in a way that modifies the default behavior for attr.s.

t4lz commented 2 years ago

PEP 681 looks great to me. Support for that in mypy would definitely address this issue. I would suggest still merging the open PR in order to support this use-case until there is support for PEP 681 in mypy, since this PR improves support for dataclass-like decorators right now, without complicating the potential implementation of PEP 681 support.

ri-gilfanov commented 2 years ago

@t4lz, is the following behavior related to this issue?

Example:

from attrs import define

DEFINE_KW: dict[str, bool] = {'frozen': True, 'hash': True, 'cache_hash': True, 'slots': True}

@define(**DEFINE_KW)
class MaxLen:
    value: int

Mypy output:

example.py:6: error: No overload variant of "define" matches argument type "Dict[str, bool]"
example.py:6: note: Possible overload variants:
example.py:6: note:     def [_C <: type] define(maybe_cls: _C, *, these: Optional[Dict[str, Any]] = ..., repr: bool = ..., hash: Optional[bool] = ..., init: bool = ..., slots: bool = ..., frozen: bool = ..., weakref_slot: bool = ..., str: bool = ..., auto_attribs: bool = ..., kw_only: bool = ..., cache_hash: bool = ..., auto_exc: bool = ..., eq: Optional[bool] = ..., order: Optional[bool] = ..., auto_detect: bool = ..., getstate_setstate: Optional[bool] = ..., on_setattr: Union[Callable[[Any, Attribute[Any], Any], Any], List[Callable[[Any, Attribute[Any], Any], Any]], _NoOpType, None] = ..., field_transformer: Optional[Callable[[type, List[Attribute[Any]]], List[Attribute[Any]]]] = ..., match_args: bool = ...) -> _C
example.py:6: note:     def define(maybe_cls: None = ..., *, these: Optional[Dict[str, Any]] = ..., repr: bool = ..., hash: Optional[bool] = ..., init: bool = ..., slots: bool = ..., frozen: bool = ..., weakref_slot: bool = ..., str: bool = ..., auto_attribs: bool = ..., kw_only: bool = ..., cache_hash: bool = ..., auto_exc: bool = ..., eq: Optional[bool] = ..., order: Optional[bool] = ..., auto_detect: bool = ..., getstate_setstate: Optional[bool] = ..., on_setattr: Union[Callable[[Any, Attribute[Any], Any], Any], List[Callable[[Any, Attribute[Any], Any], Any]], _NoOpType, None] = ..., field_transformer: Optional[Callable[[type, List[Attribute[Any]]], List[Attribute[Any]]]] = ..., match_args: bool = ...) -> Callable[[_C], _C]

Software environment:

t4lz commented 2 years ago

@ri-gilfanov not really.

If you are really set on unpacking a dictionary there, this issue and the linked PR do not facilitate this use-case. If, however, you are trying to do something else, like avoiding repetition, and are willing to do it without dictionary unpacking, then I guess maybe this issue and PR could perhaps be kind of relevant as it would enable you to define a decorator which just wraps define and sets other defaults, and then use that decorator on all classes, like this:

from attrs import define

 # This needs to be added to attr_define_makers using a "fake plugin".
def my_define(cls, frozen=True, hash=True, cache_hash=True, slots=True) -> Any:
    return define(frozen=frozen, hash=hash, cache_hash=cache_hash, slots=slots)(cls)

@my_define
class MaxLen:
    value: int