Skip to content

Commit

Permalink
Fix crash in dataclass protocol with self attribute assignment (#15157)
Browse files Browse the repository at this point in the history
Fix #15004 

FWIW I don't think dataclass protocols make much sense, but we
definitely should not crash. Also the root cause has nothing to do with
dataclasses, the problem is that a self attribute assignment in a
protocol created a new `Var` (after an original `Var` was created in
class body), which is obviously wrong.
  • Loading branch information
ilevkivskyi authored May 2, 2023
1 parent 9f69bea commit b5107a9
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 5 deletions.
18 changes: 13 additions & 5 deletions mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -3647,7 +3647,7 @@ def analyze_lvalue(
has_explicit_value=has_explicit_value,
)
elif isinstance(lval, MemberExpr):
self.analyze_member_lvalue(lval, explicit_type, is_final)
self.analyze_member_lvalue(lval, explicit_type, is_final, has_explicit_value)
if explicit_type and not self.is_self_member_ref(lval):
self.fail("Type cannot be declared in assignment to non-self attribute", lval)
elif isinstance(lval, IndexExpr):
Expand Down Expand Up @@ -3824,7 +3824,9 @@ def analyze_tuple_or_list_lvalue(self, lval: TupleExpr, explicit_type: bool = Fa
has_explicit_value=True,
)

def analyze_member_lvalue(self, lval: MemberExpr, explicit_type: bool, is_final: bool) -> None:
def analyze_member_lvalue(
self, lval: MemberExpr, explicit_type: bool, is_final: bool, has_explicit_value: bool
) -> None:
"""Analyze lvalue that is a member expression.
Arguments:
Expand Down Expand Up @@ -3853,12 +3855,18 @@ def analyze_member_lvalue(self, lval: MemberExpr, explicit_type: bool, is_final:
and explicit_type
):
self.attribute_already_defined(lval.name, lval, cur_node)
# If the attribute of self is not defined in superclasses, create a new Var, ...
if self.type.is_protocol and has_explicit_value and cur_node is not None:
# Make this variable non-abstract, it would be safer to do this only if we
# are inside __init__, but we do this always to preserve historical behaviour.
if isinstance(cur_node.node, Var):
cur_node.node.is_abstract_var = False
if (
# If the attribute of self is not defined, create a new Var, ...
node is None
or (isinstance(node.node, Var) and node.node.is_abstract_var)
# ... or if it is defined as abstract in a *superclass*.
or (cur_node is None and isinstance(node.node, Var) and node.node.is_abstract_var)
# ... also an explicit declaration on self also creates a new Var.
# Note that `explicit_type` might has been erased for bare `Final`,
# Note that `explicit_type` might have been erased for bare `Final`,
# so we also check if `is_final` is passed.
or (cur_node is None and (explicit_type or is_final))
):
Expand Down
13 changes: 13 additions & 0 deletions test-data/unit/check-dataclasses.test
Original file line number Diff line number Diff line change
Expand Up @@ -2037,3 +2037,16 @@ Foo(
present_5=5,
)
[builtins fixtures/dataclasses.pyi]

[case testProtocolNoCrash]
from typing import Protocol, Union, ClassVar
from dataclasses import dataclass, field

DEFAULT = 0

@dataclass
class Test(Protocol):
x: int
def reset(self) -> None:
self.x = DEFAULT
[builtins fixtures/dataclasses.pyi]
11 changes: 11 additions & 0 deletions test-data/unit/check-protocols.test
Original file line number Diff line number Diff line change
Expand Up @@ -4054,3 +4054,14 @@ class P(Protocol):

[file lib.py]
class C: ...

[case testAllowDefaultConstructorInProtocols]
from typing import Protocol

class P(Protocol):
x: int
def __init__(self, x: int) -> None:
self.x = x

class C(P): ...
C(0) # OK

0 comments on commit b5107a9

Please sign in to comment.