MyPy v1.t6.0 cannot infer a descriptor's type through a Protocol which v1.15.0 supports

[tdyas]

MyPy v1.t6.0 is no longer able to infer a descriptor's type through a Protocol which it could do in v1.15.0.

To Reproduce

The following code is a simplified form of the Pantsbuild option system in this file.

from dataclasses import dataclass
from typing import Any, Protocol, Union, TypeVar, Callable, Generic, cast, TYPE_CHECKING, overload

# The type of the option.
_OptT = TypeVar("_OptT")

# The type of option's default (may be _OptT or some other type like `None`)
_DefaultT = TypeVar("_DefaultT")

_SubsystemType = Any

_DynamicDefaultT = Callable[[_SubsystemType], Any]

_MaybeDynamicT = Union[_DynamicDefaultT, _DefaultT]

@dataclass
class OptionInfo:
    flag_name: tuple[str, ...] | None


class _OptionBase(Generic[_OptT, _DefaultT]):
    _flag_names: tuple[str, ...] | None
    _default: _MaybeDynamicT[_DefaultT]
    
    def __new__(
        cls,
        flag_name: str | None = None,
        *,
        default: _MaybeDynamicT[_DefaultT]
    ):
        self = super().__new__(cls)
        self._flag_names = (flag_name,) if flag_name else None
        self._default = default
        return self

    def get_option_type(self, subsystem_cls):
        return type(self).option_type

    def _convert_(self, val: Any) -> _OptT:
        return cast("_OptT", val)

    def __set_name__(self, owner, name) -> None:
        if self._flag_names is None:
            kebab_name = name.strip("_").replace("_", "-")
            self._flag_names = (f"--{kebab_name}",)

    @overload
    def __get__(self, obj: None, objtype: Any) -> OptionInfo | None: ...

    @overload
    def __get__(self, obj: object, objtype: Any) -> _OptT: ...

    def __get__(self, obj, objtype):
        assert self._flag_names is not None
        if obj is None:
            return OptionInfo(self._flag_names)
        long_name = self._flag_names[-1]
        option_value = obj.options.get(long_name[2:].replace("-", "_"))
        if option_value is None:
            return None
        return self._convert_(option_value)


_IntDefault = TypeVar("_IntDefault", int, None)

class IntOption(_OptionBase[int, _IntDefault]):
    option_type: Any = int


class ExampleOption(IntOption):
    pass


class OptionConsumer:
    example = ExampleOption(default=None)

    @property
    def options(self):
        return {"example": 30}


class HasExampleOption(Protocol):
    example: ExampleOption


oc = OptionConsumer()
oc2 = cast(HasExampleOption, oc)

if TYPE_CHECKING:
    reveal_type(oc.example)
    reveal_type(oc2.example)

(This code could probably be further simplified, but is sufficient to reproduce the bug. )

Expected Behavior

The expected behavior is that the access through the HasExampleOption protocol to the example attribute has type int. Here is the output when running v1.15.0 against the code:

src/grok.py:90: note: Revealed type is "builtins.int"
src/grok.py:91: note: Revealed type is "builtins.int"

Actual Behavior

v1.16.0 regresses and reports the type of example as the descriptor's class ExampleOption and not as int:

src/grok.py:90: note: Revealed type is "builtins.int"
src/grok.py:91: note: Revealed type is "grok.ExampleOption"

Your Environment

• Mypy version used: v1.15.0 and v1.16.0
• Mypy command-line flags: none
• Mypy configuration options from mypy.ini (and other config files): none
• Python version used: 3.11.12

[sterliakov]

Is it the same issue as below?

from typing import Any, Protocol, overload

class Descriptor:
    @overload
    def __get__(self, obj: None, objtype: Any) -> "Descriptor": ...
    @overload
    def __get__(self, obj: object, objtype: Any) -> int: ...
    def __get__(self, obj, objtype): ...

class Foo:
    example = Descriptor()

class FooP(Protocol):
    example: Descriptor


oc = Foo()
oc2: FooP = oc

reveal_type(oc.example)
reveal_type(oc2.example)

https://mypy-play.net/?mypy=latest&python=3.12&flags=strict&gist=62b88bc128305716903821b1b3333f29

I'm not exactly sure what should protocol members annotated with descriptors mean, TBH. mypy is now inconsistent (note that assignability above is fine). However, x: Something in protocol means "instance.x is compatible with Something", and that's not the case here, so revealing descriptor and rejecting assignment also sounds reasonable to me.

@ilevkivskyi was this change intentional? Bisects to #18831

[sterliakov]

Whoops, sorry, you already answered. Closing as duplicate of #19054 (sorry, I somehow missed that one entirely...)

[ilevkivskyi]

Yes, this is an intentional change. For long time mypy didn't support descriptors in protocols, so unfortunately some people used a workaround of writing descriptor type directly in the protocol. The motivation for the change is that protocols only specify the interface, so that we can reason about e.g. modules and class objects as implementation of protocols. By default, something like this

class P(Protocol):
    attr: SomeType

is equivalent to this in a regular class:

class C:
    def __init__(self) -> None:
        self.attr: SomeType

Now (on current master) you should be able to write simply this:

class HasExampleOption(Protocol):
    example: int

However, now I am thinking we should probably also allow something like this:

class HasExampleOption(Protocol):
    example: ClassVar[ExampleOption]

Let me play a bit with this.

[sterliakov]

@ilevkivskyi ClassVar[Descriptor] sounds interesting - somewhat closer to runtime semantics. And I agree using descriptor itself in annotation is too ambiguous to argue about, the new approach is more reasonable IMO (because, again, simple protocols roughly mean "this instance has an attribute of this type", and instances don't have descriptor-valued attributes). Assignability is more worrying: member access and assignability rules should stay in sync (oc2: FooP = oc should not be allowed too).

[ilevkivskyi]

@sterliakov

Assignability is more worrying

I am not sure what do you mean? On current master I get an error in oc2: FooP = oc. Or do you mean in v1.16? If latter, then yeah, it was a mistake to have a release in between the two protocol vs descriptor PRs. This may be confusing for users. As an option (and what I suggested initially to @JukkaL) we may revert the first PR in the release branch (and include the revert in a point release).

[sterliakov]

Ough, I definitely need another coffee. Yes, works as intended on master, it's only 1.16 that is broken. Perhaps consider releasing 1.16.1 (either with some PRs cherry-picked or just from current master if there aren't any huge breaking changes)?

Aside, I still think your ClassVar suggestion would be nice to implement. That would support some runtime introspection better and allow almost-consistent implementation and interface definition.

[ilevkivskyi]

@sterliakov

Aside, I still think your ClassVar suggestion would be nice to implement.

Yeah, I will probably make a PR soon.

Btw another thing is that last few days I was looking into the whole class variable vs instance variable story, and it is really messy. (And this is because in Python the distinction is blurry, similar to how we have enormous amount of complexity because positional-or-keyword arguments.) Maybe I would go as far as suggesting this is a topic for the typing council discussion. We should have some clear rules for:

• Class variables (explicit and implicit)
• Instance variables (explicit and implicit)
• Instance variable with default
• Descriptors
• Variables with callable types
• Type[...]
• And probably other edge cases I forgot

[ilevkivskyi]

Yeah, I will probably make a PR soon.

For now I think we can do something like #19277

[tdyas]

Thanks for the explanations!

Some data points for any future work here:

• I tried the suggestion of switching the example attribute's type to int from the ExampleOption descriptor. That worked when the only type being considered was HasExampleOption.
• That switch though caused a new type error. The owner class of the descriptor mixes in HasExampleOption and now has a type error similar to this:

src/python/pants/core/goals/fix.py:411:9: error: Argument 5 to "_do_fix" has incompatible type "FixSubsystem"; expected "_BatchableMultiToolGoalSubsystem"  [arg-type]
            fix_subsystem,
            ^~~~~~~~~~~~~
src/python/pants/core/goals/fix.py:411:9: note: Following member(s) of "FixSubsystem" have conflicts:
src/python/pants/core/goals/fix.py:411:9: note:     batch_size: expected "int", got "BatchSizeOption"

(_BatchableMultiToolGoalSubsystem is the concrete example of what is HasExampleOption in the reproduction code in this PR. And BatchSizeOption is the equivalent of ExampleOption.)

Thus, the owner class is no longer compliant with the protocol when switching the type of the descriptor in the protocol like that.

[sterliakov]

Could you check the same using master mypy (pip install 'git+https://github.com/python/mypy')? That will be slower, but the diagnostic is very similar to the discrepancy we discussed above, it should pass on master.

[tdyas]

Tested with git+https://github.com/python/mypy@61cbe0c3cada14eb3638de8becff8988ae8cb1db and it passes.

[JukkaL]

i will make a 1.16.1 bugfix release soon, but I think it won't include a fix to this issue. However, we are planning to start work on the 1.17 feature release pretty soon (hopefully within a week, but we'll see).