Skip to content

@property hides multiple type errors #11892

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

Closed
Dreamsorcerer opened this issue Jan 3, 2022 · 4 comments · Fixed by #18510
Closed

@property hides multiple type errors #11892

Dreamsorcerer opened this issue Jan 3, 2022 · 4 comments · Fixed by #18510
Labels
bug mypy got something wrong topic-descriptors Properties, class vs. instance attributes

Comments

@Dreamsorcerer
Copy link
Contributor

Dreamsorcerer commented Jan 3, 2022

Bug Report

This code defines self._body as Optional[bytes] and does not produce any type errors: https://github.com/aio-libs/aiohttp/blob/f1eb09b3756f4ed378a25f8a634f734952d2da35/aiohttp/web_response.py#L589
If I instead define the type on the class, like:

class Response(StreamResponse):
    _body: Optional[bytes]

Then I get a type error: Incompatible types in assignment (expression has type "Optional[bytes]", base class "StreamResponse" defined the type as "None") [assignment]

To Reproduce

See next comment.

Your Environment

@Dreamsorcerer Dreamsorcerer added the bug mypy got something wrong label Jan 3, 2022
@Dreamsorcerer
Copy link
Contributor Author

Dreamsorcerer commented Jan 3, 2022

OK, seems to be a problem with @property, managed to create a minimal reproducer. This produces no type errors, but there are actually 2 issues in this code:

from typing import Optional

class Foo:
    def __init__(self) -> None:
        self._a = None  # Foo._a is defined as None

class Bar(Foo):
    def __init__(self, a: Optional[bytes]) -> None:
        self.a = a  # setter says type should be bytes, but we are assigning Optional[bytes].

    @property
    def a(self) -> Optional[bytes]:
        return self._a

    @a.setter
    def a(self, thing: bytes) -> None:
        self._a: Optional[bytes] = thing  # We are defining _a as Optional[bytes], but superclass says it should be None.

@Dreamsorcerer Dreamsorcerer changed the title Incompatible types not recognised in some situations @property hides mulitple type errors Jan 3, 2022
@Dreamsorcerer Dreamsorcerer changed the title @property hides mulitple type errors @property hides multiple type errors Jan 3, 2022
@Dreamsorcerer
Copy link
Contributor Author

It also causes incorrect type errors. If I try to assign to a property, it will give a type error if the type does not match the return type, when it should be checking the parameter type in the setter:

    @property
    def last_modified(self) -> Optional[datetime.datetime]: ...

    @last_modified.setter
    def last_modified(self, value: Optional[Union[int, float, datetime.datetime, str]]) -> None: ...
resp.last_modified = filepath.stat().st_mtime
error: Incompatible types in assignment (expression has type "float", variable has type "Optional[datetime]")  [assignment]

@hauntsaninja
Copy link
Collaborator

mypy doesn't do setters that take different types particularly well, see #3004 There's been some movement on that issue recently, so hopefully gets fixed soon

@AlexWaygood AlexWaygood added the topic-descriptors Properties, class vs. instance attributes label Mar 27, 2022
@AlexWaygood
Copy link
Member

This produces no type errors, but there are actually 2 issues in this code

On 0.941, mypy now does report one of these errors (but still not the other one):

from typing import Optional

class Foo:
   def __init__(self) -> None:
       self._a = None  # Foo._a is defined as None

class Bar(Foo):
   def __init__(self, a: Optional[bytes]) -> None:
       self.a = a  # setter says type should be bytes, but we are assigning Optional[bytes].

   @property
   def a(self) -> Optional[bytes]:
       return self._a

   @a.setter
   def a(self, thing: bytes) -> None:
       self._a: Optional[bytes] = thing  # error: Incompatible types in assignment (expression has type "Optional[bytes]", base class "Foo" defined the type as "None")

x612skm pushed a commit to x612skm/mypy-dev that referenced this issue Feb 24, 2025
…n#18510)

Fixes python#3004
Fixes python#11892
Fixes python#12892
Fixes python#14301

_Note:_ this PR should be reviewed with "hide whitespace" option (in
couple long functions I replace huge `if x: ...` with `if not x: return;
...` to reduce indent level).

The core logic is quite straightforward (almost trivial). The only
couple things are:
* We should be careful with binder (we simpy can't use it for properties
with setter type different from getter type, since we don't know
underlying property implementation)
* We need to handle gracefully existing settable properties that are
generated by plugins

The tricky part is subclassing and protocols. The summary is as
following:
* For protocols I simply implement everything the "correct way", i.e.
for settable attributes (whether it is a variable or a settable
property) compare getter types covariantly and setter types
contravariantly. The tricky part here is generating meaningful error
messages that are also not too verbose.
* For subclassing I cannot simply do the same, because there is a flag
about covariant mutable override, that is off by default. So instead
what I do is if either subclass node, or superclass node is a "custom
property" (i.e. a property with setter type different from getter type),
then I use the "correct way", otherwise the old logic (i.e. flag
dependent check) is used.

Two things that are not implemented are multiple inheritance, and new
generic syntax (inferred variance). In these cases setter types are
simply ignored. There is nothing conceptually difficult about these, I
simply run out of steam (and the PR is getting big). I left `TODO`s in
code for these. In most cases these will generate false negatives (and
they are already kind of corner cases) so I think it is OK to postpone
these indefinitely.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong topic-descriptors Properties, class vs. instance attributes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants