Skip to content

Fix @dataclass to catch default / non-default fields order #12159

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.

Sign up for GitHub

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

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Fix @dataclass to catch default / non-default fields order #12159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
797532a
Fix `@dataclass` to catch default / non-default fields order
sobolevn Feb 10, 2022
0baa494
Do not load super-attrs if not needed
sobolevn Feb 10, 2022
54b013e
Adds new test for `kw_only` overrides
sobolevn Feb 13, 2022
a85baac
Improve comments
sobolevn Feb 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 49 additions & 23 deletions mypy/plugins/dataclasses.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,10 +400,10 @@ def collect_attributes(self) -> Optional[List[DataclassAttribute]]:
for data in info.metadata["dataclass"]["attributes"]:
name: str = data["name"]
if name not in known_attrs:
attr = DataclassAttribute.deserialize(info, data, ctx.api)
attr.expand_typevar_from_subtype(ctx.cls.info)
super_attr = DataclassAttribute.deserialize(info, data, ctx.api)
super_attr.expand_typevar_from_subtype(ctx.cls.info)
known_attrs.add(name)
super_attrs.append(attr)
super_attrs.append(super_attr)
elif all_attrs:
# How early in the attribute list an attribute appears is determined by the
# reverse MRO, not simply MRO.
Expand All @@ -413,6 +413,13 @@ def collect_attributes(self) -> Optional[List[DataclassAttribute]]:
if attr.name == name:
all_attrs.remove(attr)
super_attrs.append(attr)
# When we have an override,
# we need to check its compatibility.
self._validate_field(
attr,
found_default=data.get("has_default", False),
found_kw_sentinel=False,
)
break
all_attrs = super_attrs + all_attrs
all_attrs.sort(key=lambda a: a.kw_only)
Expand All @@ -423,31 +430,50 @@ def collect_attributes(self) -> Optional[List[DataclassAttribute]]:
# Ensure that the KW_ONLY sentinel is only provided once
found_kw_sentinel = False
for attr in all_attrs:
# If we find any attribute that is_in_init, not kw_only, and that
# doesn't have a default after one that does have one,
# then that's an error.
if found_default and attr.is_in_init and not attr.has_default and not attr.kw_only:
# If the issue comes from merging different classes, report it
# at the class definition point.
context = (Context(line=attr.line, column=attr.column) if attr in attrs
else ctx.cls)
ctx.api.fail(
'Attributes without a default cannot follow attributes with one',
context,
)

# If the issue comes from merging different classes, report it
# at the class definition point.
context = (
Context(line=attr.line, column=attr.column)
if attr in attrs
else ctx.cls
)
self._validate_field(
attr,
found_default=found_default,
found_kw_sentinel=found_kw_sentinel,
context=context,
)
found_default = found_default or (attr.has_default and attr.is_in_init)
if found_kw_sentinel and self._is_kw_only_type(attr.type):
context = (Context(line=attr.line, column=attr.column) if attr in attrs
else ctx.cls)
ctx.api.fail(
'There may not be more than one field with the KW_ONLY type',
context,
)
found_kw_sentinel = found_kw_sentinel or self._is_kw_only_type(attr.type)

return all_attrs

def _validate_field(
self,
attr: DataclassAttribute,
*,
found_default: bool,
found_kw_sentinel: bool,
context: Optional[Context] = None,
) -> None:
if context is None:
context = self._ctx.cls

# If we find any attribute that is_in_init, not kw_only, and that
# doesn't have a default after one that does have one,
# then that's an error.
if found_default and attr.is_in_init and not attr.has_default and not attr.kw_only:
self._ctx.api.fail(
'Attributes without a default cannot follow attributes with one',
context,
)

if found_kw_sentinel and self._is_kw_only_type(attr.type):
self._ctx.api.fail(
'There may not be more than one field with the KW_ONLY type',
context,
)

def _freeze(self, attributes: List[DataclassAttribute]) -> None:
"""Converts all attributes to @property methods in order to
emulate frozen classes.
Expand Down
48 changes: 48 additions & 0 deletions test-data/unit/check-dataclasses.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,26 @@ class Base:

[builtins fixtures/dataclasses.pyi]

[case testDataclassesKwOnlyOverrideInSubclass]
# flags: --python-version 3.7
from dataclasses import dataclass, field, KW_ONLY

@dataclass
class Base:
x: int
y: int = field(kw_only=True)
_: KW_ONLY
z: int

@dataclass
class Other(Base):
x: int
y: int
z: int

Other(1, 2, 3) # should be ok
[builtins fixtures/dataclasses.pyi]

[case testDataclassesClassmethods]
# flags: --python-version 3.7
from dataclasses import dataclass
Expand Down Expand Up @@ -1536,3 +1556,31 @@ A(a=1, b=2)
A(1)
A(a="foo") # E: Argument "a" to "A" has incompatible type "str"; expected "int"
[builtins fixtures/dataclasses.pyi]

[case testDataclassesFieldWithDefaultOverride]
# flags: --python-version 3.7
from dataclasses import dataclass

@dataclass
class A:
a: int = 0

@dataclass
class Correct1(A):
a: int = 0
@dataclass
class Correct2(A):
a: int = 1
b: int = 2

@dataclass
class Wrong1(A): # E: Attributes without a default cannot follow attributes with one
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interestingly the runtime accepts this one. It's pretty weird, though, and I'm OK with mypy giving an error for it.

a: int
@dataclass
class Wrong2(A): # E: Attributes without a default cannot follow attributes with one
a: int
b: int
@dataclass
class Wrong3(A):
b: int # E: Attributes without a default cannot follow attributes with one
[builtins fixtures/dataclasses.pyi]