-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Comments
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 |
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") |
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… |
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 What I see as a problem is rather that when you then try to set Maybe close this ticket? |
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 |
…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.
none of these examples seem to be fixed as of 1.15.0 |
The fix will be in the next feature release. Have you tried master? |
https://mypy-play.net/?mypy=latest&python=3.10&flags=show-error-context%2Cshow-error-codes%2Cstrict%2Ccheck-untyped-defs%2Cdisallow-any-decorated%2Cdisallow-any-expr%2Cdisallow-any-explicit%2Cdisallow-any-generics%2Cdisallow-any-unimported%2Cdisallow-incomplete-defs%2Cdisallow-subclassing-any%2Cdisallow-untyped-calls%2Cdisallow-untyped-decorators%2Cdisallow-untyped-defs%2Cno-implicit-optional%2Cno-implicit-reexport%2Clocal-partial-types%2Cstrict-equality%2Cwarn-incomplete-stub%2Cwarn-redundant-casts%2Cwarn-return-any%2Cwarn-unreachable%2Cwarn-unused-configs%2Cwarn-unused-ignores&gist=3a081cf4b94705271106cf081232887f
The text was updated successfully, but these errors were encountered: