-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ty] Assume type of self is typing.Self in method calls
#18007
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
[ty] Assume type of self is typing.Self in method calls
#18007
Conversation
9e2b11b to
419d085
Compare
775f97d to
d213dcc
Compare
|
I'm un drafting to get some suggestions. Then I'll be quick in adjusting the PR. |
58e61b4 to
95518f5
Compare
| ); | ||
| return Ok(Type::unknown()); | ||
| }; | ||
| let class_ty = infer_definition_types(db, class_def) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From an older comment on the typing.Self PR.
95518f5 to
c3c935b
Compare
c3c935b to
c795732
Compare
|
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
9a697f6 to
63531ea
Compare
typing.Selftyping.Self
typing.Selftyping.Self in method calls
6f8a2b4 to
178f48f
Compare
Co-Authored-by: David Peter <mail@david-peter.de>
c7dcdcc to
ed798a3
Compare
| reveal_type(x) # revealed: list[A | str] | ||
| for item in x: | ||
| reveal_type(item) # revealed: list[A | str] | str | ||
| reveal_type(item) # revealed: list[Any | str] | str |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This regression is tracked in astral-sh/ty#1157
|
I am planning to make a couple of additional changes here, and I'm also going to merge in other branches. So I'm going to merge this PR into a new integration branch (#20517). That also makes it a bit easier for me to run |
92e01d5
into
astral-sh:david/implicit-self-signature
…#20479) ### Summary This PR includes two changes, both of which are necessary to resolve astral-sh/ty#1196: * For a generic class `C[T]`, we previously used `C[Unknown]` as the upper bound of the `Self` type variable. There were two problems with this. For one, when `Self` appeared in contravariant position, we would materialize its upper bound to `Bottom[C[Unknown]]` (which might simplify to `C[Never]` if `C` is covariant in `T`) when accessing methods on `Top[C[Unknown]]`. This would result in `invalid-argument` errors on the `self` parameter. Also, using an upper bound of `C[Unknown]` would mean that inside methods, references to `T` would be treated as `Unknown`. This could lead to false negatives. To fix this, we now use `C[T]` (with a "nested" typevar) as the upper bound for `Self` on `C[T]`. * In order to make this work, we needed to allow assignability/subtyping of inferable typevars to other types, since we now check assignability of e.g. `C[int]` to `C[T]` (when checking assignability to the upper bound of `Self`) when calling an instance-method on `C[int]` whose `self` parameter is annotated as `self: Self` (or implicitly `Self`, following #18007). closes astral-sh/ty#1196 closes astral-sh/ty#1208 ### Test Plan Regression tests for both issues.
Part of astral-sh/ty#159 This PR only adjusts the signature of a method so if it has a `self` argument then that argument will have type of `Typing.Self` even if it's not specified. If user provides an explicit annotation then Ty will not override that annotation. - astral-sh/ty#1131 - astral-sh/ty#1157 - astral-sh/ty#1156 - astral-sh/ty#1173 - #20328 - astral-sh/ty#1163 - astral-sh/ty#1196 Added mdtests. Also some tests need #18473 to work completely. So I added a todo for those new cases that I added. --------- Co-authored-by: David Peter <mail@david-peter.de>
Part of astral-sh/ty#159 This PR only adjusts the signature of a method so if it has a `self` argument then that argument will have type of `Typing.Self` even if it's not specified. If user provides an explicit annotation then Ty will not override that annotation. - astral-sh/ty#1131 - astral-sh/ty#1157 - astral-sh/ty#1156 - astral-sh/ty#1173 - #20328 - astral-sh/ty#1163 - astral-sh/ty#1196 Added mdtests. Also some tests need #18473 to work completely. So I added a todo for those new cases that I added. --------- Co-authored-by: David Peter <mail@david-peter.de>
Part of astral-sh/ty#159 This PR only adjusts the signature of a method so if it has a `self` argument then that argument will have type of `Typing.Self` even if it's not specified. If user provides an explicit annotation then Ty will not override that annotation. - astral-sh/ty#1131 - astral-sh/ty#1157 - astral-sh/ty#1156 - astral-sh/ty#1173 - #20328 - astral-sh/ty#1163 - astral-sh/ty#1196 Added mdtests. Also some tests need #18473 to work completely. So I added a todo for those new cases that I added. --------- Co-authored-by: David Peter <mail@david-peter.de>
Part of astral-sh/ty#159 This PR only adjusts the signature of a method so if it has a `self` argument then that argument will have type of `Typing.Self` even if it's not specified. If user provides an explicit annotation then Ty will not override that annotation. - astral-sh/ty#1131 - astral-sh/ty#1157 - astral-sh/ty#1156 - astral-sh/ty#1173 - #20328 - astral-sh/ty#1163 - astral-sh/ty#1196 Added mdtests. Also some tests need #18473 to work completely. So I added a todo for those new cases that I added. --------- Co-authored-by: David Peter <mail@david-peter.de>
Part of astral-sh/ty#159 This PR only adjusts the signature of a method so if it has a `self` argument then that argument will have type of `Typing.Self` even if it's not specified. If user provides an explicit annotation then Ty will not override that annotation. - astral-sh/ty#1131 - astral-sh/ty#1157 - astral-sh/ty#1156 - astral-sh/ty#1173 - #20328 - astral-sh/ty#1163 - astral-sh/ty#1196 Added mdtests. Also some tests need #18473 to work completely. So I added a todo for those new cases that I added. --------- Co-authored-by: David Peter <mail@david-peter.de>
Part of astral-sh/ty#159 This PR only adjusts the signature of a method so if it has a `self` argument then that argument will have type of `Typing.Self` even if it's not specified. If user provides an explicit annotation then Ty will not override that annotation. - astral-sh/ty#1131 - astral-sh/ty#1157 - astral-sh/ty#1156 - astral-sh/ty#1173 - #20328 - astral-sh/ty#1163 - astral-sh/ty#1196 Added mdtests. Also some tests need #18473 to work completely. So I added a todo for those new cases that I added. --------- Co-authored-by: David Peter <mail@david-peter.de>
Part of astral-sh/ty#159 This PR only adjusts the signature of a method so if it has a `self` argument then that argument will have type of `Typing.Self` even if it's not specified. If user provides an explicit annotation then Ty will not override that annotation. - astral-sh/ty#1131 - astral-sh/ty#1157 - astral-sh/ty#1156 - astral-sh/ty#1173 - #20328 - astral-sh/ty#1163 - astral-sh/ty#1196 Added mdtests. Also some tests need #18473 to work completely. So I added a todo for those new cases that I added. --------- Co-authored-by: David Peter <mail@david-peter.de>
Summary
Part of astral-sh/ty#159
This PR only adjusts the signature of a method so if it has a
selfargument then that argument will have type ofTyping.Selfeven if it's not specified. If user provides an explicit annotation then Ty will not override that annotation.Follow Ups
selfargument when a generic context exists ty#1131Anyin return types of generic methods ty#1157Selffor fallback-methods onNamedTuples andTypedDicts #20328Top[list[Unknown]]does not satisfy upper boundBottom[MutableSequence[Unknown]]of type variableSelfty#1196Test Plan
Added mdtests.
Also some tests need #18473 to work completely. So I added a todo for those new cases that I added.