diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI034.py b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI034.py new file mode 100644 index 0000000000..850bd057dd --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI034.py @@ -0,0 +1,280 @@ +# flags: --extend-ignore=Y023 + +import abc +import builtins +import collections.abc +import typing +from abc import abstractmethod +from collections.abc import AsyncIterable, AsyncIterator, Iterable, Iterator +from typing import Any, overload + +import typing_extensions +from _typeshed import Self +from typing_extensions import final + + +class Bad( + object +): # Y040 Do not inherit from "object" explicitly, as it is redundant in Python 3 + def __new__(cls, *args: Any, **kwargs: Any) -> Bad: + ... # Y034 "__new__" methods usually return "self" at runtime. Consider using "typing_extensions.Self" in "Bad.__new__", e.g. "def __new__(cls, *args: Any, **kwargs: Any) -> Self: ..." + + def __repr__(self) -> str: + ... # Y029 Defining __repr__ or __str__ in a stub is almost always redundant + + def __str__(self) -> builtins.str: + ... # Y029 Defining __repr__ or __str__ in a stub is almost always redundant + + def __eq__(self, other: Any) -> bool: + ... # Y032 Prefer "object" to "Any" for the second parameter in "__eq__" methods + + def __ne__(self, other: typing.Any) -> typing.Any: + ... # Y032 Prefer "object" to "Any" for the second parameter in "__ne__" methods + + def __enter__(self) -> Bad: + ... # Y034 "__enter__" methods in classes like "Bad" usually return "self" at runtime. Consider using "typing_extensions.Self" in "Bad.__enter__", e.g. "def __enter__(self) -> Self: ..." + + async def __aenter__(self) -> Bad: + ... # Y034 "__aenter__" methods in classes like "Bad" usually return "self" at runtime. Consider using "typing_extensions.Self" in "Bad.__aenter__", e.g. "async def __aenter__(self) -> Self: ..." + + def __iadd__(self, other: Bad) -> Bad: + ... # Y034 "__iadd__" methods in classes like "Bad" usually return "self" at runtime. Consider using "typing_extensions.Self" in "Bad.__iadd__", e.g. "def __iadd__(self, other: Bad) -> Self: ..." + + +class AlsoBad(int, builtins.object): + ... # Y040 Do not inherit from "object" explicitly, as it is redundant in Python 3 + + +class Good: + def __new__(cls: type[Self], *args: Any, **kwargs: Any) -> Self: + ... + + @abstractmethod + def __str__(self) -> str: + ... + + @abc.abstractmethod + def __repr__(self) -> str: + ... + + def __eq__(self, other: object) -> bool: + ... + + def __ne__(self, obj: object) -> int: + ... + + def __enter__(self: Self) -> Self: + ... + + async def __aenter__(self: Self) -> Self: + ... + + def __ior__(self: Self, other: Self) -> Self: + ... + + +class Fine: + @overload + def __new__(cls, foo: int) -> FineSubclass: + ... + + @overload + def __new__(cls, *args: Any, **kwargs: Any) -> Fine: + ... + + @abc.abstractmethod + def __str__(self) -> str: + ... + + @abc.abstractmethod + def __repr__(self) -> str: + ... + + def __eq__(self, other: Any, strange_extra_arg: list[str]) -> Any: + ... + + def __ne__(self, *, kw_only_other: Any) -> bool: + ... + + def __enter__(self) -> None: + ... + + async def __aenter__(self) -> bool: + ... + + +class FineSubclass(Fine): + ... + + +class StrangeButAcceptable(str): + @typing_extensions.overload + def __new__(cls, foo: int) -> StrangeButAcceptableSubclass: + ... + + @typing_extensions.overload + def __new__(cls, *args: Any, **kwargs: Any) -> StrangeButAcceptable: + ... + + def __str__(self) -> StrangeButAcceptable: + ... + + def __repr__(self) -> StrangeButAcceptable: + ... + + +class StrangeButAcceptableSubclass(StrangeButAcceptable): + ... + + +class FineAndDandy: + def __str__(self, weird_extra_arg) -> str: + ... + + def __repr__(self, weird_extra_arg_with_default=...) -> str: + ... + + +@final +class WillNotBeSubclassed: + def __new__(cls, *args: Any, **kwargs: Any) -> WillNotBeSubclassed: + ... + + def __enter__(self) -> WillNotBeSubclassed: + ... + + async def __aenter__(self) -> WillNotBeSubclassed: + ... + + +# we don't emit an error for these; out of scope for a linter +class InvalidButPluginDoesNotCrash: + def __new__() -> InvalidButPluginDoesNotCrash: + ... + + def __enter__() -> InvalidButPluginDoesNotCrash: + ... + + async def __aenter__() -> InvalidButPluginDoesNotCrash: + ... + + +class BadIterator1(Iterator[int]): + def __iter__(self) -> Iterator[int]: + ... # Y034 "__iter__" methods in classes like "BadIterator1" usually return "self" at runtime. Consider using "typing_extensions.Self" in "BadIterator1.__iter__", e.g. "def __iter__(self) -> Self: ..." + + +class BadIterator2( + typing.Iterator[int] +): # Y022 Use "collections.abc.Iterator[T]" instead of "typing.Iterator[T]" (PEP 585 syntax) + def __iter__(self) -> Iterator[int]: + ... # Y034 "__iter__" methods in classes like "BadIterator2" usually return "self" at runtime. Consider using "typing_extensions.Self" in "BadIterator2.__iter__", e.g. "def __iter__(self) -> Self: ..." + + +class BadIterator3( + typing.Iterator[int] +): # Y022 Use "collections.abc.Iterator[T]" instead of "typing.Iterator[T]" (PEP 585 syntax) + def __iter__(self) -> collections.abc.Iterator[int]: + ... # Y034 "__iter__" methods in classes like "BadIterator3" usually return "self" at runtime. Consider using "typing_extensions.Self" in "BadIterator3.__iter__", e.g. "def __iter__(self) -> Self: ..." + + +class BadIterator4(Iterator[int]): + # Note: *Iterable*, not *Iterator*, returned! + def __iter__(self) -> Iterable[int]: + ... # Y034 "__iter__" methods in classes like "BadIterator4" usually return "self" at runtime. Consider using "typing_extensions.Self" in "BadIterator4.__iter__", e.g. "def __iter__(self) -> Self: ..." + + +class IteratorReturningIterable: + def __iter__(self) -> Iterable[str]: + ... # Y045 "__iter__" methods should return an Iterator, not an Iterable + + +class BadAsyncIterator(collections.abc.AsyncIterator[str]): + def __aiter__(self) -> typing.AsyncIterator[str]: + ... # Y034 "__aiter__" methods in classes like "BadAsyncIterator" usually return "self" at runtime. Consider using "typing_extensions.Self" in "BadAsyncIterator.__aiter__", e.g. "def __aiter__(self) -> Self: ..." # Y022 Use "collections.abc.AsyncIterator[T]" instead of "typing.AsyncIterator[T]" (PEP 585 syntax) + + +class AsyncIteratorReturningAsyncIterable: + def __aiter__(self) -> AsyncIterable[str]: + ... # Y045 "__aiter__" methods should return an AsyncIterator, not an AsyncIterable + + +class Abstract(Iterator[str]): + @abstractmethod + def __iter__(self) -> Iterator[str]: + ... + + @abstractmethod + def __enter__(self) -> Abstract: + ... + + @abstractmethod + async def __aenter__(self) -> Abstract: + ... + + +class GoodIterator(Iterator[str]): + def __iter__(self: Self) -> Self: + ... + + +class GoodAsyncIterator(AsyncIterator[int]): + def __aiter__(self: Self) -> Self: + ... + + +class DoesNotInheritFromIterator: + def __iter__(self) -> DoesNotInheritFromIterator: + ... + + +class Unannotated: + def __new__(cls, *args, **kwargs): + ... + + def __iter__(self): + ... + + def __aiter__(self): + ... + + async def __aenter__(self): + ... + + def __repr__(self): + ... + + def __str__(self): + ... + + def __eq__(self): + ... + + def __ne__(self): + ... + + def __iadd__(self): + ... + + def __ior__(self): + ... + + +def __repr__(self) -> str: + ... + + +def __str__(self) -> str: + ... + + +def __eq__(self, other: Any) -> bool: + ... + + +def __ne__(self, other: Any) -> bool: + ... + + +def __imul__(self, other: Any) -> list[str]: + ... diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI034.pyi b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI034.pyi new file mode 100644 index 0000000000..800cf14512 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI034.pyi @@ -0,0 +1,188 @@ +# flags: --extend-ignore=Y023 + +import abc +import builtins +import collections.abc +import typing +from abc import abstractmethod +from collections.abc import AsyncIterable, AsyncIterator, Iterable, Iterator +from typing import Any, overload + +import typing_extensions +from _typeshed import Self +from typing_extensions import final + +class Bad( + object +): # Y040 Do not inherit from "object" explicitly, as it is redundant in Python 3 + def __new__( + cls, *args: Any, **kwargs: Any + ) -> Bad: ... # Y034 "__new__" methods usually return "self" at runtime. Consider using "typing_extensions.Self" in "Bad.__new__", e.g. "def __new__(cls, *args: Any, **kwargs: Any) -> Self: ..." + def __repr__( + self, + ) -> str: ... # Y029 Defining __repr__ or __str__ in a stub is almost always redundant + def __str__( + self, + ) -> builtins.str: ... # Y029 Defining __repr__ or __str__ in a stub is almost always redundant + def __eq__( + self, other: Any + ) -> bool: ... # Y032 Prefer "object" to "Any" for the second parameter in "__eq__" methods + def __ne__( + self, other: typing.Any + ) -> typing.Any: ... # Y032 Prefer "object" to "Any" for the second parameter in "__ne__" methods + def __enter__( + self, + ) -> Bad: ... # Y034 "__enter__" methods in classes like "Bad" usually return "self" at runtime. Consider using "typing_extensions.Self" in "Bad.__enter__", e.g. "def __enter__(self) -> Self: ..." + async def __aenter__( + self, + ) -> Bad: ... # Y034 "__aenter__" methods in classes like "Bad" usually return "self" at runtime. Consider using "typing_extensions.Self" in "Bad.__aenter__", e.g. "async def __aenter__(self) -> Self: ..." + def __iadd__( + self, other: Bad + ) -> Bad: ... # Y034 "__iadd__" methods in classes like "Bad" usually return "self" at runtime. Consider using "typing_extensions.Self" in "Bad.__iadd__", e.g. "def __iadd__(self, other: Bad) -> Self: ..." + +class AlsoBad( + int, builtins.object +): ... # Y040 Do not inherit from "object" explicitly, as it is redundant in Python 3 + +class Good: + def __new__(cls: type[Self], *args: Any, **kwargs: Any) -> Self: ... + @abstractmethod + def __str__(self) -> str: ... + @abc.abstractmethod + def __repr__(self) -> str: ... + def __eq__(self, other: object) -> bool: ... + def __ne__(self, obj: object) -> int: ... + def __enter__(self: Self) -> Self: ... + async def __aenter__(self: Self) -> Self: ... + def __ior__(self: Self, other: Self) -> Self: ... + +class Fine: + @overload + def __new__(cls, foo: int) -> FineSubclass: ... + @overload + def __new__(cls, *args: Any, **kwargs: Any) -> Fine: ... + @abc.abstractmethod + def __str__(self) -> str: ... + @abc.abstractmethod + def __repr__(self) -> str: ... + def __eq__(self, other: Any, strange_extra_arg: list[str]) -> Any: ... + def __ne__(self, *, kw_only_other: Any) -> bool: ... + def __enter__(self) -> None: ... + async def __aenter__(self) -> bool: ... + +class FineSubclass(Fine): ... + +class StrangeButAcceptable(str): + @typing_extensions.overload + def __new__(cls, foo: int) -> StrangeButAcceptableSubclass: ... + @typing_extensions.overload + def __new__(cls, *args: Any, **kwargs: Any) -> StrangeButAcceptable: ... + def __str__(self) -> StrangeButAcceptable: ... + def __repr__(self) -> StrangeButAcceptable: ... + +class StrangeButAcceptableSubclass(StrangeButAcceptable): ... + +class FineAndDandy: + def __str__(self, weird_extra_arg) -> str: ... + def __repr__(self, weird_extra_arg_with_default=...) -> str: ... + +@final +class WillNotBeSubclassed: + def __new__(cls, *args: Any, **kwargs: Any) -> WillNotBeSubclassed: ... + def __enter__(self) -> WillNotBeSubclassed: ... + async def __aenter__(self) -> WillNotBeSubclassed: ... + +# we don't emit an error for these; out of scope for a linter +class InvalidButPluginDoesNotCrash: + def __new__() -> InvalidButPluginDoesNotCrash: ... + def __enter__() -> InvalidButPluginDoesNotCrash: ... + async def __aenter__() -> InvalidButPluginDoesNotCrash: ... + +class BadIterator1(Iterator[int]): + def __iter__( + self, + ) -> Iterator[ + int + ]: ... # Y034 "__iter__" methods in classes like "BadIterator1" usually return "self" at runtime. Consider using "typing_extensions.Self" in "BadIterator1.__iter__", e.g. "def __iter__(self) -> Self: ..." + +class BadIterator2( + typing.Iterator[int] +): # Y022 Use "collections.abc.Iterator[T]" instead of "typing.Iterator[T]" (PEP 585 syntax) + def __iter__( + self, + ) -> Iterator[ + int + ]: ... # Y034 "__iter__" methods in classes like "BadIterator2" usually return "self" at runtime. Consider using "typing_extensions.Self" in "BadIterator2.__iter__", e.g. "def __iter__(self) -> Self: ..." + +class BadIterator3( + typing.Iterator[int] +): # Y022 Use "collections.abc.Iterator[T]" instead of "typing.Iterator[T]" (PEP 585 syntax) + def __iter__( + self, + ) -> collections.abc.Iterator[ + int + ]: ... # Y034 "__iter__" methods in classes like "BadIterator3" usually return "self" at runtime. Consider using "typing_extensions.Self" in "BadIterator3.__iter__", e.g. "def __iter__(self) -> Self: ..." + +class BadIterator4(Iterator[int]): + # Note: *Iterable*, not *Iterator*, returned! + def __iter__( + self, + ) -> Iterable[ + int + ]: ... # Y034 "__iter__" methods in classes like "BadIterator4" usually return "self" at runtime. Consider using "typing_extensions.Self" in "BadIterator4.__iter__", e.g. "def __iter__(self) -> Self: ..." + +class IteratorReturningIterable: + def __iter__( + self, + ) -> Iterable[ + str + ]: ... # Y045 "__iter__" methods should return an Iterator, not an Iterable + +class BadAsyncIterator(collections.abc.AsyncIterator[str]): + def __aiter__( + self, + ) -> typing.AsyncIterator[ + str + ]: ... # Y034 "__aiter__" methods in classes like "BadAsyncIterator" usually return "self" at runtime. Consider using "typing_extensions.Self" in "BadAsyncIterator.__aiter__", e.g. "def __aiter__(self) -> Self: ..." # Y022 Use "collections.abc.AsyncIterator[T]" instead of "typing.AsyncIterator[T]" (PEP 585 syntax) + +class AsyncIteratorReturningAsyncIterable: + def __aiter__( + self, + ) -> AsyncIterable[ + str + ]: ... # Y045 "__aiter__" methods should return an AsyncIterator, not an AsyncIterable + +class Abstract(Iterator[str]): + @abstractmethod + def __iter__(self) -> Iterator[str]: ... + @abstractmethod + def __enter__(self) -> Abstract: ... + @abstractmethod + async def __aenter__(self) -> Abstract: ... + +class GoodIterator(Iterator[str]): + def __iter__(self: Self) -> Self: ... + +class GoodAsyncIterator(AsyncIterator[int]): + def __aiter__(self: Self) -> Self: ... + +class DoesNotInheritFromIterator: + def __iter__(self) -> DoesNotInheritFromIterator: ... + +class Unannotated: + def __new__(cls, *args, **kwargs): ... + def __iter__(self): ... + def __aiter__(self): ... + async def __aenter__(self): ... + def __repr__(self): ... + def __str__(self): ... + def __eq__(self): ... + def __ne__(self): ... + def __iadd__(self): ... + def __ior__(self): ... + +def __repr__(self) -> str: ... +def __str__(self) -> str: ... +def __eq__(self, other: Any) -> bool: ... +def __ne__(self, other: Any) -> bool: ... +def __imul__(self, other: Any) -> list[str]: ... diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index a28bccfa12..3690ccc3d0 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -436,6 +436,17 @@ where if self.enabled(Rule::AnyEqNeAnnotation) { flake8_pyi::rules::any_eq_ne_annotation(self, name, args); } + if self.enabled(Rule::NonSelfReturnType) { + flake8_pyi::rules::non_self_return_type( + self, + stmt, + name, + decorator_list, + returns.as_ref().map(|expr| &**expr), + args, + stmt.is_async_function_def_stmt(), + ); + } } if self.enabled(Rule::DunderFunctionName) { diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index b2585fa84a..3c4672e540 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -596,6 +596,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Pyi, "025") => (RuleGroup::Unspecified, Rule::UnaliasedCollectionsAbcSetImport), (Flake8Pyi, "032") => (RuleGroup::Unspecified, Rule::AnyEqNeAnnotation), (Flake8Pyi, "033") => (RuleGroup::Unspecified, Rule::TypeCommentInStub), + (Flake8Pyi, "034") => (RuleGroup::Unspecified, Rule::NonSelfReturnType), (Flake8Pyi, "042") => (RuleGroup::Unspecified, Rule::SnakeCaseTypeAlias), (Flake8Pyi, "043") => (RuleGroup::Unspecified, Rule::TSuffixedTypeAlias), (Flake8Pyi, "045") => (RuleGroup::Unspecified, Rule::IterMethodReturnIterable), diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 2b948c5b66..86f2851995 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -519,6 +519,7 @@ ruff_macros::register_rules!( rules::flake8_pyi::rules::IterMethodReturnIterable, rules::flake8_pyi::rules::DuplicateUnionMember, rules::flake8_pyi::rules::EllipsisInNonEmptyClassBody, + rules::flake8_pyi::rules::NonSelfReturnType, rules::flake8_pyi::rules::CollectionsNamedTuple, rules::flake8_pyi::rules::StringOrBytesTooLong, rules::flake8_pyi::rules::NonEmptyStubBody, diff --git a/crates/ruff/src/rules/flake8_pyi/mod.rs b/crates/ruff/src/rules/flake8_pyi/mod.rs index b2745a523e..ed66126659 100644 --- a/crates/ruff/src/rules/flake8_pyi/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/mod.rs @@ -20,14 +20,16 @@ mod tests { #[test_case(Rule::AssignmentDefaultInStub, Path::new("PYI015.pyi"))] #[test_case(Rule::BadVersionInfoComparison, Path::new("PYI006.py"))] #[test_case(Rule::BadVersionInfoComparison, Path::new("PYI006.pyi"))] + #[test_case(Rule::CollectionsNamedTuple, Path::new("PYI024.py"))] + #[test_case(Rule::CollectionsNamedTuple, Path::new("PYI024.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"))] #[test_case(Rule::DuplicateUnionMember, Path::new("PYI016.pyi"))] #[test_case(Rule::EllipsisInNonEmptyClassBody, Path::new("PYI013.py"))] #[test_case(Rule::EllipsisInNonEmptyClassBody, Path::new("PYI013.pyi"))] - #[test_case(Rule::CollectionsNamedTuple, Path::new("PYI024.py"))] - #[test_case(Rule::CollectionsNamedTuple, Path::new("PYI024.pyi"))] + #[test_case(Rule::NonSelfReturnType, Path::new("PYI034.py"))] + #[test_case(Rule::NonSelfReturnType, Path::new("PYI034.pyi"))] #[test_case(Rule::IterMethodReturnIterable, Path::new("PYI045.py"))] #[test_case(Rule::IterMethodReturnIterable, Path::new("PYI045.pyi"))] #[test_case(Rule::NumericLiteralTooLong, Path::new("PYI054.py"))] diff --git a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs index 662ade7444..c7a45614d0 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs @@ -12,6 +12,7 @@ pub(crate) use iter_method_return_iterable::{ iter_method_return_iterable, IterMethodReturnIterable, }; pub(crate) use non_empty_stub_body::{non_empty_stub_body, NonEmptyStubBody}; +pub(crate) use non_self_return_type::{non_self_return_type, NonSelfReturnType}; pub(crate) use numeric_literal_too_long::{numeric_literal_too_long, NumericLiteralTooLong}; pub(crate) use pass_in_class_body::{pass_in_class_body, PassInClassBody}; pub(crate) use pass_statement_stub_body::{pass_statement_stub_body, PassStatementStubBody}; @@ -45,6 +46,7 @@ mod duplicate_union_member; mod ellipsis_in_non_empty_class_body; mod iter_method_return_iterable; mod non_empty_stub_body; +mod non_self_return_type; mod numeric_literal_too_long; mod pass_in_class_body; mod pass_statement_stub_body; diff --git a/crates/ruff/src/rules/flake8_pyi/rules/non_self_return_type.rs b/crates/ruff/src/rules/flake8_pyi/rules/non_self_return_type.rs new file mode 100644 index 0000000000..52a01ef709 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/rules/non_self_return_type.rs @@ -0,0 +1,298 @@ +use rustpython_parser::ast::{self, Arguments, Expr, Stmt}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::helpers::{identifier_range, map_subscript}; +use ruff_python_semantic::analyze::visibility::{is_abstract, is_final, is_overload}; +use ruff_python_semantic::model::SemanticModel; +use ruff_python_semantic::scope::ScopeKind; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for methods that are annotated with a fixed return type, which +/// should instead be returning `self`. +/// +/// ## Why is this bad? +/// If methods like `__new__` or `__enter__` are annotated with a fixed return +/// type, and the class is subclassed, type checkers will not be able to infer +/// the correct return type. +/// +/// For example: +/// ```python +/// class Shape: +/// def set_scale(self, scale: float) -> Shape: +/// self.scale = scale +/// return self +/// +/// class Circle(Shape): +/// def set_radius(self, radius: float) -> Circle: +/// self.radius = radius +/// return self +/// +/// # This returns `Shape`, not `Circle`. +/// Circle().set_scale(0.5) +/// +/// # Thus, this expression is invalid, as `Shape` has no attribute `set_radius`. +/// Circle().set_scale(0.5).set_radius(2.7) +/// ``` +/// +/// Specifically, this check enforces that the return type of the following +/// methods is `Self`: +/// +/// 1. In-place binary operations, like `__iadd__`, `__imul__`, etc. +/// 1. `__new__`, `__enter__`, and `__aenter__`, if those methods return the +/// class name. +/// 1. `__iter__` methods that return `Iterator`, despite the class inheriting +/// directly from `Iterator`. +/// 1. `__aiter__` methods that return `AsyncIterator`, despite the class +/// inheriting directly from `AsyncIterator`. +/// +/// ## Example +/// ```python +/// class Foo: +/// def __new__(cls, *args: Any, **kwargs: Any) -> Bad: +/// ... +/// +/// def __enter__(self) -> Bad: +/// ... +/// +/// async def __aenter__(self) -> Bad: +/// ... +/// +/// def __iadd__(self, other: Bad) -> Bad: +/// ... +/// ``` +/// +/// Use instead: +/// ```python +/// from typing_extensions import Self +/// +/// +/// class Foo: +/// def __new__(cls, *args: Any, **kwargs: Any) -> Self: +/// ... +/// +/// def __enter__(self) -> Self: +/// ... +/// +/// async def __aenter__(self) -> Self: +/// ... +/// +/// def __iadd__(self, other: Bad) -> Self: +/// ... +/// ``` +/// ## References +/// - [PEP 673](https://peps.python.org/pep-0673/) +#[violation] +pub struct NonSelfReturnType { + class_name: String, + method_name: String, +} + +impl Violation for NonSelfReturnType { + #[derive_message_formats] + fn message(&self) -> String { + let NonSelfReturnType { + class_name, + method_name, + } = self; + if matches!(class_name.as_str(), "__new__") { + format!("`__new__` methods usually return `self` at runtime") + } else { + format!("`{method_name}` methods in classes like `{class_name}` usually return `self` at runtime") + } + } + + fn autofix_title(&self) -> Option { + Some("Consider using `typing_extensions.Self` as return type".to_string()) + } +} + +/// PYI034 +pub(crate) fn non_self_return_type( + checker: &mut Checker, + stmt: &Stmt, + name: &str, + decorator_list: &[Expr], + returns: Option<&Expr>, + args: &Arguments, + async_: bool, +) { + let ScopeKind::Class(class_def) = checker.semantic_model().scope().kind else { + return; + }; + + if args.args.is_empty() && args.posonlyargs.is_empty() { + return; + } + + let Some(returns) = returns else { + return; + }; + + // Skip any abstract or overloaded methods. + if is_abstract(checker.semantic_model(), decorator_list) + || is_overload(checker.semantic_model(), decorator_list) + { + return; + } + + if async_ { + if name == "__aenter__" + && is_name(returns, &class_def.name) + && !is_final(checker.semantic_model(), &class_def.decorator_list) + { + checker.diagnostics.push(Diagnostic::new( + NonSelfReturnType { + class_name: class_def.name.to_string(), + method_name: name.to_string(), + }, + identifier_range(stmt, checker.locator), + )); + } + return; + } + + // In-place methods that are expected to return `Self`. + if INPLACE_BINOP_METHODS.contains(&name) { + if !is_self(returns, checker.semantic_model()) { + checker.diagnostics.push(Diagnostic::new( + NonSelfReturnType { + class_name: class_def.name.to_string(), + method_name: name.to_string(), + }, + identifier_range(stmt, checker.locator), + )); + } + return; + } + + if is_name(returns, &class_def.name) { + if matches!(name, "__enter__" | "__new__") + && !is_final(checker.semantic_model(), &class_def.decorator_list) + { + checker.diagnostics.push(Diagnostic::new( + NonSelfReturnType { + class_name: class_def.name.to_string(), + method_name: name.to_string(), + }, + identifier_range(stmt, checker.locator), + )); + } + return; + } + + match name { + "__iter__" => { + if is_iterable(returns, checker.semantic_model()) + && is_iterator(&class_def.bases, checker.semantic_model()) + { + checker.diagnostics.push(Diagnostic::new( + NonSelfReturnType { + class_name: class_def.name.to_string(), + method_name: name.to_string(), + }, + identifier_range(stmt, checker.locator), + )); + } + } + "__aiter__" => { + if is_async_iterable(returns, checker.semantic_model()) + && is_async_iterator(&class_def.bases, checker.semantic_model()) + { + checker.diagnostics.push(Diagnostic::new( + NonSelfReturnType { + class_name: class_def.name.to_string(), + method_name: name.to_string(), + }, + identifier_range(stmt, checker.locator), + )); + } + } + _ => {} + } +} + +const INPLACE_BINOP_METHODS: &[&str] = &[ + "__iadd__", + "__isub__", + "__imul__", + "__imatmul__", + "__itruediv__", + "__ifloordiv__", + "__imod__", + "__ipow__", + "__ilshift__", + "__irshift__", + "__iand__", + "__ixor__", + "__ior__", +]; + +/// Return `true` if the given expression resolves to the given name. +fn is_name(expr: &Expr, name: &str) -> bool { + let Expr::Name(ast::ExprName { id, .. }) = expr else { + return false; + }; + id.as_str() == name +} + +/// Return `true` if the given expression resolves to `typing.Self`. +fn is_self(expr: &Expr, model: &SemanticModel) -> bool { + model.match_typing_expr(expr, "Self") +} + +/// Return `true` if the given class extends `collections.abc.Iterator`. +fn is_iterator(bases: &[Expr], model: &SemanticModel) -> bool { + bases.iter().any(|expr| { + model + .resolve_call_path(map_subscript(expr)) + .map_or(false, |call_path| { + matches!( + call_path.as_slice(), + ["typing", "Iterator"] | ["collections", "abc", "Iterator"] + ) + }) + }) +} + +/// Return `true` if the given expression resolves to `collections.abc.Iterable`. +fn is_iterable(expr: &Expr, model: &SemanticModel) -> bool { + model + .resolve_call_path(map_subscript(expr)) + .map_or(false, |call_path| { + matches!( + call_path.as_slice(), + ["typing", "Iterable" | "Iterator"] + | ["collections", "abc", "Iterable" | "Iterator"] + ) + }) +} + +/// Return `true` if the given class extends `collections.abc.AsyncIterator`. +fn is_async_iterator(bases: &[Expr], model: &SemanticModel) -> bool { + bases.iter().any(|expr| { + model + .resolve_call_path(map_subscript(expr)) + .map_or(false, |call_path| { + matches!( + call_path.as_slice(), + ["typing", "AsyncIterator"] | ["collections", "abc", "AsyncIterator"] + ) + }) + }) +} + +/// Return `true` if the given expression resolves to `collections.abc.AsyncIterable`. +fn is_async_iterable(expr: &Expr, model: &SemanticModel) -> bool { + model + .resolve_call_path(map_subscript(expr)) + .map_or(false, |call_path| { + matches!( + call_path.as_slice(), + ["typing", "AsyncIterable" | "AsyncIterator"] + | ["collections", "abc", "AsyncIterable" | "AsyncIterator"] + ) + }) +} diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI034_PYI034.py.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI034_PYI034.py.snap new file mode 100644 index 0000000000..d1aa2e9116 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI034_PYI034.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +--- + diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI034_PYI034.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI034_PYI034.pyi.snap new file mode 100644 index 0000000000..dad1a69c83 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI034_PYI034.pyi.snap @@ -0,0 +1,101 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +--- +PYI034.pyi:18:9: PYI034 `__new__` methods in classes like `Bad` usually return `self` at runtime + | +18 | object +19 | ): # Y040 Do not inherit from "object" explicitly, as it is redundant in Python 3 +20 | def __new__( + | ^^^^^^^ PYI034 +21 | cls, *args: Any, **kwargs: Any +22 | ) -> Bad: ... # Y034 "__new__" methods usually return "self" at runtime. Consider using "typing_extensions.Self" in "Bad.__new__", e.g. "def __new__(cls, *args: Any, **kwargs: Any) -> Self: ..." + | + = help: Consider using `typing_extensions.Self` as return type + +PYI034.pyi:33:9: PYI034 `__enter__` methods in classes like `Bad` usually return `self` at runtime + | +33 | self, other: typing.Any +34 | ) -> typing.Any: ... # Y032 Prefer "object" to "Any" for the second parameter in "__ne__" methods +35 | def __enter__( + | ^^^^^^^^^ PYI034 +36 | self, +37 | ) -> Bad: ... # Y034 "__enter__" methods in classes like "Bad" usually return "self" at runtime. Consider using "typing_extensions.Self" in "Bad.__enter__", e.g. "def __enter__(self) -> Self: ..." + | + = help: Consider using `typing_extensions.Self` as return type + +PYI034.pyi:36:15: PYI034 `__aenter__` methods in classes like `Bad` usually return `self` at runtime + | +36 | self, +37 | ) -> Bad: ... # Y034 "__enter__" methods in classes like "Bad" usually return "self" at runtime. Consider using "typing_extensions.Self" in "Bad.__enter__", e.g. "def __enter__(self) -> Self: ..." +38 | async def __aenter__( + | ^^^^^^^^^^ PYI034 +39 | self, +40 | ) -> Bad: ... # Y034 "__aenter__" methods in classes like "Bad" usually return "self" at runtime. Consider using "typing_extensions.Self" in "Bad.__aenter__", e.g. "async def __aenter__(self) -> Self: ..." + | + = help: Consider using `typing_extensions.Self` as return type + +PYI034.pyi:39:9: PYI034 `__iadd__` methods in classes like `Bad` usually return `self` at runtime + | +39 | self, +40 | ) -> Bad: ... # Y034 "__aenter__" methods in classes like "Bad" usually return "self" at runtime. Consider using "typing_extensions.Self" in "Bad.__aenter__", e.g. "async def __aenter__(self) -> Self: ..." +41 | def __iadd__( + | ^^^^^^^^ PYI034 +42 | self, other: Bad +43 | ) -> Bad: ... # Y034 "__iadd__" methods in classes like "Bad" usually return "self" at runtime. Consider using "typing_extensions.Self" in "Bad.__iadd__", e.g. "def __iadd__(self, other: Bad) -> Self: ..." + | + = help: Consider using `typing_extensions.Self` as return type + +PYI034.pyi:102:9: PYI034 `__iter__` methods in classes like `BadIterator1` usually return `self` at runtime + | +102 | class BadIterator1(Iterator[int]): +103 | def __iter__( + | ^^^^^^^^ PYI034 +104 | self, +105 | ) -> Iterator[ + | + = help: Consider using `typing_extensions.Self` as return type + +PYI034.pyi:111:9: PYI034 `__iter__` methods in classes like `BadIterator2` usually return `self` at runtime + | +111 | typing.Iterator[int] +112 | ): # Y022 Use "collections.abc.Iterator[T]" instead of "typing.Iterator[T]" (PEP 585 syntax) +113 | def __iter__( + | ^^^^^^^^ PYI034 +114 | self, +115 | ) -> Iterator[ + | + = help: Consider using `typing_extensions.Self` as return type + +PYI034.pyi:120:9: PYI034 `__iter__` methods in classes like `BadIterator3` usually return `self` at runtime + | +120 | typing.Iterator[int] +121 | ): # Y022 Use "collections.abc.Iterator[T]" instead of "typing.Iterator[T]" (PEP 585 syntax) +122 | def __iter__( + | ^^^^^^^^ PYI034 +123 | self, +124 | ) -> collections.abc.Iterator[ + | + = help: Consider using `typing_extensions.Self` as return type + +PYI034.pyi:128:9: PYI034 `__iter__` methods in classes like `BadIterator4` usually return `self` at runtime + | +128 | class BadIterator4(Iterator[int]): +129 | # Note: *Iterable*, not *Iterator*, returned! +130 | def __iter__( + | ^^^^^^^^ PYI034 +131 | self, +132 | ) -> Iterable[ + | + = help: Consider using `typing_extensions.Self` as return type + +PYI034.pyi:142:9: PYI034 `__aiter__` methods in classes like `BadAsyncIterator` usually return `self` at runtime + | +142 | class BadAsyncIterator(collections.abc.AsyncIterator[str]): +143 | def __aiter__( + | ^^^^^^^^^ PYI034 +144 | self, +145 | ) -> typing.AsyncIterator[ + | + = help: Consider using `typing_extensions.Self` as return type + + diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index cacb54d571..6b873603ff 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -684,12 +684,27 @@ pub fn collect_arg_names<'a>(arguments: &'a Arguments) -> FxHashSet<&'a str> { /// callable. pub fn map_callable(decorator: &Expr) -> &Expr { if let Expr::Call(ast::ExprCall { func, .. }) = decorator { + // Ex) `@decorator()` func } else { + // Ex) `@decorator` decorator } } +/// Given an [`Expr`] that can be callable or not (like a decorator, which could +/// be used with or without explicit call syntax), return the underlying +/// callable. +pub fn map_subscript(expr: &Expr) -> &Expr { + if let Expr::Subscript(ast::ExprSubscript { value, .. }) = expr { + // Ex) `Iterable[T]` + value + } else { + // Ex) `Iterable` + expr + } +} + /// Returns `true` if a statement or expression includes at least one comment. pub fn has_comments(located: &T, locator: &Locator) -> bool where diff --git a/crates/ruff_python_semantic/src/analyze/visibility.rs b/crates/ruff_python_semantic/src/analyze/visibility.rs index b5e5ac9d25..dd921a8d26 100644 --- a/crates/ruff_python_semantic/src/analyze/visibility.rs +++ b/crates/ruff_python_semantic/src/analyze/visibility.rs @@ -89,6 +89,14 @@ pub fn is_property( }) }) } + +/// Returns `true` if a class is an `final`. +pub fn is_final(model: &SemanticModel, decorator_list: &[Expr]) -> bool { + decorator_list + .iter() + .any(|expr| model.match_typing_expr(map_callable(expr), "final")) +} + /// Returns `true` if a function is a "magic method". pub fn is_magic(name: &str) -> bool { name.starts_with("__") && name.ends_with("__") diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index cedd65cf76..63b23330db 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -93,6 +93,10 @@ impl<'a> SemanticModel<'a> { return true; } + if call_path.as_slice() == ["_typeshed", target] { + return true; + } + if TYPING_EXTENSIONS.contains(target) { if call_path.as_slice() == ["typing_extensions", target] { return true; diff --git a/ruff.schema.json b/ruff.schema.json index d4a044b393..27e689151b 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2244,6 +2244,7 @@ "PYI03", "PYI032", "PYI033", + "PYI034", "PYI04", "PYI042", "PYI043",