[attrs plugin] Support kw_only=True #6107
Conversation
| auto_attribs: bool = ...) -> Callable[[_C], _C]: ... | ||
| auto_attribs: bool = ..., | ||
| kw_only: bool = ..., | ||
| cache_hash: bool = ..., |
There was a problem hiding this comment.
I noticed that some of these are missing in typeshed. Could you please make a typeshed update PR?
There was a problem hiding this comment.
Now that attrs includes stubs in the package maybe removing them from typeshed is better. python-attrs/attrs#480
test-data/unit/fine-grained.test
Outdated
| b = attr.ib("16") # type: str | ||
|
|
||
| B(b="foo", a=7) | ||
|
|
There was a problem hiding this comment.
This empty line is not needed.
| == | ||
| b.py:5: error: Non keyword-only attributes are not allowed after a keyword-only attribute. | ||
|
|
||
|
|
There was a problem hiding this comment.
We prefer single empty line between tests.
| [builtins fixtures/attr.pyi] | ||
| [out] | ||
| == | ||
| b.py:5: error: Non keyword-only attributes are not allowed after a keyword-only attribute. |
There was a problem hiding this comment.
This test doesn't test much of incremental logic. I would also add a test where base class in a.py is updated, while subclass is unchanged.
mypy/plugins/attrs.py
Outdated
| @@ -367,6 +398,11 @@ def _attribute_from_attrib_maker(ctx: 'mypy.plugin.ClassDefContext', | |||
|
|
|||
| # Read all the arguments from the call. | |||
| init = _get_bool_argument(ctx, rvalue, 'init', True) | |||
| kw_only |= _get_bool_argument(ctx, rvalue, 'kw_only', False) | |||
| if kw_only and ctx.api.options.python_version[0] < 3: | |||
| ctx.api.fail("kw_only is not supported in Python 2", stmt) | |||
There was a problem hiding this comment.
I would avoid repeating the same error message, and instead define a constant.
| @@ -367,6 +398,11 @@ def _attribute_from_attrib_maker(ctx: 'mypy.plugin.ClassDefContext', | |||
|
|
|||
| # Read all the arguments from the call. | |||
| init = _get_bool_argument(ctx, rvalue, 'init', True) | |||
| kw_only |= _get_bool_argument(ctx, rvalue, 'kw_only', False) | |||
There was a problem hiding this comment.
I noticed this (a bit counterintuitive) behavior:
import attr
@attr.s(kw_only=True)
class A:
a = attr.ib(kw_only=False)
A(1) # ErrorThe logic here captures it correctly, but I would add a short comment that this is intentional.
There was a problem hiding this comment.
I filed an issue in attrs to see if this is expected: python-attrs/attrs#481
| def _analyze_class(ctx: 'mypy.plugin.ClassDefContext', auto_attribs: bool) -> List[Attribute]: | ||
| def _analyze_class(ctx: 'mypy.plugin.ClassDefContext', | ||
| auto_attribs: bool, | ||
| kw_only: bool) -> List[Attribute]: |
There was a problem hiding this comment.
Could you please document kw_only (and auto_attribs) in the docstring?
|
Ok I made the requested changes. |
Fixes #6083