From 0b73f343bc758d2b4c0cec12fc42fcbe9a837999 Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Mon, 31 Jul 2023 13:57:21 +0200 Subject: [PATCH 1/8] Add boilerplate --- .../test/fixtures/flake8_pyi/PYI019.py | 31 +++++++ .../test/fixtures/flake8_pyi/PYI019.pyi | 31 +++++++ .../src/checkers/ast/analyze/statement.rs | 11 +++ crates/ruff/src/codes.rs | 1 + crates/ruff/src/rules/flake8_pyi/mod.rs | 2 + .../flake8_pyi/rules/custom_typevar_return.rs | 81 +++++++++++++++++++ crates/ruff/src/rules/flake8_pyi/rules/mod.rs | 2 + 7 files changed, 159 insertions(+) create mode 100644 crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.py create mode 100644 crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.pyi create mode 100644 crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.py b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.py new file mode 100644 index 0000000000000..f85694bff14c1 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.py @@ -0,0 +1,31 @@ +from typing import TypeVar, Self + +_S = TypeVar("_S", bound=BadClass) +_S2 = TypeVar("_S2", BadClass, GoodClass) + +class BadClass: + def __new__(cls: type[_S], *args: str, **kwargs: int) -> _S: ... # Ok + def bad_instance_method(self: _S, arg: bytes) -> _S: ... # Ok + @classmethod + def bad_class_method(cls: type[_S], arg: int) -> _S: ... # Ok + +class GoodClass: + def __new__(cls: type[Self], *args: list[int], **kwargs: set[str]) -> Self: ... + def good_instance_method_1(self: Self, arg: bytes) -> Self: ... + def good_instance_method_2(self, arg1: _S2, arg2: _S2) -> _S2: ... + @classmethod + def good_cls_method_1(cls: type[Self], arg: int) -> Self: ... + @classmethod + def good_cls_method_2(cls, arg1: _S, arg2: _S) -> _S: ... + @staticmethod + def static_method(arg1: _S) -> _S: ... + + +# Python > 3.12 +class PEP695BadDunderNew[T]: + def __new__[S](cls: type[S], *args: Any, ** kwargs: Any) -> S: ... # Ok + + def generic_instance_method[S](self: S) -> S: ... # Ok + +class PEP695GoodDunderNew[T]: + def __new__(cls, *args: Any, **kwargs: Any) -> Self: ... diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.pyi b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.pyi new file mode 100644 index 0000000000000..703ccdfc483ef --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.pyi @@ -0,0 +1,31 @@ +from typing import TypeVar, Self + +_S = TypeVar("_S", bound=BadClass) +_S2 = TypeVar("_S2", BadClass, GoodClass) + +class BadClass: + def __new__(cls: type[_S], *args: str, **kwargs: int) -> _S: ... # PYI019 + def bad_instance_method(self: _S, arg: bytes) -> _S: ... # PYI019 + @classmethod + def bad_class_method(cls: type[_S], arg: int) -> _S: ... # PYI019 + +class GoodClass: + def __new__(cls: type[Self], *args: list[int], **kwargs: set[str]) -> Self: ... + def good_instance_method_1(self: Self, arg: bytes) -> Self: ... + def good_instance_method_2(self, arg1: _S2, arg2: _S2) -> _S2: ... + @classmethod + def good_cls_method_1(cls: type[Self], arg: int) -> Self: ... + @classmethod + def good_cls_method_2(cls, arg1: _S, arg2: _S) -> _S: ... + @staticmethod + def static_method(arg1: _S) -> _S: ... + + +# Python > 3.12 +class PEP695BadDunderNew[T]: + def __new__[S](cls: type[S], *args: Any, ** kwargs: Any) -> S: ... # PYI019 + + def generic_instance_method[S](self: S) -> S: ... # PYI019 + +class PEP695GoodDunderNew[T]: + def __new__(cls, *args: Any, **kwargs: Any) -> Self: ... diff --git a/crates/ruff/src/checkers/ast/analyze/statement.rs b/crates/ruff/src/checkers/ast/analyze/statement.rs index f294ac6c0eecb..b8dec7b8638b7 100644 --- a/crates/ruff/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff/src/checkers/ast/analyze/statement.rs @@ -155,6 +155,17 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { stmt.is_async_function_def_stmt(), ); } + if checker.enabled(Rule::CustomTypevarReturnType) { + flake8_pyi::rules::custom_typevar_return_type( + checker, + stmt, + name, + decorator_list, + returns.as_ref().map(AsRef::as_ref), + args, + stmt.is_async_function_def_stmt(), + ) + } if checker.enabled(Rule::StrOrReprDefinedInStub) { flake8_pyi::rules::str_or_repr_defined_in_stub(checker, stmt); } diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 9018dcc946980..52541138f7d7c 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -636,6 +636,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Pyi, "016") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::DuplicateUnionMember), (Flake8Pyi, "017") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::ComplexAssignmentInStub), (Flake8Pyi, "018") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::UnusedPrivateTypeVar), + (Flake8Pyi, "019") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::CustomTypevarReturnType), (Flake8Pyi, "020") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::QuotedAnnotationInStub), (Flake8Pyi, "021") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::DocstringInStub), (Flake8Pyi, "024") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::CollectionsNamedTuple), diff --git a/crates/ruff/src/rules/flake8_pyi/mod.rs b/crates/ruff/src/rules/flake8_pyi/mod.rs index 6c363fdc6b397..b796420169e05 100644 --- a/crates/ruff/src/rules/flake8_pyi/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/mod.rs @@ -29,6 +29,8 @@ mod tests { #[test_case(Rule::ComplexAssignmentInStub, Path::new("PYI017.pyi"))] #[test_case(Rule::ComplexIfStatementInStub, Path::new("PYI002.py"))] #[test_case(Rule::ComplexIfStatementInStub, Path::new("PYI002.pyi"))] + #[test_case(Rule::CustomTypevarReturnType, Path::new("PYI019.py"))] + #[test_case(Rule::CustomTypevarReturnType, Path::new("PYI019.pyi"))] #[test_case(Rule::DocstringInStub, Path::new("PYI021.py"))] #[test_case(Rule::DocstringInStub, Path::new("PYI021.pyi"))] #[test_case(Rule::DuplicateUnionMember, Path::new("PYI016.py"))] diff --git a/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs b/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs new file mode 100644 index 0000000000000..370d172892349 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs @@ -0,0 +1,81 @@ +use crate::checkers::ast::Checker; +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::identifier::Identifier; +use ruff_python_ast::{Arguments, Decorator, Expr, Stmt}; + +// TODO: Check docs for accuracy +/// ## What it does +/// Checks if certain methods define a custom `TypeVar`s for their return annotation instead of +/// using `typing_extensions.Self`. This check currently applies for instance methods that return +/// `self`, class methods that return an instance of `cls`, and `__new__` methods. +/// +/// ## Why is this bad? +/// If certain methods are annotated with a custom `TypeVar` type, and the class is subclassed, +/// type checkers will not be able to infer the correct return type. +/// +/// ## Example +/// ```python +/// class Foo: +/// def __new__(cls: type[_S], *args: str, **kwargs: int) -> _S: +/// ... +/// +/// def foo(self: _S, arg: bytes) -> _S: +/// ... +/// +/// @classmethod +/// def bar(cls: type[_S], arg: int) -> _S: +/// ... +/// ``` +/// +/// Use instead: +/// ```python +/// from typing import Self +/// +/// +/// class Foo: +/// def __new__(cls: type[Self], *args: str, **kwargs: int) -> Self: +/// ... +/// +/// def foo(self: Self, arg: bytes) -> Self: +/// ... +/// +/// @classmethod +/// def bar(cls: type[Self], arg: int) -> Self: +/// ... +/// ``` +#[violation] +pub struct CustomTypevarReturnType { + method_name: String, + typevar_name: String, +} + +impl Violation for CustomTypevarReturnType { + #[derive_message_formats] + fn message(&self) -> String { + let CustomTypevarReturnType { + method_name, + typevar_name, + } = self; + format!("Methods like {method_name} should return `typing.Self` instead of custom typevar {typevar_name}") + } +} + +/// PYI019 +pub(crate) fn custom_typevar_return_type( + checker: &mut Checker, + stmt: &Stmt, + name: &str, + decorator_list: &[Decorator], + returns: Option<&Expr>, + args: &Arguments, + async_: bool, +) { + checker.diagnostics.push(Diagnostic::new( + CustomTypevarReturnType { + method_name: name.to_string(), + typevar_name: name.to_string(), + }, + stmt.identifier(), + )); +} diff --git a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs index 6ee3f89e8a74e..93697d2f79684 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs @@ -3,6 +3,7 @@ pub(crate) use bad_version_info_comparison::*; pub(crate) use collections_named_tuple::*; pub(crate) use complex_assignment_in_stub::*; pub(crate) use complex_if_statement_in_stub::*; +pub(crate) use custom_typevar_return::*; pub(crate) use docstring_in_stubs::*; pub(crate) use duplicate_union_member::*; pub(crate) use ellipsis_in_non_empty_class_body::*; @@ -36,6 +37,7 @@ mod bad_version_info_comparison; mod collections_named_tuple; mod complex_assignment_in_stub; mod complex_if_statement_in_stub; +mod custom_typevar_return; mod docstring_in_stubs; mod duplicate_union_member; mod ellipsis_in_non_empty_class_body; From e0b15c2ecf62d48370bd04e2ea959a1c9973cb94 Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Mon, 31 Jul 2023 17:07:20 +0200 Subject: [PATCH 2/8] Add first port of Y019 logic --- .../src/checkers/ast/analyze/statement.rs | 6 +- crates/ruff/src/codes.rs | 2 +- crates/ruff/src/rules/flake8_pyi/mod.rs | 4 +- .../flake8_pyi/rules/custom_typevar_return.rs | 189 ++++++++++++++++-- ...__flake8_pyi__tests__PYI019_PYI019.py.snap | 4 + 5 files changed, 179 insertions(+), 26 deletions(-) create mode 100644 crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI019_PYI019.py.snap diff --git a/crates/ruff/src/checkers/ast/analyze/statement.rs b/crates/ruff/src/checkers/ast/analyze/statement.rs index b8dec7b8638b7..2cacb27f070f0 100644 --- a/crates/ruff/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff/src/checkers/ast/analyze/statement.rs @@ -155,16 +155,14 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { stmt.is_async_function_def_stmt(), ); } - if checker.enabled(Rule::CustomTypevarReturnType) { + if checker.enabled(Rule::CustomTypeVarReturnType) { flake8_pyi::rules::custom_typevar_return_type( checker, - stmt, name, decorator_list, returns.as_ref().map(AsRef::as_ref), args, - stmt.is_async_function_def_stmt(), - ) + ); } if checker.enabled(Rule::StrOrReprDefinedInStub) { flake8_pyi::rules::str_or_repr_defined_in_stub(checker, stmt); diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 52541138f7d7c..c7c28e774f90f 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -636,7 +636,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Pyi, "016") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::DuplicateUnionMember), (Flake8Pyi, "017") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::ComplexAssignmentInStub), (Flake8Pyi, "018") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::UnusedPrivateTypeVar), - (Flake8Pyi, "019") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::CustomTypevarReturnType), + (Flake8Pyi, "019") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::CustomTypeVarReturnType), (Flake8Pyi, "020") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::QuotedAnnotationInStub), (Flake8Pyi, "021") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::DocstringInStub), (Flake8Pyi, "024") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::CollectionsNamedTuple), diff --git a/crates/ruff/src/rules/flake8_pyi/mod.rs b/crates/ruff/src/rules/flake8_pyi/mod.rs index b796420169e05..16dc7885d952d 100644 --- a/crates/ruff/src/rules/flake8_pyi/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/mod.rs @@ -29,8 +29,8 @@ mod tests { #[test_case(Rule::ComplexAssignmentInStub, Path::new("PYI017.pyi"))] #[test_case(Rule::ComplexIfStatementInStub, Path::new("PYI002.py"))] #[test_case(Rule::ComplexIfStatementInStub, Path::new("PYI002.pyi"))] - #[test_case(Rule::CustomTypevarReturnType, Path::new("PYI019.py"))] - #[test_case(Rule::CustomTypevarReturnType, Path::new("PYI019.pyi"))] + #[test_case(Rule::CustomTypeVarReturnType, Path::new("PYI019.py"))] + #[test_case(Rule::CustomTypeVarReturnType, Path::new("PYI019.pyi"))] #[test_case(Rule::DocstringInStub, Path::new("PYI021.py"))] #[test_case(Rule::DocstringInStub, Path::new("PYI021.pyi"))] #[test_case(Rule::DuplicateUnionMember, Path::new("PYI016.py"))] diff --git a/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs b/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs index 370d172892349..bf6f586f3ce59 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs @@ -1,8 +1,13 @@ use crate::checkers::ast::Checker; +use crate::settings::types::PythonVersion; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::identifier::Identifier; -use ruff_python_ast::{Arguments, Decorator, Expr, Stmt}; +use ruff_python_ast as ast; +use ruff_python_ast::{ArgWithDefault, Arguments, Decorator, Expr, Ranged, Stmt}; +use ruff_python_semantic::analyze::visibility::{ + is_abstract, is_classmethod, is_overload, is_staticmethod, +}; +use ruff_python_semantic::ScopeKind; // TODO: Check docs for accuracy /// ## What it does @@ -45,37 +50,183 @@ use ruff_python_ast::{Arguments, Decorator, Expr, Stmt}; /// ... /// ``` #[violation] -pub struct CustomTypevarReturnType { +pub struct CustomTypeVarReturnType { method_name: String, - typevar_name: String, } -impl Violation for CustomTypevarReturnType { +impl Violation for CustomTypeVarReturnType { #[derive_message_formats] fn message(&self) -> String { - let CustomTypevarReturnType { - method_name, - typevar_name, - } = self; - format!("Methods like {method_name} should return `typing.Self` instead of custom typevar {typevar_name}") + let CustomTypeVarReturnType { method_name } = self; + format!( + "Methods like {method_name} should return `typing.Self` instead of a custom TypeVar" + ) } } /// PYI019 pub(crate) fn custom_typevar_return_type( checker: &mut Checker, - stmt: &Stmt, name: &str, decorator_list: &[Decorator], returns: Option<&Expr>, args: &Arguments, - async_: bool, ) { - checker.diagnostics.push(Diagnostic::new( - CustomTypevarReturnType { - method_name: name.to_string(), - typevar_name: name.to_string(), - }, - stmt.identifier(), - )); + let ScopeKind::Class(_) = checker.semantic().scope().kind else { + return; + }; + + if args.args.is_empty() && args.posonlyargs.is_empty() { + return; + } + + let Some(returns) = returns else { + return; + }; + + let return_annotation = if let Expr::Subscript(ast::ExprSubscript { value, .. }) = returns { + // Ex) `Type[T]` + value + } else { + // Ex) `Type`, `typing.Type` + returns + }; + + // Skip any abstract, static and overloaded methods. + if is_abstract(decorator_list, checker.semantic()) + || is_overload(decorator_list, checker.semantic()) + || is_staticmethod(decorator_list, checker.semantic()) + { + return; + } + + let is_violation: bool = + if is_classmethod(decorator_list, checker.semantic()) || name == "__new__" { + check_class_method_for_bad_typevars(checker, args, return_annotation) + } else { + // If not static, or a class method or __new__ we know it is an instance method + check_instance_method_for_bad_typevars(checker, args, return_annotation) + }; + + if is_violation { + checker.diagnostics.push(Diagnostic::new( + CustomTypeVarReturnType { + method_name: name.to_string(), + }, + return_annotation.range(), + )); + } +} + +fn check_class_method_for_bad_typevars( + checker: &Checker, + args: &Arguments, + return_annotation: &Expr, +) -> bool { + let ArgWithDefault { def, .. } = &args.args[0]; + + let Some(annotation) = &def.annotation else { + return false + }; + + let Expr::Subscript(ast::ExprSubscript{slice, value, ..}) = annotation.as_ref() else { + return false + }; + + // Don't error if the first argument is annotated with `builtins.type[T]` or `typing.Type[T]` + // These are edge cases, and it's hard to give good error messages for them. + if let Expr::Name(ast::ExprName { id: id_value, .. }) = value.as_ref() { + return id_value == "type"; + } + + let Expr::Name(ast::ExprName { id: id_slice, .. }) = slice.as_ref() else { + return false + }; + + let Expr::Name(ast::ExprName { id: return_type, .. }) = return_annotation else { + return false + }; + + return_type == id_slice && is_likely_private_typevar(checker, args, id_slice) +} + +fn check_instance_method_for_bad_typevars( + checker: &Checker, + args: &Arguments, + return_annotation: &Expr, +) -> bool { + let ArgWithDefault { def, .. } = &args.args[0]; + + let Some(annotation) = &def.annotation else { + return false + }; + + let Expr::Name(ast::ExprName{id: first_arg_type,..}) = annotation.as_ref() else { + return false + }; + + let Expr::Name(ast::ExprName { id: return_type, .. }) = return_annotation else { + return false + }; + + if first_arg_type != return_type { + return false; + } + + is_likely_private_typevar(checker, args, first_arg_type) +} + +fn is_likely_private_typevar(checker: &Checker, args: &Arguments, tvar_name: &str) -> bool { + if tvar_name.starts_with('_') { + return true; + } + if checker.settings.target_version < PythonVersion::Py312 { + return false; + } + for ArgWithDefault { def, .. } in &args.args { + if def.arg.to_string() != tvar_name { + continue; + } + + let Some(annotation) = &def.annotation else { + continue + }; + + let Expr::Name(ast::ExprName{id,..}) = annotation.as_ref() else { + continue + }; + + // Check if the binding used in an annotation is an assignment to typing.TypeVar + let scope = checker.semantic().scope(); + let Some(binding_id) = scope.get(id) else { + continue + }; + + let binding = checker.semantic().binding(binding_id); + if binding.kind.is_assignment() || binding.kind.is_named_expr_assignment() { + if let Some(parent_id) = binding.source { + let parent = checker.semantic().stmts[parent_id]; + if let Stmt::Assign(ast::StmtAssign { value, .. }) + | Stmt::AnnAssign(ast::StmtAnnAssign { + value: Some(value), .. + }) + | Stmt::AugAssign(ast::StmtAugAssign { value, .. }) = parent + { + let Expr::Call(ast::ExprCall{func, ..}) = value.as_ref() else { + continue + }; + let Some(call_path) = checker.semantic().resolve_call_path(func) else { + continue + }; + if checker + .semantic() + .match_typing_call_path(&call_path, "TypeVar") + { + return true; + } + } + } + } + } + false } diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI019_PYI019.py.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI019_PYI019.py.snap new file mode 100644 index 0000000000000..d1aa2e9116558 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI019_PYI019.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +--- + From bcb0c8d00c5cc2f82dc0294f5eb381b287cdb5c9 Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Mon, 31 Jul 2023 17:16:15 +0200 Subject: [PATCH 3/8] Change to Python 3.11 and lower implementation --- .../test/fixtures/flake8_pyi/PYI019.py | 10 --- .../test/fixtures/flake8_pyi/PYI019.pyi | 10 --- .../flake8_pyi/rules/custom_typevar_return.rs | 73 +++---------------- ..._flake8_pyi__tests__PYI019_PYI019.pyi.snap | 33 +++++++++ 4 files changed, 42 insertions(+), 84 deletions(-) create mode 100644 crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI019_PYI019.pyi.snap diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.py b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.py index f85694bff14c1..81f9eb5374db3 100644 --- a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.py +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.py @@ -19,13 +19,3 @@ def good_cls_method_1(cls: type[Self], arg: int) -> Self: ... def good_cls_method_2(cls, arg1: _S, arg2: _S) -> _S: ... @staticmethod def static_method(arg1: _S) -> _S: ... - - -# Python > 3.12 -class PEP695BadDunderNew[T]: - def __new__[S](cls: type[S], *args: Any, ** kwargs: Any) -> S: ... # Ok - - def generic_instance_method[S](self: S) -> S: ... # Ok - -class PEP695GoodDunderNew[T]: - def __new__(cls, *args: Any, **kwargs: Any) -> Self: ... diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.pyi b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.pyi index 703ccdfc483ef..98f314342776e 100644 --- a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.pyi +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.pyi @@ -19,13 +19,3 @@ class GoodClass: def good_cls_method_2(cls, arg1: _S, arg2: _S) -> _S: ... @staticmethod def static_method(arg1: _S) -> _S: ... - - -# Python > 3.12 -class PEP695BadDunderNew[T]: - def __new__[S](cls: type[S], *args: Any, ** kwargs: Any) -> S: ... # PYI019 - - def generic_instance_method[S](self: S) -> S: ... # PYI019 - -class PEP695GoodDunderNew[T]: - def __new__(cls, *args: Any, **kwargs: Any) -> Self: ... diff --git a/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs b/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs index bf6f586f3ce59..15b027f2bf185 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs @@ -1,15 +1,13 @@ use crate::checkers::ast::Checker; -use crate::settings::types::PythonVersion; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast as ast; -use ruff_python_ast::{ArgWithDefault, Arguments, Decorator, Expr, Ranged, Stmt}; +use ruff_python_ast::{ArgWithDefault, Arguments, Decorator, Expr, Ranged}; use ruff_python_semantic::analyze::visibility::{ is_abstract, is_classmethod, is_overload, is_staticmethod, }; use ruff_python_semantic::ScopeKind; -// TODO: Check docs for accuracy /// ## What it does /// Checks if certain methods define a custom `TypeVar`s for their return annotation instead of /// using `typing_extensions.Self`. This check currently applies for instance methods that return @@ -102,12 +100,15 @@ pub(crate) fn custom_typevar_return_type( let is_violation: bool = if is_classmethod(decorator_list, checker.semantic()) || name == "__new__" { - check_class_method_for_bad_typevars(checker, args, return_annotation) + println!("Class: {}", name.to_string()); + check_class_method_for_bad_typevars(args, return_annotation) } else { // If not static, or a class method or __new__ we know it is an instance method - check_instance_method_for_bad_typevars(checker, args, return_annotation) + println!("Instance: {}", name.to_string()); + check_instance_method_for_bad_typevars(args, return_annotation) }; + println!("{:?}", is_violation); if is_violation { checker.diagnostics.push(Diagnostic::new( CustomTypeVarReturnType { @@ -119,7 +120,6 @@ pub(crate) fn custom_typevar_return_type( } fn check_class_method_for_bad_typevars( - checker: &Checker, args: &Arguments, return_annotation: &Expr, ) -> bool { @@ -133,12 +133,6 @@ fn check_class_method_for_bad_typevars( return false }; - // Don't error if the first argument is annotated with `builtins.type[T]` or `typing.Type[T]` - // These are edge cases, and it's hard to give good error messages for them. - if let Expr::Name(ast::ExprName { id: id_value, .. }) = value.as_ref() { - return id_value == "type"; - } - let Expr::Name(ast::ExprName { id: id_slice, .. }) = slice.as_ref() else { return false }; @@ -147,11 +141,10 @@ fn check_class_method_for_bad_typevars( return false }; - return_type == id_slice && is_likely_private_typevar(checker, args, id_slice) + return_type == id_slice && is_likely_private_typevar(id_slice) } fn check_instance_method_for_bad_typevars( - checker: &Checker, args: &Arguments, return_annotation: &Expr, ) -> bool { @@ -173,60 +166,12 @@ fn check_instance_method_for_bad_typevars( return false; } - is_likely_private_typevar(checker, args, first_arg_type) + is_likely_private_typevar(first_arg_type) } -fn is_likely_private_typevar(checker: &Checker, args: &Arguments, tvar_name: &str) -> bool { +fn is_likely_private_typevar(tvar_name: &str) -> bool { if tvar_name.starts_with('_') { return true; } - if checker.settings.target_version < PythonVersion::Py312 { - return false; - } - for ArgWithDefault { def, .. } in &args.args { - if def.arg.to_string() != tvar_name { - continue; - } - - let Some(annotation) = &def.annotation else { - continue - }; - - let Expr::Name(ast::ExprName{id,..}) = annotation.as_ref() else { - continue - }; - - // Check if the binding used in an annotation is an assignment to typing.TypeVar - let scope = checker.semantic().scope(); - let Some(binding_id) = scope.get(id) else { - continue - }; - - let binding = checker.semantic().binding(binding_id); - if binding.kind.is_assignment() || binding.kind.is_named_expr_assignment() { - if let Some(parent_id) = binding.source { - let parent = checker.semantic().stmts[parent_id]; - if let Stmt::Assign(ast::StmtAssign { value, .. }) - | Stmt::AnnAssign(ast::StmtAnnAssign { - value: Some(value), .. - }) - | Stmt::AugAssign(ast::StmtAugAssign { value, .. }) = parent - { - let Expr::Call(ast::ExprCall{func, ..}) = value.as_ref() else { - continue - }; - let Some(call_path) = checker.semantic().resolve_call_path(func) else { - continue - }; - if checker - .semantic() - .match_typing_call_path(&call_path, "TypeVar") - { - return true; - } - } - } - } - } false } diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI019_PYI019.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI019_PYI019.pyi.snap new file mode 100644 index 0000000000000..fb48547c05e3f --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI019_PYI019.pyi.snap @@ -0,0 +1,33 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +--- +PYI019.pyi:7:62: PYI019 Methods like __new__ should return `typing.Self` instead of a custom TypeVar + | +6 | class BadClass: +7 | def __new__(cls: type[_S], *args: str, **kwargs: int) -> _S: ... # PYI019 + | ^^ PYI019 +8 | def bad_instance_method(self: _S, arg: bytes) -> _S: ... # PYI019 +9 | @classmethod + | + +PYI019.pyi:8:54: PYI019 Methods like bad_instance_method should return `typing.Self` instead of a custom TypeVar + | + 6 | class BadClass: + 7 | def __new__(cls: type[_S], *args: str, **kwargs: int) -> _S: ... # PYI019 + 8 | def bad_instance_method(self: _S, arg: bytes) -> _S: ... # PYI019 + | ^^ PYI019 + 9 | @classmethod +10 | def bad_class_method(cls: type[_S], arg: int) -> _S: ... # PYI019 + | + +PYI019.pyi:10:54: PYI019 Methods like bad_class_method should return `typing.Self` instead of a custom TypeVar + | + 8 | def bad_instance_method(self: _S, arg: bytes) -> _S: ... # PYI019 + 9 | @classmethod +10 | def bad_class_method(cls: type[_S], arg: int) -> _S: ... # PYI019 + | ^^ PYI019 +11 | +12 | class GoodClass: + | + + From 3c7ad6c7ddad87bb2975e3ed5c06af09186c535b Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Mon, 31 Jul 2023 17:16:15 +0200 Subject: [PATCH 4/8] Revert "Change to Python 3.11 and lower implementation" This reverts commit bcb0c8d00c5cc2f82dc0294f5eb381b287cdb5c9. --- .../test/fixtures/flake8_pyi/PYI019.py | 10 +++ .../test/fixtures/flake8_pyi/PYI019.pyi | 10 +++ .../flake8_pyi/rules/custom_typevar_return.rs | 73 ++++++++++++++++--- ..._flake8_pyi__tests__PYI019_PYI019.pyi.snap | 33 --------- 4 files changed, 84 insertions(+), 42 deletions(-) delete mode 100644 crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI019_PYI019.pyi.snap diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.py b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.py index 81f9eb5374db3..f85694bff14c1 100644 --- a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.py +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.py @@ -19,3 +19,13 @@ def good_cls_method_1(cls: type[Self], arg: int) -> Self: ... def good_cls_method_2(cls, arg1: _S, arg2: _S) -> _S: ... @staticmethod def static_method(arg1: _S) -> _S: ... + + +# Python > 3.12 +class PEP695BadDunderNew[T]: + def __new__[S](cls: type[S], *args: Any, ** kwargs: Any) -> S: ... # Ok + + def generic_instance_method[S](self: S) -> S: ... # Ok + +class PEP695GoodDunderNew[T]: + def __new__(cls, *args: Any, **kwargs: Any) -> Self: ... diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.pyi b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.pyi index 98f314342776e..703ccdfc483ef 100644 --- a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.pyi +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.pyi @@ -19,3 +19,13 @@ class GoodClass: def good_cls_method_2(cls, arg1: _S, arg2: _S) -> _S: ... @staticmethod def static_method(arg1: _S) -> _S: ... + + +# Python > 3.12 +class PEP695BadDunderNew[T]: + def __new__[S](cls: type[S], *args: Any, ** kwargs: Any) -> S: ... # PYI019 + + def generic_instance_method[S](self: S) -> S: ... # PYI019 + +class PEP695GoodDunderNew[T]: + def __new__(cls, *args: Any, **kwargs: Any) -> Self: ... diff --git a/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs b/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs index 15b027f2bf185..bf6f586f3ce59 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs @@ -1,13 +1,15 @@ use crate::checkers::ast::Checker; +use crate::settings::types::PythonVersion; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast as ast; -use ruff_python_ast::{ArgWithDefault, Arguments, Decorator, Expr, Ranged}; +use ruff_python_ast::{ArgWithDefault, Arguments, Decorator, Expr, Ranged, Stmt}; use ruff_python_semantic::analyze::visibility::{ is_abstract, is_classmethod, is_overload, is_staticmethod, }; use ruff_python_semantic::ScopeKind; +// TODO: Check docs for accuracy /// ## What it does /// Checks if certain methods define a custom `TypeVar`s for their return annotation instead of /// using `typing_extensions.Self`. This check currently applies for instance methods that return @@ -100,15 +102,12 @@ pub(crate) fn custom_typevar_return_type( let is_violation: bool = if is_classmethod(decorator_list, checker.semantic()) || name == "__new__" { - println!("Class: {}", name.to_string()); - check_class_method_for_bad_typevars(args, return_annotation) + check_class_method_for_bad_typevars(checker, args, return_annotation) } else { // If not static, or a class method or __new__ we know it is an instance method - println!("Instance: {}", name.to_string()); - check_instance_method_for_bad_typevars(args, return_annotation) + check_instance_method_for_bad_typevars(checker, args, return_annotation) }; - println!("{:?}", is_violation); if is_violation { checker.diagnostics.push(Diagnostic::new( CustomTypeVarReturnType { @@ -120,6 +119,7 @@ pub(crate) fn custom_typevar_return_type( } fn check_class_method_for_bad_typevars( + checker: &Checker, args: &Arguments, return_annotation: &Expr, ) -> bool { @@ -133,6 +133,12 @@ fn check_class_method_for_bad_typevars( return false }; + // Don't error if the first argument is annotated with `builtins.type[T]` or `typing.Type[T]` + // These are edge cases, and it's hard to give good error messages for them. + if let Expr::Name(ast::ExprName { id: id_value, .. }) = value.as_ref() { + return id_value == "type"; + } + let Expr::Name(ast::ExprName { id: id_slice, .. }) = slice.as_ref() else { return false }; @@ -141,10 +147,11 @@ fn check_class_method_for_bad_typevars( return false }; - return_type == id_slice && is_likely_private_typevar(id_slice) + return_type == id_slice && is_likely_private_typevar(checker, args, id_slice) } fn check_instance_method_for_bad_typevars( + checker: &Checker, args: &Arguments, return_annotation: &Expr, ) -> bool { @@ -166,12 +173,60 @@ fn check_instance_method_for_bad_typevars( return false; } - is_likely_private_typevar(first_arg_type) + is_likely_private_typevar(checker, args, first_arg_type) } -fn is_likely_private_typevar(tvar_name: &str) -> bool { +fn is_likely_private_typevar(checker: &Checker, args: &Arguments, tvar_name: &str) -> bool { if tvar_name.starts_with('_') { return true; } + if checker.settings.target_version < PythonVersion::Py312 { + return false; + } + for ArgWithDefault { def, .. } in &args.args { + if def.arg.to_string() != tvar_name { + continue; + } + + let Some(annotation) = &def.annotation else { + continue + }; + + let Expr::Name(ast::ExprName{id,..}) = annotation.as_ref() else { + continue + }; + + // Check if the binding used in an annotation is an assignment to typing.TypeVar + let scope = checker.semantic().scope(); + let Some(binding_id) = scope.get(id) else { + continue + }; + + let binding = checker.semantic().binding(binding_id); + if binding.kind.is_assignment() || binding.kind.is_named_expr_assignment() { + if let Some(parent_id) = binding.source { + let parent = checker.semantic().stmts[parent_id]; + if let Stmt::Assign(ast::StmtAssign { value, .. }) + | Stmt::AnnAssign(ast::StmtAnnAssign { + value: Some(value), .. + }) + | Stmt::AugAssign(ast::StmtAugAssign { value, .. }) = parent + { + let Expr::Call(ast::ExprCall{func, ..}) = value.as_ref() else { + continue + }; + let Some(call_path) = checker.semantic().resolve_call_path(func) else { + continue + }; + if checker + .semantic() + .match_typing_call_path(&call_path, "TypeVar") + { + return true; + } + } + } + } + } false } diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI019_PYI019.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI019_PYI019.pyi.snap deleted file mode 100644 index fb48547c05e3f..0000000000000 --- a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI019_PYI019.pyi.snap +++ /dev/null @@ -1,33 +0,0 @@ ---- -source: crates/ruff/src/rules/flake8_pyi/mod.rs ---- -PYI019.pyi:7:62: PYI019 Methods like __new__ should return `typing.Self` instead of a custom TypeVar - | -6 | class BadClass: -7 | def __new__(cls: type[_S], *args: str, **kwargs: int) -> _S: ... # PYI019 - | ^^ PYI019 -8 | def bad_instance_method(self: _S, arg: bytes) -> _S: ... # PYI019 -9 | @classmethod - | - -PYI019.pyi:8:54: PYI019 Methods like bad_instance_method should return `typing.Self` instead of a custom TypeVar - | - 6 | class BadClass: - 7 | def __new__(cls: type[_S], *args: str, **kwargs: int) -> _S: ... # PYI019 - 8 | def bad_instance_method(self: _S, arg: bytes) -> _S: ... # PYI019 - | ^^ PYI019 - 9 | @classmethod -10 | def bad_class_method(cls: type[_S], arg: int) -> _S: ... # PYI019 - | - -PYI019.pyi:10:54: PYI019 Methods like bad_class_method should return `typing.Self` instead of a custom TypeVar - | - 8 | def bad_instance_method(self: _S, arg: bytes) -> _S: ... # PYI019 - 9 | @classmethod -10 | def bad_class_method(cls: type[_S], arg: int) -> _S: ... # PYI019 - | ^^ PYI019 -11 | -12 | class GoodClass: - | - - From b0765b659cee7f7bb8245adf5ef2c060001ceaef Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Mon, 31 Jul 2023 19:04:07 +0200 Subject: [PATCH 5/8] Fix PEP-695 heuristic --- .../test/fixtures/flake8_pyi/PYI019.py | 13 ++- .../test/fixtures/flake8_pyi/PYI019.pyi | 13 ++- .../src/checkers/ast/analyze/statement.rs | 3 + .../flake8_pyi/rules/custom_typevar_return.rs | 79 ++++++------------- ..._flake8_pyi__tests__PYI019_PYI019.pyi.snap | 38 +++++++++ ruff.schema.json | 1 + 6 files changed, 91 insertions(+), 56 deletions(-) create mode 100644 crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI019_PYI019.pyi.snap diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.py b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.py index f85694bff14c1..72bc86ac036f6 100644 --- a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.py +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.py @@ -1,14 +1,23 @@ -from typing import TypeVar, Self +from typing import TypeVar, Self, Type _S = TypeVar("_S", bound=BadClass) _S2 = TypeVar("_S2", BadClass, GoodClass) class BadClass: def __new__(cls: type[_S], *args: str, **kwargs: int) -> _S: ... # Ok + + def bad_instance_method(self: _S, arg: bytes) -> _S: ... # Ok + + @classmethod def bad_class_method(cls: type[_S], arg: int) -> _S: ... # Ok + + @classmethod + def excluded_edge_case(cls: Type[_S], arg: int) -> _S: ... # Ok + + class GoodClass: def __new__(cls: type[Self], *args: list[int], **kwargs: set[str]) -> Self: ... def good_instance_method_1(self: Self, arg: bytes) -> Self: ... @@ -25,7 +34,9 @@ def static_method(arg1: _S) -> _S: ... class PEP695BadDunderNew[T]: def __new__[S](cls: type[S], *args: Any, ** kwargs: Any) -> S: ... # Ok + def generic_instance_method[S](self: S) -> S: ... # Ok + class PEP695GoodDunderNew[T]: def __new__(cls, *args: Any, **kwargs: Any) -> Self: ... diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.pyi b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.pyi index 703ccdfc483ef..37dd8dfb574c3 100644 --- a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.pyi +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI019.pyi @@ -1,14 +1,23 @@ -from typing import TypeVar, Self +from typing import TypeVar, Self, Type _S = TypeVar("_S", bound=BadClass) _S2 = TypeVar("_S2", BadClass, GoodClass) class BadClass: def __new__(cls: type[_S], *args: str, **kwargs: int) -> _S: ... # PYI019 + + def bad_instance_method(self: _S, arg: bytes) -> _S: ... # PYI019 + + @classmethod def bad_class_method(cls: type[_S], arg: int) -> _S: ... # PYI019 + + @classmethod + def excluded_edge_case(cls: Type[_S], arg: int) -> _S: ... # Ok + + class GoodClass: def __new__(cls: type[Self], *args: list[int], **kwargs: set[str]) -> Self: ... def good_instance_method_1(self: Self, arg: bytes) -> Self: ... @@ -25,7 +34,9 @@ class GoodClass: class PEP695BadDunderNew[T]: def __new__[S](cls: type[S], *args: Any, ** kwargs: Any) -> S: ... # PYI019 + def generic_instance_method[S](self: S) -> S: ... # PYI019 + class PEP695GoodDunderNew[T]: def __new__(cls, *args: Any, **kwargs: Any) -> Self: ... diff --git a/crates/ruff/src/checkers/ast/analyze/statement.rs b/crates/ruff/src/checkers/ast/analyze/statement.rs index 2cacb27f070f0..ef3673e06a8e1 100644 --- a/crates/ruff/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff/src/checkers/ast/analyze/statement.rs @@ -75,6 +75,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { returns, args, body, + type_params, .. }) | Stmt::AsyncFunctionDef(ast::StmtAsyncFunctionDef { @@ -83,6 +84,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { returns, args, body, + type_params, .. }) => { if checker.enabled(Rule::DjangoNonLeadingReceiverDecorator) { @@ -162,6 +164,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { decorator_list, returns.as_ref().map(AsRef::as_ref), args, + type_params, ); } if checker.enabled(Rule::StrOrReprDefinedInStub) { diff --git a/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs b/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs index bf6f586f3ce59..7333a3bdb95b8 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs @@ -3,13 +3,12 @@ use crate::settings::types::PythonVersion; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast as ast; -use ruff_python_ast::{ArgWithDefault, Arguments, Decorator, Expr, Ranged, Stmt}; +use ruff_python_ast::{ArgWithDefault, Arguments, Decorator, Expr, Ranged, TypeParam}; use ruff_python_semantic::analyze::visibility::{ is_abstract, is_classmethod, is_overload, is_staticmethod, }; use ruff_python_semantic::ScopeKind; -// TODO: Check docs for accuracy /// ## What it does /// Checks if certain methods define a custom `TypeVar`s for their return annotation instead of /// using `typing_extensions.Self`. This check currently applies for instance methods that return @@ -71,6 +70,7 @@ pub(crate) fn custom_typevar_return_type( decorator_list: &[Decorator], returns: Option<&Expr>, args: &Arguments, + type_params: &[TypeParam], ) { let ScopeKind::Class(_) = checker.semantic().scope().kind else { return; @@ -102,10 +102,10 @@ pub(crate) fn custom_typevar_return_type( let is_violation: bool = if is_classmethod(decorator_list, checker.semantic()) || name == "__new__" { - check_class_method_for_bad_typevars(checker, args, return_annotation) + check_class_method_for_bad_typevars(checker, args, return_annotation, type_params) } else { // If not static, or a class method or __new__ we know it is an instance method - check_instance_method_for_bad_typevars(checker, args, return_annotation) + check_instance_method_for_bad_typevars(checker, args, return_annotation, type_params) }; if is_violation { @@ -122,6 +122,7 @@ fn check_class_method_for_bad_typevars( checker: &Checker, args: &Arguments, return_annotation: &Expr, + type_params: &[TypeParam], ) -> bool { let ArgWithDefault { def, .. } = &args.args[0]; @@ -133,11 +134,15 @@ fn check_class_method_for_bad_typevars( return false }; - // Don't error if the first argument is annotated with `builtins.type[T]` or `typing.Type[T]` + let Expr::Name(ast::ExprName{ id: id_value, .. }) = value.as_ref() else { + return false + }; + + // Don't error if the first argument is annotated with typing.Type[T] // These are edge cases, and it's hard to give good error messages for them. - if let Expr::Name(ast::ExprName { id: id_value, .. }) = value.as_ref() { - return id_value == "type"; - } + if id_value != "type" { + return false; + }; let Expr::Name(ast::ExprName { id: id_slice, .. }) = slice.as_ref() else { return false @@ -147,13 +152,14 @@ fn check_class_method_for_bad_typevars( return false }; - return_type == id_slice && is_likely_private_typevar(checker, args, id_slice) + return_type == id_slice && is_likely_private_typevar(checker, id_slice, type_params) } fn check_instance_method_for_bad_typevars( checker: &Checker, args: &Arguments, return_annotation: &Expr, + type_params: &[TypeParam], ) -> bool { let ArgWithDefault { def, .. } = &args.args[0]; @@ -173,59 +179,24 @@ fn check_instance_method_for_bad_typevars( return false; } - is_likely_private_typevar(checker, args, first_arg_type) + is_likely_private_typevar(checker, first_arg_type, type_params) } -fn is_likely_private_typevar(checker: &Checker, args: &Arguments, tvar_name: &str) -> bool { +fn is_likely_private_typevar( + checker: &Checker, + tvar_name: &str, + type_params: &[TypeParam], +) -> bool { if tvar_name.starts_with('_') { return true; } - if checker.settings.target_version < PythonVersion::Py312 { + if checker.settings.target_version < PythonVersion::Py39 { return false; } - for ArgWithDefault { def, .. } in &args.args { - if def.arg.to_string() != tvar_name { - continue; - } - - let Some(annotation) = &def.annotation else { - continue - }; - - let Expr::Name(ast::ExprName{id,..}) = annotation.as_ref() else { - continue - }; - - // Check if the binding used in an annotation is an assignment to typing.TypeVar - let scope = checker.semantic().scope(); - let Some(binding_id) = scope.get(id) else { - continue - }; - let binding = checker.semantic().binding(binding_id); - if binding.kind.is_assignment() || binding.kind.is_named_expr_assignment() { - if let Some(parent_id) = binding.source { - let parent = checker.semantic().stmts[parent_id]; - if let Stmt::Assign(ast::StmtAssign { value, .. }) - | Stmt::AnnAssign(ast::StmtAnnAssign { - value: Some(value), .. - }) - | Stmt::AugAssign(ast::StmtAugAssign { value, .. }) = parent - { - let Expr::Call(ast::ExprCall{func, ..}) = value.as_ref() else { - continue - }; - let Some(call_path) = checker.semantic().resolve_call_path(func) else { - continue - }; - if checker - .semantic() - .match_typing_call_path(&call_path, "TypeVar") - { - return true; - } - } - } + for param in type_params { + if let TypeParam::TypeVar(ast::TypeParamTypeVar { name, .. }) = param { + return name == tvar_name; } } false diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI019_PYI019.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI019_PYI019.pyi.snap new file mode 100644 index 0000000000000..a87f86e3a5f1e --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI019_PYI019.pyi.snap @@ -0,0 +1,38 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +--- +PYI019.pyi:7:62: PYI019 Methods like __new__ should return `typing.Self` instead of a custom TypeVar + | +6 | class BadClass: +7 | def __new__(cls: type[_S], *args: str, **kwargs: int) -> _S: ... # PYI019 + | ^^ PYI019 + | + +PYI019.pyi:10:54: PYI019 Methods like bad_instance_method should return `typing.Self` instead of a custom TypeVar + | +10 | def bad_instance_method(self: _S, arg: bytes) -> _S: ... # PYI019 + | ^^ PYI019 + | + +PYI019.pyi:14:54: PYI019 Methods like bad_class_method should return `typing.Self` instead of a custom TypeVar + | +13 | @classmethod +14 | def bad_class_method(cls: type[_S], arg: int) -> _S: ... # PYI019 + | ^^ PYI019 + | + +PYI019.pyi:35:63: PYI019 Methods like __new__ should return `typing.Self` instead of a custom TypeVar + | +33 | # Python > 3.12 +34 | class PEP695BadDunderNew[T]: +35 | def __new__[S](cls: type[S], *args: Any, ** kwargs: Any) -> S: ... # PYI019 + | ^ PYI019 + | + +PYI019.pyi:38:46: PYI019 Methods like generic_instance_method should return `typing.Self` instead of a custom TypeVar + | +38 | def generic_instance_method[S](self: S) -> S: ... # PYI019 + | ^ PYI019 + | + + diff --git a/ruff.schema.json b/ruff.schema.json index 87ddf7002cc55..4c0bbd07b1095 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2382,6 +2382,7 @@ "PYI016", "PYI017", "PYI018", + "PYI019", "PYI02", "PYI020", "PYI021", From 9c3a9fc12b93b92eaadc9c731f32c9fde7f5e11b Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Mon, 31 Jul 2023 19:16:54 +0200 Subject: [PATCH 6/8] Run test in Python312 --- crates/ruff/src/rules/flake8_pyi/mod.rs | 18 ++++++++++++++++-- .../flake8_pyi/rules/custom_typevar_return.rs | 2 +- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/crates/ruff/src/rules/flake8_pyi/mod.rs b/crates/ruff/src/rules/flake8_pyi/mod.rs index 16dc7885d952d..f80273aeaea69 100644 --- a/crates/ruff/src/rules/flake8_pyi/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/mod.rs @@ -12,6 +12,7 @@ mod tests { use crate::registry::Rule; use crate::test::test_path; use crate::{assert_messages, settings}; + use crate::settings::types::PythonVersion; #[test_case(Rule::AnyEqNeAnnotation, Path::new("PYI032.py"))] #[test_case(Rule::AnyEqNeAnnotation, Path::new("PYI032.pyi"))] @@ -29,8 +30,6 @@ mod tests { #[test_case(Rule::ComplexAssignmentInStub, Path::new("PYI017.pyi"))] #[test_case(Rule::ComplexIfStatementInStub, Path::new("PYI002.py"))] #[test_case(Rule::ComplexIfStatementInStub, Path::new("PYI002.pyi"))] - #[test_case(Rule::CustomTypeVarReturnType, Path::new("PYI019.py"))] - #[test_case(Rule::CustomTypeVarReturnType, Path::new("PYI019.pyi"))] #[test_case(Rule::DocstringInStub, Path::new("PYI021.py"))] #[test_case(Rule::DocstringInStub, Path::new("PYI021.pyi"))] #[test_case(Rule::DuplicateUnionMember, Path::new("PYI016.py"))] @@ -112,4 +111,19 @@ mod tests { assert_messages!(snapshot, diagnostics); Ok(()) } + + #[test_case(Path::new("PYI019.py"))] + #[test_case(Path::new("PYI019.pyi"))] + fn custom_type_var_return_type(path: &Path) -> Result<()> { + let snapshot = format!("{}_{}", "PYI019", path.to_string_lossy()); + let diagnostics = test_path( + Path::new("flake8_pyi").join(path).as_path(), + &settings::Settings { + target_version: PythonVersion::Py312, + ..settings::Settings::for_rules(vec![Rule::CustomTypeVarReturnType]) + }, + )?; + assert_messages!(snapshot, diagnostics); + Ok(()) + } } diff --git a/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs b/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs index 7333a3bdb95b8..8b5ebda1343c3 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs @@ -190,7 +190,7 @@ fn is_likely_private_typevar( if tvar_name.starts_with('_') { return true; } - if checker.settings.target_version < PythonVersion::Py39 { + if checker.settings.target_version < PythonVersion::Py312 { return false; } From 47565e6fa834b9d9c542b49efbdaa57d1556207e Mon Sep 17 00:00:00 2001 From: qdegraaf Date: Mon, 31 Jul 2023 19:18:57 +0200 Subject: [PATCH 7/8] cargo fmt --- crates/ruff/src/rules/flake8_pyi/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff/src/rules/flake8_pyi/mod.rs b/crates/ruff/src/rules/flake8_pyi/mod.rs index f80273aeaea69..68096fa93866f 100644 --- a/crates/ruff/src/rules/flake8_pyi/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/mod.rs @@ -10,9 +10,9 @@ mod tests { use test_case::test_case; use crate::registry::Rule; + use crate::settings::types::PythonVersion; use crate::test::test_path; use crate::{assert_messages, settings}; - use crate::settings::types::PythonVersion; #[test_case(Rule::AnyEqNeAnnotation, Path::new("PYI032.py"))] #[test_case(Rule::AnyEqNeAnnotation, Path::new("PYI032.pyi"))] From 1e79e8d008df760ee8a8c834881d24d89bc08cb1 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 2 Aug 2023 20:30:35 -0400 Subject: [PATCH 8/8] Tweaks --- .../src/checkers/ast/analyze/statement.rs | 6 +- .../rules/custom_type_var_return_type.rs | 212 ++++++++++++++++++ .../flake8_pyi/rules/custom_typevar_return.rs | 203 ----------------- crates/ruff/src/rules/flake8_pyi/rules/mod.rs | 4 +- ..._flake8_pyi__tests__PYI019_PYI019.pyi.snap | 10 +- 5 files changed, 222 insertions(+), 213 deletions(-) create mode 100644 crates/ruff/src/rules/flake8_pyi/rules/custom_type_var_return_type.rs delete mode 100644 crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs diff --git a/crates/ruff/src/checkers/ast/analyze/statement.rs b/crates/ruff/src/checkers/ast/analyze/statement.rs index 12fdf21ad5ecd..e697d6a476e2d 100644 --- a/crates/ruff/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff/src/checkers/ast/analyze/statement.rs @@ -158,13 +158,13 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { ); } if checker.enabled(Rule::CustomTypeVarReturnType) { - flake8_pyi::rules::custom_typevar_return_type( + flake8_pyi::rules::custom_type_var_return_type( checker, name, decorator_list, returns.as_ref().map(AsRef::as_ref), - args, - type_params, + parameters, + type_params.as_ref(), ); } if checker.enabled(Rule::StrOrReprDefinedInStub) { diff --git a/crates/ruff/src/rules/flake8_pyi/rules/custom_type_var_return_type.rs b/crates/ruff/src/rules/flake8_pyi/rules/custom_type_var_return_type.rs new file mode 100644 index 0000000000000..4ba24634a64ec --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/rules/custom_type_var_return_type.rs @@ -0,0 +1,212 @@ +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast as ast; +use ruff_python_ast::helpers::map_subscript; +use ruff_python_ast::{ + Decorator, Expr, ParameterWithDefault, Parameters, Ranged, TypeParam, TypeParams, +}; +use ruff_python_semantic::analyze::visibility::{ + is_abstract, is_classmethod, is_new, is_overload, is_staticmethod, +}; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for methods that define a custom `TypeVar` for their return type +/// annotation instead of using `typing_extensions.Self`. +/// +/// ## Why is this bad? +/// If certain methods are annotated with a custom `TypeVar` type, and the +/// class is subclassed, type checkers will not be able to infer the correct +/// return type. +/// +/// This check currently applies to instance methods that return `self`, class +/// methods that return an instance of `cls`, and `__new__` methods. +/// +/// ## Example +/// ```python +/// class Foo: +/// def __new__(cls: type[_S], *args: str, **kwargs: int) -> _S: +/// ... +/// +/// def foo(self: _S, arg: bytes) -> _S: +/// ... +/// +/// @classmethod +/// def bar(cls: type[_S], arg: int) -> _S: +/// ... +/// ``` +/// +/// Use instead: +/// ```python +/// from typing import Self +/// +/// +/// class Foo: +/// def __new__(cls: type[Self], *args: str, **kwargs: int) -> Self: +/// ... +/// +/// def foo(self: Self, arg: bytes) -> Self: +/// ... +/// +/// @classmethod +/// def bar(cls: type[Self], arg: int) -> Self: +/// ... +/// ``` +#[violation] +pub struct CustomTypeVarReturnType { + method_name: String, +} + +impl Violation for CustomTypeVarReturnType { + #[derive_message_formats] + fn message(&self) -> String { + let CustomTypeVarReturnType { method_name } = self; + format!( + "Methods like `{method_name}` should return `typing.Self` instead of a custom `TypeVar`" + ) + } +} + +/// PYI019 +pub(crate) fn custom_type_var_return_type( + checker: &mut Checker, + name: &str, + decorator_list: &[Decorator], + returns: Option<&Expr>, + args: &Parameters, + type_params: Option<&TypeParams>, +) { + if args.args.is_empty() && args.posonlyargs.is_empty() { + return; + } + + let Some(returns) = returns else { + return; + }; + + if !checker.semantic().scope().kind.is_class() { + return; + }; + + // Skip any abstract, static, and overloaded methods. + if is_abstract(decorator_list, checker.semantic()) + || is_overload(decorator_list, checker.semantic()) + || is_staticmethod(decorator_list, checker.semantic()) + { + return; + } + + let returns = map_subscript(returns); + + let uses_custom_var: bool = + if is_classmethod(decorator_list, checker.semantic()) || is_new(name) { + class_method(args, returns, type_params) + } else { + // If not static, or a class method or __new__ we know it is an instance method + instance_method(args, returns, type_params) + }; + + if uses_custom_var { + checker.diagnostics.push(Diagnostic::new( + CustomTypeVarReturnType { + method_name: name.to_string(), + }, + returns.range(), + )); + } +} + +/// Returns `true` if the class method is annotated with a custom `TypeVar` that is likely +/// private. +fn class_method( + args: &Parameters, + return_annotation: &Expr, + type_params: Option<&TypeParams>, +) -> bool { + let ParameterWithDefault { parameter, .. } = &args.args[0]; + + let Some(annotation) = ¶meter.annotation else { + return false; + }; + + let Expr::Subscript(ast::ExprSubscript { slice, value, .. }) = annotation.as_ref() else { + return false; + }; + + let Expr::Name(value) = value.as_ref() else { + return false; + }; + + // Don't error if the first argument is annotated with typing.Type[T]. + // These are edge cases, and it's hard to give good error messages for them. + if value.id != "type" { + return false; + }; + + let Expr::Name(slice) = slice.as_ref() else { + return false; + }; + + let Expr::Name(return_annotation) = return_annotation else { + return false; + }; + + if slice.id != return_annotation.id { + return false; + } + + is_likely_private_typevar(&slice.id, type_params) +} + +/// Returns `true` if the instance method is annotated with a custom `TypeVar` that is likely +/// private. +fn instance_method( + args: &Parameters, + return_annotation: &Expr, + type_params: Option<&TypeParams>, +) -> bool { + let ParameterWithDefault { parameter, .. } = &args.args[0]; + + let Some(annotation) = ¶meter.annotation else { + return false; + }; + + let Expr::Name(ast::ExprName { + id: first_arg_type, .. + }) = annotation.as_ref() + else { + return false; + }; + + let Expr::Name(ast::ExprName { + id: return_type, .. + }) = return_annotation + else { + return false; + }; + + if first_arg_type != return_type { + return false; + } + + is_likely_private_typevar(first_arg_type, type_params) +} + +/// Returns `true` if the type variable is likely private. +fn is_likely_private_typevar(type_var_name: &str, type_params: Option<&TypeParams>) -> bool { + // Ex) `_T` + if type_var_name.starts_with('_') { + return true; + } + // Ex) `class Foo[T]: ...` + type_params.is_some_and(|type_params| { + type_params.iter().any(|type_param| { + if let TypeParam::TypeVar(ast::TypeParamTypeVar { name, .. }) = type_param { + name == type_var_name + } else { + false + } + }) + }) +} diff --git a/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs b/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs deleted file mode 100644 index 8b5ebda1343c3..0000000000000 --- a/crates/ruff/src/rules/flake8_pyi/rules/custom_typevar_return.rs +++ /dev/null @@ -1,203 +0,0 @@ -use crate::checkers::ast::Checker; -use crate::settings::types::PythonVersion; -use ruff_diagnostics::{Diagnostic, Violation}; -use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast as ast; -use ruff_python_ast::{ArgWithDefault, Arguments, Decorator, Expr, Ranged, TypeParam}; -use ruff_python_semantic::analyze::visibility::{ - is_abstract, is_classmethod, is_overload, is_staticmethod, -}; -use ruff_python_semantic::ScopeKind; - -/// ## What it does -/// Checks if certain methods define a custom `TypeVar`s for their return annotation instead of -/// using `typing_extensions.Self`. This check currently applies for instance methods that return -/// `self`, class methods that return an instance of `cls`, and `__new__` methods. -/// -/// ## Why is this bad? -/// If certain methods are annotated with a custom `TypeVar` type, and the class is subclassed, -/// type checkers will not be able to infer the correct return type. -/// -/// ## Example -/// ```python -/// class Foo: -/// def __new__(cls: type[_S], *args: str, **kwargs: int) -> _S: -/// ... -/// -/// def foo(self: _S, arg: bytes) -> _S: -/// ... -/// -/// @classmethod -/// def bar(cls: type[_S], arg: int) -> _S: -/// ... -/// ``` -/// -/// Use instead: -/// ```python -/// from typing import Self -/// -/// -/// class Foo: -/// def __new__(cls: type[Self], *args: str, **kwargs: int) -> Self: -/// ... -/// -/// def foo(self: Self, arg: bytes) -> Self: -/// ... -/// -/// @classmethod -/// def bar(cls: type[Self], arg: int) -> Self: -/// ... -/// ``` -#[violation] -pub struct CustomTypeVarReturnType { - method_name: String, -} - -impl Violation for CustomTypeVarReturnType { - #[derive_message_formats] - fn message(&self) -> String { - let CustomTypeVarReturnType { method_name } = self; - format!( - "Methods like {method_name} should return `typing.Self` instead of a custom TypeVar" - ) - } -} - -/// PYI019 -pub(crate) fn custom_typevar_return_type( - checker: &mut Checker, - name: &str, - decorator_list: &[Decorator], - returns: Option<&Expr>, - args: &Arguments, - type_params: &[TypeParam], -) { - let ScopeKind::Class(_) = checker.semantic().scope().kind else { - return; - }; - - if args.args.is_empty() && args.posonlyargs.is_empty() { - return; - } - - let Some(returns) = returns else { - return; - }; - - let return_annotation = if let Expr::Subscript(ast::ExprSubscript { value, .. }) = returns { - // Ex) `Type[T]` - value - } else { - // Ex) `Type`, `typing.Type` - returns - }; - - // Skip any abstract, static and overloaded methods. - if is_abstract(decorator_list, checker.semantic()) - || is_overload(decorator_list, checker.semantic()) - || is_staticmethod(decorator_list, checker.semantic()) - { - return; - } - - let is_violation: bool = - if is_classmethod(decorator_list, checker.semantic()) || name == "__new__" { - check_class_method_for_bad_typevars(checker, args, return_annotation, type_params) - } else { - // If not static, or a class method or __new__ we know it is an instance method - check_instance_method_for_bad_typevars(checker, args, return_annotation, type_params) - }; - - if is_violation { - checker.diagnostics.push(Diagnostic::new( - CustomTypeVarReturnType { - method_name: name.to_string(), - }, - return_annotation.range(), - )); - } -} - -fn check_class_method_for_bad_typevars( - checker: &Checker, - args: &Arguments, - return_annotation: &Expr, - type_params: &[TypeParam], -) -> bool { - let ArgWithDefault { def, .. } = &args.args[0]; - - let Some(annotation) = &def.annotation else { - return false - }; - - let Expr::Subscript(ast::ExprSubscript{slice, value, ..}) = annotation.as_ref() else { - return false - }; - - let Expr::Name(ast::ExprName{ id: id_value, .. }) = value.as_ref() else { - return false - }; - - // Don't error if the first argument is annotated with typing.Type[T] - // These are edge cases, and it's hard to give good error messages for them. - if id_value != "type" { - return false; - }; - - let Expr::Name(ast::ExprName { id: id_slice, .. }) = slice.as_ref() else { - return false - }; - - let Expr::Name(ast::ExprName { id: return_type, .. }) = return_annotation else { - return false - }; - - return_type == id_slice && is_likely_private_typevar(checker, id_slice, type_params) -} - -fn check_instance_method_for_bad_typevars( - checker: &Checker, - args: &Arguments, - return_annotation: &Expr, - type_params: &[TypeParam], -) -> bool { - let ArgWithDefault { def, .. } = &args.args[0]; - - let Some(annotation) = &def.annotation else { - return false - }; - - let Expr::Name(ast::ExprName{id: first_arg_type,..}) = annotation.as_ref() else { - return false - }; - - let Expr::Name(ast::ExprName { id: return_type, .. }) = return_annotation else { - return false - }; - - if first_arg_type != return_type { - return false; - } - - is_likely_private_typevar(checker, first_arg_type, type_params) -} - -fn is_likely_private_typevar( - checker: &Checker, - tvar_name: &str, - type_params: &[TypeParam], -) -> bool { - if tvar_name.starts_with('_') { - return true; - } - if checker.settings.target_version < PythonVersion::Py312 { - return false; - } - - for param in type_params { - if let TypeParam::TypeVar(ast::TypeParamTypeVar { name, .. }) = param { - return name == tvar_name; - } - } - false -} diff --git a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs index 8e2fd9d7157d0..e340427d0499f 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs @@ -3,7 +3,7 @@ pub(crate) use bad_version_info_comparison::*; pub(crate) use collections_named_tuple::*; pub(crate) use complex_assignment_in_stub::*; pub(crate) use complex_if_statement_in_stub::*; -pub(crate) use custom_typevar_return::*; +pub(crate) use custom_type_var_return_type::*; pub(crate) use docstring_in_stubs::*; pub(crate) use duplicate_union_member::*; pub(crate) use ellipsis_in_non_empty_class_body::*; @@ -38,7 +38,7 @@ mod bad_version_info_comparison; mod collections_named_tuple; mod complex_assignment_in_stub; mod complex_if_statement_in_stub; -mod custom_typevar_return; +mod custom_type_var_return_type; mod docstring_in_stubs; mod duplicate_union_member; mod ellipsis_in_non_empty_class_body; diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI019_PYI019.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI019_PYI019.pyi.snap index a87f86e3a5f1e..121ee656304d4 100644 --- a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI019_PYI019.pyi.snap +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI019_PYI019.pyi.snap @@ -1,27 +1,27 @@ --- source: crates/ruff/src/rules/flake8_pyi/mod.rs --- -PYI019.pyi:7:62: PYI019 Methods like __new__ should return `typing.Self` instead of a custom TypeVar +PYI019.pyi:7:62: PYI019 Methods like `__new__` should return `typing.Self` instead of a custom `TypeVar` | 6 | class BadClass: 7 | def __new__(cls: type[_S], *args: str, **kwargs: int) -> _S: ... # PYI019 | ^^ PYI019 | -PYI019.pyi:10:54: PYI019 Methods like bad_instance_method should return `typing.Self` instead of a custom TypeVar +PYI019.pyi:10:54: PYI019 Methods like `bad_instance_method` should return `typing.Self` instead of a custom `TypeVar` | 10 | def bad_instance_method(self: _S, arg: bytes) -> _S: ... # PYI019 | ^^ PYI019 | -PYI019.pyi:14:54: PYI019 Methods like bad_class_method should return `typing.Self` instead of a custom TypeVar +PYI019.pyi:14:54: PYI019 Methods like `bad_class_method` should return `typing.Self` instead of a custom `TypeVar` | 13 | @classmethod 14 | def bad_class_method(cls: type[_S], arg: int) -> _S: ... # PYI019 | ^^ PYI019 | -PYI019.pyi:35:63: PYI019 Methods like __new__ should return `typing.Self` instead of a custom TypeVar +PYI019.pyi:35:63: PYI019 Methods like `__new__` should return `typing.Self` instead of a custom `TypeVar` | 33 | # Python > 3.12 34 | class PEP695BadDunderNew[T]: @@ -29,7 +29,7 @@ PYI019.pyi:35:63: PYI019 Methods like __new__ should return `typing.Self` instea | ^ PYI019 | -PYI019.pyi:38:46: PYI019 Methods like generic_instance_method should return `typing.Self` instead of a custom TypeVar +PYI019.pyi:38:46: PYI019 Methods like `generic_instance_method` should return `typing.Self` instead of a custom `TypeVar` | 38 | def generic_instance_method[S](self: S) -> S: ... # PYI019 | ^ PYI019