Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI019_0.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,3 +181,34 @@ def m[S](self: S) -> S:
class MetaTestClass(type):
def m(cls: MetaType) -> MetaType:
return cls


from __future__ import annotations

class BadClassWithStringTypeHints:
def bad_instance_method_with_string_annotations(self: "_S", arg: str) -> "_S": ... # PYI019

@classmethod
def bad_class_method_with_string_annotations(cls: "type[_S]") -> "_S": ... # PYI019


@classmethod
def bad_class_method_with_mixed_annotations_1(cls: "type[_S]") -> _S: ... # PYI019


@classmethod
def bad_class_method_with_mixed_annotations_1(cls: type[_S]) -> "_S": ... # PYI019


class BadSubscriptReturnTypeWithStringTypeHints:
@classmethod
def m[S](cls: "type[S]") -> "type[S]": ... # PYI019


class GoodClassWiStringTypeHints:
@classmethod
def good_cls_method_with_mixed_annotations(cls: "type[Self]", arg: str) -> Self: ...
@staticmethod
def good_static_method_with_string_annotations(arg: "_S") -> "_S": ...
@classmethod
def good_class_method_with_args_string_annotations(cls, arg1: "_S", arg2: "_S") -> "_S": ...
33 changes: 33 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI019_0.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -172,3 +172,36 @@ MetaType = TypeVar("MetaType")
class MetaTestClass(type):
def m(cls: MetaType) -> MetaType:
return cls


from __future__ import annotations


class BadClassWithStringTypeHints:
def bad_instance_method_with_string_annotations(self: "_S", arg: str) -> "_S": ... # PYI019

@classmethod
def bad_class_method_with_string_annotations(cls: "type[_S]") -> "_S": ... # PYI019


@classmethod
def bad_class_method_with_mixed_annotations_1(cls: "type[_S]") -> _S: ... # PYI019


@classmethod
def bad_class_method_with_mixed_annotations_1(cls: type[_S]) -> "_S": ... # PYI019


class BadSubscriptReturnTypeWithStringTypeHints:
@classmethod
def m[S](cls: "type[S]") -> "type[S]": ... # PYI019


class GoodClassWithStringTypeHints:
@classmethod
def good_cls_method_with_mixed_annotations(cls: "type[Self]", arg: str) -> Self: ...
@staticmethod
def good_static_method_with_string_annotations(arg: "_S") -> "_S": ...
@classmethod
def good_class_method_with_args_string_annotations(cls, arg1: "_S", arg2: "_S") -> "_S": ...

Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,18 @@ pub(crate) fn custom_type_var_instead_of_self(checker: &Checker, binding: &Bindi
return;
};

