mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-02 06:41:23 +00:00
[red-knot] Assignments to attributes (#16705)
## Summary This changeset adds proper support for assignments to attributes: ```py obj.attr = value ``` In particular, the following new features are now available: * We previously didn't raise any errors if you tried to assign to a non-existing attribute `attr`. This is now fixed. * If `type(obj).attr` is a data descriptor, we now call its `__set__` method instead of trying to assign to the load-context type of `obj.attr`, which can be different for data descriptors. * An initial attempt was made to support unions and intersections, as well as possibly-unbound situations. There are some remaining TODOs in tests, but they only affect edge cases. Having nested diagnostics would be one way that could help solve the remaining cases, I believe. ## Follow ups The following things are planned as follow-ups: - Write a test suite with snapshot diagnostics for various attribute assignment errors - Improve the diagnostics. An easy improvement would be to highlight the right hand side of the assignment as a secondary span (with the rhs type as additional information). Some other ideas are mentioned in TODO comments in this PR. - Improve the union/intersection/possible-unboundness handling - Add support for calling custom `__setattr__` methods (see new false positive in the ecosystem results) ## Ecosystem changes Some changes are related to assignments on attributes with a custom `__setattr__` method (see above). Since we didn't notice missing attributes at all in store context previously, these are new. The other changes are related to properties. We previously used their read-context type to test the assignment. That results in weird error messages, as we often see assignments to `self.property` and then we think that those are instance attributes *and* descriptors, leading to union types. Now we properly look them up on the meta type, see the decorated function, and try to overwrite it with the new value (as we don't understand decorators yet). Long story short: the errors are still weird, we need to understand decorators to make them go away. ## Test Plan New Markdown tests
This commit is contained in:
David Peter
GitHub
parent
14c5ed5d7d
commit
d03b12e711
4 changed files with 570 additions and 103 deletions
|
|
@ -818,40 +818,74 @@ def _(flag: bool):
|
|||
if flag:
|
||||
class C1:
|
||||
x = 1
|
||||
y: int = 1
|
||||
|
||||
else:
|
||||
class C1:
|
||||
x = 2
|
||||
y: int | str = "b"
|
||||
|
||||
reveal_type(C1.x) # revealed: Unknown | Literal[1, 2]
|
||||
reveal_type(C1.y) # revealed: int | str
|
||||
|
||||
C1.y = 100
|
||||
# error: [invalid-assignment] "Object of type `Literal["problematic"]` is not assignable to attribute `y` on type `Literal[C1, C1]`"
|
||||
C1.y = "problematic"
|
||||
|
||||
class C2:
|
||||
if flag:
|
||||
x = 3
|
||||
y: int = 3
|
||||
else:
|
||||
x = 4
|
||||
y: int | str = "d"
|
||||
|
||||
reveal_type(C2.x) # revealed: Unknown | Literal[3, 4]
|
||||
reveal_type(C2.y) # revealed: int | str
|
||||
|
||||
C2.y = 100
|
||||
# error: [invalid-assignment] "Object of type `None` is not assignable to attribute `y` of type `int | str`"
|
||||
C2.y = None
|
||||
# TODO: should be an error, needs more sophisticated union handling in `validate_attribute_assignment`
|
||||
C2.y = "problematic"
|
||||
|
||||
if flag:
|
||||
class Meta3(type):
|
||||
x = 5
|
||||
y: int = 5
|
||||
|
||||
else:
|
||||
class Meta3(type):
|
||||
x = 6
|
||||
y: int | str = "f"
|
||||
|
||||
class C3(metaclass=Meta3): ...
|
||||
reveal_type(C3.x) # revealed: Unknown | Literal[5, 6]
|
||||
reveal_type(C3.y) # revealed: int | str
|
||||
|
||||
C3.y = 100
|
||||
# error: [invalid-assignment] "Object of type `None` is not assignable to attribute `y` of type `int | str`"
|
||||
C3.y = None
|
||||
# TODO: should be an error, needs more sophisticated union handling in `validate_attribute_assignment`
|
||||
C3.y = "problematic"
|
||||
|
||||
class Meta4(type):
|
||||
if flag:
|
||||
x = 7
|
||||
y: int = 7
|
||||
else:
|
||||
x = 8
|
||||
y: int | str = "h"
|
||||
|
||||
class C4(metaclass=Meta4): ...
|
||||
reveal_type(C4.x) # revealed: Unknown | Literal[7, 8]
|
||||
reveal_type(C4.y) # revealed: int | str
|
||||
|
||||
C4.y = 100
|
||||
# error: [invalid-assignment] "Object of type `None` is not assignable to attribute `y` of type `int | str`"
|
||||
C4.y = None
|
||||
# TODO: should be an error, needs more sophisticated union handling in `validate_attribute_assignment`
|
||||
C4.y = "problematic"
|
||||
```
|
||||
|
||||
## Unions with possibly unbound paths
|
||||
|
|
@ -875,8 +909,14 @@ def _(flag1: bool, flag2: bool):
|
|||
# error: [possibly-unbound-attribute] "Attribute `x` on type `Literal[C1, C2, C3]` is possibly unbound"
|
||||
reveal_type(C.x) # revealed: Unknown | Literal[1, 3]
|
||||
|
||||
# error: [invalid-assignment] "Object of type `Literal[100]` is not assignable to attribute `x` on type `Literal[C1, C2, C3]`"
|
||||
C.x = 100
|
||||
|
||||
# error: [possibly-unbound-attribute] "Attribute `x` on type `C1 | C2 | C3` is possibly unbound"
|
||||
reveal_type(C().x) # revealed: Unknown | Literal[1, 3]
|
||||
|
||||
# error: [invalid-assignment] "Object of type `Literal[100]` is not assignable to attribute `x` on type `C1 | C2 | C3`"
|
||||
C().x = 100
|
||||
```
|
||||
|
||||
### Possibly-unbound within a class
|
||||
|
|
@ -901,10 +941,16 @@ def _(flag: bool, flag1: bool, flag2: bool):
|
|||
# error: [possibly-unbound-attribute] "Attribute `x` on type `Literal[C1, C2, C3]` is possibly unbound"
|
||||
reveal_type(C.x) # revealed: Unknown | Literal[1, 2, 3]
|
||||
|
||||
# error: [possibly-unbound-attribute]
|
||||
C.x = 100
|
||||
|
||||
# Note: we might want to consider ignoring possibly-unbound diagnostics for instance attributes eventually,
|
||||
# see the "Possibly unbound/undeclared instance attribute" section below.
|
||||
# error: [possibly-unbound-attribute] "Attribute `x` on type `C1 | C2 | C3` is possibly unbound"
|
||||
reveal_type(C().x) # revealed: Unknown | Literal[1, 2, 3]
|
||||
|
||||
# error: [possibly-unbound-attribute]
|
||||
C().x = 100
|
||||
```
|
||||
|
||||
### Possibly-unbound within gradual types
|
||||
|
|
@ -922,6 +968,9 @@ def _(flag: bool):
|
|||
x: int
|
||||
|
||||
reveal_type(Derived().x) # revealed: int | Any
|
||||
|
||||
Derived().x = 1
|
||||
Derived().x = "a"
|
||||
```
|
||||
|
||||
### Attribute possibly unbound on a subclass but not on a superclass
|
||||
|
|
@ -936,8 +985,10 @@ def _(flag: bool):
|
|||
x = 2
|
||||
|
||||
reveal_type(Bar.x) # revealed: Unknown | Literal[2, 1]
|
||||
Bar.x = 3
|
||||
|
||||
reveal_type(Bar().x) # revealed: Unknown | Literal[2, 1]
|
||||
Bar().x = 3
|
||||
```
|
||||
|
||||
### Attribute possibly unbound on a subclass and on a superclass
|
||||
|
|
@ -955,8 +1006,14 @@ def _(flag: bool):
|
|||
# error: [possibly-unbound-attribute]
|
||||
reveal_type(Bar.x) # revealed: Unknown | Literal[2, 1]
|
||||
|
||||
# error: [possibly-unbound-attribute]
|
||||
Bar.x = 3
|
||||
|
||||
# error: [possibly-unbound-attribute]
|
||||
reveal_type(Bar().x) # revealed: Unknown | Literal[2, 1]
|
||||
|
||||
# error: [possibly-unbound-attribute]
|
||||
Bar().x = 3
|
||||
```
|
||||
|
||||
### Possibly unbound/undeclared instance attribute
|
||||
|
|
@ -975,6 +1032,9 @@ def _(flag: bool):
|
|||
|
||||
# error: [possibly-unbound-attribute]
|
||||
reveal_type(Foo().x) # revealed: int | Unknown
|
||||
|
||||
# error: [possibly-unbound-attribute]
|
||||
Foo().x = 1
|
||||
```
|
||||
|
||||
#### Possibly unbound
|
||||
|
|
@ -989,6 +1049,9 @@ def _(flag: bool):
|
|||
# Emitting a diagnostic in a case like this is not something we support, and it's unclear
|
||||
# if we ever will (or want to)
|
||||
reveal_type(Foo().x) # revealed: Unknown | Literal[1]
|
||||
|
||||
# Same here
|
||||
Foo().x = 2
|
||||
```
|
||||
|
||||
### Unions with all paths unbound
|
||||
|
|
@ -1003,6 +1066,11 @@ def _(flag: bool):
|
|||
|
||||
# error: [unresolved-attribute] "Type `Literal[C1, C2]` has no attribute `x`"
|
||||
reveal_type(C.x) # revealed: Unknown
|
||||
|
||||
# TODO: This should ideally be a `unresolved-attribute` error. We need better union
|
||||
# handling in `validate_attribute_assignment` for this.
|
||||
# error: [invalid-assignment] "Object of type `Literal[1]` is not assignable to attribute `x` on type `Literal[C1, C2]`"
|
||||
C.x = 1
|
||||
```
|
||||
|
||||
## Inherited class attributes
|
||||
|
|
@ -1017,6 +1085,8 @@ class B(A): ...
|
|||
class C(B): ...
|
||||
|
||||
reveal_type(C.X) # revealed: Unknown | Literal["foo"]
|
||||
|
||||
C.X = "bar"
|
||||
```
|
||||
|
||||
### Multiple inheritance
|
||||
|
|
@ -1040,6 +1110,8 @@ reveal_type(A.__mro__)
|
|||
|
||||
# `E` is earlier in the MRO than `F`, so we should use the type of `E.X`
|
||||
reveal_type(A.X) # revealed: Unknown | Literal[42]
|
||||
|
||||
A.X = 100
|
||||
```
|
||||
|
||||
## Intersections of attributes
|
||||
|
|
@ -1057,9 +1129,13 @@ class B: ...
|
|||
def _(a_and_b: Intersection[A, B]):
|
||||
reveal_type(a_and_b.x) # revealed: int
|
||||
|
||||
a_and_b.x = 2
|
||||
|
||||
# Same for class objects
|
||||
def _(a_and_b: Intersection[type[A], type[B]]):
|
||||
reveal_type(a_and_b.x) # revealed: int
|
||||
|
||||
a_and_b.x = 2
|
||||
```
|
||||
|
||||
### Attribute available on both elements
|
||||
|
|
@ -1069,6 +1145,7 @@ from knot_extensions import Intersection
|
|||
|
||||
class P: ...
|
||||
class Q: ...
|
||||
class R(P, Q): ...
|
||||
|
||||
class A:
|
||||
x: P = P()
|
||||
|
|
@ -1078,10 +1155,12 @@ class B:
|
|||
|
||||
def _(a_and_b: Intersection[A, B]):
|
||||
reveal_type(a_and_b.x) # revealed: P & Q
|
||||
a_and_b.x = R()
|
||||
|
||||
# Same for class objects
|
||||
def _(a_and_b: Intersection[type[A], type[B]]):
|
||||
reveal_type(a_and_b.x) # revealed: P & Q
|
||||
a_and_b.x = R()
|
||||
```
|
||||
|
||||
### Possible unboundness
|
||||
|
|
@ -1091,6 +1170,7 @@ from knot_extensions import Intersection
|
|||
|
||||
class P: ...
|
||||
class Q: ...
|
||||
class R(P, Q): ...
|
||||
|
||||
def _(flag: bool):
|
||||
class A1:
|
||||
|
|
@ -1102,11 +1182,17 @@ def _(flag: bool):
|
|||
def inner1(a_and_b: Intersection[A1, B1]):
|
||||
# error: [possibly-unbound-attribute]
|
||||
reveal_type(a_and_b.x) # revealed: P
|
||||
|
||||
# error: [possibly-unbound-attribute]
|
||||
a_and_b.x = R()
|
||||
# Same for class objects
|
||||
def inner1_class(a_and_b: Intersection[type[A1], type[B1]]):
|
||||
# error: [possibly-unbound-attribute]
|
||||
reveal_type(a_and_b.x) # revealed: P
|
||||
|
||||
# error: [possibly-unbound-attribute]
|
||||
a_and_b.x = R()
|
||||
|
||||
class A2:
|
||||
if flag:
|
||||
x: P = P()
|
||||
|
|
@ -1116,6 +1202,11 @@ def _(flag: bool):
|
|||
|
||||
def inner2(a_and_b: Intersection[A2, B1]):
|
||||
reveal_type(a_and_b.x) # revealed: P & Q
|
||||
|
||||
# TODO: this should not be an error, we need better intersection
|
||||
# handling in `validate_attribute_assignment` for this
|
||||
# error: [possibly-unbound-attribute]
|
||||
a_and_b.x = R()
|
||||
# Same for class objects
|
||||
def inner2_class(a_and_b: Intersection[type[A2], type[B1]]):
|
||||
reveal_type(a_and_b.x) # revealed: P & Q
|
||||
|
|
@ -1131,21 +1222,33 @@ def _(flag: bool):
|
|||
def inner3(a_and_b: Intersection[A3, B3]):
|
||||
# error: [possibly-unbound-attribute]
|
||||
reveal_type(a_and_b.x) # revealed: P & Q
|
||||
|
||||
# error: [possibly-unbound-attribute]
|
||||
a_and_b.x = R()
|
||||
# Same for class objects
|
||||
def inner3_class(a_and_b: Intersection[type[A3], type[B3]]):
|
||||
# error: [possibly-unbound-attribute]
|
||||
reveal_type(a_and_b.x) # revealed: P & Q
|
||||
|
||||
# error: [possibly-unbound-attribute]
|
||||
a_and_b.x = R()
|
||||
|
||||
class A4: ...
|
||||
class B4: ...
|
||||
|
||||
def inner4(a_and_b: Intersection[A4, B4]):
|
||||
# error: [unresolved-attribute]
|
||||
reveal_type(a_and_b.x) # revealed: Unknown
|
||||
|
||||
# error: [invalid-assignment]
|
||||
a_and_b.x = R()
|
||||
# Same for class objects
|
||||
def inner4_class(a_and_b: Intersection[type[A4], type[B4]]):
|
||||
# error: [unresolved-attribute]
|
||||
reveal_type(a_and_b.x) # revealed: Unknown
|
||||
|
||||
# error: [invalid-assignment]
|
||||
a_and_b.x = R()
|
||||
```
|
||||
|
||||
### Intersection of implicit instance attributes
|
||||
|
|
|
|||
|
|
@ -32,14 +32,22 @@ reveal_type(c.ten) # revealed: Literal[10]
|
|||
|
||||
reveal_type(C.ten) # revealed: Literal[10]
|
||||
|
||||
# These are fine:
|
||||
# This is fine:
|
||||
c.ten = 10
|
||||
C.ten = 10
|
||||
|
||||
# error: [invalid-assignment] "Object of type `Literal[11]` is not assignable to attribute `ten` of type `Literal[10]`"
|
||||
# error: [invalid-assignment] "Invalid assignment to data descriptor attribute `ten` on type `C` with custom `__set__` method"
|
||||
c.ten = 11
|
||||
```
|
||||
|
||||
# error: [invalid-assignment] "Object of type `Literal[11]` is not assignable to attribute `ten` of type `Literal[10]`"
|
||||
When assigning to the `ten` attribute from the class object, we get an error. The descriptor
|
||||
protocol is *not* triggered in this case. Since the attribute is declared as `Ten` in the class
|
||||
body, we do not allow these assignments, preventing users from accidentally overwriting the data
|
||||
descriptor, which is what would happen at runtime:
|
||||
|
||||
```py
|
||||
# error: [invalid-assignment] "Object of type `Literal[10]` is not assignable to attribute `ten` of type `Ten`"
|
||||
C.ten = 10
|
||||
# error: [invalid-assignment] "Object of type `Literal[11]` is not assignable to attribute `ten` of type `Ten`"
|
||||
C.ten = 11
|
||||
```
|
||||
|
||||
|
|
@ -66,13 +74,11 @@ c = C()
|
|||
reveal_type(c.flexible_int) # revealed: int | None
|
||||
|
||||
c.flexible_int = 42 # okay
|
||||
# TODO: This should not be an error
|
||||
# error: [invalid-assignment]
|
||||
c.flexible_int = "42" # also okay!
|
||||
|
||||
reveal_type(c.flexible_int) # revealed: int | None
|
||||
|
||||
# TODO: This should be an error
|
||||
# error: [invalid-assignment] "Invalid assignment to data descriptor attribute `flexible_int` on type `C` with custom `__set__` method"
|
||||
c.flexible_int = None # not okay
|
||||
|
||||
reveal_type(c.flexible_int) # revealed: int | None
|
||||
|
|
@ -167,19 +173,24 @@ def f1(flag: bool):
|
|||
self.attr = "normal"
|
||||
|
||||
reveal_type(C1().attr) # revealed: Unknown | Literal["data", "normal"]
|
||||
|
||||
# Assigning to the attribute also causes no `possibly-unbound` diagnostic:
|
||||
C1().attr = 1
|
||||
```
|
||||
|
||||
We never treat implicit instance attributes as definitely bound, so we fall back to the non-data
|
||||
descriptor here:
|
||||
|
||||
```py
|
||||
def f2(flag: bool):
|
||||
class C2:
|
||||
class C2:
|
||||
def f(self):
|
||||
self.attr = "normal"
|
||||
attr = NonDataDescriptor()
|
||||
|
||||
reveal_type(C2().attr) # revealed: Unknown | Literal["non-data", "normal"]
|
||||
reveal_type(C2().attr) # revealed: Unknown | Literal["non-data", "normal"]
|
||||
|
||||
# Assignments always go to the instance attribute in this case
|
||||
C2().attr = 1
|
||||
```
|
||||
|
||||
### Descriptors only work when used as class variables
|
||||
|
|
@ -198,6 +209,12 @@ class C:
|
|||
self.ten: Ten = Ten()
|
||||
|
||||
reveal_type(C().ten) # revealed: Ten
|
||||
|
||||
C().ten = Ten()
|
||||
|
||||
# The instance attribute is declared as `Ten`, so this is an
|
||||
# error: [invalid-assignment] "Object of type `Literal[10]` is not assignable to attribute `ten` of type `Ten`"
|
||||
C().ten = 10
|
||||
```
|
||||
|
||||
## Descriptor protocol for class objects
|
||||
|
|
@ -219,7 +236,7 @@ class DataDescriptor:
|
|||
def __get__(self, instance: object, owner: type | None = None) -> Literal["data"]:
|
||||
return "data"
|
||||
|
||||
def __set__(self, instance: object, value: str) -> None:
|
||||
def __set__(self, instance: object, value: int) -> None:
|
||||
pass
|
||||
|
||||
class NonDataDescriptor:
|
||||
|
|
@ -246,7 +263,28 @@ reveal_type(C1.class_data_descriptor) # revealed: Literal["data"]
|
|||
reveal_type(C1.class_non_data_descriptor) # revealed: Literal["non-data"]
|
||||
```
|
||||
|
||||
Next, we demonstrate that a *metaclass data descriptor* takes precedence over all class-level
|
||||
Assignments to class object attribute only trigger the descriptor protocol if the data descriptor is
|
||||
on the metaclass:
|
||||
|
||||
```py
|
||||
C1.meta_data_descriptor = 1
|
||||
|
||||
# error: [invalid-assignment] "Invalid assignment to data descriptor attribute `meta_data_descriptor` on type `Literal[C1]` with custom `__set__` method"
|
||||
C1.meta_data_descriptor = "invalid"
|
||||
```
|
||||
|
||||
When writing to a class-level data descriptor from the class object itself, the descriptor protocol
|
||||
is *not* triggered (this is in contrast to what happens when you read class-level descriptor
|
||||
attributes!). So the following assignment does not call `__set__`. At runtime, the assignment would
|
||||
overwrite the data descriptor, but the attribute is declared as `DataDescriptor` in the class body,
|
||||
so we do not allow this:
|
||||
|
||||
```py
|
||||
# error: [invalid-assignment] "Object of type `Literal[1]` is not assignable to attribute `class_data_descriptor` of type `DataDescriptor`"
|
||||
C1.class_data_descriptor = 1
|
||||
```
|
||||
|
||||
We now demonstrate that a *metaclass data descriptor* takes precedence over all class-level
|
||||
attributes:
|
||||
|
||||
```py
|
||||
|
|
@ -267,6 +305,14 @@ class C2(metaclass=Meta2):
|
|||
|
||||
reveal_type(C2.meta_data_descriptor1) # revealed: Literal["data"]
|
||||
reveal_type(C2.meta_data_descriptor2) # revealed: Literal["data"]
|
||||
|
||||
C2.meta_data_descriptor1 = 1
|
||||
C2.meta_data_descriptor2 = 1
|
||||
|
||||
# error: [invalid-assignment]
|
||||
C2.meta_data_descriptor1 = "invalid"
|
||||
# error: [invalid-assignment]
|
||||
C2.meta_data_descriptor2 = "invalid"
|
||||
```
|
||||
|
||||
On the other hand, normal metaclass attributes and metaclass non-data descriptors are shadowed by
|
||||
|
|
@ -321,6 +367,16 @@ def _(flag: bool):
|
|||
reveal_type(C5.meta_data_descriptor1) # revealed: Literal["data", "value on class"]
|
||||
# error: [possibly-unbound-attribute]
|
||||
reveal_type(C5.meta_data_descriptor2) # revealed: Literal["data"]
|
||||
|
||||
# TODO: We currently emit two diagnostics here, corresponding to the two states of `flag`. The diagnostics are not
|
||||
# wrong, but they could be subsumed under a higher-level diagnostic.
|
||||
|
||||
# error: [invalid-assignment] "Invalid assignment to data descriptor attribute `meta_data_descriptor1` on type `Literal[C5]` with custom `__set__` method"
|
||||
# error: [invalid-assignment] "Object of type `None` is not assignable to attribute `meta_data_descriptor1` of type `Literal["value on class"]`"
|
||||
C5.meta_data_descriptor1 = None
|
||||
|
||||
# error: [possibly-unbound-attribute]
|
||||
C5.meta_data_descriptor2 = 1
|
||||
```
|
||||
|
||||
When a class-level attribute is possibly unbound, we union its (descriptor protocol) type with the
|
||||
|
|
@ -373,6 +429,11 @@ def _(flag: bool):
|
|||
reveal_type(C7.union_of_metaclass_data_descriptor_and_attribute) # revealed: Literal["data", 2]
|
||||
reveal_type(C7.union_of_class_attributes) # revealed: Literal[1, 2]
|
||||
reveal_type(C7.union_of_class_data_descriptor_and_attribute) # revealed: Literal["data", 2]
|
||||
|
||||
C7.union_of_metaclass_attributes = 2 if flag else 1
|
||||
C7.union_of_metaclass_data_descriptor_and_attribute = 2 if flag else 100
|
||||
C7.union_of_class_attributes = 2 if flag else 1
|
||||
C7.union_of_class_data_descriptor_and_attribute = 2 if flag else DataDescriptor()
|
||||
```
|
||||
|
||||
## Descriptors distinguishing between class and instance access
|
||||
|
|
@ -469,7 +530,7 @@ c.name = "new"
|
|||
c.name = None
|
||||
|
||||
# TODO: this should be an error, but with a proper error message
|
||||
# error: [invalid-assignment] "Object of type `Literal[42]` is not assignable to attribute `name` of type `<bound method `name` of `C`>`"
|
||||
# error: [invalid-assignment] "Implicit shadowing of function `name`; annotate to make it explicit if this is intentional"
|
||||
c.name = 42
|
||||
```
|
||||
|
||||
|
|
|
|||
|
|
@ -1169,6 +1169,22 @@ pub(super) fn report_possibly_unresolved_reference(
|
|||
);
|
||||
}
|
||||
|
||||
pub(super) fn report_possibly_unbound_attribute(
|
||||
context: &InferContext,
|
||||
target: &ast::ExprAttribute,
|
||||
attribute: &str,
|
||||
object_ty: Type,
|
||||
) {
|
||||
context.report_lint(
|
||||
&POSSIBLY_UNBOUND_ATTRIBUTE,
|
||||
target,
|
||||
format_args!(
|
||||
"Attribute `{attribute}` on type `{}` is possibly unbound",
|
||||
object_ty.display(context.db()),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
pub(super) fn report_unresolved_reference(context: &InferContext, expr_name_node: &ast::ExprName) {
|
||||
let ast::ExprName { id, .. } = expr_name_node;
|
||||
|
||||
|
|
|
|||
|
|
@ -65,13 +65,14 @@ use crate::types::call::{Argument, CallArguments, UnionCallError};
|
|||
use crate::types::diagnostic::{
|
||||
report_implicit_return_type, report_invalid_arguments_to_annotated,
|
||||
report_invalid_arguments_to_callable, report_invalid_assignment,
|
||||
report_invalid_attribute_assignment, report_invalid_return_type, report_unresolved_module,
|
||||
TypeCheckDiagnostics, CALL_NON_CALLABLE, CALL_POSSIBLY_UNBOUND_METHOD,
|
||||
CONFLICTING_DECLARATIONS, CONFLICTING_METACLASS, CYCLIC_CLASS_DEFINITION, DIVISION_BY_ZERO,
|
||||
DUPLICATE_BASE, INCONSISTENT_MRO, INVALID_ATTRIBUTE_ACCESS, INVALID_BASE, INVALID_DECLARATION,
|
||||
INVALID_PARAMETER_DEFAULT, INVALID_TYPE_FORM, INVALID_TYPE_VARIABLE_CONSTRAINTS,
|
||||
POSSIBLY_UNBOUND_ATTRIBUTE, POSSIBLY_UNBOUND_IMPORT, UNDEFINED_REVEAL, UNRESOLVED_ATTRIBUTE,
|
||||
UNRESOLVED_IMPORT, UNSUPPORTED_OPERATOR,
|
||||
report_invalid_attribute_assignment, report_invalid_return_type,
|
||||
report_possibly_unbound_attribute, report_unresolved_module, TypeCheckDiagnostics,
|
||||
CALL_NON_CALLABLE, CALL_POSSIBLY_UNBOUND_METHOD, CONFLICTING_DECLARATIONS,
|
||||
CONFLICTING_METACLASS, CYCLIC_CLASS_DEFINITION, DIVISION_BY_ZERO, DUPLICATE_BASE,
|
||||
INCONSISTENT_MRO, INVALID_ASSIGNMENT, INVALID_ATTRIBUTE_ACCESS, INVALID_BASE,
|
||||
INVALID_DECLARATION, INVALID_PARAMETER_DEFAULT, INVALID_TYPE_FORM,
|
||||
INVALID_TYPE_VARIABLE_CONSTRAINTS, POSSIBLY_UNBOUND_IMPORT, UNDEFINED_REVEAL,
|
||||
UNRESOLVED_ATTRIBUTE, UNRESOLVED_IMPORT, UNSUPPORTED_OPERATOR,
|
||||
};
|
||||
use crate::types::mro::MroErrorKind;
|
||||
use crate::types::unpacker::{UnpackResult, Unpacker};
|
||||
|
|
@ -2168,6 +2169,353 @@ impl<'db> TypeInferenceBuilder<'db> {
|
|||
self.infer_target_impl(target, assigned_ty);
|
||||
}
|
||||
|
||||
/// Make sure that the attribute assignment `obj.attribute = value` is valid.
|
||||
///
|
||||
/// `target` is the node for the left-hand side, `object_ty` is the type of `obj`, `attribute` is
|
||||
/// the name of the attribute being assigned, and `value_ty` is the type of the right-hand side of
|
||||
/// the assignment. If the assignment is invalid, emit diagnostics.
|
||||
fn validate_attribute_assignment(
|
||||
&mut self,
|
||||
target: &ast::ExprAttribute,
|
||||
object_ty: Type<'db>,
|
||||
attribute: &str,
|
||||
value_ty: Type<'db>,
|
||||
emit_diagnostics: bool,
|
||||
) -> bool {
|
||||
let db = self.db();
|
||||
|
||||
let ensure_assignable_to = |attr_ty| -> bool {
|
||||
let assignable = value_ty.is_assignable_to(db, attr_ty);
|
||||
if !assignable && emit_diagnostics {
|
||||
report_invalid_attribute_assignment(
|
||||
&self.context,
|
||||
target.into(),
|
||||
attr_ty,
|
||||
value_ty,
|
||||
attribute,
|
||||
);
|
||||
}
|
||||
assignable
|
||||
};
|
||||
|
||||
match object_ty {
|
||||
Type::Union(union) => {
|
||||
if union.elements(self.db()).iter().all(|elem| {
|
||||
self.validate_attribute_assignment(target, *elem, attribute, value_ty, false)
|
||||
}) {
|
||||
true
|
||||
} else {
|
||||
// TODO: This is not a very helpful error message, as it does not include the underlying reason
|
||||
// why the assignment is invalid. This would be a good use case for nested diagnostics.
|
||||
if emit_diagnostics {
|
||||
self.context.report_lint(&INVALID_ASSIGNMENT, target, format_args!(
|
||||
"Object of type `{}` is not assignable to attribute `{attribute}` on type `{}`",
|
||||
value_ty.display(self.db()),
|
||||
object_ty.display(self.db()),
|
||||
));
|
||||
}
|
||||
|
||||
false
|
||||
}
|
||||
}
|
||||
|
||||
Type::Intersection(intersection) => {
|
||||
// TODO: Handle negative intersection elements
|
||||
if intersection.positive(db).iter().any(|elem| {
|
||||
self.validate_attribute_assignment(target, *elem, attribute, value_ty, false)
|
||||
}) {
|
||||
true
|
||||
} else {
|
||||
if emit_diagnostics {
|
||||
// TODO: same here, see above
|
||||
self.context.report_lint(&INVALID_ASSIGNMENT, target, format_args!(
|
||||
"Object of type `{}` is not assignable to attribute `{attribute}` on type `{}`",
|
||||
value_ty.display(self.db()),
|
||||
object_ty.display(self.db()),
|
||||
));
|
||||
}
|
||||
false
|
||||
}
|
||||
}
|
||||
|
||||
Type::Dynamic(..) | Type::Never => true,
|
||||
|
||||
Type::Instance(..)
|
||||
| Type::BooleanLiteral(..)
|
||||
| Type::IntLiteral(..)
|
||||
| Type::StringLiteral(..)
|
||||
| Type::BytesLiteral(..)
|
||||
| Type::LiteralString
|
||||
| Type::SliceLiteral(..)
|
||||
| Type::Tuple(..)
|
||||
| Type::KnownInstance(..)
|
||||
| Type::FunctionLiteral(..)
|
||||
| Type::Callable(..)
|
||||
| Type::AlwaysTruthy
|
||||
| Type::AlwaysFalsy => match object_ty.class_member(db, attribute.into()) {
|
||||
meta_attr @ SymbolAndQualifiers { .. } if meta_attr.is_class_var() => {
|
||||
if emit_diagnostics {
|
||||
self.context.report_lint(
|
||||
&INVALID_ATTRIBUTE_ACCESS,
|
||||
target,
|
||||
format_args!(
|
||||
"Cannot assign to ClassVar `{attribute}` from an instance of type `{ty}`",
|
||||
ty = object_ty.display(self.db()),
|
||||
),
|
||||
);
|
||||
}
|
||||
false
|
||||
}
|
||||
SymbolAndQualifiers {
|
||||
symbol: Symbol::Type(meta_attr_ty, meta_attr_boundness),
|
||||
qualifiers: _,
|
||||
} => {
|
||||
let assignable_to_meta_attr = if let Symbol::Type(meta_dunder_set, _) =
|
||||
meta_attr_ty.class_member(db, "__set__".into()).symbol
|
||||
{
|
||||
let successful_call = meta_dunder_set
|
||||
.try_call(
|
||||
db,
|
||||
&CallArguments::positional([meta_attr_ty, object_ty, value_ty]),
|
||||
)
|
||||
.is_ok();
|
||||
|
||||
if !successful_call && emit_diagnostics {
|
||||
// TODO: Here, it would be nice to emit an additional diagnostic that explains why the call failed
|
||||
self.context.report_lint(
|
||||
&INVALID_ASSIGNMENT,
|
||||
target,
|
||||
format_args!(
|
||||
"Invalid assignment to data descriptor attribute `{attribute}` on type `{}` with custom `__set__` method",
|
||||
object_ty.display(db)
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
successful_call
|
||||
} else {
|
||||
ensure_assignable_to(meta_attr_ty)
|
||||
};
|
||||
|
||||
let assignable_to_instance_attribute =
|
||||
if meta_attr_boundness == Boundness::PossiblyUnbound {
|
||||
let (assignable, boundness) =
|
||||
if let Symbol::Type(instance_attr_ty, instance_attr_boundness) =
|
||||
object_ty.instance_member(db, attribute).symbol
|
||||
{
|
||||
(
|
||||
ensure_assignable_to(instance_attr_ty),
|
||||
instance_attr_boundness,
|
||||
)
|
||||
} else {
|
||||
(true, Boundness::PossiblyUnbound)
|
||||
};
|
||||
|
||||
if boundness == Boundness::PossiblyUnbound {
|
||||
report_possibly_unbound_attribute(
|
||||
&self.context,
|
||||
target,
|
||||
attribute,
|
||||
object_ty,
|
||||
);
|
||||
}
|
||||
|
||||
assignable
|
||||
} else {
|
||||
true
|
||||
};
|
||||
|
||||
assignable_to_meta_attr && assignable_to_instance_attribute
|
||||
}
|
||||
|
||||
SymbolAndQualifiers {
|
||||
symbol: Symbol::Unbound,
|
||||
..
|
||||
} => {
|
||||
if let Symbol::Type(instance_attr_ty, instance_attr_boundness) =
|
||||
object_ty.instance_member(db, attribute).symbol
|
||||
{
|
||||
if instance_attr_boundness == Boundness::PossiblyUnbound {
|
||||
report_possibly_unbound_attribute(
|
||||
&self.context,
|
||||
target,
|
||||
attribute,
|
||||
object_ty,
|
||||
);
|
||||
}
|
||||
|
||||
ensure_assignable_to(instance_attr_ty)
|
||||
} else {
|
||||
if emit_diagnostics {
|
||||
self.context.report_lint(
|
||||
&UNRESOLVED_ATTRIBUTE,
|
||||
target,
|
||||
format_args!(
|
||||
"Unresolved attribute `{}` on type `{}`.",
|
||||
attribute,
|
||||
object_ty.display(db)
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
false
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
Type::ClassLiteral(..) | Type::SubclassOf(..) => {
|
||||
match object_ty.class_member(db, attribute.into()) {
|
||||
SymbolAndQualifiers {
|
||||
symbol: Symbol::Type(meta_attr_ty, meta_attr_boundness),
|
||||
qualifiers: _,
|
||||
} => {
|
||||
let assignable_to_meta_attr = if let Symbol::Type(meta_dunder_set, _) =
|
||||
meta_attr_ty.class_member(db, "__set__".into()).symbol
|
||||
{
|
||||
let successful_call = meta_dunder_set
|
||||
.try_call(
|
||||
db,
|
||||
&CallArguments::positional([meta_attr_ty, object_ty, value_ty]),
|
||||
)
|
||||
.is_ok();
|
||||
|
||||
if !successful_call && emit_diagnostics {
|
||||
// TODO: Here, it would be nice to emit an additional diagnostic that explains why the call failed
|
||||
self.context.report_lint(
|
||||
&INVALID_ASSIGNMENT,
|
||||
target,
|
||||
format_args!(
|
||||
"Invalid assignment to data descriptor attribute `{attribute}` on type `{}` with custom `__set__` method",
|
||||
object_ty.display(db)
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
successful_call
|
||||
} else {
|
||||
ensure_assignable_to(meta_attr_ty)
|
||||
};
|
||||
|
||||
let assignable_to_class_attr = if meta_attr_boundness
|
||||
== Boundness::PossiblyUnbound
|
||||
{
|
||||
let (assignable, boundness) = if let Symbol::Type(
|
||||
class_attr_ty,
|
||||
class_attr_boundness,
|
||||
) = object_ty
|
||||
.find_name_in_mro(db, attribute)
|
||||
.expect("called on Type::ClassLiteral or Type::SubclassOf")
|
||||
.symbol
|
||||
{
|
||||
(ensure_assignable_to(class_attr_ty), class_attr_boundness)
|
||||
} else {
|
||||
(true, Boundness::PossiblyUnbound)
|
||||
};
|
||||
|
||||
if boundness == Boundness::PossiblyUnbound {
|
||||
report_possibly_unbound_attribute(
|
||||
&self.context,
|
||||
target,
|
||||
attribute,
|
||||
object_ty,
|
||||
);
|
||||
}
|
||||
|
||||
assignable
|
||||
} else {
|
||||
true
|
||||
};
|
||||
|
||||
assignable_to_meta_attr && assignable_to_class_attr
|
||||
}
|
||||
SymbolAndQualifiers {
|
||||
symbol: Symbol::Unbound,
|
||||
..
|
||||
} => {
|
||||
if let Symbol::Type(class_attr_ty, class_attr_boundness) = object_ty
|
||||
.find_name_in_mro(db, attribute)
|
||||
.expect("called on Type::ClassLiteral or Type::SubclassOf")
|
||||
.symbol
|
||||
{
|
||||
if class_attr_boundness == Boundness::PossiblyUnbound {
|
||||
report_possibly_unbound_attribute(
|
||||
&self.context,
|
||||
target,
|
||||
attribute,
|
||||
object_ty,
|
||||
);
|
||||
}
|
||||
|
||||
ensure_assignable_to(class_attr_ty)
|
||||
} else {
|
||||
let attribute_is_bound_on_instance =
|
||||
object_ty.to_instance(self.db()).is_some_and(|instance| {
|
||||
!instance
|
||||
.instance_member(self.db(), attribute)
|
||||
.symbol
|
||||
.is_unbound()
|
||||
});
|
||||
|
||||
// Attribute is declared or bound on instance. Forbid access from the class object
|
||||
if emit_diagnostics {
|
||||
if attribute_is_bound_on_instance {
|
||||
self.context.report_lint(
|
||||
&INVALID_ATTRIBUTE_ACCESS,
|
||||
target,
|
||||
format_args!(
|
||||
"Cannot assign to instance attribute `{attribute}` from the class object `{ty}`",
|
||||
ty = object_ty.display(self.db()),
|
||||
));
|
||||
} else {
|
||||
self.context.report_lint(
|
||||
&UNRESOLVED_ATTRIBUTE,
|
||||
target,
|
||||
format_args!(
|
||||
"Unresolved attribute `{}` on type `{}`.",
|
||||
attribute,
|
||||
object_ty.display(db)
|
||||
),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
false
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Type::ModuleLiteral(module) => {
|
||||
if let Symbol::Type(attr_ty, _) = module.static_member(db, attribute) {
|
||||
let assignable = value_ty.is_assignable_to(db, attr_ty);
|
||||
if !assignable {
|
||||
report_invalid_attribute_assignment(
|
||||
&self.context,
|
||||
target.into(),
|
||||
attr_ty,
|
||||
value_ty,
|
||||
attribute,
|
||||
);
|
||||
}
|
||||
|
||||
false
|
||||
} else {
|
||||
self.context.report_lint(
|
||||
&UNRESOLVED_ATTRIBUTE,
|
||||
target,
|
||||
format_args!(
|
||||
"Unresolved attribute `{}` on type `{}`.",
|
||||
attribute,
|
||||
object_ty.display(db)
|
||||
),
|
||||
);
|
||||
|
||||
false
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn infer_target_impl(&mut self, target: &ast::Expr, assigned_ty: Option<Type<'db>>) {
|
||||
match target {
|
||||
ast::Expr::Name(name) => self.infer_definition(name),
|
||||
|
|
@ -2185,27 +2533,27 @@ impl<'db> TypeInferenceBuilder<'db> {
|
|||
}
|
||||
}
|
||||
ast::Expr::Attribute(
|
||||
lhs_expr @ ast::ExprAttribute {
|
||||
attr_expr @ ast::ExprAttribute {
|
||||
value: object,
|
||||
ctx: ExprContext::Store,
|
||||
attr,
|
||||
..
|
||||
},
|
||||
) => {
|
||||
let attribute_expr_ty = self.infer_attribute_expression(lhs_expr);
|
||||
self.store_expression_type(target, attribute_expr_ty);
|
||||
self.store_expression_type(target, Type::Never);
|
||||
|
||||
let object_ty = self.infer_expression(object);
|
||||
|
||||
if let Some(assigned_ty) = assigned_ty {
|
||||
if !assigned_ty.is_assignable_to(self.db(), attribute_expr_ty) {
|
||||
report_invalid_attribute_assignment(
|
||||
&self.context,
|
||||
target.into(),
|
||||
attribute_expr_ty,
|
||||
self.validate_attribute_assignment(
|
||||
attr_expr,
|
||||
object_ty,
|
||||
attr.id(),
|
||||
assigned_ty,
|
||||
attr.as_str(),
|
||||
true,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
_ => {
|
||||
// TODO: Remove this once we handle all possible assignment targets.
|
||||
self.infer_expression(target);
|
||||
|
|
@ -3988,15 +4336,13 @@ impl<'db> TypeInferenceBuilder<'db> {
|
|||
Type::unknown().into()
|
||||
}
|
||||
LookupError::PossiblyUnbound(type_when_bound) => {
|
||||
self.context.report_lint(
|
||||
&POSSIBLY_UNBOUND_ATTRIBUTE,
|
||||
report_possibly_unbound_attribute(
|
||||
&self.context,
|
||||
attribute,
|
||||
format_args!(
|
||||
"Attribute `{}` on type `{}` is possibly unbound",
|
||||
attr.id,
|
||||
value_type.display(db),
|
||||
),
|
||||
&attr.id,
|
||||
value_type,
|
||||
);
|
||||
|
||||
type_when_bound
|
||||
}
|
||||
}).inner_type()
|
||||
|
|
@ -4005,73 +4351,14 @@ impl<'db> TypeInferenceBuilder<'db> {
|
|||
fn infer_attribute_expression(&mut self, attribute: &ast::ExprAttribute) -> Type<'db> {
|
||||
let ast::ExprAttribute {
|
||||
value,
|
||||
attr,
|
||||
attr: _,
|
||||
range: _,
|
||||
ctx,
|
||||
} = attribute;
|
||||
|
||||
match ctx {
|
||||
ExprContext::Load => self.infer_attribute_load(attribute),
|
||||
ExprContext::Store => {
|
||||
let value_ty = self.infer_expression(value);
|
||||
|
||||
let symbol = match value_ty {
|
||||
Type::Instance(_) => {
|
||||
let instance_member = value_ty.member(self.db(), &attr.id);
|
||||
if instance_member.is_class_var() {
|
||||
self.context.report_lint(
|
||||
&INVALID_ATTRIBUTE_ACCESS,
|
||||
attribute,
|
||||
format_args!(
|
||||
"Cannot assign to ClassVar `{attr}` from an instance of type `{ty}`",
|
||||
ty = value_ty.display(self.db()),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
instance_member.symbol
|
||||
}
|
||||
Type::ClassLiteral(_) | Type::SubclassOf(_) => {
|
||||
let class_member = value_ty.member(self.db(), &attr.id).symbol;
|
||||
|
||||
if class_member.is_unbound() {
|
||||
let class = match value_ty {
|
||||
Type::ClassLiteral(class) => Some(class.class()),
|
||||
Type::SubclassOf(subclass_of @ SubclassOfType { .. }) => {
|
||||
match subclass_of.subclass_of() {
|
||||
ClassBase::Class(class) => Some(class),
|
||||
ClassBase::Dynamic(_) => unreachable!("Attribute lookup on a dynamic `SubclassOf` type should always return a bound symbol"),
|
||||
}
|
||||
}
|
||||
_ => None,
|
||||
};
|
||||
if let Some(class) = class {
|
||||
let instance_member = class.instance_member(self.db(), attr).symbol;
|
||||
|
||||
// Attribute is declared or bound on instance. Forbid access from the class object
|
||||
if !instance_member.is_unbound() {
|
||||
self.context.report_lint(
|
||||
&INVALID_ATTRIBUTE_ACCESS,
|
||||
attribute,
|
||||
format_args!(
|
||||
"Cannot assign to instance attribute `{attr}` from the class object `{ty}`",
|
||||
ty = value_ty.display(self.db()),
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
class_member
|
||||
}
|
||||
_ => value_ty.member(self.db(), &attr.id).symbol,
|
||||
};
|
||||
|
||||
// TODO: The unbound-case might also yield a diagnostic, but we can not activate
|
||||
// this yet until we understand implicit instance attributes (those that are not
|
||||
// defined in the class body), as there would be too many false positives.
|
||||
symbol.ignore_possibly_unbound().unwrap_or(Type::unknown())
|
||||
}
|
||||
ExprContext::Del => {
|
||||
ExprContext::Store | ExprContext::Del => {
|
||||
self.infer_expression(value);
|
||||
Type::Never
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue