Skip to content

signatures of setters are not checked #12892

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
DetachHead opened this issue May 28, 2022 · 7 comments · Fixed by #18510
Closed

signatures of setters are not checked #12892

DetachHead opened this issue May 28, 2022 · 7 comments · Fixed by #18510
Labels
bug mypy got something wrong topic-descriptors Properties, class vs. instance attributes topic-runtime-semantics mypy doesn't model runtime semantics correctly

Comments

@DetachHead
Copy link
Contributor

@DetachHead DetachHead added the bug mypy got something wrong label May 28, 2022
@DetachHead
Copy link
Contributor Author

also no error when there's no second argument:

class A:
    @property
    def foo(self) -> int:
        ...
    @foo.setter
    def foo(self) -> None: # no mypy error
        ...


a = A()

a.foo = 1 # runtime error - TypeError: A.foo() takes 1 positional argument but 2 were given

@DetachHead DetachHead changed the title no error when setter has too many arguments signatures of setters are not checked May 28, 2022
@AlexWaygood AlexWaygood added topic-runtime-semantics mypy doesn't model runtime semantics correctly topic-descriptors Properties, class vs. instance attributes labels May 28, 2022
@DetachHead
Copy link
Contributor Author

and also no error when the setter type is different to the getter type

class Foo:
    @property
    def foo(self) -> int:
        ...

    @foo.setter
    def foo(self, value: str) -> None:
        ...

however you get an error when attempting to use the setter:

f = Foo()
f.foo = "" # Incompatible types in assignment (expression has type "str", variable has type "int")

@q-wertz
Copy link

q-wertz commented Feb 3, 2023

Similar example:

class A:
    def init(self) -> None:
        self._a: int | None = None

    @property
    def a(self) -> int:
        if self._a is None:
            raise ValueError
        return self._a

    @a.setter
    def a(self, val: int | None) -> None:
        self._a = val


a = A()

a.a = None  # Incompatible types in assignment (expression has type "None", variable has type "int")  [assignment]

In my scenario I would like it to work like this but if it is bad practice that the setter allows different types than the getter it should warn in the definition of the setter…

@ydirson
Copy link

ydirson commented May 12, 2023

I would argue that it is not necessarily a problem in itself. We can use this to implement a "smart setter" that takes as type to build on this example Union[int, str], and tries to convert a str to an int.

What I see as a problem is rather that when you then try to set foo in a way compatible with the declaration and implementation (ie. assign a string to it), then despite the code being correct, mypy will ignore the type declared in the setter, and validate against the getter type instead - see #3004 where the consensus is that this is a valid use case.

Maybe close this ticket?

@DetachHead
Copy link
Contributor Author

Even if #3004 gets implemented, there are still several examples here of invalid setters that should be errors (incorrect number of arguments) so I'm leaving this issue open regardless

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.
@DetachHead
Copy link
Contributor Author

none of these examples seem to be fixed as of 1.15.0

@JukkaL
Copy link
Collaborator

JukkaL commented Mar 27, 2025

The fix will be in the next feature release. Have you tried master?

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 topic-runtime-semantics mypy doesn't model runtime semantics correctly
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants