[red-knot] Assignments to attributes

[sharkdp]

Summary

This changeset adds proper support for assignments to attributes:

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

[github-actions]

mypy_primer results

Changes were detected when running on open source projects

zipp (https://github.com/jaraco/zipp)
+    |
+ 
+ error: lint:unresolved-attribute
+   --> /tmp/mypy_primer/projects/zipp/zipp/compat/overlay.py:34:1
+    |
+ 33 | zipfile = HashableNamespace(**vars(importlib.import_module('zipfile')))
+ 34 | zipfile.Path = zipp.Path
+    | ^^^^^^^^^^^^ Unresolved attribute `Path` on type `HashableNamespace`.
+ 35 | zipfile._path = zipp
+    |
+ 
+ error: lint:unresolved-attribute
+   --> /tmp/mypy_primer/projects/zipp/zipp/compat/overlay.py:35:1
+    |
+ 33 | zipfile = HashableNamespace(**vars(importlib.import_module('zipfile')))
+ 34 | zipfile.Path = zipp.Path
+ 35 | zipfile._path = zipp
+    | ^^^^^^^^^^^^^ Unresolved attribute `_path` on type `HashableNamespace`.
+ 36 |
+ 37 | sys.modules[__name__ + '.zipfile'] = zipfile  # type: ignore[assignment]
- Found 8 diagnostics
+ Found 10 diagnostics

black (https://github.com/psf/black)
+ 
+     |
+     |
-     |             ^^^^^^^^^^^^^^ Object of type `Literal[""]` is not assignable to attribute `prefix` of type `<bound method `prefix` of `Leaf`>`
+     |             ^^^^^^^^^^^^^^ Implicit shadowing of function `prefix`; annotate to make it explicit if this is intentional
-     |             ^^^^^^^^^^^^^^ Object of type `Literal[""]` is not assignable to attribute `prefix` of type `<bound method `prefix` of `Leaf`>`
+     |             ^^^^^^^^^^^^^^ Implicit shadowing of function `prefix`; annotate to make it explicit if this is intentional
-     |                 ^^^^^^^^^^^^^^ Object of type `Literal[""]` is not assignable to attribute `prefix` of type `<bound method `prefix` of `Leaf`>`
+     |                 ^^^^^^^^^^^^^^ Implicit shadowing of function `prefix`; annotate to make it explicit if this is intentional
+ error: lint:invalid-assignment
+     |         ^^^^^^^^^^^ Implicit shadowing of function `prefix`; annotate to make it explicit if this is intentional
+     |         ^^^^^^^^^^^ Implicit shadowing of function `prefix`; annotate to make it explicit if this is intentional
+    --> /tmp/mypy_primer/projects/black/src/black/ranges.py:330:5
+ 328 |     # this is actually required to not break incremental reformatting.
+ 329 |     prefix = first.prefix
+ 330 |     first.prefix = ""
-     |         ^^^^^^^^^^^ Object of type `Literal[""]` is not assignable to attribute `prefix` of type `<bound method `prefix` of `Leaf`>`
+     |     ^^^^^^^^^^^^ Object of type `Literal[""]` is not assignable to attribute `prefix` on type `Leaf & ~AlwaysFalsy`
+ 331 |     index = node.remove()
+ 332 |     if index is not None:
-     |         ^^^^^^^^^^^ Object of type `Literal[""]` is not assignable to attribute `prefix` of type `<bound method `prefix` of `Leaf`>`
+    --> /tmp/mypy_primer/projects/black/src/black/ranges.py:356:5
+ 354 |         return
+ 355 |     prefix = first.prefix
+ 356 |     first.prefix = ""
+     |     ^^^^^^^^^^^^ Object of type `Literal[""]` is not assignable to attribute `prefix` on type `Leaf & ~AlwaysFalsy`
+ 357 |     value = "".join(str(node) for node in nodes)
+ 358 |     # The prefix comment on the NEWLINE leaf is the trailing comment of the statement.
+     |         ^^^^^^^^^^^^^^ Implicit shadowing of function `prefix`; annotate to make it explicit if this is intentional
-     |         ^^^^^^^^^^^^^^ Object of type `Literal[""]` is not assignable to attribute `prefix` of type `<bound method `prefix` of `Leaf`>`
-    --> /tmp/mypy_primer/projects/black/src/black/linegen.py:158:17
- 157 |             if any_open_brackets:
- 158 |                 node.prefix = ""
-     |                 ^^^^^^^^^^^ Object of type `Literal[""]` is not assignable to attribute `prefix` of type `@Todo & <bound method `prefix` of `Leaf`>`
- 159 |             if node.type not in WHITESPACE:
- 160 |                 self.current_line.append(node)
-     |             ^^^^^^^^^^^ Object of type `Literal[""]` is not assignable to attribute `prefix` of type `<bound method `prefix` of `Node`> | Literal[prefix]`
+     |             ^^^^^^^^^^^ Implicit shadowing of function `prefix`; annotate to make it explicit if this is intentional
-     |             ^^^^^^^^^^^ Object of type `<bound method `prefix` of `Node`> | Literal[prefix]` is not assignable to attribute `prefix` of type `<bound method `prefix` of `Leaf`>`
+     |             ^^^^^^^^^^^ Implicit shadowing of function `prefix`; annotate to make it explicit if this is intentional
-     |                 ^^^^^^^^^^^ Object of type `Literal[""]` is not assignable to attribute `prefix` of type `<bound method `prefix` of `Node`> | Literal[prefix]`
+     |                 ^^^^^^^^^^^ Implicit shadowing of function `prefix`; annotate to make it explicit if this is intentional
- Found 298 diagnostics
+ Found 299 diagnostics

[carljm]

Looks great!

[carljm]

We will want this diagnostic (and others below) to name the expected type, but this is fine to leave as follow up for when we have the new diagnostics system.

I think "data descriptor attribute... with custom __set__ method" is perhaps redundant? But also fine for now.

[sharkdp]

We will want this diagnostic (and others below) to name the expected type

Yes, absolutely. I didn't attempt to do this here, as I think it would naturally fall out of a sub-diagnostic that would include the call binding error.

[carljm]

"possibly unbound" is a weird phrasing for an attribute we are assigning to, but we can revisit this in future.

[carljm]

This is off the top of my head, but I think we can treat all negative intersection elements as object for this purpose (I don't think a negative type ever gives us information about attributes.) And because any positive element must inherit object, I think we actually can totally ignore negative elements except in the case where there are zero positive elements, in which case we treat the entire intersection as implicitly object.

[sharkdp]

This is off the top of my head, but I think we can treat all negative intersection elements as object for this purpose (I don't think a negative type ever gives us information about attributes.)

Let's consider two extreme cases. ~object = object & ~object = Never and ~Never = object & ~Never = object. In the former case, every attribute would exist (with type Never). So every attribute assignment to ~object should succeed. In the latter case, only the attributes on object exist, so almost every attribute assignment to ~Never should fail.

So in this sense, negative intersection elements do seem to provide information about attributes? I think we have special handling for these cases and we would not end up in this branch, but I struggled to justify why we can completely neglect negative elements here.

[carljm]

Yes, I was implicitly excluding from consideration Not[object], since that will simplify to Never, and Not[Never] since that will simplify to object (if alone) or disappear from the intersection.

In the realm of non-total negative elements that can actually exist in our intersection representation, I don't think that negative elements can contribute information about attributes, because the type system is not closed; there are always infinite possible objects with infinite possible attributes inhabiting a negation type.