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

Adds support for __slots__ assignment, refs #10801 #10864

Merged
merged 15 commits into from
Sep 21, 2021
13 changes: 13 additions & 0 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -2086,6 +2086,19 @@ def accept_items(e: Expression) -> None:
def check_assignment(self, lvalue: Lvalue, rvalue: Expression, infer_lvalue_type: bool = True,
new_syntax: bool = False) -> None:
"""Type check a single assignment: lvalue = rvalue."""
if isinstance(lvalue, MemberExpr) and lvalue.node:
name = lvalue.node.name
inst = get_proper_type(self.expr_checker.accept(lvalue.expr))
if (isinstance(inst, Instance) and
Copy link
Collaborator

Choose a reason for hiding this comment

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

If a class has a mix of slots and non-slots members, wouldn't this report an error incorrectly?

E.g.

class A:
    __slots__ = ('a',)
    b = 4

Edit: ah based on the tests it looks like this does not report an error incorrectly

Copy link
Member Author

Choose a reason for hiding this comment

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

I will add a test case for this, thank you. It actually was not consistent with Python's runtime behavior:

class A:
    __slots__ = ('a',)
    b = 4

    def __init__(self) -> None:
        self.a = 1
        self.b = 2  # error here in runtime

A()
# AttributeError: 'A' object attribute 'b' is read-only

I will change the code to make this an error with mypy as well.

inst.type.slots is not None and
name not in inst.type.slots):
self.fail(
'Trying to assign name "{}" that is not in "__slots__" of type "{}"'.format(
name, inst.type.fullname,
),
lvalue,
)

if isinstance(lvalue, TupleExpr) or isinstance(lvalue, ListExpr):
self.check_assignment_to_multiple_lvalues(lvalue.items, rvalue, rvalue,
infer_lvalue_type)
Expand Down
5 changes: 5 additions & 0 deletions mypy/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -2298,6 +2298,10 @@ class is generic then it will be a type constructor of higher kind.
runtime_protocol = False # Does this protocol support isinstance checks?
abstract_attributes: List[str]
deletable_attributes: List[str] # Used by mypyc only
# Does this type have concrete `__slots__` defined?
# If class does not have `__slots__` defined then it is `None`,
# if it has empty `__slots__` then it is an empty set.
slots: Optional[Set[str]]

# The attributes 'assuming' and 'assuming_proper' represent structural subtype matrices.
#
Expand Down Expand Up @@ -2401,6 +2405,7 @@ def __init__(self, names: 'SymbolTable', defn: ClassDef, module_name: str) -> No
self.is_abstract = False
self.abstract_attributes = []
self.deletable_attributes = []
self.slots = None
self.assuming = []
self.assuming_proper = []
self.inferring = []
Expand Down
57 changes: 57 additions & 0 deletions mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -2038,6 +2038,7 @@ def visit_assignment_stmt(self, s: AssignmentStmt) -> None:
self.process_module_assignment(s.lvalues, s.rvalue, s)
self.process__all__(s)
self.process__deletable__(s)
self.process__slots__(s)

def analyze_identity_global_assignment(self, s: AssignmentStmt) -> bool:
"""Special case 'X = X' in global scope.
Expand Down Expand Up @@ -3353,6 +3354,62 @@ def process__deletable__(self, s: AssignmentStmt) -> None:
assert self.type
self.type.deletable_attributes = attrs

def process__slots__(self, s: AssignmentStmt) -> None:
"""
Processing ``__slots__`` if defined in type.

