[red-knot] Descriptor protocol

[sharkdp]

We need to understand the descriptor protocol in order to infer proper types for attribute accesses like in this simple example:

from typing import Literal

class Ten:
    def __get__(self, instance: object, owner: type | None = None) -> Literal[10]:
        return 10

    def __set__(self, instance: object, value: Literal[10]) -> None:
        pass

class C:
    ten = Ten()

c = C()

# TODO: this should be `Literal[10]`
reveal_type(c.ten)  # revealed: Unknown | Ten

# TODO: This should `Literal[10]`
reveal_type(C.ten)  # revealed: Unknown | Ten

# These are fine:
c.ten = 10
C.ten = 10

# TODO: Both of these should be errors
c.ten = 11
C.ten = 11

References: Python Documentation: Descriptor Guide

part of: #14164

[carljm]

An important case to consider here that isn't shown in the example in summary is where the value argument to __set__ does not have the same type as the return type of __get__, meaning we need to use a different type for type-checking assignments to the attribute than we use for inferring the type of an attribute access.

[sharkdp]

where the value argument to __set__ does not have the same type as the return type of __get__

Yes, thanks. This is already covered in the provisional tests (#15972).

[mishamsk]

@carljm @sharkdp sorry, for delay

Here is my thinking, proposed plan & rough design.

Guiding principles I've used:

• Design-wise: Avoid special-casing if possible
• Plan such that each steps adds one new aspect of the protocol support, while addressing the next most popular usage scenario

Design

• Descriptors should be modeled as Type::Instance, no special Type variant or anything fancy.
  • This rule should apply to everything. Including the most common @property, @classmethod, @staticmethod; attrbiutes of slotted classes and functions (methods)
• @property, @classmethod, @staticmethod should produce an instance of KnownClass variant(s)
• Introduce KnownMethod enum, starting with descriptor dunders that will provide
• Decorators on methods should be processed at inference time and change the binding type to instance (unlike current implementation)
  • This would make it much easier to handle things, like writeable properties
  • Later this would allow supporting user provided functions/decorators that create descriptors
• If class has slots, all bindings should become data-descriptors - instances of another new KnownClass variant

Obviously there are going to be a lot of minute details in actual implementation, but overall it feels like descriptors do not introduce any special needs really.

Plan

The plan, is to start with known classes and avoid bothering with full inference of descriptor dunder method signatures. Implement the non-data descriptors first, using the most popular builtins. Then expand to set only data descriptor and build-up support for known descriptor classes. Then tackle everything else.

Assuming red_knot is targeting "an average" python user, I'd suggest implementing in the following order based on frequency of use:

1. Patterns common in non-library-like user code. So code devs write for a FastApi controller, not FastApi internals
   1. readonly property aka non-data descriptor
      1. Step 1: just understanding a member is a descriptor and properly handling in call_bound/member functions
      2. Step 2: diagnostic when non-data descriptor is LHS of assignment
   2. classmethod and staticmethod
      1. This would involve upgrading call_bound to handle classes (second argument in __get__). Otherwise just adding more known classes, should be easy
   3. property with setter
      1. this will be an inference "upgrade" - we can swtich from methods being function literals with decorators, to instances on this step
2. Popular descriptor generating components from stdlib
   1. functools.cached_property (adding more known classes, should be very easy)
3. Custom descriptor-generating classes in popular libraries
   1. ??? Not sure, needs research
4. Less popular / library use-cases
   1. Slotted classes
   2. property (with delete)
   3. maybe functools.singledispatchmethod
5. Generic inference for arbitrary classes that implement the protocol
   1. Implement full correct search logic in member/call_bound for instances (this), classes & super()
   2. Add __set_name__ support, handle overriden __getattribute__ etc.

PS. I can theoretically augment this with samples for each step, but this can also be done in individual issues.

[sharkdp]

Here is my thinking, proposed plan & rough design.

Thank you!

Guiding principles I've used:

* Design-wise: Avoid special-casing if possible

* Plan such that each steps adds one new aspect of the protocol support, while addressing the next most popular usage scenario

Both sound reasonable 👍

Descriptors should be modeled as Type::Instance, no special Type variant or anything fancy.

👍

@property, @classmethod, @staticmethod should produce an instance of KnownClass variant(s)

Maybe? Probably? But I wouldn't focus too much on use cases for now and rather get the basics straight. I think we could even defer proper decorator support until after we implemented the descriptor protocol. We might this for issues like #15961, but I'm not sure why it would be required here. Yes, those are examples of descriptors, but I don't see why we need to recognize them as such by introducing KnownClass variants. We want to recognize descriptors by detecting __get__/__set__/__delete__.

Introduce KnownMethod enum, starting with descriptor dunders that will provide

Our KnownXyz enums are typically used for special classes/functions from the standard library that require special treatment. They typically refer to exactly one individual symbol/name. For the descriptors, we want to look up if a method like __get__ exists on a particular class. Maybe we want an enum for that, but it would be different from the KnownXyz enums (in spirit), I think.

• Decorators on methods should be processed at inference time and change the binding type to instance (unlike current implementation)

  • This would make it much easier to handle things, like writeable properties
  • Later this would allow supporting user provided functions/decorators that create descriptors

• If class has slots, all bindings should become data-descriptors - instances of another new KnownClass variant

It would be great if we could defer decorators and slotted classes for now.

[mishamsk]

@sharkdp sounds like you'd prefer going the other way around - first support custom descriptors by checking special methods, then handle common cases, like @property.

Overall this means changing:

ruff/crates/red_knot_python_semantic/src/types/infer.rs

Line 3472 in 6e34f74

fn infer_attribute_expression(&mut self, attribute: &ast::ExprAttribute) -> Type<'db> {

and methods that it calls (infer_attribute_load, Type::member, Class::instance_member and indirectly Class:own_instance_member ).

The main "obstacle" I see right now is that Type::member is used in a lot of places, but doesn't have a context parameter. But in this case we'll need to distinguish Load/Store/Del. I'd prefer trying rather than hypothetically designing the right way to address it. The simplest approach would be to implement a separate context-aware member call that will defer to the old member for everything except Class, SubclassOf, Instance and KnownInstance.

Funny enough, but implementing Store is actually the easiest and minimal option, since it calls into Class::instance_member directly and can pass the context with minimal code changes.

However, maybe a reasonable scope for one PR could be:

1. Weave attribute access context into member call chain (one way or another)
2. Add special handling of instances

• Check for __get__ method and infer actual member ty using Type::call on the getter
• Same for setter and del dunders

here:

ruff/crates/red_knot_python_semantic/src/types.rs

Lines 4300 to 4311 in 6e34f74

	if let Some(function) = declared_ty.into_function_literal() {
	// TODO: Eventually, we are going to process all decorators correctly. This is
	// just a temporary heuristic to provide a broad categorization into properties
	// and non-property methods.
	if function.has_decorator(db, KnownClass::Property.to_class_literal(db)) {
	SymbolAndQualifiers::todo("@property")
	} else {
	SymbolAndQualifiers::todo("bound method")
	}
	} else {
	SymbolAndQualifiers(Symbol::bound(declared_ty), qualifiers)
	}

4. Probably extract Class::own_instance_member implementation into a salsa query for performance (I'd create a new one rather than using symbol and extending it's logic for this case)

Does this better match your thinking?

[sharkdp]

@sharkdp sounds like you'd prefer going the other way around - first support custom descriptors by checking special methods, then handle common cases, like @property.

Yes. I do have an old branch where I did a preliminary investigation in this direction. I picked it up today to see how far this would take us (mainly to see what to suggest here), and it looks promising. I hope you don't mind if I make an initial proposal here for the first step (also given that #16111 might need one or two iterations before we can merge it)? I'm sure there are enough follow-up tasks, as you outlined in your plan above, if you want to contribute something.

[mishamsk]

just to re-iterate discord discussion - I'm obviously totally ok with you moving forward with this, and really appreciate that you asked!