let Some(self_or_cls_annotation) = self_or_cls_parameter.annotation() else {
let Some(self_or_cls_annotation_unchecked) = self_or_cls_parameter.annotation() else {
return;
};
let self_or_cls_annotation = match self_or_cls_annotation_unchecked {
ast::Expr::StringLiteral(literal_expr) => {
let Ok(parsed_expr) = checker.parse_type_annotation(literal_expr) else {
return;
};
parsed_expr.expression()
}
_ => self_or_cls_annotation_unchecked,
};
let Some(parent_class) = current_scope.kind.as_class() else {
return;
};
Expand Down Expand Up @@ -202,7 +211,7 @@ pub(crate) fn custom_type_var_instead_of_self(checker: &Checker, binding: &Bindi
function_def,
custom_typevar,
self_or_cls_parameter,
self_or_cls_annotation,
self_or_cls_annotation_unchecked,
)
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The 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 "_S" with "Self". I think it should just be an unquoted Self.

Copy link
Contributor Author

@close2code-palm close2code-palm Jun 14, 2025

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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 Self import if all of the Self instances were quoted, but that doesn't appear to be the case: https://play.ruff.rs/014e3efe-829c-42b9-af40-f3527508bc57 (no import warnings with ALL rules selected). So I'm happy moving forward with the current version. Thanks!

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:
Original file line number Diff line number Diff line change
Expand Up @@ -682,3 +682,96 @@ PYI019_0.pyi:167:10: PYI019 [*] Use `Self` instead of custom TypeVar `S`
168 168 |
169 169 |
170 170 | MetaType = TypeVar("MetaType")

PYI019_0.pyi:181:52: PYI019 [*] Use `Self` instead of custom TypeVar `_S`
|
180 | class BadClassWithStringTypeHints:
181 | def bad_instance_method_with_string_annotations(self: "_S", arg: str) -> "_S": ... # PYI019
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI019
182 |
183 | @classmethod
|
= help: Replace TypeVar `_S` with `Self`

ℹ Safe fix
178 178 |
179 179 |
180 180 | class BadClassWithStringTypeHints:
181 |- def bad_instance_method_with_string_annotations(self: "_S", arg: str) -> "_S": ... # PYI019
181 |+ def bad_instance_method_with_string_annotations(self, arg: str) -> "Self": ... # PYI019
182 182 |
183 183 | @classmethod
184 184 | def bad_class_method_with_string_annotations(cls: "type[_S]") -> "_S": ... # PYI019

PYI019_0.pyi:184:49: PYI019 [*] Use `Self` instead of custom TypeVar `_S`
|
183 | @classmethod
184 | def bad_class_method_with_string_annotations(cls: "type[_S]") -> "_S": ... # PYI019
| ^^^^^^^^^^^^^^^^^^^^^^^^^ PYI019
|
= help: Replace TypeVar `_S` with `Self`

ℹ Safe fix
181 181 | def bad_instance_method_with_string_annotations(self: "_S", arg: str) -> "_S": ... # PYI019
182 182 |
183 183 | @classmethod
184 |- def bad_class_method_with_string_annotations(cls: "type[_S]") -> "_S": ... # PYI019
184 |+ def bad_class_method_with_string_annotations(cls) -> "Self": ... # PYI019
185 185 |
186 186 |
187 187 | @classmethod

PYI019_0.pyi:188:50: PYI019 [*] Use `Self` instead of custom TypeVar `_S`
|
187 | @classmethod
188 | def bad_class_method_with_mixed_annotations_1(cls: "type[_S]") -> _S: ... # PYI019
| ^^^^^^^^^^^^^^^^^^^^^^^ PYI019
|
= help: Replace TypeVar `_S` with `Self`

ℹ Safe fix
185 185 |
186 186 |
187 187 | @classmethod
188 |- def bad_class_method_with_mixed_annotations_1(cls: "type[_S]") -> _S: ... # PYI019
188 |+ def bad_class_method_with_mixed_annotations_1(cls) -> Self: ... # PYI019
189 189 |
190 190 |
191 191 | @classmethod

PYI019_0.pyi:192:50: PYI019 [*] Use `Self` instead of custom TypeVar `_S`
|
191 | @classmethod
192 | def bad_class_method_with_mixed_annotations_1(cls: type[_S]) -> "_S": ... # PYI019
| ^^^^^^^^^^^^^^^^^^^^^^^ PYI019
|
= help: Replace TypeVar `_S` with `Self`

ℹ Safe fix
189 189 |
190 190 |
191 191 | @classmethod
192 |- def bad_class_method_with_mixed_annotations_1(cls: type[_S]) -> "_S": ... # PYI019
192 |+ def bad_class_method_with_mixed_annotations_1(cls) -> "Self": ... # PYI019
193 193 |
194 194 |
195 195 | class BadSubscriptReturnTypeWithStringTypeHints:

PYI019_0.pyi:197:10: PYI019 [*] Use `Self` instead of custom TypeVar `S`
|
195 | class BadSubscriptReturnTypeWithStringTypeHints:
196 | @classmethod
197 | def m[S](cls: "type[S]") -> "type[S]": ... # PYI019
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI019
|
= help: Replace TypeVar `S` with `Self`

ℹ Safe fix
194 194 |
195 195 | class BadSubscriptReturnTypeWithStringTypeHints:
196 196 | @classmethod
197 |- def m[S](cls: "type[S]") -> "type[S]": ... # PYI019
197 |+ def m(cls) -> "type[Self]": ... # PYI019
198 198 |
199 199 |
200 200 | class GoodClassWithStringTypeHints:
Loading