-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[flake8-pyi] Fix custom-typevar-for-self with string annotations (PYI019)
#18311
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
Changes from 7 commits
2c7dcd3
4381158
87c9e60
4ca6eca
99e8ffe
a40fad6
6cd6a6c
b2d8710
b5dc72d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -690,3 +690,96 @@ PYI019_0.py:173:10: PYI019 [*] Use `Self` instead of custom TypeVar `S` | |
| 174 174 | type S = int | ||
| 175 175 | print(S) # not a reference to the type variable, so not touched by the autofix | ||
| 176 176 | return 42 | ||
|
|
||
| PYI019_0.py:189:52: PYI019 [*] Use `Self` instead of custom TypeVar `_S` | ||
| | | ||
| 188 | class BadClassWithStringTypeHints: | ||
| 189 | def bad_instance_method_with_string_annotations(self: "_S", arg: str) -> "_S": ... # PYI019 | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI019 | ||
| 190 | | ||
| 191 | @classmethod | ||
| | | ||
| = help: Replace TypeVar `_S` with `Self` | ||
|
|
||
| ℹ Safe fix | ||
| 186 186 | from __future__ import annotations | ||
| 187 187 | | ||
| 188 188 | class BadClassWithStringTypeHints: | ||
| 189 |- def bad_instance_method_with_string_annotations(self: "_S", arg: str) -> "_S": ... # PYI019 | ||
| 189 |+ def bad_instance_method_with_string_annotations(self, arg: str) -> "Self": ... # PYI019 | ||
|
Comment on lines
+708
to
+709
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We may also need to adjust the replacement range. I don't think we want to replace
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried to look around for the right way how to extend this behaviour but failed even with ugly approach. So, do we need this, as some other rule applies a fix to this moment right after, we just see "Self" in tests only related to this rule? I can't disagree that rule should do this on its own, but it will require more code changes and help from others or more time.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, you may be right. I looked at this a bit and didn't see a good solution either. I was afraid that we might be adding an unused |
||
| 190 190 | | ||
| 191 191 | @classmethod | ||
| 192 192 | def bad_class_method_with_string_annotations(cls: "type[_S]") -> "_S": ... # PYI019 | ||
|
|
||
| PYI019_0.py:192:49: PYI019 [*] Use `Self` instead of custom TypeVar `_S` | ||
| | | ||
| 191 | @classmethod | ||
| 192 | def bad_class_method_with_string_annotations(cls: "type[_S]") -> "_S": ... # PYI019 | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^ PYI019 | ||
| | | ||
| = help: Replace TypeVar `_S` with `Self` | ||
|
|
||
| ℹ Safe fix | ||
| 189 189 | def bad_instance_method_with_string_annotations(self: "_S", arg: str) -> "_S": ... # PYI019 | ||
| 190 190 | | ||
| 191 191 | @classmethod | ||
| 192 |- def bad_class_method_with_string_annotations(cls: "type[_S]") -> "_S": ... # PYI019 | ||
| 192 |+ def bad_class_method_with_string_annotations(cls) -> "Self": ... # PYI019 | ||
| 193 193 | | ||
| 194 194 | | ||
| 195 195 | @classmethod | ||
|
|
||
| PYI019_0.py:196:50: PYI019 [*] Use `Self` instead of custom TypeVar `_S` | ||
| | | ||
| 195 | @classmethod | ||
| 196 | def bad_class_method_with_mixed_annotations_1(cls: "type[_S]") -> _S: ... # PYI019 | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^ PYI019 | ||
| | | ||
| = help: Replace TypeVar `_S` with `Self` | ||
|
|
||
| ℹ Safe fix | ||
| 193 193 | | ||
| 194 194 | | ||
| 195 195 | @classmethod | ||
| 196 |- def bad_class_method_with_mixed_annotations_1(cls: "type[_S]") -> _S: ... # PYI019 | ||
| 196 |+ def bad_class_method_with_mixed_annotations_1(cls) -> Self: ... # PYI019 | ||
| 197 197 | | ||
| 198 198 | | ||
| 199 199 | @classmethod | ||
|
|
||
| PYI019_0.py:200:50: PYI019 [*] Use `Self` instead of custom TypeVar `_S` | ||
| | | ||
| 199 | @classmethod | ||
| 200 | def bad_class_method_with_mixed_annotations_1(cls: type[_S]) -> "_S": ... # PYI019 | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^ PYI019 | ||
| | | ||
| = help: Replace TypeVar `_S` with `Self` | ||
|
|
||
| ℹ Safe fix | ||
| 197 197 | | ||
| 198 198 | | ||
| 199 199 | @classmethod | ||
| 200 |- def bad_class_method_with_mixed_annotations_1(cls: type[_S]) -> "_S": ... # PYI019 | ||
| 200 |+ def bad_class_method_with_mixed_annotations_1(cls) -> "Self": ... # PYI019 | ||
| 201 201 | | ||
| 202 202 | | ||
| 203 203 | class BadSubscriptReturnTypeWithStringTypeHints: | ||
|
|
||
| PYI019_0.py:205:10: PYI019 [*] Use `Self` instead of custom TypeVar `S` | ||
| | | ||
| 203 | class BadSubscriptReturnTypeWithStringTypeHints: | ||
| 204 | @classmethod | ||
| 205 | def m[S](cls: "type[S]") -> "type[S]": ... # PYI019 | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI019 | ||
| | | ||
| = help: Replace TypeVar `S` with `Self` | ||
|
|
||
| ℹ Safe fix | ||
| 202 202 | | ||
| 203 203 | class BadSubscriptReturnTypeWithStringTypeHints: | ||
| 204 204 | @classmethod | ||
| 205 |- def m[S](cls: "type[S]") -> "type[S]": ... # PYI019 | ||
| 205 |+ def m(cls) -> "type[Self]": ... # PYI019 | ||
| 206 206 | | ||
| 207 207 | | ||
| 208 208 | class GoodClassWiStringTypeHints: | ||
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.
I have two suggestions here:
One is just to simplify the
as_string_literal_expr, which can be combined with thematcharm itself, and the second is that the original code didn't restrict theExprto aSubscriptorName, so I don't think we should do that here either. Instead any otherExprshould be allowed, as before, with the wildcard arm (_).