See: https://docs.python.org/3/reference/datamodel.html#slots
"""
# Later we can support `__slots__` defined as `__slots__ = other = ('a', 'b')`
if (isinstance(self.type, TypeInfo) and
len(s.lvalues) == 1 and isinstance(s.lvalues[0], NameExpr) and
s.lvalues[0].name == '__slots__' and s.lvalues[0].kind == MDEF):

# We understand `__slots__` defined as string, tuple, list, set, and dict:
if not isinstance(s.rvalue, (StrExpr, ListExpr, TupleExpr, SetExpr, DictExpr)):
# For example, `__slots__` can be defined as a variable,
# we don't support it for now.
return

if any(p.slots is None for p in self.type.mro[1:-1]):
# At least one type in mro (excluding `self` and `object`)
# does not have concrete `__slots__` defined. Ignoring.
return
sobolevn marked this conversation as resolved.
Show resolved Hide resolved

concrete_slots = True
rvalue: List[Expression] = []
if isinstance(s.rvalue, StrExpr):
rvalue.append(s.rvalue)
elif isinstance(s.rvalue, (ListExpr, TupleExpr, SetExpr)):
rvalue.extend(s.rvalue.items)
else:
# We have a special treatment of `dict` with possible `{**kwargs}` usage.
# In this case we consider all `__slots__` to be non-concrete.
for key, _ in s.rvalue.items:
if concrete_slots and key is not None:
rvalue.append(key)
else:
concrete_slots = False

slots = []
for item in rvalue:
# Special case for `'__dict__'` value:
# when specified it will still allow any attribute assignment.
if isinstance(item, StrExpr) and item.value != '__dict__':
slots.append(item.value)
else:
concrete_slots = False
if not concrete_slots:
# Some slot items are dynamic, we don't want any false positives,
# so, we just pretend that this type does not have any slots at all.
return

# We need to copy all slots from super types:
for super_type in self.type.mro[1:-1]:
assert super_type.slots is not None
slots.extend(super_type.slots)
self.type.slots = set(slots)

#
# Misc statements
#
Expand Down
1 change: 1 addition & 0 deletions mypy/test/testcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
'check-typeguard.test',
'check-functools.test',
'check-singledispatch.test',
'check-slots.test',
]

# Tests that use Python 3.8-only AST features (like expression-scoped ignores):
Expand Down
278 changes: 278 additions & 0 deletions test-data/unit/check-slots.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,278 @@
[case testSlotsDefinitionWithStrAndListAndTuple]
class A:
__slots__ = "a"
def __init__(self) -> None:
self.a = 1
self.b = 2 # E: Trying to assign name "b" that is not in "__slots__" of type "__main__.A"

class B:
__slots__ = ("a", "b")
def __init__(self) -> None:
self.a = 1
self.b = 2
self.c = 3 # E: Trying to assign name "c" that is not in "__slots__" of type "__main__.B"

class C:
__slots__ = ['c']
def __init__(self) -> None:
self.a = 1 # E: Trying to assign name "a" that is not in "__slots__" of type "__main__.C"
self.c = 3

class WithVariable:
__fields__ = ['a', 'b']
__slots__ = __fields__

def __init__(self) -> None:
self.a = 1
self.b = 2
self.c = 3
[builtins fixtures/tuple.pyi]
[builtins fixtures/list.pyi]


[case testSlotsDefinitionWithDict]
class D:
__slots__ = {'key': 'docs'}
def __init__(self) -> None:
self.key = 1
self.missing = 2 # E: Trying to assign name "missing" that is not in "__slots__" of type "__main__.D"
[builtins fixtures/dict.pyi]


[case testSlotsDefinitionWithDynamicDict]
slot_kwargs = {'b': 'docs'}
class WithDictKwargs:
__slots__ = {'a': 'docs', **slot_kwargs}
def __init__(self) -> None:
self.a = 1
self.b = 2
self.c = 3
[builtins fixtures/dict.pyi]


[case testSlotsDefinitionWithSet]
class E:
__slots__ = {'e'}
def __init__(self) -> None:
self.e = 1
self.missing = 2 # E: Trying to assign name "missing" that is not in "__slots__" of type "__main__.E"
[builtins fixtures/set.pyi]


[case testSlotsDefinitionOutsideOfClass]
__slots__ = ("a", "b")
class A:
def __init__(self) -> None:
self.x = 1
self.y = 2
[builtins fixtures/tuple.pyi]


[case testSlotsDefinitionMutlipleVars1]
sobolevn marked this conversation as resolved.
Show resolved Hide resolved
class A:
__slots__ = __fields__ = ("a", "b")
def __init__(self) -> None:
self.x = 1
self.y = 2
[builtins fixtures/tuple.pyi]



[case testSlotsDefinitionMutlipleVars2]
sobolevn marked this conversation as resolved.
Show resolved Hide resolved
class A:
__fields__ = __slots__ = ("a", "b")
def __init__(self) -> None:
self.x = 1
self.y = 2
[builtins fixtures/tuple.pyi]


[case testSlotsAssignmentEmptySlots]
class A:
__slots__ = ()
def __init__(self) -> None:
self.a = 1 # E: Trying to assign name "a" that is not in "__slots__" of type "__main__.A"
self.b = 2 # E: Trying to assign name "b" that is not in "__slots__" of type "__main__.A"
sobolevn marked this conversation as resolved.
Show resolved Hide resolved

a = A()
a.a = 1
a.b = 2
a.missing = 2 # E: "A" has no attribute "missing"
[builtins fixtures/tuple.pyi]


[case testSlotsAssignmentWithSuper]
class A:
__slots__ = ("a",)
class B(A):
__slots__ = ("b", "c")

def __init__(self) -> None:
self.a = 1
self.b = 2
self._one = 1 # E: Trying to assign name "_one" that is not in "__slots__" of type "__main__.B"

b = B()
b.a = 1
b.b = 2
b.c = 3 # E: "B" has no attribute "c"
b._two = 2 # E: "B" has no attribute "_two"
[builtins fixtures/tuple.pyi]


[case testSlotsAssignmentWithSuperDuplicateSlots]
class A:
__slots__ = ("a",)
class B(A):
__slots__ = ("a", "b",)

def __init__(self) -> None:
self.a = 1
self.b = 2
self._one = 1 # E: Trying to assign name "_one" that is not in "__slots__" of type "__main__.B"
[builtins fixtures/tuple.pyi]


[case testSlotsAssignmentWithMixing]
class A:
__slots__ = ("a",)
class Mixin:
__slots__ = ("m",)
class B(A, Mixin):
__slots__ = ("b",)

def __init__(self) -> None:
self.a = 1
self.m = 2
self._one = 1 # E: Trying to assign name "_one" that is not in "__slots__" of type "__main__.B"

b = B()
b.a = 1
b.m = 2
b.b = 2 # E: "B" has no attribute "b"
b._two = 2 # E: "B" has no attribute "_two"
[builtins fixtures/tuple.pyi]


[case testSlotsAssignmentWithSlottedSuperButNoChildSlots]
class A:
__slots__ = ("a",)
class B(A):
def __init__(self) -> None:
self.a = 1
self.b = 1

b = B()
b.a = 1
b.b = 2
[builtins fixtures/tuple.pyi]


[case testSlotsAssignmentWithoutSuperSlots]
class A:
pass # no slots
class B(A):
__slots__ = ("a", "b")

def __init__(self) -> None:
self.a = 1
self.b = 2
self.missing = 3
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't this an error if B didn't inherit from another class? It seems weird to me this would be accepted since it isn't in the __slots__ for B nor in the mro.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is how Python works in runtime. If any base class has __dict__ that the child class itself would have __dict__. So, when using B(A) you will implicitly have __dict__ in place. Try this:

class A:
    pass  # no slots

class B(A):
    __slots__ = ("a", "b")

    def __init__(self) -> None:
        self.a = 1
        self.b = 2
        self.missing = 3

b = B()
b.a = 1
b.b = 2
b.missing = 3
b.extra = 4  # E: "B" has no attribute "extra"

But, without A super class it would be different:

class B:
    __slots__ = ("a", "b")

    def __init__(self) -> None:
        self.a = 1
        self.b = 2
        self.missing = 3  # AttributeError: 'B' object has no attribute 'missing'

b = B()

Copy link
Member Author

Choose a reason for hiding this comment

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

I think, that I need to add this to the docs of mypy. Do you agree?
What would be the best place for it? Common issues?

Copy link
Collaborator

@emmatyping emmatyping Sep 15, 2021

Choose a reason for hiding this comment

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

Aha, I see, that makes more sense. I would probably add it to class basics: https://mypy.readthedocs.io/en/stable/class_basics.html#class-basics


b = B()
b.a = 1
b.b = 2
b.missing = 3
b.extra = 4 # E: "B" has no attribute "extra"
[builtins fixtures/tuple.pyi]


[case testSlotsAssignmentWithoutSuperMixingSlots]
class A:
__slots__ = ()
class Mixin:
pass # no slots
class B(A, Mixin):
__slots__ = ("a", "b")

def __init__(self) -> None:
self.a = 1
self.b = 2
self.missing = 3

b = B()
b.a = 1
b.b = 2
b.missing = 3
b.extra = 4 # E: "B" has no attribute "extra"
[builtins fixtures/tuple.pyi]


[case testSlotsAssignmentWithExplicitDict]
class A:
__slots__ = ("a",)
class B(A):
__slots__ = ("__dict__",)

def __init__(self) -> None:
self.a = 1
self.b = 2

b = B()
b.a = 1
b.b = 2
b.c = 3 # E: "B" has no attribute "c"
[builtins fixtures/tuple.pyi]


[case testSlotsAssignmentWithExplicitSuperDict]
class A:
__slots__ = ("__dict__",)
class B(A):
__slots__ = ("a",)

def __init__(self) -> None:
self.a = 1
self.b = 2

b = B()
b.a = 1
b.b = 2
b.c = 3 # E: "B" has no attribute "c"
[builtins fixtures/tuple.pyi]


[case testSlotsAssignmentWithVariable]
slot_name = "b"
class A:
__slots__ = ("a", slot_name)
def __init__(self) -> None:
self.a = 1
self.b = 2
self.c = 3

a = A()
a.a = 1
a.b = 2
a.c = 3
a.d = 4 # E: "A" has no attribute "d"
[builtins fixtures/tuple.pyi]


[case testSlotsAssignmentMultipleLeftValues]
class A:
__slots__ = ("a", "b")
def __init__(self) -> None:
self.a, self.b, self.c = (1, 2, 3) # E: Trying to assign name "c" that is not in "__slots__" of type "__main__.A"
[builtins fixtures/tuple.pyi]


[case testSlotsAssignmentMultipleAssignments]
class A:
__slots__ = ("a",)
def __init__(self) -> None:
self.a = self.b = self.c = 1
[out]
main:4: error: Trying to assign name "b" that is not in "__slots__" of type "__main__.A"
main:4: error: Trying to assign name "c" that is not in "__slots__" of type "__main__.A"
[builtins fixtures/tuple.pyi]